[PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support

[PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support

[Ezequiel Garcia]

This series adds VP8 decoding support on RK3399 SoC.

I'm including a set of commits from Boris' recent H264 series [1].
These commits add some helpers that are also useful for RK3399 VP8,
and at the same time cleanup the driver nicely.

Finally, there's a fix by Francois Buergisser from Chromium team.

VP8 and MPEG-2 tested on RK3399 RockPi and RK3288 Rock2 boards.

[1] https://patchwork.kernel.org/cover/11003971/

Changes from v1:

 * Remove unused variables.
 * Use buffer helpers in places missing in v1.

Boris Brezillon (4):
  media: hantro: Simplify the controls creation logic
  media: hantro: Constify the control array
  media: hantro: Add hantro_get_{src,dst}_buf() helpers
  media: hantro: Add helpers to prepare/finish a run

Ezequiel Garcia (1):
  media: hantro: Move VP8 common code

Francois Buergisser (1):
  media: hantro: Set DMA max segment size

Jeffy Chen (1):
  media: hantro: Support RK3399 VP8 decoding

 drivers/staging/media/hantro/Makefile         |   1 +
 drivers/staging/media/hantro/hantro.h         |  15 +-
 drivers/staging/media/hantro/hantro_drv.c     |  53 +-
 .../media/hantro/hantro_g1_mpeg2_dec.c        |  14 +-
 .../staging/media/hantro/hantro_g1_vp8_dec.c  |  34 +-
 .../staging/media/hantro/hantro_h1_jpeg_enc.c |  11 +-
 drivers/staging/media/hantro/hantro_hw.h      |   7 +
 drivers/staging/media/hantro/hantro_vp8.c     |  15 +
 drivers/staging/media/hantro/rk3399_vpu_hw.c  |  22 +-
 .../media/hantro/rk3399_vpu_hw_jpeg_enc.c     |  12 +-
 .../media/hantro/rk3399_vpu_hw_mpeg2_dec.c    |  14 +-
 .../media/hantro/rk3399_vpu_hw_vp8_dec.c      | 595 ++++++++++++++++++
 12 files changed, 708 insertions(+), 85 deletions(-)
 create mode 100644 drivers/staging/media/hantro/rk3399_vpu_hw_vp8_dec.c

-- 
2.22.0

[PATCH v2 1/7] media: hantro: Set DMA max segment size

[Ezequiel Garcia]

From: Francois Buergisser <fbuergisser@chromium.org>

The Hantro codec is typically used in platforms with an IOMMU,
so we need to set a proper DMA segment size. Devices without an
IOMMU will still fallback to default 64KiB segments.

Cc: stable@vger.kernel.org
Fixes: 775fec69008d3 ("media: add Rockchip VPU JPEG encoder driver")
Signed-off-by: Francois Buergisser <fbuergisser@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index b71a06e9159e..4eae1dbb1ac8 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -731,6 +731,7 @@ static int hantro_probe(struct platform_device *pdev)
 		dev_err(vpu->dev, "Could not set DMA coherent mask.\n");
 		return ret;
 	}
+	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
 
 	for (i = 0; i < vpu->variant->num_irqs; i++) {
 		const char *irq_name = vpu->variant->irqs[i].name;
-- 
2.22.0

[PATCH v2 2/7] media: hantro: Simplify the controls creation logic

[Ezequiel Garcia]

From: Boris Brezillon <boris.brezillon@collabora.com>

v4l2_ctrl_new_custom() should work for any kind of control, including
standard ones. With that change, we automatically get support for
menu controls.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro.h     |  2 --
 drivers/staging/media/hantro/hantro_drv.c | 28 +++++++----------------
 2 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 4d7cb7780bde..81e25dfc98b7 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -113,12 +113,10 @@ enum hantro_codec_mode {
 
 /*
  * struct hantro_ctrl - helper type to declare supported controls
- * @id:		V4L2 control ID (V4L2_CID_xxx)
  * @codec:	codec id this control belong to (HANTRO_JPEG_ENCODER, etc.)
  * @cfg:	control configuration
  */
 struct hantro_ctrl {
-	unsigned int id;
 	unsigned int codec;
 	struct v4l2_ctrl_config cfg;
 };
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 4eae1dbb1ac8..ff2dcbf43e81 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -264,31 +264,29 @@ static const struct v4l2_ctrl_ops hantro_ctrl_ops = {
 
 static struct hantro_ctrl controls[] = {
 	{
-		.id = V4L2_CID_JPEG_COMPRESSION_QUALITY,
 		.codec = HANTRO_JPEG_ENCODER,
 		.cfg = {
+			.id = V4L2_CID_JPEG_COMPRESSION_QUALITY,
 			.min = 5,
 			.max = 100,
 			.step = 1,
 			.def = 50,
+			.ops = &hantro_ctrl_ops,
 		},
 	}, {
-		.id = V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS,
 		.codec = HANTRO_MPEG2_DECODER,
 		.cfg = {
-			.elem_size = sizeof(struct v4l2_ctrl_mpeg2_slice_params),
+			.id = V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS,
 		},
 	}, {
-		.id = V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION,
 		.codec = HANTRO_MPEG2_DECODER,
 		.cfg = {
-			.elem_size = sizeof(struct v4l2_ctrl_mpeg2_quantization),
+			.id = V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION,
 		},
 	}, {
-		.id = V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER,
 		.codec = HANTRO_VP8_DECODER,
 		.cfg = {
-			.elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header),
+			.id = V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER,
 		},
 	},
 };
@@ -304,22 +302,12 @@ static int hantro_ctrls_setup(struct hantro_dev *vpu,
 	for (i = 0; i < num_ctrls; i++) {
 		if (!(allowed_codecs & controls[i].codec))
 			continue;
-		if (!controls[i].cfg.elem_size) {
-			v4l2_ctrl_new_std(&ctx->ctrl_handler,
-					  &hantro_ctrl_ops,
-					  controls[i].id, controls[i].cfg.min,
-					  controls[i].cfg.max,
-					  controls[i].cfg.step,
-					  controls[i].cfg.def);
-		} else {
-			controls[i].cfg.id = controls[i].id;
-			v4l2_ctrl_new_custom(&ctx->ctrl_handler,
-					     &controls[i].cfg, NULL);
-		}
 
+		v4l2_ctrl_new_custom(&ctx->ctrl_handler,
+				     &controls[i].cfg, NULL);
 		if (ctx->ctrl_handler.error) {
 			vpu_err("Adding control (%d) failed %d\n",
-				controls[i].id,
+				controls[i].cfg.id,
 				ctx->ctrl_handler.error);
 			v4l2_ctrl_handler_free(&ctx->ctrl_handler);
 			return ctx->ctrl_handler.error;
-- 
2.22.0

[PATCH v2 3/7] media: hantro: Constify the control array

[Ezequiel Garcia]

From: Boris Brezillon <boris.brezillon@collabora.com>

controls[] is not supposed to be modified at runtime, let's make it
explicit by adding a const specifier.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index ff2dcbf43e81..398618b0d586 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -262,7 +262,7 @@ static const struct v4l2_ctrl_ops hantro_ctrl_ops = {
 	.s_ctrl = hantro_s_ctrl,
 };
 
-static struct hantro_ctrl controls[] = {
+static const struct hantro_ctrl controls[] = {
 	{
 		.codec = HANTRO_JPEG_ENCODER,
 		.cfg = {
-- 
2.22.0

[PATCH v2 4/7] media: hantro: Add hantro_get_{src,dst}_buf() helpers

[Ezequiel Garcia]

From: Boris Brezillon <boris.brezillon@collabora.com>

And replace all calls to v4l2_m2m_next_{src,dst}_buf() by
hantro_get_{src,dst}_buf() one.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro.h               | 13 +++++++++++++
 drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c  |  4 ++--
 drivers/staging/media/hantro/hantro_g1_vp8_dec.c    |  8 ++++----
 drivers/staging/media/hantro/hantro_h1_jpeg_enc.c   |  4 ++--
 .../staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c   |  4 ++--
 .../staging/media/hantro/rk3399_vpu_hw_mpeg2_dec.c  |  4 ++--
 6 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 81e25dfc98b7..c4c86c32ea2d 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -20,6 +20,7 @@
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-mem2mem.h>
 #include <media/videobuf2-core.h>
 #include <media/videobuf2-dma-contig.h>
 
@@ -373,4 +374,16 @@ bool hantro_is_encoder_ctx(const struct hantro_ctx *ctx);
 void *hantro_get_ctrl(struct hantro_ctx *ctx, u32 id);
 dma_addr_t hantro_get_ref(struct vb2_queue *q, u64 ts);
 
+static inline struct vb2_v4l2_buffer *
+hantro_get_src_buf(struct hantro_ctx *ctx)
+{
+	return v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+}
+
+static inline struct vb2_v4l2_buffer *
+hantro_get_dst_buf(struct hantro_ctx *ctx)
+{
+	return v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+}
+
 #endif /* HANTRO_H_ */
diff --git a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
index e592c1b66375..55f861e96108 100644
--- a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
@@ -167,8 +167,8 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
 	const struct v4l2_mpeg2_picture *picture;
 	u32 reg;
 
-	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
-	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+	src_buf = hantro_get_src_buf(ctx);
+	dst_buf = hantro_get_dst_buf(ctx);
 
 	/* Apply request controls if any */
 	v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
index 72d983a11ca1..6032cbbecb06 100644
--- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
@@ -260,7 +260,7 @@ static void cfg_parts(struct hantro_ctx *ctx,
 	u32 count = 0;
 	unsigned int i;
 
-	vb2_src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+	vb2_src = hantro_get_src_buf(ctx);
 	src_dma = vb2_dma_contig_plane_dma_addr(&vb2_src->vb2_buf, 0);
 
 	/*
@@ -391,7 +391,7 @@ static void cfg_ref(struct hantro_ctx *ctx,
 	struct vb2_v4l2_buffer *vb2_dst;
 	dma_addr_t ref;
 
-	vb2_dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+	vb2_dst = hantro_get_dst_buf(ctx);
 
 	ref = hantro_get_ref(cap_q, hdr->last_frame_ts);
 	if (!ref)
@@ -424,7 +424,7 @@ static void cfg_buffers(struct hantro_ctx *ctx,
 	dma_addr_t dst_dma;
 	u32 reg;
 
-	vb2_dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+	vb2_dst = hantro_get_dst_buf(ctx);
 
 	/* Set probability table buffer address */
 	vdpu_write_relaxed(vpu, ctx->vp8_dec.prob_tbl.dma,
@@ -453,7 +453,7 @@ void hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
 	u32 mb_width, mb_height;
 	u32 reg;
 
-	vb2_src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+	vb2_src = hantro_get_src_buf(ctx);
 	v4l2_ctrl_request_setup(vb2_src->vb2_buf.req_obj.req,
 				&ctx->ctrl_handler);
 
diff --git a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c b/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
index 0c1e3043dc7e..f5adb5cbde50 100644
--- a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
+++ b/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
@@ -84,8 +84,8 @@ void hantro_h1_jpeg_enc_run(struct hantro_ctx *ctx)
 	struct hantro_jpeg_ctx jpeg_ctx;
 	u32 reg;
 
-	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
-	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+	src_buf = hantro_get_src_buf(ctx);
+	dst_buf = hantro_get_dst_buf(ctx);
 
 	memset(&jpeg_ctx, 0, sizeof(jpeg_ctx));
 	jpeg_ctx.buffer = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c b/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c
index ae66354d2d93..82c5af822766 100644
--- a/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c
+++ b/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c
@@ -116,8 +116,8 @@ void rk3399_vpu_jpeg_enc_run(struct hantro_ctx *ctx)
 	struct media_request *src_req;
 	u32 reg;
 
-	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
-	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+	src_buf = hantro_get_src_buf(ctx);
+	dst_buf = hantro_get_dst_buf(ctx);
 
 	src_req = src_buf->vb2_buf.req_obj.req;
 	v4l2_ctrl_request_setup(src_req, &ctx->ctrl_handler);
diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw_mpeg2_dec.c b/drivers/staging/media/hantro/rk3399_vpu_hw_mpeg2_dec.c
index 8685bddfbcab..451bfcceadba 100644
--- a/drivers/staging/media/hantro/rk3399_vpu_hw_mpeg2_dec.c
+++ b/drivers/staging/media/hantro/rk3399_vpu_hw_mpeg2_dec.c
@@ -169,8 +169,8 @@ void rk3399_vpu_mpeg2_dec_run(struct hantro_ctx *ctx)
 	const struct v4l2_mpeg2_picture *picture;
 	u32 reg;
 
-	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
-	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+	src_buf = hantro_get_src_buf(ctx);
+	dst_buf = hantro_get_dst_buf(ctx);
 
 	/* Apply request controls if any */
 	v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
-- 
2.22.0

[PATCH v2 5/7] media: hantro: Add helpers to prepare/finish a run

[Ezequiel Garcia]

From: Boris Brezillon <boris.brezillon@collabora.com>

And use them where appropriate.

We might want to move hantro_{prepare,finish}_run() calls to
device_run() and have a 2-step approach similar to cedrus (prepare +
trigger) at some point, but let's keep that for later.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
Changes from v1:
* Remove unused variable.
---
 drivers/staging/media/hantro/hantro_drv.c     | 22 +++++++++++++++++++
 .../media/hantro/hantro_g1_mpeg2_dec.c        | 10 ++-------
 .../staging/media/hantro/hantro_g1_vp8_dec.c  | 11 ++--------
 .../staging/media/hantro/hantro_h1_jpeg_enc.c |  7 ++++--
 drivers/staging/media/hantro/hantro_hw.h      |  2 ++
 .../media/hantro/rk3399_vpu_hw_jpeg_enc.c     |  8 ++-----
 .../media/hantro/rk3399_vpu_hw_mpeg2_dec.c    | 10 ++-------
 7 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 398618b0d586..4af6ee80229e 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -153,6 +153,28 @@ void hantro_watchdog(struct work_struct *work)
 	}
 }
 
+void hantro_prepare_run(struct hantro_ctx *ctx)
+{
+	struct vb2_v4l2_buffer *src_buf;
+
+	src_buf = hantro_get_src_buf(ctx);
+	v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
+				&ctx->ctrl_handler);
+}
+
+void hantro_finish_run(struct hantro_ctx *ctx)
+{
+	struct vb2_v4l2_buffer *src_buf;
+
+	src_buf = hantro_get_src_buf(ctx);
+	v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
+				   &ctx->ctrl_handler);
+
+	/* Kick the watchdog. */
+	schedule_delayed_work(&ctx->dev->watchdog_work,
+			      msecs_to_jiffies(2000));
+}
+
 static void device_run(void *priv)
 {
 	struct hantro_ctx *ctx = priv;
diff --git a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
index 55f861e96108..80f0e94f8afa 100644
--- a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
@@ -171,8 +171,7 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
 	dst_buf = hantro_get_dst_buf(ctx);
 
 	/* Apply request controls if any */
-	v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
-				&ctx->ctrl_handler);
+	hantro_prepare_run(ctx);
 
 	slice_params = hantro_get_ctrl(ctx,
 				       V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS);
@@ -248,12 +247,7 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
 					&dst_buf->vb2_buf,
 					sequence, picture, slice_params);
 
-	/* Controls no longer in-use, we can complete them */
-	v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
-				   &ctx->ctrl_handler);
-
-	/* Kick the watchdog and start decoding */
-	schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
+	hantro_finish_run(ctx);
 
 	reg = G1_REG_DEC_E(1);
 	vdpu_write(vpu, reg, G1_SWREG(1));
diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
index 6032cbbecb06..cd1fbd3a0d5f 100644
--- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
@@ -449,13 +449,10 @@ void hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
 	struct hantro_dev *vpu = ctx->dev;
 	size_t height = ctx->dst_fmt.height;
 	size_t width = ctx->dst_fmt.width;
-	struct vb2_v4l2_buffer *vb2_src;
 	u32 mb_width, mb_height;
 	u32 reg;
 
-	vb2_src = hantro_get_src_buf(ctx);
-	v4l2_ctrl_request_setup(vb2_src->vb2_buf.req_obj.req,
-				&ctx->ctrl_handler);
+	hantro_prepare_run(ctx);
 
 	hdr = hantro_get_ctrl(ctx, V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER);
 	if (WARN_ON(!hdr))
@@ -516,11 +513,7 @@ void hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
 	cfg_ref(ctx, hdr);
 	cfg_buffers(ctx, hdr);
 
-	/* Controls no longer in-use, we can complete them */
-	v4l2_ctrl_request_complete(vb2_src->vb2_buf.req_obj.req,
-				   &ctx->ctrl_handler);
-
-	schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
+	hantro_finish_run(ctx);
 
 	vdpu_write(vpu, G1_REG_INTERRUPT_DEC_E, G1_REG_INTERRUPT);
 }
diff --git a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c b/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
index f5adb5cbde50..ecd34a7db190 100644
--- a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
+++ b/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
@@ -87,6 +87,8 @@ void hantro_h1_jpeg_enc_run(struct hantro_ctx *ctx)
 	src_buf = hantro_get_src_buf(ctx);
 	dst_buf = hantro_get_dst_buf(ctx);
 
+	hantro_prepare_run(ctx);
+
 	memset(&jpeg_ctx, 0, sizeof(jpeg_ctx));
 	jpeg_ctx.buffer = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
 	jpeg_ctx.width = ctx->dst_fmt.width;
@@ -119,7 +121,8 @@ void hantro_h1_jpeg_enc_run(struct hantro_ctx *ctx)
 		| H1_REG_ENC_CTRL_ENC_MODE_JPEG
 		| H1_REG_ENC_PIC_INTRA
 		| H1_REG_ENC_CTRL_EN_BIT;
-	/* Kick the watchdog and start encoding */
-	schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
+
+	hantro_finish_run(ctx);
+
 	vepu_write(vpu, reg, H1_REG_ENC_CTRL);
 }
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 7849852affde..34ef24e3a9ef 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -97,6 +97,8 @@ void hantro_watchdog(struct work_struct *work);
 void hantro_run(struct hantro_ctx *ctx);
 void hantro_irq_done(struct hantro_dev *vpu, unsigned int bytesused,
 		     enum vb2_buffer_state result);
+void hantro_prepare_run(struct hantro_ctx *ctx);
+void hantro_finish_run(struct hantro_ctx *ctx);
 
 void hantro_h1_jpeg_enc_run(struct hantro_ctx *ctx);
 void rk3399_vpu_jpeg_enc_run(struct hantro_ctx *ctx);
diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c b/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c
index 82c5af822766..06162f569b5e 100644
--- a/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c
+++ b/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c
@@ -113,14 +113,12 @@ void rk3399_vpu_jpeg_enc_run(struct hantro_ctx *ctx)
 	struct hantro_dev *vpu = ctx->dev;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
 	struct hantro_jpeg_ctx jpeg_ctx;
-	struct media_request *src_req;
 	u32 reg;
 
 	src_buf = hantro_get_src_buf(ctx);
 	dst_buf = hantro_get_dst_buf(ctx);
 
-	src_req = src_buf->vb2_buf.req_obj.req;
-	v4l2_ctrl_request_setup(src_req, &ctx->ctrl_handler);
+	hantro_prepare_run(ctx);
 
 	memset(&jpeg_ctx, 0, sizeof(jpeg_ctx));
 	jpeg_ctx.buffer = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
@@ -157,9 +155,7 @@ void rk3399_vpu_jpeg_enc_run(struct hantro_ctx *ctx)
 		| VEPU_REG_ENCODE_FORMAT_JPEG
 		| VEPU_REG_ENCODE_ENABLE;
 
-	v4l2_ctrl_request_complete(src_req, &ctx->ctrl_handler);
-
 	/* Kick the watchdog and start encoding */
-	schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
+	hantro_finish_run(ctx);
 	vepu_write(vpu, reg, VEPU_REG_ENCODE_START);
 }
diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw_mpeg2_dec.c b/drivers/staging/media/hantro/rk3399_vpu_hw_mpeg2_dec.c
index 451bfcceadba..e7ba5c0441cc 100644
--- a/drivers/staging/media/hantro/rk3399_vpu_hw_mpeg2_dec.c
+++ b/drivers/staging/media/hantro/rk3399_vpu_hw_mpeg2_dec.c
@@ -172,9 +172,7 @@ void rk3399_vpu_mpeg2_dec_run(struct hantro_ctx *ctx)
 	src_buf = hantro_get_src_buf(ctx);
 	dst_buf = hantro_get_dst_buf(ctx);
 
-	/* Apply request controls if any */
-	v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
-				&ctx->ctrl_handler);
+	hantro_prepare_run(ctx);
 
 	slice_params = hantro_get_ctrl(ctx,
 				       V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS);
@@ -254,12 +252,8 @@ void rk3399_vpu_mpeg2_dec_run(struct hantro_ctx *ctx)
 					 &dst_buf->vb2_buf,
 					 sequence, picture, slice_params);
 
-	/* Controls no longer in-use, we can complete them */
-	v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
-				   &ctx->ctrl_handler);
-
 	/* Kick the watchdog and start decoding */
