Sumit Semwal | 5 Jan 2012 11:41
Picon
Favicon

[RFCv1 0/4] v4l: DMA buffer sharing support as a user

Hello Everyone,

A very happy new year 2012! :)

This patchset is an RFC for the way videobuf2 can be adapted to add support for
DMA buffer sharing framework[1].

The original patch-set for the idea, and PoC of buffer sharing was by 
Tomasz Stanislawski <t.stanislaws <at> samsung.com>, who demonstrated buffer sharing
between two v4l2 devices[2]. This RFC is needed to adapt these patches to the
changes that have happened in the DMA buffer sharing framework over past few
months.

To begin with, I have tried to adapt only the dma-contig allocator, and only as
a user of dma-buf buffer. I am currently working on the v4l2-as-an-exporter
changes, and will share as soon as I get it in some shape.

As with the PoC [2], the handle for sharing buffers is a file-descriptor (fd).
The usage documentation is also a part of [1].

So, the current RFC has the following limitations:
- Only buffer sharing as a buffer user,
- doesn't handle cases where even for a contiguous buffer, the sg_table can have
   more than one scatterlist entry.

Thanks and best regards,
~Sumit.

[1]: dma-buf patchset at: https://lkml.org/lkml/2011/12/26/29
[2]: http://lwn.net/Articles/454389
(Continue reading)

Sumit Semwal | 5 Jan 2012 11:41
Picon
Favicon

[RFCv1 1/4] v4l: Add DMABUF as a memory type

Adds DMABUF memory type to v4l framework. Also adds the related file
descriptor in v4l2_plane and v4l2_buffer.

Signed-off-by: Tomasz Stanislawski <t.stanislaws <at> samsung.com>
   [original work in the PoC for buffer sharing]
Signed-off-by: Sumit Semwal <sumit.semwal <at> ti.com>
Signed-off-by: Sumit Semwal <sumit.semwal <at> linaro.org>
---
 include/linux/videodev2.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 4b752d5..3c0ade1 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
 <at>  <at>  -185,6 +185,7  <at>  <at>  enum v4l2_memory {
 	V4L2_MEMORY_MMAP             = 1,
 	V4L2_MEMORY_USERPTR          = 2,
 	V4L2_MEMORY_OVERLAY          = 3,
+	V4L2_MEMORY_DMABUF           = 4,
 };

 /* see also http://vektor.theorem.ca/graphics/ycbcr/ */
 <at>  <at>  -574,6 +575,8  <at>  <at>  struct v4l2_requestbuffers {
  *			should be passed to mmap() called on the video node)
  *  <at> userptr:		when memory is V4L2_MEMORY_USERPTR, a userspace pointer
  *			pointing to this plane
+ *  <at> fd:			when memory is V4L2_MEMORY_DMABUF, a userspace file
+ *			descriptor associated with this plane
  *  <at> data_offset:	offset in the plane to the start of data; usually 0,
(Continue reading)

Sumit Semwal | 5 Jan 2012 11:41
Picon
Favicon

[RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

This patch adds support for DMABUF memory type in videobuf2. It calls relevant
APIs of dma_buf for v4l reqbuf / qbuf / dqbuf operations.

For this version, the support is for videobuf2 as a user of the shared buffer;
so the allocation of the buffer is done outside of V4L2. [A sample allocator of
dma-buf shared buffer is given at [1]]

[1]: Rob Clark's DRM:
   https://github.com/robclark/kernel-omap4/commits/drmplane-dmabuf

Signed-off-by: Tomasz Stanislawski <t.stanislaws <at> samsung.com>
   [original work in the PoC for buffer sharing]
Signed-off-by: Sumit Semwal <sumit.semwal <at> ti.com>
Signed-off-by: Sumit Semwal <sumit.semwal <at> linaro.org>
---
 drivers/media/video/videobuf2-core.c |  186 +++++++++++++++++++++++++++++++++-
 include/media/videobuf2-core.h       |   30 ++++++
 2 files changed, 215 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 95a3f5e..6cd2f97 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
 <at>  <at>  -107,6 +107,27  <at>  <at>  static void __vb2_buf_userptr_put(struct vb2_buffer *vb)
 }

 /**
+ * __vb2_buf_dmabuf_put() - release memory associated with
+ * a DMABUF shared buffer
+ */
(Continue reading)

Sakari Ailus | 14 Jan 2012 21:38
Picon
Picon
Favicon

Re: [Linaro-mm-sig] [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Sumit,

Thanks for the patch!

Sumit Semwal wrote:
...
>  <at>  <at>  -962,6 +1030,109  <at>  <at>  static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>  }
>  
>  /**
> + * __qbuf_dmabuf() - handle qbuf of a DMABUF buffer
> + */
> +static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
> +{
> +	struct v4l2_plane planes[VIDEO_MAX_PLANES];
> +	struct vb2_queue *q = vb->vb2_queue;
> +	void *mem_priv;
> +	unsigned int plane;
> +	int ret;
> +	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
> +
> +	/* Verify and copy relevant information provided by the userspace */
> +	ret = __fill_vb2_buffer(vb, b, planes);
> +	if (ret)
> +		return ret;
> +
> +	for (plane = 0; plane < vb->num_planes; ++plane) {
> +		struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
> +
> +		if (IS_ERR_OR_NULL(dbuf)) {
(Continue reading)

Pawel Osciak | 19 Jan 2012 20:07

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Sumit,
Thank you for your work. Please find my comments below.

On Thu, Jan 5, 2012 at 2:41 AM, Sumit Semwal <sumit.semwal <at> ti.com> wrote:
> This patch adds support for DMABUF memory type in videobuf2. It calls relevant
> APIs of dma_buf for v4l reqbuf / qbuf / dqbuf operations.
>
> For this version, the support is for videobuf2 as a user of the shared buffer;
> so the allocation of the buffer is done outside of V4L2. [A sample allocator of
> dma-buf shared buffer is given at [1]]
>
> [1]: Rob Clark's DRM:
>   https://github.com/robclark/kernel-omap4/commits/drmplane-dmabuf
>
> Signed-off-by: Tomasz Stanislawski <t.stanislaws <at> samsung.com>
>   [original work in the PoC for buffer sharing]
> Signed-off-by: Sumit Semwal <sumit.semwal <at> ti.com>
> Signed-off-by: Sumit Semwal <sumit.semwal <at> linaro.org>
> ---
>  drivers/media/video/videobuf2-core.c |  186 +++++++++++++++++++++++++++++++++-
>  include/media/videobuf2-core.h       |   30 ++++++
>  2 files changed, 215 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
> index 95a3f5e..6cd2f97 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
>  <at>  <at>  -107,6 +107,27  <at>  <at>  static void __vb2_buf_userptr_put(struct vb2_buffer *vb)
>  }
>
(Continue reading)

Sumit Semwal | 20 Jan 2012 11:41
Favicon

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On 20 January 2012 00:37, Pawel Osciak <pawel <at> osciak.com> wrote:
> Hi Sumit,
> Thank you for your work. Please find my comments below.
Hi Pawel,

Thank you for finding time for this review, and your comments :) - my
comments inline.
[Also, as an aside, Tomasz has also been working on the vb2 adaptation
to dma-buf, and his patches should be more comprehensive, in that he
is also planning to include 'vb2 as exporter' of dma-buf. He might
take and improve on this RFC, so it might be worthwhile to wait for
it?]
>
<snip>
>>  * __setup_offsets() - setup unique offsets ("cookies") for every plane in
>>  * every buffer on the queue
>>  */
>>  <at>  <at>  -228,6 +249,8  <at>  <at>  static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers)
>>                /* Free MMAP buffers or release USERPTR buffers */
>>                if (q->memory == V4L2_MEMORY_MMAP)
>>                        __vb2_buf_mem_free(vb);
>> +               if (q->memory == V4L2_MEMORY_DMABUF)
>> +                       __vb2_buf_dmabuf_put(vb);
>>                else
>>                        __vb2_buf_userptr_put(vb);
>
> This looks like a bug. If memory is MMAP, you'd __vb2_buf_mem_free(vb)
> AND __vb2_buf_userptr_put(vb), which is wrong. Have you tested MMAP
> and USERPTR with those patches applied?
>
(Continue reading)

