[PATCH 00/10] staging: media: cedrus: assorted fixes and VP8 4K support

[PATCH 00/10] staging: media: cedrus: assorted fixes and VP8 4K support

[Jernej Skrabec]

This series fixes a number of bugs in the Cedrus driver, found while
running the conformance suites and comparing with the vendor library,
mostly on H616.

Patches 1-9 are fixes:
- patch 1 fixes MV-col buffer leak when the file handle is closed
  without STREAMOFF,
- patches 2 and 7 validate H.264/HEVC SPS and VP8 frame size against
  the hardware limits and allocated capture buffers, preventing memory
  overruns and hardware hangs,
- patch 3 marks H.264 long-term references, which fixes MR8_BT_B and
  MR9_BT_B conformance samples,
- patches 4-6 fix MPEG-2 field picture decoding (field height, field
  order detection and references of the second field),
- patch 8 enlarges VP8 probability table to the size used by the vendor
  libraries, since the hardware uses the area past it as scratch space,
- patch 9 switches H616 to the 256-bit DDR mode, which fixes corrupted
  reference chroma in H.264 and VP8.

Patch 10 is new functionality: VP8 decoding of frames wider than 2048
pixels.

Please take a look.

Best regards,
Jernej

Jernej Skrabec (10):
  staging: media: cedrus: free MV-col buffers in buf_cleanup
  staging: media: cedrus: reject too large H.264/HEVC SPS
  staging: media: cedrus: h264: mark long-term references in the frame
    info
  staging: media: cedrus: mpeg2: use the field height for field pictures
  staging: media: cedrus: mpeg2: derive the field order of field
    pictures
  staging: media: cedrus: mpeg2: fix the references of field pictures
  staging: media: cedrus: validate VP8 frame size
  staging: media: cedrus: vp8: use the vendor probability table size
  staging: media: cedrus: h616: use the 256-bit DDR mode
  staging: media: cedrus: vp8: support frames wider than 2048 pixels

 drivers/staging/media/sunxi/cedrus/cedrus.c   | 60 +++++++++++++++-
 drivers/staging/media/sunxi/cedrus/cedrus.h   | 15 ++++
 .../staging/media/sunxi/cedrus/cedrus_h264.c  | 40 +++++++----
 .../staging/media/sunxi/cedrus/cedrus_h265.c  | 28 ++++----
 .../staging/media/sunxi/cedrus/cedrus_hw.c    | 14 ++--
 .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 72 ++++++++++++++++---
 .../staging/media/sunxi/cedrus/cedrus_regs.h  |  8 +++
 .../staging/media/sunxi/cedrus/cedrus_video.c | 15 ++--
 .../staging/media/sunxi/cedrus/cedrus_video.h |  5 ++
 .../staging/media/sunxi/cedrus/cedrus_vp8.c   | 66 ++++++++++++++++-
 10 files changed, 276 insertions(+), 47 deletions(-)


base-commit: 27285b099fbc5cb2ae3aef20f3bbd7b3c88b3005
-- 
2.43.0

[PATCH 01/10] staging: media: cedrus: free MV-col buffers in buf_cleanup

[Jernej Skrabec]

v4l2_m2m_ctx_release() releases the CAPTURE queue before the OUTPUT
queue. When userspace closes the file handle without STREAMOFF, the
CAPTURE buffers are already gone by the time the OUTPUT stop_streaming
calls the codec stop() callback, so the per-buffer H.264/HEVC MV-col
buffers are never freed. This is easily reproduced by killing a player
during H.264 or HEVC decoding. Each killed 1080p session leaks about
2.3 MiB.

Add a buf_cleanup codec op, called from the vb2 buf_cleanup of the
CAPTURE queue, and free the MV-col buffer there. stop() keeps using the
same helper.

Fixes: fec94f8c9954 ("media: cedrus: h264: Optimize mv col buffer allocation")
Fixes: 0ee952c2f484 ("media: cedrus: h265: Associate mv col buffers with buffer")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.h   |  1 +
 .../staging/media/sunxi/cedrus/cedrus_h264.c  | 28 +++++++++++--------
 .../staging/media/sunxi/cedrus/cedrus_h265.c  | 28 +++++++++++--------
 .../staging/media/sunxi/cedrus/cedrus_video.c | 10 +++++++
 4 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index c4b1217c14b6..54f867949fba 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -167,6 +167,7 @@ struct cedrus_dec_ops {
 	int (*setup)(struct cedrus_ctx *ctx, struct cedrus_run *run);
 	int (*start)(struct cedrus_ctx *ctx);
 	void (*stop)(struct cedrus_ctx *ctx);
+	void (*buf_cleanup)(struct cedrus_ctx *ctx, struct cedrus_buffer *buf);
 	void (*trigger)(struct cedrus_ctx *ctx);
 	unsigned int (*extra_cap_size)(struct cedrus_ctx *ctx,
 				       struct v4l2_pix_format *pix_fmt);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
index fc54d993b11f..892193c12b2f 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
@@ -647,10 +647,23 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx)
 	return ret;
 }
 
+static void cedrus_h264_buf_cleanup(struct cedrus_ctx *ctx,
+				    struct cedrus_buffer *buf)
+{
+	if (buf->codec.h264.mv_col_buf_size > 0) {
+		dma_free_attrs(ctx->dev->dev,
+			       buf->codec.h264.mv_col_buf_size,
+			       buf->codec.h264.mv_col_buf,
+			       buf->codec.h264.mv_col_buf_dma,
+			       DMA_ATTR_NO_KERNEL_MAPPING);
+
+		buf->codec.h264.mv_col_buf_size = 0;
+	}
+}
+
 static void cedrus_h264_stop(struct cedrus_ctx *ctx)
 {
 	struct cedrus_dev *dev = ctx->dev;
-	struct cedrus_buffer *buf;
 	struct vb2_queue *vq;
 	unsigned int i;
 
@@ -662,17 +675,7 @@ static void cedrus_h264_stop(struct cedrus_ctx *ctx)
 		if (!vb)
 			continue;
 
-		buf = vb2_to_cedrus_buffer(vb);
-
-		if (buf->codec.h264.mv_col_buf_size > 0) {
-			dma_free_attrs(dev->dev,
-				       buf->codec.h264.mv_col_buf_size,
-				       buf->codec.h264.mv_col_buf,
-				       buf->codec.h264.mv_col_buf_dma,
-				       DMA_ATTR_NO_KERNEL_MAPPING);
-
-			buf->codec.h264.mv_col_buf_size = 0;
-		}
+		cedrus_h264_buf_cleanup(ctx, vb2_to_cedrus_buffer(vb));
 	}
 
 	dma_free_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE,
@@ -710,5 +713,6 @@ struct cedrus_dec_ops cedrus_dec_ops_h264 = {
 	.setup		= cedrus_h264_setup,
 	.start		= cedrus_h264_start,
 	.stop		= cedrus_h264_stop,
+	.buf_cleanup	= cedrus_h264_buf_cleanup,
 	.trigger	= cedrus_h264_trigger,
 };
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
index 780da4a8b5af..6c643862ab73 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
@@ -858,10 +858,23 @@ static int cedrus_h265_start(struct cedrus_ctx *ctx)
 	return 0;
 }
 
+static void cedrus_h265_buf_cleanup(struct cedrus_ctx *ctx,
+				    struct cedrus_buffer *buf)
+{
+	if (buf->codec.h265.mv_col_buf_size > 0) {
+		dma_free_attrs(ctx->dev->dev,
+			       buf->codec.h265.mv_col_buf_size,
+			       buf->codec.h265.mv_col_buf,
+			       buf->codec.h265.mv_col_buf_dma,
+			       DMA_ATTR_NO_KERNEL_MAPPING);
+
+		buf->codec.h265.mv_col_buf_size = 0;
+	}
+}
+
 static void cedrus_h265_stop(struct cedrus_ctx *ctx)
 {
 	struct cedrus_dev *dev = ctx->dev;
-	struct cedrus_buffer *buf;
 	struct vb2_queue *vq;
 	unsigned int i;
 
@@ -873,17 +886,7 @@ static void cedrus_h265_stop(struct cedrus_ctx *ctx)
 		if (!vb)
 			continue;
 
-		buf = vb2_to_cedrus_buffer(vb);
-
-		if (buf->codec.h265.mv_col_buf_size > 0) {
-			dma_free_attrs(dev->dev,
-				       buf->codec.h265.mv_col_buf_size,
-				       buf->codec.h265.mv_col_buf,
-				       buf->codec.h265.mv_col_buf_dma,
-				       DMA_ATTR_NO_KERNEL_MAPPING);
-
-			buf->codec.h265.mv_col_buf_size = 0;
-		}
+		cedrus_h265_buf_cleanup(ctx, vb2_to_cedrus_buffer(vb));
 	}
 
 	dma_free_attrs(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE,
@@ -918,6 +921,7 @@ struct cedrus_dec_ops cedrus_dec_ops_h265 = {
 	.setup		= cedrus_h265_setup,
 	.start		= cedrus_h265_start,
 	.stop		= cedrus_h265_stop,
+	.buf_cleanup	= cedrus_h265_buf_cleanup,
 	.trigger	= cedrus_h265_trigger,
 	.extra_cap_size	= cedrus_h265_extra_cap_size,
 };
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index ad4ec3490775..adc7f255d7cf 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -560,6 +560,15 @@ static void cedrus_stop_streaming(struct vb2_queue *vq)
 	cedrus_queue_cleanup(vq, VB2_BUF_STATE_ERROR);
 }
 