-	schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
+	hantro_finish_run(ctx);
 
 	reg = vdpu_read(vpu, VDPU_SWREG(57)) | VDPU_REG_DEC_E(1);
 	vdpu_write(vpu, reg, VDPU_SWREG(57));
-- 
2.22.0

[PATCH v2 6/7] media: hantro: Move VP8 common code

[Ezequiel Garcia]

In order to introduce support for RK3399 VP8 decoding,
move some common VP8 code. This will be reused by
the RK3399 implementation, reducing code duplication.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../staging/media/hantro/hantro_g1_vp8_dec.c    | 17 -----------------
 drivers/staging/media/hantro/hantro_hw.h        |  4 ++++
 drivers/staging/media/hantro/hantro_vp8.c       | 15 +++++++++++++++
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
index cd1fbd3a0d5f..181e2f76d8cb 100644
--- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
@@ -16,8 +16,6 @@
 #include "hantro.h"
 #include "hantro_g1_regs.h"
 
-#define DEC_8190_ALIGN_MASK	0x07U
-
 /* DCT partition base address regs */
 static const struct hantro_reg vp8_dec_dct_base[8] = {
 	{ G1_REG_ADDR_STR, 0, 0xffffffff },
@@ -131,21 +129,6 @@ static const struct hantro_reg vp8_dec_pred_bc_tap[8][4] = {
 	},
 };
 