Tomasz Stanislawski | 20 Jan 2012 11:58

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)


Hi Sumit and Pawel,
Please find comments below.
On 01/20/2012 11:41 AM, Sumit Semwal wrote:
> On 20 January 2012 00:37, Pawel Osciak<pawel <at> osciak.com>  wrote:
>> Hi Sumit,
>> Thank you for your work. Please find my comments below.
> Hi Pawel,
>
> Thank you for finding time for this review, and your comments :) - my
> comments inline.
> [Also, as an aside, Tomasz has also been working on the vb2 adaptation
> to dma-buf, and his patches should be more comprehensive, in that he
> is also planning to include 'vb2 as exporter' of dma-buf. He might
> take and improve on this RFC, so it might be worthwhile to wait for
> it?]
>>

<snip>

>>>   struct vb2_mem_ops {
>>>         void            *(*alloc)(void *alloc_ctx, unsigned long size);
>>>  <at>  <at>  -65,6 +82,16  <at>  <at>  struct vb2_mem_ops {
>>>                                         unsigned long size, int write);
>>>         void            (*put_userptr)(void *buf_priv);
>>>
>>> +       /* Comment from Rob Clark: XXX: I think the attach / detach could be handled
>>> +        * in the vb2 core, and vb2_mem_ops really just need to get/put the
>>> +        * sglist (and make sure that the sglist fits it's needs..)
>>> +        */
(Continue reading)

Laurent Pinchart | 20 Jan 2012 16:12

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Tomasz,

On Friday 20 January 2012 11:58:39 Tomasz Stanislawski wrote:
> On 01/20/2012 11:41 AM, Sumit Semwal wrote:
> > On 20 January 2012 00:37, Pawel Osciak<pawel <at> osciak.com>  wrote:
> >> Hi Sumit,
> >> Thank you for your work. Please find my comments below.
> > 
> > Hi Pawel,
> > 
> > Thank you for finding time for this review, and your comments :) - my
> > comments inline.
> > [Also, as an aside, Tomasz has also been working on the vb2 adaptation
> > to dma-buf, and his patches should be more comprehensive, in that he
> > is also planning to include 'vb2 as exporter' of dma-buf. He might
> > take and improve on this RFC, so it might be worthwhile to wait for
> > it?]
> 
> <snip>
> 
> >>>   struct vb2_mem_ops {
> >>>   
> >>>         void            *(*alloc)(void *alloc_ctx, unsigned long size);
> >>> 
> >>>  <at>  <at>  -65,6 +82,16  <at>  <at>  struct vb2_mem_ops {
> >>> 
> >>>                                         unsigned long size, int write);
> >>>         
> >>>         void            (*put_userptr)(void *buf_priv);
> >>> 
(Continue reading)

Tomasz Stanislawski | 20 Jan 2012 16:53

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Laurent,

On 01/20/2012 04:12 PM, Laurent Pinchart wrote:
> Hi Tomasz,
>
> On Friday 20 January 2012 11:58:39 Tomasz Stanislawski wrote:
>> On 01/20/2012 11:41 AM, Sumit Semwal wrote:
>>> On 20 January 2012 00:37, Pawel Osciak<pawel <at> osciak.com>   wrote:
>>>> Hi Sumit,
>>>> Thank you for your work. Please find my comments below.
>>>
>>> Hi Pawel,
>>>
<snip>
>>>>>    struct vb2_mem_ops {
>>>>>
>>>>>          void            *(*alloc)(void *alloc_ctx, unsigned long size);
>>>>>
>>>>>  <at>  <at>  -65,6 +82,16  <at>  <at>  struct vb2_mem_ops {
>>>>>
>>>>>                                          unsigned long size, int write);
>>>>>
>>>>>          void            (*put_userptr)(void *buf_priv);
>>>>>
>>>>> +       /* Comment from Rob Clark: XXX: I think the attach / detach
>>>>> could be handled +        * in the vb2 core, and vb2_mem_ops really
>>>>> just need to get/put the +        * sglist (and make sure that the
>>>>> sglist fits it's needs..) +        */
>>>>
>>>> I *strongly* agree with Rob here. Could you explain the reason behind
(Continue reading)

Laurent Pinchart | 20 Jan 2012 17:11

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Tomasz,

On Friday 20 January 2012 16:53:20 Tomasz Stanislawski wrote:
> On 01/20/2012 04:12 PM, Laurent Pinchart wrote:
> > On Friday 20 January 2012 11:58:39 Tomasz Stanislawski wrote:
> >> On 01/20/2012 11:41 AM, Sumit Semwal wrote:
> >>> On 20 January 2012 00:37, Pawel Osciak<pawel <at> osciak.com>   wrote:
> >>>> Hi Sumit,
> >>>> Thank you for your work. Please find my comments below.
> >>> 
> >>> Hi Pawel,
> 
> <snip>
> 
> >>>>>    struct vb2_mem_ops {
> >>>>>    
> >>>>>          void            *(*alloc)(void *alloc_ctx, unsigned long
> >>>>>          size);
> >>>>> 
> >>>>>  <at>  <at>  -65,6 +82,16  <at>  <at>  struct vb2_mem_ops {
> >>>>> 
> >>>>>                                          unsigned long size, int
> >>>>>                                          write);
> >>>>>          
> >>>>>          void            (*put_userptr)(void *buf_priv);
> >>>>> 
> >>>>> +       /* Comment from Rob Clark: XXX: I think the attach / detach
> >>>>> could be handled +        * in the vb2 core, and vb2_mem_ops really
> >>>>> just need to get/put the +        * sglist (and make sure that the
> >>>>> sglist fits it's needs..) +        */
(Continue reading)

Tomasz Stanislawski | 20 Jan 2012 17:20

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

>>
>> IMO, One way to do this is adding field 'struct device *dev' to struct
>> vb2_queue. This field should be filled by a driver prior to calling
>> vb2_queue_init.
>
> I haven't looked into the details, but that sounds good to me. Do we have use
> cases where a queue is allocated before knowing which physical device it will
> be used for ?
>

I don't think so. In case of S5P drivers, vb2_queue_init is called while 
opening /dev/videoX.

BTW. This struct device may help vb2 to produce logs with more 
descriptive client annotation.

What happens if such a device is NULL. It would happen for vmalloc 
allocator used by VIVI?

Regards,
Tomasz Stanislawski
Laurent Pinchart | 20 Jan 2012 17:28

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
> >> IMO, One way to do this is adding field 'struct device *dev' to struct
> >> vb2_queue. This field should be filled by a driver prior to calling
> >> vb2_queue_init.
> > 
> > I haven't looked into the details, but that sounds good to me. Do we have
> > use cases where a queue is allocated before knowing which physical
> > device it will be used for ?
> 
> I don't think so. In case of S5P drivers, vb2_queue_init is called while
> opening /dev/videoX.
> 
> BTW. This struct device may help vb2 to produce logs with more
> descriptive client annotation.
> 
> What happens if such a device is NULL. It would happen for vmalloc
> allocator used by VIVI?

Good question. Should dma-buf accept NULL devices ? Or should vivi pass its 
V4L2 device to vb2 ?

--

-- 
Regards,

Laurent Pinchart
Marek Szyprowski | 23 Jan 2012 10:06

RE: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hello,

On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote:

> On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
> > >> IMO, One way to do this is adding field 'struct device *dev' to struct
> > >> vb2_queue. This field should be filled by a driver prior to calling
> > >> vb2_queue_init.
> > >
> > > I haven't looked into the details, but that sounds good to me. Do we have
> > > use cases where a queue is allocated before knowing which physical
> > > device it will be used for ?
> >
> > I don't think so. In case of S5P drivers, vb2_queue_init is called while
> > opening /dev/videoX.
> >
> > BTW. This struct device may help vb2 to produce logs with more
> > descriptive client annotation.
> >
> > What happens if such a device is NULL. It would happen for vmalloc
> > allocator used by VIVI?
> 
> Good question. Should dma-buf accept NULL devices ? Or should vivi pass its
> V4L2 device to vb2 ?

I assume you suggested using struct video_device->dev entry in such case. 
It will not work. DMA-mapping API requires some parameters to be set for the 
client device, like for example dma mask. struct video_device contains only an
artificial struct device entry, which has no relation to any physical device 
and cannot be used for calling DMA-mapping functions.
(Continue reading)

Daniel Vetter | 23 Jan 2012 10:40
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Mon, Jan 23, 2012 at 10:06:57AM +0100, Marek Szyprowski wrote:
> Hello,
> 
> On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote:
> 
> > On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
> > > >> IMO, One way to do this is adding field 'struct device *dev' to struct
> > > >> vb2_queue. This field should be filled by a driver prior to calling
> > > >> vb2_queue_init.
> > > >
> > > > I haven't looked into the details, but that sounds good to me. Do we have
> > > > use cases where a queue is allocated before knowing which physical
> > > > device it will be used for ?
> > >
> > > I don't think so. In case of S5P drivers, vb2_queue_init is called while
> > > opening /dev/videoX.
> > >
> > > BTW. This struct device may help vb2 to produce logs with more
> > > descriptive client annotation.
> > >
> > > What happens if such a device is NULL. It would happen for vmalloc
> > > allocator used by VIVI?
> > 
> > Good question. Should dma-buf accept NULL devices ? Or should vivi pass its
> > V4L2 device to vb2 ?
> 
> I assume you suggested using struct video_device->dev entry in such case. 
> It will not work. DMA-mapping API requires some parameters to be set for the 
> client device, like for example dma mask. struct video_device contains only an
> artificial struct device entry, which has no relation to any physical device 
(Continue reading)

Daniel Vetter | 23 Jan 2012 10:45
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Mon, Jan 23, 2012 at 10:40:07AM +0100, Daniel Vetter wrote:
> On Mon, Jan 23, 2012 at 10:06:57AM +0100, Marek Szyprowski wrote:
> > Hello,
> > 
> > On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote:
> > 
> > > On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
> > > > >> IMO, One way to do this is adding field 'struct device *dev' to struct
> > > > >> vb2_queue. This field should be filled by a driver prior to calling
> > > > >> vb2_queue_init.
> > > > >
> > > > > I haven't looked into the details, but that sounds good to me. Do we have
> > > > > use cases where a queue is allocated before knowing which physical
> > > > > device it will be used for ?
> > > >
> > > > I don't think so. In case of S5P drivers, vb2_queue_init is called while
> > > > opening /dev/videoX.
> > > >
> > > > BTW. This struct device may help vb2 to produce logs with more
> > > > descriptive client annotation.
> > > >
> > > > What happens if such a device is NULL. It would happen for vmalloc
> > > > allocator used by VIVI?
> > > 
> > > Good question. Should dma-buf accept NULL devices ? Or should vivi pass its
> > > V4L2 device to vb2 ?
> > 
> > I assume you suggested using struct video_device->dev entry in such case. 
> > It will not work. DMA-mapping API requires some parameters to be set for the 
> > client device, like for example dma mask. struct video_device contains only an
(Continue reading)

Laurent Pinchart | 23 Jan 2012 10:48

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Marek,

On Monday 23 January 2012 10:06:57 Marek Szyprowski wrote:
> On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote:
> > On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
> > > >> IMO, One way to do this is adding field 'struct device *dev' to
> > > >> struct vb2_queue. This field should be filled by a driver prior to
> > > >> calling vb2_queue_init.
> > > > 
> > > > I haven't looked into the details, but that sounds good to me. Do we
> > > > have use cases where a queue is allocated before knowing which
> > > > physical device it will be used for ?
> > > 
> > > I don't think so. In case of S5P drivers, vb2_queue_init is called
> > > while opening /dev/videoX.
> > > 
> > > BTW. This struct device may help vb2 to produce logs with more
> > > descriptive client annotation.
> > > 
> > > What happens if such a device is NULL. It would happen for vmalloc
> > > allocator used by VIVI?
> > 
> > Good question. Should dma-buf accept NULL devices ? Or should vivi pass
> > its V4L2 device to vb2 ?
> 
> I assume you suggested using struct video_device->dev entry in such case.
> It will not work. DMA-mapping API requires some parameters to be set for
> the client device, like for example dma mask. struct video_device contains
> only an artificial struct device entry, which has no relation to any
> physical device and cannot be used for calling DMA-mapping functions.
(Continue reading)

Daniel Vetter | 23 Jan 2012 11:35
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Mon, Jan 23, 2012 at 10:48, Laurent Pinchart
<laurent.pinchart <at> ideasonboard.com> wrote:
> Hi Marek,
>
> On Monday 23 January 2012 10:06:57 Marek Szyprowski wrote:
>> On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote:
>> > On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
>> > > >> IMO, One way to do this is adding field 'struct device *dev' to
>> > > >> struct vb2_queue. This field should be filled by a driver prior to
>> > > >> calling vb2_queue_init.
>> > > >
>> > > > I haven't looked into the details, but that sounds good to me. Do we
>> > > > have use cases where a queue is allocated before knowing which
>> > > > physical device it will be used for ?
>> > >
>> > > I don't think so. In case of S5P drivers, vb2_queue_init is called
>> > > while opening /dev/videoX.
>> > >
>> > > BTW. This struct device may help vb2 to produce logs with more
>> > > descriptive client annotation.
>> > >
>> > > What happens if such a device is NULL. It would happen for vmalloc
>> > > allocator used by VIVI?
>> >
>> > Good question. Should dma-buf accept NULL devices ? Or should vivi pass
>> > its V4L2 device to vb2 ?
>>
>> I assume you suggested using struct video_device->dev entry in such case.
>> It will not work. DMA-mapping API requires some parameters to be set for
>> the client device, like for example dma mask. struct video_device contains
(Continue reading)

Laurent Pinchart | 23 Jan 2012 11:54

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Daniel,

On Monday 23 January 2012 11:35:01 Daniel Vetter wrote:
> On Mon, Jan 23, 2012 at 10:48, Laurent Pinchart wrote:
> > On Monday 23 January 2012 10:06:57 Marek Szyprowski wrote:
> >> On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote:
> >> > On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
> >> > > >> IMO, One way to do this is adding field 'struct device *dev' to
> >> > > >> struct vb2_queue. This field should be filled by a driver prior
> >> > > >> to calling vb2_queue_init.
> >> > > > 
> >> > > > I haven't looked into the details, but that sounds good to me. Do
> >> > > > we have use cases where a queue is allocated before knowing which
> >> > > > physical device it will be used for ?
> >> > > 
> >> > > I don't think so. In case of S5P drivers, vb2_queue_init is called
> >> > > while opening /dev/videoX.
> >> > > 
> >> > > BTW. This struct device may help vb2 to produce logs with more
> >> > > descriptive client annotation.
> >> > > 
> >> > > What happens if such a device is NULL. It would happen for vmalloc
> >> > > allocator used by VIVI?
> >> > 
> >> > Good question. Should dma-buf accept NULL devices ? Or should vivi
> >> > pass its V4L2 device to vb2 ?
> >> 
> >> I assume you suggested using struct video_device->dev entry in such
> >> case. It will not work. DMA-mapping API requires some parameters to be
> >> set for the client device, like for example dma mask. struct
(Continue reading)

Clark, Rob | 24 Jan 2012 01:26
Picon
Favicon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Mon, Jan 23, 2012 at 4:54 AM, Laurent Pinchart
<laurent.pinchart <at> ideasonboard.com> wrote:
> Hi Daniel,
>
> On Monday 23 January 2012 11:35:01 Daniel Vetter wrote:
>> On Mon, Jan 23, 2012 at 10:48, Laurent Pinchart wrote:
>> > On Monday 23 January 2012 10:06:57 Marek Szyprowski wrote:
>> >> On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote:
>> >> > On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
>> >> > > >> IMO, One way to do this is adding field 'struct device *dev' to
>> >> > > >> struct vb2_queue. This field should be filled by a driver prior
>> >> > > >> to calling vb2_queue_init.
>> >> > > >
>> >> > > > I haven't looked into the details, but that sounds good to me. Do
>> >> > > > we have use cases where a queue is allocated before knowing which
>> >> > > > physical device it will be used for ?
>> >> > >
>> >> > > I don't think so. In case of S5P drivers, vb2_queue_init is called
>> >> > > while opening /dev/videoX.
>> >> > >
>> >> > > BTW. This struct device may help vb2 to produce logs with more
>> >> > > descriptive client annotation.
>> >> > >
>> >> > > What happens if such a device is NULL. It would happen for vmalloc
>> >> > > allocator used by VIVI?
>> >> >
>> >> > Good question. Should dma-buf accept NULL devices ? Or should vivi
>> >> > pass its V4L2 device to vb2 ?
>> >>
>> >> I assume you suggested using struct video_device->dev entry in such
(Continue reading)

Laurent Pinchart | 24 Jan 2012 10:34

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Rob,

On Tuesday 24 January 2012 01:26:15 Clark, Rob wrote:
> On Mon, Jan 23, 2012 at 4:54 AM, Laurent Pinchart wrote:
> > On Monday 23 January 2012 11:35:01 Daniel Vetter wrote:
> >> On Mon, Jan 23, 2012 at 10:48, Laurent Pinchart wrote:
> >> > On Monday 23 January 2012 10:06:57 Marek Szyprowski wrote:
> >> >> On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote:
> >> >> > On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
> >> >> > > >> IMO, One way to do this is adding field 'struct device *dev'
> >> >> > > >> to struct vb2_queue. This field should be filled by a driver
> >> >> > > >> prior to calling vb2_queue_init.
> >> >> > > > 
> >> >> > > > I haven't looked into the details, but that sounds good to me.
> >> >> > > > Do we have use cases where a queue is allocated before knowing
> >> >> > > > which physical device it will be used for ?
> >> >> > > 
> >> >> > > I don't think so. In case of S5P drivers, vb2_queue_init is
> >> >> > > called while opening /dev/videoX.
> >> >> > > 
> >> >> > > BTW. This struct device may help vb2 to produce logs with more
> >> >> > > descriptive client annotation.
> >> >> > > 
> >> >> > > What happens if such a device is NULL. It would happen for
> >> >> > > vmalloc allocator used by VIVI?
> >> >> > 
> >> >> > Good question. Should dma-buf accept NULL devices ? Or should vivi
> >> >> > pass its V4L2 device to vb2 ?
> >> >> 
> >> >> I assume you suggested using struct video_device->dev entry in such
(Continue reading)

Daniel Vetter | 24 Jan 2012 11:52
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Tue, Jan 24, 2012 at 10:34:38AM +0100, Laurent Pinchart wrote:
> > > I'm not sure I would like a callback approach. If we add a sync
> > > operation, the exporter could signal to the importer that it must unmap
> > > the buffer by returning an appropriate value from the sync operation.
> > > Would that be usable for DRM ?
> > 
> > It does seem a bit over-complicated..  and deadlock prone.  Is there a
> > reason the importer couldn't just unmap when DMA is completed, and the
> > exporter give some hint on next map() that the buffer hasn't actually
> > moved?
> 
> If the importer unmaps the buffer completely when DMA is completed, it will 
> have to map it again for the next DMA transfer, even if the 
> dma_buf_map_attachement() calls returns an hint that the buffer hasn't moved.
> 
> What I want to avoid here is having to map/unmap buffers to the device IOMMU 
> around each DMA if the buffer doesn't move. To avoid that, the importer needs 
> to keep the IOMMU mapping after DMA completes if the buffer won't move until 
> the next DMA transfer, but that information isn't available when the first DMA 
> completes.

The exporter should cache the iommu mapping for all attachments. The
driver would the only need to adjust the dma address - I was kinda hoping
this would be little enough overhead that we could just ignore it, at
least for the moment (especially for hw that can only deal with contigious
buffers).

The issue with adding explicit support for evicting buffers is that it's
hilariously deadlock-prone, especially when both sides are exporters (dev1
waits for dev2 to finish processing buffer A while dev2 waits for dev1 to
(Continue reading)

Daniel Vetter | 24 Jan 2012 14:03
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Mon, Jan 23, 2012 at 11:54:20AM +0100, Laurent Pinchart wrote:
> On Monday 23 January 2012 11:35:01 Daniel Vetter wrote:
> > See my other mail, dma_buf v1 does not support cpu access.
> 
> v1 is in the kernel now, let's start discussing v2 ;-)

Ok, I'm in ;-)

I've thought a bit about this, and I think a reasonable next step would be
to enable cpu access from kernelspace. Userspace and mmap support is a
hole different beast altogether and I think we should postpone that until
we've got the kernel part hashed out.

I'm thinking about adding 3 pairs of function to dma_buf (not to
dma_buf_attachment).

dma_buf_get_backing_storage/put_backing_storage
This will be used before/after kernel cpu access to ensure that the
backing storage is in memory. E.g. gem objects can be swapped out, so
they need to be pinned before we can access them. For exporters with
static allocations this would be a no-op.

I think a start, length range would make sense, but the exporter is free
to just swap in the entire object unconditionally. The range is specified
in multiples of PAGE_SIZE - I don't think there's any usescase for a
get/put_backing_storage which deals in smaller units.

The get/put functions are allowed to block and grab all kinds of looks.
get is allowed to fail with e.g. -ENOMEM.

(Continue reading)

Daniel Vetter | 25 Jan 2012 09:09
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Tue, Jan 24, 2012 at 02:03:22PM +0100, Daniel Vetter wrote:
> On Mon, Jan 23, 2012 at 11:54:20AM +0100, Laurent Pinchart wrote:
> > On Monday 23 January 2012 11:35:01 Daniel Vetter wrote:
> > > See my other mail, dma_buf v1 does not support cpu access.
> > 
> > v1 is in the kernel now, let's start discussing v2 ;-)
> 
> Ok, I'm in ;-)
> 
> I've thought a bit about this, and I think a reasonable next step would be
> to enable cpu access from kernelspace. Userspace and mmap support is a
> hole different beast altogether and I think we should postpone that until
> we've got the kernel part hashed out.
> 
> I'm thinking about adding 3 pairs of function to dma_buf (not to
> dma_buf_attachment).
> 
> dma_buf_get_backing_storage/put_backing_storage
> This will be used before/after kernel cpu access to ensure that the
> backing storage is in memory. E.g. gem objects can be swapped out, so
> they need to be pinned before we can access them. For exporters with
> static allocations this would be a no-op.
> 
> I think a start, length range would make sense, but the exporter is free
> to just swap in the entire object unconditionally. The range is specified
> in multiples of PAGE_SIZE - I don't think there's any usescase for a
> get/put_backing_storage which deals in smaller units.
> 
> The get/put functions are allowed to block and grab all kinds of looks.
> get is allowed to fail with e.g. -ENOMEM.
(Continue reading)

Laurent Pinchart | 30 Jan 2012 15:44

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Daniel,

On Tuesday 24 January 2012 14:03:22 Daniel Vetter wrote:
> On Mon, Jan 23, 2012 at 11:54:20AM +0100, Laurent Pinchart wrote:
> > On Monday 23 January 2012 11:35:01 Daniel Vetter wrote:
> > > See my other mail, dma_buf v1 does not support cpu access.
> > 
> > v1 is in the kernel now, let's start discussing v2 ;-)
> 
> Ok, I'm in ;-)
> 
> I've thought a bit about this, and I think a reasonable next step would be
> to enable cpu access from kernelspace. Userspace and mmap support is a hole
> different beast altogether and I think we should postpone that until we've
> got the kernel part hashed out.

Userspace access through mmap can already be handled by the subsystem that 
exports the buffer, so I'm fine with postponing implementing this inside dma-
buf.

> I'm thinking about adding 3 pairs of function to dma_buf (not to
> dma_buf_attachment).
> 
> dma_buf_get_backing_storage/put_backing_storage
> This will be used before/after kernel cpu access to ensure that the
> backing storage is in memory. E.g. gem objects can be swapped out, so
> they need to be pinned before we can access them. For exporters with
> static allocations this would be a no-op.
> 
> I think a start, length range would make sense, but the exporter is free
(Continue reading)

Daniel Vetter | 30 Jan 2012 17:29
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Mon, Jan 30, 2012 at 03:44:20PM +0100, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Tuesday 24 January 2012 14:03:22 Daniel Vetter wrote:
> > On Mon, Jan 23, 2012 at 11:54:20AM +0100, Laurent Pinchart wrote:
> > > On Monday 23 January 2012 11:35:01 Daniel Vetter wrote:
> > > > See my other mail, dma_buf v1 does not support cpu access.
> > > 
> > > v1 is in the kernel now, let's start discussing v2 ;-)
> > 
> > Ok, I'm in ;-)
> > 
> > I've thought a bit about this, and I think a reasonable next step would be
> > to enable cpu access from kernelspace. Userspace and mmap support is a hole
> > different beast altogether and I think we should postpone that until we've
> > got the kernel part hashed out.
> 
> Userspace access through mmap can already be handled by the subsystem that 
> exports the buffer, so I'm fine with postponing implementing this inside dma-
> buf.

Actually I think the interface for mmap won't be that horribly. If we add
a bit of magic clue so that the exporter can intercept pagefaults we
should be able to fake coherent mmaps in all cases. And hence avoid
exposing importers and userspace to a lot of ugly things.

> > I'm thinking about adding 3 pairs of function to dma_buf (not to
> > dma_buf_attachment).
> > 
> > dma_buf_get_backing_storage/put_backing_storage
(Continue reading)

