linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] udmabuf: add vmap and vunmap methods to udmabuf_ops
@ 2022-11-17  4:58 Lukasz Wiecaszek
  2022-11-17  9:04 ` Dmitry Osipenko
  0 siblings, 1 reply; 6+ messages in thread
From: Lukasz Wiecaszek @ 2022-11-17  4:58 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Dmitry Osipenko, Lukasz Wiecaszek, Sumit Semwal,
	Christian König, dri-devel, linux-media, linaro-mm-sig,
	linux-kernel

The reason behind that patch is associated with videobuf2 subsystem
(or more genrally with v4l2 framework) and user created
dma buffers (udmabuf). In some circumstances
when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem
wants to use dma_buf_vmap() method on the attached dma buffer.
As udmabuf does not have .vmap operation implemented,
such dma_buf_vmap() natually fails.

videobuf2_common: __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each
videobuf2_common: __prepare_dmabuf: buffer for plane 0 changed
videobuf2_common: __prepare_dmabuf: failed to map dmabuf for plane 0
videobuf2_common: __buf_prepare: buffer preparation failed: -14

The patch itself seems to be strighforward.
It adds implementation of .vmap and .vunmap methods
to 'struct dma_buf_ops udmabuf_ops'.
.vmap method itself uses vm_map_ram() to map pages linearly
into the kernel virtual address space.
.vunmap removes mapping created earlier by .vmap.
All locking and 'vmapping counting' is done in dma_buf.c
so it seems to be redundant/unnecessary in .vmap/.vunmap.

Signed-off-by: Lukasz Wiecaszek <lukasz.wiecaszek@gmail.com>
---
v1: https://lore.kernel.org/linux-media/202211120352.G7WPASoP-lkp@intel.com/T/#t
v2: https://lore.kernel.org/linux-media/20221114052944.GA7264@thinkpad-p72/T/#t
v3: https://lore.kernel.org/linux-media/4f92e95f-a0dc-4eac-4c08-0df85de78ae7@collabora.com/T/#t

v3 -> v4: Removed line/info 'reported by kernel test robot'
v2 -> v3: Added .vunmap to 'struct dma_buf_ops udmabuf_ops'
v1 -> v2: Patch prepared and tested against 6.1.0-rc2+

 drivers/dma-buf/udmabuf.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 283816fbd72f..740d6e426ee9 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -13,6 +13,8 @@
 #include <linux/slab.h>
 #include <linux/udmabuf.h>
 #include <linux/hugetlb.h>
+#include <linux/vmalloc.h>
+#include <linux/iosys-map.h>
 
 static int list_limit = 1024;
 module_param(list_limit, int, 0644);
@@ -60,6 +62,30 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma)
 	return 0;
 }
 
+static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map)
+{
+	struct udmabuf *ubuf = buf->priv;
+	void *vaddr;
+
+	dma_resv_assert_held(buf->resv);
+
+	vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1);
+	if (!vaddr)
+		return -EINVAL;
+
+	iosys_map_set_vaddr(map, vaddr);
+	return 0;
+}
+
+static void vunmap_udmabuf(struct dma_buf *buf, struct iosys_map *map)
+{
+	struct udmabuf *ubuf = buf->priv;
+
+	dma_resv_assert_held(buf->resv);
+
+	vm_unmap_ram(map->vaddr, ubuf->pagecount);
+}
+
 static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
 				     enum dma_data_direction direction)
 {
@@ -162,6 +188,8 @@ static const struct dma_buf_ops udmabuf_ops = {
 	.unmap_dma_buf	   = unmap_udmabuf,
 	.release	   = release_udmabuf,
 	.mmap		   = mmap_udmabuf,
+	.vmap		   = vmap_udmabuf,
+	.vunmap		   = vunmap_udmabuf,
 	.begin_cpu_access  = begin_cpu_udmabuf,
 	.end_cpu_access    = end_cpu_udmabuf,
 };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] udmabuf: add vmap and vunmap methods to udmabuf_ops
  2022-11-17  4:58 [PATCH v4] udmabuf: add vmap and vunmap methods to udmabuf_ops Lukasz Wiecaszek
@ 2022-11-17  9:04 ` Dmitry Osipenko
  2022-11-17 17:08   ` Lukasz Wiecaszek
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Osipenko @ 2022-11-17  9:04 UTC (permalink / raw)
  To: Lukasz Wiecaszek, Gerd Hoffmann
  Cc: Lukasz Wiecaszek, Sumit Semwal, Christian König, dri-devel,
	linux-media, linaro-mm-sig, linux-kernel

Hi,

On 11/17/22 07:58, Lukasz Wiecaszek wrote:
> The reason behind that patch is associated with videobuf2 subsystem
> (or more genrally with v4l2 framework) and user created
> dma buffers (udmabuf). In some circumstances
> when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem
> wants to use dma_buf_vmap() method on the attached dma buffer.
> As udmabuf does not have .vmap operation implemented,
> such dma_buf_vmap() natually fails.
> 
> videobuf2_common: __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each
> videobuf2_common: __prepare_dmabuf: buffer for plane 0 changed
> videobuf2_common: __prepare_dmabuf: failed to map dmabuf for plane 0
> videobuf2_common: __buf_prepare: buffer preparation failed: -14
> 
> The patch itself seems to be strighforward.
> It adds implementation of .vmap and .vunmap methods
> to 'struct dma_buf_ops udmabuf_ops'.
> .vmap method itself uses vm_map_ram() to map pages linearly
> into the kernel virtual address space.
> .vunmap removes mapping created earlier by .vmap.
> All locking and 'vmapping counting' is done in dma_buf.c
> so it seems to be redundant/unnecessary in .vmap/.vunmap.
> 
> Signed-off-by: Lukasz Wiecaszek <lukasz.wiecaszek@gmail.com>

If new patch version doesn't contain significant changes and you got
acks/reviews for the previous version, then you should add the given
acked-by and reviewed-by tags to the commit message by yourself.

-- 
Best regards,
Dmitry


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] udmabuf: add vmap and vunmap methods to udmabuf_ops
  2022-11-17  9:04 ` Dmitry Osipenko
