[PATCH v2] [media] vivid: support for contiguous DMA buffers

[PATCH v2] [media] vivid: support for contiguous DMA buffers

[Vincent Abriou]

It allows to simulate the behavior of hardware with such limitations or
to connect vivid to real hardware with such limitations.

Add the "allocators" module parameter option to let vivid use the
dma-contig instead of vmalloc.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Vincent Abriou <vincent.abriou@st.com>

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/media/v4l-drivers/vivid.rst |  8 ++++++++
 drivers/media/platform/vivid/Kconfig      |  2 ++
 drivers/media/platform/vivid/vivid-core.c | 32 ++++++++++++++++++++++++++-----
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/Documentation/media/v4l-drivers/vivid.rst b/Documentation/media/v4l-drivers/vivid.rst
index c8cf371..3e44b22 100644
--- a/Documentation/media/v4l-drivers/vivid.rst
+++ b/Documentation/media/v4l-drivers/vivid.rst
@@ -263,6 +263,14 @@ all configurable using the following module options:
 	removed. Unless overridden by ccs_cap_mode and/or ccs_out_mode the
 	will default to enabling crop, compose and scaling.
 
+- allocators:
+
+	memory allocator selection, default is 0. It specifies the way buffers
+	will be allocated.
+
+		- 0: vmalloc
+		- 1: dma-contig
+
 Taken together, all these module options allow you to precisely customize
 the driver behavior and test your application with all sorts of permutations.
 It is also very suitable to emulate hardware that is not yet available, e.g.
diff --git a/drivers/media/platform/vivid/Kconfig b/drivers/media/platform/vivid/Kconfig
index 8e6918c..2e238a1 100644
--- a/drivers/media/platform/vivid/Kconfig
+++ b/drivers/media/platform/vivid/Kconfig
@@ -1,6 +1,7 @@
 config VIDEO_VIVID
 	tristate "Virtual Video Test Driver"
 	depends on VIDEO_DEV && VIDEO_V4L2 && !SPARC32 && !SPARC64 && FB
+	depends on HAS_DMA
 	select FONT_SUPPORT
 	select FONT_8x16
 	select FB_CFB_FILLRECT
@@ -8,6 +9,7 @@ config VIDEO_VIVID
 	select FB_CFB_IMAGEBLIT
 	select MEDIA_CEC_EDID
 	select VIDEOBUF2_VMALLOC
+	select VIDEOBUF2_DMA_CONTIG
 	select VIDEO_V4L2_TPG
 	default n
 	---help---
diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
index 741460a..02e1909 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -30,6 +30,7 @@
 #include <linux/videodev2.h>
 #include <linux/v4l2-dv-timings.h>
 #include <media/videobuf2-vmalloc.h>
+#include <media/videobuf2-dma-contig.h>
 #include <media/v4l2-dv-timings.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-fh.h>
@@ -151,6 +152,12 @@ static bool no_error_inj;
 module_param(no_error_inj, bool, 0444);
 MODULE_PARM_DESC(no_error_inj, " if set disable the error injecting controls");
 