Sakari Ailus | 26 Jan 2012 00:28
Picon
Picon
Favicon

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Daniel and Laurent,

On Mon, Jan 23, 2012 at 11:35:01AM +0100, Daniel Vetter wrote:
> On Mon, Jan 23, 2012 at 10:48, Laurent Pinchart
> <laurent.pinchart <at> ideasonboard.com> wrote:
> > Hi Marek,
> >
> > On Monday 23 January 2012 10:06:57 Marek Szyprowski wrote:
> >> On Friday, January 20, 2012 5:29 PM Laurent Pinchart wrote:
> >> > On Friday 20 January 2012 17:20:22 Tomasz Stanislawski wrote:
> >> > > >> IMO, One way to do this is adding field 'struct device *dev' to
> >> > > >> struct vb2_queue. This field should be filled by a driver prior to
> >> > > >> calling vb2_queue_init.
> >> > > >
> >> > > > I haven't looked into the details, but that sounds good to me. Do we
> >> > > > have use cases where a queue is allocated before knowing which
> >> > > > physical device it will be used for ?
> >> > >
> >> > > I don't think so. In case of S5P drivers, vb2_queue_init is called
> >> > > while opening /dev/videoX.
> >> > >
> >> > > BTW. This struct device may help vb2 to produce logs with more
> >> > > descriptive client annotation.
> >> > >
> >> > > What happens if such a device is NULL. It would happen for vmalloc
> >> > > allocator used by VIVI?
> >> >
> >> > Good question. Should dma-buf accept NULL devices ? Or should vivi pass
> >> > its V4L2 device to vb2 ?
> >>
(Continue reading)