+static void cedrus_buf_cleanup(struct vb2_buffer *vb)
+{
+	struct cedrus_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+
+	if (V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type) &&
+	    ctx->current_codec && ctx->current_codec->buf_cleanup)
+		ctx->current_codec->buf_cleanup(ctx, vb2_to_cedrus_buffer(vb));
+}
+
 static void cedrus_buf_queue(struct vb2_buffer *vb)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -579,6 +588,7 @@ static const struct vb2_ops cedrus_qops = {
 	.queue_setup		= cedrus_queue_setup,
 	.buf_prepare		= cedrus_buf_prepare,
 	.buf_queue		= cedrus_buf_queue,
+	.buf_cleanup		= cedrus_buf_cleanup,
 	.buf_out_validate	= cedrus_buf_out_validate,
 	.buf_request_complete	= cedrus_buf_request_complete,
 	.start_streaming	= cedrus_start_streaming,
-- 
2.43.0

[PATCH 02/10] staging: media: cedrus: reject too large H.264/HEVC SPS

[Jernej Skrabec]

The hardware decodes whatever picture size the SPS describes. An SPS
beyond CEDRUS_MAX_WIDTH/HEIGHT, or beyond the capture buffers once they
are allocated, makes the hardware overrun memory and hang. On H616, the
JCT-VC PICSIZE_A..D streams (1056x8440, 8440x1056, 528x4216 and
4216x528) leave the hardware wedged, so that every following job times
out until reboot.

Validate the SPS picture size in try_ctrl. Move the size limits to
cedrus_video.h for that.

Fixes: 6eb9b758e307 ("media: cedrus: Add H264 decoding support")
Fixes: 86caab29da78 ("media: cedrus: Add HEVC/H.265 decoding support")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c   | 40 ++++++++++++++++++-
 .../staging/media/sunxi/cedrus/cedrus_video.c |  5 ---
 .../staging/media/sunxi/cedrus/cedrus_video.h |  5 +++
 3 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index c3331b4f3f02..45deebddb630 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -28,10 +28,38 @@
 #include "cedrus_dec.h"
 #include "cedrus_hw.h"
 
+/*
+ * The hardware decodes whatever picture size the bitstream headers describe.
+ * Reject sizes beyond the hardware limits, or beyond the capture buffers once
+ * they are allocated, instead of letting the hardware overrun memory and hang.
+ */
+static int cedrus_check_pic_size(struct cedrus_ctx *ctx, u32 width, u32 height)
+{
+	struct vb2_queue *vq;
+
+	if (width > CEDRUS_MAX_WIDTH || height > CEDRUS_MAX_HEIGHT)
+		return -EINVAL;
+
+	/*
+	 * The hardware decodes whole 16x16 macroblocks, and all capture
+	 * formats pad the buffers to at least 16 pixels in both directions.
+	 */
+	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+	if (vb2_is_busy(vq) &&
+	    (width > ALIGN(ctx->src_fmt.width, 16) ||
+	     height > ALIGN(ctx->src_fmt.height, 16)))
+		return -EINVAL;
+
+	return 0;
+}
+
 static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct cedrus_ctx *ctx = container_of(ctrl->handler, struct cedrus_ctx, hdl);
+
 	if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
 		const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
+		u32 height;
 
 		if (sps->chroma_format_idc != 1)
 			/* Only 4:2:0 is supported */
@@ -42,9 +70,15 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
 		if (sps->bit_depth_luma_minus8 != 0)
 			/* Only 8-bit is supported */
 			return -EINVAL;
+		height = (sps->pic_height_in_map_units_minus1 + 1) * 16;
+		if (!(sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY))
+			height *= 2;
+		if (cedrus_check_pic_size(ctx,
+					  (sps->pic_width_in_mbs_minus1 + 1) * 16,
+					  height))
+			return -EINVAL;
 	} else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) {
 		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
-		struct cedrus_ctx *ctx = container_of(ctrl->handler, struct cedrus_ctx, hdl);
 		unsigned int bit_depth, max_depth;
 		struct vb2_queue *vq;
 
@@ -52,6 +86,10 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
 			/* Only 4:2:0 is supported */
 			return -EINVAL;
 
+		if (cedrus_check_pic_size(ctx, sps->pic_width_in_luma_samples,
+					  sps->pic_height_in_luma_samples))
+			return -EINVAL;
+
 		bit_depth = max(sps->bit_depth_luma_minus8,
 				sps->bit_depth_chroma_minus8) + 8;
 
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index adc7f255d7cf..2521315abdb7 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -29,11 +29,6 @@
 #define CEDRUS_DECODE_SRC	BIT(0)
 #define CEDRUS_DECODE_DST	BIT(1)
 
-#define CEDRUS_MIN_WIDTH	16U
-#define CEDRUS_MIN_HEIGHT	16U
-#define CEDRUS_MAX_WIDTH	4096U
-#define CEDRUS_MAX_HEIGHT	2304U
-
 static struct cedrus_format cedrus_formats[] = {
 	{
 		.pixelformat	= V4L2_PIX_FMT_MPEG2_SLICE,
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.h b/drivers/staging/media/sunxi/cedrus/cedrus_video.h
index 8e1afc16a6a1..367e52158f01 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.h
@@ -22,6 +22,11 @@ struct cedrus_format {
 	unsigned int	capabilities;
 };
 
+#define CEDRUS_MIN_WIDTH	16U
+#define CEDRUS_MIN_HEIGHT	16U
+#define CEDRUS_MAX_WIDTH	4096U
+#define CEDRUS_MAX_HEIGHT	2304U
+
 extern const struct v4l2_ioctl_ops cedrus_ioctl_ops;
 
 int cedrus_queue_init(void *priv, struct vb2_queue *src_vq,
-- 
2.43.0

[PATCH 03/10] staging: media: cedrus: h264: mark long-term references in the frame info

[Jernej Skrabec]

The frame info of a reference has a reference type for each field, in
bits 1:0 for the top field and 5:4 for the bottom one, which is 1 for a
long-term reference. The driver never sets it, so the hardware handles
long-term references like short-term ones, also where they differ, like
in temporal direct and implicit weighted prediction. The vendor library
sets it.

This fixes MR8_BT_B and MR9_BT_B of the H.264 conformance suite.

Fixes: 6eb9b758e307 ("media: cedrus: Add H264 decoding support")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 12 +++++++++++-
 drivers/staging/media/sunxi/cedrus/cedrus_regs.h |  8 ++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
index 892193c12b2f..a716955655f7 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
@@ -69,13 +69,21 @@ static void cedrus_fill_ref_pic(struct cedrus_ctx *ctx,
 				struct cedrus_buffer *buf,
 				unsigned int top_field_order_cnt,
 				unsigned int bottom_field_order_cnt,
+				bool long_term,
 				struct cedrus_h264_sram_ref_pic *pic)
 {
 	struct vb2_buffer *vbuf = &buf->m2m_buf.vb.vb2_buf;
+	u32 frame_info, ref_type;
 
 	pic->top_field_order_cnt = cpu_to_le32(top_field_order_cnt);
 	pic->bottom_field_order_cnt = cpu_to_le32(bottom_field_order_cnt);
-	pic->frame_info = cpu_to_le32(buf->codec.h264.pic_type << 8);
+
+	ref_type = long_term ? VE_H264_FRAME_INFO_REF_TYPE_LONG_TERM :
+			       VE_H264_FRAME_INFO_REF_TYPE_SHORT_TERM;
+	frame_info = VE_H264_FRAME_INFO_TOP_REF_TYPE(ref_type) |
+		     VE_H264_FRAME_INFO_BOT_REF_TYPE(ref_type) |
+		     VE_H264_FRAME_INFO_PIC_TYPE(buf->codec.h264.pic_type);
+	pic->frame_info = cpu_to_le32(frame_info);
 
 	pic->luma_ptr = cpu_to_le32(cedrus_buf_addr(vbuf, &ctx->dst_fmt, 0));
 	pic->chroma_ptr = cpu_to_le32(cedrus_buf_addr(vbuf, &ctx->dst_fmt, 1));
@@ -128,6 +136,7 @@ static int cedrus_write_frame_list(struct cedrus_ctx *ctx,
 		cedrus_fill_ref_pic(ctx, cedrus_buf,
 				    dpb->top_field_order_cnt,
 				    dpb->bottom_field_order_cnt,
+				    dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM,
 				    &pic_list[position]);
 	}
 
@@ -174,6 +183,7 @@ static int cedrus_write_frame_list(struct cedrus_ctx *ctx,
 	cedrus_fill_ref_pic(ctx, output_buf,
 			    decode->top_field_order_cnt,
 			    decode->bottom_field_order_cnt,
+			    false,
 			    &pic_list[position]);
 
 	cedrus_h264_write_sram(dev, CEDRUS_SRAM_H264_FRAMEBUFFER_LIST,
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
index 05e6cbc548ab..961fe023bb43 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
@@ -616,6 +616,14 @@
 #define VE_AVC_SRAM_PORT_OFFSET		0x2e0
 #define VE_AVC_SRAM_PORT_DATA		0x2e4
 
+/* Frame info word of a frame buffer list entry in the AVC SRAM */
+#define VE_H264_FRAME_INFO_PIC_TYPE(v)		SHIFT_AND_MASK_BITS(v, 9, 8)
+#define VE_H264_FRAME_INFO_BOT_REF_TYPE(v)	SHIFT_AND_MASK_BITS(v, 5, 4)
+#define VE_H264_FRAME_INFO_TOP_REF_TYPE(v)	SHIFT_AND_MASK_BITS(v, 1, 0)
+
+#define VE_H264_FRAME_INFO_REF_TYPE_SHORT_TERM	0
+#define VE_H264_FRAME_INFO_REF_TYPE_LONG_TERM	1
+
 #define VE_VP8_PPS			0x214
 #define VE_VP8_PPS_PIC_TYPE_P_FRAME		BIT(31)
 #define VE_VP8_PPS_LAST_SHARPNESS_LEVEL(v)	SHIFT_AND_MASK_BITS(v, 30, 28)
-- 
2.43.0

[PATCH 04/10] staging: media: cedrus: mpeg2: use the field height for field pictures

[Jernej Skrabec]

A field picture covers only half of the macroblock rows of a frame, but
the driver programs the coded and the boundary size of the whole frame
for it, as for a frame picture.

Program the height of a field for field pictures, like the vendor
library does.

Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index 10e98f08aafc..ca66a3c6b830 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -58,6 +58,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	struct vb2_queue *vq;
 	const u8 *matrix;
 	unsigned int i;
+	bool field;
 	u32 reg;
 
 	seq = run->mpeg2.sequence;
@@ -108,13 +109,21 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 
 	/* Set frame dimensions. */
 
+	/*
+	 * Field pictures cover half of the frame's macroblock rows; the
+	 * vendor library halves both the coded and the boundary height.
+	 */
+	field = pic->picture_structure != V4L2_MPEG2_PIC_FRAME;
+
 	reg = VE_DEC_MPEG_PICCODEDSIZE_WIDTH(seq->horizontal_size);
-	reg |= VE_DEC_MPEG_PICCODEDSIZE_HEIGHT(seq->vertical_size);
+	reg |= VE_DEC_MPEG_PICCODEDSIZE_HEIGHT(field ?
+			ALIGN(seq->vertical_size, 32) / 2 : seq->vertical_size);
 
 	cedrus_write(dev, VE_DEC_MPEG_PICCODEDSIZE, reg);
 
 	reg = VE_DEC_MPEG_PICBOUNDSIZE_WIDTH(ctx->src_fmt.width);
-	reg |= VE_DEC_MPEG_PICBOUNDSIZE_HEIGHT(ctx->src_fmt.height);
+	reg |= VE_DEC_MPEG_PICBOUNDSIZE_HEIGHT(field ? ctx->src_fmt.height / 2 :
+						   ctx->src_fmt.height);
 
 	cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
 
-- 
2.43.0

[PATCH 05/10] staging: media: cedrus: mpeg2: derive the field order of field pictures

[Jernej Skrabec]

The hardware uses the top field first bit of the picture header to tell
the first field of a frame from the second one. However,
top_field_first is always 0 in field pictures, so userspace passes
V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST cleared, and the second field of
every top field first pair is predicted from the wrong reference.

Detect the second field in the driver instead: it has the opposite
parity of the previous field and is decoded into the same capture
buffer. This works with unmodified userspace.

Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.h   |  5 +++
 .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 32 +++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 54f867949fba..a523b56a9b8c 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -144,6 +144,11 @@ struct cedrus_ctx {
 			void		*entry_points_buf;
 			dma_addr_t	entry_points_buf_addr;
 		} h265;
+		struct {
+			bool		pair_pending;
+			unsigned int	pair_dst;
+			u8		pair_structure;
+		} mpeg2;
 		struct {
 			unsigned int	last_frame_p_type;
 			unsigned int	last_filter_type;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index ca66a3c6b830..4c8accee5389 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -58,7 +58,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	struct vb2_queue *vq;
 	const u8 *matrix;
 	unsigned int i;
-	bool field;
+	bool field, tff;
 	u32 reg;
 
 	seq = run->mpeg2.sequence;
@@ -87,6 +87,26 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 		cedrus_write(dev, VE_DEC_MPEG_IQMINPUT, reg);
 	}
 
+	/*
+	 * top_field_first is always 0 in field pictures, but the hardware
+	 * uses this bit to tell the first field of a frame from the second.
+	 * A field is the second one when it has the opposite parity of the
+	 * previous field and is decoded into the same capture buffer.
+	 */
+	if (pic->picture_structure == V4L2_MPEG2_PIC_FRAME) {
+		tff = !!(pic->flags & V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST);
+		ctx->codec.mpeg2.pair_pending = false;
+	} else {
+		bool second = ctx->codec.mpeg2.pair_pending &&
+			      ctx->codec.mpeg2.pair_structure != pic->picture_structure &&
+			      ctx->codec.mpeg2.pair_dst == run->dst->vb2_buf.index;
+
+		tff = (pic->picture_structure == V4L2_MPEG2_PIC_TOP_FIELD) != second;
+		ctx->codec.mpeg2.pair_pending = !second;
+		ctx->codec.mpeg2.pair_structure = pic->picture_structure;
+		ctx->codec.mpeg2.pair_dst = run->dst->vb2_buf.index;
+	}
+
 	/* Set MPEG picture header. */
 
 	reg = VE_DEC_MPEG_MP12HDR_SLICE_TYPE(pic->picture_coding_type);
@@ -96,7 +116,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	reg |= VE_DEC_MPEG_MP12HDR_F_CODE(1, 1, pic->f_code[1][1]);
 	reg |= VE_DEC_MPEG_MP12HDR_INTRA_DC_PRECISION(pic->intra_dc_precision);
 	reg |= VE_DEC_MPEG_MP12HDR_INTRA_PICTURE_STRUCTURE(pic->picture_structure);
-	reg |= VE_DEC_MPEG_MP12HDR_TOP_FIELD_FIRST(pic->flags & V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST);
+	reg |= VE_DEC_MPEG_MP12HDR_TOP_FIELD_FIRST(tff);
 	reg |= VE_DEC_MPEG_MP12HDR_FRAME_PRED_FRAME_DCT(pic->flags & V4L2_MPEG2_PIC_FLAG_FRAME_PRED_DCT);
 	reg |= VE_DEC_MPEG_MP12HDR_CONCEALMENT_MOTION_VECTORS(pic->flags & V4L2_MPEG2_PIC_FLAG_CONCEALMENT_MV);
 	reg |= VE_DEC_MPEG_MP12HDR_Q_SCALE_TYPE(pic->flags & V4L2_MPEG2_PIC_FLAG_Q_SCALE_TYPE);
@@ -186,6 +206,13 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	return 0;
 }
 
+static int cedrus_mpeg2_start(struct cedrus_ctx *ctx)
+{
+	memset(&ctx->codec.mpeg2, 0, sizeof(ctx->codec.mpeg2));
+
+	return 0;
+}
+
 static void cedrus_mpeg2_trigger(struct cedrus_ctx *ctx)
 {
 	struct cedrus_dev *dev = ctx->dev;
@@ -203,5 +230,6 @@ struct cedrus_dec_ops cedrus_dec_ops_mpeg2 = {
 	.irq_disable	= cedrus_mpeg2_irq_disable,
 	.irq_status	= cedrus_mpeg2_irq_status,
 	.setup		= cedrus_mpeg2_setup,
+	.start		= cedrus_mpeg2_start,
 	.trigger	= cedrus_mpeg2_trigger,
 };
-- 
2.43.0

[PATCH 06/10] staging: media: cedrus: mpeg2: fix the references of field pictures

[Jernej Skrabec]

The second field of an I or P frame can be predicted from the first
field of the same frame, which the hardware takes from the backward
reference. The driver programs the backward reference passed by
userspace instead, which doesn't point to the frame being decoded, so
the second field is predicted from the wrong picture.

Use the current frame as the backward reference of I and P pictures,
like the vendor library does. Also fall back to the current frame
instead of address 0 when a reference buffer is missing, so the
hardware never reads from an invalid address.

Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 29 ++++++++++++++-----
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index 4c8accee5389..0a2f8e77fb29 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -56,6 +56,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	dma_addr_t src_buf_addr, dst_luma_addr, dst_chroma_addr;
 	struct cedrus_dev *dev = ctx->dev;
 	struct vb2_queue *vq;
+	struct vb2_buffer *fwd, *bwd;
 	const u8 *matrix;
 	unsigned int i;
 	bool field, tff;
@@ -147,15 +148,29 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 
 	cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
 
-	/* Forward and backward prediction reference buffers. */
+	/*
+	 * Forward and backward prediction reference buffers. As the vendor
+	 * library does, I and P pictures use the current frame as backward
+	 * reference: the second field of a frame predicts from the first one.
+	 * Missing references also fall back to the current frame.
+	 */
 	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 
-	cedrus_write_ref_buf_addr(ctx, vq, pic->forward_ref_ts,
-				  VE_DEC_MPEG_FWD_REF_LUMA_ADDR,
-				  VE_DEC_MPEG_FWD_REF_CHROMA_ADDR);
-	cedrus_write_ref_buf_addr(ctx, vq, pic->backward_ref_ts,
-				  VE_DEC_MPEG_BWD_REF_LUMA_ADDR,
-				  VE_DEC_MPEG_BWD_REF_CHROMA_ADDR);
+	fwd = vb2_find_buffer(vq, pic->forward_ref_ts) ?: &run->dst->vb2_buf;
+	if (pic->picture_coding_type == V4L2_MPEG2_PIC_CODING_TYPE_B)
+		bwd = vb2_find_buffer(vq, pic->backward_ref_ts) ?:
+		      &run->dst->vb2_buf;
+	else
+		bwd = &run->dst->vb2_buf;
+
+	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR,
+		     cedrus_dst_buf_addr(ctx, fwd, 0));
+	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR,
+		     cedrus_dst_buf_addr(ctx, fwd, 1));
+	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_LUMA_ADDR,
+		     cedrus_dst_buf_addr(ctx, bwd, 0));
+	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_CHROMA_ADDR,
+		     cedrus_dst_buf_addr(ctx, bwd, 1));
 
 	/* Destination luma and chroma buffers. */
 
-- 
2.43.0

[PATCH 07/10] staging: media: cedrus: validate VP8 frame size

[Jernej Skrabec]

A VP8 key frame can change the frame size. If userspace doesn't
reallocate the capture buffers, the hardware writes and reads past the
buffers, which shows up as IOMMU page faults and decode timeouts.

Check the frame size in try_ctrl, like for the H.264 and HEVC SPS.

Fixes: 3c39a16d30d8 ("media: cedrus: Add support for VP8 decoding")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 45deebddb630..756243795de8 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -77,6 +77,11 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
 					  (sps->pic_width_in_mbs_minus1 + 1) * 16,
 					  height))
 			return -EINVAL;
+	} else if (ctrl->id == V4L2_CID_STATELESS_VP8_FRAME) {
+		const struct v4l2_ctrl_vp8_frame *frame = ctrl->p_new.p_vp8_frame;
+
+		if (cedrus_check_pic_size(ctx, frame->width, frame->height))
+			return -EINVAL;
 	} else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) {
 		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
 		unsigned int bit_depth, max_depth;
@@ -270,6 +275,7 @@ static const struct cedrus_control cedrus_controls[] = {
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_VP8_FRAME,
+			.ops	= &cedrus_ctrl_ops,
 		},
 		.capabilities	= CEDRUS_CAPABILITY_VP8_DEC,
 	},
