Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1500884 > unrolled thread

[PATCH 0/2] [media] DMA direction support in vb2_queue

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] [media] DMA direction support in vb2_queue Thierry Escande <thierry.escande@collabora.com> - 2016-10-14 14:20 +0200
    [PATCH 1/2] [media] vb2: Store dma_dir in vb2_queue Thierry Escande <thierry.escande@collabora.com> - 2016-10-14 14:20 +0200
    [PATCH 2/2] [media] vb2: Add support for use_dma_bidirectional queue flag Thierry Escande <thierry.escande@collabora.com> - 2016-10-14 14:20 +0200
      Re: [PATCH 2/2] [media] vb2: Add support for use_dma_bidirectional  queue flag Sakari Ailus <sakari.ailus@iki.fi> - 2016-10-17 12:20 +0200
        Re: [PATCH 2/2] [media] vb2: Add support for use_dma_bidirectional  queue flag Thierry Escande <thierry.escande@collabora.com> - 2016-10-18 18:10 +0200
          Re: [PATCH 2/2] [media] vb2: Add support for use_dma_bidirectional  queue flag Sakari Ailus <sakari.ailus@iki.fi> - 2016-10-19 17:20 +0200

#1500884 — [PATCH 0/2] [media] DMA direction support in vb2_queue

Hi,

This series adds a dma_dir field to the vb2_queue structure in order to
store the DMA direction once for all in vb2_queue_init();

It also adds a new use_dma_bidirectional flag to the vb2_queue structure
allowing the hardware to read from the CAPTURE buffer. This flag is
ignored for OUTPUT queues. This is used on ChromeOS by the rockchip-vpu
driver.

Pawel Osciak (2):
  [media] vb2: Store dma_dir in vb2_queue
  [media] vb2: Add support for use_dma_bidirectional queue flag

 drivers/media/v4l2-core/videobuf2-core.c | 12 +++---------
 drivers/media/v4l2-core/videobuf2-v4l2.c |  6 ++++++
 include/media/videobuf2-core.h           |  6 ++++++
 3 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1500885 — [PATCH 1/2] [media] vb2: Store dma_dir in vb2_queue

From: Pawel Osciak <posciak@chromium.org>

Store dma_dir in struct vb2_queue and reuse it, instead of recalculating
it each time.

Signed-off-by: Pawel Osciak <posciak@chromium.org>
Tested-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-by: Owen Lin <owenlin@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 12 +++---------
 drivers/media/v4l2-core/videobuf2-v4l2.c |  2 ++
 include/media/videobuf2-core.h           |  2 ++
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 21900202..f12103c 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -194,8 +194,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb);
 static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
-	enum dma_data_direction dma_dir =
-		q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	void *mem_priv;
 	int plane;
 	int ret = -ENOMEM;
@@ -209,7 +207,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 
 		mem_priv = call_ptr_memop(vb, alloc,
 				q->alloc_devs[plane] ? : q->dev,
-				q->dma_attrs, size, dma_dir, q->gfp_flags);
+				q->dma_attrs, size, q->dma_dir, q->gfp_flags);
 		if (IS_ERR(mem_priv)) {
 			if (mem_priv)
 				ret = PTR_ERR(mem_priv);
@@ -978,8 +976,6 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const void *pb)
 	void *mem_priv;
 	unsigned int plane;
 	int ret = 0;
-	enum dma_data_direction dma_dir =
-		q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	bool reacquired = vb->planes[0].mem_priv == NULL;
 
 	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1030,7 +1026,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const void *pb)
 		mem_priv = call_ptr_memop(vb, get_userptr,
 				q->alloc_devs[plane] ? : q->dev,
 				planes[plane].m.userptr,
-				planes[plane].length, dma_dir);
+				planes[plane].length, q->dma_dir);
 		if (IS_ERR(mem_priv)) {
 			dprintk(1, "failed acquiring userspace "
 						"memory for plane %d\n", plane);
@@ -1096,8 +1092,6 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const void *pb)
 	void *mem_priv;
 	unsigned int plane;
 	int ret = 0;
