[PATCH v2 0/6] media: verisilicon: Fix MPEG-2 field pictures, 4K VP8 and the AV1 tile buffers

[PATCH v2 0/6] media: verisilicon: Fix MPEG-2 field pictures, 4K VP8 and the AV1 tile buffers

[Arun Iyengar via B4 Relay]

Fixes for the Hantro decoders on Rockchip SoCs, found while running
conformance streams and v4l2-compliance on the RK3588.

Patches 1 and 2 make field-coded MPEG-2 decode on the G1 and VPU2: the
driver refused V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF for MPEG-2, so the
second field of a frame could not land in the same CAPTURE buffer as the
first one, and once it can, the second field was identified with
top_field_first, which is always 0 in field pictures, so it predicted
from the wrong field. None of the field-picture streams of the ISO/IEC
13818-4 main-profile suite decoded before; they decode now.

Patch 3 stops advertising VP8 above 1080p on the VDPU2 and RK3399
tables, whose only CAPTURE format stops at 1920x1088, so that a 4K VP8
stream is refused instead of mis-decoded.

Patch 4 fixes the reuse check of the VPU981 AV1 tile column buffers,
which only compared the deblocking data buffer although the five buffers
scale differently with the frame height, bit depth and number of tile
columns, and clears the sizes when the buffers are freed.

Patches 5 and 6 reject malformed AV1 frames before the decoder is
programmed with them. With no tile columns the tile column buffers are
sized to zero and the decoder runs without them (patch 5). A frame that
is larger than the CAPTURE format does not fit in the buffer the decoder
writes it to, whose size and layout come from the format, and a frame
height beyond the limit of the decoder also wraps the 32-bit size
computation of the tile column buffers, which leaves them too small for
the height programmed into the hardware (patch 6). Both are rejected at
prepare time.

Tested on an Orange Pi 5 (RK3588S). Patches 1 and 2 were verified with
the ISO/IEC 13818-4 main-profile conformance streams through GStreamer's
v4l2slmpeg2dec on the VPU121, compared with FFmpeg: the field-picture
streams (sony-ct1..4, tcela-8/9/10, hhi_burst_short/long) went from
rejected to 53-67 dB PSNR, and the 26 frame-picture streams are
unchanged. Patch 3: the VPU121 decoder lists VP8F frame sizes from 48x48
to 1920x1088, S_FMT of a 3840x2160 VP8 OUTPUT format is clamped to
1920x1088, a 1080p VP8 clip still decodes bit-exact against libvpx, and
the 4K clip is now refused instead of decoded into 1920x1088 buffers.
Patch 4: one v4l2slav1dec context decoding a 1920x1080 10-bit
4-tile-column stream and then an 8-bit 5-column one (and 2048x128 with 4
then 8 columns) is bit-exact against libdav1d, 32/32 frames each, and a
kprobe trace of the DMA allocations shows the tile buffers reallocated
when the second stream needs larger ones.

The new rejects of patches 5 and 6 were tested on the board with v2:
requests with a no-tile AV1 frame, with a frame height that wraps, and
with a frame that is wider, taller or, through superres, upscaled wider
than the 1920x1088 CAPTURE format (3840x2160 among them) are each not
decoded and both of their buffers come back with V4L2_BUF_FLAG_ERROR,
and a frame of the format size and a smaller one after them are still
accepted. Four runs of the fluster AV1 conformance suite give 203/242
each, failing the same 39 vectors as an earlier run on the same board;
the streams that change the frame size and the ones with spatial layers
still decode. In a fifth run av1-1-b8-06-mfmv also failed, with one
decoder timeout and no reject; it passed in the other four runs. The
board kernel is 7.3-based and also carries a fix for the ->run() error
paths (the AV1 one finishes a refused job twice), which Sascha Hauer's
series "media: verisilicon: Fix error paths in hantro drivers" addresses
upstream [1].

[1] https://lore.kernel.org/all/20260824-rk3588-jpegdec-v1-0-180a30a2852d@pengutronix.de/

This series was developed with Claude (Anthropic) as a coding assistant:
it wrote the code and the commit messages, and I reviewed and tested
every change as described above.

Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
Changes in v2:
- New patch 5: reject an AV1 frame with no tile columns, which would
  otherwise size the tile column buffers to zero (Sashiko: AV1
  tile_cols == 0)
