Re: [PATCH v2 3/6] media: qcom: iris: gen2: add support for 10bit decoding
From: Neil Armstrong
Date: Thu Apr 30 2026 - 03:41:58 EST
On 4/22/26 15:43, Neil Armstrong wrote:
On 4/22/26 11:13, Dikshita Agarwal wrote:
On 4/17/2026 3:07 PM, Neil Armstrong wrote:
Add the necessary plumbing into the HFi Gen2 to signal the decoder
the right 10bit pixel format and stride when in compressed mode.
Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
---
.../platform/qcom/iris/iris_hfi_gen2_command.c | 71 +++++++++++++++++++++-
.../platform/qcom/iris/iris_hfi_gen2_defines.h | 1 +
drivers/media/platform/qcom/iris/iris_utils.c | 4 +-
3 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
index 30bfd90d423b..d664e606e886 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
@@ -481,8 +481,20 @@ static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
if (inst->domain == DECODER) {
pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
- hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
- HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
+ switch (pixelformat) {
+ case V4L2_PIX_FMT_NV12:
+ hfi_colorformat = HFI_COLOR_FMT_NV12;
+ break;
+ case V4L2_PIX_FMT_QC08C:
+ hfi_colorformat = HFI_COLOR_FMT_NV12_UBWC;
+ break;
+ case V4L2_PIX_FMT_P010:
+ hfi_colorformat = HFI_COLOR_FMT_P010;
+ break;
+ case V4L2_PIX_FMT_QC10C:
+ hfi_colorformat = HFI_COLOR_FMT_TP10_UBWC;
+ break;
+ };
} else {
pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
@@ -517,7 +529,8 @@ static int iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
stride_uv = stride_y;
scanline_uv = scanline_y / 2;
- if (pixelformat != V4L2_PIX_FMT_NV12)
+ if (pixelformat != V4L2_PIX_FMT_NV12 &&
+ pixelformat != V4L2_PIX_FMT_P010)
return 0;
payload[0] = stride_y << 16 | scanline_y;
@@ -532,6 +545,57 @@ static int iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
sizeof(u64));
}
+static int iris_hfi_gen2_set_ubwc_stride_scanline(struct iris_inst *inst, u32 plane)
+{
+ u32 meta_stride_y, meta_scanline_y, meta_stride_uv, meta_scanline_uv;
+ u32 stride_y, scanline_y, stride_uv, scanline_uv;
+ u32 port = iris_hfi_gen2_get_port(inst, plane);
+ u32 pixelformat, width, height;
+ u32 payload[4];
+
+ pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
+ width = inst->fmt_dst->fmt.pix_mp.width;
+ height = inst->fmt_dst->fmt.pix_mp.height;
+
+ switch (pixelformat) {
+ case V4L2_PIX_FMT_QC08C:
+ stride_y = ALIGN(width, 128);
+ scanline_y = ALIGN(height, 32);
+ stride_uv = ALIGN(width, 128);
+ scanline_uv = ALIGN((height + 1) >> 1, 32);
+ meta_stride_y = ALIGN(DIV_ROUND_UP(width, 32), 64);
+ meta_scanline_y = ALIGN(DIV_ROUND_UP(height, 8), 16);
+ meta_stride_uv = ALIGN(DIV_ROUND_UP((width + 1) >> 1, 16), 64);
+ meta_scanline_uv = ALIGN(DIV_ROUND_UP((height + 1) >> 1, 8), 16);
+ break;
+ case V4L2_PIX_FMT_QC10C:
+ stride_y = ALIGN(width * 4 / 3, 256);
+ scanline_y = ALIGN(height, 16);
+ stride_uv = ALIGN(width * 4 / 3, 256);
+ scanline_uv = ALIGN((height + 1) >> 1, 16);
+ meta_stride_y = ALIGN(DIV_ROUND_UP(width, 48), 64);
+ meta_scanline_y = ALIGN(DIV_ROUND_UP(height, 4), 16);
+ meta_stride_uv = ALIGN(DIV_ROUND_UP((width + 1) >> 1, 24), 64);
+ meta_scanline_uv = ALIGN(DIV_ROUND_UP((height + 1) >> 1, 4), 16);
+ break;
+ default:
+ return 0;
+ }
+
+ payload[0] = stride_y << 16 | scanline_y;
+ payload[1] = stride_uv << 16 | scanline_uv;
+ payload[2] = meta_stride_y << 16 | meta_scanline_y;
+ payload[3] = meta_stride_uv << 16 | meta_scanline_uv;
+
+ return iris_hfi_gen2_session_set_property(inst,
+ HFI_PROP_UBWC_STRIDE_SCANLINE,
+ HFI_HOST_FLAGS_NONE,
+ port,
+ HFI_PAYLOAD_U32_ARRAY,
+ &payload[0],
+ sizeof(u32) * 4);
+}
I’m still not convinced this change is needed for non‑AV1 codecs.
I’m concerned this may be masking an underlying alignment issue, since this
property is not intended to be used for non‑AV1 codecs. Even FW team
doesn't recommend setting this property for other codecs as it can lead to
wasted memory due to over‑allocation and padding.
I don't see the issue since here we simply pass the same strides as we expect
on the drm msm and iris driver buffer calculation, as we already do for the
linear formats BTW.
It makes no sense to not call HFI_PROP_UBWC_STRIDE_SCANLINE for ubwc formats and
let the firmware use it's default undocumented strides.
Could you check with the firmware team we shouldn’t set any _STRIDE_SCANLINE property when in UBWC except when in AV1 ?
This doesn't seem coherent, and I don't really understand why it would waste memory.
The whole goal here is to make sure we use the same stride calculation with the produce (iris)
and the consumers (mdss, gpu, iris?...).
If it's a bug in the firmware, then would it be possible to have the exact stride
formula used by the HEVC & VP9 decoders ?
Neil
Neil
Thanks,
Dikshita