@ 2022-11-17 17:08   ` Lukasz Wiecaszek
  2022-11-17 17:32     ` Dmitry Osipenko
  0 siblings, 1 reply; 6+ messages in thread
From: Lukasz Wiecaszek @ 2022-11-17 17:08 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Lukasz Wiecaszek, Gerd Hoffmann, Sumit Semwal,
	Christian König, dri-devel, linux-media, linaro-mm-sig,
	linux-kernel

On Thu, Nov 17, 2022 at 12:04:35PM +0300, Dmitry Osipenko wrote:
> Hi,
> 
> On 11/17/22 07:58, Lukasz Wiecaszek wrote:
> > The reason behind that patch is associated with videobuf2 subsystem
> > (or more genrally with v4l2 framework) and user created
> > dma buffers (udmabuf). In some circumstances
> > when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem
> > wants to use dma_buf_vmap() method on the attached dma buffer.
> > As udmabuf does not have .vmap operation implemented,
> > such dma_buf_vmap() natually fails.
> > 
> > videobuf2_common: __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each
> > videobuf2_common: __prepare_dmabuf: buffer for plane 0 changed
> > videobuf2_common: __prepare_dmabuf: failed to map dmabuf for plane 0
> > videobuf2_common: __buf_prepare: buffer preparation failed: -14
> > 
> > The patch itself seems to be strighforward.
> > It adds implementation of .vmap and .vunmap methods
> > to 'struct dma_buf_ops udmabuf_ops'.
> > .vmap method itself uses vm_map_ram() to map pages linearly
> > into the kernel virtual address space.
> > .vunmap removes mapping created earlier by .vmap.
> > All locking and 'vmapping counting' is done in dma_buf.c
> > so it seems to be redundant/unnecessary in .vmap/.vunmap.
> > 
> > Signed-off-by: Lukasz Wiecaszek <lukasz.wiecaszek@gmail.com>
> 
> If new patch version doesn't contain significant changes and you got
> acks/reviews for the previous version, then you should add the given
> acked-by and reviewed-by tags to the commit message by yourself.
> 
> -- 
> Best regards,
> Dmitry
>