-/*
- * filter taps taken to 7-bit precision,
- * reference RFC6386#Page-16, filters[8][6]
- */
-static const u32 vp8_dec_mc_filter[8][6] = {
-	{ 0, 0, 128, 0, 0, 0 },
-	{ 0, -6, 123, 12, -1, 0 },
-	{ 2, -11, 108, 36, -8, 1 },
-	{ 0, -9, 93, 50, -6, 0 },
-	{ 3, -16, 77, 77, -16, 3 },
-	{ 0, -6, 50, 93, -9, 0 },
-	{ 1, -8, 36, 108, -11, 2 },
-	{ 0, -1, 12, 123, -6, 0 }
-};
-
 /*
  * Set loop filters
  */
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 34ef24e3a9ef..185e27d47e47 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -15,6 +15,8 @@
 #include <media/vp8-ctrls.h>
 #include <media/videobuf2-core.h>
 
+#define DEC_8190_ALIGN_MASK	0x07U
+
 struct hantro_dev;
 struct hantro_ctx;
 struct hantro_buf;
@@ -93,6 +95,8 @@ extern const struct hantro_variant rk3399_vpu_variant;
 extern const struct hantro_variant rk3328_vpu_variant;
 extern const struct hantro_variant rk3288_vpu_variant;
 