-- 
2.43.0

[PATCH 08/10] staging: media: cedrus: vp8: use the vendor probability table size

[Jernej Skrabec]

The driver allocates the VP8 probability table with 0x2400 bytes, which
is the size the old Allwinner A10 library uses. All newer vendor
libraries allocate 0x5400 bytes and never write the area past 0x11b0
from the CPU. The hardware uses that area as scratch space, most likely
for the segment map, and its usage grows with the frame size: a
3840x2160 stream makes the hardware access memory past 0x2400 and
causes IOMMU page faults.

Allocate the table with 0x5400 bytes, like the vendor libraries do.

Fixes: 3c39a16d30d8 ("media: cedrus: Add support for VP8 decoding")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus_vp8.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
index 969677a3bbf9..6b00efa77c22 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
@@ -25,7 +25,7 @@
 #include "cedrus_hw.h"
 #include "cedrus_regs.h"
 
-#define CEDRUS_ENTROPY_PROBS_SIZE 0x2400
+#define CEDRUS_ENTROPY_PROBS_SIZE 0x5400
 #define VP8_PROB_HALF 128
 #define QUANT_DELTA_COUNT 5
 
-- 
2.43.0

[PATCH 09/10] staging: media: cedrus: h616: use the 256-bit DDR mode

[Jernej Skrabec]

With the DDR mode in VE_MODE set to 128-bit, which the driver uses on
all SoCs, the H616 H.264 engine reads corrupted reference chroma, while
luma is fine. H.264 and VP8 P frames come out with bit-exact luma, but
with about 22 dB PSNR chroma. H.265 and MPEG-2 are unaffected.