Daniel Vetter | 26 Jan 2012 12:27
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Thu, Jan 26, 2012 at 01:28:16AM +0200, Sakari Ailus wrote:
> Why you "should not hang onto mappings forever"? This is currently done by
> virtually all V4L2 drivers where such mappings are relevant. Not doing so
> would really kill the performance i.e. it's infeasible. Same goes to (m)any
> other multimedia devices dealing with buffers containing streaming video
> data.

Because I want dynamic memory managemt simple because everything else does
not make sense. I know that in v4l things don't work that way, but in drm
they _do_. And if you share tons of buffers with drm drivers and don't
follow the rules, the OOM killer will come around and shot at your apps.
Because at least in i915 we do slurp in as much memory as we can until the
oom killer starts growling, at which point we kick out stuff.

I know that current dma_buf isn't there and for many use-cases discussed
here we can get away without that complexity. So you actually can just map
your dma_buf and never ever let go of that mapping again in many cases.

The only reason I'm such a stuborn bastard about all this is that drm/*
will do dynamic bo management even with dma_buf sooner or later and you
should better know that and why and the implications if you choose to
ignore it.

And obviously, the generic dma_buf interface needs to be able to support
it.

Cheers, Daniel
--

-- 
Daniel Vetter
Mail: daniel <at> ffwll.ch
(Continue reading)

