From mboxrd@z Thu Jan 1 00:00:00 1970 From: Subash Patel Subject: Re: [Linaro-mm-sig] [RFCv2 PATCH 4/9] v4l: add buffer exporting via dmabuf Date: Thu, 22 Mar 2012 20:29:57 +0530 Message-ID: <4F6B3E6D.20705@gmail.com> References: <1331633827-503-1-git-send-email-t.stanislaws@samsung.com> <2646632.PEZlYAVAnk@avalon> <4F6B2FAD.50900@gmail.com> <2231087.KvxzldpGDu@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-gy0-f177.google.com (mail-gy0-f177.google.com [209.85.160.177]) by gabe.freedesktop.org (Postfix) with ESMTP id D5242A0E15 for ; Thu, 22 Mar 2012 08:00:03 -0700 (PDT) Received: by ghbf11 with SMTP id f11so2061575ghb.36 for ; Thu, 22 Mar 2012 08:00:03 -0700 (PDT) In-Reply-To: <2231087.KvxzldpGDu@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Laurent Pinchart Cc: Tomasz Stanislawski , kyungmin.park@samsung.com, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, sakari.ailus@iki.fi, airlied@redhat.com List-Id: dri-devel@lists.freedesktop.org On 03/22/2012 07:37 PM, Laurent Pinchart wrote: > Hi Subash, > > On Thursday 22 March 2012 19:27:01 Subash Patel wrote: >> On 03/22/2012 04:46 PM, Laurent Pinchart wrote: >>> On Tuesday 13 March 2012 11:17:02 Tomasz Stanislawski wrote: > > [snip] > >>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h >>>> index bb6844e..e71c787 100644 >>>> --- a/include/linux/videodev2.h >>>> +++ b/include/linux/videodev2.h >>>> @@ -680,6 +680,25 @@ struct v4l2_buffer { >>>> >>>> #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800 >>>> #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000 >>>> >>>> +/** >>>> + * struct v4l2_exportbuffer - export of video buffer as DMABUF file >>>> descriptor >>>> + * >>>> + * @fd: file descriptor associated with DMABUF (set by driver) >>>> + * @mem_offset: for non-multiplanar buffers with memory == >>>> V4L2_MEMORY_MMAP; >>> >>> I don't think we will ever support exporting anything else than >>> V4L2_MEMORY_MMAP buffers. What will happen for multiplanar buffers ? >>> >>>> + * offset from the start of the device memory for this plane, >>>> + * (or a "cookie" that should be passed to mmap() as offset) >>>> + * >>> >>> Shouldn't the mem_offset field always be set to the mmap cookie value ? >>> I'm a bit confused by the "or" part, it seems to have been copied from >>> the v4l2_buffer documentation directly. We should clarify that. >>> >>>> + * Contains data used for exporting a video buffer as DMABUF file >>>> + * descriptor. Uses the same 'cookie' as mmap() syscall. All reserved >>>> fields >>>> + * must be set to zero. >>>> + */ >>>> +struct v4l2_exportbuffer { >>>> + __u32 fd; >>>> + __u32 reserved0; >>> >>> Why is there a reserved field here ? >> >> +1 to Laurent. Any particular need for reserved0 and reserved[13] below? >> I think one void user pointer is sufficient even for future need. > > I'd rather have more than one void user pointer, just in case. A couple of > bytes won't be expensive, Just an off-topic note. When I learnt to hit keyboard for programming(in linux for embedded), I had strict guidelines not to declare variables as I like, as memory and computing was very precious then. A decade later, people no more think its expensive to keep 14*3 bytes*(who knows how many dma_buf objects) in the system. Just a side note, thats all :) and they will save lots of hassle in the future if > we need to add a couple of fields. I was just wondering why there was a > reserved field between fd and mem_offset. > >>>> + __u32 mem_offset; >>>> + __u32 reserved[13]; >>>> +}; >>>> + >> >> Also, what is the reason for returning the fd through this structure? To >> keep it aligned with other v4l2 calls? I liked(or now hate making change >> in the app) how it was being returned through the ioctl in your last patch. > > Probably to be consistent with the V4L2 API, yes. It won't make a big > difference for applications, I would favor consistency in this case. >