[PATCH v2] media: vimc: Add support for contiguous DMA buffers

[PATCH v2] media: vimc: Add support for contiguous DMA buffers

[Laurent Pinchart]

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@ideasonboard.com>
---
Changes since v1:

- Add vimc_allocator_type enum
- Document the new parameter in vimc.rst
---
 Documentation/admin-guide/media/vimc.rst       | 10 +++++++++-
 drivers/media/test-drivers/vimc/vimc-capture.c |  9 +++++++--
 drivers/media/test-drivers/vimc/vimc-common.h  |  7 +++++++
 drivers/media/test-drivers/vimc/vimc-core.c    | 10 ++++++++++
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/media/vimc.rst b/Documentation/admin-guide/media/vimc.rst
index 211cc8972410..df57f67fef5f 100644
--- a/Documentation/admin-guide/media/vimc.rst
+++ b/Documentation/admin-guide/media/vimc.rst
@@ -80,7 +80,15 @@ vimc-capture:
 Module options
 --------------
 
-Vimc has a module parameter to configure the driver.
+Vimc has module parameters to configure the driver.
+
+* ``allocator=<unsigned int>``
+
+	memory allocator selection, default is 0. It specifies the way buffers
+	will be allocated.
+
+		- 0: vmalloc
+		- 1: dma-contig
 
 * ``sca_mult=<unsigned int>``
 
diff --git a/drivers/media/test-drivers/vimc/vimc-capture.c b/drivers/media/test-drivers/vimc/vimc-capture.c
index 5e9fd902cd37..d1e2d0739c00 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;
+	if (vimc_allocator == VIMC_ALLOCATOR_VMALLOC)
+		q->io_modes |= VB2_USERPTR;
 	q->drv_priv = vcap;
 	q->buf_struct_size = sizeof(struct vimc_cap_buffer);
 	q->ops = &vimc_cap_qops;
-	q->mem_ops = &vb2_vmalloc_memops;
+	q->mem_ops = vimc_allocator == VIMC_ALLOCATOR_DMA_CONTIG
+		   ? &vb2_dma_contig_memops : &vb2_vmalloc_memops;
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	q->min_buffers_needed = 2;
 	q->lock = &vcap->lock;
+	q->dev = v4l2_dev->dev;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
diff --git a/drivers/media/test-drivers/vimc/vimc-common.h b/drivers/media/test-drivers/vimc/vimc-common.h
index a289434e75ba..ba1930772589 100644
--- a/drivers/media/test-drivers/vimc/vimc-common.h
+++ b/drivers/media/test-drivers/vimc/vimc-common.h
@@ -35,6 +35,13 @@
 
 #define VIMC_PIX_FMT_MAX_CODES 8
 
+extern unsigned int vimc_allocator;
+
+enum vimc_allocator_type {
+	VIMC_ALLOCATOR_VMALLOC = 0,
+	VIMC_ALLOCATOR_DMA_CONTIG = 1,
+};
+
 /**
  * vimc_colorimetry_clamp - Adjust colorimetry parameters
  *
diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
index 4b0ae6f51d76..06edf9d4d92c 100644
--- a/drivers/media/test-drivers/vimc/vimc-core.c
+++ b/drivers/media/test-drivers/vimc/vimc-core.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2015-2017 Helen Koike <helen.fornazier@gmail.com>
  */
 
+#include <linux/dma-mapping.h>
 #include <linux/font.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -15,6 +16,12 @@
 
 #include "vimc-common.h"
 
+unsigned int vimc_allocator;
+module_param_named(allocator, vimc_allocator, uint, 0444);
+MODULE_PARM_DESC(allocator, " memory allocator selection, default is 0.\n"
+			     "\t\t    0 == vmalloc\n"
+			     "\t\t    1 == dma-contig");
+
 #define VIMC_MDEV_MODEL_NAME "VIMC MDEV"
 
 #define VIMC_ENT_LINK(src, srcpad, sink, sinkpad, link_flags) {	\
@@ -278,6 +285,9 @@ static int vimc_probe(struct platform_device *pdev)
 
 	tpg_set_font(font->data);
 
+	if (vimc_allocator == VIMC_ALLOCATOR_DMA_CONTIG)
+		dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+
 	vimc = kzalloc(sizeof(*vimc), GFP_KERNEL);
 	if (!vimc)
 		return -ENOMEM;
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] media: vimc: Add support for contiguous DMA buffers

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1804 bytes --]

Hi Laurent,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/media-vimc-Add-support-for-contiguous-DMA-buffers/20210730-211911
base:   git://linuxtv.org/media_tree.git master
config: h8300-randconfig-r013-20210730 (attached as .config)
compiler: h8300-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/e693ef0f7e48d92b7c227692e7a378845caff15a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Laurent-Pinchart/media-vimc-Add-support-for-contiguous-DMA-buffers/20210730-211911
        git checkout e693ef0f7e48d92b7c227692e7a378845caff15a
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=h8300 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   h8300-linux-ld: drivers/media/test-drivers/vimc/vimc-capture.o: in function `.L84':
>> vimc-capture.c:(.text+0x89a): undefined reference to `vb2_dma_contig_memops'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39922 bytes --]

Re: [PATCH v2] media: vimc: Add support for contiguous DMA buffers

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5499 bytes --]