- New patch 6: reject an AV1 frame larger than the CAPTURE format, which
  does not fit in the buffers of the decoder and whose height can
  otherwise wrap the 32-bit size computation of the tile column buffers
  (Sashiko: u32 overflow from frame_height_minus_1)
- Patches 1-4 unchanged
- Link to v1: https://patch.msgid.link/20261110-hantro-codec-fixes-v1-0-0fd5372ce831@gmail.com

---
Arun Iyengar (6):
      media: verisilicon: Support holding the CAPTURE buffer for MPEG-2
      media: verisilicon: Find the second MPEG-2 field without top_field_first
      media: verisilicon: Don't offer VP8 above 1080p on the Rockchip VDPU2
      media: verisilicon: Check every VPU981 AV1 tile column buffer before reusing them
      media: verisilicon: Reject AV1 frames without tiles
      media: verisilicon: Reject AV1 frames larger than the CAPTURE format

 .../platform/verisilicon/hantro_g1_mpeg2_dec.c     |  8 ++--
 drivers/media/platform/verisilicon/hantro_hw.h     |  8 ++++
 drivers/media/platform/verisilicon/hantro_mpeg2.c  | 31 +++++++++++++
 drivers/media/platform/verisilicon/hantro_v4l2.c   |  2 +-
 .../verisilicon/rockchip_vpu2_hw_mpeg2_dec.c       |  8 ++--
 .../verisilicon/rockchip_vpu981_hw_av1_dec.c       | 53 +++++++++++++++++++---
 .../media/platform/verisilicon/rockchip_vpu_hw.c   |  8 ++--
 7 files changed, 98 insertions(+), 20 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261110-hantro-codec-fixes-59d42b5609c4

Best regards,
--  
Arun Iyengar <aiyengar1990@gmail.com>

[PATCH v2 1/6] media: verisilicon: Support holding the CAPTURE buffer for MPEG-2

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

An MPEG-2 frame can be coded as two field pictures, each decoded by its
own request. Both fields have to land in the same CAPTURE buffer: the
G1 and VPU2 back-ends already write a bottom field one line into the
destination buffer and use the current buffer as the reference for the
second field of a P picture, so the second field must be decoded into
the buffer that holds the first one.

Userspace asks for that with V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF, which
the driver only allows for H.264. GStreamer's v4l2slmpeg2dec sets it on
the first field of every field-coded frame, so queueing that request
fails ("Driver does not support holding capture buffer") and decoding
stops. On RK3588 none of the field-picture streams of the ISO/IEC
13818-4 main-profile conformance suite (sony-ct1..4, tcela-8/9/10/16/19,
hhi_burst_*) decoded.

Allow holding the CAPTURE buffer for MPEG-2 too. Job completion goes
through v4l2_m2m_buf_done_and_job_finish(), which already keeps a held
CAPTURE buffer queued until the last field has been decoded.