+extern const u32 vp8_dec_mc_filter[8][6];
+
 void hantro_watchdog(struct work_struct *work);
 void hantro_run(struct hantro_ctx *ctx);
 void hantro_irq_done(struct hantro_dev *vpu, unsigned int bytesused,
diff --git a/drivers/staging/media/hantro/hantro_vp8.c b/drivers/staging/media/hantro/hantro_vp8.c
index 66c45335d871..be5cb01d1309 100644
--- a/drivers/staging/media/hantro/hantro_vp8.c
+++ b/drivers/staging/media/hantro/hantro_vp8.c
@@ -31,6 +31,21 @@ struct vp8_prob_tbl_packed {
 	u8 padding3[96];
 };
 
+/*
+ * filter taps taken to 7-bit precision,
+ * reference RFC6386#Page-16, filters[8][6]
+ */
+const u32 vp8_dec_mc_filter[8][6] = {
+	{ 0, 0, 128, 0, 0, 0 },
+	{ 0, -6, 123, 12, -1, 0 },
+	{ 2, -11, 108, 36, -8, 1 },
+	{ 0, -9, 93, 50, -6, 0 },
+	{ 3, -16, 77, 77, -16, 3 },
+	{ 0, -6, 50, 93, -9, 0 },
+	{ 1, -8, 36, 108, -11, 2 },
+	{ 0, -1, 12, 123, -6, 0 }
+};
+
 void hantro_vp8_prob_update(struct hantro_ctx *ctx,
 			    const struct v4l2_ctrl_vp8_frame_header *hdr)
 {
-- 
2.22.0

[PATCH v2 7/7] media: hantro: Support RK3399 VP8 decoding

[Ezequiel Garcia]

From: Jeffy Chen <jeffy.chen@rock-chips.com>

Rockchip RK3399 SoC has the same Hantro G1 IP block
as RK3288, but the registers are entirely different.

In a similar fashion as MPEG-2 decoding, it's simpler
to just add a separate implementation.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
Changes from v1:
* Remove unused variable.
* Used buffer helpers where possible.
---
 drivers/staging/media/hantro/Makefile         |   1 +
 drivers/staging/media/hantro/hantro_hw.h      |   1 +
 drivers/staging/media/hantro/rk3399_vpu_hw.c  |  22 +-
 .../media/hantro/rk3399_vpu_hw_vp8_dec.c      | 595 ++++++++++++++++++
 4 files changed, 618 insertions(+), 1 deletion(-)
 create mode 100644 drivers/staging/media/hantro/rk3399_vpu_hw_vp8_dec.c

diff --git a/drivers/staging/media/hantro/Makefile b/drivers/staging/media/hantro/Makefile
index a627aee77f75..f5ec597d9e08 100644
--- a/drivers/staging/media/hantro/Makefile
+++ b/drivers/staging/media/hantro/Makefile
@@ -8,6 +8,7 @@ hantro-vpu-y += \
 		hantro_g1_vp8_dec.o \
 		rk3399_vpu_hw_jpeg_enc.o \
 		rk3399_vpu_hw_mpeg2_dec.o \
+		rk3399_vpu_hw_vp8_dec.o \
 		hantro_jpeg.o \
 		hantro_mpeg2.o \
 		hantro_vp8.o
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 185e27d47e47..e86c84fbfe1a 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -117,6 +117,7 @@ int hantro_mpeg2_dec_init(struct hantro_ctx *ctx);
 void hantro_mpeg2_dec_exit(struct hantro_ctx *ctx);
 
 void hantro_g1_vp8_dec_run(struct hantro_ctx *ctx);
+void rk3399_vpu_vp8_dec_run(struct hantro_ctx *ctx);
 int hantro_vp8_dec_init(struct hantro_ctx *ctx);
 void hantro_vp8_dec_exit(struct hantro_ctx *ctx);
 void hantro_vp8_prob_update(struct hantro_ctx *ctx,
diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw.c b/drivers/staging/media/hantro/rk3399_vpu_hw.c
index f8400e49bc50..414b1d3fbb1f 100644
--- a/drivers/staging/media/hantro/rk3399_vpu_hw.c
+++ b/drivers/staging/media/hantro/rk3399_vpu_hw.c
@@ -73,6 +73,19 @@ static const struct hantro_fmt rk3399_vpu_dec_fmts[] = {
 			.step_height = MPEG2_MB_DIM,
 		},
 	},
+	{
+		.fourcc = V4L2_PIX_FMT_VP8_FRAME,
+		.codec_mode = HANTRO_MODE_VP8_DEC,
+		.max_depth = 2,
+		.frmsize = {
+			.min_width = 48,
+			.max_width = 3840,
+			.step_width = VP8_MB_DIM,
+			.min_height = 48,
+			.max_height = 2160,
+			.step_height = VP8_MB_DIM,
+		},
+	},
 };
 
 static irqreturn_t rk3399_vepu_irq(int irq, void *dev_id)
@@ -154,6 +167,12 @@ static const struct hantro_codec_ops rk3399_vpu_codec_ops[] = {
 		.init = hantro_mpeg2_dec_init,
 		.exit = hantro_mpeg2_dec_exit,
 	},
+	[HANTRO_MODE_VP8_DEC] = {
+		.run = rk3399_vpu_vp8_dec_run,
+		.reset = rk3399_vpu_dec_reset,
+		.init = hantro_vp8_dec_init,
+		.exit = hantro_vp8_dec_exit,
+	},
 };
 
 /*
@@ -176,7 +195,8 @@ const struct hantro_variant rk3399_vpu_variant = {
 	.dec_offset = 0x400,
 	.dec_fmts = rk3399_vpu_dec_fmts,
 	.num_dec_fmts = ARRAY_SIZE(rk3399_vpu_dec_fmts),
-	.codec = HANTRO_JPEG_ENCODER | HANTRO_MPEG2_DECODER,
+	.codec = HANTRO_JPEG_ENCODER | HANTRO_MPEG2_DECODER |
+		 HANTRO_VP8_DECODER,
 	.codec_ops = rk3399_vpu_codec_ops,
 	.irqs = rk3399_irqs,
 	.num_irqs = ARRAY_SIZE(rk3399_irqs),
diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw_vp8_dec.c b/drivers/staging/media/hantro/rk3399_vpu_hw_vp8_dec.c
new file mode 100644
index 000000000000..c5e9f8befe9c
--- /dev/null
+++ b/drivers/staging/media/hantro/rk3399_vpu_hw_vp8_dec.c
@@ -0,0 +1,595 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rockchip VPU codec vp8 decode driver
+ *
+ * Copyright (C) 2014 Rockchip Electronics Co., Ltd.
+ *	ZhiChao Yu <zhichao.yu@rock-chips.com>
+ *
+ * Copyright (C) 2014 Google LLC.
+ *      Tomasz Figa <tfiga@chromium.org>
+ *
+ * Copyright (C) 2015 Rockchip Electronics Co., Ltd.
+ *      Alpha Lin <alpha.lin@rock-chips.com>
+ */
+
+#include <media/v4l2-mem2mem.h>
+#include <media/vp8-ctrls.h>
+
+#include "hantro_hw.h"
+#include "hantro.h"
+#include "hantro_g1_regs.h"
+
+#define VDPU_REG_DEC_CTRL0			0x0c8
+#define VDPU_REG_STREAM_LEN			0x0cc
+#define VDPU_REG_DEC_FORMAT			0x0d4
+#define     VDPU_REG_DEC_CTRL0_DEC_MODE(x)		(((x) & 0xf) << 0)
+#define VDPU_REG_DATA_ENDIAN			0x0d8
+#define     VDPU_REG_CONFIG_DEC_STRENDIAN_E		BIT(5)
+#define     VDPU_REG_CONFIG_DEC_STRSWAP32_E		BIT(4)
+#define     VDPU_REG_CONFIG_DEC_OUTSWAP32_E		BIT(3)
+#define     VDPU_REG_CONFIG_DEC_INSWAP32_E		BIT(2)
+#define     VDPU_REG_CONFIG_DEC_OUT_ENDIAN		BIT(1)
+#define     VDPU_REG_CONFIG_DEC_IN_ENDIAN		BIT(0)
+#define VDPU_REG_AXI_CTRL			0x0e0
+#define     VDPU_REG_CONFIG_DEC_MAX_BURST(x)		(((x) & 0x1f) << 16)
+#define VDPU_REG_EN_FLAGS			0x0e4
+#define     VDPU_REG_DEC_CTRL0_PIC_INTER_E		BIT(14)
+#define     VDPU_REG_CONFIG_DEC_TIMEOUT_E		BIT(5)
+#define     VDPU_REG_CONFIG_DEC_CLK_GATE_E		BIT(4)
+#define VDPU_REG_PRED_FLT			0x0ec
+#define VDPU_REG_ADDR_QTABLE			0x0f4
+#define VDPU_REG_ADDR_DST			0x0fc
+#define VDPU_REG_ADDR_STR			0x100
+#define VDPU_REG_VP8_PIC_MB_SIZE		0x1e0
+#define VDPU_REG_VP8_DCT_START_BIT		0x1e4
+#define     VDPU_REG_DEC_CTRL4_VC1_HEIGHT_EXT		BIT(13)
+#define     VDPU_REG_DEC_CTRL4_BILIN_MC_E		BIT(12)
+#define VDPU_REG_VP8_CTRL0			0x1e8
+#define VDPU_REG_VP8_DATA_VAL			0x1f0
+#define VDPU_REG_PRED_FLT7			0x1f4
+#define VDPU_REG_PRED_FLT8			0x1f8
+#define VDPU_REG_PRED_FLT9			0x1fc
+#define VDPU_REG_PRED_FLT10			0x200
+#define VDPU_REG_FILTER_LEVEL			0x204
+#define VDPU_REG_VP8_QUANTER0			0x208
+#define VDPU_REG_VP8_ADDR_REF0			0x20c
+#define VDPU_REG_FILTER_MB_ADJ			0x210
+#define     VDPU_REG_REF_PIC_FILT_TYPE_E		BIT(31)
+#define     VDPU_REG_REF_PIC_FILT_SHARPNESS(x)		(((x) & 0x7) << 28)
+#define VDPU_REG_FILTER_REF_ADJ			0x214
+#define VDPU_REG_VP8_ADDR_REF2_5(i)		(0x218 + ((i) * 0x4))
+#define     VDPU_REG_VP8_GREF_SIGN_BIAS			BIT(0)
+#define     VDPU_REG_VP8_AREF_SIGN_BIAS			BIT(0)
+#define VDPU_REG_VP8_DCT_BASE(i)		\
+		(0x230 + ((((i) < 5) ? (i) : ((i) + 1)) * 0x4))
+#define VDPU_REG_VP8_ADDR_CTRL_PART		0x244
+#define VDPU_REG_VP8_SEGMENT_VAL		0x254
+#define     VDPU_REG_FWD_PIC1_SEGMENT_BASE(x)		((x) << 0)
+#define     VDPU_REG_FWD_PIC1_SEGMENT_UPD_E		BIT(1)
+#define     VDPU_REG_FWD_PIC1_SEGMENT_E			BIT(0)
+#define VDPU_REG_VP8_DCT_START_BIT2		0x258
+#define VDPU_REG_VP8_QUANTER1			0x25c
+#define VDPU_REG_VP8_QUANTER2			0x260
+#define VDPU_REG_PRED_FLT1			0x264
+#define VDPU_REG_PRED_FLT2			0x268
+#define VDPU_REG_PRED_FLT3			0x26c
+#define VDPU_REG_PRED_FLT4			0x270
+#define VDPU_REG_PRED_FLT5			0x274
+#define VDPU_REG_PRED_FLT6			0x278
+
+static const struct hantro_reg vp8_dec_dct_base[8] = {
+	{ VDPU_REG_ADDR_STR, 0, 0xffffffff },
+	{ VDPU_REG_VP8_DCT_BASE(0), 0, 0xffffffff },
+	{ VDPU_REG_VP8_DCT_BASE(1), 0, 0xffffffff },
+	{ VDPU_REG_VP8_DCT_BASE(2), 0, 0xffffffff },
+	{ VDPU_REG_VP8_DCT_BASE(3), 0, 0xffffffff },
+	{ VDPU_REG_VP8_DCT_BASE(4), 0, 0xffffffff },
+	{ VDPU_REG_VP8_DCT_BASE(5), 0, 0xffffffff },
+	{ VDPU_REG_VP8_DCT_BASE(6), 0, 0xffffffff },
+};
+
+static const struct hantro_reg vp8_dec_lf_level[4] = {
+	{ VDPU_REG_FILTER_LEVEL, 18, 0x3f },
+	{ VDPU_REG_FILTER_LEVEL, 12, 0x3f },
+	{ VDPU_REG_FILTER_LEVEL, 6, 0x3f },
+	{ VDPU_REG_FILTER_LEVEL, 0, 0x3f },
+};
+
+static const struct hantro_reg vp8_dec_mb_adj[4] = {
+	{ VDPU_REG_FILTER_MB_ADJ, 21, 0x7f },
+	{ VDPU_REG_FILTER_MB_ADJ, 14, 0x7f },
+	{ VDPU_REG_FILTER_MB_ADJ, 7, 0x7f },
+	{ VDPU_REG_FILTER_MB_ADJ, 0, 0x7f },
+};
+
+static const struct hantro_reg vp8_dec_ref_adj[4] = {
+	{ VDPU_REG_FILTER_REF_ADJ, 21, 0x7f },
+	{ VDPU_REG_FILTER_REF_ADJ, 14, 0x7f },
+	{ VDPU_REG_FILTER_REF_ADJ, 7, 0x7f },
+	{ VDPU_REG_FILTER_REF_ADJ, 0, 0x7f },
+};
+
+static const struct hantro_reg vp8_dec_quant[4] = {
+	{ VDPU_REG_VP8_QUANTER0, 11, 0x7ff },
+	{ VDPU_REG_VP8_QUANTER0, 0, 0x7ff },
+	{ VDPU_REG_VP8_QUANTER1, 11, 0x7ff },
+	{ VDPU_REG_VP8_QUANTER1, 0, 0x7ff },
+};
+
+static const struct hantro_reg vp8_dec_quant_delta[5] = {
+	{ VDPU_REG_VP8_QUANTER0, 27, 0x1f },
+	{ VDPU_REG_VP8_QUANTER0, 22, 0x1f },
+	{ VDPU_REG_VP8_QUANTER1, 27, 0x1f },
+	{ VDPU_REG_VP8_QUANTER1, 22, 0x1f },
+	{ VDPU_REG_VP8_QUANTER2, 27, 0x1f },
+};
+
+static const struct hantro_reg vp8_dec_dct_start_bits[8] = {
+	{ VDPU_REG_VP8_CTRL0, 26, 0x3f },
+	{ VDPU_REG_VP8_DCT_START_BIT, 26, 0x3f },
+	{ VDPU_REG_VP8_DCT_START_BIT, 20, 0x3f },
+	{ VDPU_REG_VP8_DCT_START_BIT2, 24, 0x3f },
+	{ VDPU_REG_VP8_DCT_START_BIT2, 18, 0x3f },
+	{ VDPU_REG_VP8_DCT_START_BIT2, 12, 0x3f },
+	{ VDPU_REG_VP8_DCT_START_BIT2, 6, 0x3f },
+	{ VDPU_REG_VP8_DCT_START_BIT2, 0, 0x3f },
+};
+
+static const struct hantro_reg vp8_dec_pred_bc_tap[8][6] = {
+	{
+		{ 0, 0, 0},
+		{ VDPU_REG_PRED_FLT, 22, 0x3ff },
+		{ VDPU_REG_PRED_FLT, 12, 0x3ff },
+		{ VDPU_REG_PRED_FLT, 2, 0x3ff },
+		{ VDPU_REG_PRED_FLT1, 22, 0x3ff },
+		{ 0, 0, 0},
+	}, {
+		{ 0, 0, 0},
+		{ VDPU_REG_PRED_FLT1, 12, 0x3ff },
+		{ VDPU_REG_PRED_FLT1, 2, 0x3ff },
+		{ VDPU_REG_PRED_FLT2, 22, 0x3ff },
+		{ VDPU_REG_PRED_FLT2, 12, 0x3ff },
+		{ 0, 0, 0},
+	}, {
+		{ VDPU_REG_PRED_FLT10, 10, 0x3 },
+		{ VDPU_REG_PRED_FLT2, 2, 0x3ff },
+		{ VDPU_REG_PRED_FLT3, 22, 0x3ff },
+		{ VDPU_REG_PRED_FLT3, 12, 0x3ff },
+		{ VDPU_REG_PRED_FLT3, 2, 0x3ff },
+		{ VDPU_REG_PRED_FLT10, 8, 0x3},
+	}, {
+		{ 0, 0, 0},
+		{ VDPU_REG_PRED_FLT4, 22, 0x3ff },
+		{ VDPU_REG_PRED_FLT4, 12, 0x3ff },
+		{ VDPU_REG_PRED_FLT4, 2, 0x3ff },
+		{ VDPU_REG_PRED_FLT5, 22, 0x3ff },
+		{ 0, 0, 0},
+	}, {
+		{ VDPU_REG_PRED_FLT10, 6, 0x3 },
+		{ VDPU_REG_PRED_FLT5, 12, 0x3ff },
+		{ VDPU_REG_PRED_FLT5, 2, 0x3ff },
+		{ VDPU_REG_PRED_FLT6, 22, 0x3ff },
+		{ VDPU_REG_PRED_FLT6, 12, 0x3ff },
+		{ VDPU_REG_PRED_FLT10, 4, 0x3 },
+	}, {
+		{ 0, 0, 0},
+		{ VDPU_REG_PRED_FLT6, 2, 0x3ff },
+		{ VDPU_REG_PRED_FLT7, 22, 0x3ff },
+		{ VDPU_REG_PRED_FLT7, 12, 0x3ff },
+		{ VDPU_REG_PRED_FLT7, 2, 0x3ff },
+		{ 0, 0, 0},
+	}, {
+		{ VDPU_REG_PRED_FLT10, 2, 0x3 },
+		{ VDPU_REG_PRED_FLT8, 22, 0x3ff },
+		{ VDPU_REG_PRED_FLT8, 12, 0x3ff },
+		{ VDPU_REG_PRED_FLT8, 2, 0x3ff },
+		{ VDPU_REG_PRED_FLT9, 22, 0x3ff },
+		{ VDPU_REG_PRED_FLT10, 0, 0x3 },
+	}, {
+		{ 0, 0, 0},
+		{ VDPU_REG_PRED_FLT9, 12, 0x3ff },
+		{ VDPU_REG_PRED_FLT9, 2, 0x3ff },
+		{ VDPU_REG_PRED_FLT10, 22, 0x3ff },
+		{ VDPU_REG_PRED_FLT10, 12, 0x3ff },
+		{ 0, 0, 0},
+	},
+};
+
+static const struct hantro_reg vp8_dec_mb_start_bit = {
+	.base = VDPU_REG_VP8_CTRL0,
+	.shift = 18,
+	.mask = 0x3f
+};
+
+static const struct hantro_reg vp8_dec_mb_aligned_data_len = {
+	.base = VDPU_REG_VP8_DATA_VAL,
+	.shift = 0,
+	.mask = 0x3fffff
+};
+
+static const struct hantro_reg vp8_dec_num_dct_partitions = {
+	.base = VDPU_REG_VP8_DATA_VAL,
+	.shift = 24,
+	.mask = 0xf
+};
+
+static const struct hantro_reg vp8_dec_stream_len = {
+	.base = VDPU_REG_STREAM_LEN,
+	.shift = 0,
+	.mask = 0xffffff
+};
+
+static const struct hantro_reg vp8_dec_mb_width = {
+	.base = VDPU_REG_VP8_PIC_MB_SIZE,
+	.shift = 23,
+	.mask = 0x1ff
+};
+
+static const struct hantro_reg vp8_dec_mb_height = {
+	.base = VDPU_REG_VP8_PIC_MB_SIZE,
+	.shift = 11,
+	.mask = 0xff
+};
+
+static const struct hantro_reg vp8_dec_mb_width_ext = {
+	.base = VDPU_REG_VP8_PIC_MB_SIZE,
+	.shift = 3,
+	.mask = 0x7
+};
+
+static const struct hantro_reg vp8_dec_mb_height_ext = {
+	.base = VDPU_REG_VP8_PIC_MB_SIZE,
+	.shift = 0,
+	.mask = 0x7
+};
+
+static const struct hantro_reg vp8_dec_bool_range = {
+	.base = VDPU_REG_VP8_CTRL0,
+	.shift = 0,
+	.mask = 0xff
+};
+
+static const struct hantro_reg vp8_dec_bool_value = {
+	.base = VDPU_REG_VP8_CTRL0,
+	.shift = 8,
+	.mask = 0xff
+};
+
+static const struct hantro_reg vp8_dec_filter_disable = {
+	.base = VDPU_REG_DEC_CTRL0,
+	.shift = 8,
+	.mask = 1
+};
+
+static const struct hantro_reg vp8_dec_skip_mode = {
+	.base = VDPU_REG_DEC_CTRL0,
+	.shift = 9,
+	.mask = 1
+};
+
+static const struct hantro_reg vp8_dec_start_dec = {
+	.base = VDPU_REG_EN_FLAGS,
+	.shift = 0,
+	.mask = 1
+};
+
+static void cfg_lf(struct hantro_ctx *ctx,
+		   const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+	const struct v4l2_vp8_segment_header *seg = &hdr->segment_header;
+	const struct v4l2_vp8_loopfilter_header *lf = &hdr->lf_header;
+	struct hantro_dev *vpu = ctx->dev;
+	unsigned int i;
+	u32 reg;
+
+	if (!(seg->flags & V4L2_VP8_SEGMENT_HEADER_FLAG_ENABLED)) {
+		hantro_reg_write(vpu, &vp8_dec_lf_level[0], lf->level);
+	} else if (seg->flags & V4L2_VP8_SEGMENT_HEADER_FLAG_DELTA_VALUE_MODE) {
+		for (i = 0; i < 4; i++) {
+			u32 lf_level = clamp(lf->level + seg->lf_update[i],
+					     0, 63);
+
+			hantro_reg_write(vpu, &vp8_dec_lf_level[i], lf_level);
+		}
+	} else {
+		for (i = 0; i < 4; i++)
+			hantro_reg_write(vpu, &vp8_dec_lf_level[i],
+					 seg->lf_update[i]);
+	}
+
+	reg = VDPU_REG_REF_PIC_FILT_SHARPNESS(lf->sharpness_level);
+	if (lf->flags & V4L2_VP8_LF_FILTER_TYPE_SIMPLE)
+		reg |= VDPU_REG_REF_PIC_FILT_TYPE_E;
+	vdpu_write_relaxed(vpu, reg, VDPU_REG_FILTER_MB_ADJ);
+
+	if (lf->flags & V4L2_VP8_LF_HEADER_ADJ_ENABLE) {
+		for (i = 0; i < 4; i++) {
+			hantro_reg_write(vpu, &vp8_dec_mb_adj[i],
+					 lf->mb_mode_delta[i]);
+			hantro_reg_write(vpu, &vp8_dec_ref_adj[i],
+					 lf->ref_frm_delta[i]);
+		}
+	}
+}
+
+static void cfg_qp(struct hantro_ctx *ctx,
+		   const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+	const struct v4l2_vp8_quantization_header *q = &hdr->quant_header;
+	const struct v4l2_vp8_segment_header *seg = &hdr->segment_header;
+	struct hantro_dev *vpu = ctx->dev;
+	unsigned int i;
+
+	if (!(seg->flags & V4L2_VP8_SEGMENT_HEADER_FLAG_ENABLED)) {
+		hantro_reg_write(vpu, &vp8_dec_quant[0], q->y_ac_qi);
+	} else if (seg->flags & V4L2_VP8_SEGMENT_HEADER_FLAG_DELTA_VALUE_MODE) {
+		for (i = 0; i < 4; i++) {
+			u32 quant = clamp(q->y_ac_qi + seg->quant_update[i],
+					  0, 127);
+
+			hantro_reg_write(vpu, &vp8_dec_quant[i], quant);
+		}
+	} else {
+		for (i = 0; i < 4; i++)
+			hantro_reg_write(vpu, &vp8_dec_quant[i],
+					 seg->quant_update[i]);
+	}
+
+	hantro_reg_write(vpu, &vp8_dec_quant_delta[0], q->y_dc_delta);
+	hantro_reg_write(vpu, &vp8_dec_quant_delta[1], q->y2_dc_delta);
+	hantro_reg_write(vpu, &vp8_dec_quant_delta[2], q->y2_ac_delta);
+	hantro_reg_write(vpu, &vp8_dec_quant_delta[3], q->uv_dc_delta);
+	hantro_reg_write(vpu, &vp8_dec_quant_delta[4], q->uv_ac_delta);
+}
+
+static void cfg_parts(struct hantro_ctx *ctx,
+		      const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+	struct hantro_dev *vpu = ctx->dev;
+	struct vb2_v4l2_buffer *vb2_src;
+	u32 first_part_offset = VP8_FRAME_IS_KEY_FRAME(hdr) ? 10 : 3;
+	u32 mb_size, mb_offset_bytes, mb_offset_bits, mb_start_bits;
+	u32 dct_size_part_size, dct_part_offset;
+	dma_addr_t src_dma;
+	u32 dct_part_total_len = 0;
+	u32 count = 0;
+	unsigned int i;
+
+	vb2_src = hantro_get_src_buf(ctx);
+	src_dma = vb2_dma_contig_plane_dma_addr(&vb2_src->vb2_buf, 0);
+
+	/*
+	 * Calculate control partition mb data info
+	 * @first_part_header_bits:	bits offset of mb data from first
+	 *				part start pos
+	 * @mb_offset_bits:		bits offset of mb data from src_dma
+	 *				base addr
+	 * @mb_offset_byte:		bytes offset of mb data from src_dma
+	 *				base addr
+	 * @mb_start_bits:		bits offset of mb data from mb data
+	 *				64bits alignment addr
+	 */
+	mb_offset_bits = first_part_offset * 8 +
+			 hdr->first_part_header_bits + 8;
+	mb_offset_bytes = mb_offset_bits / 8;
+	mb_start_bits = mb_offset_bits -
+			(mb_offset_bytes & (~DEC_8190_ALIGN_MASK)) * 8;
+	mb_size = hdr->first_part_size -
+		  (mb_offset_bytes - first_part_offset) +
+		  (mb_offset_bytes & DEC_8190_ALIGN_MASK);
+
+	/* Macroblock data aligned base addr */
+	vdpu_write_relaxed(vpu, (mb_offset_bytes & (~DEC_8190_ALIGN_MASK)) +
+			   src_dma, VDPU_REG_VP8_ADDR_CTRL_PART);
+	hantro_reg_write(vpu, &vp8_dec_mb_start_bit, mb_start_bits);
+	hantro_reg_write(vpu, &vp8_dec_mb_aligned_data_len, mb_size);
+
+	/*
+	 * Calculate DCT partition info
+	 * @dct_size_part_size: Containing sizes of DCT part, every DCT part
+	 *			has 3 bytes to store its size, except the last
+	 *			DCT part
+	 * @dct_part_offset:	bytes offset of DCT parts from src_dma base addr
+	 * @dct_part_total_len: total size of all DCT parts
+	 */
+	dct_size_part_size = (hdr->num_dct_parts - 1) * 3;
+	dct_part_offset = first_part_offset + hdr->first_part_size;
+	for (i = 0; i < hdr->num_dct_parts; i++)
+		dct_part_total_len += hdr->dct_part_sizes[i];
+	dct_part_total_len += dct_size_part_size;
+	dct_part_total_len += (dct_part_offset & DEC_8190_ALIGN_MASK);
+
+	/* Number of DCT partitions */
+	hantro_reg_write(vpu, &vp8_dec_num_dct_partitions,
+			 hdr->num_dct_parts - 1);
+
+	/* DCT partition length */
+	hantro_reg_write(vpu, &vp8_dec_stream_len, dct_part_total_len);
+
+	/* DCT partitions base address */
+	for (i = 0; i < hdr->num_dct_parts; i++) {
+		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
+		u32 base_addr = byte_offset + src_dma;
+
+		hantro_reg_write(vpu, &vp8_dec_dct_base[i],
+				 base_addr & (~DEC_8190_ALIGN_MASK));
+
+		hantro_reg_write(vpu, &vp8_dec_dct_start_bits[i],
+				 (byte_offset & DEC_8190_ALIGN_MASK) * 8);
+
+		count += hdr->dct_part_sizes[i];
+	}
+}
+
+/*
+ * prediction filter taps
+ * normal 6-tap filters
+ */
+static void cfg_tap(struct hantro_ctx *ctx,
+		    const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+	struct hantro_dev *vpu = ctx->dev;
+	int i, j;
+
+	if ((hdr->version & 0x03) != 0)
+		return; /* Tap filter not used. */
+
+	for (i = 0; i < 8; i++) {
+		for (j = 0; j < 6; j++) {
+			if (vp8_dec_pred_bc_tap[i][j].base != 0)
+				hantro_reg_write(vpu,
+						 &vp8_dec_pred_bc_tap[i][j],
+						 vp8_dec_mc_filter[i][j]);
+		}
+	}
+}
+
+static void cfg_ref(struct hantro_ctx *ctx,
+		    const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+	struct hantro_dev *vpu = ctx->dev;
+	struct vb2_v4l2_buffer *vb2_dst;
+	struct vb2_queue *cap_q;
+	dma_addr_t ref;
+
+	cap_q = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+	vb2_dst = hantro_get_dst_buf(ctx);
+
+	ref = hantro_get_ref(cap_q, hdr->last_frame_ts);
+	if (!ref)
+		ref = vb2_dma_contig_plane_dma_addr(&vb2_dst->vb2_buf, 0);
+	vdpu_write_relaxed(vpu, ref, VDPU_REG_VP8_ADDR_REF0);
+
+	ref = hantro_get_ref(cap_q, hdr->golden_frame_ts);
+	WARN_ON(!ref && hdr->golden_frame_ts);
+	if (!ref)
+		ref = vb2_dma_contig_plane_dma_addr(&vb2_dst->vb2_buf, 0);
+	if (hdr->flags & V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_GOLDEN)
+		ref |= VDPU_REG_VP8_GREF_SIGN_BIAS;
+	vdpu_write_relaxed(vpu, ref, VDPU_REG_VP8_ADDR_REF2_5(2));
+
+	ref = hantro_get_ref(cap_q, hdr->alt_frame_ts);
+	WARN_ON(!ref && hdr->alt_frame_ts);
+	if (!ref)
+		ref = vb2_dma_contig_plane_dma_addr(&vb2_dst->vb2_buf, 0);
+	if (hdr->flags & V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_ALT)
+		ref |= VDPU_REG_VP8_AREF_SIGN_BIAS;
+	vdpu_write_relaxed(vpu, ref, VDPU_REG_VP8_ADDR_REF2_5(3));
+}
+
+static void cfg_buffers(struct hantro_ctx *ctx,
+			const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+	const struct v4l2_vp8_segment_header *seg = &hdr->segment_header;
+	struct hantro_dev *vpu = ctx->dev;
+	struct vb2_v4l2_buffer *vb2_dst;
+	dma_addr_t dst_dma;
+	u32 reg;
+
+	vb2_dst = hantro_get_dst_buf(ctx);
+
+	/* Set probability table buffer address */
+	vdpu_write_relaxed(vpu, ctx->vp8_dec.prob_tbl.dma,
+			   VDPU_REG_ADDR_QTABLE);
+
+	/* Set segment map address */
+	reg = VDPU_REG_FWD_PIC1_SEGMENT_BASE(ctx->vp8_dec.segment_map.dma);
+	if (seg->flags & V4L2_VP8_SEGMENT_HEADER_FLAG_ENABLED) {
+		reg |= VDPU_REG_FWD_PIC1_SEGMENT_E;
+		if (seg->flags & V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_MAP)
+			reg |= VDPU_REG_FWD_PIC1_SEGMENT_UPD_E;
+	}
+	vdpu_write_relaxed(vpu, reg, VDPU_REG_VP8_SEGMENT_VAL);
+
+	/* set output frame buffer address */
+	dst_dma = vb2_dma_contig_plane_dma_addr(&vb2_dst->vb2_buf, 0);
+	vdpu_write_relaxed(vpu, dst_dma, VDPU_REG_ADDR_DST);
+}
+
+void rk3399_vpu_vp8_dec_run(struct hantro_ctx *ctx)
+{
+	const struct v4l2_ctrl_vp8_frame_header *hdr;
+	struct hantro_dev *vpu = ctx->dev;
+	size_t height = ctx->dst_fmt.height;
+	size_t width = ctx->dst_fmt.width;
+	u32 mb_width, mb_height;
+	u32 reg;
+
+	hantro_prepare_run(ctx);
+
+	hdr = hantro_get_ctrl(ctx, V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER);
+	if (WARN_ON(!hdr))
+		return;
+
+	/* Reset segment_map buffer in keyframe */
+	if (VP8_FRAME_IS_KEY_FRAME(hdr) && ctx->vp8_dec.segment_map.cpu)
+		memset(ctx->vp8_dec.segment_map.cpu, 0,
+		       ctx->vp8_dec.segment_map.size);
+
+	hantro_vp8_prob_update(ctx, hdr);
+
+	/*
+	 * Extensive testing shows that the hardware does not properly
+	 * clear the internal state from previous a decoding run. This
+	 * causes corruption in decoded frames for multi-instance use cases.
+	 * A soft reset before programming the registers has been found
+	 * to resolve those problems.
+	 */
+	ctx->codec_ops->reset(ctx);
+
+	reg = VDPU_REG_CONFIG_DEC_TIMEOUT_E
+		| VDPU_REG_CONFIG_DEC_CLK_GATE_E;
+	if (!VP8_FRAME_IS_KEY_FRAME(hdr))
+		reg |= VDPU_REG_DEC_CTRL0_PIC_INTER_E;
+	vdpu_write_relaxed(vpu, reg, VDPU_REG_EN_FLAGS);
+
+	reg = VDPU_REG_CONFIG_DEC_STRENDIAN_E
+		| VDPU_REG_CONFIG_DEC_INSWAP32_E
+		| VDPU_REG_CONFIG_DEC_STRSWAP32_E
+		| VDPU_REG_CONFIG_DEC_OUTSWAP32_E
+		| VDPU_REG_CONFIG_DEC_IN_ENDIAN
+		| VDPU_REG_CONFIG_DEC_OUT_ENDIAN;
+	vdpu_write_relaxed(vpu, reg, VDPU_REG_DATA_ENDIAN);
+
+	reg = VDPU_REG_CONFIG_DEC_MAX_BURST(16);
+	vdpu_write_relaxed(vpu, reg, VDPU_REG_AXI_CTRL);
+
+	reg = VDPU_REG_DEC_CTRL0_DEC_MODE(10);
+	vdpu_write_relaxed(vpu, reg, VDPU_REG_DEC_FORMAT);
+
+	if (!(hdr->flags & V4L2_VP8_FRAME_HEADER_FLAG_MB_NO_SKIP_COEFF))
+		hantro_reg_write(vpu, &vp8_dec_skip_mode, 1);
+	if (hdr->lf_header.level == 0)
+		hantro_reg_write(vpu, &vp8_dec_filter_disable, 1);
+
+	/* Frame dimensions */
+	mb_width = VP8_MB_WIDTH(width);
+	mb_height = VP8_MB_HEIGHT(height);
+
+	hantro_reg_write(vpu, &vp8_dec_mb_width, mb_width);
+	hantro_reg_write(vpu, &vp8_dec_mb_height, mb_height);
+	hantro_reg_write(vpu, &vp8_dec_mb_width_ext, mb_width >> 9);
+	hantro_reg_write(vpu, &vp8_dec_mb_height_ext, mb_height >> 8);
+
+	/* Boolean decoder */
+	hantro_reg_write(vpu, &vp8_dec_bool_range, hdr->coder_state.range);
+	hantro_reg_write(vpu, &vp8_dec_bool_value, hdr->coder_state.value);
+
+	reg = vdpu_read(vpu, VDPU_REG_VP8_DCT_START_BIT);
+	if (hdr->version != 3)
+		reg |= VDPU_REG_DEC_CTRL4_VC1_HEIGHT_EXT;
+	if (hdr->version & 0x3)
+		reg |= VDPU_REG_DEC_CTRL4_BILIN_MC_E;
+	vdpu_write_relaxed(vpu, reg, VDPU_REG_VP8_DCT_START_BIT);
+
+	cfg_lf(ctx, hdr);
+	cfg_qp(ctx, hdr);
+	cfg_parts(ctx, hdr);
+	cfg_tap(ctx, hdr);
+	cfg_ref(ctx, hdr);
+	cfg_buffers(ctx, hdr);
+
+	hantro_finish_run(ctx);
+
+	hantro_reg_write(vpu, &vp8_dec_start_dec, 1);
+}
-- 
2.22.0