Sakari Ailus | 29 Jan 2012 12:03
Picon
Picon
Favicon

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Daniel,

Daniel Vetter wrote:
> On Thu, Jan 26, 2012 at 01:28:16AM +0200, Sakari Ailus wrote:
>> Why you "should not hang onto mappings forever"? This is currently done by
>> virtually all V4L2 drivers where such mappings are relevant. Not doing so
>> would really kill the performance i.e. it's infeasible. Same goes to (m)any
>> other multimedia devices dealing with buffers containing streaming video
>> data.
> 
> Because I want dynamic memory managemt simple because everything else does
> not make sense. I know that in v4l things don't work that way, but in drm
> they _do_. And if you share tons of buffers with drm drivers and don't
> follow the rules, the OOM killer will come around and shot at your apps.
> Because at least in i915 we do slurp in as much memory as we can until the
> oom killer starts growling, at which point we kick out stuff.
> 
> I know that current dma_buf isn't there and for many use-cases discussed
> here we can get away without that complexity. So you actually can just map
> your dma_buf and never ever let go of that mapping again in many cases.
> 
> The only reason I'm such a stuborn bastard about all this is that drm/*
> will do dynamic bo management even with dma_buf sooner or later and you
> should better know that and why and the implications if you choose to
> ignore it.
> 
> And obviously, the generic dma_buf interface needs to be able to support
> it.

I do not think we should completely ignore this issue, but I think we
(Continue reading)

Daniel Vetter | 29 Jan 2012 14:03
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Sun, Jan 29, 2012 at 01:03:39PM +0200, Sakari Ailus wrote:
> Daniel Vetter wrote:
> > On Thu, Jan 26, 2012 at 01:28:16AM +0200, Sakari Ailus wrote:
> >> Why you "should not hang onto mappings forever"? This is currently done by
> >> virtually all V4L2 drivers where such mappings are relevant. Not doing so
> >> would really kill the performance i.e. it's infeasible. Same goes to (m)any
> >> other multimedia devices dealing with buffers containing streaming video
> >> data.
> > 
> > Because I want dynamic memory managemt simple because everything else does
> > not make sense. I know that in v4l things don't work that way, but in drm
> > they _do_. And if you share tons of buffers with drm drivers and don't
> > follow the rules, the OOM killer will come around and shot at your apps.
> > Because at least in i915 we do slurp in as much memory as we can until the
> > oom killer starts growling, at which point we kick out stuff.
> > 
> > I know that current dma_buf isn't there and for many use-cases discussed
> > here we can get away without that complexity. So you actually can just map
> > your dma_buf and never ever let go of that mapping again in many cases.
> > 
> > The only reason I'm such a stuborn bastard about all this is that drm/*
> > will do dynamic bo management even with dma_buf sooner or later and you
> > should better know that and why and the implications if you choose to
> > ignore it.
> > 
> > And obviously, the generic dma_buf interface needs to be able to support
> > it.
> 
> I do not think we should completely ignore this issue, but I think we
> might want to at least postpone the implementation for non-DRM
(Continue reading)