I would like to thank you all for your patience and on the same time say
sorry that I still cannot follow the process (although I have read
'submitting patches' chapter).


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] udmabuf: add vmap and vunmap methods to udmabuf_ops
  2022-11-17 17:08   ` Lukasz Wiecaszek
@ 2022-11-17 17:32     ` Dmitry Osipenko
  2022-11-17 18:01       ` Christian König
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Osipenko @ 2022-11-17 17:32 UTC (permalink / raw)
  To: Lukasz Wiecaszek
  Cc: Gerd Hoffmann, Sumit Semwal, Christian König, dri-devel,
	linux-media, linaro-mm-sig, linux-kernel

On 11/17/22 20:08, Lukasz Wiecaszek wrote:
> On Thu, Nov 17, 2022 at 12:04:35PM +0300, Dmitry Osipenko wrote:
>> Hi,
>>
>> On 11/17/22 07:58, Lukasz Wiecaszek wrote:
>>> The reason behind that patch is associated with videobuf2 subsystem
>>> (or more genrally with v4l2 framework) and user created
>>> dma buffers (udmabuf). In some circumstances
>>> when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem
>>> wants to use dma_buf_vmap() method on the attached dma buffer.
>>> As udmabuf does not have .vmap operation implemented,
>>> such dma_buf_vmap() natually fails.
>>>
>>> videobuf2_common: __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each
>>> videobuf2_common: __prepare_dmabuf: buffer for plane 0 changed
>>> videobuf2_common: __prepare_dmabuf: failed to map dmabuf for plane 0
>>> videobuf2_common: __buf_prepare: buffer preparation failed: -14
>>>
>>> The patch itself seems to be strighforward.
>>> It adds implementation of .vmap and .vunmap methods
>>> to 'struct dma_buf_ops udmabuf_ops'.
>>> .vmap method itself uses vm_map_ram() to map pages linearly
>>> into the kernel virtual address space.
>>> .vunmap removes mapping created earlier by .vmap.
>>> All locking and 'vmapping counting' is done in dma_buf.c
>>> so it seems to be redundant/unnecessary in .vmap/.vunmap.
>>>
>>> Signed-off-by: Lukasz Wiecaszek <lukasz.wiecaszek@gmail.com>
>>
>> If new patch version doesn't contain significant changes and you got
>> acks/reviews for the previous version, then you should add the given
>> acked-by and reviewed-by tags to the commit message by yourself.
>>
>> -- 
>> Best regards,
>> Dmitry
>>
> 
> I would like to thank you all for your patience and on the same time say
> sorry that I still cannot follow the process (although I have read
> 'submitting patches' chapter).

If you'll continue to contribute actively, you'll find things that
aren't documented at all. Don't worry about it, usually somebody will
tell you about what's missing. Just apply the new knowledge next time ;)

-- 
Best regards,
Dmitry


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] udmabuf: add vmap and vunmap methods to udmabuf_ops
  2022-11-17 17:32     ` Dmitry Osipenko
@ 2022-11-17 18:01       ` Christian König
  2022-11-18  9:42         ` Lukasz Wiecaszek
  0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2022-11-17 18:01 UTC (permalink / raw)
  To: Dmitry Osipenko, Lukasz Wiecaszek
  Cc: Gerd Hoffmann, Sumit Semwal, dri-devel, linux-media,
	linaro-mm-sig, linux-kernel

Am 17.11.22 um 18:32 schrieb Dmitry Osipenko:
> On 11/17/22 20:08, Lukasz Wiecaszek wrote:
>> On Thu, Nov 17, 2022 at 12:04:35PM +0300, Dmitry Osipenko wrote:
>>> Hi,
>>>
>>> On 11/17/22 07:58, Lukasz Wiecaszek wrote:
>>>> The reason behind that patch is associated with videobuf2 subsystem
>>>> (or more genrally with v4l2 framework) and user created
>>>> dma buffers (udmabuf). In some circumstances
>>>> when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem
>>>> wants to use dma_buf_vmap() method on the attached dma buffer.
>>>> As udmabuf does not have .vmap operation implemented,
>>>> such dma_buf_vmap() natually fails.
>>>>
>>>> videobuf2_common: __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each
>>>> videobuf2_common: __prepare_dmabuf: buffer for plane 0 changed
>>>> videobuf2_common: __prepare_dmabuf: failed to map dmabuf for plane 0
>>>> videobuf2_common: __buf_prepare: buffer preparation failed: -14
>>>>
>>>> The patch itself seems to be strighforward.
>>>> It adds implementation of .vmap and .vunmap methods
>>>> to 'struct dma_buf_ops udmabuf_ops'.
>>>> .vmap method itself uses vm_map_ram() to map pages linearly
>>>> into the kernel virtual address space.
>>>> .vunmap removes mapping created earlier by .vmap.
>>>> All locking and 'vmapping counting' is done in dma_buf.c
>>>> so it seems to be redundant/unnecessary in .vmap/.vunmap.
>>>>
>>>> Signed-off-by: Lukasz Wiecaszek <lukasz.wiecaszek@gmail.com>
>>> If new patch version doesn't contain significant changes and you got
>>> acks/reviews for the previous version, then you should add the given
>>> acked-by and reviewed-by tags to the commit message by yourself.
>>>
>>> -- 
>>> Best regards,
>>> Dmitry
>>>
>> I would like to thank you all for your patience and on the same time say
>> sorry that I still cannot follow the process (although I have read
>> 'submitting patches' chapter).
> If you'll continue to contribute actively, you'll find things that
> aren't documented at all. Don't worry about it, usually somebody will
> tell you about what's missing. Just apply the new knowledge next time ;)

