linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] hantro: set of small cleanups and fixes
@ 2020-03-11 17:42 Ezequiel Garcia
  2020-03-11 17:42 ` [PATCH 1/6] v4l2-mem2mem: return CAPTURE buffer first Ezequiel Garcia
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2020-03-11 17:42 UTC (permalink / raw)
  To: linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Ezequiel Garcia

Hi all,

Here's a few patches with some cleanups and fixes.

The main idea here is to address two issues, and while
at it, clean the driver a bit.

The first issue can be found in Patch 1, when the Request
API is used, the CAPTURE buffer should be returned _before_
the OUTPUT buffer, to avoid waking up userspace prematurely.

I noticed this issue while working on the rkvdec driver,
but this time I've decided to tackle it at the core,
in v4l2_m2m_buf_done_and_job_finish().

The second issue is a simple compliance issue, which is solved
by refactoring the driver, dealing with internal set format
properly.

I suspect it's really late for v5.7, but if we are still
in time, that would be lovely.

Thanks,
Ezequiel

Ezequiel Garcia (6):
  v4l2-mem2mem: return CAPTURE buffer first
  hantro: Set buffers' zeroth plane payload in .buf_prepare
  hantro: Use v4l2_m2m_buf_done_and_job_finish
  hantro: Remove unneeded hantro_dec_buf_finish
  hantro: Move H264 motion vector calculation to a helper
  hantro: Refactor for V4L2 API spec compliancy

 drivers/media/v4l2-core/v4l2-mem2mem.c     |  11 +-
 drivers/staging/media/hantro/hantro.h      |   4 -
 drivers/staging/media/hantro/hantro_drv.c  |  37 ++-----
 drivers/staging/media/hantro/hantro_hw.h   |  31 ++++++
 drivers/staging/media/hantro/hantro_v4l2.c | 111 +++++++++++----------
 5 files changed, 108 insertions(+), 86 deletions(-)

-- 
2.25.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/6] v4l2-mem2mem: return CAPTURE buffer first
  2020-03-11 17:42 [PATCH 0/6] hantro: set of small cleanups and fixes Ezequiel Garcia
@ 2020-03-11 17:42 ` Ezequiel Garcia
  2020-03-11 18:53   ` Nicolas Dufresne
  2020-03-11 17:42 ` [PATCH 2/6] hantro: Set buffers' zeroth plane payload in .buf_prepare Ezequiel Garcia
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Ezequiel Garcia @ 2020-03-11 17:42 UTC (permalink / raw)
  To: linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Ezequiel Garcia

When the request API is used, typically an OUTPUT (src) buffer
will be part of a request. A userspace process will be typically
blocked, waiting on the request file descriptor.

Returning the OUTPUT (src) buffer will wake-up such processes,
who will immediately attempt to dequeue the CAPTURE buffer,
only to find it's still unavailable.

Therefore, change v4l2_m2m_buf_done_and_job_finish returning
the CAPTURE (dst) buffer first, to avoid signalling the request
file descriptor prematurely, i.e. before the CAPTURE buffer is done.