Laurent Pinchart | 30 Jan 2012 15:33

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Daniel,

On Sunday 29 January 2012 14:03:40 Daniel Vetter wrote:
> On Sun, Jan 29, 2012 at 01:03:39PM +0200, Sakari Ailus wrote:
> > Daniel Vetter wrote:
> > > On Thu, Jan 26, 2012 at 01:28:16AM +0200, Sakari Ailus wrote:
> > >> Why you "should not hang onto mappings forever"? This is currently
> > >> done by virtually all V4L2 drivers where such mappings are relevant.
> > >> Not doing so would really kill the performance i.e. it's infeasible.
> > >> Same goes to (m)any other multimedia devices dealing with buffers
> > >> containing streaming video data.
> > > 
> > > Because I want dynamic memory managemt simple because everything else
> > > does not make sense. I know that in v4l things don't work that way,
> > > but in drm they _do_. And if you share tons of buffers with drm
> > > drivers and don't follow the rules, the OOM killer will come around
> > > and shot at your apps. Because at least in i915 we do slurp in as much
> > > memory as we can until the oom killer starts growling, at which point
> > > we kick out stuff.
> > > 
> > > I know that current dma_buf isn't there and for many use-cases
> > > discussed here we can get away without that complexity. So you
> > > actually can just map your dma_buf and never ever let go of that
> > > mapping again in many cases.
> > > 
> > > The only reason I'm such a stuborn bastard about all this is that drm/*
> > > will do dynamic bo management even with dma_buf sooner or later and you
> > > should better know that and why and the implications if you choose to
> > > ignore it.
> > > 
(Continue reading)

Sakari Ailus | 30 Jan 2012 23:01
Picon
Picon
Favicon

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Daniel,

On Sun, Jan 29, 2012 at 02:03:40PM +0100, Daniel Vetter wrote:
> On Sun, Jan 29, 2012 at 01:03:39PM +0200, Sakari Ailus wrote:
> > Daniel Vetter wrote:
> > > On Thu, Jan 26, 2012 at 01:28:16AM +0200, Sakari Ailus wrote:
> > >> Why you "should not hang onto mappings forever"? This is currently done by
> > >> virtually all V4L2 drivers where such mappings are relevant. Not doing so
> > >> would really kill the performance i.e. it's infeasible. Same goes to (m)any
> > >> other multimedia devices dealing with buffers containing streaming video
> > >> data.
> > > 
> > > Because I want dynamic memory managemt simple because everything else does
> > > not make sense. I know that in v4l things don't work that way, but in drm
> > > they _do_. And if you share tons of buffers with drm drivers and don't
> > > follow the rules, the OOM killer will come around and shot at your apps.
> > > Because at least in i915 we do slurp in as much memory as we can until the
> > > oom killer starts growling, at which point we kick out stuff.
> > > 
> > > I know that current dma_buf isn't there and for many use-cases discussed
> > > here we can get away without that complexity. So you actually can just map
> > > your dma_buf and never ever let go of that mapping again in many cases.
> > > 
> > > The only reason I'm such a stuborn bastard about all this is that drm/*
> > > will do dynamic bo management even with dma_buf sooner or later and you
> > > should better know that and why and the implications if you choose to
> > > ignore it.
> > > 
> > > And obviously, the generic dma_buf interface needs to be able to support
> > > it.
(Continue reading)

Clark, Rob | 31 Jan 2012 16:38
Picon
Favicon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Mon, Jan 30, 2012 at 4:01 PM, Sakari Ailus <sakari.ailus <at> iki.fi> wrote:
>
>> So to summarize I understand your constraints - gpu drivers have worked
>> like v4l a few years ago. The thing I'm trying to achieve with this
>> constant yelling is just to raise awereness for these issues so that
>> people aren't suprised when drm starts pulling tricks on dma_bufs.
>
> I think we should be able to mark dma_bufs non-relocatable so also DRM can
> work with these buffers. Or alternatively, as Laurent proposed, V4L2 be
> prepared for moving the buffers around. Are there other reasons to do so
> than paging them out of system memory to make room for something else?

fwiw, from GPU perspective, the DRM device wouldn't be actively
relocating buffers just for the fun of it.  I think it is more that we
want to give the GPU driver the flexibility to relocate when it really
needs to.  For example, maybe user has camera app running, then puts
it in the background and opens firefox which tries to allocate a big
set of pixmaps putting pressure on GPU memory..

I guess the root issue is who is doing the IOMMU programming for the
camera driver.  I guess if this is something built in to the camera
driver then when it calls dma_buf_map() it probably wants some hint
that the backing pages haven't moved so in the common case (ie. buffer
hasn't moved) it doesn't have to do anything expensive.

On omap4 v4l2+drm example I have running, it is actually the DRM
driver doing the "IOMMU" programming.. so v4l2 camera really doesn't
need to care about it.  (And the IOMMU programming here is pretty
fast.)  But I suppose this maybe doesn't represent all cases.  I
suppose if a camera didn't really sit behind an IOMMU but uses
(Continue reading)

Laurent Pinchart | 2 Feb 2012 11:19

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Rob,

On Tuesday 31 January 2012 16:38:35 Clark, Rob wrote:
> On Mon, Jan 30, 2012 at 4:01 PM, Sakari Ailus <sakari.ailus <at> iki.fi> wrote:
> >> So to summarize I understand your constraints - gpu drivers have worked
> >> like v4l a few years ago. The thing I'm trying to achieve with this
> >> constant yelling is just to raise awereness for these issues so that
> >> people aren't suprised when drm starts pulling tricks on dma_bufs.
> > 
> > I think we should be able to mark dma_bufs non-relocatable so also DRM
> > can work with these buffers. Or alternatively, as Laurent proposed, V4L2
> > be prepared for moving the buffers around. Are there other reasons to do
> > so than paging them out of system memory to make room for something
> > else?
> 
> fwiw, from GPU perspective, the DRM device wouldn't be actively
> relocating buffers just for the fun of it.  I think it is more that we
> want to give the GPU driver the flexibility to relocate when it really
> needs to.  For example, maybe user has camera app running, then puts
> it in the background and opens firefox which tries to allocate a big
> set of pixmaps putting pressure on GPU memory..

On an embedded system putting the camera application in the background will 
usually stop streaming, so buffers will be unmapped. On other systems, or even 
on some embedded systems, that will not be the case though.

I'm perfectly fine with relocating buffers when needed. What I want is to 
avoid unmapping and remapping them for every frame if they haven't moved. I'm 
sure we can come up with an API to handle that.

(Continue reading)

