All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support
@ 2019-07-25 14:17 Ezequiel Garcia
  2019-07-25 14:17 ` [PATCH v2 1/7] media: hantro: Set DMA max segment size Ezequiel Garcia
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 14:17 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
	Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser, linux-kernel,
	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


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

* [PATCH v2 1/7] media: hantro: Set DMA max segment size
  2019-07-25 14:17 [PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support Ezequiel Garcia
@ 2019-07-25 14:17 ` Ezequiel Garcia
  2019-07-25 15:36   ` Philipp Zabel
  2019-07-25 14:17 ` [PATCH v2 2/7] media: hantro: Simplify the controls creation logic Ezequiel Garcia
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 14:17 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
	Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser, linux-kernel,
	stable, 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


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

* [PATCH v2 2/7] media: hantro: Simplify the controls creation logic
  2019-07-25 14:17 [PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support Ezequiel Garcia
  2019-07-25 14:17 ` [PATCH v2 1/7] media: hantro: Set DMA max segment size Ezequiel Garcia
@ 2019-07-25 14:17 ` Ezequiel Garcia
  2019-07-25 14:17 ` [PATCH v2 3/7] media: hantro: Constify the control array Ezequiel Garcia
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 14:17 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
	Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser, linux-kernel,
	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


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

* [PATCH v2 3/7] media: hantro: Constify the control array
  2019-07-25 14:17 [PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support Ezequiel Garcia
  2019-07-25 14:17 ` [PATCH v2 1/7] media: hantro: Set DMA max segment size Ezequiel Garcia
  2019-07-25 14:17 ` [PATCH v2 2/7] media: hantro: Simplify the controls creation logic Ezequiel Garcia
@ 2019-07-25 14:17 ` Ezequiel Garcia
  2019-07-25 14:17 ` [PATCH v2 4/7] media: hantro: Add hantro_get_{src,dst}_buf() helpers Ezequiel Garcia
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 14:17 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
	Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser, linux-kernel,
	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


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

* [PATCH v2 4/7] media: hantro: Add hantro_get_{src,dst}_buf() helpers
  2019-07-25 14:17 [PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support Ezequiel Garcia
                   ` (2 preceding siblings ...)
  2019-07-25 14:17 ` [PATCH v2 3/7] media: hantro: Constify the control array Ezequiel Garcia
@ 2019-07-25 14:17 ` Ezequiel Garcia
  2019-07-25 14:17 ` [PATCH v2 5/7] media: hantro: Add helpers to prepare/finish a run Ezequiel Garcia
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 14:17 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
	Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser, linux-kernel,
	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


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

* [PATCH v2 5/7] media: hantro: Add helpers to prepare/finish a run
  2019-07-25 14:17 [PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support Ezequiel Garcia
                   ` (3 preceding siblings ...)
  2019-07-25 14:17 ` [PATCH v2 4/7] media: hantro: Add hantro_get_{src,dst}_buf() helpers Ezequiel Garcia
@ 2019-07-25 14:17 ` Ezequiel Garcia
  2019-07-25 14:17   ` Ezequiel Garcia
  2019-07-25 14:17 ` [PATCH v2 7/7] media: hantro: Support RK3399 VP8 decoding Ezequiel Garcia
  6 siblings, 0 replies; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 14:17 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
	Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser, linux-kernel,
	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


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

* [PATCH v2 6/7] media: hantro: Move VP8 common code
@ 2019-07-25 14:17   ` Ezequiel Garcia
  0 siblings, 0 replies; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 14:17 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
	Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser, linux-kernel,
	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


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

* [PATCH v2 6/7] media: hantro: Move VP8 common code
@ 2019-07-25 14:17   ` Ezequiel Garcia
  0 siblings, 0 replies; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 14:17 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil
  Cc: fbuergisser-F7+t8E8rja9g9hUCZPvPmw, Nicolas Dufresne,
	Heiko Stuebner, Alexandre Courbot, Jonas Karlman,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tomasz Figa,
	Paul Kocialkowski,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Boris Brezillon,
	Philipp Zabel, kernel-ZGY8ohtN/8qB+jHODAdFcQ, 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-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 .../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

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

* [PATCH v2 7/7] media: hantro: Support RK3399 VP8 decoding
  2019-07-25 14:17 [PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support Ezequiel Garcia
                   ` (5 preceding siblings ...)
  2019-07-25 14:17   ` Ezequiel Garcia
@ 2019-07-25 14:17 ` Ezequiel Garcia
  6 siblings, 0 replies; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 14:17 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
	Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser, linux-kernel,
	Jeffy Chen, 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


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

* Re: [PATCH v2 1/7] media: hantro: Set DMA max segment size
  2019-07-25 14:17 ` [PATCH v2 1/7] media: hantro: Set DMA max segment size Ezequiel Garcia
@ 2019-07-25 15:36   ` Philipp Zabel
  2019-07-25 15:41     ` Tomasz Figa
  0 siblings, 1 reply; 15+ messages in thread
From: Philipp Zabel @ 2019-07-25 15:36 UTC (permalink / raw)
  To: Ezequiel Garcia, linux-media, Hans Verkuil
  Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
	Heiko Stuebner, Jonas Karlman, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser, linux-kernel,
	stable

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

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

* Re: [PATCH v2 1/7] media: hantro: Set DMA max segment size
  2019-07-25 15:36   ` Philipp Zabel
@ 2019-07-25 15:41     ` Tomasz Figa
  0 siblings, 0 replies; 15+ messages in thread
From: Tomasz Figa @ 2019-07-25 15:41 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Ezequiel Garcia, Linux Media Mailing List, Hans Verkuil, kernel,
	Nicolas Dufresne, open list:ARM/Rockchip SoC...,
	Heiko Stuebner, Jonas Karlman, Boris Brezillon,
	Paul Kocialkowski, Alexandre Courbot, fbuergisser,
	Linux Kernel Mailing List, stable

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

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

* Re: [PATCH v2 6/7] media: hantro: Move VP8 common code
  2019-07-25 14:17   ` Ezequiel Garcia
  (?)
@ 2019-07-25 16:22   ` Mauro Carvalho Chehab
  2019-07-25 16:30     ` Ezequiel Garcia
  -1 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2019-07-25 16:22 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-media, Hans Verkuil, kernel, Nicolas Dufresne, Tomasz Figa,
	linux-rockchip, Heiko Stuebner, Jonas Karlman, Philipp Zabel,
	Boris Brezillon, Paul Kocialkowski, Alexandre Courbot,
	fbuergisser, linux-kernel

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

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

* Re: [PATCH v2 6/7] media: hantro: Move VP8 common code
  2019-07-25 16:22   ` Mauro Carvalho Chehab
@ 2019-07-25 16:30     ` Ezequiel Garcia
  2019-07-25 16:38       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 16:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, Hans Verkuil, kernel, Nicolas Dufresne, Tomasz Figa,
	linux-rockchip, Heiko Stuebner, Jonas Karlman, Philipp Zabel,
	Boris Brezillon, Paul Kocialkowski, Alexandre Courbot,
	fbuergisser, linux-kernel

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


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

* Re: [PATCH v2 6/7] media: hantro: Move VP8 common code
  2019-07-25 16:30     ` Ezequiel Garcia
@ 2019-07-25 16:38       ` Mauro Carvalho Chehab
  2019-07-25 17:25         ` Ezequiel Garcia
  0 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2019-07-25 16:38 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-media, Hans Verkuil, kernel, Nicolas Dufresne, Tomasz Figa,
	linux-rockchip, Heiko Stuebner, Jonas Karlman, Philipp Zabel,
	Boris Brezillon, Paul Kocialkowski, Alexandre Courbot,
	fbuergisser, linux-kernel

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

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

* Re: [PATCH v2 6/7] media: hantro: Move VP8 common code
  2019-07-25 16:38       ` Mauro Carvalho Chehab
@ 2019-07-25 17:25         ` Ezequiel Garcia
  0 siblings, 0 replies; 15+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 17:25 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, Hans Verkuil, kernel, Nicolas Dufresne, Tomasz Figa,
	linux-rockchip, Heiko Stuebner, Jonas Karlman, Philipp Zabel,
	Boris Brezillon, Paul Kocialkowski, Alexandre Courbot,
	fbuergisser, linux-kernel

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


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

end of thread, other threads:[~2019-07-25 17:25 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-25 14:17 [PATCH v2 0/7] hantro: Add RK3399 VP8 decoding support Ezequiel Garcia
2019-07-25 14:17 ` [PATCH v2 1/7] media: hantro: Set DMA max segment size Ezequiel Garcia
2019-07-25 15:36   ` Philipp Zabel
2019-07-25 15:41     ` Tomasz Figa
2019-07-25 14:17 ` [PATCH v2 2/7] media: hantro: Simplify the controls creation logic Ezequiel Garcia
2019-07-25 14:17 ` [PATCH v2 3/7] media: hantro: Constify the control array Ezequiel Garcia
2019-07-25 14:17 ` [PATCH v2 4/7] media: hantro: Add hantro_get_{src,dst}_buf() helpers Ezequiel Garcia
2019-07-25 14:17 ` [PATCH v2 5/7] media: hantro: Add helpers to prepare/finish a run Ezequiel Garcia
2019-07-25 14:17 ` [PATCH v2 6/7] media: hantro: Move VP8 common code Ezequiel Garcia
2019-07-25 14:17   ` Ezequiel Garcia
2019-07-25 16:22   ` Mauro Carvalho Chehab
2019-07-25 16:30     ` Ezequiel Garcia
2019-07-25 16:38       ` Mauro Carvalho Chehab
2019-07-25 17:25         ` Ezequiel Garcia
2019-07-25 14:17 ` [PATCH v2 7/7] media: hantro: Support RK3399 VP8 decoding Ezequiel Garcia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.