Yeah, it's more learning by doing. Especially I suspect you don't have 
commit rights to drm-misc-next (or do you want to upstream it through 
some other branch?), so as soon as nobody has any more objections ping 
Dmitry or me to push this.

Cheers,
Christian

PS: The Signed-of-by, Reviewed-by, Acked-by etc... lines are usually 
added in chronological order, e.g. your Signed-of-by line should always 
come first.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] udmabuf: add vmap and vunmap methods to udmabuf_ops
  2022-11-17 18:01       ` Christian König
@ 2022-11-18  9:42         ` Lukasz Wiecaszek
  0 siblings, 0 replies; 6+ messages in thread
From: Lukasz Wiecaszek @ 2022-11-18  9:42 UTC (permalink / raw)
  To: Christian König
  Cc: Dmitry Osipenko, Lukasz Wiecaszek, Gerd Hoffmann, Sumit Semwal,
	dri-devel, linux-media, linaro-mm-sig, linux-kernel

On Thu, Nov 17, 2022 at 07:01:05PM +0100, Christian König wrote:
> Am 17.11.22 um 18:32 schrieb Dmitry Osipenko:
> > On 11/17/22 20:08, Lukasz Wiecaszek wrote:
> > > On Thu, Nov 17, 2022 at 12:04:35PM +0300, Dmitry Osipenko wrote:
> > > > Hi,
> > > > 
> > > > On 11/17/22 07:58, Lukasz Wiecaszek wrote:
> > > > > The reason behind that patch is associated with videobuf2 subsystem
> > > > > (or more genrally with v4l2 framework) and user created
> > > > > dma buffers (udmabuf). In some circumstances
> > > > > when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem
> > > > > wants to use dma_buf_vmap() method on the attached dma buffer.
> > > > > As udmabuf does not have .vmap operation implemented,
> > > > > such dma_buf_vmap() natually fails.
> > > > > 
> > > > > videobuf2_common: __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each
> > > > > videobuf2_common: __prepare_dmabuf: buffer for plane 0 changed
> > > > > videobuf2_common: __prepare_dmabuf: failed to map dmabuf for plane 0
> > > > > videobuf2_common: __buf_prepare: buffer preparation failed: -14
> > > > > 
> > > > > The patch itself seems to be strighforward.
> > > > > It adds implementation of .vmap and .vunmap methods
> > > > > to 'struct dma_buf_ops udmabuf_ops'.
> > > > > .vmap method itself uses vm_map_ram() to map pages linearly
> > > > > into the kernel virtual address space.
> > > > > .vunmap removes mapping created earlier by .vmap.
> > > > > All locking and 'vmapping counting' is done in dma_buf.c
> > > > > so it seems to be redundant/unnecessary in .vmap/.vunmap.
> > > > > 
> > > > > Signed-off-by: Lukasz Wiecaszek <lukasz.wiecaszek@gmail.com>
> > > > If new patch version doesn't contain significant changes and you got
> > > > acks/reviews for the previous version, then you should add the given
> > > > acked-by and reviewed-by tags to the commit message by yourself.
> > > > 
> > > > -- 
> > > > Best regards,
> > > > Dmitry
> > > > 
> > > I would like to thank you all for your patience and on the same time say
> > > sorry that I still cannot follow the process (although I have read
> > > 'submitting patches' chapter).
> > If you'll continue to contribute actively, you'll find things that
> > aren't documented at all. Don't worry about it, usually somebody will
> > tell you about what's missing. Just apply the new knowledge next time ;)
> 
> Yeah, it's more learning by doing. Especially I suspect you don't have
> commit rights to drm-misc-next (or do you want to upstream it through some
> other branch?), so as soon as nobody has any more objections ping Dmitry or
> me to push this.
> 
> Cheers,
> Christian
> 
> PS: The Signed-of-by, Reviewed-by, Acked-by etc... lines are usually added
> in chronological order, e.g. your Signed-of-by line should always come
> first.
> 
>
Thanks one more time. Funny thing, but at the very beginning I had
Signed-of-by as the first line. Then I looked at 'git log' and spoted
different order, so I change mine as well. Ahhh. But this chronological
order of course make sense. So if you feel ok with this 'out of order'
issue, please push/merge this commit. If not, please let me know. I
already submitted version 5 of that work. So if change is required, I
will prepare version 6.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-18  9:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-17  4:58 [PATCH v4] udmabuf: add vmap and vunmap methods to udmabuf_ops Lukasz Wiecaszek
2022-11-17  9:04 ` Dmitry Osipenko
2022-11-17 17:08   ` Lukasz Wiecaszek
2022-11-17 17:32     ` Dmitry Osipenko
2022-11-17 18:01       ` Christian König
2022-11-18  9:42         ` Lukasz Wiecaszek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).