All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: hantro: Use post processor scaling capacities
@ 2022-04-04 14:08 ` Benjamin Gaignard
  0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Gaignard @ 2022-04-04 14:08 UTC (permalink / raw)
  To: ezequiel, p.zabel, mchehab, gregkh
  Cc: linux-media, linux-rockchip, linux-staging, linux-kernel, kernel,
	Benjamin Gaignard

Hantro G2 post processor is able to down scale decoded frames
by 2, 4, or 8 factor.
Add enum_framesizes() ops to postproc_ops structure to enumerate the
possible output sizes for a given input resolution.
For G2 post-processor use fsize->index (from 0 to 3) has power of 2
diviser. As describe in v4l2 documentation return -EINVAL when scaling
down isn't possible.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
Fluster score with this patch is 77/147 like before.

 drivers/staging/media/hantro/hantro.h         |  2 +
 drivers/staging/media/hantro/hantro_g2_regs.h |  6 +++
 drivers/staging/media/hantro/hantro_hw.h      |  8 ++-
 .../staging/media/hantro/hantro_postproc.c    | 53 ++++++++++++++++++-
 drivers/staging/media/hantro/hantro_v4l2.c    | 15 +++---
 5 files changed, 72 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 357f83b86809..26308bb29adc 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -475,5 +475,7 @@ void hantro_postproc_disable(struct hantro_ctx *ctx);
 void hantro_postproc_enable(struct hantro_ctx *ctx);
 void hantro_postproc_free(struct hantro_ctx *ctx);
 int hantro_postproc_alloc(struct hantro_ctx *ctx);
+int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
+				   struct v4l2_frmsizeenum *fsize);
 
 #endif /* HANTRO_H_ */
diff --git a/drivers/staging/media/hantro/hantro_g2_regs.h b/drivers/staging/media/hantro/hantro_g2_regs.h
index b7c6f9877b9d..877d663a8181 100644
--- a/drivers/staging/media/hantro/hantro_g2_regs.h
+++ b/drivers/staging/media/hantro/hantro_g2_regs.h
@@ -290,6 +290,10 @@
 #define g2_buswidth		G2_DEC_REG(58, 8,  0x7)
 #define g2_max_burst		G2_DEC_REG(58, 0,  0xff)
 
+#define g2_down_scale_e		G2_DEC_REG(184, 7, 0x1)
+#define g2_down_scale_y		G2_DEC_REG(184, 2, 0x3)
+#define g2_down_scale_x		G2_DEC_REG(184, 0, 0x3)
+
 #define G2_REG_CONFIG				G2_SWREG(58)
 #define G2_REG_CONFIG_DEC_CLK_GATE_E		BIT(16)
 #define G2_REG_CONFIG_DEC_CLK_GATE_IDLE_E	BIT(17)
@@ -312,6 +316,8 @@
 #define G2_TILE_FILTER_ADDR		(G2_SWREG(179))
 #define G2_TILE_SAO_ADDR		(G2_SWREG(181))
 #define G2_TILE_BSD_ADDR		(G2_SWREG(183))
+#define G2_DS_DST			(G2_SWREG(186))
+#define G2_DS_DST_CHR			(G2_SWREG(188))
 
 #define g2_strm_buffer_len	G2_DEC_REG(258, 0, 0xffffffff)
 #define g2_strm_start_offset	G2_DEC_REG(259, 0, 0xffffffff)
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index ed018e293ba0..09f17a3ceb95 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -245,12 +245,16 @@ struct hantro_postproc_ctx {
 /**
  * struct hantro_postproc_ops - post-processor operations
  *
- * @enable:	Enable the post-processor block. Optional.
- * @disable:	Disable the post-processor block. Optional.
+ * @enable:		Enable the post-processor block. Optional.
+ * @disable:		Disable the post-processor block. Optional.
+ * @enum_framesizes:	Enumerate possible scaled output formats.
+ *			Returns zero if OK, a negative value in error cases.
+ *			Optional.
  */
 struct hantro_postproc_ops {
 	void (*enable)(struct hantro_ctx *ctx);
 	void (*disable)(struct hantro_ctx *ctx);
+	int (*enum_framesizes)(struct hantro_ctx *ctx, struct v4l2_frmsizeenum *fsize);
 };
 
 /**
diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
index 248abe5423f0..ab168c1c0d28 100644
--- a/drivers/staging/media/hantro/hantro_postproc.c
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -100,21 +100,58 @@ static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
 	HANTRO_PP_REG_WRITE(vpu, display_width, ctx->dst_fmt.width);
 }
 
+static int down_scale_factor(struct hantro_ctx *ctx)
+{
+	if (ctx->src_fmt.width == ctx->dst_fmt.width)
+		return 0;
+
+	return DIV_ROUND_CLOSEST(ctx->src_fmt.width, ctx->dst_fmt.width);
+}
+
 static void hantro_postproc_g2_enable(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
 	struct vb2_v4l2_buffer *dst_buf;
 	size_t chroma_offset = ctx->dst_fmt.width * ctx->dst_fmt.height;
+	int down_scale = down_scale_factor(ctx);
 	dma_addr_t dst_dma;
 
 	dst_buf = hantro_get_dst_buf(ctx);
 	dst_dma = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
 
-	hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
-	hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
+	if (down_scale) {
+		hantro_reg_write(vpu, &g2_down_scale_e, 1);
+		hantro_reg_write(vpu, &g2_down_scale_y, down_scale >> 2);
+		hantro_reg_write(vpu, &g2_down_scale_x, down_scale >> 2);
+		hantro_write_addr(vpu, G2_DS_DST, dst_dma);
+		hantro_write_addr(vpu, G2_DS_DST_CHR, dst_dma + (chroma_offset >> down_scale));
+	} else {
+		hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
+		hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
+	}
 	hantro_reg_write(vpu, &g2_out_rs_e, 1);
 }
 
+static int hantro_postproc_g2_enum_framesizes(struct hantro_ctx *ctx,
+					      struct v4l2_frmsizeenum *fsize)
+{
+	/**
+	 * G2 scaler can scale down by 0, 2, 4 or 8
+	 * use fsize->index has power of 2 diviser
+	 **/
+	if (fsize->index > 3)
+		return -EINVAL;
+
+	if (!ctx->src_fmt.width || !ctx->src_fmt.height)
+		return -EINVAL;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+	fsize->discrete.width = ctx->src_fmt.width >> fsize->index;
+	fsize->discrete.height = ctx->src_fmt.height >> fsize->index;
+
+	return 0;
+}
+
 void hantro_postproc_free(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
@@ -197,6 +234,17 @@ void hantro_postproc_enable(struct hantro_ctx *ctx)
 		vpu->variant->postproc_ops->enable(ctx);
 }
 
+int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
+				   struct v4l2_frmsizeenum *fsize)
+{
+	struct hantro_dev *vpu = ctx->dev;
+
+	if (vpu->variant->postproc_ops && vpu->variant->postproc_ops->enum_framesizes)
+		return vpu->variant->postproc_ops->enum_framesizes(ctx, fsize);
+
+	return -EINVAL;
+}
+
 const struct hantro_postproc_ops hantro_g1_postproc_ops = {
 	.enable = hantro_postproc_g1_enable,
 	.disable = hantro_postproc_g1_disable,
@@ -205,4 +253,5 @@ const struct hantro_postproc_ops hantro_g1_postproc_ops = {
 const struct hantro_postproc_ops hantro_g2_postproc_ops = {
 	.enable = hantro_postproc_g2_enable,
 	.disable = hantro_postproc_g2_disable,
+	.enum_framesizes = hantro_postproc_g2_enum_framesizes,
 };
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 67148ba346f5..8314a2efd4c8 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -116,12 +116,6 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
 	struct hantro_ctx *ctx = fh_to_ctx(priv);
 	const struct hantro_fmt *fmt;
 
-	if (fsize->index != 0) {
-		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
-			  fsize->index);
-		return -EINVAL;
-	}
-
 	fmt = hantro_find_format(ctx, fsize->pixel_format);
 	if (!fmt) {
 		vpu_debug(0, "unsupported bitstream format (%08x)\n",
@@ -129,9 +123,14 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
 		return -EINVAL;
 	}
 
-	/* This only makes sense for coded formats */
-	if (fmt->codec_mode == HANTRO_MODE_NONE)
+	/* For non-coded formats check if postprocessing scaling is possible */
+	if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
+		return hanto_postproc_enum_framesizes(ctx, fsize);
+	} else if (fsize->index != 0) {
+		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
+			  fsize->index);
 		return -EINVAL;
+	}
 
 	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
 	fsize->stepwise = fmt->frmsize;
-- 
2.32.0


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

* [PATCH] media: hantro: Use post processor scaling capacities
@ 2022-04-04 14:08 ` Benjamin Gaignard
  0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Gaignard @ 2022-04-04 14:08 UTC (permalink / raw)
  To: ezequiel, p.zabel, mchehab, gregkh
  Cc: linux-media, linux-rockchip, linux-staging, linux-kernel, kernel,
	Benjamin Gaignard