+static unsigned int allocators[VIVID_MAX_DEVS] = { [0 ... (VIVID_MAX_DEVS - 1)] = 0 };
+module_param_array(allocators, uint, NULL, 0444);
+MODULE_PARM_DESC(allocators, " memory allocator selection, default is 0.\n"
+			     "\t\t    0 == vmalloc\n"
+			     "\t\t    1 == dma-contig");
+
 static struct vivid_dev *vivid_devs[VIVID_MAX_DEVS];
 
 const struct v4l2_rect vivid_min_rect = {
@@ -636,6 +643,10 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 {
 	static const struct v4l2_dv_timings def_dv_timings =
 					V4L2_DV_BT_CEA_1280X720P60;
+	static const struct vb2_mem_ops * const vivid_mem_ops[2] = {
+		&vb2_vmalloc_memops,
+		&vb2_dma_contig_memops,
+	};
 	unsigned in_type_counter[4] = { 0, 0, 0, 0 };
 	unsigned out_type_counter[4] = { 0, 0, 0, 0 };
 	int ccs_cap = ccs_cap_mode[inst];
@@ -646,6 +657,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 	struct video_device *vfd;
 	struct vb2_queue *q;
 	unsigned node_type = node_types[inst];
+	unsigned int allocator = allocators[inst];
 	v4l2_std_id tvnorms_cap = 0, tvnorms_out = 0;
 	int ret;
 	int i;
@@ -1036,6 +1048,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 	if (!dev->cec_workqueue)
 		goto unreg_dev;
 
+	if (allocator == 1)
+		dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	else if (allocator >= ARRAY_SIZE(vivid_mem_ops))
+		allocator = 0;
+
 	/* start creating the vb2 queues */
 	if (dev->has_vid_cap) {
 		/* initialize vid_cap queue */
@@ -1046,10 +1063,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->drv_priv = dev;
 		q->buf_struct_size = sizeof(struct vivid_buffer);
 		q->ops = &vivid_vid_cap_qops;
-		q->mem_ops = &vb2_vmalloc_memops;
+		q->mem_ops = vivid_mem_ops[allocator];
 		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 		q->min_buffers_needed = 2;
 		q->lock = &dev->mutex;
+		q->dev = dev->v4l2_dev.dev;
 
 		ret = vb2_queue_init(q);
 		if (ret)
@@ -1065,10 +1083,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->drv_priv = dev;
 		q->buf_struct_size = sizeof(struct vivid_buffer);
 		q->ops = &vivid_vid_out_qops;
-		q->mem_ops = &vb2_vmalloc_memops;
+		q->mem_ops = vivid_mem_ops[allocator];
 		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 		q->min_buffers_needed = 2;
 		q->lock = &dev->mutex;
+		q->dev = dev->v4l2_dev.dev;
 
 		ret = vb2_queue_init(q);
 		if (ret)
@@ -1084,10 +1103,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->drv_priv = dev;
 		q->buf_struct_size = sizeof(struct vivid_buffer);
 		q->ops = &vivid_vbi_cap_qops;
-		q->mem_ops = &vb2_vmalloc_memops;
+		q->mem_ops = vivid_mem_ops[allocator];
 		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 		q->min_buffers_needed = 2;
 		q->lock = &dev->mutex;
+		q->dev = dev->v4l2_dev.dev;
 
 		ret = vb2_queue_init(q);
 		if (ret)
@@ -1103,10 +1123,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->drv_priv = dev;
 		q->buf_struct_size = sizeof(struct vivid_buffer);
 		q->ops = &vivid_vbi_out_qops;
-		q->mem_ops = &vb2_vmalloc_memops;
+		q->mem_ops = vivid_mem_ops[allocator];
 		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 		q->min_buffers_needed = 2;
 		q->lock = &dev->mutex;
+		q->dev = dev->v4l2_dev.dev;
 
 		ret = vb2_queue_init(q);
 		if (ret)
@@ -1121,10 +1142,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->drv_priv = dev;
 		q->buf_struct_size = sizeof(struct vivid_buffer);
 		q->ops = &vivid_sdr_cap_qops;
-		q->mem_ops = &vb2_vmalloc_memops;
+		q->mem_ops = vivid_mem_ops[allocator];
 		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 		q->min_buffers_needed = 8;
 		q->lock = &dev->mutex;
+		q->dev = dev->v4l2_dev.dev;
 
 		ret = vb2_queue_init(q);
 		if (ret)
-- 
1.9.1

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

[Javier Martinez Canillas]

Hello Vincent,

On Mon, Sep 12, 2016 at 4:47 AM, Vincent Abriou <vincent.abriou@st.com> wrote:
> It allows to simulate the behavior of hardware with such limitations or
> to connect vivid to real hardware with such limitations.
>
> Add the "allocators" module parameter option to let vivid use the
> dma-contig instead of vmalloc.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> ---

The patch looks good to me.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

I've also tested on an Exynos5 board to share DMA buffers between a
vivid capture device and the Exynos DRM driver, so:

Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Before $SUBJECT, when vivid was always using the vb2 vmalloc memory
allocator, the Exynos DRM driver wasn't able to import the dma-buf
because the GEM buffers are non-contiguous:

$ gst-launch-1.0 v4l2src device=/dev/video7 io-mode=dmabuf ! kmssink
Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
0:00:00.853895814  2957    0xd6260 ERROR           kmsallocator
gstkmsallocator.c:334:gst_kms_allocator_add_fb:<KMSMemory::allocator>
Failed to bind to framebuffer: Invalid argument (-22)

[ 1757.390564] [drm:exynos_drm_framebuffer_init] *ERROR* cannot use
this gem memory type for fb.

The issue goes away when using the the vb2 DMA contig memory allocator.

Best regards,
Javier

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

[Vincent ABRIOU]

Hi Hans,

Is there any issue so that those 2 patches cannot be merged?
[media] vivid: support for contiguous DMA buffer
[media] uvcvideo: support for contiguous DMA buffers

They both have same approach and have been tested against ARM and X86 
platform.

Thanks.
BR
Vincent

On 09/12/2016 05:56 PM, Javier Martinez Canillas wrote:
> Hello Vincent,
>
> On Mon, Sep 12, 2016 at 4:47 AM, Vincent Abriou <vincent.abriou@st.com> wrote:
>> It allows to simulate the behavior of hardware with such limitations or
>> to connect vivid to real hardware with such limitations.
>>
>> Add the "allocators" module parameter option to let vivid use the
>> dma-contig instead of vmalloc.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
>>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>
> The patch looks good to me.
>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
> I've also tested on an Exynos5 board to share DMA buffers between a
> vivid capture device and the Exynos DRM driver, so:
>
> Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
> Before $SUBJECT, when vivid was always using the vb2 vmalloc memory
> allocator, the Exynos DRM driver wasn't able to import the dma-buf
> because the GEM buffers are non-contiguous:
>
> $ gst-launch-1.0 v4l2src device=/dev/video7 io-mode=dmabuf ! kmssink
> Setting pipeline to PAUSED ...
> Pipeline is live and does not need PREROLL ...
> Setting pipeline to PLAYING ...
> New clock: GstSystemClock
> 0:00:00.853895814  2957    0xd6260 ERROR           kmsallocator
> gstkmsallocator.c:334:gst_kms_allocator_add_fb:<KMSMemory::allocator>
> Failed to bind to framebuffer: Invalid argument (-22)
>
> [ 1757.390564] [drm:exynos_drm_framebuffer_init] *ERROR* cannot use
> this gem memory type for fb.
>
> The issue goes away when using the the vb2 DMA contig memory allocator.
>
> Best regards,
> Javier
>

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

[Hans Verkuil]

On 11/22/16 14:18, Vincent ABRIOU wrote:
> Hi Hans,
>
> Is there any issue so that those 2 patches cannot be merged?
> [media] vivid: support for contiguous DMA buffer
> [media] uvcvideo: support for contiguous DMA buffers

Lack of time, really. I'll see if I can take a look at these soonish.

Regards,

	Hans

>
> They both have same approach and have been tested against ARM and X86
> platform.
>
> Thanks.
> BR
> Vincent
>
> On 09/12/2016 05:56 PM, Javier Martinez Canillas wrote:
>> Hello Vincent,
>>
>> On Mon, Sep 12, 2016 at 4:47 AM, Vincent Abriou <vincent.abriou@st.com> wrote:
>>> It allows to simulate the behavior of hardware with such limitations or
>>> to connect vivid to real hardware with such limitations.
>>>
>>> Add the "allocators" module parameter option to let vivid use the
>>> dma-contig instead of vmalloc.
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
>>>
>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
>>> ---
>>
>> The patch looks good to me.
>>
>> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> I've also tested on an Exynos5 board to share DMA buffers between a
>> vivid capture device and the Exynos DRM driver, so:
>>
>> Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> Before $SUBJECT, when vivid was always using the vb2 vmalloc memory
>> allocator, the Exynos DRM driver wasn't able to import the dma-buf
>> because the GEM buffers are non-contiguous:
>>
>> $ gst-launch-1.0 v4l2src device=/dev/video7 io-mode=dmabuf ! kmssink
>> Setting pipeline to PAUSED ...
>> Pipeline is live and does not need PREROLL ...
>> Setting pipeline to PLAYING ...
>> New clock: GstSystemClock
>> 0:00:00.853895814  2957    0xd6260 ERROR           kmsallocator
>> gstkmsallocator.c:334:gst_kms_allocator_add_fb:<KMSMemory::allocator>
>> Failed to bind to framebuffer: Invalid argument (-22)
>>
>> [ 1757.390564] [drm:exynos_drm_framebuffer_init] *ERROR* cannot use
>> this gem memory type for fb.
>>
>> The issue goes away when using the the vb2 DMA contig memory allocator.
>>
>> Best regards,
>> Javier
>> N�����r��y���b�X��ǧv�^�)޺{.n�+����{���bj)���w*\x1fjg���\x1e�����ݢj/���z�ޖ��2�ޙ���&�)ߡ�a��\x7f��\x1e�G���h�\x0f�j:+v���w�٥

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

[Hans Verkuil]

On 09/12/2016 10:47 AM, Vincent Abriou wrote:
> It allows to simulate the behavior of hardware with such limitations or
> to connect vivid to real hardware with such limitations.
> 
> Add the "allocators" module parameter option to let vivid use the
> dma-contig instead of vmalloc.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
> 
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  Documentation/media/v4l-drivers/vivid.rst |  8 ++++++++
>  drivers/media/platform/vivid/Kconfig      |  2 ++
>  drivers/media/platform/vivid/vivid-core.c | 32 ++++++++++++++++++++++++++-----
>  3 files changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/media/v4l-drivers/vivid.rst b/Documentation/media/v4l-drivers/vivid.rst
> index c8cf371..3e44b22 100644
> --- a/Documentation/media/v4l-drivers/vivid.rst
> +++ b/Documentation/media/v4l-drivers/vivid.rst
> @@ -263,6 +263,14 @@ all configurable using the following module options:
>  	removed. Unless overridden by ccs_cap_mode and/or ccs_out_mode the
>  	will default to enabling crop, compose and scaling.
>  
> +- allocators:
> +
> +	memory allocator selection, default is 0. It specifies the way buffers
> +	will be allocated.
> +
> +		- 0: vmalloc
> +		- 1: dma-contig

Could you add support for dma-sg as well? I think that would be fairly trivial (unless
I missed something).

Once that's added (or it's clear dma-sg won't work for some reason), then I'll merge this.

Regards,

	Hans

> +
>  Taken together, all these module options allow you to precisely customize
>  the driver behavior and test your application with all sorts of permutations.
>  It is also very suitable to emulate hardware that is not yet available, e.g.
> diff --git a/drivers/media/platform/vivid/Kconfig b/drivers/media/platform/vivid/Kconfig
> index 8e6918c..2e238a1 100644
> --- a/drivers/media/platform/vivid/Kconfig
> +++ b/drivers/media/platform/vivid/Kconfig
> @@ -1,6 +1,7 @@
>  config VIDEO_VIVID
>  	tristate "Virtual Video Test Driver"
>  	depends on VIDEO_DEV && VIDEO_V4L2 && !SPARC32 && !SPARC64 && FB
> +	depends on HAS_DMA
>  	select FONT_SUPPORT
>  	select FONT_8x16
>  	select FB_CFB_FILLRECT
> @@ -8,6 +9,7 @@ config VIDEO_VIVID
>  	select FB_CFB_IMAGEBLIT
>  	select MEDIA_CEC_EDID
>  	select VIDEOBUF2_VMALLOC
> +	select VIDEOBUF2_DMA_CONTIG
>  	select VIDEO_V4L2_TPG
>  	default n
>  	---help---
> diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
> index 741460a..02e1909 100644
> --- a/drivers/media/platform/vivid/vivid-core.c
> +++ b/drivers/media/platform/vivid/vivid-core.c
> @@ -30,6 +30,7 @@
>  #include <linux/videodev2.h>
>  #include <linux/v4l2-dv-timings.h>
>  #include <media/videobuf2-vmalloc.h>
> +#include <media/videobuf2-dma-contig.h>
>  #include <media/v4l2-dv-timings.h>
>  #include <media/v4l2-ioctl.h>
>  #include <media/v4l2-fh.h>
> @@ -151,6 +152,12 @@ static bool no_error_inj;
>  module_param(no_error_inj, bool, 0444);
>  MODULE_PARM_DESC(no_error_inj, " if set disable the error injecting controls");
>  
> +static unsigned int allocators[VIVID_MAX_DEVS] = { [0 ... (VIVID_MAX_DEVS - 1)] = 0 };
> +module_param_array(allocators, uint, NULL, 0444);
> +MODULE_PARM_DESC(allocators, " memory allocator selection, default is 0.\n"
> +			     "\t\t    0 == vmalloc\n"
> +			     "\t\t    1 == dma-contig");
> +
>  static struct vivid_dev *vivid_devs[VIVID_MAX_DEVS];
>  
>  const struct v4l2_rect vivid_min_rect = {
> @@ -636,6 +643,10 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>  {
>  	static const struct v4l2_dv_timings def_dv_timings =
>  					V4L2_DV_BT_CEA_1280X720P60;
> +	static const struct vb2_mem_ops * const vivid_mem_ops[2] = {
> +		&vb2_vmalloc_memops,
> +		&vb2_dma_contig_memops,
> +	};
>  	unsigned in_type_counter[4] = { 0, 0, 0, 0 };
>  	unsigned out_type_counter[4] = { 0, 0, 0, 0 };
>  	int ccs_cap = ccs_cap_mode[inst];
> @@ -646,6 +657,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>  	struct video_device *vfd;
>  	struct vb2_queue *q;
>  	unsigned node_type = node_types[inst];
> +	unsigned int allocator = allocators[inst];
>  	v4l2_std_id tvnorms_cap = 0, tvnorms_out = 0;
>  	int ret;
>  	int i;
> @@ -1036,6 +1048,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>  	if (!dev->cec_workqueue)
>  		goto unreg_dev;
>  
> +	if (allocator == 1)
> +		dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	else if (allocator >= ARRAY_SIZE(vivid_mem_ops))
> +		allocator = 0;
> +
>  	/* start creating the vb2 queues */
>  	if (dev->has_vid_cap) {
>  		/* initialize vid_cap queue */
> @@ -1046,10 +1063,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>  		q->drv_priv = dev;
>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>  		q->ops = &vivid_vid_cap_qops;
> -		q->mem_ops = &vb2_vmalloc_memops;
> +		q->mem_ops = vivid_mem_ops[allocator];
>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  		q->min_buffers_needed = 2;
>  		q->lock = &dev->mutex;
> +		q->dev = dev->v4l2_dev.dev;
>  
>  		ret = vb2_queue_init(q);
>  		if (ret)
> @@ -1065,10 +1083,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>  		q->drv_priv = dev;
>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>  		q->ops = &vivid_vid_out_qops;
> -		q->mem_ops = &vb2_vmalloc_memops;
> +		q->mem_ops = vivid_mem_ops[allocator];
>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  		q->min_buffers_needed = 2;
>  		q->lock = &dev->mutex;
> +		q->dev = dev->v4l2_dev.dev;
>  
>  		ret = vb2_queue_init(q);
>  		if (ret)
> @@ -1084,10 +1103,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>  		q->drv_priv = dev;
>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>  		q->ops = &vivid_vbi_cap_qops;
> -		q->mem_ops = &vb2_vmalloc_memops;
> +		q->mem_ops = vivid_mem_ops[allocator];
>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  		q->min_buffers_needed = 2;
>  		q->lock = &dev->mutex;
> +		q->dev = dev->v4l2_dev.dev;
>  
>  		ret = vb2_queue_init(q);
>  		if (ret)
> @@ -1103,10 +1123,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>  		q->drv_priv = dev;
>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>  		q->ops = &vivid_vbi_out_qops;
> -		q->mem_ops = &vb2_vmalloc_memops;
> +		q->mem_ops = vivid_mem_ops[allocator];
>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  		q->min_buffers_needed = 2;
>  		q->lock = &dev->mutex;
> +		q->dev = dev->v4l2_dev.dev;
>  
>  		ret = vb2_queue_init(q);
>  		if (ret)
> @@ -1121,10 +1142,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>  		q->drv_priv = dev;
>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>  		q->ops = &vivid_sdr_cap_qops;
> -		q->mem_ops = &vb2_vmalloc_memops;
> +		q->mem_ops = vivid_mem_ops[allocator];
>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>  		q->min_buffers_needed = 8;
>  		q->lock = &dev->mutex;
> +		q->dev = dev->v4l2_dev.dev;
>  
>  		ret = vb2_queue_init(q);
>  		if (ret)
> 

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

[Hans Verkuil]

On 11/22/2016 02:18 PM, Vincent ABRIOU wrote:
> Hi Hans,
> 
> Is there any issue so that those 2 patches cannot be merged?
> [media] vivid: support for contiguous DMA buffer

I've requested support for dma-sg, see my reply to the patch. Looks good
otherwise.

> [media] uvcvideo: support for contiguous DMA buffers

This is up to Laurent (CC-ed).

Regards,

	Hans

> 
> They both have same approach and have been tested against ARM and X86 
> platform.
> 
> Thanks.
> BR
> Vincent
> 
> On 09/12/2016 05:56 PM, Javier Martinez Canillas wrote:
>> Hello Vincent,
>>
>> On Mon, Sep 12, 2016 at 4:47 AM, Vincent Abriou <vincent.abriou@st.com> wrote:
>>> It allows to simulate the behavior of hardware with such limitations or
>>> to connect vivid to real hardware with such limitations.
>>>
>>> Add the "allocators" module parameter option to let vivid use the
>>> dma-contig instead of vmalloc.
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
>>>
>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
>>> ---
>>
>> The patch looks good to me.
>>
>> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> I've also tested on an Exynos5 board to share DMA buffers between a
>> vivid capture device and the Exynos DRM driver, so:
>>
>> Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> Before $SUBJECT, when vivid was always using the vb2 vmalloc memory
>> allocator, the Exynos DRM driver wasn't able to import the dma-buf
>> because the GEM buffers are non-contiguous:
>>
>> $ gst-launch-1.0 v4l2src device=/dev/video7 io-mode=dmabuf ! kmssink
>> Setting pipeline to PAUSED ...
>> Pipeline is live and does not need PREROLL ...
>> Setting pipeline to PLAYING ...
>> New clock: GstSystemClock
>> 0:00:00.853895814  2957    0xd6260 ERROR           kmsallocator
>> gstkmsallocator.c:334:gst_kms_allocator_add_fb:<KMSMemory::allocator>
>> Failed to bind to framebuffer: Invalid argument (-22)
>>
>> [ 1757.390564] [drm:exynos_drm_framebuffer_init] *ERROR* cannot use
>> this gem memory type for fb.
>>
>> The issue goes away when using the the vb2 DMA contig memory allocator.
>>
>> Best regards,
>> Javier
>> N�����r��y���b�X��ǧv�^�)޺{.n�+����{���bj)���w*\x1fjg���\x1e�����ݢj/���z�ޖ��2�ޙ���&�)ߡ�a��\x7f��\x1e�G���h�\x0f�j:+v���w�٥

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

[Vincent ABRIOU]

On 01/09/2017 04:10 PM, Hans Verkuil wrote:
> On 09/12/2016 10:47 AM, Vincent Abriou wrote:
>> It allows to simulate the behavior of hardware with such limitations or
>> to connect vivid to real hardware with such limitations.
>>
>> Add the "allocators" module parameter option to let vivid use the
>> dma-contig instead of vmalloc.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
>>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  Documentation/media/v4l-drivers/vivid.rst |  8 ++++++++
>>  drivers/media/platform/vivid/Kconfig      |  2 ++
>>  drivers/media/platform/vivid/vivid-core.c | 32 ++++++++++++++++++++++++++-----
>>  3 files changed, 37 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/media/v4l-drivers/vivid.rst b/Documentation/media/v4l-drivers/vivid.rst
>> index c8cf371..3e44b22 100644
>> --- a/Documentation/media/v4l-drivers/vivid.rst
>> +++ b/Documentation/media/v4l-drivers/vivid.rst
>> @@ -263,6 +263,14 @@ all configurable using the following module options:
>>  	removed. Unless overridden by ccs_cap_mode and/or ccs_out_mode the
>>  	will default to enabling crop, compose and scaling.
>>
>> +- allocators:
>> +
>> +	memory allocator selection, default is 0. It specifies the way buffers
>> +	will be allocated.
>> +
>> +		- 0: vmalloc
>> +		- 1: dma-contig
>
> Could you add support for dma-sg as well? I think that would be fairly trivial (unless
> I missed something).
>
> Once that's added (or it's clear dma-sg won't work for some reason), then I'll merge this.
>
> Regards,
>
> 	Hans
>

Hi Hans,

What is the difference between a vmalloc allocation exported in DMABUF 
that will populate the sg and dma-sg allocation?

BR
Vincent

>> +
>>  Taken together, all these module options allow you to precisely customize
>>  the driver behavior and test your application with all sorts of permutations.
>>  It is also very suitable to emulate hardware that is not yet available, e.g.
>> diff --git a/drivers/media/platform/vivid/Kconfig b/drivers/media/platform/vivid/Kconfig
>> index 8e6918c..2e238a1 100644
>> --- a/drivers/media/platform/vivid/Kconfig
>> +++ b/drivers/media/platform/vivid/Kconfig
>> @@ -1,6 +1,7 @@
>>  config VIDEO_VIVID
>>  	tristate "Virtual Video Test Driver"
>>  	depends on VIDEO_DEV && VIDEO_V4L2 && !SPARC32 && !SPARC64 && FB
>> +	depends on HAS_DMA
>>  	select FONT_SUPPORT
>>  	select FONT_8x16
>>  	select FB_CFB_FILLRECT
>> @@ -8,6 +9,7 @@ config VIDEO_VIVID
>>  	select FB_CFB_IMAGEBLIT
>>  	select MEDIA_CEC_EDID
>>  	select VIDEOBUF2_VMALLOC
>> +	select VIDEOBUF2_DMA_CONTIG
>>  	select VIDEO_V4L2_TPG
>>  	default n
>>  	---help---
>> diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
>> index 741460a..02e1909 100644
>> --- a/drivers/media/platform/vivid/vivid-core.c
>> +++ b/drivers/media/platform/vivid/vivid-core.c
>> @@ -30,6 +30,7 @@
>>  #include <linux/videodev2.h>
>>  #include <linux/v4l2-dv-timings.h>
>>  #include <media/videobuf2-vmalloc.h>
>> +#include <media/videobuf2-dma-contig.h>
>>  #include <media/v4l2-dv-timings.h>
>>  #include <media/v4l2-ioctl.h>
>>  #include <media/v4l2-fh.h>
>> @@ -151,6 +152,12 @@ static bool no_error_inj;
>>  module_param(no_error_inj, bool, 0444);
>>  MODULE_PARM_DESC(no_error_inj, " if set disable the error injecting controls");
>>
>> +static unsigned int allocators[VIVID_MAX_DEVS] = { [0 ... (VIVID_MAX_DEVS - 1)] = 0 };
>> +module_param_array(allocators, uint, NULL, 0444);
>> +MODULE_PARM_DESC(allocators, " memory allocator selection, default is 0.\n"
>> +			     "\t\t    0 == vmalloc\n"
>> +			     "\t\t    1 == dma-contig");
>> +
>>  static struct vivid_dev *vivid_devs[VIVID_MAX_DEVS];
>>
>>  const struct v4l2_rect vivid_min_rect = {
>> @@ -636,6 +643,10 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>>  {
>>  	static const struct v4l2_dv_timings def_dv_timings =
>>  					V4L2_DV_BT_CEA_1280X720P60;
>> +	static const struct vb2_mem_ops * const vivid_mem_ops[2] = {
>> +		&vb2_vmalloc_memops,
>> +		&vb2_dma_contig_memops,
>> +	};
>>  	unsigned in_type_counter[4] = { 0, 0, 0, 0 };
>>  	unsigned out_type_counter[4] = { 0, 0, 0, 0 };
>>  	int ccs_cap = ccs_cap_mode[inst];
>> @@ -646,6 +657,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>>  	struct video_device *vfd;
>>  	struct vb2_queue *q;
>>  	unsigned node_type = node_types[inst];
>> +	unsigned int allocator = allocators[inst];
>>  	v4l2_std_id tvnorms_cap = 0, tvnorms_out = 0;
>>  	int ret;
>>  	int i;
>> @@ -1036,6 +1048,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>>  	if (!dev->cec_workqueue)
>>  		goto unreg_dev;
>>
>> +	if (allocator == 1)
>> +		dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> +	else if (allocator >= ARRAY_SIZE(vivid_mem_ops))
>> +		allocator = 0;
>> +
>>  	/* start creating the vb2 queues */
>>  	if (dev->has_vid_cap) {
>>  		/* initialize vid_cap queue */
>> @@ -1046,10 +1063,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>>  		q->drv_priv = dev;
>>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>>  		q->ops = &vivid_vid_cap_qops;
>> -		q->mem_ops = &vb2_vmalloc_memops;
>> +		q->mem_ops = vivid_mem_ops[allocator];
>>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>>  		q->min_buffers_needed = 2;
>>  		q->lock = &dev->mutex;
>> +		q->dev = dev->v4l2_dev.dev;
>>
>>  		ret = vb2_queue_init(q);
>>  		if (ret)
>> @@ -1065,10 +1083,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>>  		q->drv_priv = dev;
>>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>>  		q->ops = &vivid_vid_out_qops;
>> -		q->mem_ops = &vb2_vmalloc_memops;
>> +		q->mem_ops = vivid_mem_ops[allocator];
>>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>>  		q->min_buffers_needed = 2;
>>  		q->lock = &dev->mutex;
>> +		q->dev = dev->v4l2_dev.dev;
>>
>>  		ret = vb2_queue_init(q);
>>  		if (ret)
>> @@ -1084,10 +1103,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>>  		q->drv_priv = dev;
>>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>>  		q->ops = &vivid_vbi_cap_qops;
>> -		q->mem_ops = &vb2_vmalloc_memops;
>> +		q->mem_ops = vivid_mem_ops[allocator];
>>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>>  		q->min_buffers_needed = 2;
>>  		q->lock = &dev->mutex;
>> +		q->dev = dev->v4l2_dev.dev;
>>
>>  		ret = vb2_queue_init(q);
>>  		if (ret)
>> @@ -1103,10 +1123,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>>  		q->drv_priv = dev;
>>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>>  		q->ops = &vivid_vbi_out_qops;
>> -		q->mem_ops = &vb2_vmalloc_memops;
>> +		q->mem_ops = vivid_mem_ops[allocator];
>>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>>  		q->min_buffers_needed = 2;
>>  		q->lock = &dev->mutex;
>> +		q->dev = dev->v4l2_dev.dev;
>>
>>  		ret = vb2_queue_init(q);
>>  		if (ret)
>> @@ -1121,10 +1142,11 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>>  		q->drv_priv = dev;
>>  		q->buf_struct_size = sizeof(struct vivid_buffer);
>>  		q->ops = &vivid_sdr_cap_qops;
>> -		q->mem_ops = &vb2_vmalloc_memops;
>> +		q->mem_ops = vivid_mem_ops[allocator];
>>  		q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>>  		q->min_buffers_needed = 8;
>>  		q->lock = &dev->mutex;
>> +		q->dev = dev->v4l2_dev.dev;
>>
>>  		ret = vb2_queue_init(q);
>>  		if (ret)
>>
>

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

[Hans Verkuil]

On 01/10/2017 12:10 PM, Vincent ABRIOU wrote:
> 
> 
> On 01/09/2017 04:10 PM, Hans Verkuil wrote:
>> On 09/12/2016 10:47 AM, Vincent Abriou wrote:
>>> It allows to simulate the behavior of hardware with such limitations or
>>> to connect vivid to real hardware with such limitations.
>>>
>>> Add the "allocators" module parameter option to let vivid use the
>>> dma-contig instead of vmalloc.
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
>>>
>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
>>> ---
>>>  Documentation/media/v4l-drivers/vivid.rst |  8 ++++++++
>>>  drivers/media/platform/vivid/Kconfig      |  2 ++
>>>  drivers/media/platform/vivid/vivid-core.c | 32 ++++++++++++++++++++++++++-----
>>>  3 files changed, 37 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/media/v4l-drivers/vivid.rst b/Documentation/media/v4l-drivers/vivid.rst
>>> index c8cf371..3e44b22 100644
>>> --- a/Documentation/media/v4l-drivers/vivid.rst
>>> +++ b/Documentation/media/v4l-drivers/vivid.rst
>>> @@ -263,6 +263,14 @@ all configurable using the following module options:
>>>  	removed. Unless overridden by ccs_cap_mode and/or ccs_out_mode the
>>>  	will default to enabling crop, compose and scaling.
>>>
>>> +- allocators:
>>> +
>>> +	memory allocator selection, default is 0. It specifies the way buffers
>>> +	will be allocated.
>>> +
>>> +		- 0: vmalloc
>>> +		- 1: dma-contig
>>
>> Could you add support for dma-sg as well? I think that would be fairly trivial (unless
>> I missed something).
>>
>> Once that's added (or it's clear dma-sg won't work for some reason), then I'll merge this.
>>
>> Regards,
>>
>> 	Hans
>>
> 
> Hi Hans,
> 
> What is the difference between a vmalloc allocation exported in DMABUF 
> that will populate the sg and dma-sg allocation?

True. That wouldn't matter.

I'm merging this for 4.12.

Thanks!

	Hans