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, 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.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