Hantro G2 post processor is able to down scale decoded frames
by 2, 4, or 8 factor.
Add enum_framesizes() ops to postproc_ops structure to enumerate the
possible output sizes for a given input resolution.
For G2 post-processor use fsize->index (from 0 to 3) has power of 2
diviser. As describe in v4l2 documentation return -EINVAL when scaling
down isn't possible.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
Fluster score with this patch is 77/147 like before.

 drivers/staging/media/hantro/hantro.h         |  2 +
 drivers/staging/media/hantro/hantro_g2_regs.h |  6 +++
 drivers/staging/media/hantro/hantro_hw.h      |  8 ++-
 .../staging/media/hantro/hantro_postproc.c    | 53 ++++++++++++++++++-
 drivers/staging/media/hantro/hantro_v4l2.c    | 15 +++---
 5 files changed, 72 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 357f83b86809..26308bb29adc 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -475,5 +475,7 @@ void hantro_postproc_disable(struct hantro_ctx *ctx);
 void hantro_postproc_enable(struct hantro_ctx *ctx);
 void hantro_postproc_free(struct hantro_ctx *ctx);
 int hantro_postproc_alloc(struct hantro_ctx *ctx);
+int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
+				   struct v4l2_frmsizeenum *fsize);
 
 #endif /* HANTRO_H_ */
diff --git a/drivers/staging/media/hantro/hantro_g2_regs.h b/drivers/staging/media/hantro/hantro_g2_regs.h
index b7c6f9877b9d..877d663a8181 100644
--- a/drivers/staging/media/hantro/hantro_g2_regs.h
+++ b/drivers/staging/media/hantro/hantro_g2_regs.h
@@ -290,6 +290,10 @@
 #define g2_buswidth		G2_DEC_REG(58, 8,  0x7)
 #define g2_max_burst		G2_DEC_REG(58, 0,  0xff)
 
+#define g2_down_scale_e		G2_DEC_REG(184, 7, 0x1)
+#define g2_down_scale_y		G2_DEC_REG(184, 2, 0x3)
+#define g2_down_scale_x		G2_DEC_REG(184, 0, 0x3)
+
 #define G2_REG_CONFIG				G2_SWREG(58)
 #define G2_REG_CONFIG_DEC_CLK_GATE_E		BIT(16)
 #define G2_REG_CONFIG_DEC_CLK_GATE_IDLE_E	BIT(17)
@@ -312,6 +316,8 @@
 #define G2_TILE_FILTER_ADDR		(G2_SWREG(179))
 #define G2_TILE_SAO_ADDR		(G2_SWREG(181))
 #define G2_TILE_BSD_ADDR		(G2_SWREG(183))
+#define G2_DS_DST			(G2_SWREG(186))
+#define G2_DS_DST_CHR			(G2_SWREG(188))
 
 #define g2_strm_buffer_len	G2_DEC_REG(258, 0, 0xffffffff)
 #define g2_strm_start_offset	G2_DEC_REG(259, 0, 0xffffffff)
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index ed018e293ba0..09f17a3ceb95 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -245,12 +245,16 @@ struct hantro_postproc_ctx {
 /**
  * struct hantro_postproc_ops - post-processor operations
  *
- * @enable:	Enable the post-processor block. Optional.
- * @disable:	Disable the post-processor block. Optional.
+ * @enable:		Enable the post-processor block. Optional.
+ * @disable:		Disable the post-processor block. Optional.
+ * @enum_framesizes:	Enumerate possible scaled output formats.
+ *			Returns zero if OK, a negative value in error cases.
+ *			Optional.
  */
 struct hantro_postproc_ops {
 	void (*enable)(struct hantro_ctx *ctx);
 	void (*disable)(struct hantro_ctx *ctx);
+	int (*enum_framesizes)(struct hantro_ctx *ctx, struct v4l2_frmsizeenum *fsize);
 };
 
 /**
diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
index 248abe5423f0..ab168c1c0d28 100644
--- a/drivers/staging/media/hantro/hantro_postproc.c
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -100,21 +100,58 @@ static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
 	HANTRO_PP_REG_WRITE(vpu, display_width, ctx->dst_fmt.width);
 }
 
+static int down_scale_factor(struct hantro_ctx *ctx)
+{
+	if (ctx->src_fmt.width == ctx->dst_fmt.width)
+		return 0;
+
+	return DIV_ROUND_CLOSEST(ctx->src_fmt.width, ctx->dst_fmt.width);
+}
+
 static void hantro_postproc_g2_enable(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
 	struct vb2_v4l2_buffer *dst_buf;
 	size_t chroma_offset = ctx->dst_fmt.width * ctx->dst_fmt.height;
+	int down_scale = down_scale_factor(ctx);
 	dma_addr_t dst_dma;
 
 	dst_buf = hantro_get_dst_buf(ctx);
 	dst_dma = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
 
-	hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
-	hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
+	if (down_scale) {
+		hantro_reg_write(vpu, &g2_down_scale_e, 1);
+		hantro_reg_write(vpu, &g2_down_scale_y, down_scale >> 2);
+		hantro_reg_write(vpu, &g2_down_scale_x, down_scale >> 2);
+		hantro_write_addr(vpu, G2_DS_DST, dst_dma);
+		hantro_write_addr(vpu, G2_DS_DST_CHR, dst_dma + (chroma_offset >> down_scale));
+	} else {
+		hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
+		hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
+	}
 	hantro_reg_write(vpu, &g2_out_rs_e, 1);
 }
 
+static int hantro_postproc_g2_enum_framesizes(struct hantro_ctx *ctx,
+					      struct v4l2_frmsizeenum *fsize)
+{
+	/**
+	 * G2 scaler can scale down by 0, 2, 4 or 8
+	 * use fsize->index has power of 2 diviser
+	 **/
+	if (fsize->index > 3)
+		return -EINVAL;
+
+	if (!ctx->src_fmt.width || !ctx->src_fmt.height)
+		return -EINVAL;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+	fsize->discrete.width = ctx->src_fmt.width >> fsize->index;
+	fsize->discrete.height = ctx->src_fmt.height >> fsize->index;
+
+	return 0;
+}
+
 void hantro_postproc_free(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
@@ -197,6 +234,17 @@ void hantro_postproc_enable(struct hantro_ctx *ctx)
 		vpu->variant->postproc_ops->enable(ctx);
 }
 