Hi Laurent,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v5.14-rc4 next-20210804]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/media-vimc-Add-support-for-contiguous-DMA-buffers/20210730-211911
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-s021-20210805 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-348-gf0e6938b-dirty
        # https://github.com/0day-ci/linux/commit/e693ef0f7e48d92b7c227692e7a378845caff15a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Laurent-Pinchart/media-vimc-Add-support-for-contiguous-DMA-buffers/20210730-211911
        git checkout e693ef0f7e48d92b7c227692e7a378845caff15a
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/media/test-drivers/vimc/vimc-capture.o: in function `vimc_cap_add':
>> drivers/media/test-drivers/vimc/vimc-capture.c:434: undefined reference to `vb2_dma_contig_memops'


vim +434 drivers/media/test-drivers/vimc/vimc-capture.c

   396	
   397	static struct vimc_ent_device *vimc_cap_add(struct vimc_device *vimc,
   398						    const char *vcfg_name)
   399	{
   400		struct v4l2_device *v4l2_dev = &vimc->v4l2_dev;
   401		const struct vimc_pix_map *vpix;
   402		struct vimc_cap_device *vcap;
   403		struct video_device *vdev;
   404		struct vb2_queue *q;
   405		int ret;
   406	
   407		/* Allocate the vimc_cap_device struct */
   408		vcap = kzalloc(sizeof(*vcap), GFP_KERNEL);
   409		if (!vcap)
   410			return ERR_PTR(-ENOMEM);
   411	
   412		/* Initialize the media entity */
   413		vcap->vdev.entity.name = vcfg_name;
   414		vcap->vdev.entity.function = MEDIA_ENT_F_IO_V4L;
   415		vcap->pad.flags = MEDIA_PAD_FL_SINK;
   416		ret = media_entity_pads_init(&vcap->vdev.entity,
   417					     1, &vcap->pad);
   418		if (ret)
   419			goto err_free_vcap;
   420	
   421		/* Initialize the lock */
   422		mutex_init(&vcap->lock);
   423	
   424		/* Initialize the vb2 queue */
   425		q = &vcap->queue;
   426		q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   427		q->io_modes = VB2_MMAP | VB2_DMABUF;
   428		if (vimc_allocator == VIMC_ALLOCATOR_VMALLOC)
   429			q->io_modes |= VB2_USERPTR;
   430		q->drv_priv = vcap;
   431		q->buf_struct_size = sizeof(struct vimc_cap_buffer);
   432		q->ops = &vimc_cap_qops;
   433		q->mem_ops = vimc_allocator == VIMC_ALLOCATOR_DMA_CONTIG
 > 434			   ? &vb2_dma_contig_memops : &vb2_vmalloc_memops;
   435		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
   436		q->min_buffers_needed = 2;
   437		q->lock = &vcap->lock;
   438		q->dev = v4l2_dev->dev;
   439	
   440		ret = vb2_queue_init(q);
   441		if (ret) {
   442			dev_err(vimc->mdev.dev, "%s: vb2 queue init failed (err=%d)\n",
   443				vcfg_name, ret);
   444			goto err_clean_m_ent;
   445		}
   446	
   447		/* Initialize buffer list and its lock */
   448		INIT_LIST_HEAD(&vcap->buf_list);
   449		spin_lock_init(&vcap->qlock);
   450	
   451		/* Set default frame format */
   452		vcap->format = fmt_default;
   453		vpix = vimc_pix_map_by_pixelformat(vcap->format.pixelformat);
   454		vcap->format.bytesperline = vcap->format.width * vpix->bpp;
   455		vcap->format.sizeimage = vcap->format.bytesperline *
   456					 vcap->format.height;
   457	
   458		/* Fill the vimc_ent_device struct */
   459		vcap->ved.ent = &vcap->vdev.entity;
   460		vcap->ved.process_frame = vimc_cap_process_frame;
   461		vcap->ved.vdev_get_format = vimc_cap_get_format;
   462		vcap->ved.dev = vimc->mdev.dev;
   463	
   464		/* Initialize the video_device struct */
   465		vdev = &vcap->vdev;
   466		vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING
   467				  | V4L2_CAP_IO_MC;
   468		vdev->entity.ops = &vimc_cap_mops;
   469		vdev->release = video_device_release_empty;
   470		vdev->fops = &vimc_cap_fops;
   471		vdev->ioctl_ops = &vimc_cap_ioctl_ops;
   472		vdev->lock = &vcap->lock;
   473		vdev->queue = q;
   474		vdev->v4l2_dev = v4l2_dev;
   475		vdev->vfl_dir = VFL_DIR_RX;
   476		strscpy(vdev->name, vcfg_name, sizeof(vdev->name));
   477		video_set_drvdata(vdev, &vcap->ved);
   478	
   479		/* Register the video_device with the v4l2 and the media framework */
   480		ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
   481		if (ret) {
   482			dev_err(vimc->mdev.dev, "%s: video register failed (err=%d)\n",
   483				vcap->vdev.name, ret);
   484			goto err_clean_m_ent;
   485		}
   486	
   487		return &vcap->ved;
   488	
   489	err_clean_m_ent:
   490		media_entity_cleanup(&vcap->vdev.entity);
   491	err_free_vcap:
   492		kfree(vcap);
   493	
   494		return ERR_PTR(ret);
   495	}
   496	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34669 bytes --]