Hi Ezequiel, On Fri, Jul 30, 2021 at 01:13:20PM -0300, Ezequiel Garcia wrote: > On Fri, 30 Jul 2021 at 09:35, Dafna Hirschfeld wrote: > > On 30.07.21 02:19, Laurent Pinchart wrote: > > > The vimc driver is used for testing purpose, and some test use cases > > > involve sharing buffers with a consumer device. Consumers often require > > > DMA contiguous memory, which vimc doesn't currently support. This leads > > > in the best case to usage of bounce buffers, which is very slow, and in > > > the worst case in a complete failure. > > > > > > Add support for the dma-contig allocator in vimc to support those use > > > cases properly. The allocator is selected through a new "allocator" > > > module parameter, which defaults to vmalloc. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > > --- > > > drivers/media/test-drivers/vimc/vimc-capture.c | 9 +++++++-- > > > drivers/media/test-drivers/vimc/vimc-common.h | 2 ++ > > > drivers/media/test-drivers/vimc/vimc-core.c | 10 ++++++++++ > > > 3 files changed, 19 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/media/test-drivers/vimc/vimc-capture.c b/drivers/media/test-drivers/vimc/vimc-capture.c > > > index 5e9fd902cd37..92b69a6529fb 100644 > > > --- a/drivers/media/test-drivers/vimc/vimc-capture.c > > > +++ b/drivers/media/test-drivers/vimc/vimc-capture.c > > > @@ -7,6 +7,7 @@ > > > > > > #include <media/v4l2-ioctl.h> > > > #include <media/videobuf2-core.h> > > > +#include <media/videobuf2-dma-contig.h> > > > #include <media/videobuf2-vmalloc.h> > > > > > > #include "vimc-common.h" > > > @@ -423,14 +424,18 @@ static struct vimc_ent_device *vimc_cap_add(struct vimc_device *vimc, > > > /* Initialize the vb2 queue */ > > > q = &vcap->queue; > > > q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; > > > - q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_USERPTR; > > > + q->io_modes = VB2_MMAP | VB2_DMABUF; > > > > maybe to be on the safe side VB2_DMABUF should be set only if vimc_allocator==1 ? > > > > > + if (vimc_allocator != 1) > > > > maybe define a macro instead of `1` ? > > This is maybe an overkill, but you can make the parameter accept strings > instead of integers, which makes it "modprobe vimc allocator=vmalloc", > and improves a bit user-friendlyness. > > See drivers/media/pci/tw686x/tw686x-core.c. > > For a test driver, it is worth the trouble, maybe? I copied the values from vivid, which uses an array of integers. As vimc needs a single parameter only, a string could make more sense. I'll submit a v3 with a string if there's a consensus for that. -- Regards, Laurent Pinchart