Hi Hans, On Mon, Nov 20, 2023 at 10:27 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > > Tomasz, > > Can you review this? I did review it a few weeks ago, with some comments pending: https://patchwork.kernel.org/project/linux-media/patch/20231031230435.3356103-1-m.grzeschik@xxxxxxxxxxxxxx/#25577798 Best, Tomasz > > Michael, I have one comment below: > > On 01/11/2023 00:04, Michael Grzeschik wrote: > > For dmabuf import users to be able to use the vaddr from another > > videobuf2-dma-sg source, the exporter needs to set a proper vaddr on > > vb2_dma_sg_dmabuf_ops_vmap callback. > > > > This patch adds vmap on map if buf->vaddr was not set, and also > > adds the vb2_dma_sg_dmabuf_ops_vunmap function to remove the mapping > > afterwards. > > > > Cc: stable@xxxxxxxxxx > > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx> > > > > --- > > v2 -> v3: resend as a single patch > > v1 -> v2: using vmap and vunmap instead of vm_map_ram and vm_unmap_ram > > > > .../media/common/videobuf2/videobuf2-dma-sg.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > index 28f3fdfe23a298..05b43edda94a7e 100644 > > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > @@ -489,11 +489,26 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf, > > { > > struct vb2_dma_sg_buf *buf = dbuf->priv; > > > > + if (!buf->vaddr) > > + buf->vaddr = vmap(buf->pages, buf->num_pages, VM_MAP, > > + PAGE_KERNEL); > > Shouldn't this check for success and return an error code if it fails? > > Regards, > > Hans > > > + > > iosys_map_set_vaddr(map, buf->vaddr); > > > > return 0; > > } > > > > +static void vb2_dma_sg_dmabuf_ops_vunmap(struct dma_buf *dbuf, > > + struct iosys_map *map) > > +{ > > + struct vb2_dma_sg_buf *buf = dbuf->priv; > > + > > + if (buf->vaddr) > > + vunmap(buf->vaddr); > > + > > + buf->vaddr = NULL; > > +} > > + > > static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf, > > struct vm_area_struct *vma) > > { > > @@ -508,6 +523,7 @@ static const struct dma_buf_ops vb2_dma_sg_dmabuf_ops = { > > .begin_cpu_access = vb2_dma_sg_dmabuf_ops_begin_cpu_access, > > .end_cpu_access = vb2_dma_sg_dmabuf_ops_end_cpu_access, > > .vmap = vb2_dma_sg_dmabuf_ops_vmap, > > + .vunmap = vb2_dma_sg_dmabuf_ops_vunmap, > > .mmap = vb2_dma_sg_dmabuf_ops_mmap, > > .release = vb2_dma_sg_dmabuf_ops_release, > > }; >