-	enum dma_data_direction dma_dir =
-		q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	bool reacquired = vb->planes[0].mem_priv == NULL;
 
 	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1156,7 +1150,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const void *pb)
 		/* Acquire each plane's memory */
 		mem_priv = call_ptr_memop(vb, attach_dmabuf,
 				q->alloc_devs[plane] ? : q->dev,
-				dbuf, planes[plane].length, dma_dir);
+				dbuf, planes[plane].length, q->dma_dir);
 		if (IS_ERR(mem_priv)) {
 			dprintk(1, "failed to attach dmabuf\n");
 			ret = PTR_ERR(mem_priv);
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 52ef883..fde1e2d 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -659,6 +659,8 @@ int vb2_queue_init(struct vb2_queue *q)
 	 * queues will always initialize waiting_for_buffers to false.
 	 */
 	q->quirk_poll_must_check_waiting_for_buffers = true;
+	q->dma_dir = V4L2_TYPE_IS_OUTPUT(q->type)
+		   ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 
 	return vb2_core_queue_init(q);
 }
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index ac5898a..38410dd 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -489,6 +489,7 @@ struct vb2_buf_ops {
  *		when a buffer with the V4L2_BUF_FLAG_LAST is dequeued.
  * @fileio:	file io emulator internal data, used only if emulator is active
  * @threadio:	thread io internal data, used only if thread is active
+ * @dma_dir:	DMA direction to use for buffers on this queue
  */
 struct vb2_queue {
 	unsigned int			type;
@@ -540,6 +541,7 @@ struct vb2_queue {
 
 	struct vb2_fileio_data		*fileio;
 	struct vb2_threadio_data	*threadio;
+	enum dma_data_direction		dma_dir;
 
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	/*
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1500886 — [PATCH 2/2] [media] vb2: Add support for use_dma_bidirectional queue flag

From: Pawel Osciak <posciak@chromium.org>

When this flag is set for CAPTURE queues by the driver on calling
vb2_queue_init(), it forces the buffers on the queue to be
allocated/mapped with DMA_BIDIRECTIONAL direction flag, instead of
DMA_FROM_DEVICE. This allows the device not only to write to the
buffers, but also read out from them. This may be useful e.g. for codec
hardware, which may be using CAPTURE buffers as reference to decode
other buffers.

This flag is ignored for OUTPUT queues, as we don't want to allow HW to
be able to write to OUTPUT buffers.

Signed-off-by: Pawel Osciak <posciak@chromium.org>
Tested-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/media/v4l2-core/videobuf2-v4l2.c | 8 ++++++--
 include/media/videobuf2-core.h           | 4 ++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c
index fde1e2d..9255291 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -659,8 +659,12 @@ int vb2_queue_init(struct vb2_queue *q)
 	 * queues will always initialize waiting_for_buffers to false.
 	 */
 	q->quirk_poll_must_check_waiting_for_buffers = true;
-	q->dma_dir = V4L2_TYPE_IS_OUTPUT(q->type)
-		   ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+
+	if (V4L2_TYPE_IS_OUTPUT(q->type))
+		q->dma_dir = DMA_TO_DEVICE;
+	else
+		q->dma_dir = q->use_dma_bidirectional
+			   ? DMA_BIDIRECTIONAL : DMA_FROM_DEVICE;
 
 	return vb2_core_queue_init(q);
 }
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 38410dd..e613c74 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -433,6 +433,9 @@ struct vb2_buf_ops {
  * @quirk_poll_must_check_waiting_for_buffers: Return POLLERR at poll when QBUF
  *              has not been called. This is a vb1 idiom that has been adopted
  *              also by vb2.
+ * @use_dma_bidirectional:	use DMA_BIDIRECTIONAL for CAPTURE buffers; this
+ *				allows HW to read from the CAPTURE buffers in
+ *				addition to writing; ignored for OUTPUT queues
  * @lock:	pointer to a mutex that protects the vb2_queue struct. The
  *		driver can set this to a mutex to let the v4l2 core serialize
  *		the queuing ioctls. If the driver wants to handle locking
@@ -500,6 +503,7 @@ struct vb2_queue {
 	unsigned			fileio_write_immediately:1;
 	unsigned			allow_zero_bytesused:1;
 	unsigned		   quirk_poll_must_check_waiting_for_buffers:1;
+	unsigned			use_dma_bidirectional:1;
 
 	struct mutex			*lock;
 	void				*owner;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1501852 — Re: [PATCH 2/2] [media] vb2: Add support for use_dma_bidirectional queue flag

Hi Thierry,

Thanks for the set. A few comments below.

On Fri, Oct 14, 2016 at 02:08:14PM +0200, Thierry Escande wrote:
> From: Pawel Osciak <posciak@chromium.org>
> 
> When this flag is set for CAPTURE queues by the driver on calling
> vb2_queue_init(), it forces the buffers on the queue to be
> allocated/mapped with DMA_BIDIRECTIONAL direction flag, instead of
> DMA_FROM_DEVICE. This allows the device not only to write to the
> buffers, but also read out from them. This may be useful e.g. for codec
> hardware, which may be using CAPTURE buffers as reference to decode
> other buffers.

Just out of curiosity --- when do you return these buffers back to the user?
Once they're no longer needed as reference frames?

> 
> This flag is ignored for OUTPUT queues, as we don't want to allow HW to
> be able to write to OUTPUT buffers.
> 
> Signed-off-by: Pawel Osciak <posciak@chromium.org>
> Tested-by: Pawel Osciak <posciak@chromium.org>
> Reviewed-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  drivers/media/v4l2-core/videobuf2-v4l2.c | 8 ++++++--
>  include/media/videobuf2-core.h           | 4 ++++
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c
> index fde1e2d..9255291 100644
> --- a/drivers/media/v4l2-core/videobuf2-v4l2.c
> +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
> @@ -659,8 +659,12 @@ int vb2_queue_init(struct vb2_queue *q)
>  	 * queues will always initialize waiting_for_buffers to false.
>  	 */
>  	q->quirk_poll_must_check_waiting_for_buffers = true;
> -	q->dma_dir = V4L2_TYPE_IS_OUTPUT(q->type)
> -		   ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> +
> +	if (V4L2_TYPE_IS_OUTPUT(q->type))
> +		q->dma_dir = DMA_TO_DEVICE;
> +	else
> +		q->dma_dir = q->use_dma_bidirectional
> +			   ? DMA_BIDIRECTIONAL : DMA_FROM_DEVICE;
>  
>  	return vb2_core_queue_init(q);
>  }
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 38410dd..e613c74 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -433,6 +433,9 @@ struct vb2_buf_ops {
>   * @quirk_poll_must_check_waiting_for_buffers: Return POLLERR at poll when QBUF
>   *              has not been called. This is a vb1 idiom that has been adopted
>   *              also by vb2.
> + * @use_dma_bidirectional:	use DMA_BIDIRECTIONAL for CAPTURE buffers; this
> + *				allows HW to read from the CAPTURE buffers in
> + *				addition to writing; ignored for OUTPUT queues
>   * @lock:	pointer to a mutex that protects the vb2_queue struct. The
>   *		driver can set this to a mutex to let the v4l2 core serialize
>   *		the queuing ioctls. If the driver wants to handle locking
> @@ -500,6 +503,7 @@ struct vb2_queue {
>  	unsigned			fileio_write_immediately:1;
>  	unsigned			allow_zero_bytesused:1;
>  	unsigned		   quirk_poll_must_check_waiting_for_buffers:1;
> +	unsigned			use_dma_bidirectional:1;

This field is in the same struct as dma_dir which it directly affects.

How about adding a macro instead to give you the queue DMA direction
instead?

E.g.

#define vb2_dma_dir(q) \
	(V4L2_TYPE_IS_OUTPUT((q)->type) ? DMA_TO_DEVICE : \
	 (q)->use_dma_bidirectional ? DMA_BIDIRECTIONAL : DMA_FROM_DEVICE)

I would call this capture_dma_bidirectional as it only affects capture. Or
simply choose DMA_BIDIRECTIONAL whenever the flag is set.

I wonder what others think.

>  
>  	struct mutex			*lock;
>  	void				*owner;

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1503193 — Re: [PATCH 2/2] [media] vb2: Add support for use_dma_bidirectional queue flag

Hi Sakari,

On 17/10/2016 12:06, Sakari Ailus wrote:
> Hi Thierry,
>
> Thanks for the set. A few comments below.
>
> On Fri, Oct 14, 2016 at 02:08:14PM +0200, Thierry Escande wrote:
>> From: Pawel Osciak <posciak@chromium.org>
>>
>> When this flag is set for CAPTURE queues by the driver on calling
>> vb2_queue_init(), it forces the buffers on the queue to be
>> allocated/mapped with DMA_BIDIRECTIONAL direction flag, instead of
>> DMA_FROM_DEVICE. This allows the device not only to write to the
>> buffers, but also read out from them. This may be useful e.g. for codec
>> hardware, which may be using CAPTURE buffers as reference to decode
>> other buffers.
>
> Just out of curiosity --- when do you return these buffers back to the user?
> Once they're no longer needed as reference frames?
Tbh, I don't now. This is used by a rockchip vpu driver not yet 
upstreamed in the chromeos v4.4 kernel tree. Pawel might answer this 
question I guess.

>
>>
>> This flag is ignored for OUTPUT queues, as we don't want to allow HW to
>> be able to write to OUTPUT buffers.
>>
>> Signed-off-by: Pawel Osciak <posciak@chromium.org>
>> Tested-by: Pawel Osciak <posciak@chromium.org>
>> Reviewed-by: Tomasz Figa <tfiga@chromium.org>
>> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
>> ---
>>  drivers/media/v4l2-core/videobuf2-v4l2.c | 8 ++++++--
>>  include/media/videobuf2-core.h           | 4 ++++
>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c
>> index fde1e2d..9255291 100644
>> --- a/drivers/media/v4l2-core/videobuf2-v4l2.c
>> +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
>> @@ -659,8 +659,12 @@ int vb2_queue_init(struct vb2_queue *q)
>>  	 * queues will always initialize waiting_for_buffers to false.
>>  	 */
>>  	q->quirk_poll_must_check_waiting_for_buffers = true;
>> -	q->dma_dir = V4L2_TYPE_IS_OUTPUT(q->type)
>> -		   ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>> +
>> +	if (V4L2_TYPE_IS_OUTPUT(q->type))
>> +		q->dma_dir = DMA_TO_DEVICE;
>> +	else
>> +		q->dma_dir = q->use_dma_bidirectional
>> +			   ? DMA_BIDIRECTIONAL : DMA_FROM_DEVICE;
>>
>>  	return vb2_core_queue_init(q);
>>  }
>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>> index 38410dd..e613c74 100644
>> --- a/include/media/videobuf2-core.h
>> +++ b/include/media/videobuf2-core.h
>> @@ -433,6 +433,9 @@ struct vb2_buf_ops {
>>   * @quirk_poll_must_check_waiting_for_buffers: Return POLLERR at poll when QBUF
>>   *              has not been called. This is a vb1 idiom that has been adopted
>>   *              also by vb2.
>> + * @use_dma_bidirectional:	use DMA_BIDIRECTIONAL for CAPTURE buffers; this
>> + *				allows HW to read from the CAPTURE buffers in
>> + *				addition to writing; ignored for OUTPUT queues
>>   * @lock:	pointer to a mutex that protects the vb2_queue struct. The
>>   *		driver can set this to a mutex to let the v4l2 core serialize
>>   *		the queuing ioctls. If the driver wants to handle locking
>> @@ -500,6 +503,7 @@ struct vb2_queue {
>>  	unsigned			fileio_write_immediately:1;
>>  	unsigned			allow_zero_bytesused:1;
>>  	unsigned		   quirk_poll_must_check_waiting_for_buffers:1;
>> +	unsigned			use_dma_bidirectional:1;
>
> This field is in the same struct as dma_dir which it directly affects.
>
> How about adding a macro instead to give you the queue DMA direction
> instead?
>
> E.g.
>
> #define vb2_dma_dir(q) \
> 	(V4L2_TYPE_IS_OUTPUT((q)->type) ? DMA_TO_DEVICE : \
> 	 (q)->use_dma_bidirectional ? DMA_BIDIRECTIONAL : DMA_FROM_DEVICE)
>
> I would call this capture_dma_bidirectional as it only affects capture. Or
> simply choose DMA_BIDIRECTIONAL whenever the flag is set.
Sure. Will do.

Regards,
  Thierry

[toc] | [prev] | [next] | [standalone]

#1503856 — Re: [PATCH 2/2] [media] vb2: Add support for use_dma_bidirectional queue flag

On Tue, Oct 18, 2016 at 06:08:53PM +0200, Thierry Escande wrote:
> >#define vb2_dma_dir(q) \

^

VB2_DMA_DIR(), as most of our other macros use capitals as well.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web