Fixes: 340ce50f75a6 ("media: hantro: Enable HOLD_CAPTURE_BUF for H.264")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/media/platform/verisilicon/hantro_v4l2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index 83af9fa1ce94..822c4b371671 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -513,13 +513,13 @@ hantro_update_requires_hold_capture_buf(struct hantro_ctx *ctx, u32 fourcc)
 
 	switch (fourcc) {
 	case V4L2_PIX_FMT_JPEG:
-	case V4L2_PIX_FMT_MPEG2_SLICE:
 	case V4L2_PIX_FMT_VP8_FRAME:
 	case V4L2_PIX_FMT_HEVC_SLICE:
 	case V4L2_PIX_FMT_VP9_FRAME:
 		vq->subsystem_flags &= ~(VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF);
 		break;
 	case V4L2_PIX_FMT_H264_SLICE:
+	case V4L2_PIX_FMT_MPEG2_SLICE:
 		vq->subsystem_flags |= VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF;
 		break;
 	default:

-- 
2.55.0

[PATCH v2 2/6] media: verisilicon: Find the second MPEG-2 field without top_field_first

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

The second field of a field-coded P frame can predict from the first
field of the same frame, which is already in the destination buffer. The
G1 and VPU2 back-ends then point the reference of that parity at the
current buffer, and both back-ends tell the first field from the second
one with V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST: a top field is the first
field if the flag is set, a bottom field if it is not.

But top_field_first is always 0 in a field picture (ISO/IEC 13818-2,
6.3.10: "In a field picture top_field_first shall have the value '0'").
So the top field of every top-field-first frame is taken as a second
field, and predicts from the not-yet-decoded bottom field of the buffer,
while the bottom field is taken as a first field and loses its
reference to the top field. The error grows over the GOP: the field
picture conformance streams of ISO/IEC 13818-4 decoded at 6-25 dB
against FFmpeg (tcela-8, tcela-9, tcela-10, and sony-ct1/2/4 once they
switch to field pictures).

The uAPI has no flag for this, but the second field is the one of
opposite parity that follows the first with the same OUTPUT timestamp,
the two fields being decoded into one CAPTURE buffer held with
V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF. Track that in the context and use
it in both back-ends. This also replaces the VPU2 back-end's test of
V4L2_MPEG2_PIC_TOP_FIELD, a picture_structure value, against the flags.

Fixes: ceaac6dc5b7a ("media: rockchip/vpu: Add support for MPEG-2 decoding on RK3288")
Fixes: 879dee56a40c ("media: rockchip/vpu: Add MPEG2 decoding support to RK3399")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 .../platform/verisilicon/hantro_g1_mpeg2_dec.c     |  8 +++---
 drivers/media/platform/verisilicon/hantro_hw.h     |  8 ++++++
 drivers/media/platform/verisilicon/hantro_mpeg2.c  | 31 ++++++++++++++++++++++
 .../verisilicon/rockchip_vpu2_hw_mpeg2_dec.c       |  8 +++---
 4 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_g1_mpeg2_dec.c b/drivers/media/platform/verisilicon/hantro_g1_mpeg2_dec.c
index 6a942f4b8a2d..5f1ce21fa6e3 100644
--- a/drivers/media/platform/verisilicon/hantro_g1_mpeg2_dec.c
+++ b/drivers/media/platform/verisilicon/hantro_g1_mpeg2_dec.c
@@ -97,6 +97,9 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
 {
 	dma_addr_t forward_addr = 0, backward_addr = 0;
 	dma_addr_t current_addr, addr;
+	bool second_field;
+
+	second_field = hantro_mpeg2_dec_second_field(ctx, pic, src_buf->timestamp);
 
 	switch (pic->picture_coding_type) {
 	case V4L2_MPEG2_PIC_CODING_TYPE_B:
@@ -126,10 +129,7 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
 	/* Set forward ref frame (top/bottom field) */
 	if (pic->picture_structure == V4L2_MPEG2_PIC_FRAME ||
 	    pic->picture_coding_type == V4L2_MPEG2_PIC_CODING_TYPE_B ||
-	    (pic->picture_structure == V4L2_MPEG2_PIC_TOP_FIELD &&
-	     pic->flags & V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST) ||
-	    (pic->picture_structure == V4L2_MPEG2_PIC_BOTTOM_FIELD &&
-	     !(pic->flags & V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST))) {
+	    !second_field) {
 		vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER0_BASE);
 		vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER1_BASE);
 	} else if (pic->picture_structure == V4L2_MPEG2_PIC_TOP_FIELD) {
diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
index 13e573f1f19d..303e42b08c40 100644
--- a/drivers/media/platform/verisilicon/hantro_hw.h
+++ b/drivers/media/platform/verisilicon/hantro_hw.h
@@ -171,9 +171,14 @@ struct hantro_hevc_dec_hw_ctx {
  * struct hantro_mpeg2_dec_hw_ctx
  *
  * @qtable:		Quantization table
+ * @field_ts:		OUTPUT timestamp of the last field picture decoded, if it
+ *			was the first field of a frame
+ * @field_structure:	picture_structure of that field, 0 if there is none
  */
 struct hantro_mpeg2_dec_hw_ctx {
 	struct hantro_aux_buf qtable;
+	u64 field_ts;
+	u8 field_structure;
 };
 
 /**
@@ -567,6 +572,9 @@ int hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx);
 int rockchip_vpu2_mpeg2_dec_run(struct hantro_ctx *ctx);
 void hantro_mpeg2_dec_copy_qtable(u8 *qtable,
 				  const struct v4l2_ctrl_mpeg2_quantisation *ctrl);
+bool hantro_mpeg2_dec_second_field(struct hantro_ctx *ctx,
+				   const struct v4l2_ctrl_mpeg2_picture *pic,
+				   u64 timestamp);
 int hantro_mpeg2_dec_init(struct hantro_ctx *ctx);
 void hantro_mpeg2_dec_exit(struct hantro_ctx *ctx);
 
diff --git a/drivers/media/platform/verisilicon/hantro_mpeg2.c b/drivers/media/platform/verisilicon/hantro_mpeg2.c
index 04e545eb0a83..e5d70c64d7a7 100644
--- a/drivers/media/platform/verisilicon/hantro_mpeg2.c
+++ b/drivers/media/platform/verisilicon/hantro_mpeg2.c
@@ -35,10 +35,41 @@ void hantro_mpeg2_dec_copy_qtable(u8 *qtable,
 	}
 }
 
+/*
+ * Tell whether a field picture is the second field of its frame, which
+ * decides where its references are: the second field can predict from the
+ * first one, already decoded into the same buffer. top_field_first can't
+ * tell, it is always 0 in field pictures (ISO/IEC 13818-2, 6.3.10). The
+ * second field is the field of opposite parity decoded right after the
+ * first one, with the same OUTPUT timestamp.
+ */
+bool hantro_mpeg2_dec_second_field(struct hantro_ctx *ctx,
+				   const struct v4l2_ctrl_mpeg2_picture *pic,
+				   u64 timestamp)
+{
+	struct hantro_mpeg2_dec_hw_ctx *mpeg2 = &ctx->mpeg2_dec;
+	bool second;
+
+	if (pic->picture_structure == V4L2_MPEG2_PIC_FRAME) {
+		mpeg2->field_structure = 0;
+		return false;
+	}
+
+	second = mpeg2->field_structure &&
+		 mpeg2->field_structure != pic->picture_structure &&
+		 mpeg2->field_ts == timestamp;
+
+	mpeg2->field_structure = second ? 0 : pic->picture_structure;
+	mpeg2->field_ts = timestamp;
+
+	return second;
+}
+
 int hantro_mpeg2_dec_init(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
 
+	ctx->mpeg2_dec.field_structure = 0;
 	ctx->mpeg2_dec.qtable.size = ARRAY_SIZE(zigzag) * 4;
 	ctx->mpeg2_dec.qtable.cpu =
 		dma_alloc_coherent(vpu->dev,
diff --git a/drivers/media/platform/verisilicon/rockchip_vpu2_hw_mpeg2_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_mpeg2_dec.c
index 50a3a3eeaa00..57907dd5ea9c 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu2_hw_mpeg2_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_mpeg2_dec.c
@@ -100,6 +100,9 @@ rockchip_vpu2_mpeg2_dec_set_buffers(struct hantro_dev *vpu,
 {
 	dma_addr_t forward_addr = 0, backward_addr = 0;
 	dma_addr_t current_addr, addr;
+	bool second_field;
+
+	second_field = hantro_mpeg2_dec_second_field(ctx, pic, src_buf->timestamp);
 
 	switch (pic->picture_coding_type) {
 	case V4L2_MPEG2_PIC_CODING_TYPE_B:
@@ -129,10 +132,7 @@ rockchip_vpu2_mpeg2_dec_set_buffers(struct hantro_dev *vpu,
 	/* Set forward ref frame (top/bottom field) */
 	if (pic->picture_structure == V4L2_MPEG2_PIC_FRAME ||
 	    pic->picture_coding_type == V4L2_MPEG2_PIC_CODING_TYPE_B ||
-	    (pic->picture_structure == V4L2_MPEG2_PIC_TOP_FIELD &&
-	     pic->flags & V4L2_MPEG2_PIC_TOP_FIELD) ||
-	    (pic->picture_structure == V4L2_MPEG2_PIC_BOTTOM_FIELD &&
-	     !(pic->flags & V4L2_MPEG2_PIC_TOP_FIELD))) {
+	    !second_field) {
 		vdpu_write_relaxed(vpu, forward_addr, VDPU_REG_REFER0_BASE);
 		vdpu_write_relaxed(vpu, forward_addr, VDPU_REG_REFER1_BASE);
 	} else if (pic->picture_structure == V4L2_MPEG2_PIC_TOP_FIELD) {

-- 
2.55.0

[PATCH v2 3/6] media: verisilicon: Don't offer VP8 above 1080p on the Rockchip VDPU2

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

The VDPU2 decoder format tables (RK3328, RK3568, RK3588 VPU121) and the
RK3399 one let the VP8 OUTPUT format go up to 3840x2160, but their only
CAPTURE format, NV12, stops at 1920x1088, and the CAPTURE size is
clamped to it. A 4K VP8 stream is therefore accepted on OUTPUT and then
decoded into a 1920x1088 buffer: on RK3588 GStreamer v4l2slvp8dec runs
into "gst_video_frame_map_id: assertion 'info->width <= meta->width'
failed" for every frame of a 3840x2160 stream and none of the frames
match FFmpeg.

Nothing above 1080p can be decoded with these tables, so don't
advertise it: limit VP8 to the 1920x1088 of the other codecs and of the
CAPTURE format. Userspace that checks VIDIOC_ENUM_FRAMESIZES, and the
S_FMT clamping, then know that a 4K VP8 stream needs another decoder.

Fixes: 73fb5bfa53e9 ("media: hantro: Support RK3399 VP8 decoding")
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/media/platform/verisilicon/rockchip_vpu_hw.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
index 02673be9878e..2b6242c1fb86 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
@@ -274,10 +274,10 @@ static const struct hantro_fmt rockchip_vdpu2_dec_fmts[] = {
 		.max_depth = 2,
 		.frmsize = {
 			.min_width = FMT_MIN_WIDTH,
-			.max_width = FMT_UHD_WIDTH,
+			.max_width = FMT_FHD_WIDTH,
 			.step_width = MB_DIM,
 			.min_height = FMT_MIN_HEIGHT,
-			.max_height = FMT_UHD_HEIGHT,
+			.max_height = FMT_FHD_HEIGHT,
 			.step_height = MB_DIM,
 		},
 	},
@@ -315,10 +315,10 @@ static const struct hantro_fmt rk3399_vpu_dec_fmts[] = {
 		.max_depth = 2,
 		.frmsize = {
 			.min_width = FMT_MIN_WIDTH,
-			.max_width = FMT_UHD_WIDTH,
+			.max_width = FMT_FHD_WIDTH,
 			.step_width = MB_DIM,
 			.min_height = FMT_MIN_HEIGHT,
-			.max_height = FMT_UHD_HEIGHT,
+			.max_height = FMT_FHD_HEIGHT,
 			.step_height = MB_DIM,
 		},
 	},

-- 
2.55.0

[PATCH v2 4/6] media: verisilicon: Check every VPU981 AV1 tile column buffer before reusing them

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

rockchip_vpu981_av1_dec_tiles_reallocate() keeps the five tile column
buffers (deblocking data and control, CDEF, super-res, loop restoration)
when the deblocking data buffer is big enough for the current frame. The
buffers don't scale the same way: the deblocking data one grows with the
frame height and the bit depth, the super-res one only with the number of
64 pixel rows, and the loop restoration one with the number of
restoration stripes, which is one more than the number of superblock
rows. A frame can therefore fit in the old deblocking data buffer and
still need a larger buffer of another kind. For example, after a 10-bit
1088 line frame with 4 tile columns, an 8-bit frame of the same height
with 5 tile columns needs 65280 bytes of deblocking data (65536
allocated) but 367360 bytes of super-res column data (293888 allocated);
an 8-bit 128 line frame with 4 columns followed by a 64 line frame with 8
columns has the same problem with the loop restoration buffer. The
decoder then writes past the end of the smaller buffer.

The sizes are also not reset when the buffers are freed, so after a
failed allocation the stale size of the deblocking data buffer lets the
next frame skip the allocation and run with NULL buffers.

Compare each buffer with the size the current frame needs and clear the
sizes together with the buffers.

Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 .../verisilicon/rockchip_vpu981_hw_av1_dec.c       | 34 +++++++++++++++++-----
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
index 2a145433e6d1..e817c91224d3 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
@@ -236,27 +236,32 @@ static void rockchip_vpu981_av1_dec_tiles_free(struct hantro_ctx *ctx)
 				  av1_dec->db_data_col.cpu,
 				  av1_dec->db_data_col.dma);
 	av1_dec->db_data_col.cpu = NULL;
+	av1_dec->db_data_col.size = 0;
 
 	if (av1_dec->db_ctrl_col.cpu)
 		dma_free_coherent(vpu->dev, av1_dec->db_ctrl_col.size,
 				  av1_dec->db_ctrl_col.cpu,
 				  av1_dec->db_ctrl_col.dma);
 	av1_dec->db_ctrl_col.cpu = NULL;
+	av1_dec->db_ctrl_col.size = 0;
 
 	if (av1_dec->cdef_col.cpu)
 		dma_free_coherent(vpu->dev, av1_dec->cdef_col.size,
 				  av1_dec->cdef_col.cpu, av1_dec->cdef_col.dma);
 	av1_dec->cdef_col.cpu = NULL;
+	av1_dec->cdef_col.size = 0;
 
 	if (av1_dec->sr_col.cpu)
 		dma_free_coherent(vpu->dev, av1_dec->sr_col.size,
 				  av1_dec->sr_col.cpu, av1_dec->sr_col.dma);
 	av1_dec->sr_col.cpu = NULL;
+	av1_dec->sr_col.size = 0;
 
 	if (av1_dec->lr_col.cpu)
 		dma_free_coherent(vpu->dev, av1_dec->lr_col.size,
 				  av1_dec->lr_col.cpu, av1_dec->lr_col.dma);
 	av1_dec->lr_col.cpu = NULL;
+	av1_dec->lr_col.size = 0;
 }
 
 static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
@@ -269,15 +274,30 @@ static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
 	unsigned int height = ALIGN(ctrls->frame->frame_height_minus_1 + 1, 64);
 	unsigned int height_in_sb = height / 64;
 	unsigned int stripe_num = ((height + 8) + 63) / 64;
+	size_t db_data_size, db_ctrl_size, cdef_size, sr_size, lr_size;
 	size_t size;
 
-	if (av1_dec->db_data_col.size >=
-	    ALIGN(height * 12 * ctx->bit_depth / 8, 128) * num_tile_cols)
+	db_data_size = ALIGN(height * 12 * ctx->bit_depth / 8, 128) * num_tile_cols;
+	db_ctrl_size = ALIGN(height * 2 * 16 / 4, 128) * num_tile_cols;
+	cdef_size = ALIGN(height_in_sb * 44 * ctx->bit_depth * 16 / 8, 128) * num_tile_cols;
+	sr_size = ALIGN(height_in_sb * (3040 + 1280), 128) * num_tile_cols;
+	lr_size = ALIGN(stripe_num * 1536 * ctx->bit_depth / 8, 128) * num_tile_cols;
+
+	/*
+	 * The five buffers scale differently with the frame height, the
+	 * number of tile columns and the bit depth, so each of them has to be
+	 * big enough for the current frame, not only the deblocking data one.
+	 */
+	if (av1_dec->db_data_col.size >= db_data_size &&
+	    av1_dec->db_ctrl_col.size >= db_ctrl_size &&
+	    av1_dec->cdef_col.size >= cdef_size &&
+	    av1_dec->sr_col.size >= sr_size &&
+	    av1_dec->lr_col.size >= lr_size)
 		return 0;
 
 	rockchip_vpu981_av1_dec_tiles_free(ctx);
 
-	size = ALIGN(height * 12 * ctx->bit_depth / 8, 128) * num_tile_cols;
+	size = db_data_size;
 	av1_dec->db_data_col.cpu = dma_alloc_coherent(vpu->dev, size,
 						      &av1_dec->db_data_col.dma,
 						      GFP_KERNEL);
@@ -285,7 +305,7 @@ static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
 		goto buffer_allocation_error;
 	av1_dec->db_data_col.size = size;
 
-	size = ALIGN(height * 2 * 16 / 4, 128) * num_tile_cols;
+	size = db_ctrl_size;
 	av1_dec->db_ctrl_col.cpu = dma_alloc_coherent(vpu->dev, size,
 						      &av1_dec->db_ctrl_col.dma,
 						      GFP_KERNEL);
@@ -293,7 +313,7 @@ static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
 		goto buffer_allocation_error;
 	av1_dec->db_ctrl_col.size = size;
 
-	size = ALIGN(height_in_sb * 44 * ctx->bit_depth * 16 / 8, 128) * num_tile_cols;
+	size = cdef_size;
 	av1_dec->cdef_col.cpu = dma_alloc_coherent(vpu->dev, size,
 						   &av1_dec->cdef_col.dma,
 						   GFP_KERNEL);
@@ -301,7 +321,7 @@ static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
 		goto buffer_allocation_error;
 	av1_dec->cdef_col.size = size;
 
-	size = ALIGN(height_in_sb * (3040 + 1280), 128) * num_tile_cols;
+	size = sr_size;
 	av1_dec->sr_col.cpu = dma_alloc_coherent(vpu->dev, size,
 						 &av1_dec->sr_col.dma,
 						 GFP_KERNEL);
@@ -309,7 +329,7 @@ static int rockchip_vpu981_av1_dec_tiles_reallocate(struct hantro_ctx *ctx)
 		goto buffer_allocation_error;
 	av1_dec->sr_col.size = size;
 
-	size = ALIGN(stripe_num * 1536 * ctx->bit_depth / 8, 128) * num_tile_cols;
+	size = lr_size;
 	av1_dec->lr_col.cpu = dma_alloc_coherent(vpu->dev, size,
 						 &av1_dec->lr_col.dma,
 						 GFP_KERNEL);

-- 
2.55.0

[PATCH v2 5/6] media: verisilicon: Reject AV1 frames without tiles

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

The AV1 control validation only checks the upper bounds of tile_cols
and tile_rows, and rockchip_vpu981_av1_dec_prepare_run() only checks the
number of tiles against the tile group entries and the tile buffer, so
a frame with tile_cols or tile_rows set to 0 is accepted. With no tile
columns, rockchip_vpu981_av1_dec_tiles_reallocate() computes a size of
0 for each of the five tile column buffers, which the buffers it has
always satisfy, so it allocates nothing. On the first frame of a context
the decoder is then started without tile column buffers (DMA address 0),
and after a failed allocation with the addresses of the freed ones.

Every AV1 frame has at least one tile, reject frames without any.

Fixes: 65e9f52ffdc8 ("media: verisilicon: Fix auxiliary buffer allocation size")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
index e817c91224d3..8154b405ca63 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
@@ -526,6 +526,13 @@ static int rockchip_vpu981_av1_dec_prepare_run(struct hantro_ctx *ctx)
 	if (num_tiles > tge->elems || num_tiles > AV1_MAX_TILES)
 		return -EINVAL;
 
+	/*
+	 * Every AV1 frame has at least one tile. Without tile columns, the
+	 * tile column buffers would be sized to zero and not allocated.
+	 */
+	if (!num_tiles)
+		return -EINVAL;
+
 	ctrls->film_grain =
 	    hantro_get_ctrl(ctx, V4L2_CID_STATELESS_AV1_FILM_GRAIN);
 

-- 
2.55.0

[PATCH v2 6/6] media: verisilicon: Reject AV1 frames larger than the CAPTURE format

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

frame_width_minus_1, frame_height_minus_1 and upscaled_width of the AV1
frame control are not validated against anything, and the decoder is
programmed with them.

The decoder writes the luma, the chroma and the motion vectors of a
frame into one buffer. The size of that buffer and the offsets of the
chroma and of the motion vectors in it are computed from the width and
height of the CAPTURE format, so a frame that is larger than the format
does not fit in the buffer the decoder is given.

rockchip_vpu981_av1_dec_tiles_reallocate() also sizes the tile column
buffers from the frame height in 32-bit arithmetic, which wraps around
for large heights, while the hardware is given the height as a 13-bit
number of 8-pixel blocks. The buffers can then be smaller than the
programmed height needs. For instance, frame_height_minus_1 = 0xffffffc0
rounds up to a height of 0, so the buffers of the previous frame are
found big enough and kept, and the decoder is started with a picture
height of 8185 blocks, 65480 lines.

Reject frames that are wider or taller than the CAPTURE format. The
format is never larger than the decoder supports, 4096x2304, so this
covers the heights that wrap as well. Frames that are smaller than the
format, as streams with spatial layers have, are still accepted.

Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c  | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
index 8154b405ca63..016f11f04a80 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
@@ -533,6 +533,18 @@ static int rockchip_vpu981_av1_dec_prepare_run(struct hantro_ctx *ctx)
 	if (!num_tiles)
 		return -EINVAL;
 
+	/*
+	 * The frame size comes from userspace. The decoder writes a frame of
+	 * that size, upscaled to upscaled_width, into buffers that are made
+	 * for the CAPTURE format, so the frame has to fit in that format. As
+	 * the format is within the limits of the decoder, this also keeps the
+	 * frame height in the range the tile column buffers can be sized for.
+	 */
+	if (ctrls->frame->frame_width_minus_1 >= ctx->dst_fmt.width ||
+	    ctrls->frame->frame_height_minus_1 >= ctx->dst_fmt.height ||
+	    ctrls->frame->upscaled_width > ctx->dst_fmt.width)
+		return -EINVAL;
+
 	ctrls->film_grain =
 	    hantro_get_ctrl(ctx, V4L2_CID_STATELESS_AV1_FILM_GRAIN);
 

-- 
2.55.0