When the request API is not used, this change should have
no impact.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 8986c31176e9..62ac9424c92a 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -504,12 +504,21 @@ void v4l2_m2m_buf_done_and_job_finish(struct v4l2_m2m_dev *m2m_dev,
 
 	if (WARN_ON(!src_buf || !dst_buf))
 		goto unlock;
-	v4l2_m2m_buf_done(src_buf, state);
 	dst_buf->is_held = src_buf->flags & V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF;
 	if (!dst_buf->is_held) {
 		v4l2_m2m_dst_buf_remove(m2m_ctx);
 		v4l2_m2m_buf_done(dst_buf, state);
 	}
+	/*
+	 * If the request API is being used, returning the OUTPUT
+	 * (src) buffer will wake-up any process waiting on the
+	 * request file descriptor.
+	 *
+	 * Therefore, return the CAPTURE (dst) buffer first,
+	 * to avoid signalling the request file descriptor
+	 * before the CAPTURE buffer is done.
+	 */
+	v4l2_m2m_buf_done(src_buf, state);
 	schedule_next = _v4l2_m2m_job_finish(m2m_dev, m2m_ctx);
 unlock:
 	spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/6] hantro: Set buffers' zeroth plane payload in .buf_prepare
  2020-03-11 17:42 [PATCH 0/6] hantro: set of small cleanups and fixes Ezequiel Garcia
  2020-03-11 17:42 ` [PATCH 1/6] v4l2-mem2mem: return CAPTURE buffer first Ezequiel Garcia
@ 2020-03-11 17:42 ` Ezequiel Garcia
  2020-03-11 17:42 ` [PATCH 3/6] hantro: Use v4l2_m2m_buf_done_and_job_finish Ezequiel Garcia
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2020-03-11 17:42 UTC (permalink / raw)
  To: linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Ezequiel Garcia

Buffers' zeroth plane payload size is calculated at format
negotiation time, and so it can be set in .buf_prepare.

Keep in mind that, to make this change easier, hantro_buf_prepare
is refactored, using the cedrus driver as reference. This results
in cleaner code as byproduct.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro_v4l2.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index f4ae2cee0f18..3142ab6697d5 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -608,7 +608,7 @@ hantro_queue_setup(struct vb2_queue *vq, unsigned int *num_buffers,
 }
 
 static int
-hantro_buf_plane_check(struct vb2_buffer *vb, const struct hantro_fmt *vpu_fmt,
+hantro_buf_plane_check(struct vb2_buffer *vb,
 		       struct v4l2_pix_format_mplane *pixfmt)
 {
 	unsigned int sz;
@@ -630,12 +630,18 @@ static int hantro_buf_prepare(struct vb2_buffer *vb)
 {
 	struct vb2_queue *vq = vb->vb2_queue;
 	struct hantro_ctx *ctx = vb2_get_drv_priv(vq);
+	struct v4l2_pix_format_mplane *pix_fmt;
+	int ret;
 
 	if (V4L2_TYPE_IS_OUTPUT(vq->type))
-		return hantro_buf_plane_check(vb, ctx->vpu_src_fmt,
-						  &ctx->src_fmt);
-
-	return hantro_buf_plane_check(vb, ctx->vpu_dst_fmt, &ctx->dst_fmt);
+		pix_fmt = &ctx->src_fmt;
+	else
+		pix_fmt = &ctx->dst_fmt;
+	ret = hantro_buf_plane_check(vb, pix_fmt);
+	if (ret)
+		return ret;
+	vb2_set_plane_payload(vb, 0, pix_fmt->plane_fmt[0].sizeimage);
+	return 0;
 }
 
 static void hantro_buf_queue(struct vb2_buffer *vb)
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/6] hantro: Use v4l2_m2m_buf_done_and_job_finish
  2020-03-11 17:42 [PATCH 0/6] hantro: set of small cleanups and fixes Ezequiel Garcia
  2020-03-11 17:42 ` [PATCH 1/6] v4l2-mem2mem: return CAPTURE buffer first Ezequiel Garcia
  2020-03-11 17:42 ` [PATCH 2/6] hantro: Set buffers' zeroth plane payload in .buf_prepare Ezequiel Garcia
@ 2020-03-11 17:42 ` Ezequiel Garcia
  2020-03-11 17:42 ` [PATCH 4/6] hantro: Remove unneeded hantro_dec_buf_finish Ezequiel Garcia
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2020-03-11 17:42 UTC (permalink / raw)
  To: linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Ezequiel Garcia

Let the core sort out the nuances of returning buffers
to userspace, by using the v4l2_m2m_buf_done_and_job_finish
helper.

This change also removes usage of buffer sequence fields,
which shouldn't have any meaning for stateless decoders.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro_drv.c | 27 ++++++++---------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 0b1200fc0e1a..ec889d755cd6 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -94,32 +94,23 @@ static void hantro_job_finish(struct hantro_dev *vpu,
 			      unsigned int bytesused,
 			      enum vb2_buffer_state result)
 {
-	struct vb2_v4l2_buffer *src, *dst;
 	int ret;
 
 	pm_runtime_mark_last_busy(vpu->dev);
 	pm_runtime_put_autosuspend(vpu->dev);
 	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
 
-	src = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
-	dst = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
-
-	if (WARN_ON(!src))
-		return;
-	if (WARN_ON(!dst))
-		return;
-
-	src->sequence = ctx->sequence_out++;
-	dst->sequence = ctx->sequence_cap++;
-
-	ret = ctx->buf_finish(ctx, &dst->vb2_buf, bytesused);
-	if (ret)
-		result = VB2_BUF_STATE_ERROR;
+	if (ctx->buf_finish) {
+		struct vb2_v4l2_buffer *dst;
 
-	v4l2_m2m_buf_done(src, result);
-	v4l2_m2m_buf_done(dst, result);
+		dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+		ret = ctx->buf_finish(ctx, &dst->vb2_buf, bytesused);
+		if (ret)
+			result = VB2_BUF_STATE_ERROR;
+	}
 
-	v4l2_m2m_job_finish(vpu->m2m_dev, ctx->fh.m2m_ctx);
+	v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx,
+					 result);
 }
 
 void hantro_irq_done(struct hantro_dev *vpu, unsigned int bytesused,
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/6] hantro: Remove unneeded hantro_dec_buf_finish
  2020-03-11 17:42 [PATCH 0/6] hantro: set of small cleanups and fixes Ezequiel Garcia
                   ` (2 preceding siblings ...)
  2020-03-11 17:42 ` [PATCH 3/6] hantro: Use v4l2_m2m_buf_done_and_job_finish Ezequiel Garcia
@ 2020-03-11 17:42 ` Ezequiel Garcia
  2020-03-11 17:42 ` [PATCH 5/6] hantro: Move H264 motion vector calculation to a helper Ezequiel Garcia
  2020-03-11 17:43 ` [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy Ezequiel Garcia
  5 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2020-03-11 17:42 UTC (permalink / raw)
  To: linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Ezequiel Garcia

Since now .buf_prepare takes care of setting the
buffer payload size, we can get rid of this,
at least for decoders.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro_drv.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index ec889d755cd6..bd204da6c669 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -80,15 +80,6 @@ hantro_enc_buf_finish(struct hantro_ctx *ctx, struct vb2_buffer *buf,
 	return 0;
 }
 
-static int
-hantro_dec_buf_finish(struct hantro_ctx *ctx, struct vb2_buffer *buf,
-		      unsigned int bytesused)
-{
-	/* For decoders set bytesused as per the output picture. */
-	buf->planes[0].bytesused = ctx->dst_fmt.plane_fmt[0].sizeimage;
-	return 0;
-}
-
 static void hantro_job_finish(struct hantro_dev *vpu,
 			      struct hantro_ctx *ctx,
 			      unsigned int bytesused,
@@ -422,7 +413,6 @@ static int hantro_open(struct file *filp)
 		ctx->buf_finish = hantro_enc_buf_finish;
 	} else if (func->id == MEDIA_ENT_F_PROC_VIDEO_DECODER) {
 		allowed_codecs = vpu->variant->codec & HANTRO_DECODERS;
-		ctx->buf_finish = hantro_dec_buf_finish;
 	} else {
 		ret = -ENODEV;
 		goto err_ctx_free;
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/6] hantro: Move H264 motion vector calculation to a helper
  2020-03-11 17:42 [PATCH 0/6] hantro: set of small cleanups and fixes Ezequiel Garcia
                   ` (3 preceding siblings ...)
  2020-03-11 17:42 ` [PATCH 4/6] hantro: Remove unneeded hantro_dec_buf_finish Ezequiel Garcia
@ 2020-03-11 17:42 ` Ezequiel Garcia
  2020-03-11 17:43 ` [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy Ezequiel Garcia
  5 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2020-03-11 17:42 UTC (permalink / raw)
  To: linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Ezequiel Garcia

Move the extra bytes calculation that are needed for H264
motion vector to a helper. This is just a cosmetic cleanup.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro.h      |  4 ---
 drivers/staging/media/hantro/hantro_hw.h   | 31 ++++++++++++++++++++++
 drivers/staging/media/hantro/hantro_v4l2.c | 25 ++---------------
 3 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 327ddef45345..2089f88a44a2 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -26,10 +26,6 @@
 
 #include "hantro_hw.h"
 
-#define MB_DIM			16
-#define MB_WIDTH(w)		DIV_ROUND_UP(w, MB_DIM)
-#define MB_HEIGHT(h)		DIV_ROUND_UP(h, MB_DIM)
-
 struct hantro_ctx;
 struct hantro_codec_ops;
 
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 2398d4c1f207..435f30ae89fd 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -18,6 +18,10 @@
 
 #define DEC_8190_ALIGN_MASK	0x07U
 
+#define MB_DIM			16
+#define MB_WIDTH(w)		DIV_ROUND_UP(w, MB_DIM)
+#define MB_HEIGHT(h)		DIV_ROUND_UP(h, MB_DIM)
+
 struct hantro_dev;
 struct hantro_ctx;
 struct hantro_buf;
@@ -175,6 +179,33 @@ void hantro_g1_h264_dec_run(struct hantro_ctx *ctx);
 int hantro_h264_dec_init(struct hantro_ctx *ctx);
 void hantro_h264_dec_exit(struct hantro_ctx *ctx);
 
+static inline size_t
+hantro_h264_mv_size(unsigned int width, unsigned int height)
+{
+	/*
+	 * A decoded 8-bit 4:2:0 NV12 frame may need memory for up to
+	 * 448 bytes per macroblock with additional 32 bytes on
+	 * multi-core variants.
+	 *
+	 * The H264 decoder needs extra space on the output buffers
+	 * to store motion vectors. This is needed for reference
+	 * frames and only if the format is non-post-processed NV12.
+	 *
+	 * Memory layout is as follow:
+	 *
+	 * +---------------------------+
+	 * | Y-plane   256 bytes x MBs |
+	 * +---------------------------+
+	 * | UV-plane  128 bytes x MBs |
+	 * +---------------------------+
+	 * | MV buffer  64 bytes x MBs |
+	 * +---------------------------+
+	 * | MC sync          32 bytes |
+	 * +---------------------------+
+	 */
+	return 64 * MB_WIDTH(width) * MB_WIDTH(height) + 32;
+}
+
 void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx);
 void rk3399_vpu_mpeg2_dec_run(struct hantro_ctx *ctx);
 void hantro_mpeg2_dec_copy_qtable(u8 *qtable,
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 3142ab6697d5..458b502ff01b 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -273,32 +273,11 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,
 		/* Fill remaining fields */
 		v4l2_fill_pixfmt_mp(pix_mp, fmt->fourcc, pix_mp->width,
 				    pix_mp->height);
-		/*
-		 * A decoded 8-bit 4:2:0 NV12 frame may need memory for up to
-		 * 448 bytes per macroblock with additional 32 bytes on
-		 * multi-core variants.
-		 *
-		 * The H264 decoder needs extra space on the output buffers
-		 * to store motion vectors. This is needed for reference
-		 * frames and only if the format is non-post-processed NV12.
-		 *
-		 * Memory layout is as follow:
-		 *
-		 * +---------------------------+
-		 * | Y-plane   256 bytes x MBs |
-		 * +---------------------------+
-		 * | UV-plane  128 bytes x MBs |
-		 * +---------------------------+
-		 * | MV buffer  64 bytes x MBs |
-		 * +---------------------------+
-		 * | MC sync          32 bytes |
-		 * +---------------------------+
-		 */
 		if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE &&
 		    !hantro_needs_postproc(ctx, fmt))
 			pix_mp->plane_fmt[0].sizeimage +=
-				64 * MB_WIDTH(pix_mp->width) *
-				     MB_WIDTH(pix_mp->height) + 32;
+				hantro_h264_mv_size(pix_mp->width,
+						    pix_mp->height);
 	} else if (!pix_mp->plane_fmt[0].sizeimage) {
 		/*
 		 * For coded formats the application can specify
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy
  2020-03-11 17:42 [PATCH 0/6] hantro: set of small cleanups and fixes Ezequiel Garcia
                   ` (4 preceding siblings ...)
  2020-03-11 17:42 ` [PATCH 5/6] hantro: Move H264 motion vector calculation to a helper Ezequiel Garcia
@ 2020-03-11 17:43 ` Ezequiel Garcia
  2020-03-11 19:01   ` Ezequiel Garcia
                     ` (2 more replies)
  5 siblings, 3 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2020-03-11 17:43 UTC (permalink / raw)
  To: linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Ezequiel Garcia, Nicolas Dufresne

Refactor how S_FMT and TRY_FMT are handled, and also make sure
internal initial format and format reset are done properly.

The latter is achieved by making sure the same hantro_{set,try}_fmt
helpers are called on all paths that set the format (which is
part of the driver state).

This commit removes the following v4l2-compliance warnings:

test VIDIOC_G_FMT: OK
	fail: v4l2-test-formats.cpp(711): Video Capture Multiplanar: TRY_FMT(G_FMT) != G_FMT
test VIDIOC_TRY_FMT: FAIL
	fail: v4l2-test-formats.cpp(1116): Video Capture Multiplanar: S_FMT(G_FMT) != G_FMT
test VIDIOC_S_FMT: FAIL

Reported-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro_v4l2.c | 70 ++++++++++++++--------
 1 file changed, 45 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 458b502ff01b..f28a94e2fa93 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -30,6 +30,11 @@
 #include "hantro_hw.h"
 #include "hantro_v4l2.h"
 
+static int hantro_set_fmt_out(struct hantro_ctx *ctx,
+			      struct v4l2_pix_format_mplane *pix_mp);
+static int hantro_set_fmt_cap(struct hantro_ctx *ctx,
+			      struct v4l2_pix_format_mplane *pix_mp);
+
 static const struct hantro_fmt *
 hantro_get_formats(const struct hantro_ctx *ctx, unsigned int *num_fmts)
 {
@@ -227,12 +232,12 @@ static int vidioc_g_fmt_cap_mplane(struct file *file, void *priv,
 	return 0;
 }
 
-static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,
-			  bool capture)
+static int hantro_try_fmt(const struct hantro_ctx *ctx,
+			  struct v4l2_pix_format_mplane *pix_mp,
+			  enum v4l2_buf_type type)
 {
-	struct hantro_ctx *ctx = fh_to_ctx(priv);
-	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
 	const struct hantro_fmt *fmt, *vpu_fmt;
+	bool capture = !V4L2_TYPE_IS_OUTPUT(type);
 	bool coded;
 
 	coded = capture == hantro_is_encoder_ctx(ctx);
@@ -246,7 +251,7 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,
 	fmt = hantro_find_format(ctx, pix_mp->pixelformat);
 	if (!fmt) {
 		fmt = hantro_get_default_fmt(ctx, coded);
-		f->fmt.pix_mp.pixelformat = fmt->fourcc;
+		pix_mp->pixelformat = fmt->fourcc;
 	}
 
 	if (coded) {
@@ -294,13 +299,13 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,
 static int vidioc_try_fmt_cap_mplane(struct file *file, void *priv,
 				     struct v4l2_format *f)
 {
-	return vidioc_try_fmt(file, priv, f, true);
+	return hantro_try_fmt(fh_to_ctx(priv), &f->fmt.pix_mp, f->type);
 }
 
 static int vidioc_try_fmt_out_mplane(struct file *file, void *priv,
 				     struct v4l2_format *f)
 {
-	return vidioc_try_fmt(file, priv, f, false);
+	return hantro_try_fmt(fh_to_ctx(priv), &f->fmt.pix_mp, f->type);
 }
 
 static void
@@ -334,11 +339,12 @@ hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
 	}
 
 	hantro_reset_fmt(fmt, vpu_fmt);
-	fmt->num_planes = 1;
 	fmt->width = vpu_fmt->frmsize.min_width;
 	fmt->height = vpu_fmt->frmsize.min_height;
-	fmt->plane_fmt[0].sizeimage = vpu_fmt->header_size +
-				fmt->width * fmt->height * vpu_fmt->max_depth;
+	if (hantro_is_encoder_ctx(ctx))
+		hantro_set_fmt_cap(ctx, fmt);
+	else
+		hantro_set_fmt_out(ctx, fmt);
 }
 
 static void
@@ -360,9 +366,12 @@ hantro_reset_raw_fmt(struct hantro_ctx *ctx)
 	}
 
 	hantro_reset_fmt(raw_fmt, raw_vpu_fmt);
-	v4l2_fill_pixfmt_mp(raw_fmt, raw_vpu_fmt->fourcc,
-			    encoded_fmt->width,
-			    encoded_fmt->height);
+	raw_fmt->width = encoded_fmt->width;
+	raw_fmt->width = encoded_fmt->width;
+	if (hantro_is_encoder_ctx(ctx))
+		hantro_set_fmt_out(ctx, raw_fmt);
+	else
+		hantro_set_fmt_cap(ctx, raw_fmt);
 }
 
 void hantro_reset_fmts(struct hantro_ctx *ctx)
@@ -388,15 +397,15 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
 	}
 }
 
-static int
-vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
+static int hantro_set_fmt_out(struct hantro_ctx *ctx,
+			      struct v4l2_pix_format_mplane *pix_mp)
 {
-	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
-	struct hantro_ctx *ctx = fh_to_ctx(priv);
-	struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
+	struct vb2_queue *vq;
 	int ret;
 
-	ret = vidioc_try_fmt_out_mplane(file, priv, f);
+	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+			     V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
+	ret = hantro_try_fmt(ctx, pix_mp, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
 	if (ret)
 		return ret;
 
@@ -458,16 +467,15 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
 	return 0;
 }
 
-static int vidioc_s_fmt_cap_mplane(struct file *file, void *priv,
-				   struct v4l2_format *f)
+static int hantro_set_fmt_cap(struct hantro_ctx *ctx,
+			      struct v4l2_pix_format_mplane *pix_mp)
 {
-	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
-	struct hantro_ctx *ctx = fh_to_ctx(priv);
 	struct vb2_queue *vq;
 	int ret;
 
 	/* Change not allowed if queue is busy. */
-	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
+	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+			     V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
 	if (vb2_is_busy(vq))
 		return -EBUSY;
 
@@ -488,7 +496,7 @@ static int vidioc_s_fmt_cap_mplane(struct file *file, void *priv,
 			return -EBUSY;
 	}
 
-	ret = vidioc_try_fmt_cap_mplane(file, priv, f);
+	ret = hantro_try_fmt(ctx, pix_mp, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
 	if (ret)
 		return ret;
 
@@ -522,6 +530,18 @@ static int vidioc_s_fmt_cap_mplane(struct file *file, void *priv,
 	return 0;
 }
 
+static int
+vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
+{
+	return hantro_set_fmt_out(fh_to_ctx(priv), &f->fmt.pix_mp);
+}
+
+static int
+vidioc_s_fmt_cap_mplane(struct file *file, void *priv, struct v4l2_format *f)
+{
+	return hantro_set_fmt_cap(fh_to_ctx(priv), &f->fmt.pix_mp);
+}
+
 const struct v4l2_ioctl_ops hantro_ioctl_ops = {
 	.vidioc_querycap = vidioc_querycap,
 	.vidioc_enum_framesizes = vidioc_enum_framesizes,
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] v4l2-mem2mem: return CAPTURE buffer first
  2020-03-11 17:42 ` [PATCH 1/6] v4l2-mem2mem: return CAPTURE buffer first Ezequiel Garcia
@ 2020-03-11 18:53   ` Nicolas Dufresne
  0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Dufresne @ 2020-03-11 18:53 UTC (permalink / raw)
  To: Ezequiel Garcia, linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, kernel, Jonas Karlman, Heiko Stuebner, Hans Verkuil,
	Alexandre Courbot, Jeffrey Kardatzke

Le mercredi 11 mars 2020 à 14:42 -0300, Ezequiel Garcia a écrit :
> When the request API is used, typically an OUTPUT (src) buffer
> will be part of a request. A userspace process will be typically
> blocked, waiting on the request file descriptor.
> 
> Returning the OUTPUT (src) buffer will wake-up such processes,
> who will immediately attempt to dequeue the CAPTURE buffer,
> only to find it's still unavailable.
> 
> Therefore, change v4l2_m2m_buf_done_and_job_finish returning
> the CAPTURE (dst) buffer first, to avoid signalling the request
> file descriptor prematurely, i.e. before the CAPTURE buffer is done.
> 
> When the request API is not used, this change should have
> no impact.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>

This was tested with upcoming GStreamer element with O_NONBLOCK flag passed when
opening the video node. Before this change, EAGAIN would from time to time be
returned when DQBUF(CAPTURE) was called.

  gst-launch-1.0 filesrc location=somefile.mp4 ! parsebin ! v4l2slh264dec ! fakevideosink
  https://gitlab.freedesktop.org/ndufresne/gst-plugins-bad/-/blob/v4l2codecs-hantro-v3/sys/v4l2codecs/gstv4l2decoder.c#L139

Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-
> core/v4l2-mem2mem.c
> index 8986c31176e9..62ac9424c92a 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -504,12 +504,21 @@ void v4l2_m2m_buf_done_and_job_finish(struct
> v4l2_m2m_dev *m2m_dev,
>  
>  	if (WARN_ON(!src_buf || !dst_buf))
>  		goto unlock;
> -	v4l2_m2m_buf_done(src_buf, state);
>  	dst_buf->is_held = src_buf->flags & V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF;
>  	if (!dst_buf->is_held) {
>  		v4l2_m2m_dst_buf_remove(m2m_ctx);
>  		v4l2_m2m_buf_done(dst_buf, state);
>  	}
> +	/*
> +	 * If the request API is being used, returning the OUTPUT
> +	 * (src) buffer will wake-up any process waiting on the
> +	 * request file descriptor.
> +	 *
> +	 * Therefore, return the CAPTURE (dst) buffer first,
> +	 * to avoid signalling the request file descriptor
> +	 * before the CAPTURE buffer is done.
> +	 */
> +	v4l2_m2m_buf_done(src_buf, state);
>  	schedule_next = _v4l2_m2m_job_finish(m2m_dev, m2m_ctx);
>  unlock:
>  	spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy
  2020-03-11 17:43 ` [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy Ezequiel Garcia
@ 2020-03-11 19:01   ` Ezequiel Garcia
  2020-03-11 19:05     ` Nicolas Dufresne
  2020-03-12  0:18   ` kbuild test robot
  2020-03-12 15:18   ` kbuild test robot
  2 siblings, 1 reply; 12+ messages in thread
From: Ezequiel Garcia @ 2020-03-11 19:01 UTC (permalink / raw)
  To: linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Nicolas Dufresne

On Wed, 2020-03-11 at 14:43 -0300, Ezequiel Garcia wrote:
> Refactor how S_FMT and TRY_FMT are handled, and also make sure
> internal initial format and format reset are done properly.
> 
> The latter is achieved by making sure the same hantro_{set,try}_fmt
> helpers are called on all paths that set the format (which is
> part of the driver state).
> 
> This commit removes the following v4l2-compliance warnings:
> 
> test VIDIOC_G_FMT: OK
> 	fail: v4l2-test-formats.cpp(711): Video Capture Multiplanar: TRY_FMT(G_FMT) != G_FMT
> test VIDIOC_TRY_FMT: FAIL
> 	fail: v4l2-test-formats.cpp(1116): Video Capture Multiplanar: S_FMT(G_FMT) != G_FMT
> test VIDIOC_S_FMT: FAIL
> 
> Reported-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
[..]
> @@ -227,12 +232,12 @@ static int vidioc_g_fmt_cap_mplane(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,
> -			  bool capture)
> +static int hantro_try_fmt(const struct hantro_ctx *ctx,

Oops, it seems there's a warning due to ctx being const-qualified.

That should be fixed of course.

Regards,
Ezequiel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy
  2020-03-11 19:01   ` Ezequiel Garcia
@ 2020-03-11 19:05     ` Nicolas Dufresne
  0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Dufresne @ 2020-03-11 19:05 UTC (permalink / raw)
  To: Ezequiel Garcia, linux-media, linux-rockchip, linux-kernel
  Cc: Tomasz Figa, kernel, Jonas Karlman, Heiko Stuebner, Hans Verkuil,
	Alexandre Courbot, Jeffrey Kardatzke

Le mercredi 11 mars 2020 à 16:01 -0300, Ezequiel Garcia a écrit :
> On Wed, 2020-03-11 at 14:43 -0300, Ezequiel Garcia wrote:
> > Refactor how S_FMT and TRY_FMT are handled, and also make sure
> > internal initial format and format reset are done properly.
> > 
> > The latter is achieved by making sure the same hantro_{set,try}_fmt
> > helpers are called on all paths that set the format (which is
> > part of the driver state).
> > 
> > This commit removes the following v4l2-compliance warnings:
> > 
> > test VIDIOC_G_FMT: OK
> > 	fail: v4l2-test-formats.cpp(711): Video Capture Multiplanar: TRY_FMT(G_FMT) != G_FMT
> > test VIDIOC_TRY_FMT: FAIL
> > 	fail: v4l2-test-formats.cpp(1116): Video Capture Multiplanar: S_FMT(G_FMT) != G_FMT
> > test VIDIOC_S_FMT: FAIL
> > 
> > Reported-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> [..]
> > @@ -227,12 +232,12 @@ static int vidioc_g_fmt_cap_mplane(struct file *file, void *priv,
> >  	return 0;
> >  }
> >  
> > -static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,
> > -			  bool capture)
> > +static int hantro_try_fmt(const struct hantro_ctx *ctx,
> 
> Oops, it seems there's a warning due to ctx being const-qualified.

Indeed:

drivers/staging/media/hantro/hantro_v4l2.c: In function ‘hantro_try_fmt’:
drivers/staging/media/hantro/hantro_v4l2.c:282:30: warning: passing argument 1 of ‘hantro_needs_postproc’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  282 |       !hantro_needs_postproc(ctx, fmt))
      |                              ^~~
In file included from drivers/staging/media/hantro/hantro_v4l2.c:29:
drivers/staging/media/hantro/hantro.h:420:42: note: expected ‘struct hantro_ctx *’ but argument is of type ‘const struct hantro_ctx *’
  420 | hantro_needs_postproc(struct hantro_ctx *ctx, const struct hantro_fmt *fmt)
      |                       ~~~~~~~~~~~~~~~~~~~^~~

> 
> That should be fixed of course.
> 
> Regards,
> Ezequiel
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy
  2020-03-11 17:43 ` [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy Ezequiel Garcia
  2020-03-11 19:01   ` Ezequiel Garcia
@ 2020-03-12  0:18   ` kbuild test robot
  2020-03-12 15:18   ` kbuild test robot
  2 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2020-03-12  0:18 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: kbuild-all, linux-media, linux-rockchip, linux-kernel,
	Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Ezequiel Garcia, Nicolas Dufresne

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

Hi Ezequiel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.6-rc5 next-20200311]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Ezequiel-Garcia/hantro-set-of-small-cleanups-and-fixes/20200312-061234
base:   git://linuxtv.org/media_tree.git master
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.2.0 make.cross ARCH=sh 

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

All warnings (new ones prefixed by >>):

   drivers/staging/media/hantro/hantro_v4l2.c: In function 'hantro_try_fmt':
>> drivers/staging/media/hantro/hantro_v4l2.c:282:30: warning: passing argument 1 of 'hantro_needs_postproc' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     282 |       !hantro_needs_postproc(ctx, fmt))
         |                              ^~~
   In file included from drivers/staging/media/hantro/hantro_v4l2.c:29:
   drivers/staging/media/hantro/hantro.h:420:42: note: expected 'struct hantro_ctx *' but argument is of type 'const struct hantro_ctx *'
     420 | hantro_needs_postproc(struct hantro_ctx *ctx, const struct hantro_fmt *fmt)
         |                       ~~~~~~~~~~~~~~~~~~~^~~

vim +282 drivers/staging/media/hantro/hantro_v4l2.c

775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  234  
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  235  static int hantro_try_fmt(const struct hantro_ctx *ctx,
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  236  			  struct v4l2_pix_format_mplane *pix_mp,
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  237  			  enum v4l2_buf_type type)
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  238  {
5980d40276b36b drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2019-12-05  239  	const struct hantro_fmt *fmt, *vpu_fmt;
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  240  	bool capture = !V4L2_TYPE_IS_OUTPUT(type);
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  241  	bool coded;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  242  
a29add8c9bb29d drivers/staging/media/hantro/hantro_v4l2.c             Philipp Zabel   2019-06-12  243  	coded = capture == hantro_is_encoder_ctx(ctx);
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  244  
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  245  	vpu_debug(4, "trying format %c%c%c%c\n",
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  246  		  (pix_mp->pixelformat & 0x7f),
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  247  		  (pix_mp->pixelformat >> 8) & 0x7f,
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  248  		  (pix_mp->pixelformat >> 16) & 0x7f,
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  249  		  (pix_mp->pixelformat >> 24) & 0x7f);
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  250  
5980d40276b36b drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2019-12-05  251  	fmt = hantro_find_format(ctx, pix_mp->pixelformat);
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  252  	if (!fmt) {
5980d40276b36b drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2019-12-05  253  		fmt = hantro_get_default_fmt(ctx, coded);
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  254  		pix_mp->pixelformat = fmt->fourcc;
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  255  	}
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  256  
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  257  	if (coded) {
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  258  		pix_mp->num_planes = 1;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  259  		vpu_fmt = fmt;
a29add8c9bb29d drivers/staging/media/hantro/hantro_v4l2.c             Philipp Zabel   2019-06-12  260  	} else if (hantro_is_encoder_ctx(ctx)) {
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  261  		vpu_fmt = ctx->vpu_dst_fmt;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  262  	} else {
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  263  		vpu_fmt = ctx->vpu_src_fmt;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  264  		/*
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  265  		 * Width/height on the CAPTURE end of a decoder are ignored and
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  266  		 * replaced by the OUTPUT ones.
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  267  		 */
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  268  		pix_mp->width = ctx->src_fmt.width;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  269  		pix_mp->height = ctx->src_fmt.height;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  270  	}
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  271  
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  272  	pix_mp->field = V4L2_FIELD_NONE;
0a4f091c12b3ea drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Boris Brezillon 2019-05-28  273  
0a4f091c12b3ea drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Boris Brezillon 2019-05-28  274  	v4l2_apply_frmsize_constraints(&pix_mp->width, &pix_mp->height,
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  275  				       &vpu_fmt->frmsize);
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  276  
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  277  	if (!coded) {
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  278  		/* Fill remaining fields */
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  279  		v4l2_fill_pixfmt_mp(pix_mp, fmt->fourcc, pix_mp->width,
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  280  				    pix_mp->height);
8c2d66b036c778 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2019-12-05  281  		if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE &&
042584e9055b61 drivers/staging/media/hantro/hantro_v4l2.c             Philipp Zabel   2020-01-27 @282  		    !hantro_needs_postproc(ctx, fmt))
a9471e25629b02 drivers/staging/media/hantro/hantro_v4l2.c             Hertz Wong      2019-08-16  283  			pix_mp->plane_fmt[0].sizeimage +=
6574b7394c10e2 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  284  				hantro_h264_mv_size(pix_mp->width,
6574b7394c10e2 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  285  						    pix_mp->height);
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  286  	} else if (!pix_mp->plane_fmt[0].sizeimage) {
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  287  		/*
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  288  		 * For coded formats the application can specify
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  289  		 * sizeimage. If the application passes a zero sizeimage,
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  290  		 * let's default to the maximum frame size.
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  291  		 */
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  292  		pix_mp->plane_fmt[0].sizeimage = fmt->header_size +
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  293  			pix_mp->width * pix_mp->height * fmt->max_depth;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  294  	}
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  295  
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  296  	return 0;
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  297  }
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  298  

:::::: The code at line 282 was first introduced by commit
:::::: 042584e9055b615ac917239884fb0d65690f56ec media: hantro: fix extra MV/MC sync space calculation

:::::: TO: Philipp Zabel <p.zabel@pengutronix.de>
:::::: CC: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy
  2020-03-11 17:43 ` [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy Ezequiel Garcia
  2020-03-11 19:01   ` Ezequiel Garcia
  2020-03-12  0:18   ` kbuild test robot
@ 2020-03-12 15:18   ` kbuild test robot
  2 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2020-03-12 15:18 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: kbuild-all, linux-media, linux-rockchip, linux-kernel,
	Tomasz Figa, Nicolas Dufresne, kernel, Jonas Karlman,
	Heiko Stuebner, Hans Verkuil, Alexandre Courbot,
	Jeffrey Kardatzke, Ezequiel Garcia, Nicolas Dufresne

Hi Ezequiel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.6-rc5 next-20200312]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Ezequiel-Garcia/hantro-set-of-small-cleanups-and-fixes/20200312-061234
base:   git://linuxtv.org/media_tree.git master
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-174-g094d5a94-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

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


sparse warnings: (new ones prefixed by >>)

>> drivers/staging/media/hantro/hantro_v4l2.c:282:44: sparse: sparse: incorrect type in argument 1 (different modifiers) @@    expected struct hantro_ctx *ctx @@    got structstruct hantro_ctx *ctx @@
>> drivers/staging/media/hantro/hantro_v4l2.c:282:44: sparse:    expected struct hantro_ctx *ctx
>> drivers/staging/media/hantro/hantro_v4l2.c:282:44: sparse:    got struct hantro_ctx const *ctx

vim +282 drivers/staging/media/hantro/hantro_v4l2.c

775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  234  
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  235  static int hantro_try_fmt(const struct hantro_ctx *ctx,
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  236  			  struct v4l2_pix_format_mplane *pix_mp,
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  237  			  enum v4l2_buf_type type)
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  238  {
5980d40276b36b drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2019-12-05  239  	const struct hantro_fmt *fmt, *vpu_fmt;
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  240  	bool capture = !V4L2_TYPE_IS_OUTPUT(type);
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  241  	bool coded;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  242  
a29add8c9bb29d drivers/staging/media/hantro/hantro_v4l2.c             Philipp Zabel   2019-06-12  243  	coded = capture == hantro_is_encoder_ctx(ctx);
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  244  
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  245  	vpu_debug(4, "trying format %c%c%c%c\n",
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  246  		  (pix_mp->pixelformat & 0x7f),
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  247  		  (pix_mp->pixelformat >> 8) & 0x7f,
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  248  		  (pix_mp->pixelformat >> 16) & 0x7f,
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  249  		  (pix_mp->pixelformat >> 24) & 0x7f);
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  250  
5980d40276b36b drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2019-12-05  251  	fmt = hantro_find_format(ctx, pix_mp->pixelformat);
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  252  	if (!fmt) {
5980d40276b36b drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2019-12-05  253  		fmt = hantro_get_default_fmt(ctx, coded);
0f07bff2f08018 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  254  		pix_mp->pixelformat = fmt->fourcc;
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  255  	}
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  256  
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  257  	if (coded) {
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  258  		pix_mp->num_planes = 1;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  259  		vpu_fmt = fmt;
a29add8c9bb29d drivers/staging/media/hantro/hantro_v4l2.c             Philipp Zabel   2019-06-12  260  	} else if (hantro_is_encoder_ctx(ctx)) {
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  261  		vpu_fmt = ctx->vpu_dst_fmt;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  262  	} else {
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  263  		vpu_fmt = ctx->vpu_src_fmt;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  264  		/*
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  265  		 * Width/height on the CAPTURE end of a decoder are ignored and
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  266  		 * replaced by the OUTPUT ones.
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  267  		 */
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  268  		pix_mp->width = ctx->src_fmt.width;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  269  		pix_mp->height = ctx->src_fmt.height;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  270  	}
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  271  
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  272  	pix_mp->field = V4L2_FIELD_NONE;
0a4f091c12b3ea drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Boris Brezillon 2019-05-28  273  
0a4f091c12b3ea drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Boris Brezillon 2019-05-28  274  	v4l2_apply_frmsize_constraints(&pix_mp->width, &pix_mp->height,
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  275  				       &vpu_fmt->frmsize);
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  276  
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  277  	if (!coded) {
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  278  		/* Fill remaining fields */
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  279  		v4l2_fill_pixfmt_mp(pix_mp, fmt->fourcc, pix_mp->width,
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  280  				    pix_mp->height);
8c2d66b036c778 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2019-12-05  281  		if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE &&
042584e9055b61 drivers/staging/media/hantro/hantro_v4l2.c             Philipp Zabel   2020-01-27 @282  		    !hantro_needs_postproc(ctx, fmt))
a9471e25629b02 drivers/staging/media/hantro/hantro_v4l2.c             Hertz Wong      2019-08-16  283  			pix_mp->plane_fmt[0].sizeimage +=
6574b7394c10e2 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  284  				hantro_h264_mv_size(pix_mp->width,
6574b7394c10e2 drivers/staging/media/hantro/hantro_v4l2.c             Ezequiel Garcia 2020-03-11  285  						    pix_mp->height);
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  286  	} else if (!pix_mp->plane_fmt[0].sizeimage) {
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  287  		/*
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  288  		 * For coded formats the application can specify
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  289  		 * sizeimage. If the application passes a zero sizeimage,
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  290  		 * let's default to the maximum frame size.
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  291  		 */
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  292  		pix_mp->plane_fmt[0].sizeimage = fmt->header_size +
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  293  			pix_mp->width * pix_mp->height * fmt->max_depth;
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  294  	}
953aaa1492c538 drivers/staging/media/rockchip/vpu/rockchip_vpu_v4l2.c Boris Brezillon 2019-05-28  295  
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  296  	return 0;
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  297  }
775fec69008d30 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c  Ezequiel Garcia 2018-12-05  298  

:::::: The code at line 282 was first introduced by commit
:::::: 042584e9055b615ac917239884fb0d65690f56ec media: hantro: fix extra MV/MC sync space calculation

:::::: TO: Philipp Zabel <p.zabel@pengutronix.de>
:::::: CC: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-03-12 15:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 17:42 [PATCH 0/6] hantro: set of small cleanups and fixes Ezequiel Garcia
2020-03-11 17:42 ` [PATCH 1/6] v4l2-mem2mem: return CAPTURE buffer first Ezequiel Garcia
2020-03-11 18:53   ` Nicolas Dufresne
2020-03-11 17:42 ` [PATCH 2/6] hantro: Set buffers' zeroth plane payload in .buf_prepare Ezequiel Garcia
2020-03-11 17:42 ` [PATCH 3/6] hantro: Use v4l2_m2m_buf_done_and_job_finish Ezequiel Garcia
2020-03-11 17:42 ` [PATCH 4/6] hantro: Remove unneeded hantro_dec_buf_finish Ezequiel Garcia
2020-03-11 17:42 ` [PATCH 5/6] hantro: Move H264 motion vector calculation to a helper Ezequiel Garcia
2020-03-11 17:43 ` [PATCH 6/6] hantro: Refactor for V4L2 API spec compliancy Ezequiel Garcia
2020-03-11 19:01   ` Ezequiel Garcia
2020-03-11 19:05     ` Nicolas Dufresne
2020-03-12  0:18   ` kbuild test robot
2020-03-12 15:18   ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).