The vendor library selects the DDR mode based on the VE IC version. It
uses the 256-bit mode without the 2 MiB reconstruction write mode on
the H616, and keeps the 128-bit mode with the 2 MiB reconstruction
write mode on older SoCs, like the H6. Doing the same on the H616 makes
all codecs bit-exact (tested with BA_MW_D, CABA2_Sony_E,
vp80-00-comprehensive-002, AMP_A_Samsung_7 and MEI.stream16).

Make the VE_MODE DDR settings a variant property and add an H616
variant, which uses the 256-bit mode. Replace the FIXME about the
DDR mode with a description of what the vendor library does.

Fixes: 1f24d4307515 ("staging: media: sunxi: cedrus: add H616 variant")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c    | 14 +++++++++++++-
 drivers/staging/media/sunxi/cedrus/cedrus.h    |  2 ++
 drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 14 ++++++++++----
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 756243795de8..2bbc79b44f99 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -27,6 +27,7 @@
 #include "cedrus_video.h"
 #include "cedrus_dec.h"
 #include "cedrus_hw.h"
+#include "cedrus_regs.h"
 
 /*
  * The hardware decodes whatever picture size the bitstream headers describe.
@@ -697,6 +698,17 @@ static const struct cedrus_variant sun50i_h6_cedrus_variant = {
 	.mod_rate	= 600000000,
 };
 
+static const struct cedrus_variant sun50i_h616_cedrus_variant = {
+	.capabilities	= CEDRUS_CAPABILITY_UNTILED |
+			  CEDRUS_CAPABILITY_MPEG2_DEC |
+			  CEDRUS_CAPABILITY_H264_DEC |
+			  CEDRUS_CAPABILITY_H265_DEC |
+			  CEDRUS_CAPABILITY_H265_10_DEC |
+			  CEDRUS_CAPABILITY_VP8_DEC,
+	.mod_rate	= 600000000,
+	.ve_mode_ddr	= VE_MODE_DDR_MODE_BW_256,
+};
+
 static const struct of_device_id cedrus_dt_match[] = {
 	{
 		.compatible = "allwinner,sun4i-a10-video-engine",
@@ -744,7 +756,7 @@ static const struct of_device_id cedrus_dt_match[] = {
 	},
 	{
 		.compatible = "allwinner,sun50i-h616-video-engine",
-		.data = &sun50i_h6_cedrus_variant,
+		.data = &sun50i_h616_cedrus_variant,
 	},
 	{ /* sentinel */ }
 };
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index a523b56a9b8c..d28fe91562d1 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -181,6 +181,7 @@ struct cedrus_dec_ops {
 struct cedrus_variant {
 	unsigned int	capabilities;
 	unsigned int	mod_rate;
+	u32		ve_mode_ddr;
 };
 
 struct cedrus_dev {
@@ -204,6 +205,7 @@ struct cedrus_dev {
 	struct reset_control	*rstc;
 
 	unsigned int		capabilities;
+	u32			ve_mode_ddr;
 
 	struct delayed_work	watchdog_work;
 };
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index 444fb53878d1..b66d855daaf8 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -36,11 +36,16 @@ int cedrus_engine_enable(struct cedrus_ctx *ctx)
 	u32 reg = 0;
 
 	/*
-	 * FIXME: This is only valid on 32-bits DDR's, we should test
-	 * it on the A13/A33.
+	 * The DDR mode depends on the SoC. Like the vendor library, use the
+	 * 128-bit mode with the 2 MiB reconstruction write mode, unless the
+	 * variant says otherwise.
 	 */
-	reg |= VE_MODE_REC_WR_MODE_2MB;
-	reg |= VE_MODE_DDR_MODE_BW_128;
+	if (ctx->dev->ve_mode_ddr) {
+		reg |= ctx->dev->ve_mode_ddr;
+	} else {
+		reg |= VE_MODE_REC_WR_MODE_2MB;
+		reg |= VE_MODE_DDR_MODE_BW_128;
+	}
 
 	switch (ctx->src_fmt.pixelformat) {
 	case V4L2_PIX_FMT_MPEG2_SLICE:
@@ -254,6 +259,7 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
 		return -EINVAL;
 
 	dev->capabilities = variant->capabilities;
+	dev->ve_mode_ddr = variant->ve_mode_ddr;
 
 	irq_dec = platform_get_irq(dev->pdev, 0);
 	if (irq_dec <= 0)
-- 
2.43.0

[PATCH 10/10] staging: media: cedrus: vp8: support frames wider than 2048 pixels

[Jernej Skrabec]

Above 2048 pixels, the deblocking and intra prediction row buffers don't
fit into the internal SRAM anymore. Put them into DRAM like H.264 does.
The vendor library sizes the deblocking buffer as 24 bytes per column.

Tested on H616 with 2560x1440 and 3840x2160 libvpx streams, which are
bit-exact with the FFmpeg software decoder.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.h   |  7 ++
 .../staging/media/sunxi/cedrus/cedrus_vp8.c   | 64 +++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index d28fe91562d1..ca4efa9302fc 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -156,6 +156,13 @@ struct cedrus_ctx {
 
 			u8		*entropy_probs_buf;
 			dma_addr_t	entropy_probs_buf_dma;
+
+			void		*deblk_buf;
+			dma_addr_t	deblk_buf_dma;
+			ssize_t		deblk_buf_size;
+			void		*intra_pred_buf;
+			dma_addr_t	intra_pred_buf_dma;
+			ssize_t		intra_pred_buf_size;
 		} vp8;
 	} codec;
 };
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
index 6b00efa77c22..954c0ceb98ed 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
@@ -666,6 +666,20 @@ static int cedrus_vp8_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 
 	cedrus_write(dev, VE_H264_CTRL, VE_H264_CTRL_VP8);
 
+	if (ctx->codec.vp8.deblk_buf_size) {
+		cedrus_write(dev, VE_BUF_CTRL,
+			     VE_BUF_CTRL_INTRAPRED_MIXED_RAM |
+			     VE_BUF_CTRL_DBLK_MIXED_RAM);
+		cedrus_write(dev, VE_DBLK_DRAM_BUF_ADDR,
+			     ctx->codec.vp8.deblk_buf_dma);
+		cedrus_write(dev, VE_INTRAPRED_DRAM_BUF_ADDR,
+			     ctx->codec.vp8.intra_pred_buf_dma);
+	} else {
+		cedrus_write(dev, VE_BUF_CTRL,
+			     VE_BUF_CTRL_INTRAPRED_INT_SRAM |
+			     VE_BUF_CTRL_DBLK_INT_SRAM);
+	}
+
 	cedrus_vp8_update_probs(slice, ctx->codec.vp8.entropy_probs_buf);
 
 	reg = slice->first_part_size * 8;
@@ -849,7 +863,44 @@ static int cedrus_vp8_start(struct cedrus_ctx *ctx)
 	memcpy(&ctx->codec.vp8.entropy_probs_buf[2048],
 	       prob_table_init, sizeof(prob_table_init));
 
+	/*
+	 * Above 2048 pixels the deblocking and intra prediction row buffers
+	 * no longer fit into the internal SRAM. The vendor library sizes the
+	 * deblocking buffer as 24 bytes per (32 aligned) column.
+	 */
+	if (ctx->src_fmt.width > 2048) {
+		ctx->codec.vp8.deblk_buf_size =
+			ALIGN(ctx->src_fmt.width, 32) * 24;
+		ctx->codec.vp8.deblk_buf =
+			dma_alloc_attrs(dev->dev, ctx->codec.vp8.deblk_buf_size,
+					&ctx->codec.vp8.deblk_buf_dma,
+					GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING);
+		if (!ctx->codec.vp8.deblk_buf)
+			goto err_probs;
+
+		ctx->codec.vp8.intra_pred_buf_size =
+			ALIGN(ctx->src_fmt.width, 64) * 5 * 2;
+		ctx->codec.vp8.intra_pred_buf =
+			dma_alloc_attrs(dev->dev,
+					ctx->codec.vp8.intra_pred_buf_size,
+					&ctx->codec.vp8.intra_pred_buf_dma,
+					GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING);
+		if (!ctx->codec.vp8.intra_pred_buf)
+			goto err_deblk;
+	}
+
 	return 0;
+
+err_deblk:
+	dma_free_attrs(dev->dev, ctx->codec.vp8.deblk_buf_size,
+		       ctx->codec.vp8.deblk_buf, ctx->codec.vp8.deblk_buf_dma,
+		       DMA_ATTR_NO_KERNEL_MAPPING);
+	ctx->codec.vp8.deblk_buf_size = 0;
+err_probs:
+	dma_free_coherent(dev->dev, CEDRUS_ENTROPY_PROBS_SIZE,
+			  ctx->codec.vp8.entropy_probs_buf,
+			  ctx->codec.vp8.entropy_probs_buf_dma);
+	return -ENOMEM;
 }
 
 static void cedrus_vp8_stop(struct cedrus_ctx *ctx)
@@ -861,6 +912,19 @@ static void cedrus_vp8_stop(struct cedrus_ctx *ctx)
 	dma_free_coherent(dev->dev, CEDRUS_ENTROPY_PROBS_SIZE,
 			  ctx->codec.vp8.entropy_probs_buf,
 			  ctx->codec.vp8.entropy_probs_buf_dma);
+
+	if (ctx->codec.vp8.deblk_buf_size) {
+		dma_free_attrs(dev->dev, ctx->codec.vp8.deblk_buf_size,
+			       ctx->codec.vp8.deblk_buf,
+			       ctx->codec.vp8.deblk_buf_dma,
+			       DMA_ATTR_NO_KERNEL_MAPPING);
+		dma_free_attrs(dev->dev, ctx->codec.vp8.intra_pred_buf_size,
+			       ctx->codec.vp8.intra_pred_buf,
+			       ctx->codec.vp8.intra_pred_buf_dma,
+			       DMA_ATTR_NO_KERNEL_MAPPING);
+		ctx->codec.vp8.deblk_buf_size = 0;
+		ctx->codec.vp8.intra_pred_buf_size = 0;
+	}
 }
 
 static void cedrus_vp8_trigger(struct cedrus_ctx *ctx)
-- 
2.43.0

Re: [PATCH 01/10] staging: media: cedrus: free MV-col buffers in buf_cleanup

[Nicolas Dufresne]

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

