From patchwork Thu Jun 7 14:28:45 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Subash Patel X-Patchwork-Id: 9162 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 1D36523F19 for ; Thu, 7 Jun 2012 14:29:03 +0000 (UTC) Received: from mail-yx0-f180.google.com (mail-yx0-f180.google.com [209.85.213.180]) by fiordland.canonical.com (Postfix) with ESMTP id BA8B1A18028 for ; Thu, 7 Jun 2012 14:29:02 +0000 (UTC) Received: by yenq6 with SMTP id q6so456608yen.11 for ; Thu, 07 Jun 2012 07:29:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf :dkim-signature:message-id:date:from:user-agent:mime-version:to :references:in-reply-to:content-type:cc:subject:x-beenthere :x-mailman-version:precedence:list-id:list-unsubscribe:list-archive :list-post:list-help:list-subscribe:sender:errors-to :x-gm-message-state; bh=Bz6nFKHsKI5EXC1hB+tcPWK+rWwzRrIzrPMZfs+NVDQ=; b=I7BckncdVpFyvGtogmeHiYs0iClWH/b3Ob9aqjVtOKBFQemV5nLAKiVV/OZMmE20A9 yqRS1n0yilZI5eOmwXTubrihBTBExpYcACKjXG2vqaetsPXigjbKv/hgIMoCNWCiPGUR 3cYnsAoYShflYPWIG+uTInXKRs1hZA65dfPwV5uzyz43Vs7tZAmqGp4rGVnqm+rmXJSX eyI6FXWlpp3KaWzoUGB2CmJHolSfw5UopQGv9LSaUUJDxSwUSMCDMrN6QDOUwufP0YKX 75iJcsYa73Wq4Di5ckecgFSXGfGV+KL5s/uKVXUgmFiac/jGhDfNqpK1eeyAfIUdLXrt rRnw== Received: by 10.42.210.193 with SMTP id gl1mr1539508icb.57.1339079342010; Thu, 07 Jun 2012 07:29:02 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.24.148 with SMTP id v20csp145380ibb; Thu, 7 Jun 2012 07:29:01 -0700 (PDT) Received: by 10.205.133.6 with SMTP id hw6mr1628270bkc.93.1339079340074; Thu, 07 Jun 2012 07:29:00 -0700 (PDT) Received: from mombin.canonical.com (mombin.canonical.com. [91.189.95.16]) by mx.google.com with ESMTP id fk5si2527556bkc.70.2012.06.07.07.28.58; Thu, 07 Jun 2012 07:29:00 -0700 (PDT) Received-SPF: neutral (google.com: 91.189.95.16 is neither permitted nor denied by best guess record for domain of linaro-mm-sig-bounces@lists.linaro.org) client-ip=91.189.95.16; Authentication-Results: mx.google.com; spf=neutral (google.com: 91.189.95.16 is neither permitted nor denied by best guess record for domain of linaro-mm-sig-bounces@lists.linaro.org) smtp.mail=linaro-mm-sig-bounces@lists.linaro.org; dkim=neutral (body hash did not verify) header.i=@gmail.com Received: from localhost ([127.0.0.1] helo=mombin.canonical.com) by mombin.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1Scdho-0001In-K8; Thu, 07 Jun 2012 14:28:56 +0000 Received: from mail-gh0-f170.google.com ([209.85.160.170]) by mombin.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1Scdhm-0001HW-1O for linaro-mm-sig@lists.linaro.org; Thu, 07 Jun 2012 14:28:54 +0000 Received: by ghbg2 with SMTP id g2so869389ghb.1 for ; Thu, 07 Jun 2012 07:28:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type; bh=8tZSt2A/LRrA2EPwf9H6W4BiZwHNSZcAYnjvb6hWw+Q=; b=V+yFHiCnds517QG+6ObbwcFVQo30DYZPjf3XUaXAebT8ux5rz/y8XVGJR/ec5Qq89I 3ltN+sJ8bXhaUv6kiIKxwpfzYXoUcS7UMe7ypcSJpb/ipKalNxRORXk3ObaQBVa+Diq/ Y+3ftqAbMQXpTBmHPg3aAlg6SvoM3Tn0Yc9NvI0N8H3nHtaMtfGMbJaGRnRjJw+j+7s3 eYM94vcFxXst+UtQPzIqbOKpS1iL6o6E4SlgFTCzQrm2tNm5tNo2LOF8wxjtPMsbmKx9 hTpQ30RXzacrOULxpNbe6vxptLSSXuy72Ii/wYuMb8wKYsRnPYo1xGXQsOIyXXWYgJ6H imrQ== Received: by 10.68.222.133 with SMTP id qm5mr8381010pbc.113.1339079332823; Thu, 07 Jun 2012 07:28:52 -0700 (PDT) Received: from [10.10.10.40] ([115.113.119.130]) by mx.google.com with ESMTPS id pg3sm4301335pbc.2.2012.06.07.07.28.47 (version=SSLv3 cipher=OTHER); Thu, 07 Jun 2012 07:28:51 -0700 (PDT) Message-ID: <4FD0BA9D.6010704@gmail.com> Date: Thu, 07 Jun 2012 19:58:45 +0530 From: Subash Patel User-Agent: Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 MIME-Version: 1.0 To: Laurent Pinchart References: <1337778455-27912-1-git-send-email-t.stanislaws@samsung.com> <2101386.6sj5B2hAyl@avalon> <4FCF457A.9000201@samsung.com> <3066605.JkMnQZX3Q6@avalon> In-Reply-To: <3066605.JkMnQZX3Q6@avalon> Cc: pawel@osciak.com, sumit.semwal@ti.com, mchehab@redhat.com, robdclark@gmail.com, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, kyungmin.park@samsung.com, airlied@redhat.com, remi@remlab.net, linux-media@vger.kernel.org, g.liakhovetski@gmx.de Subject: Re: [Linaro-mm-sig] [PATCH 04/12] v4l: vb2-dma-contig: add setup of sglist for MMAP buffers X-BeenThere: linaro-mm-sig@lists.linaro.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Unified memory management interest group." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linaro-mm-sig-bounces@lists.linaro.org Errors-To: linaro-mm-sig-bounces@lists.linaro.org X-Gm-Message-State: ALoCoQmatDQzd9Cj3PTUHqWq7THXTrr2qW5XmZ5jjvrq0Zza0RVf2YjNZ2n00RX06sQYISahEdJF Hello Tomasz, On 06/07/2012 06:06 AM, Laurent Pinchart wrote: > Hi Tomasz, > > On Wednesday 06 June 2012 13:56:42 Tomasz Stanislawski wrote: >> On 06/06/2012 10:06 AM, Laurent Pinchart wrote: >>> On Wednesday 23 May 2012 15:07:27 Tomasz Stanislawski wrote: >>>> This patch adds the setup of sglist list for MMAP buffers. >>>> It is needed for buffer exporting via DMABUF mechanism. >>>> >>>> Signed-off-by: Tomasz Stanislawski >>>> Signed-off-by: Kyungmin Park >>>> --- >>>> >>>> drivers/media/video/videobuf2-dma-contig.c | 70 +++++++++++++++++++++- >>>> 1 file changed, 68 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/media/video/videobuf2-dma-contig.c >>>> b/drivers/media/video/videobuf2-dma-contig.c index 52b4f59..ae656be >>>> 100644 >>>> --- a/drivers/media/video/videobuf2-dma-contig.c >>>> +++ b/drivers/media/video/videobuf2-dma-contig.c > > [snip] > >>>> +static int vb2_dc_kaddr_to_pages(unsigned long kaddr, >>>> + struct page **pages, unsigned int n_pages) >>>> +{ >>>> + unsigned int i; >>>> + unsigned long pfn; >>>> + struct vm_area_struct vma = { >>>> + .vm_flags = VM_IO | VM_PFNMAP, >>>> + .vm_mm = current->mm, >>>> + }; >>>> + >>>> + for (i = 0; i< n_pages; ++i, kaddr += PAGE_SIZE) { >>> >>> The follow_pfn() kerneldoc mentions that it looks up a PFN for a user >>> address. The only users I've found in the kernel sources pass a user >>> address. Is it legal to use it for kernel addresses ? >> >> It is not completely legal :). As I understand the mm code, the follow_pfn >> works only for IO/PFN mappings. This is the typical case (every case?) of >> mappings created by dma_alloc_coherent. >> >> In order to make this function work for a kernel pointer, one has to create >> an artificial VMA that has IO/PFN bits on. >> >> This solution is a hack-around for dma_get_pages (aka dma_get_sgtable). This >> way the dependency on dma_get_pages was broken giving a small hope of >> merging vb2 exporting. >> >> Marek prepared a patchset 'ARM: DMA-mapping: new extensions for buffer >> sharing' that adds dma buffers with no kernel mappings and dma_get_sgtable >> function. >> >> However this patchset is still in a RFC state. > > That's totally understood :-) I'm fine with keeping the hack for now until the > dma_get_sgtable() gets in a usable/mergeable state, please just mention it in > the code with something like > > /* HACK: This is a temporary workaround until the dma_get_sgtable() function > becomes available. */ > >> I have prepared a patch that removes vb2_dc_kaddr_to_pages and substitutes >> it with dma_get_pages. It will become a part of vb2-exporter patches just >> after dma_get_sgtable is merged (or at least acked by major maintainers). > The above function call (because of follow_pfn) doesn't succeed for all the allocated pages. Hence I created a patch(attached) which is based on [1] series. One can apply it for using your present patch-set in the meantime. Regards, Subash [1] http://www.spinics.net/lists/kernel/msg1343092.html >From f9b2eace2eef0038a1830e5e6dd55f3bb6017e1a Mon Sep 17 00:00:00 2001 From: Subash Patel Date: Thu, 7 Jun 2012 19:49:10 +0530 Subject: [PATCH] v4l2: vb2: use dma_get_sgtable This is patch to remove vb2_dc_kaddr_to_pages() function with dma_get_sgtable() in the patch set posted by Tomasz. It was observed that the former function fails to get the pages, as follow_pfn() can fail for remapped kernel va provided by the dma-mapping sub-system. As Tomasz mentioned, the later call was temporary patch until dma-mapping author finalizes the implementation of dma_get_sgtable(). One can use this patch to use this vb2 patch-set for his/her work in the meantime. Signed-off-by: Subash Patel --- drivers/media/video/videobuf2-dma-contig.c | 53 ++------------------------- 1 files changed, 4 insertions(+), 49 deletions(-) diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index e8da7f1..1b9023a 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -143,32 +143,11 @@ static void vb2_dc_put(void *buf_priv) kfree(buf); } -static int vb2_dc_kaddr_to_pages(unsigned long kaddr, - struct page **pages, unsigned int n_pages) -{ - unsigned int i; - unsigned long pfn; - struct vm_area_struct vma = { - .vm_flags = VM_IO | VM_PFNMAP, - .vm_mm = current->mm, - }; - - for (i = 0; i < n_pages; ++i, kaddr += PAGE_SIZE) { - if (follow_pfn(&vma, kaddr, &pfn)) - break; - pages[i] = pfn_to_page(pfn); - } - - return i; -} - static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size) { struct device *dev = alloc_ctx; struct vb2_dc_buf *buf; int ret = -ENOMEM; - int n_pages; - struct page **pages = NULL; buf = kzalloc(sizeof *buf, GFP_KERNEL); if (!buf) @@ -183,35 +162,14 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size) WARN_ON((unsigned long)buf->vaddr & ~PAGE_MASK); WARN_ON(buf->dma_addr & ~PAGE_MASK); - n_pages = PAGE_ALIGN(size) >> PAGE_SHIFT; + ret = dma_get_sgtable(dev, &buf->sgt_base, buf->vaddr, + buf->dma_addr, size, NULL); - pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL); - if (!pages) { - dev_err(dev, "failed to alloc page table\n"); - goto fail_dma; - } - - ret = vb2_dc_kaddr_to_pages((unsigned long)buf->vaddr, pages, n_pages); if (ret < 0) { - dev_err(dev, "failed to get buffer pages from DMA API\n"); - goto fail_pages; - } - if (ret != n_pages) { - ret = -EFAULT; - dev_err(dev, "failed to get all pages from DMA API\n"); - goto fail_pages; - } - - ret = sg_alloc_table_from_pages(&buf->sgt_base, - pages, n_pages, 0, size, GFP_KERNEL); - if (ret) { - dev_err(dev, "failed to prepare sg table\n"); - goto fail_pages; + dev_err(dev, "failed to get the SGT for the allocated pages\n"); + goto fail_dma; } - /* pages are no longer needed */ - kfree(pages); - buf->dev = dev; buf->size = size; @@ -223,9 +181,6 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size) return buf; -fail_pages: - kfree(pages); - fail_dma: dma_free_coherent(dev, size, buf->vaddr, buf->dma_addr); -- 1.7.5.4