+int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
+				   struct v4l2_frmsizeenum *fsize)
+{
+	struct hantro_dev *vpu = ctx->dev;
+
+	if (vpu->variant->postproc_ops && vpu->variant->postproc_ops->enum_framesizes)
+		return vpu->variant->postproc_ops->enum_framesizes(ctx, fsize);
+
+	return -EINVAL;
+}
+
 const struct hantro_postproc_ops hantro_g1_postproc_ops = {
 	.enable = hantro_postproc_g1_enable,
 	.disable = hantro_postproc_g1_disable,
@@ -205,4 +253,5 @@ const struct hantro_postproc_ops hantro_g1_postproc_ops = {
 const struct hantro_postproc_ops hantro_g2_postproc_ops = {
 	.enable = hantro_postproc_g2_enable,
 	.disable = hantro_postproc_g2_disable,
+	.enum_framesizes = hantro_postproc_g2_enum_framesizes,
 };
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 67148ba346f5..8314a2efd4c8 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -116,12 +116,6 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
 	struct hantro_ctx *ctx = fh_to_ctx(priv);
 	const struct hantro_fmt *fmt;
 
-	if (fsize->index != 0) {
-		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
-			  fsize->index);
-		return -EINVAL;
-	}
-
 	fmt = hantro_find_format(ctx, fsize->pixel_format);
 	if (!fmt) {
 		vpu_debug(0, "unsupported bitstream format (%08x)\n",
@@ -129,9 +123,14 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
 		return -EINVAL;
 	}
 
-	/* This only makes sense for coded formats */
-	if (fmt->codec_mode == HANTRO_MODE_NONE)
+	/* For non-coded formats check if postprocessing scaling is possible */
+	if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
+		return hanto_postproc_enum_framesizes(ctx, fsize);
+	} else if (fsize->index != 0) {
+		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
+			  fsize->index);
 		return -EINVAL;
+	}
 
 	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
 	fsize->stepwise = fmt->frmsize;
-- 
2.32.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH] media: hantro: Use post processor scaling capacities
  2022-04-04 14:08 ` Benjamin Gaignard
@ 2022-04-04 15:38   ` Sebastian Fricke
  -1 siblings, 0 replies; 6+ messages in thread
From: Sebastian Fricke @ 2022-04-04 15:38 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: ezequiel, p.zabel, mchehab, gregkh, linux-media, linux-rockchip,
	linux-staging, linux-kernel, kernel

Hey Benjamin,

On 04.04.2022 16:08, Benjamin Gaignard wrote:
>Hantro G2 post processor is able to down scale decoded frames
>by 2, 4, or 8 factor.

s/by 2, 4, or 8 factor/by a factor of 2, 4, or 8/

>Add enum_framesizes() ops to postproc_ops structure to enumerate the
>possible output sizes for a given input resolution.
>For G2 post-processor use fsize->index (from 0 to 3) has power of 2

Maybe:
s/) has/), which has a/
?


>diviser. As describe in v4l2 documentation return -EINVAL when scaling

s/diviser/divisor/

or just

s/diviser/divider/

s/As describe/As described/


>down isn't possible.
>
>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

Applies and compiles without problems.

>---
>Fluster score with this patch is 77/147 like before.

These scores are for HEVC.