Le lundi 02 novembre 2026 à 18:02 +0100, Jernej Skrabec a écrit :
> v4l2_m2m_ctx_release() releases the CAPTURE queue before the OUTPUT
> queue. When userspace closes the file handle without STREAMOFF, the
> CAPTURE buffers are already gone by the time the OUTPUT stop_streaming
> calls the codec stop() callback, so the per-buffer H.264/HEVC MV-col
> buffers are never freed. This is easily reproduced by killing a player
> during H.264 or HEVC decoding. Each killed 1080p session leaks about
> 2.3 MiB.
> 
> Add a buf_cleanup codec op, called from the vb2 buf_cleanup of the
> CAPTURE queue, and free the MV-col buffer there. stop() keeps using the
> same helper.
> 
> Fixes: fec94f8c9954 ("media: cedrus: h264: Optimize mv col buffer allocation")
> Fixes: 0ee952c2f484 ("media: cedrus: h265: Associate mv col buffers with buffer")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

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

> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.h   |  1 +
>  .../staging/media/sunxi/cedrus/cedrus_h264.c  | 28 +++++++++++--------
>  .../staging/media/sunxi/cedrus/cedrus_h265.c  | 28 +++++++++++--------
>  .../staging/media/sunxi/cedrus/cedrus_video.c | 10 +++++++
>  4 files changed, 43 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index c4b1217c14b6..54f867949fba 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -167,6 +167,7 @@ struct cedrus_dec_ops {
>  	int (*setup)(struct cedrus_ctx *ctx, struct cedrus_run *run);
>  	int (*start)(struct cedrus_ctx *ctx);
>  	void (*stop)(struct cedrus_ctx *ctx);
> +	void (*buf_cleanup)(struct cedrus_ctx *ctx, struct cedrus_buffer *buf);
>  	void (*trigger)(struct cedrus_ctx *ctx);
>  	unsigned int (*extra_cap_size)(struct cedrus_ctx *ctx,
>  				       struct v4l2_pix_format *pix_fmt);
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> index fc54d993b11f..892193c12b2f 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -647,10 +647,23 @@ static int cedrus_h264_start(struct cedrus_ctx *ctx)
>  	return ret;
>  }
>  
> +static void cedrus_h264_buf_cleanup(struct cedrus_ctx *ctx,
> +				    struct cedrus_buffer *buf)
> +{
> +	if (buf->codec.h264.mv_col_buf_size > 0) {
> +		dma_free_attrs(ctx->dev->dev,
> +			       buf->codec.h264.mv_col_buf_size,
> +			       buf->codec.h264.mv_col_buf,
> +			       buf->codec.h264.mv_col_buf_dma,
> +			       DMA_ATTR_NO_KERNEL_MAPPING);
> +
> +		buf->codec.h264.mv_col_buf_size = 0;
> +	}
> +}
> +
>  static void cedrus_h264_stop(struct cedrus_ctx *ctx)
>  {
>  	struct cedrus_dev *dev = ctx->dev;
> -	struct cedrus_buffer *buf;
>  	struct vb2_queue *vq;
>  	unsigned int i;
>  
> @@ -662,17 +675,7 @@ static void cedrus_h264_stop(struct cedrus_ctx *ctx)
>  		if (!vb)
>  			continue;
>  
> -		buf = vb2_to_cedrus_buffer(vb);
> -
> -		if (buf->codec.h264.mv_col_buf_size > 0) {
> -			dma_free_attrs(dev->dev,
> -				       buf->codec.h264.mv_col_buf_size,
> -				       buf->codec.h264.mv_col_buf,
> -				       buf->codec.h264.mv_col_buf_dma,
> -				       DMA_ATTR_NO_KERNEL_MAPPING);
> -
> -			buf->codec.h264.mv_col_buf_size = 0;
> -		}
> +		cedrus_h264_buf_cleanup(ctx, vb2_to_cedrus_buffer(vb));
>  	}
>  
>  	dma_free_attrs(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE,
> @@ -710,5 +713,6 @@ struct cedrus_dec_ops cedrus_dec_ops_h264 = {
>  	.setup		= cedrus_h264_setup,
>  	.start		= cedrus_h264_start,
>  	.stop		= cedrus_h264_stop,
> +	.buf_cleanup	= cedrus_h264_buf_cleanup,
>  	.trigger	= cedrus_h264_trigger,
>  };
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> index 780da4a8b5af..6c643862ab73 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> @@ -858,10 +858,23 @@ static int cedrus_h265_start(struct cedrus_ctx *ctx)
>  	return 0;
>  }
>  
> +static void cedrus_h265_buf_cleanup(struct cedrus_ctx *ctx,
> +				    struct cedrus_buffer *buf)
> +{
> +	if (buf->codec.h265.mv_col_buf_size > 0) {
> +		dma_free_attrs(ctx->dev->dev,
> +			       buf->codec.h265.mv_col_buf_size,
> +			       buf->codec.h265.mv_col_buf,
> +			       buf->codec.h265.mv_col_buf_dma,
> +			       DMA_ATTR_NO_KERNEL_MAPPING);
> +
> +		buf->codec.h265.mv_col_buf_size = 0;
> +	}
> +}
> +
>  static void cedrus_h265_stop(struct cedrus_ctx *ctx)
>  {
>  	struct cedrus_dev *dev = ctx->dev;
> -	struct cedrus_buffer *buf;
>  	struct vb2_queue *vq;
>  	unsigned int i;
>  
> @@ -873,17 +886,7 @@ static void cedrus_h265_stop(struct cedrus_ctx *ctx)
>  		if (!vb)
>  			continue;
>  
> -		buf = vb2_to_cedrus_buffer(vb);
> -
> -		if (buf->codec.h265.mv_col_buf_size > 0) {
> -			dma_free_attrs(dev->dev,
> -				       buf->codec.h265.mv_col_buf_size,
> -				       buf->codec.h265.mv_col_buf,
> -				       buf->codec.h265.mv_col_buf_dma,
> -				       DMA_ATTR_NO_KERNEL_MAPPING);
> -
> -			buf->codec.h265.mv_col_buf_size = 0;
> -		}
> +		cedrus_h265_buf_cleanup(ctx, vb2_to_cedrus_buffer(vb));
>  	}
>  
>  	dma_free_attrs(dev->dev, CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE,
> @@ -918,6 +921,7 @@ struct cedrus_dec_ops cedrus_dec_ops_h265 = {
>  	.setup		= cedrus_h265_setup,
>  	.start		= cedrus_h265_start,
>  	.stop		= cedrus_h265_stop,
> +	.buf_cleanup	= cedrus_h265_buf_cleanup,
>  	.trigger	= cedrus_h265_trigger,
>  	.extra_cap_size	= cedrus_h265_extra_cap_size,
>  };
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index ad4ec3490775..adc7f255d7cf 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -560,6 +560,15 @@ static void cedrus_stop_streaming(struct vb2_queue *vq)
>  	cedrus_queue_cleanup(vq, VB2_BUF_STATE_ERROR);
>  }
>  
> +static void cedrus_buf_cleanup(struct vb2_buffer *vb)
> +{
> +	struct cedrus_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> +	if (V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type) &&
> +	    ctx->current_codec && ctx->current_codec->buf_cleanup)
> +		ctx->current_codec->buf_cleanup(ctx, vb2_to_cedrus_buffer(vb));
> +}
> +
>  static void cedrus_buf_queue(struct vb2_buffer *vb)
>  {
>  	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> @@ -579,6 +588,7 @@ static const struct vb2_ops cedrus_qops = {
>  	.queue_setup		= cedrus_queue_setup,
>  	.buf_prepare		= cedrus_buf_prepare,
>  	.buf_queue		= cedrus_buf_queue,
> +	.buf_cleanup		= cedrus_buf_cleanup,
>  	.buf_out_validate	= cedrus_buf_out_validate,
>  	.buf_request_complete	= cedrus_buf_request_complete,
>  	.start_streaming	= cedrus_start_streaming,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 02/10] staging: media: cedrus: reject too large H.264/HEVC SPS

[Nicolas Dufresne]

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

Le lundi 02 novembre 2026 à 18:02 +0100, Jernej Skrabec a écrit :
> The hardware decodes whatever picture size the SPS describes. An SPS
> beyond CEDRUS_MAX_WIDTH/HEIGHT, or beyond the capture buffers once they
> are allocated, makes the hardware overrun memory and hang. On H616, the
> JCT-VC PICSIZE_A..D streams (1056x8440, 8440x1056, 528x4216 and
> 4216x528) leave the hardware wedged, so that every following job times
> out until reboot.
> 
> Validate the SPS picture size in try_ctrl. Move the size limits to
> cedrus_video.h for that.
> 
> Fixes: 6eb9b758e307 ("media: cedrus: Add H264 decoding support")
> Fixes: 86caab29da78 ("media: cedrus: Add HEVC/H.265 decoding support")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.c   | 40 ++++++++++++++++++-
>  .../staging/media/sunxi/cedrus/cedrus_video.c |  5 ---
>  .../staging/media/sunxi/cedrus/cedrus_video.h |  5 +++
>  3 files changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index c3331b4f3f02..45deebddb630 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -28,10 +28,38 @@
>  #include "cedrus_dec.h"
>  #include "cedrus_hw.h"
>  
> +/*
> + * The hardware decodes whatever picture size the bitstream headers describe.
> + * Reject sizes beyond the hardware limits, or beyond the capture buffers once
> + * they are allocated, instead of letting the hardware overrun memory and hang.
> + */
> +static int cedrus_check_pic_size(struct cedrus_ctx *ctx, u32 width, u32 height)
> +{
> +	struct vb2_queue *vq;
> +
> +	if (width > CEDRUS_MAX_WIDTH || height > CEDRUS_MAX_HEIGHT)
> +		return -EINVAL;
> +
> +	/*
> +	 * The hardware decodes whole 16x16 macroblocks, and all capture
> +	 * formats pad the buffers to at least 16 pixels in both directions.
> +	 */
> +	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +	if (vb2_is_busy(vq) &&
> +	    (width > ALIGN(ctx->src_fmt.width, 16) ||
> +	     height > ALIGN(ctx->src_fmt.height, 16)))
> +		return -EINVAL;

If my memory is correct, Hans rejected this kind of construct in try_ctrl,
because if you are only doing try() it shouldn't fail like this. Instead we can
validate in the real s_ctrl, but I believe the downside is that it makes the
error asynchronous when a request is used (perhaps that's actually wanted ?)

Nicolas

> +
> +	return 0;
> +}
> +
>  static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
>  {
> +	struct cedrus_ctx *ctx = container_of(ctrl->handler, struct cedrus_ctx, hdl);
> +
>  	if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
>  		const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
> +		u32 height;
>  
>  		if (sps->chroma_format_idc != 1)
>  			/* Only 4:2:0 is supported */
> @@ -42,9 +70,15 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
>  		if (sps->bit_depth_luma_minus8 != 0)
>  			/* Only 8-bit is supported */
>  			return -EINVAL;
> +		height = (sps->pic_height_in_map_units_minus1 + 1) * 16;
> +		if (!(sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY))
> +			height *= 2;
> +		if (cedrus_check_pic_size(ctx,
> +					  (sps->pic_width_in_mbs_minus1 + 1) * 16,
> +					  height))
> +			return -EINVAL;
>  	} else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) {
>  		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
> -		struct cedrus_ctx *ctx = container_of(ctrl->handler, struct cedrus_ctx, hdl);
>  		unsigned int bit_depth, max_depth;
>  		struct vb2_queue *vq;
>  
> @@ -52,6 +86,10 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
>  			/* Only 4:2:0 is supported */
>  			return -EINVAL;
>  
> +		if (cedrus_check_pic_size(ctx, sps->pic_width_in_luma_samples,
> +					  sps->pic_height_in_luma_samples))
> +			return -EINVAL;
> +
>  		bit_depth = max(sps->bit_depth_luma_minus8,
>  				sps->bit_depth_chroma_minus8) + 8;
>  
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index adc7f255d7cf..2521315abdb7 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -29,11 +29,6 @@
>  #define CEDRUS_DECODE_SRC	BIT(0)
>  #define CEDRUS_DECODE_DST	BIT(1)
>  
> -#define CEDRUS_MIN_WIDTH	16U
> -#define CEDRUS_MIN_HEIGHT	16U
> -#define CEDRUS_MAX_WIDTH	4096U
> -#define CEDRUS_MAX_HEIGHT	2304U
> -
>  static struct cedrus_format cedrus_formats[] = {
>  	{
>  		.pixelformat	= V4L2_PIX_FMT_MPEG2_SLICE,
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.h b/drivers/staging/media/sunxi/cedrus/cedrus_video.h
> index 8e1afc16a6a1..367e52158f01 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.h
> @@ -22,6 +22,11 @@ struct cedrus_format {
>  	unsigned int	capabilities;
>  };
>  
> +#define CEDRUS_MIN_WIDTH	16U
> +#define CEDRUS_MIN_HEIGHT	16U
> +#define CEDRUS_MAX_WIDTH	4096U
> +#define CEDRUS_MAX_HEIGHT	2304U
> +
>  extern const struct v4l2_ioctl_ops cedrus_ioctl_ops;
>  
>  int cedrus_queue_init(void *priv, struct vb2_queue *src_vq,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 03/10] staging: media: cedrus: h264: mark long-term references in the frame info

[Nicolas Dufresne]

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

Hi,

Le lundi 02 novembre 2026 à 18:02 +0100, Jernej Skrabec a écrit :
> The frame info of a reference has a reference type for each field, in
> bits 1:0 for the top field and 5:4 for the bottom one, which is 1 for a
> long-term reference. The driver never sets it, so the hardware handles
> long-term references like short-term ones, also where they differ, like
> in temporal direct and implicit weighted prediction. The vendor library
> sets it.
> 
> This fixes MR8_BT_B and MR9_BT_B of the H.264 conformance suite.
> 
> Fixes: 6eb9b758e307 ("media: cedrus: Add H264 decoding support")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 12 +++++++++++-
>  drivers/staging/media/sunxi/cedrus/cedrus_regs.h |  8 ++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> index 892193c12b2f..a716955655f7 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -69,13 +69,21 @@ static void cedrus_fill_ref_pic(struct cedrus_ctx *ctx,
>  				struct cedrus_buffer *buf,
>  				unsigned int top_field_order_cnt,
>  				unsigned int bottom_field_order_cnt,
> +				bool long_term,
>  				struct cedrus_h264_sram_ref_pic *pic)
>  {
>  	struct vb2_buffer *vbuf = &buf->m2m_buf.vb.vb2_buf;
> +	u32 frame_info, ref_type;
>  
>  	pic->top_field_order_cnt = cpu_to_le32(top_field_order_cnt);
>  	pic->bottom_field_order_cnt = cpu_to_le32(bottom_field_order_cnt);
> -	pic->frame_info = cpu_to_le32(buf->codec.h264.pic_type << 8);
> +
> +	ref_type = long_term ? VE_H264_FRAME_INFO_REF_TYPE_LONG_TERM :
> +			       VE_H264_FRAME_INFO_REF_TYPE_SHORT_TERM;
> +	frame_info = VE_H264_FRAME_INFO_TOP_REF_TYPE(ref_type) |
> +		     VE_H264_FRAME_INFO_BOT_REF_TYPE(ref_type) |
> +		     VE_H264_FRAME_INFO_PIC_TYPE(buf->codec.h264.pic_type);
> +	pic->frame_info = cpu_to_le32(frame_info);
>  
>  	pic->luma_ptr = cpu_to_le32(cedrus_buf_addr(vbuf, &ctx->dst_fmt, 0));
>  	pic->chroma_ptr = cpu_to_le32(cedrus_buf_addr(vbuf, &ctx->dst_fmt, 1));
> @@ -128,6 +136,7 @@ static int cedrus_write_frame_list(struct cedrus_ctx *ctx,
>  		cedrus_fill_ref_pic(ctx, cedrus_buf,
>  				    dpb->top_field_order_cnt,
>  				    dpb->bottom_field_order_cnt,
> +				    dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM,
>  				    &pic_list[position]);
>  	}
>  
> @@ -174,6 +183,7 @@ static int cedrus_write_frame_list(struct cedrus_ctx *ctx,
>  	cedrus_fill_ref_pic(ctx, output_buf,
>  			    decode->top_field_order_cnt,
>  			    decode->bottom_field_order_cnt,
> +			    false,

nit: Not a hard objection, but I really hate boolean parameters. It could be a
simple as:

+			    false, /* short term reference */

Or using an enum. My preference being documented here, its yours to decide.

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

>  			    &pic_list[position]);
>  
>  	cedrus_h264_write_sram(dev, CEDRUS_SRAM_H264_FRAMEBUFFER_LIST,
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
> index 05e6cbc548ab..961fe023bb43 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
> @@ -616,6 +616,14 @@
>  #define VE_AVC_SRAM_PORT_OFFSET		0x2e0
>  #define VE_AVC_SRAM_PORT_DATA		0x2e4
>  
> +/* Frame info word of a frame buffer list entry in the AVC SRAM */
> +#define VE_H264_FRAME_INFO_PIC_TYPE(v)		SHIFT_AND_MASK_BITS(v, 9, 8)
> +#define VE_H264_FRAME_INFO_BOT_REF_TYPE(v)	SHIFT_AND_MASK_BITS(v, 5, 4)
> +#define VE_H264_FRAME_INFO_TOP_REF_TYPE(v)	SHIFT_AND_MASK_BITS(v, 1, 0)
> +
> +#define VE_H264_FRAME_INFO_REF_TYPE_SHORT_TERM	0
> +#define VE_H264_FRAME_INFO_REF_TYPE_LONG_TERM	1
> +
>  #define VE_VP8_PPS			0x214
>  #define VE_VP8_PPS_PIC_TYPE_P_FRAME		BIT(31)
>  #define VE_VP8_PPS_LAST_SHARPNESS_LEVEL(v)	SHIFT_AND_MASK_BITS(v, 30, 28)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 04/10] staging: media: cedrus: mpeg2: use the field height for field pictures

[Nicolas Dufresne]

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

Le lundi 02 novembre 2026 à 18:02 +0100, Jernej Skrabec a écrit :
> A field picture covers only half of the macroblock rows of a frame, but
> the driver programs the coded and the boundary size of the whole frame
> for it, as for a frame picture.
> 
> Program the height of a field for field pictures, like the vendor
> library does.
> 
> Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> index 10e98f08aafc..ca66a3c6b830 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> @@ -58,6 +58,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  	struct vb2_queue *vq;
>  	const u8 *matrix;
>  	unsigned int i;
> +	bool field;
>  	u32 reg;
>  
>  	seq = run->mpeg2.sequence;
> @@ -108,13 +109,21 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  
>  	/* Set frame dimensions. */
>  
> +	/*
> +	 * Field pictures cover half of the frame's macroblock rows; the
> +	 * vendor library halves both the coded and the boundary height.
> +	 */
> +	field = pic->picture_structure != V4L2_MPEG2_PIC_FRAME;
> +
>  	reg = VE_DEC_MPEG_PICCODEDSIZE_WIDTH(seq->horizontal_size);
> -	reg |= VE_DEC_MPEG_PICCODEDSIZE_HEIGHT(seq->vertical_size);
> +	reg |= VE_DEC_MPEG_PICCODEDSIZE_HEIGHT(field ?
> +			ALIGN(seq->vertical_size, 32) / 2 : seq->vertical_size);
>  
>  	cedrus_write(dev, VE_DEC_MPEG_PICCODEDSIZE, reg);
>  
>  	reg = VE_DEC_MPEG_PICBOUNDSIZE_WIDTH(ctx->src_fmt.width);
> -	reg |= VE_DEC_MPEG_PICBOUNDSIZE_HEIGHT(ctx->src_fmt.height);
> +	reg |= VE_DEC_MPEG_PICBOUNDSIZE_HEIGHT(field ? ctx->src_fmt.height / 2 :
> +						   ctx->src_fmt.height);

I'm not entirely sure, but I see:

	fsize->stepwise.step_height = 1;

But I don't think interlaced video can have odd height in the first place. But
it also have an implication here as you are not rounding up to a multiple of 2.
Not sure what is the proper fix, but I have this impression something is
missing.

Nicolas

>  
>  	cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
>  

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 05/10] staging: media: cedrus: mpeg2: derive the field order of field pictures

[Nicolas Dufresne]

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

Le lundi 02 novembre 2026 à 18:02 +0100, Jernej Skrabec a écrit :
> The hardware uses the top field first bit of the picture header to tell
> the first field of a frame from the second one. However,
> top_field_first is always 0 in field pictures, so userspace passes
> V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST cleared, and the second field of
> every top field first pair is predicted from the wrong reference.
> 
> Detect the second field in the driver instead: it has the opposite
> parity of the previous field and is decoded into the same capture
> buffer. This works with unmodified userspace.
> 
> Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Its quite likely other MPEG-2 decoder are affected. Would have been nice to pass
the picture structure explicitly in the decode params in the first place. Not
your fault.

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

> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.h   |  5 +++
>  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 32 +++++++++++++++++--
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 54f867949fba..a523b56a9b8c 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -144,6 +144,11 @@ struct cedrus_ctx {
>  			void		*entry_points_buf;
>  			dma_addr_t	entry_points_buf_addr;
>  		} h265;
> +		struct {
> +			bool		pair_pending;
> +			unsigned int	pair_dst;
> +			u8		pair_structure;
> +		} mpeg2;
>  		struct {
>  			unsigned int	last_frame_p_type;
>  			unsigned int	last_filter_type;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> index ca66a3c6b830..4c8accee5389 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> @@ -58,7 +58,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  	struct vb2_queue *vq;
>  	const u8 *matrix;
>  	unsigned int i;
> -	bool field;
> +	bool field, tff;
>  	u32 reg;
>  
>  	seq = run->mpeg2.sequence;
> @@ -87,6 +87,26 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  		cedrus_write(dev, VE_DEC_MPEG_IQMINPUT, reg);
>  	}
>  
> +	/*
> +	 * top_field_first is always 0 in field pictures, but the hardware
> +	 * uses this bit to tell the first field of a frame from the second.
> +	 * A field is the second one when it has the opposite parity of the
> +	 * previous field and is decoded into the same capture buffer.
> +	 */
> +	if (pic->picture_structure == V4L2_MPEG2_PIC_FRAME) {
> +		tff = !!(pic->flags & V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST);
> +		ctx->codec.mpeg2.pair_pending = false;
> +	} else {
> +		bool second = ctx->codec.mpeg2.pair_pending &&
> +			      ctx->codec.mpeg2.pair_structure != pic->picture_structure &&
> +			      ctx->codec.mpeg2.pair_dst == run->dst->vb2_buf.index;
> +
> +		tff = (pic->picture_structure == V4L2_MPEG2_PIC_TOP_FIELD) != second;
> +		ctx->codec.mpeg2.pair_pending = !second;
> +		ctx->codec.mpeg2.pair_structure = pic->picture_structure;
> +		ctx->codec.mpeg2.pair_dst = run->dst->vb2_buf.index;
> +	}
> +
>  	/* Set MPEG picture header. */
>  
>  	reg = VE_DEC_MPEG_MP12HDR_SLICE_TYPE(pic->picture_coding_type);
> @@ -96,7 +116,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  	reg |= VE_DEC_MPEG_MP12HDR_F_CODE(1, 1, pic->f_code[1][1]);
>  	reg |= VE_DEC_MPEG_MP12HDR_INTRA_DC_PRECISION(pic->intra_dc_precision);
>  	reg |= VE_DEC_MPEG_MP12HDR_INTRA_PICTURE_STRUCTURE(pic->picture_structure);
> -	reg |= VE_DEC_MPEG_MP12HDR_TOP_FIELD_FIRST(pic->flags & V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST);
> +	reg |= VE_DEC_MPEG_MP12HDR_TOP_FIELD_FIRST(tff);
>  	reg |= VE_DEC_MPEG_MP12HDR_FRAME_PRED_FRAME_DCT(pic->flags & V4L2_MPEG2_PIC_FLAG_FRAME_PRED_DCT);
>  	reg |= VE_DEC_MPEG_MP12HDR_CONCEALMENT_MOTION_VECTORS(pic->flags & V4L2_MPEG2_PIC_FLAG_CONCEALMENT_MV);
>  	reg |= VE_DEC_MPEG_MP12HDR_Q_SCALE_TYPE(pic->flags & V4L2_MPEG2_PIC_FLAG_Q_SCALE_TYPE);
> @@ -186,6 +206,13 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  	return 0;
>  }
>  
> +static int cedrus_mpeg2_start(struct cedrus_ctx *ctx)
> +{
> +	memset(&ctx->codec.mpeg2, 0, sizeof(ctx->codec.mpeg2));
> +
> +	return 0;
> +}
> +
>  static void cedrus_mpeg2_trigger(struct cedrus_ctx *ctx)
>  {
>  	struct cedrus_dev *dev = ctx->dev;
> @@ -203,5 +230,6 @@ struct cedrus_dec_ops cedrus_dec_ops_mpeg2 = {
>  	.irq_disable	= cedrus_mpeg2_irq_disable,
>  	.irq_status	= cedrus_mpeg2_irq_status,
>  	.setup		= cedrus_mpeg2_setup,
> +	.start		= cedrus_mpeg2_start,
>  	.trigger	= cedrus_mpeg2_trigger,
>  };

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 06/10] staging: media: cedrus: mpeg2: fix the references of field pictures

[Nicolas Dufresne]

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

Hi,

Le lundi 02 novembre 2026 à 18:02 +0100, Jernej Skrabec a écrit :
> The second field of an I or P frame can be predicted from the first
> field of the same frame, which the hardware takes from the backward
> reference. The driver programs the backward reference passed by
> userspace instead, which doesn't point to the frame being decoded, so
> the second field is predicted from the wrong picture.
> 
> Use the current frame as the backward reference of I and P pictures,
> like the vendor library does. Also fall back to the current frame
> instead of address 0 when a reference buffer is missing, so the
> hardware never reads from an invalid address.
> 
> Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

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

> ---
>  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 29 ++++++++++++++-----
>  1 file changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> index 4c8accee5389..0a2f8e77fb29 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> @@ -56,6 +56,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  	dma_addr_t src_buf_addr, dst_luma_addr, dst_chroma_addr;
>  	struct cedrus_dev *dev = ctx->dev;
>  	struct vb2_queue *vq;
> +	struct vb2_buffer *fwd, *bwd;
>  	const u8 *matrix;
>  	unsigned int i;
>  	bool field, tff;
> @@ -147,15 +148,29 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  
>  	cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
>  
> -	/* Forward and backward prediction reference buffers. */
> +	/*
> +	 * Forward and backward prediction reference buffers. As the vendor
> +	 * library does, I and P pictures use the current frame as backward
> +	 * reference: the second field of a frame predicts from the first one.
> +	 * Missing references also fall back to the current frame.
> +	 */
>  	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
>  
> -	cedrus_write_ref_buf_addr(ctx, vq, pic->forward_ref_ts,
> -				  VE_DEC_MPEG_FWD_REF_LUMA_ADDR,
> -				  VE_DEC_MPEG_FWD_REF_CHROMA_ADDR);
> -	cedrus_write_ref_buf_addr(ctx, vq, pic->backward_ref_ts,
> -				  VE_DEC_MPEG_BWD_REF_LUMA_ADDR,
> -				  VE_DEC_MPEG_BWD_REF_CHROMA_ADDR);
> +	fwd = vb2_find_buffer(vq, pic->forward_ref_ts) ?: &run->dst->vb2_buf;
> +	if (pic->picture_coding_type == V4L2_MPEG2_PIC_CODING_TYPE_B)
> +		bwd = vb2_find_buffer(vq, pic->backward_ref_ts) ?:
> +		      &run->dst->vb2_buf;
> +	else
> +		bwd = &run->dst->vb2_buf;
> +
> +	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR,
> +		     cedrus_dst_buf_addr(ctx, fwd, 0));
> +	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR,
> +		     cedrus_dst_buf_addr(ctx, fwd, 1));
> +	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_LUMA_ADDR,
> +		     cedrus_dst_buf_addr(ctx, bwd, 0));
> +	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_CHROMA_ADDR,
> +		     cedrus_dst_buf_addr(ctx, bwd, 1));
>  
>  	/* Destination luma and chroma buffers. */
>  

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 07/10] staging: media: cedrus: validate VP8 frame size