Clark, Rob | 2 Feb 2012 15:01
Picon
Favicon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Thu, Feb 2, 2012 at 4:19 AM, Laurent Pinchart
<laurent.pinchart <at> ideasonboard.com> wrote:
> Hi Rob,
>
> On Tuesday 31 January 2012 16:38:35 Clark, Rob wrote:
>> On Mon, Jan 30, 2012 at 4:01 PM, Sakari Ailus <sakari.ailus <at> iki.fi> wrote:
>> >> So to summarize I understand your constraints - gpu drivers have worked
>> >> like v4l a few years ago. The thing I'm trying to achieve with this
>> >> constant yelling is just to raise awereness for these issues so that
>> >> people aren't suprised when drm starts pulling tricks on dma_bufs.
>> >
>> > I think we should be able to mark dma_bufs non-relocatable so also DRM
>> > can work with these buffers. Or alternatively, as Laurent proposed, V4L2
>> > be prepared for moving the buffers around. Are there other reasons to do
>> > so than paging them out of system memory to make room for something
>> > else?
>>
>> fwiw, from GPU perspective, the DRM device wouldn't be actively
>> relocating buffers just for the fun of it.  I think it is more that we
>> want to give the GPU driver the flexibility to relocate when it really
>> needs to.  For example, maybe user has camera app running, then puts
>> it in the background and opens firefox which tries to allocate a big
>> set of pixmaps putting pressure on GPU memory..
>
> On an embedded system putting the camera application in the background will
> usually stop streaming, so buffers will be unmapped. On other systems, or even
> on some embedded systems, that will not be the case though.
>
> I'm perfectly fine with relocating buffers when needed. What I want is to
> avoid unmapping and remapping them for every frame if they haven't moved. I'm
(Continue reading)

Sumit Semwal | 2 Feb 2012 15:40
Favicon

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On 2 February 2012 19:31, Clark, Rob <rob <at> ti.com> wrote:
> On Thu, Feb 2, 2012 at 4:19 AM, Laurent Pinchart
> <laurent.pinchart <at> ideasonboard.com> wrote:
>> Hi Rob,
>>
>> On Tuesday 31 January 2012 16:38:35 Clark, Rob wrote:
>>> On Mon, Jan 30, 2012 at 4:01 PM, Sakari Ailus <sakari.ailus <at> iki.fi> wrote:
>>> >> So to summarize I understand your constraints - gpu drivers have worked
>>> >> like v4l a few years ago. The thing I'm trying to achieve with this
>>> >> constant yelling is just to raise awereness for these issues so that
>>> >> people aren't suprised when drm starts pulling tricks on dma_bufs.
>>> >
>>> > I think we should be able to mark dma_bufs non-relocatable so also DRM
>>> > can work with these buffers. Or alternatively, as Laurent proposed, V4L2
>>> > be prepared for moving the buffers around. Are there other reasons to do
>>> > so than paging them out of system memory to make room for something
>>> > else?
>>>
>>> fwiw, from GPU perspective, the DRM device wouldn't be actively
>>> relocating buffers just for the fun of it.  I think it is more that we
>>> want to give the GPU driver the flexibility to relocate when it really
>>> needs to.  For example, maybe user has camera app running, then puts
>>> it in the background and opens firefox which tries to allocate a big
>>> set of pixmaps putting pressure on GPU memory..
>>
>> On an embedded system putting the camera application in the background will
>> usually stop streaming, so buffers will be unmapped. On other systems, or even
>> on some embedded systems, that will not be the case though.
>>
>> I'm perfectly fine with relocating buffers when needed. What I want is to
(Continue reading)

Daniel Vetter | 2 Feb 2012 21:23
Picon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Thu, Feb 2, 2012 at 11:19, Laurent Pinchart <laurent.pinchart <at> ideasonboard.com> wrote:
>> On omap4 v4l2+drm example I have running, it is actually the DRM driver
>> doing the "IOMMU" programming.. so v4l2 camera really doesn't need to care
>> about it.  (And the IOMMU programming here is pretty fast.)  But I suppose
>> this maybe doesn't represent all cases. I suppose if a camera didn't really
>> sit behind an IOMMU but uses something more like a DMA descriptor list would
>> want to know if it needed to regenerate it's descriptor list. Or likewise if
>> camera has an IOMMU that isn't really using the IOMMU framework (although
>> maybe that is easier to solve).  But I think a hint returned from
>> dma_buf_map() would do the job?
>
> I see at least three possible solutions to this problem.
>
> 1. At dma_buf_unmap() time, the exporter will tell the importer that the
> buffer will move, and that it should be unmapped from whatever the importer
> mapped it to. That's probably the easiest solution to implement on the
> importer's side, but I expect it to be difficult for the exporter to know at
> dma_buf_unmap() time if the buffer will need to be moved or not.
>
> 2. Adding a callback to request the importer to unmap the buffer. This might
> be racy, and locking might be difficult to handle.
>
> 3. At dma_buf_unmap() time, keep importer's mappings around. The exporter is
> then free to move the buffer if needed, in which case the mappings will be
> invalid. This shouldn't be a problem in theory, as the buffer isn't being used
> by the importer at that time, but can cause stability issues when dealing with
> rogue hardware as this would punch holes in the IOMMU fence. At dma_buf_map()
> time the exporter would tell the importer whether the buffer moved or not. If
> it moved, the importer will tear down the mappings it kept, and create new
> ones.
(Continue reading)

Clark, Rob | 2 Feb 2012 21:49
Picon
Favicon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Thu, Feb 2, 2012 at 2:23 PM, Daniel Vetter <daniel <at> ffwll.ch> wrote:
> On Thu, Feb 2, 2012 at 11:19, Laurent Pinchart <laurent.pinchart <at> ideasonboard.com> wrote:
>>> On omap4 v4l2+drm example I have running, it is actually the DRM driver
>>> doing the "IOMMU" programming.. so v4l2 camera really doesn't need to care
>>> about it.  (And the IOMMU programming here is pretty fast.)  But I suppose
>>> this maybe doesn't represent all cases. I suppose if a camera didn't really
>>> sit behind an IOMMU but uses something more like a DMA descriptor list would
>>> want to know if it needed to regenerate it's descriptor list. Or likewise if
>>> camera has an IOMMU that isn't really using the IOMMU framework (although
>>> maybe that is easier to solve).  But I think a hint returned from
>>> dma_buf_map() would do the job?
>>
>> I see at least three possible solutions to this problem.
>>
>> 1. At dma_buf_unmap() time, the exporter will tell the importer that the
>> buffer will move, and that it should be unmapped from whatever the importer
>> mapped it to. That's probably the easiest solution to implement on the
>> importer's side, but I expect it to be difficult for the exporter to know at
>> dma_buf_unmap() time if the buffer will need to be moved or not.
>>
>> 2. Adding a callback to request the importer to unmap the buffer. This might
>> be racy, and locking might be difficult to handle.
>>
>> 3. At dma_buf_unmap() time, keep importer's mappings around. The exporter is
>> then free to move the buffer if needed, in which case the mappings will be
>> invalid. This shouldn't be a problem in theory, as the buffer isn't being used
>> by the importer at that time, but can cause stability issues when dealing with
>> rogue hardware as this would punch holes in the IOMMU fence. At dma_buf_map()
>> time the exporter would tell the importer whether the buffer moved or not. If
>> it moved, the importer will tear down the mappings it kept, and create new
(Continue reading)

Sakari Ailus | 4 Feb 2012 12:43
Picon
Picon
Favicon

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Rob,

Clark, Rob wrote:
> On Mon, Jan 30, 2012 at 4:01 PM, Sakari Ailus <sakari.ailus <at> iki.fi> wrote:
>>
>>> So to summarize I understand your constraints - gpu drivers have worked
>>> like v4l a few years ago. The thing I'm trying to achieve with this
>>> constant yelling is just to raise awereness for these issues so that
>>> people aren't suprised when drm starts pulling tricks on dma_bufs.
>>
>> I think we should be able to mark dma_bufs non-relocatable so also DRM can
>> work with these buffers. Or alternatively, as Laurent proposed, V4L2 be
>> prepared for moving the buffers around. Are there other reasons to do so
>> than paging them out of system memory to make room for something else?
> 
> fwiw, from GPU perspective, the DRM device wouldn't be actively
> relocating buffers just for the fun of it.  I think it is more that we
> want to give the GPU driver the flexibility to relocate when it really
> needs to.  For example, maybe user has camera app running, then puts
> it in the background and opens firefox which tries to allocate a big
> set of pixmaps putting pressure on GPU memory..
> 
> I guess the root issue is who is doing the IOMMU programming for the
> camera driver.  I guess if this is something built in to the camera
> driver then when it calls dma_buf_map() it probably wants some hint
> that the backing pages haven't moved so in the common case (ie. buffer
> hasn't moved) it doesn't have to do anything expensive.
> 
> On omap4 v4l2+drm example I have running, it is actually the DRM
> driver doing the "IOMMU" programming.. so v4l2 camera really doesn't
(Continue reading)