Re: [PATCH v2 1/7] media: hantro: Set DMA max segment size

[Philipp Zabel]

On Thu, 2019-07-25 at 11:17 -0300, Ezequiel Garcia wrote:
> From: Francois Buergisser <fbuergisser@chromium.org>
> 
> The Hantro codec is typically used in platforms with an IOMMU,
> so we need to set a proper DMA segment size.

... to make sure the DMA-mapping subsystem produces contiguous mappings?

> Devices without an
> IOMMU will still fallback to default 64KiB segments.

I don't understand this comment. The default max_seg_size may be 64 KiB,
but if we are always setting it to DMA_BUT_MASK(32), there is no falling
back.

> Cc: stable@vger.kernel.org
> Fixes: 775fec69008d3 ("media: add Rockchip VPU JPEG encoder driver")
> Signed-off-by: Francois Buergisser <fbuergisser@chromium.org>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/staging/media/hantro/hantro_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index b71a06e9159e..4eae1dbb1ac8 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -731,6 +731,7 @@ static int hantro_probe(struct platform_device *pdev)
>  		dev_err(vpu->dev, "Could not set DMA coherent mask.\n");
>  		return ret;
>  	}
> +	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));

This should be complemented by a call to
vb2_dma_contig_clear_max_seg_size() in _remove,
to avoid leaking dev->dma_parms.