[Nicolas Dufresne]

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

Hi,

Le lundi 02 novembre 2026 à 18:03 +0100, Jernej Skrabec a écrit :
> A VP8 key frame can change the frame size. If userspace doesn't
> reallocate the capture buffers, the hardware writes and reads past the
> buffers, which shows up as IOMMU page faults and decode timeouts.
> 
> Check the frame size in try_ctrl, like for the H.264 and HEVC SPS.
> 
> Fixes: 3c39a16d30d8 ("media: cedrus: Add support for VP8 decoding")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index 45deebddb630..756243795de8 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -77,6 +77,11 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
>  					  (sps->pic_width_in_mbs_minus1 + 1) * 16,
>  					  height))
>  			return -EINVAL;
> +	} else if (ctrl->id == V4L2_CID_STATELESS_VP8_FRAME) {
> +		const struct v4l2_ctrl_vp8_frame *frame = ctrl->p_new.p_vp8_frame;
> +
> +		if (cedrus_check_pic_size(ctx, frame->width, frame->height))
> +			return -EINVAL;

This is affected by my previous comment, let's discuss over the other report
first.

Nicolas

>  	} else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) {
>  		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
>  		unsigned int bit_depth, max_depth;
> @@ -270,6 +275,7 @@ static const struct cedrus_control cedrus_controls[] = {
>  	{
>  		.cfg = {
>  			.id	= V4L2_CID_STATELESS_VP8_FRAME,
> +			.ops	= &cedrus_ctrl_ops,
>  		},
>  		.capabilities	= CEDRUS_CAPABILITY_VP8_DEC,
>  	},

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 08/10] staging: media: cedrus: vp8: use the vendor probability table size

[Nicolas Dufresne]

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

Hi,

Le lundi 02 novembre 2026 à 18:03 +0100, Jernej Skrabec a écrit :
> The driver allocates the VP8 probability table with 0x2400 bytes, which
> is the size the old Allwinner A10 library uses. All newer vendor
> libraries allocate 0x5400 bytes and never write the area past 0x11b0
> from the CPU. The hardware uses that area as scratch space, most likely
> for the segment map, and its usage grows with the frame size: a
> 3840x2160 stream makes the hardware access memory past 0x2400 and
> causes IOMMU page faults.
> 
> Allocate the table with 0x5400 bytes, like the vendor libraries do.
> 
> Fixes: 3c39a16d30d8 ("media: cedrus: Add support for VP8 decoding")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

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

> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_vp8.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> index 969677a3bbf9..6b00efa77c22 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> @@ -25,7 +25,7 @@
>  #include "cedrus_hw.h"
>  #include "cedrus_regs.h"
>  
> -#define CEDRUS_ENTROPY_PROBS_SIZE 0x2400
> +#define CEDRUS_ENTROPY_PROBS_SIZE 0x5400
>  #define VP8_PROB_HALF 128
>  #define QUANT_DELTA_COUNT 5
>  

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 09/10] staging: media: cedrus: h616: use the 256-bit DDR mode