>
> drivers/staging/media/hantro/hantro.h         |  2 +
> drivers/staging/media/hantro/hantro_g2_regs.h |  6 +++
> drivers/staging/media/hantro/hantro_hw.h      |  8 ++-
> .../staging/media/hantro/hantro_postproc.c    | 53 ++++++++++++++++++-
> drivers/staging/media/hantro/hantro_v4l2.c    | 15 +++---
> 5 files changed, 72 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
>index 357f83b86809..26308bb29adc 100644
>--- a/drivers/staging/media/hantro/hantro.h
>+++ b/drivers/staging/media/hantro/hantro.h
>@@ -475,5 +475,7 @@ void hantro_postproc_disable(struct hantro_ctx *ctx);
> void hantro_postproc_enable(struct hantro_ctx *ctx);
> void hantro_postproc_free(struct hantro_ctx *ctx);
> int hantro_postproc_alloc(struct hantro_ctx *ctx);
>+int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
>+				   struct v4l2_frmsizeenum *fsize);
>
> #endif /* HANTRO_H_ */
>diff --git a/drivers/staging/media/hantro/hantro_g2_regs.h b/drivers/staging/media/hantro/hantro_g2_regs.h
>index b7c6f9877b9d..877d663a8181 100644
>--- a/drivers/staging/media/hantro/hantro_g2_regs.h
>+++ b/drivers/staging/media/hantro/hantro_g2_regs.h
>@@ -290,6 +290,10 @@
> #define g2_buswidth		G2_DEC_REG(58, 8,  0x7)
> #define g2_max_burst		G2_DEC_REG(58, 0,  0xff)
>
>+#define g2_down_scale_e		G2_DEC_REG(184, 7, 0x1)
>+#define g2_down_scale_y		G2_DEC_REG(184, 2, 0x3)
>+#define g2_down_scale_x		G2_DEC_REG(184, 0, 0x3)
>+
> #define G2_REG_CONFIG				G2_SWREG(58)
> #define G2_REG_CONFIG_DEC_CLK_GATE_E		BIT(16)
> #define G2_REG_CONFIG_DEC_CLK_GATE_IDLE_E	BIT(17)
>@@ -312,6 +316,8 @@
> #define G2_TILE_FILTER_ADDR		(G2_SWREG(179))
> #define G2_TILE_SAO_ADDR		(G2_SWREG(181))
> #define G2_TILE_BSD_ADDR		(G2_SWREG(183))
>+#define G2_DS_DST			(G2_SWREG(186))
>+#define G2_DS_DST_CHR			(G2_SWREG(188))
>
> #define g2_strm_buffer_len	G2_DEC_REG(258, 0, 0xffffffff)
> #define g2_strm_start_offset	G2_DEC_REG(259, 0, 0xffffffff)
>diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
>index ed018e293ba0..09f17a3ceb95 100644
>--- a/drivers/staging/media/hantro/hantro_hw.h
>+++ b/drivers/staging/media/hantro/hantro_hw.h
>@@ -245,12 +245,16 @@ struct hantro_postproc_ctx {
> /**
>  * struct hantro_postproc_ops - post-processor operations
>  *
>- * @enable:	Enable the post-processor block. Optional.
>- * @disable:	Disable the post-processor block. Optional.
>+ * @enable:		Enable the post-processor block. Optional.
>+ * @disable:		Disable the post-processor block. Optional.
>+ * @enum_framesizes:	Enumerate possible scaled output formats.
>+ *			Returns zero if OK, a negative value in error cases.
>+ *			Optional.
>  */
> struct hantro_postproc_ops {
> 	void (*enable)(struct hantro_ctx *ctx);
> 	void (*disable)(struct hantro_ctx *ctx);
>+	int (*enum_framesizes)(struct hantro_ctx *ctx, struct v4l2_frmsizeenum *fsize);
> };
>
> /**
>diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
>index 248abe5423f0..ab168c1c0d28 100644
>--- a/drivers/staging/media/hantro/hantro_postproc.c
>+++ b/drivers/staging/media/hantro/hantro_postproc.c
>@@ -100,21 +100,58 @@ static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
> 	HANTRO_PP_REG_WRITE(vpu, display_width, ctx->dst_fmt.width);
> }
>
>+static int down_scale_factor(struct hantro_ctx *ctx)
>+{
>+	if (ctx->src_fmt.width == ctx->dst_fmt.width)
>+		return 0;
>+
>+	return DIV_ROUND_CLOSEST(ctx->src_fmt.width, ctx->dst_fmt.width);
>+}
>+
> static void hantro_postproc_g2_enable(struct hantro_ctx *ctx)
> {
> 	struct hantro_dev *vpu = ctx->dev;
> 	struct vb2_v4l2_buffer *dst_buf;
> 	size_t chroma_offset = ctx->dst_fmt.width * ctx->dst_fmt.height;
>+	int down_scale = down_scale_factor(ctx);
> 	dma_addr_t dst_dma;
>
> 	dst_buf = hantro_get_dst_buf(ctx);
> 	dst_dma = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
>
>-	hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
>-	hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
>+	if (down_scale) {
>+		hantro_reg_write(vpu, &g2_down_scale_e, 1);
>+		hantro_reg_write(vpu, &g2_down_scale_y, down_scale >> 2);
>+		hantro_reg_write(vpu, &g2_down_scale_x, down_scale >> 2);
>+		hantro_write_addr(vpu, G2_DS_DST, dst_dma);
>+		hantro_write_addr(vpu, G2_DS_DST_CHR, dst_dma + (chroma_offset >> down_scale));
>+	} else {
>+		hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
>+		hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
>+	}
> 	hantro_reg_write(vpu, &g2_out_rs_e, 1);
> }
>
>+static int hantro_postproc_g2_enum_framesizes(struct hantro_ctx *ctx,
>+					      struct v4l2_frmsizeenum *fsize)
>+{
>+	/**
>+	 * G2 scaler can scale down by 0, 2, 4 or 8

s/by/by a factor of/

>+	 * use fsize->index has power of 2 diviser

This comment is hard to understand, I don't get it.
So, fsize->index has a power of 2 divider and fsize->index
cannot be greater than 3. How are these two related?

Greetings,
Sebastian

>+	 **/
>+	if (fsize->index > 3)
>+		return -EINVAL;
>+
>+	if (!ctx->src_fmt.width || !ctx->src_fmt.height)
>+		return -EINVAL;
>+
>+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
>+	fsize->discrete.width = ctx->src_fmt.width >> fsize->index;
>+	fsize->discrete.height = ctx->src_fmt.height >> fsize->index;
>+
>+	return 0;
>+}
>+
> void hantro_postproc_free(struct hantro_ctx *ctx)
> {
> 	struct hantro_dev *vpu = ctx->dev;
>@@ -197,6 +234,17 @@ void hantro_postproc_enable(struct hantro_ctx *ctx)
> 		vpu->variant->postproc_ops->enable(ctx);
> }
>
>+int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
>+				   struct v4l2_frmsizeenum *fsize)
>+{
>+	struct hantro_dev *vpu = ctx->dev;
>+
>+	if (vpu->variant->postproc_ops && vpu->variant->postproc_ops->enum_framesizes)
>+		return vpu->variant->postproc_ops->enum_framesizes(ctx, fsize);
>+
>+	return -EINVAL;
>+}
>+
> const struct hantro_postproc_ops hantro_g1_postproc_ops = {
> 	.enable = hantro_postproc_g1_enable,
> 	.disable = hantro_postproc_g1_disable,
>@@ -205,4 +253,5 @@ const struct hantro_postproc_ops hantro_g1_postproc_ops = {
> const struct hantro_postproc_ops hantro_g2_postproc_ops = {
> 	.enable = hantro_postproc_g2_enable,
> 	.disable = hantro_postproc_g2_disable,
>+	.enum_framesizes = hantro_postproc_g2_enum_framesizes,
> };
>diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
>index 67148ba346f5..8314a2efd4c8 100644
>--- a/drivers/staging/media/hantro/hantro_v4l2.c
>+++ b/drivers/staging/media/hantro/hantro_v4l2.c
>@@ -116,12 +116,6 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
> 	struct hantro_ctx *ctx = fh_to_ctx(priv);
> 	const struct hantro_fmt *fmt;
>
>-	if (fsize->index != 0) {
>-		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
>-			  fsize->index);
>-		return -EINVAL;
>-	}
>-
> 	fmt = hantro_find_format(ctx, fsize->pixel_format);
> 	if (!fmt) {
> 		vpu_debug(0, "unsupported bitstream format (%08x)\n",
>@@ -129,9 +123,14 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
> 		return -EINVAL;
> 	}
>
>-	/* This only makes sense for coded formats */
>-	if (fmt->codec_mode == HANTRO_MODE_NONE)
>+	/* For non-coded formats check if postprocessing scaling is possible */
>+	if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
>+		return hanto_postproc_enum_framesizes(ctx, fsize);
>+	} else if (fsize->index != 0) {
>+		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
>+			  fsize->index);
> 		return -EINVAL;
>+	}
>
> 	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> 	fsize->stepwise = fmt->frmsize;
>-- 
>2.32.0
>

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

