On Thu, Nov 24, 2022 at 10:35 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>
> On 21/11/2022 00:44, Michael Grzeschik wrote:
> > The comments before the vm_map_ram function state that it should be used
> > for up to 256 KB only, and video buffers are definitely much larger. It
> > recommends using vmap in that case.
> >
> > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> > ---
> > drivers/media/common/videobuf2/videobuf2-dma-sg.c | 7 ++++---
>
> drivers/media/common/videobuf2/videobuf2-vmalloc.c uses it as well,
> probably also incorrectly. It makes sense to change that one as well.
Comparing vm_map_ram() and vmap(..., VM_MAP, PAGE_KERNEL), for blocks
bigger than VMAP_MAX_ALLOC they're equivalent and for smaller blocks
the former should be faster, so I don't see what's wrong with the
current code.
Best regards,
Tomasz
>
> Regards,
>
> Hans
>
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > index dcb8de5ab3e84a..e86621fba350f3 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > @@ -188,7 +188,7 @@ static void vb2_dma_sg_put(void *buf_priv)
> > dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,
> > DMA_ATTR_SKIP_CPU_SYNC);
> > if (buf->vaddr)
> > - vm_unmap_ram(buf->vaddr, buf->num_pages);
> > + vunmap(buf->vaddr);
> > sg_free_table(buf->dma_sgt);
> > while (--i >= 0)
> > __free_page(buf->pages[i]);
> > @@ -289,7 +289,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
> > __func__, buf->num_pages);
> > dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
> > if (buf->vaddr)
> > - vm_unmap_ram(buf->vaddr, buf->num_pages);
> > + vunmap(buf->vaddr);
> > sg_free_table(buf->dma_sgt);
> > if (buf->dma_dir == DMA_FROM_DEVICE ||
> > buf->dma_dir == DMA_BIDIRECTIONAL)
> > @@ -312,7 +312,8 @@ static void *vb2_dma_sg_vaddr(struct vb2_buffer *vb, void *buf_priv)
> > ret = dma_buf_vmap(buf->db_attach->dmabuf, &map);
> > buf->vaddr = ret ? NULL : map.vaddr;
> > } else {
> > - buf->vaddr = vm_map_ram(buf->pages, buf->num_pages, -1);
> > + buf->vaddr = vmap(buf->pages, buf->num_pages, VM_MAP,
> > + PAGE_KERNEL);
> > }
> > }
> >
>