[Nicolas Dufresne]

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

Hi,

Le lundi 02 novembre 2026 à 18:03 +0100, Jernej Skrabec a écrit :
> With the DDR mode in VE_MODE set to 128-bit, which the driver uses on
> all SoCs, the H616 H.264 engine reads corrupted reference chroma, while
> luma is fine. H.264 and VP8 P frames come out with bit-exact luma, but
> with about 22 dB PSNR chroma. H.265 and MPEG-2 are unaffected.
> 
> The vendor library selects the DDR mode based on the VE IC version. It
> uses the 256-bit mode without the 2 MiB reconstruction write mode on
> the H616, and keeps the 128-bit mode with the 2 MiB reconstruction
> write mode on older SoCs, like the H6. Doing the same on the H616 makes
> all codecs bit-exact (tested with BA_MW_D, CABA2_Sony_E,
> vp80-00-comprehensive-002, AMP_A_Samsung_7 and MEI.stream16).
> 
> Make the VE_MODE DDR settings a variant property and add an H616
> variant, which uses the 256-bit mode. Replace the FIXME about the
> DDR mode with a description of what the vendor library does.
> 
> Fixes: 1f24d4307515 ("staging: media: sunxi: cedrus: add H616 variant")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

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

> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.c    | 14 +++++++++++++-
>  drivers/staging/media/sunxi/cedrus/cedrus.h    |  2 ++
>  drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 14 ++++++++++----
>  3 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index 756243795de8..2bbc79b44f99 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -27,6 +27,7 @@
>  #include "cedrus_video.h"
>  #include "cedrus_dec.h"
>  #include "cedrus_hw.h"
> +#include "cedrus_regs.h"
>  
>  /*
>   * The hardware decodes whatever picture size the bitstream headers describe.
> @@ -697,6 +698,17 @@ static const struct cedrus_variant sun50i_h6_cedrus_variant = {
>  	.mod_rate	= 600000000,
>  };
>  
> +static const struct cedrus_variant sun50i_h616_cedrus_variant = {
> +	.capabilities	= CEDRUS_CAPABILITY_UNTILED |
> +			  CEDRUS_CAPABILITY_MPEG2_DEC |
> +			  CEDRUS_CAPABILITY_H264_DEC |
> +			  CEDRUS_CAPABILITY_H265_DEC |
> +			  CEDRUS_CAPABILITY_H265_10_DEC |
> +			  CEDRUS_CAPABILITY_VP8_DEC,
> +	.mod_rate	= 600000000,
> +	.ve_mode_ddr	= VE_MODE_DDR_MODE_BW_256,
> +};
> +
>  static const struct of_device_id cedrus_dt_match[] = {
>  	{
>  		.compatible = "allwinner,sun4i-a10-video-engine",
> @@ -744,7 +756,7 @@ static const struct of_device_id cedrus_dt_match[] = {
>  	},
>  	{
>  		.compatible = "allwinner,sun50i-h616-video-engine",
> -		.data = &sun50i_h6_cedrus_variant,
> +		.data = &sun50i_h616_cedrus_variant,
>  	},
>  	{ /* sentinel */ }
>  };
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index a523b56a9b8c..d28fe91562d1 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -181,6 +181,7 @@ struct cedrus_dec_ops {
>  struct cedrus_variant {
>  	unsigned int	capabilities;
>  	unsigned int	mod_rate;
> +	u32		ve_mode_ddr;
>  };
>  
>  struct cedrus_dev {
> @@ -204,6 +205,7 @@ struct cedrus_dev {
>  	struct reset_control	*rstc;
>  
>  	unsigned int		capabilities;
> +	u32			ve_mode_ddr;
>  
>  	struct delayed_work	watchdog_work;
>  };
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> index 444fb53878d1..b66d855daaf8 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> @@ -36,11 +36,16 @@ int cedrus_engine_enable(struct cedrus_ctx *ctx)
>  	u32 reg = 0;
>  
>  	/*
> -	 * FIXME: This is only valid on 32-bits DDR's, we should test
> -	 * it on the A13/A33.
> +	 * The DDR mode depends on the SoC. Like the vendor library, use the
> +	 * 128-bit mode with the 2 MiB reconstruction write mode, unless the
> +	 * variant says otherwise.
>  	 */
> -	reg |= VE_MODE_REC_WR_MODE_2MB;
> -	reg |= VE_MODE_DDR_MODE_BW_128;
> +	if (ctx->dev->ve_mode_ddr) {
> +		reg |= ctx->dev->ve_mode_ddr;
> +	} else {
> +		reg |= VE_MODE_REC_WR_MODE_2MB;
> +		reg |= VE_MODE_DDR_MODE_BW_128;
> +	}
>  
>  	switch (ctx->src_fmt.pixelformat) {
>  	case V4L2_PIX_FMT_MPEG2_SLICE:
> @@ -254,6 +259,7 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
>  		return -EINVAL;
>  
>  	dev->capabilities = variant->capabilities;
> +	dev->ve_mode_ddr = variant->ve_mode_ddr;
>  
>  	irq_dec = platform_get_irq(dev->pdev, 0);
>  	if (irq_dec <= 0)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 10/10] staging: media: cedrus: vp8: support frames wider than 2048 pixels

[Nicolas Dufresne]

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

Le lundi 02 novembre 2026 à 18:03 +0100, Jernej Skrabec a écrit :
> Above 2048 pixels, the deblocking and intra prediction row buffers don't
> fit into the internal SRAM anymore. Put them into DRAM like H.264 does.
> The vendor library sizes the deblocking buffer as 24 bytes per column.
> 
> Tested on H616 with 2560x1440 and 3840x2160 libvpx streams, which are
> bit-exact with the FFmpeg software decoder.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

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

> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.h   |  7 ++
>  .../staging/media/sunxi/cedrus/cedrus_vp8.c   | 64 +++++++++++++++++++
>  2 files changed, 71 insertions(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index d28fe91562d1..ca4efa9302fc 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -156,6 +156,13 @@ struct cedrus_ctx {
>  
>  			u8		*entropy_probs_buf;
>  			dma_addr_t	entropy_probs_buf_dma;
> +
> +			void		*deblk_buf;
> +			dma_addr_t	deblk_buf_dma;
> +			ssize_t		deblk_buf_size;
> +			void		*intra_pred_buf;
> +			dma_addr_t	intra_pred_buf_dma;
> +			ssize_t		intra_pred_buf_size;
>  		} vp8;
>  	} codec;
>  };
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> index 6b00efa77c22..954c0ceb98ed 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> @@ -666,6 +666,20 @@ static int cedrus_vp8_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  
>  	cedrus_write(dev, VE_H264_CTRL, VE_H264_CTRL_VP8);
>  
> +	if (ctx->codec.vp8.deblk_buf_size) {
> +		cedrus_write(dev, VE_BUF_CTRL,
> +			     VE_BUF_CTRL_INTRAPRED_MIXED_RAM |
> +			     VE_BUF_CTRL_DBLK_MIXED_RAM);
> +		cedrus_write(dev, VE_DBLK_DRAM_BUF_ADDR,
> +			     ctx->codec.vp8.deblk_buf_dma);
> +		cedrus_write(dev, VE_INTRAPRED_DRAM_BUF_ADDR,
> +			     ctx->codec.vp8.intra_pred_buf_dma);
> +	} else {
> +		cedrus_write(dev, VE_BUF_CTRL,
> +			     VE_BUF_CTRL_INTRAPRED_INT_SRAM |
> +			     VE_BUF_CTRL_DBLK_INT_SRAM);
> +	}
> +
>  	cedrus_vp8_update_probs(slice, ctx->codec.vp8.entropy_probs_buf);
>  
>  	reg = slice->first_part_size * 8;
> @@ -849,7 +863,44 @@ static int cedrus_vp8_start(struct cedrus_ctx *ctx)
>  	memcpy(&ctx->codec.vp8.entropy_probs_buf[2048],
>  	       prob_table_init, sizeof(prob_table_init));
>  
> +	/*
> +	 * Above 2048 pixels the deblocking and intra prediction row buffers
> +	 * no longer fit into the internal SRAM. The vendor library sizes the
> +	 * deblocking buffer as 24 bytes per (32 aligned) column.
> +	 */
> +	if (ctx->src_fmt.width > 2048) {
> +		ctx->codec.vp8.deblk_buf_size =
> +			ALIGN(ctx->src_fmt.width, 32) * 24;
> +		ctx->codec.vp8.deblk_buf =
> +			dma_alloc_attrs(dev->dev, ctx->codec.vp8.deblk_buf_size,
> +					&ctx->codec.vp8.deblk_buf_dma,
> +					GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING);
> +		if (!ctx->codec.vp8.deblk_buf)
> +			goto err_probs;
> +
> +		ctx->codec.vp8.intra_pred_buf_size =
> +			ALIGN(ctx->src_fmt.width, 64) * 5 * 2;
> +		ctx->codec.vp8.intra_pred_buf =
> +			dma_alloc_attrs(dev->dev,
> +					ctx->codec.vp8.intra_pred_buf_size,
> +					&ctx->codec.vp8.intra_pred_buf_dma,
> +					GFP_KERNEL, DMA_ATTR_NO_KERNEL_MAPPING);
> +		if (!ctx->codec.vp8.intra_pred_buf)
> +			goto err_deblk;
> +	}
> +
>  	return 0;
> +
> +err_deblk:
> +	dma_free_attrs(dev->dev, ctx->codec.vp8.deblk_buf_size,
> +		       ctx->codec.vp8.deblk_buf, ctx->codec.vp8.deblk_buf_dma,
> +		       DMA_ATTR_NO_KERNEL_MAPPING);
> +	ctx->codec.vp8.deblk_buf_size = 0;
> +err_probs:
> +	dma_free_coherent(dev->dev, CEDRUS_ENTROPY_PROBS_SIZE,
> +			  ctx->codec.vp8.entropy_probs_buf,
> +			  ctx->codec.vp8.entropy_probs_buf_dma);
> +	return -ENOMEM;
>  }
>  
>  static void cedrus_vp8_stop(struct cedrus_ctx *ctx)
> @@ -861,6 +912,19 @@ static void cedrus_vp8_stop(struct cedrus_ctx *ctx)
>  	dma_free_coherent(dev->dev, CEDRUS_ENTROPY_PROBS_SIZE,
>  			  ctx->codec.vp8.entropy_probs_buf,
>  			  ctx->codec.vp8.entropy_probs_buf_dma);
> +
> +	if (ctx->codec.vp8.deblk_buf_size) {
> +		dma_free_attrs(dev->dev, ctx->codec.vp8.deblk_buf_size,
> +			       ctx->codec.vp8.deblk_buf,
> +			       ctx->codec.vp8.deblk_buf_dma,
> +			       DMA_ATTR_NO_KERNEL_MAPPING);
> +		dma_free_attrs(dev->dev, ctx->codec.vp8.intra_pred_buf_size,
> +			       ctx->codec.vp8.intra_pred_buf,
> +			       ctx->codec.vp8.intra_pred_buf_dma,
> +			       DMA_ATTR_NO_KERNEL_MAPPING);
> +		ctx->codec.vp8.deblk_buf_size = 0;
> +		ctx->codec.vp8.intra_pred_buf_size = 0;
> +	}
>  }
>  
>  static void cedrus_vp8_trigger(struct cedrus_ctx *ctx)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]