* Re: [PATCH] media: hantro: Use post processor scaling capacities
@ 2022-04-04 15:38   ` Sebastian Fricke
  0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Fricke @ 2022-04-04 15:38 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: ezequiel, p.zabel, mchehab, gregkh, linux-media, linux-rockchip,
	linux-staging, linux-kernel, kernel

Hey Benjamin,

On 04.04.2022 16:08, Benjamin Gaignard wrote:
>Hantro G2 post processor is able to down scale decoded frames
>by 2, 4, or 8 factor.

s/by 2, 4, or 8 factor/by a factor of 2, 4, or 8/

>Add enum_framesizes() ops to postproc_ops structure to enumerate the
>possible output sizes for a given input resolution.
>For G2 post-processor use fsize->index (from 0 to 3) has power of 2

Maybe:
s/) has/), which has a/
?


>diviser. As describe in v4l2 documentation return -EINVAL when scaling

s/diviser/divisor/

or just

s/diviser/divider/

s/As describe/As described/


>down isn't possible.
>
>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

Applies and compiles without problems.

>---
>Fluster score with this patch is 77/147 like before.

These scores are for HEVC.

>
> drivers/staging/media/hantro/hantro.h         |  2 +
> drivers/staging/media/hantro/hantro_g2_regs.h |  6 +++
> drivers/staging/media/hantro/hantro_hw.h      |  8 ++-
> .../staging/media/hantro/hantro_postproc.c    | 53 ++++++++++++++++++-
> drivers/staging/media/hantro/hantro_v4l2.c    | 15 +++---
> 5 files changed, 72 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
>index 357f83b86809..26308bb29adc 100644
>--- a/drivers/staging/media/hantro/hantro.h
>+++ b/drivers/staging/media/hantro/hantro.h
>@@ -475,5 +475,7 @@ void hantro_postproc_disable(struct hantro_ctx *ctx);
> void hantro_postproc_enable(struct hantro_ctx *ctx);
> void hantro_postproc_free(struct hantro_ctx *ctx);
> int hantro_postproc_alloc(struct hantro_ctx *ctx);
>+int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
>+				   struct v4l2_frmsizeenum *fsize);
>
> #endif /* HANTRO_H_ */
>diff --git a/drivers/staging/media/hantro/hantro_g2_regs.h b/drivers/staging/media/hantro/hantro_g2_regs.h
>index b7c6f9877b9d..877d663a8181 100644
>--- a/drivers/staging/media/hantro/hantro_g2_regs.h
>+++ b/drivers/staging/media/hantro/hantro_g2_regs.h
>@@ -290,6 +290,10 @@
> #define g2_buswidth		G2_DEC_REG(58, 8,  0x7)
> #define g2_max_burst		G2_DEC_REG(58, 0,  0xff)
>
>+#define g2_down_scale_e		G2_DEC_REG(184, 7, 0x1)
>+#define g2_down_scale_y		G2_DEC_REG(184, 2, 0x3)
>+#define g2_down_scale_x		G2_DEC_REG(184, 0, 0x3)
>+
> #define G2_REG_CONFIG				G2_SWREG(58)
> #define G2_REG_CONFIG_DEC_CLK_GATE_E		BIT(16)
> #define G2_REG_CONFIG_DEC_CLK_GATE_IDLE_E	BIT(17)
>@@ -312,6 +316,8 @@
> #define G2_TILE_FILTER_ADDR		(G2_SWREG(179))
> #define G2_TILE_SAO_ADDR		(G2_SWREG(181))
> #define G2_TILE_BSD_ADDR		(G2_SWREG(183))
>+#define G2_DS_DST			(G2_SWREG(186))
>+#define G2_DS_DST_CHR			(G2_SWREG(188))
>
> #define g2_strm_buffer_len	G2_DEC_REG(258, 0, 0xffffffff)
> #define g2_strm_start_offset	G2_DEC_REG(259, 0, 0xffffffff)
>diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
>index ed018e293ba0..09f17a3ceb95 100644
>--- a/drivers/staging/media/hantro/hantro_hw.h
>+++ b/drivers/staging/media/hantro/hantro_hw.h
>@@ -245,12 +245,16 @@ struct hantro_postproc_ctx {
> /**
>  * struct hantro_postproc_ops - post-processor operations
>  *
>- * @enable:	Enable the post-processor block. Optional.
>- * @disable:	Disable the post-processor block. Optional.
>+ * @enable:		Enable the post-processor block. Optional.
>+ * @disable:		Disable the post-processor block. Optional.
>+ * @enum_framesizes:	Enumerate possible scaled output formats.
>+ *			Returns zero if OK, a negative value in error cases.
>+ *			Optional.
>  */
> struct hantro_postproc_ops {
> 	void (*enable)(struct hantro_ctx *ctx);
> 	void (*disable)(struct hantro_ctx *ctx);
>+	int (*enum_framesizes)(struct hantro_ctx *ctx, struct v4l2_frmsizeenum *fsize);
> };
>
> /**
>diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
>index 248abe5423f0..ab168c1c0d28 100644
>--- a/drivers/staging/media/hantro/hantro_postproc.c
>+++ b/drivers/staging/media/hantro/hantro_postproc.c
>@@ -100,21 +100,58 @@ static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
> 	HANTRO_PP_REG_WRITE(vpu, display_width, ctx->dst_fmt.width);
> }
>
>+static int down_scale_factor(struct hantro_ctx *ctx)
>+{
>+	if (ctx->src_fmt.width == ctx->dst_fmt.width)
>+		return 0;
>+
>+	return DIV_ROUND_CLOSEST(ctx->src_fmt.width, ctx->dst_fmt.width);
>+}
>+
> static void hantro_postproc_g2_enable(struct hantro_ctx *ctx)
> {
> 	struct hantro_dev *vpu = ctx->dev;
> 	struct vb2_v4l2_buffer *dst_buf;
> 	size_t chroma_offset = ctx->dst_fmt.width * ctx->dst_fmt.height;
>+	int down_scale = down_scale_factor(ctx);
> 	dma_addr_t dst_dma;
>
> 	dst_buf = hantro_get_dst_buf(ctx);
> 	dst_dma = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
>
>-	hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
>-	hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
>+	if (down_scale) {
>+		hantro_reg_write(vpu, &g2_down_scale_e, 1);
>+		hantro_reg_write(vpu, &g2_down_scale_y, down_scale >> 2);
>+		hantro_reg_write(vpu, &g2_down_scale_x, down_scale >> 2);
>+		hantro_write_addr(vpu, G2_DS_DST, dst_dma);
>+		hantro_write_addr(vpu, G2_DS_DST_CHR, dst_dma + (chroma_offset >> down_scale));
>+	} else {
>+		hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
>+		hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
>+	}
> 	hantro_reg_write(vpu, &g2_out_rs_e, 1);
> }
>
>+static int hantro_postproc_g2_enum_framesizes(struct hantro_ctx *ctx,
>+					      struct v4l2_frmsizeenum *fsize)
>+{
>+	/**
>+	 * G2 scaler can scale down by 0, 2, 4 or 8

s/by/by a factor of/

>+	 * use fsize->index has power of 2 diviser

This comment is hard to understand, I don't get it.
So, fsize->index has a power of 2 divider and fsize->index
cannot be greater than 3. How are these two related?

Greetings,
Sebastian

>+	 **/
>+	if (fsize->index > 3)
>+		return -EINVAL;
>+
>+	if (!ctx->src_fmt.width || !ctx->src_fmt.height)
>+		return -EINVAL;
>+
>+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
>+	fsize->discrete.width = ctx->src_fmt.width >> fsize->index;
>+	fsize->discrete.height = ctx->src_fmt.height >> fsize->index;
>+
>+	return 0;
>+}
>+
> void hantro_postproc_free(struct hantro_ctx *ctx)
> {
> 	struct hantro_dev *vpu = ctx->dev;
>@@ -197,6 +234,17 @@ void hantro_postproc_enable(struct hantro_ctx *ctx)
> 		vpu->variant->postproc_ops->enable(ctx);
> }
>
>+int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
>+				   struct v4l2_frmsizeenum *fsize)
>+{
>+	struct hantro_dev *vpu = ctx->dev;
>+
>+	if (vpu->variant->postproc_ops && vpu->variant->postproc_ops->enum_framesizes)
>+		return vpu->variant->postproc_ops->enum_framesizes(ctx, fsize);
>+
>+	return -EINVAL;
>+}
>+
> const struct hantro_postproc_ops hantro_g1_postproc_ops = {
> 	.enable = hantro_postproc_g1_enable,
> 	.disable = hantro_postproc_g1_disable,
>@@ -205,4 +253,5 @@ const struct hantro_postproc_ops hantro_g1_postproc_ops = {
> const struct hantro_postproc_ops hantro_g2_postproc_ops = {
> 	.enable = hantro_postproc_g2_enable,
> 	.disable = hantro_postproc_g2_disable,
>+	.enum_framesizes = hantro_postproc_g2_enum_framesizes,
> };
>diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
>index 67148ba346f5..8314a2efd4c8 100644
>--- a/drivers/staging/media/hantro/hantro_v4l2.c
>+++ b/drivers/staging/media/hantro/hantro_v4l2.c
>@@ -116,12 +116,6 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
> 	struct hantro_ctx *ctx = fh_to_ctx(priv);
> 	const struct hantro_fmt *fmt;
>
>-	if (fsize->index != 0) {
>-		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
>-			  fsize->index);
>-		return -EINVAL;
>-	}
>-
> 	fmt = hantro_find_format(ctx, fsize->pixel_format);
> 	if (!fmt) {
> 		vpu_debug(0, "unsupported bitstream format (%08x)\n",
>@@ -129,9 +123,14 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
> 		return -EINVAL;
> 	}
>
>-	/* This only makes sense for coded formats */
>-	if (fmt->codec_mode == HANTRO_MODE_NONE)
>+	/* For non-coded formats check if postprocessing scaling is possible */
>+	if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
>+		return hanto_postproc_enum_framesizes(ctx, fsize);
>+	} else if (fsize->index != 0) {
>+		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
>+			  fsize->index);
> 		return -EINVAL;
>+	}
>
> 	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> 	fsize->stepwise = fmt->frmsize;
>-- 
>2.32.0
>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH] media: hantro: Use post processor scaling capacities
  2022-04-04 14:08 ` Benjamin Gaignard