>  
>  	for (i = 0; i < vpu->variant->num_irqs; i++) {
>  		const char *irq_name = vpu->variant->irqs[i].name;

regards
Philipp

Re: [PATCH v2 1/7] media: hantro: Set DMA max segment size

[Tomasz Figa]

On Fri, Jul 26, 2019 at 12:36 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Thu, 2019-07-25 at 11:17 -0300, Ezequiel Garcia wrote:
> > From: Francois Buergisser <fbuergisser@chromium.org>
> >
> > The Hantro codec is typically used in platforms with an IOMMU,
> > so we need to set a proper DMA segment size.
>
> ... to make sure the DMA-mapping subsystem produces contiguous mappings?
>
> > Devices without an
> > IOMMU will still fallback to default 64KiB segments.
>
> I don't understand this comment. The default max_seg_size may be 64 KiB,
> but if we are always setting it to DMA_BUT_MASK(32), there is no falling
> back.
>

DMA mask and segment size are two completely orthogonal parameters.
Please check https://elixir.bootlin.com/linux/v5.3-rc1/source/drivers/iommu/dma-iommu.c#L740
for an example of how the latter is used.

Best regards,
Tomasz

Re: [PATCH v2 6/7] media: hantro: Move VP8 common code

[Mauro Carvalho Chehab]

Em Thu, 25 Jul 2019 11:17:55 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:

> In order to introduce support for RK3399 VP8 decoding,
> move some common VP8 code. This will be reused by
> the RK3399 implementation, reducing code duplication.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  .../staging/media/hantro/hantro_g1_vp8_dec.c    | 17 -----------------
>  drivers/staging/media/hantro/hantro_hw.h        |  4 ++++
>  drivers/staging/media/hantro/hantro_vp8.c       | 15 +++++++++++++++
>  3 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> index cd1fbd3a0d5f..181e2f76d8cb 100644
> --- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> +++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> @@ -16,8 +16,6 @@
>  #include "hantro.h"
>  #include "hantro_g1_regs.h"
>  
> -#define DEC_8190_ALIGN_MASK	0x07U
> -
>  /* DCT partition base address regs */
>  static const struct hantro_reg vp8_dec_dct_base[8] = {
>  	{ G1_REG_ADDR_STR, 0, 0xffffffff },
> @@ -131,21 +129,6 @@ static const struct hantro_reg vp8_dec_pred_bc_tap[8][4] = {
>  	},
>  };
>  
> -/*
> - * filter taps taken to 7-bit precision,
> - * reference RFC6386#Page-16, filters[8][6]
> - */
> -static const u32 vp8_dec_mc_filter[8][6] = {
> -	{ 0, 0, 128, 0, 0, 0 },
> -	{ 0, -6, 123, 12, -1, 0 },
> -	{ 2, -11, 108, 36, -8, 1 },
> -	{ 0, -9, 93, 50, -6, 0 },
> -	{ 3, -16, 77, 77, -16, 3 },
> -	{ 0, -6, 50, 93, -9, 0 },
> -	{ 1, -8, 36, 108, -11, 2 },
> -	{ 0, -1, 12, 123, -6, 0 }
> -};
> -
>  /*
>   * Set loop filters
>   */
> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> index 34ef24e3a9ef..185e27d47e47 100644
> --- a/drivers/staging/media/hantro/hantro_hw.h
> +++ b/drivers/staging/media/hantro/hantro_hw.h
> @@ -15,6 +15,8 @@
>  #include <media/vp8-ctrls.h>
>  #include <media/videobuf2-core.h>
>  
> +#define DEC_8190_ALIGN_MASK	0x07U
> +
>  struct hantro_dev;
>  struct hantro_ctx;
>  struct hantro_buf;
> @@ -93,6 +95,8 @@ extern const struct hantro_variant rk3399_vpu_variant;
>  extern const struct hantro_variant rk3328_vpu_variant;
>  extern const struct hantro_variant rk3288_vpu_variant;
>  
> +extern const u32 vp8_dec_mc_filter[8][6];

Please don't do that, as a symbol like that can easily cause
namespace clashes in the future. For all exported symbols,
please prepend the driver name, like:

	hantro_vp8_dec_mc_filter

Regards,
Mauro


> +
>  void hantro_watchdog(struct work_struct *work);
>  void hantro_run(struct hantro_ctx *ctx);
>  void hantro_irq_done(struct hantro_dev *vpu, unsigned int bytesused,
> diff --git a/drivers/staging/media/hantro/hantro_vp8.c b/drivers/staging/media/hantro/hantro_vp8.c
> index 66c45335d871..be5cb01d1309 100644
> --- a/drivers/staging/media/hantro/hantro_vp8.c
> +++ b/drivers/staging/media/hantro/hantro_vp8.c
> @@ -31,6 +31,21 @@ struct vp8_prob_tbl_packed {
>  	u8 padding3[96];
>  };
>  
> +/*
> + * filter taps taken to 7-bit precision,
> + * reference RFC6386#Page-16, filters[8][6]
> + */
> +const u32 vp8_dec_mc_filter[8][6] = {
> +	{ 0, 0, 128, 0, 0, 0 },
> +	{ 0, -6, 123, 12, -1, 0 },
> +	{ 2, -11, 108, 36, -8, 1 },
> +	{ 0, -9, 93, 50, -6, 0 },
> +	{ 3, -16, 77, 77, -16, 3 },
> +	{ 0, -6, 50, 93, -9, 0 },
> +	{ 1, -8, 36, 108, -11, 2 },
> +	{ 0, -1, 12, 123, -6, 0 }
> +};
> +
>  void hantro_vp8_prob_update(struct hantro_ctx *ctx,
>  			    const struct v4l2_ctrl_vp8_frame_header *hdr)
>  {



Thanks,
Mauro

Re: [PATCH v2 6/7] media: hantro: Move VP8 common code

[Ezequiel Garcia]

On Thu, 2019-07-25 at 13:22 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 25 Jul 2019 11:17:55 -0300
> Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> 
> > In order to introduce support for RK3399 VP8 decoding,
> > move some common VP8 code. This will be reused by
> > the RK3399 implementation, reducing code duplication.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  .../staging/media/hantro/hantro_g1_vp8_dec.c    | 17 -----------------
> >  drivers/staging/media/hantro/hantro_hw.h        |  4 ++++
> >  drivers/staging/media/hantro/hantro_vp8.c       | 15 +++++++++++++++
> >  3 files changed, 19 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> > index cd1fbd3a0d5f..181e2f76d8cb 100644
> > --- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> > +++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> > @@ -16,8 +16,6 @@
> >  #include "hantro.h"
> >  #include "hantro_g1_regs.h"
> >  
> > -#define DEC_8190_ALIGN_MASK	0x07U
> > -
> >  /* DCT partition base address regs */
> >  static const struct hantro_reg vp8_dec_dct_base[8] = {
> >  	{ G1_REG_ADDR_STR, 0, 0xffffffff },
> > @@ -131,21 +129,6 @@ static const struct hantro_reg vp8_dec_pred_bc_tap[8][4] = {
> >  	},
> >  };
> >  
> > -/*
> > - * filter taps taken to 7-bit precision,
> > - * reference RFC6386#Page-16, filters[8][6]
> > - */
> > -static const u32 vp8_dec_mc_filter[8][6] = {
> > -	{ 0, 0, 128, 0, 0, 0 },
> > -	{ 0, -6, 123, 12, -1, 0 },
> > -	{ 2, -11, 108, 36, -8, 1 },
> > -	{ 0, -9, 93, 50, -6, 0 },
> > -	{ 3, -16, 77, 77, -16, 3 },
> > -	{ 0, -6, 50, 93, -9, 0 },
> > -	{ 1, -8, 36, 108, -11, 2 },
> > -	{ 0, -1, 12, 123, -6, 0 }
> > -};
> > -
> >  /*
> >   * Set loop filters
> >   */
> > diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> > index 34ef24e3a9ef..185e27d47e47 100644
> > --- a/drivers/staging/media/hantro/hantro_hw.h
> > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > @@ -15,6 +15,8 @@
> >  #include <media/vp8-ctrls.h>
> >  #include <media/videobuf2-core.h>
> >  
> > +#define DEC_8190_ALIGN_MASK	0x07U
> > +
> >  struct hantro_dev;
> >  struct hantro_ctx;
> >  struct hantro_buf;
> > @@ -93,6 +95,8 @@ extern const struct hantro_variant rk3399_vpu_variant;
> >  extern const struct hantro_variant rk3328_vpu_variant;
> >  extern const struct hantro_variant rk3288_vpu_variant;
> >  
> > +extern const u32 vp8_dec_mc_filter[8][6];
> 
> Please don't do that, as a symbol like that can easily cause
> namespace clashes in the future. For all exported symbols,
> please prepend the driver name, like:
> 
> 	hantro_vp8_dec_mc_filter
> 

Right. Would you be OK, with taking Hans' PR and accept a follow-up
patch fixing this?

Thanks,
Eze

Re: [PATCH v2 6/7] media: hantro: Move VP8 common code

[Mauro Carvalho Chehab]

Em Thu, 25 Jul 2019 13:30:07 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:

> On Thu, 2019-07-25 at 13:22 -0300, Mauro Carvalho Chehab wrote:
> > Em Thu, 25 Jul 2019 11:17:55 -0300
> > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> >   
> > > In order to introduce support for RK3399 VP8 decoding,
> > > move some common VP8 code. This will be reused by
> > > the RK3399 implementation, reducing code duplication.
> > > 
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > >  .../staging/media/hantro/hantro_g1_vp8_dec.c    | 17 -----------------
> > >  drivers/staging/media/hantro/hantro_hw.h        |  4 ++++
> > >  drivers/staging/media/hantro/hantro_vp8.c       | 15 +++++++++++++++
> > >  3 files changed, 19 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> > > index cd1fbd3a0d5f..181e2f76d8cb 100644
> > > --- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> > > +++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> > > @@ -16,8 +16,6 @@
> > >  #include "hantro.h"
> > >  #include "hantro_g1_regs.h"
> > >  
> > > -#define DEC_8190_ALIGN_MASK	0x07U
> > > -
> > >  /* DCT partition base address regs */
> > >  static const struct hantro_reg vp8_dec_dct_base[8] = {
> > >  	{ G1_REG_ADDR_STR, 0, 0xffffffff },
> > > @@ -131,21 +129,6 @@ static const struct hantro_reg vp8_dec_pred_bc_tap[8][4] = {
> > >  	},
> > >  };
> > >  
> > > -/*
> > > - * filter taps taken to 7-bit precision,
> > > - * reference RFC6386#Page-16, filters[8][6]
> > > - */
> > > -static const u32 vp8_dec_mc_filter[8][6] = {
> > > -	{ 0, 0, 128, 0, 0, 0 },
> > > -	{ 0, -6, 123, 12, -1, 0 },
> > > -	{ 2, -11, 108, 36, -8, 1 },
> > > -	{ 0, -9, 93, 50, -6, 0 },
> > > -	{ 3, -16, 77, 77, -16, 3 },
> > > -	{ 0, -6, 50, 93, -9, 0 },
> > > -	{ 1, -8, 36, 108, -11, 2 },
> > > -	{ 0, -1, 12, 123, -6, 0 }
> > > -};
> > > -
> > >  /*
> > >   * Set loop filters
> > >   */
> > > diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> > > index 34ef24e3a9ef..185e27d47e47 100644
> > > --- a/drivers/staging/media/hantro/hantro_hw.h
> > > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > > @@ -15,6 +15,8 @@
> > >  #include <media/vp8-ctrls.h>
> > >  #include <media/videobuf2-core.h>
> > >  
> > > +#define DEC_8190_ALIGN_MASK	0x07U
> > > +
> > >  struct hantro_dev;
> > >  struct hantro_ctx;
> > >  struct hantro_buf;
> > > @@ -93,6 +95,8 @@ extern const struct hantro_variant rk3399_vpu_variant;
> > >  extern const struct hantro_variant rk3328_vpu_variant;
> > >  extern const struct hantro_variant rk3288_vpu_variant;
> > >  
> > > +extern const u32 vp8_dec_mc_filter[8][6];  
> > 
> > Please don't do that, as a symbol like that can easily cause
> > namespace clashes in the future. For all exported symbols,
> > please prepend the driver name, like:
> > 
> > 	hantro_vp8_dec_mc_filter
> >   
> 
> Right. Would you be OK, with taking Hans' PR and accept a follow-up
> patch fixing this?

No need. I went ahead and applied a fixup. Just remember about that
next time, as we don't want to mess with Kernel export symbol
namespace.

Thanks,
Mauro

Re: [PATCH v2 6/7] media: hantro: Move VP8 common code

[Ezequiel Garcia]

On Thu, 2019-07-25 at 13:38 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 25 Jul 2019 13:30:07 -0300
> Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> 
> > On Thu, 2019-07-25 at 13:22 -0300, Mauro Carvalho Chehab wrote:
> > > Em Thu, 25 Jul 2019 11:17:55 -0300
> > > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> > >   
> > > > In order to introduce support for RK3399 VP8 decoding,
> > > > move some common VP8 code. This will be reused by
> > > > the RK3399 implementation, reducing code duplication.
> > > > 
> > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > > ---
> > > >  .../staging/media/hantro/hantro_g1_vp8_dec.c    | 17 -----------------
> > > >  drivers/staging/media/hantro/hantro_hw.h        |  4 ++++
> > > >  drivers/staging/media/hantro/hantro_vp8.c       | 15 +++++++++++++++
> > > >  3 files changed, 19 insertions(+), 17 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> > > > index cd1fbd3a0d5f..181e2f76d8cb 100644
> > > > --- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> > > > +++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> > > > @@ -16,8 +16,6 @@
> > > >  #include "hantro.h"
> > > >  #include "hantro_g1_regs.h"
> > > >  
> > > > -#define DEC_8190_ALIGN_MASK	0x07U
> > > > -
> > > >  /* DCT partition base address regs */
> > > >  static const struct hantro_reg vp8_dec_dct_base[8] = {
> > > >  	{ G1_REG_ADDR_STR, 0, 0xffffffff },
> > > > @@ -131,21 +129,6 @@ static const struct hantro_reg vp8_dec_pred_bc_tap[8][4] = {
> > > >  	},
> > > >  };
> > > >  
> > > > -/*
> > > > - * filter taps taken to 7-bit precision,
> > > > - * reference RFC6386#Page-16, filters[8][6]
> > > > - */
> > > > -static const u32 vp8_dec_mc_filter[8][6] = {
> > > > -	{ 0, 0, 128, 0, 0, 0 },
> > > > -	{ 0, -6, 123, 12, -1, 0 },
> > > > -	{ 2, -11, 108, 36, -8, 1 },
> > > > -	{ 0, -9, 93, 50, -6, 0 },
> > > > -	{ 3, -16, 77, 77, -16, 3 },
> > > > -	{ 0, -6, 50, 93, -9, 0 },
> > > > -	{ 1, -8, 36, 108, -11, 2 },
> > > > -	{ 0, -1, 12, 123, -6, 0 }
> > > > -};
> > > > -
> > > >  /*
> > > >   * Set loop filters
> > > >   */
> > > > diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> > > > index 34ef24e3a9ef..185e27d47e47 100644
> > > > --- a/drivers/staging/media/hantro/hantro_hw.h
> > > > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > > > @@ -15,6 +15,8 @@
> > > >  #include <media/vp8-ctrls.h>
> > > >  #include <media/videobuf2-core.h>
> > > >  
> > > > +#define DEC_8190_ALIGN_MASK	0x07U
> > > > +
> > > >  struct hantro_dev;
> > > >  struct hantro_ctx;
> > > >  struct hantro_buf;
> > > > @@ -93,6 +95,8 @@ extern const struct hantro_variant rk3399_vpu_variant;
> > > >  extern const struct hantro_variant rk3328_vpu_variant;
> > > >  extern const struct hantro_variant rk3288_vpu_variant;
> > > >  
> > > > +extern const u32 vp8_dec_mc_filter[8][6];  
> > > 
> > > Please don't do that, as a symbol like that can easily cause
> > > namespace clashes in the future. For all exported symbols,
> > > please prepend the driver name, like:
> > > 
> > > 	hantro_vp8_dec_mc_filter
> > >   
> > 
> > Right. Would you be OK, with taking Hans' PR and accept a follow-up
> > patch fixing this?
> 
> No need. I went ahead and applied a fixup. Just remember about that
> next time, as we don't want to mess with Kernel export symbol
> namespace.
> 

Yes, definitely makes sense.

Thanks for taking care of this,
Eze