Clark, Rob | 5 Feb 2012 16:08
Picon
Favicon
Gravatar

Re: [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

On Sat, Feb 4, 2012 at 5:43 AM, Sakari Ailus <sakari.ailus <at> iki.fi> wrote:
> Hi Rob,
>
> Clark, Rob wrote:
>> On Mon, Jan 30, 2012 at 4:01 PM, Sakari Ailus <sakari.ailus <at> iki.fi> wrote:
>>>
>>>> So to summarize I understand your constraints - gpu drivers have worked
>>>> like v4l a few years ago. The thing I'm trying to achieve with this
>>>> constant yelling is just to raise awereness for these issues so that
>>>> people aren't suprised when drm starts pulling tricks on dma_bufs.
>>>
>>> I think we should be able to mark dma_bufs non-relocatable so also DRM can
>>> work with these buffers. Or alternatively, as Laurent proposed, V4L2 be
>>> prepared for moving the buffers around. Are there other reasons to do so
>>> than paging them out of system memory to make room for something else?
>>
>> fwiw, from GPU perspective, the DRM device wouldn't be actively
>> relocating buffers just for the fun of it.  I think it is more that we
>> want to give the GPU driver the flexibility to relocate when it really
>> needs to.  For example, maybe user has camera app running, then puts
>> it in the background and opens firefox which tries to allocate a big
>> set of pixmaps putting pressure on GPU memory..
>>
>> I guess the root issue is who is doing the IOMMU programming for the
>> camera driver.  I guess if this is something built in to the camera
>> driver then when it calls dma_buf_map() it probably wants some hint
>> that the backing pages haven't moved so in the common case (ie. buffer
>> hasn't moved) it doesn't have to do anything expensive.
>>
>> On omap4 v4l2+drm example I have running, it is actually the DRM
(Continue reading)

Laurent Pinchart | 20 Jan 2012 15:55

Re: [Linaro-mm-sig] [RFCv1 2/4] v4l:vb2: add support for shared buffer (dma_buf)

Hi Sumit,

On Thursday 05 January 2012 11:41:56 Sumit Semwal wrote:
> This patch adds support for DMABUF memory type in videobuf2. It calls
> relevant APIs of dma_buf for v4l reqbuf / qbuf / dqbuf operations.
> 
> For this version, the support is for videobuf2 as a user of the shared
> buffer; so the allocation of the buffer is done outside of V4L2. [A sample
> allocator of dma-buf shared buffer is given at [1]]
> 
> [1]: Rob Clark's DRM:
>    https://github.com/robclark/kernel-omap4/commits/drmplane-dmabuf
> 
> Signed-off-by: Tomasz Stanislawski <t.stanislaws <at> samsung.com>
>    [original work in the PoC for buffer sharing]
> Signed-off-by: Sumit Semwal <sumit.semwal <at> ti.com>
> Signed-off-by: Sumit Semwal <sumit.semwal <at> linaro.org>
> ---
>  drivers/media/video/videobuf2-core.c |  186 ++++++++++++++++++++++++++++++-
>  include/media/videobuf2-core.h       |   30 ++++++
>  2 files changed, 215 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c index 95a3f5e..6cd2f97 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
>  <at>  <at>  -107,6 +107,27  <at>  <at>  static void __vb2_buf_userptr_put(struct vb2_buffer
> *vb) }
> 
>  /**
(Continue reading)

Sumit Semwal | 5 Jan 2012 11:41
Picon
Favicon

[RFCv1 3/4] v4l:vb: remove warnings about MEMORY_DMABUF

Adding DMABUF memory type causes videobuf to complain about not using it
in some switch cases. This patch removes these warnings.

Signed-off-by: Sumit Semwal <sumit.semwal <at> ti.com>
---
 drivers/media/video/videobuf-core.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
index de4fa4e..b457c8b 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
 <at>  <at>  -335,6 +335,9  <at>  <at>  static void videobuf_status(struct videobuf_queue *q, struct v4l2_buffer *b,
 	case V4L2_MEMORY_OVERLAY:
 		b->m.offset  = vb->boff;
 		break;
+	case V4L2_MEMORY_DMABUF:
+		/* DMABUF is not handled in videobuf framework */
+		break;
 	}

 	b->flags    = 0;
 <at>  <at>  -411,6 +414,7  <at>  <at>  int __videobuf_mmap_setup(struct videobuf_queue *q,
 			break;
 		case V4L2_MEMORY_USERPTR:
 		case V4L2_MEMORY_OVERLAY:
+		case V4L2_MEMORY_DMABUF:
 			/* nothing */
 			break;
 		}
(Continue reading)

Sumit Semwal | 5 Jan 2012 11:41
Picon
Favicon

[RFCv1 4/4] v4l:vb2: Add dma-contig allocator as dma_buf user

This patch makes changes for adding dma-contig as a dma_buf user. It provides
function implementations for the {attach, detach, map, unmap}_dmabuf()
mem_ops of DMABUF memory type.

Signed-off-by: Sumit Semwal <sumit.semwal <at> ti.com>
Signed-off-by: Sumit Semwal <sumit.semwal <at> linaro.org>
---
 drivers/media/video/videobuf2-dma-contig.c |  125 ++++++++++++++++++++++++++++
 1 files changed, 125 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index f17ad98..e671371 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
 <at>  <at>  -13,6 +13,8  <at>  <at> 
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/dma-mapping.h>
+#include <linux/scatterlist.h>
+#include <linux/dma-buf.h>

 #include <media/videobuf2-core.h>
 #include <media/videobuf2-memops.h>
 <at>  <at>  -27,6 +29,7  <at>  <at>  struct vb2_dc_buf {
 	dma_addr_t			dma_addr;
 	unsigned long			size;
 	struct vm_area_struct		*vma;
+	struct dma_buf_attachment	*db_attach;
 	atomic_t			refcount;
 	struct vb2_vmarea_handler	handler;
(Continue reading)

Kyungmin Park | 16 Jan 2012 08:57
Favicon

Re: [RFCv1 0/4] v4l: DMA buffer sharing support as a user

Hi,

Since dma_buf is merged at v3.3-rc, I hope to merge this one also at
this merge window.
Now it's tested at OMAP. also it's used at exynos but not yet fully tested.

Thank you,
Kyungmin Park

On 1/5/12, Sumit Semwal <sumit.semwal <at> ti.com> wrote:
> Hello Everyone,
>
> A very happy new year 2012! :)
>
> This patchset is an RFC for the way videobuf2 can be adapted to add support
> for
> DMA buffer sharing framework[1].
>
> The original patch-set for the idea, and PoC of buffer sharing was by
> Tomasz Stanislawski <t.stanislaws <at> samsung.com>, who demonstrated buffer
> sharing
> between two v4l2 devices[2]. This RFC is needed to adapt these patches to
> the
> changes that have happened in the DMA buffer sharing framework over past few
> months.
>
> To begin with, I have tried to adapt only the dma-contig allocator, and only
> as
> a user of dma-buf buffer. I am currently working on the v4l2-as-an-exporter
> changes, and will share as soon as I get it in some shape.
(Continue reading)


Gmane