@ 2022-04-20 12:29   ` Benjamin Gaignard
  -1 siblings, 0 replies; 6+ messages in thread
From: Benjamin Gaignard @ 2022-04-20 12:29 UTC (permalink / raw)
  To: ezequiel, p.zabel, mchehab, gregkh
  Cc: linux-media, linux-rockchip, linux-staging, linux-kernel, kernel


Le 04/04/2022 à 16:08, Benjamin Gaignard a écrit :
> Hantro G2 post processor is able to down scale decoded frames
> by 2, 4, or 8 factor.
> Add enum_framesizes() ops to postproc_ops structure to enumerate the
> possible output sizes for a given input resolution.
> For G2 post-processor use fsize->index (from 0 to 3) has power of 2
> diviser. As describe in v4l2 documentation return -EINVAL when scaling
> down isn't possible.

Gentle ping for this patch.

Thanks,
Benjamin

>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
> Fluster score with this patch is 77/147 like before.
>
>   drivers/staging/media/hantro/hantro.h         |  2 +
>   drivers/staging/media/hantro/hantro_g2_regs.h |  6 +++
>   drivers/staging/media/hantro/hantro_hw.h      |  8 ++-
>   .../staging/media/hantro/hantro_postproc.c    | 53 ++++++++++++++++++-
>   drivers/staging/media/hantro/hantro_v4l2.c    | 15 +++---
>   5 files changed, 72 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
> index 357f83b86809..26308bb29adc 100644
> --- a/drivers/staging/media/hantro/hantro.h
> +++ b/drivers/staging/media/hantro/hantro.h
> @@ -475,5 +475,7 @@ void hantro_postproc_disable(struct hantro_ctx *ctx);
>   void hantro_postproc_enable(struct hantro_ctx *ctx);
>   void hantro_postproc_free(struct hantro_ctx *ctx);
>   int hantro_postproc_alloc(struct hantro_ctx *ctx);
> +int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
> +				   struct v4l2_frmsizeenum *fsize);
>   
>   #endif /* HANTRO_H_ */
> diff --git a/drivers/staging/media/hantro/hantro_g2_regs.h b/drivers/staging/media/hantro/hantro_g2_regs.h
> index b7c6f9877b9d..877d663a8181 100644
> --- a/drivers/staging/media/hantro/hantro_g2_regs.h
> +++ b/drivers/staging/media/hantro/hantro_g2_regs.h
> @@ -290,6 +290,10 @@
>   #define g2_buswidth		G2_DEC_REG(58, 8,  0x7)
>   #define g2_max_burst		G2_DEC_REG(58, 0,  0xff)
>   
> +#define g2_down_scale_e		G2_DEC_REG(184, 7, 0x1)
> +#define g2_down_scale_y		G2_DEC_REG(184, 2, 0x3)
> +#define g2_down_scale_x		G2_DEC_REG(184, 0, 0x3)
> +
>   #define G2_REG_CONFIG				G2_SWREG(58)
>   #define G2_REG_CONFIG_DEC_CLK_GATE_E		BIT(16)
>   #define G2_REG_CONFIG_DEC_CLK_GATE_IDLE_E	BIT(17)
> @@ -312,6 +316,8 @@
>   #define G2_TILE_FILTER_ADDR		(G2_SWREG(179))
>   #define G2_TILE_SAO_ADDR		(G2_SWREG(181))
>   #define G2_TILE_BSD_ADDR		(G2_SWREG(183))
> +#define G2_DS_DST			(G2_SWREG(186))
> +#define G2_DS_DST_CHR			(G2_SWREG(188))
>   
>   #define g2_strm_buffer_len	G2_DEC_REG(258, 0, 0xffffffff)
>   #define g2_strm_start_offset	G2_DEC_REG(259, 0, 0xffffffff)
> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> index ed018e293ba0..09f17a3ceb95 100644
> --- a/drivers/staging/media/hantro/hantro_hw.h
> +++ b/drivers/staging/media/hantro/hantro_hw.h
> @@ -245,12 +245,16 @@ struct hantro_postproc_ctx {
>   /**
>    * struct hantro_postproc_ops - post-processor operations
>    *
> - * @enable:	Enable the post-processor block. Optional.
> - * @disable:	Disable the post-processor block. Optional.
> + * @enable:		Enable the post-processor block. Optional.
> + * @disable:		Disable the post-processor block. Optional.
> + * @enum_framesizes:	Enumerate possible scaled output formats.
> + *			Returns zero if OK, a negative value in error cases.
> + *			Optional.
>    */
>   struct hantro_postproc_ops {
>   	void (*enable)(struct hantro_ctx *ctx);
>   	void (*disable)(struct hantro_ctx *ctx);
> +	int (*enum_framesizes)(struct hantro_ctx *ctx, struct v4l2_frmsizeenum *fsize);
>   };
>   
>   /**
> diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
> index 248abe5423f0..ab168c1c0d28 100644
> --- a/drivers/staging/media/hantro/hantro_postproc.c
> +++ b/drivers/staging/media/hantro/hantro_postproc.c
> @@ -100,21 +100,58 @@ static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
>   	HANTRO_PP_REG_WRITE(vpu, display_width, ctx->dst_fmt.width);
>   }
>   
> +static int down_scale_factor(struct hantro_ctx *ctx)
> +{
> +	if (ctx->src_fmt.width == ctx->dst_fmt.width)
> +		return 0;
> +
> +	return DIV_ROUND_CLOSEST(ctx->src_fmt.width, ctx->dst_fmt.width);
> +}
> +
>   static void hantro_postproc_g2_enable(struct hantro_ctx *ctx)
>   {
>   	struct hantro_dev *vpu = ctx->dev;
>   	struct vb2_v4l2_buffer *dst_buf;
>   	size_t chroma_offset = ctx->dst_fmt.width * ctx->dst_fmt.height;
> +	int down_scale = down_scale_factor(ctx);
>   	dma_addr_t dst_dma;
>   
>   	dst_buf = hantro_get_dst_buf(ctx);
>   	dst_dma = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
>   
> -	hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
> -	hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
> +	if (down_scale) {
> +		hantro_reg_write(vpu, &g2_down_scale_e, 1);
> +		hantro_reg_write(vpu, &g2_down_scale_y, down_scale >> 2);
> +		hantro_reg_write(vpu, &g2_down_scale_x, down_scale >> 2);
> +		hantro_write_addr(vpu, G2_DS_DST, dst_dma);
> +		hantro_write_addr(vpu, G2_DS_DST_CHR, dst_dma + (chroma_offset >> down_scale));
> +	} else {
> +		hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
> +		hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
> +	}
>   	hantro_reg_write(vpu, &g2_out_rs_e, 1);
>   }
>   
> +static int hantro_postproc_g2_enum_framesizes(struct hantro_ctx *ctx,
> +					      struct v4l2_frmsizeenum *fsize)
> +{
> +	/**
> +	 * G2 scaler can scale down by 0, 2, 4 or 8
> +	 * use fsize->index has power of 2 diviser
> +	 **/
> +	if (fsize->index > 3)
> +		return -EINVAL;
> +
> +	if (!ctx->src_fmt.width || !ctx->src_fmt.height)
> +		return -EINVAL;
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> +	fsize->discrete.width = ctx->src_fmt.width >> fsize->index;
> +	fsize->discrete.height = ctx->src_fmt.height >> fsize->index;
> +
> +	return 0;
> +}
> +
>   void hantro_postproc_free(struct hantro_ctx *ctx)
>   {
>   	struct hantro_dev *vpu = ctx->dev;
> @@ -197,6 +234,17 @@ void hantro_postproc_enable(struct hantro_ctx *ctx)
>   		vpu->variant->postproc_ops->enable(ctx);
>   }
>   
> +int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
> +				   struct v4l2_frmsizeenum *fsize)
> +{
> +	struct hantro_dev *vpu = ctx->dev;
> +
> +	if (vpu->variant->postproc_ops && vpu->variant->postproc_ops->enum_framesizes)
> +		return vpu->variant->postproc_ops->enum_framesizes(ctx, fsize);
> +
> +	return -EINVAL;
> +}
> +
>   const struct hantro_postproc_ops hantro_g1_postproc_ops = {
>   	.enable = hantro_postproc_g1_enable,
>   	.disable = hantro_postproc_g1_disable,
> @@ -205,4 +253,5 @@ const struct hantro_postproc_ops hantro_g1_postproc_ops = {
>   const struct hantro_postproc_ops hantro_g2_postproc_ops = {
>   	.enable = hantro_postproc_g2_enable,
>   	.disable = hantro_postproc_g2_disable,
> +	.enum_framesizes = hantro_postproc_g2_enum_framesizes,
>   };
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> index 67148ba346f5..8314a2efd4c8 100644
> --- a/drivers/staging/media/hantro/hantro_v4l2.c
> +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> @@ -116,12 +116,6 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
>   	struct hantro_ctx *ctx = fh_to_ctx(priv);
>   	const struct hantro_fmt *fmt;
>   
> -	if (fsize->index != 0) {
> -		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
> -			  fsize->index);
> -		return -EINVAL;
> -	}
> -
>   	fmt = hantro_find_format(ctx, fsize->pixel_format);
>   	if (!fmt) {
>   		vpu_debug(0, "unsupported bitstream format (%08x)\n",
> @@ -129,9 +123,14 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
>   		return -EINVAL;
>   	}
>   
> -	/* This only makes sense for coded formats */
> -	if (fmt->codec_mode == HANTRO_MODE_NONE)
> +	/* For non-coded formats check if postprocessing scaling is possible */
> +	if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
> +		return hanto_postproc_enum_framesizes(ctx, fsize);
> +	} else if (fsize->index != 0) {
> +		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
> +			  fsize->index);
>   		return -EINVAL;
> +	}
>   
>   	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
>   	fsize->stepwise = fmt->frmsize;

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

* Re: [PATCH] media: hantro: Use post processor scaling capacities
@ 2022-04-20 12:29   ` Benjamin Gaignard
  0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Gaignard @ 2022-04-20 12:29 UTC (permalink / raw)
  To: ezequiel, p.zabel, mchehab, gregkh
  Cc: linux-media, linux-rockchip, linux-staging, linux-kernel, kernel


Le 04/04/2022 à 16:08, Benjamin Gaignard a écrit :
> Hantro G2 post processor is able to down scale decoded frames
> by 2, 4, or 8 factor.
> Add enum_framesizes() ops to postproc_ops structure to enumerate the
> possible output sizes for a given input resolution.
> For G2 post-processor use fsize->index (from 0 to 3) has power of 2
> diviser. As describe in v4l2 documentation return -EINVAL when scaling
> down isn't possible.

Gentle ping for this patch.

Thanks,
Benjamin

>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
> Fluster score with this patch is 77/147 like before.
>
>   drivers/staging/media/hantro/hantro.h         |  2 +
>   drivers/staging/media/hantro/hantro_g2_regs.h |  6 +++
>   drivers/staging/media/hantro/hantro_hw.h      |  8 ++-
>   .../staging/media/hantro/hantro_postproc.c    | 53 ++++++++++++++++++-
>   drivers/staging/media/hantro/hantro_v4l2.c    | 15 +++---
>   5 files changed, 72 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
> index 357f83b86809..26308bb29adc 100644
> --- a/drivers/staging/media/hantro/hantro.h
> +++ b/drivers/staging/media/hantro/hantro.h
> @@ -475,5 +475,7 @@ void hantro_postproc_disable(struct hantro_ctx *ctx);
>   void hantro_postproc_enable(struct hantro_ctx *ctx);
>   void hantro_postproc_free(struct hantro_ctx *ctx);
>   int hantro_postproc_alloc(struct hantro_ctx *ctx);
> +int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
> +				   struct v4l2_frmsizeenum *fsize);
>   
>   #endif /* HANTRO_H_ */
> diff --git a/drivers/staging/media/hantro/hantro_g2_regs.h b/drivers/staging/media/hantro/hantro_g2_regs.h
> index b7c6f9877b9d..877d663a8181 100644
> --- a/drivers/staging/media/hantro/hantro_g2_regs.h
> +++ b/drivers/staging/media/hantro/hantro_g2_regs.h
> @@ -290,6 +290,10 @@
>   #define g2_buswidth		G2_DEC_REG(58, 8,  0x7)
>   #define g2_max_burst		G2_DEC_REG(58, 0,  0xff)
>   
> +#define g2_down_scale_e		G2_DEC_REG(184, 7, 0x1)
> +#define g2_down_scale_y		G2_DEC_REG(184, 2, 0x3)
> +#define g2_down_scale_x		G2_DEC_REG(184, 0, 0x3)
> +
>   #define G2_REG_CONFIG				G2_SWREG(58)
>   #define G2_REG_CONFIG_DEC_CLK_GATE_E		BIT(16)
>   #define G2_REG_CONFIG_DEC_CLK_GATE_IDLE_E	BIT(17)
> @@ -312,6 +316,8 @@
>   #define G2_TILE_FILTER_ADDR		(G2_SWREG(179))
>   #define G2_TILE_SAO_ADDR		(G2_SWREG(181))
>   #define G2_TILE_BSD_ADDR		(G2_SWREG(183))
> +#define G2_DS_DST			(G2_SWREG(186))
> +#define G2_DS_DST_CHR			(G2_SWREG(188))
>   
>   #define g2_strm_buffer_len	G2_DEC_REG(258, 0, 0xffffffff)
>   #define g2_strm_start_offset	G2_DEC_REG(259, 0, 0xffffffff)
> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> index ed018e293ba0..09f17a3ceb95 100644
> --- a/drivers/staging/media/hantro/hantro_hw.h
> +++ b/drivers/staging/media/hantro/hantro_hw.h
> @@ -245,12 +245,16 @@ struct hantro_postproc_ctx {
>   /**
>    * struct hantro_postproc_ops - post-processor operations
>    *
> - * @enable:	Enable the post-processor block. Optional.
> - * @disable:	Disable the post-processor block. Optional.
> + * @enable:		Enable the post-processor block. Optional.
> + * @disable:		Disable the post-processor block. Optional.
> + * @enum_framesizes:	Enumerate possible scaled output formats.
> + *			Returns zero if OK, a negative value in error cases.
> + *			Optional.
>    */
>   struct hantro_postproc_ops {
>   	void (*enable)(struct hantro_ctx *ctx);
>   	void (*disable)(struct hantro_ctx *ctx);
> +	int (*enum_framesizes)(struct hantro_ctx *ctx, struct v4l2_frmsizeenum *fsize);
>   };
>   
>   /**
> diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
> index 248abe5423f0..ab168c1c0d28 100644
> --- a/drivers/staging/media/hantro/hantro_postproc.c
> +++ b/drivers/staging/media/hantro/hantro_postproc.c
> @@ -100,21 +100,58 @@ static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
>   	HANTRO_PP_REG_WRITE(vpu, display_width, ctx->dst_fmt.width);
>   }
>   
> +static int down_scale_factor(struct hantro_ctx *ctx)
> +{
> +	if (ctx->src_fmt.width == ctx->dst_fmt.width)
> +		return 0;
> +
> +	return DIV_ROUND_CLOSEST(ctx->src_fmt.width, ctx->dst_fmt.width);
> +}
> +
>   static void hantro_postproc_g2_enable(struct hantro_ctx *ctx)
>   {
>   	struct hantro_dev *vpu = ctx->dev;
>   	struct vb2_v4l2_buffer *dst_buf;
>   	size_t chroma_offset = ctx->dst_fmt.width * ctx->dst_fmt.height;
> +	int down_scale = down_scale_factor(ctx);
>   	dma_addr_t dst_dma;
>   
>   	dst_buf = hantro_get_dst_buf(ctx);
>   	dst_dma = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
>   
> -	hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
> -	hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
> +	if (down_scale) {
> +		hantro_reg_write(vpu, &g2_down_scale_e, 1);
> +		hantro_reg_write(vpu, &g2_down_scale_y, down_scale >> 2);
> +		hantro_reg_write(vpu, &g2_down_scale_x, down_scale >> 2);
> +		hantro_write_addr(vpu, G2_DS_DST, dst_dma);
> +		hantro_write_addr(vpu, G2_DS_DST_CHR, dst_dma + (chroma_offset >> down_scale));
> +	} else {
> +		hantro_write_addr(vpu, G2_RS_OUT_LUMA_ADDR, dst_dma);
> +		hantro_write_addr(vpu, G2_RS_OUT_CHROMA_ADDR, dst_dma + chroma_offset);
> +	}
>   	hantro_reg_write(vpu, &g2_out_rs_e, 1);
>   }
>   
> +static int hantro_postproc_g2_enum_framesizes(struct hantro_ctx *ctx,
> +					      struct v4l2_frmsizeenum *fsize)
> +{
> +	/**
> +	 * G2 scaler can scale down by 0, 2, 4 or 8
> +	 * use fsize->index has power of 2 diviser
> +	 **/
> +	if (fsize->index > 3)
> +		return -EINVAL;
> +
> +	if (!ctx->src_fmt.width || !ctx->src_fmt.height)
> +		return -EINVAL;
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> +	fsize->discrete.width = ctx->src_fmt.width >> fsize->index;
> +	fsize->discrete.height = ctx->src_fmt.height >> fsize->index;
> +
> +	return 0;
> +}
> +
>   void hantro_postproc_free(struct hantro_ctx *ctx)
>   {
>   	struct hantro_dev *vpu = ctx->dev;
> @@ -197,6 +234,17 @@ void hantro_postproc_enable(struct hantro_ctx *ctx)
>   		vpu->variant->postproc_ops->enable(ctx);
>   }
>   
> +int hanto_postproc_enum_framesizes(struct hantro_ctx *ctx,
> +				   struct v4l2_frmsizeenum *fsize)
> +{
> +	struct hantro_dev *vpu = ctx->dev;
> +
> +	if (vpu->variant->postproc_ops && vpu->variant->postproc_ops->enum_framesizes)
> +		return vpu->variant->postproc_ops->enum_framesizes(ctx, fsize);
> +
> +	return -EINVAL;
> +}
> +
>   const struct hantro_postproc_ops hantro_g1_postproc_ops = {
>   	.enable = hantro_postproc_g1_enable,
>   	.disable = hantro_postproc_g1_disable,
> @@ -205,4 +253,5 @@ const struct hantro_postproc_ops hantro_g1_postproc_ops = {
>   const struct hantro_postproc_ops hantro_g2_postproc_ops = {
>   	.enable = hantro_postproc_g2_enable,
>   	.disable = hantro_postproc_g2_disable,
> +	.enum_framesizes = hantro_postproc_g2_enum_framesizes,
>   };
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> index 67148ba346f5..8314a2efd4c8 100644
> --- a/drivers/staging/media/hantro/hantro_v4l2.c
> +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> @@ -116,12 +116,6 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
>   	struct hantro_ctx *ctx = fh_to_ctx(priv);
>   	const struct hantro_fmt *fmt;
>   
> -	if (fsize->index != 0) {
> -		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
> -			  fsize->index);
> -		return -EINVAL;
> -	}
> -
>   	fmt = hantro_find_format(ctx, fsize->pixel_format);
>   	if (!fmt) {
>   		vpu_debug(0, "unsupported bitstream format (%08x)\n",
> @@ -129,9 +123,14 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
>   		return -EINVAL;
>   	}
>   
> -	/* This only makes sense for coded formats */
> -	if (fmt->codec_mode == HANTRO_MODE_NONE)
> +	/* For non-coded formats check if postprocessing scaling is possible */
> +	if (fmt->codec_mode == HANTRO_MODE_NONE && hantro_needs_postproc(ctx, fmt)) {
> +		return hanto_postproc_enum_framesizes(ctx, fsize);
> +	} else if (fsize->index != 0) {
> +		vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
> +			  fsize->index);
>   		return -EINVAL;
> +	}
>   
>   	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
>   	fsize->stepwise = fmt->frmsize;

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

end of thread, other threads:[~2022-04-20 12:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04 14:08 [PATCH] media: hantro: Use post processor scaling capacities Benjamin Gaignard
2022-04-04 14:08 ` Benjamin Gaignard
2022-04-04 15:38 ` Sebastian Fricke
2022-04-04 15:38   ` Sebastian Fricke
2022-04-20 12:29 ` Benjamin Gaignard
2022-04-20 12:29   ` Benjamin Gaignard

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.