public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add support for QC08C format in iris driver
@ 2025-09-19 15:47 Dikshita Agarwal
  2025-09-19 15:47 ` [PATCH 1/2] media: iris: Add support for QC08C format for decoder Dikshita Agarwal
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Dikshita Agarwal @ 2025-09-19 15:47 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, Dikshita Agarwal

Add support for the QC08C color format in both the encoder and decoder 
paths of the iris driver. The changes include:

- Adding QC08C format handling in the driver for both encoding and 
decoding.
- Updating format enumeration to properly return supported formats.
- Ensuring the correct HFI format is set for firmware communication.
-Making all related changes required for seamless integration of QC08C 
support.

The changes have been validated using v4l2-ctl, compliance, and GStreamer (GST) tests.
Both GST and v4l2-ctl tests were performed using the NV12 format, as 
these clients do not support the QCOM-specific QC08C format, and all 
tests passed successfully.

During v4l2-ctl testing, a regression was observed when using the NV12 
color format after adding QC08C support. A fix for this regression has 
also been posted [1].

[1]: https://lore.kernel.org/linux-media/20250918103235.4066441-1-dikshita.agarwal@oss.qualcomm.com/T/#u 

Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
Dikshita Agarwal (2):
      media: iris: Add support for QC08C format for decoder
      media: iris: Add support for QC08C format for encoder

 drivers/media/platform/qcom/iris/iris_buffer.c     | 17 ++++--
 .../platform/qcom/iris/iris_hfi_gen1_command.c     | 15 ++++--
 .../platform/qcom/iris/iris_hfi_gen2_command.c     | 21 +++++++-
 .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
 drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
 .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
 drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
 drivers/media/platform/qcom/iris/iris_vdec.c       | 61 ++++++++++++++++++----
 drivers/media/platform/qcom/iris/iris_venc.c       | 59 +++++++++++++++++----
 9 files changed, 152 insertions(+), 33 deletions(-)
---
base-commit: 40b7a19f321e65789612ebaca966472055dab48c
change-id: 20250918-video-iris-ubwc-enable-87eac6f41fa4

Best regards,
-- 
Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>


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

* [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-19 15:47 [PATCH 0/2] Add support for QC08C format in iris driver Dikshita Agarwal
@ 2025-09-19 15:47 ` Dikshita Agarwal
  2025-09-19 16:54   ` Dmitry Baryshkov
                     ` (2 more replies)
  2025-09-19 15:47 ` [PATCH 2/2] media: iris: Add support for QC08C format for encoder Dikshita Agarwal
  2025-10-01  8:39 ` [PATCH 0/2] Add support for QC08C format in iris driver Neil Armstrong
  2 siblings, 3 replies; 20+ messages in thread
From: Dikshita Agarwal @ 2025-09-19 15:47 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, Dikshita Agarwal

Introduce handling for the QC08C format in the decoder.
Update format checks and configuration to enable decoding of QC08C
streams.

Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_buffer.c     |  5 +-
 .../platform/qcom/iris/iris_hfi_gen1_command.c     | 12 +++--
 .../platform/qcom/iris/iris_hfi_gen2_command.c     | 18 ++++++-
 .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
 drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
 .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
 drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
 drivers/media/platform/qcom/iris/iris_vdec.c       | 61 ++++++++++++++++++----
 8 files changed, 89 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
index c0900038e7defccf7de3cb60e17c71e36a0e8ead..83dcf49e57ec1473bc4edd26c48ab0b247d23818 100644
--- a/drivers/media/platform/qcom/iris/iris_buffer.c
+++ b/drivers/media/platform/qcom/iris/iris_buffer.c
@@ -261,7 +261,10 @@ int iris_get_buffer_size(struct iris_inst *inst,
 		case BUF_INPUT:
 			return iris_dec_bitstream_buffer_size(inst);
 		case BUF_OUTPUT:
-			return iris_yuv_buffer_size_nv12(inst);
+			if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
+				return iris_yuv_buffer_size_qc08c(inst);
+			else
+				return iris_yuv_buffer_size_nv12(inst);
 		case BUF_DPB:
 			return iris_yuv_buffer_size_qc08c(inst);
 		default:
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
index e1788c266bb1080921f17248fd5ee60156b3143d..e458d3349ce09aadb75d056ae84e3aab95f03078 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
@@ -774,20 +774,21 @@ static int iris_hfi_gen1_set_raw_format(struct iris_inst *inst, u32 plane)
 		pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
 		if (iris_split_mode_enabled(inst)) {
 			fmt.buffer_type = HFI_BUFFER_OUTPUT;
-			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
-				HFI_COLOR_FORMAT_NV12_UBWC : 0;
+			fmt.format = HFI_COLOR_FORMAT_NV12_UBWC;
 
 			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
 			if (ret)
 				return ret;
 
 			fmt.buffer_type = HFI_BUFFER_OUTPUT2;
-			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
+			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
+				HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
 
 			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
 		} else {
 			fmt.buffer_type = HFI_BUFFER_OUTPUT;
-			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
+			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
+				HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
 
 			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
 		}
@@ -806,6 +807,9 @@ static int iris_hfi_gen1_set_format_constraints(struct iris_inst *inst, u32 plan
 	const u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_PLANE_ACTUAL_CONSTRAINTS_INFO;
 	struct hfi_uncompressed_plane_actual_constraints_info pconstraint;
 
+	if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
+		return 0;
+
 	pconstraint.buffer_type = HFI_BUFFER_OUTPUT2;
 	pconstraint.num_planes = 2;
 	pconstraint.plane_format[0].stride_multiples = 128;
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 4ce71a14250832440099e4cf3835b4aedfb749e8..5ad202d3fcdc57a2b7b43de15763a686ce0f7bd7 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
@@ -422,6 +422,20 @@ static int iris_hfi_gen2_set_level(struct iris_inst *inst, u32 plane)
 						  sizeof(u32));
 }
 
+static int iris_hfi_gen2_set_opb_enable(struct iris_inst *inst, u32 plane)
+{
+	u32 port = iris_hfi_gen2_get_port(inst, plane);
+	u32 opb_enable = iris_split_mode_enabled(inst);
+
+	return iris_hfi_gen2_session_set_property(inst,
+						  HFI_PROP_OPB_ENABLE,
+						  HFI_HOST_FLAGS_NONE,
+						  port,
+						  HFI_PAYLOAD_U32,
+						  &opb_enable,
+						  sizeof(u32));
+}
+
 static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
 {
 	u32 port = iris_hfi_gen2_get_port(inst, plane);
@@ -429,7 +443,8 @@ 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 : 0;
+		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
+			HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
 	} else {
 		pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
 		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FMT_NV12 : 0;
@@ -527,6 +542,7 @@ static int iris_hfi_gen2_session_set_config_params(struct iris_inst *inst, u32 p
 		{HFI_PROP_SIGNAL_COLOR_INFO,          iris_hfi_gen2_set_colorspace             },
 		{HFI_PROP_PROFILE,                    iris_hfi_gen2_set_profile                },
 		{HFI_PROP_LEVEL,                      iris_hfi_gen2_set_level                  },
+		{HFI_PROP_OPB_ENABLE,                 iris_hfi_gen2_set_opb_enable             },
 		{HFI_PROP_COLOR_FORMAT,               iris_hfi_gen2_set_colorformat            },
 		{HFI_PROP_LINEAR_STRIDE_SCANLINE,     iris_hfi_gen2_set_linear_stride_scanline },
 		{HFI_PROP_TIER,                       iris_hfi_gen2_set_tier                   },
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
index aa1f795f5626c1f76a32dd650302633877ce67be..1b6a4dbac828ffea53c1be0d3624a033c283c972 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
@@ -91,6 +91,7 @@ enum hfi_seq_header_mode {
 #define HFI_PROP_BUFFER_MARK			0x0300016c
 #define HFI_PROP_RAW_RESOLUTION		0x03000178
 #define HFI_PROP_TOTAL_PEAK_BITRATE		0x0300017C
+#define HFI_PROP_OPB_ENABLE			0x03000184
 #define HFI_PROP_COMV_BUFFER_COUNT		0x03000193
 #define HFI_PROP_END				0x03FFFFFF
 
diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
index 5982d7adefeab80905478b32cddba7bd4651a691..11134f75c26dd1f6c92ba72fbf4e56e41a72de64 100644
--- a/drivers/media/platform/qcom/iris/iris_instance.h
+++ b/drivers/media/platform/qcom/iris/iris_instance.h
@@ -15,12 +15,17 @@
 #define DEFAULT_WIDTH 320
 #define DEFAULT_HEIGHT 240
 
-enum iris_fmt_type {
+enum iris_fmt_type_out {
 	IRIS_FMT_H264,
 	IRIS_FMT_HEVC,
 	IRIS_FMT_VP9,
 };
 
+enum iris_fmt_type_cap {
+	IRIS_FMT_NV12,
+	IRIS_FMT_UBWC,
+};
+
 struct iris_fmt {
 	u32 pixfmt;
 	u32 type;
diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
index 36d69cc73986b74534a2912524c8553970fd862e..69c952c68e939f305f25511e2e4763487ec8e0de 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
@@ -691,6 +691,7 @@ static const u32 sm8550_venc_input_config_params[] = {
 };
 
 static const u32 sm8550_vdec_output_config_params[] = {
+	HFI_PROP_OPB_ENABLE,
 	HFI_PROP_COLOR_FORMAT,
 	HFI_PROP_LINEAR_STRIDE_SCANLINE,
 };
diff --git a/drivers/media/platform/qcom/iris/iris_utils.c b/drivers/media/platform/qcom/iris/iris_utils.c
index 85c70a62b1fd2c409fc18b28f64771cb0097a7fd..e2f1131de43128254d8211343771e657e425541e 100644
--- a/drivers/media/platform/qcom/iris/iris_utils.c
+++ b/drivers/media/platform/qcom/iris/iris_utils.c
@@ -34,7 +34,8 @@ int iris_get_mbpf(struct iris_inst *inst)
 
 bool iris_split_mode_enabled(struct iris_inst *inst)
 {
-	return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12;
+	return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12 ||
+		inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C;
 }
 
 void iris_helper_buffers_done(struct iris_inst *inst, unsigned int type,
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
index ae13c3e1b426bfd81a7b46dc6c3ff5eb5c4860cb..7fa745c6bf8950d192c2d2fc1770c4b6fd7b8c4c 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.c
+++ b/drivers/media/platform/qcom/iris/iris_vdec.c
@@ -67,7 +67,7 @@ void iris_vdec_inst_deinit(struct iris_inst *inst)
 	kfree(inst->fmt_src);
 }
 
-static const struct iris_fmt iris_vdec_formats[] = {
+static const struct iris_fmt iris_vdec_formats_out[] = {
 	[IRIS_FMT_H264] = {
 		.pixfmt = V4L2_PIX_FMT_H264,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
@@ -82,12 +82,35 @@ static const struct iris_fmt iris_vdec_formats[] = {
 	},
 };
 
+static const struct iris_fmt iris_vdec_formats_cap[] = {
+	[IRIS_FMT_NV12] = {
+		.pixfmt = V4L2_PIX_FMT_NV12,
+		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+	},
+	[IRIS_FMT_UBWC] = {
+		.pixfmt = V4L2_PIX_FMT_QC08C,
+		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+	},
+};
+
 static const struct iris_fmt *
 find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
 {
-	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
-	const struct iris_fmt *fmt = iris_vdec_formats;
+	const struct iris_fmt *fmt = NULL;
+	unsigned int size = 0;
 	unsigned int i;
+	switch (type) {
+	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+		fmt = iris_vdec_formats_out;
+		size = ARRAY_SIZE(iris_vdec_formats_out);
+		break;
+	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		fmt = iris_vdec_formats_cap;
+		size = ARRAY_SIZE(iris_vdec_formats_cap);
+		break;
+	default:
+		return NULL;
+	}
 
 	for (i = 0; i < size; i++) {
 		if (fmt[i].pixfmt == pixfmt)
@@ -103,8 +126,21 @@ find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
 static const struct iris_fmt *
 find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
 {
-	const struct iris_fmt *fmt = iris_vdec_formats;
-	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
+	const struct iris_fmt *fmt = NULL;
+	unsigned int size = 0;
+
+	switch (type) {
+	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+		fmt = iris_vdec_formats_out;
+		size = ARRAY_SIZE(iris_vdec_formats_out);
+		break;
+	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		fmt = iris_vdec_formats_cap;
+		size = ARRAY_SIZE(iris_vdec_formats_cap);
+		break;
+	default:
+		return NULL;
+	}
 
 	if (index >= size || fmt[index].type != type)
 		return NULL;
@@ -126,9 +162,10 @@ int iris_vdec_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f)
 		f->flags = V4L2_FMT_FLAG_COMPRESSED | V4L2_FMT_FLAG_DYN_RESOLUTION;
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
-		if (f->index)
+		fmt = find_format_by_index(inst, f->index, f->type);
+		if (!fmt)
 			return -EINVAL;
-		f->pixelformat = V4L2_PIX_FMT_NV12;
+		f->pixelformat = fmt->pixfmt;
 		break;
 	default:
 		return -EINVAL;
@@ -157,7 +194,7 @@ int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f)
 		}
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
-		if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) {
+		if (!fmt) {
 			f_inst = inst->fmt_dst;
 			f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat;
 			f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width;
@@ -238,10 +275,11 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
 		inst->crop.height = f->fmt.pix_mp.height;
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		if (!(find_format(inst, f->fmt.pix_mp.pixelformat, f->type)))
+			return -EINVAL;
+
 		fmt = inst->fmt_dst;
 		fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
-		if (fmt->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12)
-			return -EINVAL;
 		fmt->fmt.pix_mp.pixelformat = f->fmt.pix_mp.pixelformat;
 		fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
 		fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
@@ -268,7 +306,8 @@ int iris_vdec_validate_format(struct iris_inst *inst, u32 pixelformat)
 {
 	const struct iris_fmt *fmt = NULL;
 
-	if (pixelformat != V4L2_PIX_FMT_NV12) {
+	fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
+	if (!fmt) {
 		fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
 		if (!fmt)
 			return -EINVAL;

-- 
2.34.1


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

* [PATCH 2/2] media: iris: Add support for QC08C format for encoder
  2025-09-19 15:47 [PATCH 0/2] Add support for QC08C format in iris driver Dikshita Agarwal
  2025-09-19 15:47 ` [PATCH 1/2] media: iris: Add support for QC08C format for decoder Dikshita Agarwal
@ 2025-09-19 15:47 ` Dikshita Agarwal
  2025-09-24 12:40   ` Markus Elfring
  2025-10-01  8:39 ` [PATCH 0/2] Add support for QC08C format in iris driver Neil Armstrong
  2 siblings, 1 reply; 20+ messages in thread
From: Dikshita Agarwal @ 2025-09-19 15:47 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, Dikshita Agarwal

Introduce handling for the QC08C format in the encoder. Update format
checks and configuration to enable encoding to QC08C streams.

Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_buffer.c     | 12 ++++-
 .../platform/qcom/iris/iris_hfi_gen1_command.c     |  3 +-
 .../platform/qcom/iris/iris_hfi_gen2_command.c     |  3 +-
 drivers/media/platform/qcom/iris/iris_venc.c       | 59 ++++++++++++++++++----
 4 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
index 83dcf49e57ec1473bc4edd26c48ab0b247d23818..b89b1ee06cce151e7c04a80956380d154643c116 100644
--- a/drivers/media/platform/qcom/iris/iris_buffer.c
+++ b/drivers/media/platform/qcom/iris/iris_buffer.c
@@ -171,9 +171,14 @@ static u32 iris_yuv_buffer_size_nv12(struct iris_inst *inst)
 static u32 iris_yuv_buffer_size_qc08c(struct iris_inst *inst)
 {
 	u32 y_plane, uv_plane, y_stride, uv_stride;
-	struct v4l2_format *f = inst->fmt_dst;
 	u32 uv_meta_stride, uv_meta_plane;
 	u32 y_meta_stride, y_meta_plane;
+	struct v4l2_format *f = NULL;
+
+	if (inst->domain == DECODER)
+		f = inst->fmt_dst;
+	else
+		f = inst->fmt_src;
 
 	y_meta_stride = ALIGN(DIV_ROUND_UP(f->fmt.pix_mp.width, META_STRIDE_ALIGNED >> 1),
 			      META_STRIDE_ALIGNED);
@@ -273,7 +278,10 @@ int iris_get_buffer_size(struct iris_inst *inst,
 	} else {
 		switch (buffer_type) {
 		case BUF_INPUT:
-			return iris_yuv_buffer_size_nv12(inst);
+			if (inst->fmt_src->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
+				return iris_yuv_buffer_size_qc08c(inst);
+			else
+				return iris_yuv_buffer_size_nv12(inst);
 		case BUF_OUTPUT:
 			return iris_enc_bitstream_buffer_size(inst);
 		default:
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
index e458d3349ce09aadb75d056ae84e3aab95f03078..52da7ef7bab08fb1cb2ac804ccc6e3c7f9677890 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
@@ -795,7 +795,8 @@ static int iris_hfi_gen1_set_raw_format(struct iris_inst *inst, u32 plane)
 	} else {
 		pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
 		fmt.buffer_type = HFI_BUFFER_INPUT;
-		fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
+		fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
+			HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
 		ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
 	}
 
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 5ad202d3fcdc57a2b7b43de15763a686ce0f7bd7..6a772db2ec33fb002d8884753a41dc98b3a8439d 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
@@ -447,7 +447,8 @@ static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
 			HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
 	} else {
 		pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
-		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FMT_NV12 : 0;
+		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
+			HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
 	}
 
 	return iris_hfi_gen2_session_set_property(inst,
diff --git a/drivers/media/platform/qcom/iris/iris_venc.c b/drivers/media/platform/qcom/iris/iris_venc.c
index 099bd5ed4ae0294725860305254c4cad1ec88d7e..51fa4a06a70e15310ad8e32d7e16b60b2ab11d5a 100644
--- a/drivers/media/platform/qcom/iris/iris_venc.c
+++ b/drivers/media/platform/qcom/iris/iris_venc.c
@@ -80,7 +80,7 @@ void iris_venc_inst_deinit(struct iris_inst *inst)
 	kfree(inst->fmt_src);
 }
 
-static const struct iris_fmt iris_venc_formats[] = {
+static const struct iris_fmt iris_venc_formats_cap[] = {
 	[IRIS_FMT_H264] = {
 		.pixfmt = V4L2_PIX_FMT_H264,
 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
@@ -91,12 +91,35 @@ static const struct iris_fmt iris_venc_formats[] = {
 	},
 };
 
+static const struct iris_fmt iris_venc_formats_out[] = {
+	[IRIS_FMT_NV12] = {
+		.pixfmt = V4L2_PIX_FMT_NV12,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	},
+	[IRIS_FMT_UBWC] = {
+		.pixfmt = V4L2_PIX_FMT_QC08C,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	},
+};
+
 static const struct iris_fmt *
 find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
 {
-	const struct iris_fmt *fmt = iris_venc_formats;
-	unsigned int size = ARRAY_SIZE(iris_venc_formats);
+	const struct iris_fmt *fmt = NULL;
+	unsigned int size = 0;
 	unsigned int i;
+	switch (type) {
+	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+		fmt = iris_venc_formats_out;
+		size = ARRAY_SIZE(iris_venc_formats_out);
+		break;
+	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		fmt = iris_venc_formats_cap;
+		size = ARRAY_SIZE(iris_venc_formats_cap);
+		break;
+	default:
+		return NULL;
+	}
 
 	for (i = 0; i < size; i++) {
 		if (fmt[i].pixfmt == pixfmt)
@@ -112,8 +135,21 @@ find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
 static const struct iris_fmt *
 find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
 {
-	const struct iris_fmt *fmt = iris_venc_formats;
-	unsigned int size = ARRAY_SIZE(iris_venc_formats);
+	const struct iris_fmt *fmt = NULL;
+	unsigned int size = 0;
+
+	switch (type) {
+	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+		fmt = iris_venc_formats_out;
+		size = ARRAY_SIZE(iris_venc_formats_out);
+		break;
+	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		fmt = iris_venc_formats_cap;
+		size = ARRAY_SIZE(iris_venc_formats_cap);
+		break;
+	default:
+		return NULL;
+	}
 
 	if (index >= size || fmt[index].type != type)
 		return NULL;
@@ -127,9 +163,11 @@ int iris_venc_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f)
 
 	switch (f->type) {
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
-		if (f->index)
+		fmt = find_format_by_index(inst, f->index, f->type);
+		if (!fmt)
 			return -EINVAL;
-		f->pixelformat = V4L2_PIX_FMT_NV12;
+
+		f->pixelformat = fmt->pixfmt;
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 		fmt = find_format_by_index(inst, f->index, f->type);
@@ -156,7 +194,7 @@ int iris_venc_try_fmt(struct iris_inst *inst, struct v4l2_format *f)
 	fmt = find_format(inst, pixmp->pixelformat, f->type);
 	switch (f->type) {
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
-		if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) {
+		if (!fmt) {
 			f_inst = inst->fmt_src;
 			f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width;
 			f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height;
@@ -221,7 +259,7 @@ static int iris_venc_s_fmt_input(struct iris_inst *inst, struct v4l2_format *f)
 
 	iris_venc_try_fmt(inst, f);
 
-	if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12)
+	if (!(find_format(inst, f->fmt.pix_mp.pixelformat, f->type)))
 		return -EINVAL;
 
 	fmt = inst->fmt_src;
@@ -289,7 +327,8 @@ int iris_venc_validate_format(struct iris_inst *inst, u32 pixelformat)
 {
 	const struct iris_fmt *fmt = NULL;
 
-	if (pixelformat != V4L2_PIX_FMT_NV12) {
+	fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
+	if (!fmt) {
 		fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
 		if (!fmt)
 			return -EINVAL;

-- 
2.34.1


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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-19 15:47 ` [PATCH 1/2] media: iris: Add support for QC08C format for decoder Dikshita Agarwal
@ 2025-09-19 16:54   ` Dmitry Baryshkov
  2025-09-24 10:54     ` Dikshita Agarwal
  2025-09-24 12:32   ` Markus Elfring
  2025-09-24 13:28   ` Bryan O'Donoghue
  2 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2025-09-19 16:54 UTC (permalink / raw)
  To: Dikshita Agarwal
  Cc: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel

On Fri, Sep 19, 2025 at 09:17:16PM +0530, Dikshita Agarwal wrote:
> Introduce handling for the QC08C format in the decoder.
> Update format checks and configuration to enable decoding of QC08C
> streams.

What is QC08C? Is it supported on all devices?

> 
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
>  drivers/media/platform/qcom/iris/iris_buffer.c     |  5 +-
>  .../platform/qcom/iris/iris_hfi_gen1_command.c     | 12 +++--
>  .../platform/qcom/iris/iris_hfi_gen2_command.c     | 18 ++++++-
>  .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>  drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
>  .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
>  drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
>  drivers/media/platform/qcom/iris/iris_vdec.c       | 61 ++++++++++++++++++----
>  8 files changed, 89 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index c0900038e7defccf7de3cb60e17c71e36a0e8ead..83dcf49e57ec1473bc4edd26c48ab0b247d23818 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -261,7 +261,10 @@ int iris_get_buffer_size(struct iris_inst *inst,
>  		case BUF_INPUT:
>  			return iris_dec_bitstream_buffer_size(inst);
>  		case BUF_OUTPUT:
> -			return iris_yuv_buffer_size_nv12(inst);
> +			if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
> +				return iris_yuv_buffer_size_qc08c(inst);
> +			else
> +				return iris_yuv_buffer_size_nv12(inst);
>  		case BUF_DPB:
>  			return iris_yuv_buffer_size_qc08c(inst);
>  		default:
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index e1788c266bb1080921f17248fd5ee60156b3143d..e458d3349ce09aadb75d056ae84e3aab95f03078 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -774,20 +774,21 @@ static int iris_hfi_gen1_set_raw_format(struct iris_inst *inst, u32 plane)
>  		pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>  		if (iris_split_mode_enabled(inst)) {
>  			fmt.buffer_type = HFI_BUFFER_OUTPUT;
> -			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
> -				HFI_COLOR_FORMAT_NV12_UBWC : 0;
> +			fmt.format = HFI_COLOR_FORMAT_NV12_UBWC;

This needs some explanation.

>  
>  			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>  			if (ret)
>  				return ret;
>  
>  			fmt.buffer_type = HFI_BUFFER_OUTPUT2;
> -			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
> +			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
> +				HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
>  
>  			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>  		} else {
>  			fmt.buffer_type = HFI_BUFFER_OUTPUT;
> -			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
> +			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
> +				HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
>  
>  			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>  		}
> @@ -806,6 +807,9 @@ static int iris_hfi_gen1_set_format_constraints(struct iris_inst *inst, u32 plan
>  	const u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_PLANE_ACTUAL_CONSTRAINTS_INFO;
>  	struct hfi_uncompressed_plane_actual_constraints_info pconstraint;
>  
> +	if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
> +		return 0;
> +
>  	pconstraint.buffer_type = HFI_BUFFER_OUTPUT2;
>  	pconstraint.num_planes = 2;
>  	pconstraint.plane_format[0].stride_multiples = 128;
> 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 4ce71a14250832440099e4cf3835b4aedfb749e8..5ad202d3fcdc57a2b7b43de15763a686ce0f7bd7 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> @@ -422,6 +422,20 @@ static int iris_hfi_gen2_set_level(struct iris_inst *inst, u32 plane)
>  						  sizeof(u32));
>  }
>  
> +static int iris_hfi_gen2_set_opb_enable(struct iris_inst *inst, u32 plane)
> +{
> +	u32 port = iris_hfi_gen2_get_port(inst, plane);
> +	u32 opb_enable = iris_split_mode_enabled(inst);
> +
> +	return iris_hfi_gen2_session_set_property(inst,
> +						  HFI_PROP_OPB_ENABLE,
> +						  HFI_HOST_FLAGS_NONE,
> +						  port,
> +						  HFI_PAYLOAD_U32,
> +						  &opb_enable,
> +						  sizeof(u32));
> +}
> +
>  static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
>  {
>  	u32 port = iris_hfi_gen2_get_port(inst, plane);
> @@ -429,7 +443,8 @@ 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 : 0;
> +		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
> +			HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
>  	} else {
>  		pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
>  		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FMT_NV12 : 0;
> @@ -527,6 +542,7 @@ static int iris_hfi_gen2_session_set_config_params(struct iris_inst *inst, u32 p
>  		{HFI_PROP_SIGNAL_COLOR_INFO,          iris_hfi_gen2_set_colorspace             },
>  		{HFI_PROP_PROFILE,                    iris_hfi_gen2_set_profile                },
>  		{HFI_PROP_LEVEL,                      iris_hfi_gen2_set_level                  },
> +		{HFI_PROP_OPB_ENABLE,                 iris_hfi_gen2_set_opb_enable             },
>  		{HFI_PROP_COLOR_FORMAT,               iris_hfi_gen2_set_colorformat            },
>  		{HFI_PROP_LINEAR_STRIDE_SCANLINE,     iris_hfi_gen2_set_linear_stride_scanline },
>  		{HFI_PROP_TIER,                       iris_hfi_gen2_set_tier                   },
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> index aa1f795f5626c1f76a32dd650302633877ce67be..1b6a4dbac828ffea53c1be0d3624a033c283c972 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> @@ -91,6 +91,7 @@ enum hfi_seq_header_mode {
>  #define HFI_PROP_BUFFER_MARK			0x0300016c
>  #define HFI_PROP_RAW_RESOLUTION		0x03000178
>  #define HFI_PROP_TOTAL_PEAK_BITRATE		0x0300017C
> +#define HFI_PROP_OPB_ENABLE			0x03000184
>  #define HFI_PROP_COMV_BUFFER_COUNT		0x03000193
>  #define HFI_PROP_END				0x03FFFFFF
>  
> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
> index 5982d7adefeab80905478b32cddba7bd4651a691..11134f75c26dd1f6c92ba72fbf4e56e41a72de64 100644
> --- a/drivers/media/platform/qcom/iris/iris_instance.h
> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
> @@ -15,12 +15,17 @@
>  #define DEFAULT_WIDTH 320
>  #define DEFAULT_HEIGHT 240
>  
> -enum iris_fmt_type {
> +enum iris_fmt_type_out {
>  	IRIS_FMT_H264,
>  	IRIS_FMT_HEVC,
>  	IRIS_FMT_VP9,
>  };
>  
> +enum iris_fmt_type_cap {
> +	IRIS_FMT_NV12,
> +	IRIS_FMT_UBWC,

UBWC is not a format on its own, it's a modifier of the format. Please
come up with a better naming.

> +};
> +
>  struct iris_fmt {
>  	u32 pixfmt;
>  	u32 type;
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
> index 36d69cc73986b74534a2912524c8553970fd862e..69c952c68e939f305f25511e2e4763487ec8e0de 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
> @@ -691,6 +691,7 @@ static const u32 sm8550_venc_input_config_params[] = {
>  };
>  
>  static const u32 sm8550_vdec_output_config_params[] = {
> +	HFI_PROP_OPB_ENABLE,
>  	HFI_PROP_COLOR_FORMAT,
>  	HFI_PROP_LINEAR_STRIDE_SCANLINE,
>  };
> diff --git a/drivers/media/platform/qcom/iris/iris_utils.c b/drivers/media/platform/qcom/iris/iris_utils.c
> index 85c70a62b1fd2c409fc18b28f64771cb0097a7fd..e2f1131de43128254d8211343771e657e425541e 100644
> --- a/drivers/media/platform/qcom/iris/iris_utils.c
> +++ b/drivers/media/platform/qcom/iris/iris_utils.c
> @@ -34,7 +34,8 @@ int iris_get_mbpf(struct iris_inst *inst)
>  
>  bool iris_split_mode_enabled(struct iris_inst *inst)
>  {
> -	return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12;
> +	return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12 ||
> +		inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C;
>  }
>  
>  void iris_helper_buffers_done(struct iris_inst *inst, unsigned int type,
> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index ae13c3e1b426bfd81a7b46dc6c3ff5eb5c4860cb..7fa745c6bf8950d192c2d2fc1770c4b6fd7b8c4c 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
> @@ -67,7 +67,7 @@ void iris_vdec_inst_deinit(struct iris_inst *inst)
>  	kfree(inst->fmt_src);
>  }
>  
> -static const struct iris_fmt iris_vdec_formats[] = {
> +static const struct iris_fmt iris_vdec_formats_out[] = {
>  	[IRIS_FMT_H264] = {
>  		.pixfmt = V4L2_PIX_FMT_H264,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> @@ -82,12 +82,35 @@ static const struct iris_fmt iris_vdec_formats[] = {
>  	},
>  };
>  
> +static const struct iris_fmt iris_vdec_formats_cap[] = {

s/formats_cap/formats_capture/

> +	[IRIS_FMT_NV12] = {
> +		.pixfmt = V4L2_PIX_FMT_NV12,
> +		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> +	},
> +	[IRIS_FMT_UBWC] = {
> +		.pixfmt = V4L2_PIX_FMT_QC08C,
> +		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> +	},
> +};
> +
>  static const struct iris_fmt *
>  find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>  {
> -	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
> -	const struct iris_fmt *fmt = iris_vdec_formats;
> +	const struct iris_fmt *fmt = NULL;
> +	unsigned int size = 0;
>  	unsigned int i;
> +	switch (type) {
> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> +		fmt = iris_vdec_formats_out;
> +		size = ARRAY_SIZE(iris_vdec_formats_out);
> +		break;
> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> +		fmt = iris_vdec_formats_cap;
> +		size = ARRAY_SIZE(iris_vdec_formats_cap);
> +		break;
> +	default:
> +		return NULL;
> +	}
>  
>  	for (i = 0; i < size; i++) {
>  		if (fmt[i].pixfmt == pixfmt)
> @@ -103,8 +126,21 @@ find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>  static const struct iris_fmt *
>  find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
>  {
> -	const struct iris_fmt *fmt = iris_vdec_formats;
> -	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
> +	const struct iris_fmt *fmt = NULL;
> +	unsigned int size = 0;
> +
> +	switch (type) {
> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> +		fmt = iris_vdec_formats_out;
> +		size = ARRAY_SIZE(iris_vdec_formats_out);
> +		break;
> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> +		fmt = iris_vdec_formats_cap;
> +		size = ARRAY_SIZE(iris_vdec_formats_cap);
> +		break;
> +	default:
> +		return NULL;
> +	}
>  
>  	if (index >= size || fmt[index].type != type)
>  		return NULL;
> @@ -126,9 +162,10 @@ int iris_vdec_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f)
>  		f->flags = V4L2_FMT_FLAG_COMPRESSED | V4L2_FMT_FLAG_DYN_RESOLUTION;
>  		break;
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> -		if (f->index)
> +		fmt = find_format_by_index(inst, f->index, f->type);
> +		if (!fmt)
>  			return -EINVAL;
> -		f->pixelformat = V4L2_PIX_FMT_NV12;
> +		f->pixelformat = fmt->pixfmt;
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -157,7 +194,7 @@ int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f)
>  		}
>  		break;
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> -		if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) {
> +		if (!fmt) {
>  			f_inst = inst->fmt_dst;
>  			f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat;
>  			f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width;
> @@ -238,10 +275,11 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
>  		inst->crop.height = f->fmt.pix_mp.height;
>  		break;
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> +		if (!(find_format(inst, f->fmt.pix_mp.pixelformat, f->type)))
> +			return -EINVAL;
> +
>  		fmt = inst->fmt_dst;
>  		fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> -		if (fmt->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12)
> -			return -EINVAL;
>  		fmt->fmt.pix_mp.pixelformat = f->fmt.pix_mp.pixelformat;
>  		fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
>  		fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
> @@ -268,7 +306,8 @@ int iris_vdec_validate_format(struct iris_inst *inst, u32 pixelformat)
>  {
>  	const struct iris_fmt *fmt = NULL;
>  
> -	if (pixelformat != V4L2_PIX_FMT_NV12) {
> +	fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	if (!fmt) {
>  		fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>  		if (!fmt)
>  			return -EINVAL;
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-19 16:54   ` Dmitry Baryshkov
@ 2025-09-24 10:54     ` Dikshita Agarwal
  2025-09-24 16:23       ` Dmitry Baryshkov
  0 siblings, 1 reply; 20+ messages in thread
From: Dikshita Agarwal @ 2025-09-24 10:54 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel



On 9/19/2025 10:24 PM, Dmitry Baryshkov wrote:
> On Fri, Sep 19, 2025 at 09:17:16PM +0530, Dikshita Agarwal wrote:
>> Introduce handling for the QC08C format in the decoder.
>> Update format checks and configuration to enable decoding of QC08C
>> streams.
> 
> What is QC08C? Is it supported on all devices?

It's qualcomm specific compressed format, it's defined here
https://elixir.bootlin.com/linux/v6.17-rc2/source/include/uapi/linux/videodev2.h#L820

And Yes, it's supported on all Qualcomm devices supported by iris driver.

> 
>>
>> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
>> ---
>>  drivers/media/platform/qcom/iris/iris_buffer.c     |  5 +-
>>  .../platform/qcom/iris/iris_hfi_gen1_command.c     | 12 +++--
>>  .../platform/qcom/iris/iris_hfi_gen2_command.c     | 18 ++++++-
>>  .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>>  drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
>>  .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
>>  drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
>>  drivers/media/platform/qcom/iris/iris_vdec.c       | 61 ++++++++++++++++++----
>>  8 files changed, 89 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
>> index c0900038e7defccf7de3cb60e17c71e36a0e8ead..83dcf49e57ec1473bc4edd26c48ab0b247d23818 100644
>> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
>> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
>> @@ -261,7 +261,10 @@ int iris_get_buffer_size(struct iris_inst *inst,
>>  		case BUF_INPUT:
>>  			return iris_dec_bitstream_buffer_size(inst);
>>  		case BUF_OUTPUT:
>> -			return iris_yuv_buffer_size_nv12(inst);
>> +			if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
>> +				return iris_yuv_buffer_size_qc08c(inst);
>> +			else
>> +				return iris_yuv_buffer_size_nv12(inst);
>>  		case BUF_DPB:
>>  			return iris_yuv_buffer_size_qc08c(inst);
>>  		default:
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> index e1788c266bb1080921f17248fd5ee60156b3143d..e458d3349ce09aadb75d056ae84e3aab95f03078 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> @@ -774,20 +774,21 @@ static int iris_hfi_gen1_set_raw_format(struct iris_inst *inst, u32 plane)
>>  		pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>>  		if (iris_split_mode_enabled(inst)) {
>>  			fmt.buffer_type = HFI_BUFFER_OUTPUT;
>> -			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> -				HFI_COLOR_FORMAT_NV12_UBWC : 0;
>> +			fmt.format = HFI_COLOR_FORMAT_NV12_UBWC;
> 
> This needs some explanation.

We are enabling QC08C format in iris driver with split mode, which means we
need to set UBWC format for DPB (reference buffer) buffers to firmware,
since firmware only supports UBWC format for DPB buffers.

> 
>>  
>>  			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>>  			if (ret)
>>  				return ret;
>>  
>>  			fmt.buffer_type = HFI_BUFFER_OUTPUT2;
>> -			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
>> +			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> +				HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
>>  
>>  			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>>  		} else {
>>  			fmt.buffer_type = HFI_BUFFER_OUTPUT;
>> -			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
>> +			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> +				HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
>>  
>>  			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>>  		}
>> @@ -806,6 +807,9 @@ static int iris_hfi_gen1_set_format_constraints(struct iris_inst *inst, u32 plan
>>  	const u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_PLANE_ACTUAL_CONSTRAINTS_INFO;
>>  	struct hfi_uncompressed_plane_actual_constraints_info pconstraint;
>>  
>> +	if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
>> +		return 0;
>> +
>>  	pconstraint.buffer_type = HFI_BUFFER_OUTPUT2;
>>  	pconstraint.num_planes = 2;
>>  	pconstraint.plane_format[0].stride_multiples = 128;
>> 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 4ce71a14250832440099e4cf3835b4aedfb749e8..5ad202d3fcdc57a2b7b43de15763a686ce0f7bd7 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> @@ -422,6 +422,20 @@ static int iris_hfi_gen2_set_level(struct iris_inst *inst, u32 plane)
>>  						  sizeof(u32));
>>  }
>>  
>> +static int iris_hfi_gen2_set_opb_enable(struct iris_inst *inst, u32 plane)
>> +{
>> +	u32 port = iris_hfi_gen2_get_port(inst, plane);
>> +	u32 opb_enable = iris_split_mode_enabled(inst);
>> +
>> +	return iris_hfi_gen2_session_set_property(inst,
>> +						  HFI_PROP_OPB_ENABLE,
>> +						  HFI_HOST_FLAGS_NONE,
>> +						  port,
>> +						  HFI_PAYLOAD_U32,
>> +						  &opb_enable,
>> +						  sizeof(u32));
>> +}
>> +
>>  static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
>>  {
>>  	u32 port = iris_hfi_gen2_get_port(inst, plane);
>> @@ -429,7 +443,8 @@ 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 : 0;
>> +		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
>> +			HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
>>  	} else {
>>  		pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
>>  		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FMT_NV12 : 0;
>> @@ -527,6 +542,7 @@ static int iris_hfi_gen2_session_set_config_params(struct iris_inst *inst, u32 p
>>  		{HFI_PROP_SIGNAL_COLOR_INFO,          iris_hfi_gen2_set_colorspace             },
>>  		{HFI_PROP_PROFILE,                    iris_hfi_gen2_set_profile                },
>>  		{HFI_PROP_LEVEL,                      iris_hfi_gen2_set_level                  },
>> +		{HFI_PROP_OPB_ENABLE,                 iris_hfi_gen2_set_opb_enable             },
>>  		{HFI_PROP_COLOR_FORMAT,               iris_hfi_gen2_set_colorformat            },
>>  		{HFI_PROP_LINEAR_STRIDE_SCANLINE,     iris_hfi_gen2_set_linear_stride_scanline },
>>  		{HFI_PROP_TIER,                       iris_hfi_gen2_set_tier                   },
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> index aa1f795f5626c1f76a32dd650302633877ce67be..1b6a4dbac828ffea53c1be0d3624a033c283c972 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> @@ -91,6 +91,7 @@ enum hfi_seq_header_mode {
>>  #define HFI_PROP_BUFFER_MARK			0x0300016c
>>  #define HFI_PROP_RAW_RESOLUTION		0x03000178
>>  #define HFI_PROP_TOTAL_PEAK_BITRATE		0x0300017C
>> +#define HFI_PROP_OPB_ENABLE			0x03000184
>>  #define HFI_PROP_COMV_BUFFER_COUNT		0x03000193
>>  #define HFI_PROP_END				0x03FFFFFF
>>  
>> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
>> index 5982d7adefeab80905478b32cddba7bd4651a691..11134f75c26dd1f6c92ba72fbf4e56e41a72de64 100644
>> --- a/drivers/media/platform/qcom/iris/iris_instance.h
>> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
>> @@ -15,12 +15,17 @@
>>  #define DEFAULT_WIDTH 320
>>  #define DEFAULT_HEIGHT 240
>>  
>> -enum iris_fmt_type {
>> +enum iris_fmt_type_out {
>>  	IRIS_FMT_H264,
>>  	IRIS_FMT_HEVC,
>>  	IRIS_FMT_VP9,
>>  };
>>  
>> +enum iris_fmt_type_cap {
>> +	IRIS_FMT_NV12,
>> +	IRIS_FMT_UBWC,
> 
> UBWC is not a format on its own, it's a modifier of the format. Please
> come up with a better naming.

Sure, will rename this to IRIS_FMT_QC08C inline with v4l2 definition.

> 
>> +};
>> +
>>  struct iris_fmt {
>>  	u32 pixfmt;
>>  	u32 type;
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> index 36d69cc73986b74534a2912524c8553970fd862e..69c952c68e939f305f25511e2e4763487ec8e0de 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> @@ -691,6 +691,7 @@ static const u32 sm8550_venc_input_config_params[] = {
>>  };
>>  
>>  static const u32 sm8550_vdec_output_config_params[] = {
>> +	HFI_PROP_OPB_ENABLE,
>>  	HFI_PROP_COLOR_FORMAT,
>>  	HFI_PROP_LINEAR_STRIDE_SCANLINE,
>>  };
>> diff --git a/drivers/media/platform/qcom/iris/iris_utils.c b/drivers/media/platform/qcom/iris/iris_utils.c
>> index 85c70a62b1fd2c409fc18b28f64771cb0097a7fd..e2f1131de43128254d8211343771e657e425541e 100644
>> --- a/drivers/media/platform/qcom/iris/iris_utils.c
>> +++ b/drivers/media/platform/qcom/iris/iris_utils.c
>> @@ -34,7 +34,8 @@ int iris_get_mbpf(struct iris_inst *inst)
>>  
>>  bool iris_split_mode_enabled(struct iris_inst *inst)
>>  {
>> -	return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12;
>> +	return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12 ||
>> +		inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C;
>>  }
>>  
>>  void iris_helper_buffers_done(struct iris_inst *inst, unsigned int type,
>> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
>> index ae13c3e1b426bfd81a7b46dc6c3ff5eb5c4860cb..7fa745c6bf8950d192c2d2fc1770c4b6fd7b8c4c 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
>> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
>> @@ -67,7 +67,7 @@ void iris_vdec_inst_deinit(struct iris_inst *inst)
>>  	kfree(inst->fmt_src);
>>  }
>>  
>> -static const struct iris_fmt iris_vdec_formats[] = {
>> +static const struct iris_fmt iris_vdec_formats_out[] = {
>>  	[IRIS_FMT_H264] = {
>>  		.pixfmt = V4L2_PIX_FMT_H264,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>> @@ -82,12 +82,35 @@ static const struct iris_fmt iris_vdec_formats[] = {
>>  	},
>>  };
>>  
>> +static const struct iris_fmt iris_vdec_formats_cap[] = {
> 
> s/formats_cap/formats_capture/

Ok.

Thanks,
Dikshita

> 
>> +	[IRIS_FMT_NV12] = {
>> +		.pixfmt = V4L2_PIX_FMT_NV12,
>> +		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> +	},
>> +	[IRIS_FMT_UBWC] = {
>> +		.pixfmt = V4L2_PIX_FMT_QC08C,
>> +		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> +	},
>> +};
>> +
>>  static const struct iris_fmt *
>>  find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>>  {
>> -	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
>> -	const struct iris_fmt *fmt = iris_vdec_formats;
>> +	const struct iris_fmt *fmt = NULL;
>> +	unsigned int size = 0;
>>  	unsigned int i;
>> +	switch (type) {
>> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>> +		fmt = iris_vdec_formats_out;
>> +		size = ARRAY_SIZE(iris_vdec_formats_out);
>> +		break;
>> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> +		fmt = iris_vdec_formats_cap;
>> +		size = ARRAY_SIZE(iris_vdec_formats_cap);
>> +		break;
>> +	default:
>> +		return NULL;
>> +	}
>>  
>>  	for (i = 0; i < size; i++) {
>>  		if (fmt[i].pixfmt == pixfmt)
>> @@ -103,8 +126,21 @@ find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>>  static const struct iris_fmt *
>>  find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
>>  {
>> -	const struct iris_fmt *fmt = iris_vdec_formats;
>> -	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
>> +	const struct iris_fmt *fmt = NULL;
>> +	unsigned int size = 0;
>> +
>> +	switch (type) {
>> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>> +		fmt = iris_vdec_formats_out;
>> +		size = ARRAY_SIZE(iris_vdec_formats_out);
>> +		break;
>> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> +		fmt = iris_vdec_formats_cap;
>> +		size = ARRAY_SIZE(iris_vdec_formats_cap);
>> +		break;
>> +	default:
>> +		return NULL;
>> +	}
>>  
>>  	if (index >= size || fmt[index].type != type)
>>  		return NULL;
>> @@ -126,9 +162,10 @@ int iris_vdec_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f)
>>  		f->flags = V4L2_FMT_FLAG_COMPRESSED | V4L2_FMT_FLAG_DYN_RESOLUTION;
>>  		break;
>>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> -		if (f->index)
>> +		fmt = find_format_by_index(inst, f->index, f->type);
>> +		if (!fmt)
>>  			return -EINVAL;
>> -		f->pixelformat = V4L2_PIX_FMT_NV12;
>> +		f->pixelformat = fmt->pixfmt;
>>  		break;
>>  	default:
>>  		return -EINVAL;
>> @@ -157,7 +194,7 @@ int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f)
>>  		}
>>  		break;
>>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> -		if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) {
>> +		if (!fmt) {
>>  			f_inst = inst->fmt_dst;
>>  			f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat;
>>  			f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width;
>> @@ -238,10 +275,11 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
>>  		inst->crop.height = f->fmt.pix_mp.height;
>>  		break;
>>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> +		if (!(find_format(inst, f->fmt.pix_mp.pixelformat, f->type)))
>> +			return -EINVAL;
>> +
>>  		fmt = inst->fmt_dst;
>>  		fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>> -		if (fmt->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12)
>> -			return -EINVAL;
>>  		fmt->fmt.pix_mp.pixelformat = f->fmt.pix_mp.pixelformat;
>>  		fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
>>  		fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
>> @@ -268,7 +306,8 @@ int iris_vdec_validate_format(struct iris_inst *inst, u32 pixelformat)
>>  {
>>  	const struct iris_fmt *fmt = NULL;
>>  
>> -	if (pixelformat != V4L2_PIX_FMT_NV12) {
>> +	fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
>> +	if (!fmt) {
>>  		fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>>  		if (!fmt)
>>  			return -EINVAL;
>>
>> -- 
>> 2.34.1
>>
> 

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-19 15:47 ` [PATCH 1/2] media: iris: Add support for QC08C format for decoder Dikshita Agarwal
  2025-09-19 16:54   ` Dmitry Baryshkov
@ 2025-09-24 12:32   ` Markus Elfring
  2025-09-24 15:50     ` Nicolas Dufresne
  2025-09-24 13:28   ` Bryan O'Donoghue
  2 siblings, 1 reply; 20+ messages in thread
From: Markus Elfring @ 2025-09-24 12:32 UTC (permalink / raw)
  To: Dikshita Agarwal, linux-media, linux-arm-msm
  Cc: LKML, Abhinav Kumar, Bryan O'Donoghue, Mauro Carvalho Chehab,
	Vikash Garodia

…
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -261,7 +261,10 @@ int iris_get_buffer_size(struct iris_inst *inst,
>  		case BUF_INPUT:
>  			return iris_dec_bitstream_buffer_size(inst);
>  		case BUF_OUTPUT:
> -			return iris_yuv_buffer_size_nv12(inst);
> +			if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
> +				return iris_yuv_buffer_size_qc08c(inst);
> +			else
> +				return iris_yuv_buffer_size_nv12(inst);
…

How do you think about to use a source code variant like the following?

			return (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
				? iris_yuv_buffer_size_qc08c(inst)
				: iris_yuv_buffer_size_nv12(inst);


Regards,
Markus

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

* Re: [PATCH 2/2] media: iris: Add support for QC08C format for encoder
  2025-09-19 15:47 ` [PATCH 2/2] media: iris: Add support for QC08C format for encoder Dikshita Agarwal
@ 2025-09-24 12:40   ` Markus Elfring
  2025-09-24 13:31     ` Bryan O'Donoghue
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Elfring @ 2025-09-24 12:40 UTC (permalink / raw)
  To: Dikshita Agarwal, linux-media, linux-arm-msm
  Cc: LKML, Abhinav Kumar, Bryan O'Donoghue, Mauro Carvalho Chehab,
	Vikash Garodia

…
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -171,9 +171,14 @@ static u32 iris_yuv_buffer_size_nv12(struct iris_inst *inst)
>  static u32 iris_yuv_buffer_size_qc08c(struct iris_inst *inst)
>  {
>  	u32 y_plane, uv_plane, y_stride, uv_stride;
> -	struct v4l2_format *f = inst->fmt_dst;
>  	u32 uv_meta_stride, uv_meta_plane;
>  	u32 y_meta_stride, y_meta_plane;
> +	struct v4l2_format *f = NULL;
> +
> +	if (inst->domain == DECODER)
> +		f = inst->fmt_dst;
> +	else
> +		f = inst->fmt_src;
…

How do you think about to use a source code variant like the following?

	struct v4l2_format *f = (inst->domain == DECODER) ? inst->fmt_dst : inst->fmt_src;


Regards,
Markus

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-19 15:47 ` [PATCH 1/2] media: iris: Add support for QC08C format for decoder Dikshita Agarwal
  2025-09-19 16:54   ` Dmitry Baryshkov
  2025-09-24 12:32   ` Markus Elfring
@ 2025-09-24 13:28   ` Bryan O'Donoghue
  2025-10-01  8:36     ` Neil Armstrong
  2025-10-01  8:38     ` Dikshita Agarwal
  2 siblings, 2 replies; 20+ messages in thread
From: Bryan O'Donoghue @ 2025-09-24 13:28 UTC (permalink / raw)
  To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
	Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel

On 19/09/2025 16:47, Dikshita Agarwal wrote:
> Introduce handling for the QC08C format in the decoder.
> Update format checks and configuration to enable decoding of QC08C
> streams.

Since QC08C is a Qualcomm specific pixel format, you should detail in 
your commit log exactly what the packing/ordering of pixels is.

In other words tell the reader more about QC08C.

> 
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/iris/iris_buffer.c     |  5 +-
>   .../platform/qcom/iris/iris_hfi_gen1_command.c     | 12 +++--
>   .../platform/qcom/iris/iris_hfi_gen2_command.c     | 18 ++++++-
>   .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>   drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
>   .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
>   drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
>   drivers/media/platform/qcom/iris/iris_vdec.c       | 61 ++++++++++++++++++----
>   8 files changed, 89 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index c0900038e7defccf7de3cb60e17c71e36a0e8ead..83dcf49e57ec1473bc4edd26c48ab0b247d23818 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -261,7 +261,10 @@ int iris_get_buffer_size(struct iris_inst *inst,
>   		case BUF_INPUT:
>   			return iris_dec_bitstream_buffer_size(inst);
>   		case BUF_OUTPUT:
> -			return iris_yuv_buffer_size_nv12(inst);
> +			if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)

It'd be nice to get a pointer to the final level of indireciton you need 
generally, instead of these very->long->lists->of.indirections.

Please consider getting a final pointer as much as possible especially 
in functions where you end up writing those long chains over and over again.

> +				return iris_yuv_buffer_size_qc08c(inst);
> +			else
> +				return iris_yuv_buffer_size_nv12(inst);
>   		case BUF_DPB:
>   			return iris_yuv_buffer_size_qc08c(inst);
>   		default:
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index e1788c266bb1080921f17248fd5ee60156b3143d..e458d3349ce09aadb75d056ae84e3aab95f03078 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -774,20 +774,21 @@ static int iris_hfi_gen1_set_raw_format(struct iris_inst *inst, u32 plane)
>   		pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>   		if (iris_split_mode_enabled(inst)) {
>   			fmt.buffer_type = HFI_BUFFER_OUTPUT;
> -			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
> -				HFI_COLOR_FORMAT_NV12_UBWC : 0;
> +			fmt.format = HFI_COLOR_FORMAT_NV12_UBWC;
> 
>   			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>   			if (ret)
>   				return ret;
> 
>   			fmt.buffer_type = HFI_BUFFER_OUTPUT2;
> -			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
> +			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
> +				HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
> 
>   			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>   		} else {
>   			fmt.buffer_type = HFI_BUFFER_OUTPUT;
> -			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
> +			fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
> +				HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
> 
>   			ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>   		}
> @@ -806,6 +807,9 @@ static int iris_hfi_gen1_set_format_constraints(struct iris_inst *inst, u32 plan
>   	const u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_PLANE_ACTUAL_CONSTRAINTS_INFO;
>   	struct hfi_uncompressed_plane_actual_constraints_info pconstraint;
> 
> +	if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
> +		return 0;
> +
>   	pconstraint.buffer_type = HFI_BUFFER_OUTPUT2;
>   	pconstraint.num_planes = 2;
>   	pconstraint.plane_format[0].stride_multiples = 128;
> 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 4ce71a14250832440099e4cf3835b4aedfb749e8..5ad202d3fcdc57a2b7b43de15763a686ce0f7bd7 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> @@ -422,6 +422,20 @@ static int iris_hfi_gen2_set_level(struct iris_inst *inst, u32 plane)
>   						  sizeof(u32));
>   }
> 
> +static int iris_hfi_gen2_set_opb_enable(struct iris_inst *inst, u32 plane)
> +{
> +	u32 port = iris_hfi_gen2_get_port(inst, plane);
> +	u32 opb_enable = iris_split_mode_enabled(inst);
> +
> +	return iris_hfi_gen2_session_set_property(inst,
> +						  HFI_PROP_OPB_ENABLE,
> +						  HFI_HOST_FLAGS_NONE,
> +						  port,
> +						  HFI_PAYLOAD_U32,
> +						  &opb_enable,
> +						  sizeof(u32));
> +}
> +
>   static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
>   {
>   	u32 port = iris_hfi_gen2_get_port(inst, plane);
> @@ -429,7 +443,8 @@ 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 : 0;
> +		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
> +			HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
>   	} else {
>   		pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
>   		hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FMT_NV12 : 0;
> @@ -527,6 +542,7 @@ static int iris_hfi_gen2_session_set_config_params(struct iris_inst *inst, u32 p
>   		{HFI_PROP_SIGNAL_COLOR_INFO,          iris_hfi_gen2_set_colorspace             },
>   		{HFI_PROP_PROFILE,                    iris_hfi_gen2_set_profile                },
>   		{HFI_PROP_LEVEL,                      iris_hfi_gen2_set_level                  },
> +		{HFI_PROP_OPB_ENABLE,                 iris_hfi_gen2_set_opb_enable             },


As I read this code I end up asking myself "what does OPB" mean ?

For preference the introduction of OPB would be its own patch with its 
own commit log that explains OPB as an individual thing.

You can enable your QC08C format as an additional step.


>   		{HFI_PROP_COLOR_FORMAT,               iris_hfi_gen2_set_colorformat            },
>   		{HFI_PROP_LINEAR_STRIDE_SCANLINE,     iris_hfi_gen2_set_linear_stride_scanline },
>   		{HFI_PROP_TIER,                       iris_hfi_gen2_set_tier                   },
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> index aa1f795f5626c1f76a32dd650302633877ce67be..1b6a4dbac828ffea53c1be0d3624a033c283c972 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> @@ -91,6 +91,7 @@ enum hfi_seq_header_mode {
>   #define HFI_PROP_BUFFER_MARK			0x0300016c
>   #define HFI_PROP_RAW_RESOLUTION		0x03000178
>   #define HFI_PROP_TOTAL_PEAK_BITRATE		0x0300017C
> +#define HFI_PROP_OPB_ENABLE			0x03000184
>   #define HFI_PROP_COMV_BUFFER_COUNT		0x03000193
>   #define HFI_PROP_END				0x03FFFFFF
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
> index 5982d7adefeab80905478b32cddba7bd4651a691..11134f75c26dd1f6c92ba72fbf4e56e41a72de64 100644
> --- a/drivers/media/platform/qcom/iris/iris_instance.h
> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
> @@ -15,12 +15,17 @@
>   #define DEFAULT_WIDTH 320
>   #define DEFAULT_HEIGHT 240
> 
> -enum iris_fmt_type {
> +enum iris_fmt_type_out {
>   	IRIS_FMT_H264,
>   	IRIS_FMT_HEVC,
>   	IRIS_FMT_VP9,
>   };
> 
> +enum iris_fmt_type_cap {
> +	IRIS_FMT_NV12,
> +	IRIS_FMT_UBWC,
> +};
> +
>   struct iris_fmt {
>   	u32 pixfmt;
>   	u32 type;
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
> index 36d69cc73986b74534a2912524c8553970fd862e..69c952c68e939f305f25511e2e4763487ec8e0de 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
> @@ -691,6 +691,7 @@ static const u32 sm8550_venc_input_config_params[] = {
>   };
> 
>   static const u32 sm8550_vdec_output_config_params[] = {
> +	HFI_PROP_OPB_ENABLE,
>   	HFI_PROP_COLOR_FORMAT,
>   	HFI_PROP_LINEAR_STRIDE_SCANLINE,
>   };
> diff --git a/drivers/media/platform/qcom/iris/iris_utils.c b/drivers/media/platform/qcom/iris/iris_utils.c
> index 85c70a62b1fd2c409fc18b28f64771cb0097a7fd..e2f1131de43128254d8211343771e657e425541e 100644
> --- a/drivers/media/platform/qcom/iris/iris_utils.c
> +++ b/drivers/media/platform/qcom/iris/iris_utils.c
> @@ -34,7 +34,8 @@ int iris_get_mbpf(struct iris_inst *inst)
> 
>   bool iris_split_mode_enabled(struct iris_inst *inst)
>   {
> -	return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12;
> +	return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12 ||
> +		inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C;
>   }
> 
>   void iris_helper_buffers_done(struct iris_inst *inst, unsigned int type,
> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index ae13c3e1b426bfd81a7b46dc6c3ff5eb5c4860cb..7fa745c6bf8950d192c2d2fc1770c4b6fd7b8c4c 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
> @@ -67,7 +67,7 @@ void iris_vdec_inst_deinit(struct iris_inst *inst)
>   	kfree(inst->fmt_src);
>   }
> 
> -static const struct iris_fmt iris_vdec_formats[] = {
> +static const struct iris_fmt iris_vdec_formats_out[] = {
>   	[IRIS_FMT_H264] = {
>   		.pixfmt = V4L2_PIX_FMT_H264,
>   		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> @@ -82,12 +82,35 @@ static const struct iris_fmt iris_vdec_formats[] = {
>   	},
>   };
> 
> +static const struct iris_fmt iris_vdec_formats_cap[] = {
> +	[IRIS_FMT_NV12] = {
> +		.pixfmt = V4L2_PIX_FMT_NV12,
> +		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> +	},
> +	[IRIS_FMT_UBWC] = {
> +		.pixfmt = V4L2_PIX_FMT_QC08C,
> +		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> +	},
> +};
> +
>   static const struct iris_fmt *
>   find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>   {
> -	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
> -	const struct iris_fmt *fmt = iris_vdec_formats;
> +	const struct iris_fmt *fmt = NULL;
> +	unsigned int size = 0;
>   	unsigned int i;
> +	switch (type) {
> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> +		fmt = iris_vdec_formats_out;
> +		size = ARRAY_SIZE(iris_vdec_formats_out);
> +		break;
> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> +		fmt = iris_vdec_formats_cap;
> +		size = ARRAY_SIZE(iris_vdec_formats_cap);
> +		break;
> +	default:
> +		return NULL;
> +	}
> 
>   	for (i = 0; i < size; i++) {
>   		if (fmt[i].pixfmt == pixfmt)
> @@ -103,8 +126,21 @@ find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>   static const struct iris_fmt *
>   find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
>   {
> -	const struct iris_fmt *fmt = iris_vdec_formats;
> -	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
> +	const struct iris_fmt *fmt = NULL;
> +	unsigned int size = 0;
> +
> +	switch (type) {
> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> +		fmt = iris_vdec_formats_out;
> +		size = ARRAY_SIZE(iris_vdec_formats_out);
> +		break;
> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> +		fmt = iris_vdec_formats_cap;
> +		size = ARRAY_SIZE(iris_vdec_formats_cap);
> +		break;
> +	default:
> +		return NULL;
> +	}
> 
>   	if (index >= size || fmt[index].type != type)
>   		return NULL;
> @@ -126,9 +162,10 @@ int iris_vdec_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f)
>   		f->flags = V4L2_FMT_FLAG_COMPRESSED | V4L2_FMT_FLAG_DYN_RESOLUTION;
>   		break;
>   	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> -		if (f->index)
> +		fmt = find_format_by_index(inst, f->index, f->type);
> +		if (!fmt)
>   			return -EINVAL;
> -		f->pixelformat = V4L2_PIX_FMT_NV12;
> +		f->pixelformat = fmt->pixfmt;
>   		break;
>   	default:
>   		return -EINVAL;
> @@ -157,7 +194,7 @@ int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f)
>   		}
>   		break;
>   	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> -		if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) {
> +		if (!fmt) {
>   			f_inst = inst->fmt_dst;
>   			f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat;
>   			f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width;
> @@ -238,10 +275,11 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
>   		inst->crop.height = f->fmt.pix_mp.height;
>   		break;
>   	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> +		if (!(find_format(inst, f->fmt.pix_mp.pixelformat, f->type)))
> +			return -EINVAL;
> +
>   		fmt = inst->fmt_dst;
>   		fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> -		if (fmt->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12)
> -			return -EINVAL;
>   		fmt->fmt.pix_mp.pixelformat = f->fmt.pix_mp.pixelformat;
>   		fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
>   		fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
> @@ -268,7 +306,8 @@ int iris_vdec_validate_format(struct iris_inst *inst, u32 pixelformat)
>   {
>   	const struct iris_fmt *fmt = NULL;
> 
> -	if (pixelformat != V4L2_PIX_FMT_NV12) {
> +	fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	if (!fmt) {
>   		fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>   		if (!fmt)
>   			return -EINVAL;
> 
> --
> 2.34.1
> 


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

* Re: [PATCH 2/2] media: iris: Add support for QC08C format for encoder
  2025-09-24 12:40   ` Markus Elfring
@ 2025-09-24 13:31     ` Bryan O'Donoghue
  0 siblings, 0 replies; 20+ messages in thread
From: Bryan O'Donoghue @ 2025-09-24 13:31 UTC (permalink / raw)
  To: Markus Elfring, Dikshita Agarwal, linux-media, linux-arm-msm
  Cc: LKML, Abhinav Kumar, Mauro Carvalho Chehab, Vikash Garodia

On 24/09/2025 13:40, Markus Elfring wrote:
> …
>> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
>> @@ -171,9 +171,14 @@ static u32 iris_yuv_buffer_size_nv12(struct iris_inst *inst)
>>   static u32 iris_yuv_buffer_size_qc08c(struct iris_inst *inst)
>>   {
>>   	u32 y_plane, uv_plane, y_stride, uv_stride;
>> -	struct v4l2_format *f = inst->fmt_dst;
>>   	u32 uv_meta_stride, uv_meta_plane;
>>   	u32 y_meta_stride, y_meta_plane;
>> +	struct v4l2_format *f = NULL;
>> +
>> +	if (inst->domain == DECODER)
>> +		f = inst->fmt_dst;
>> +	else
>> +		f = inst->fmt_src;
> …
> 
> How do you think about to use a source code variant like the following?
> 
> 	struct v4l2_format *f = (inst->domain == DECODER) ? inst->fmt_dst : inst->fmt_src;
> 
> 
> Regards,
> Markus

My personal preference is for if / else as above.

Trying to encourage the Iris people to move away from ternary operators ...

---
bod

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-24 12:32   ` Markus Elfring
@ 2025-09-24 15:50     ` Nicolas Dufresne
  0 siblings, 0 replies; 20+ messages in thread
From: Nicolas Dufresne @ 2025-09-24 15:50 UTC (permalink / raw)
  To: Markus Elfring, Dikshita Agarwal, linux-media, linux-arm-msm
  Cc: LKML, Abhinav Kumar, Bryan O'Donoghue, Mauro Carvalho Chehab,
	Vikash Garodia

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

Hi,

Le mercredi 24 septembre 2025 à 14:32 +0200, Markus Elfring a écrit :
> …
> > +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> > @@ -261,7 +261,10 @@ int iris_get_buffer_size(struct iris_inst *inst,
> >  		case BUF_INPUT:
> >  			return iris_dec_bitstream_buffer_size(inst);
> >  		case BUF_OUTPUT:
> > -			return iris_yuv_buffer_size_nv12(inst);
> > +			if (inst->fmt_dst->fmt.pix_mp.pixelformat ==
> > V4L2_PIX_FMT_QC08C)
> > +				return iris_yuv_buffer_size_qc08c(inst);
> > +			else
> > +				return iris_yuv_buffer_size_nv12(inst);
> …
> 
> How do you think about to use a source code variant like the following?
> 
> 			return (inst->fmt_dst->fmt.pix_mp.pixelformat ==
> V4L2_PIX_FMT_QC08C)
> 				? iris_yuv_buffer_size_qc08c(inst)
> 				: iris_yuv_buffer_size_nv12(inst);

Please don't, this is less readable and have no explained technical advantages.

Nicolas

> 
> 
> Regards,
> Markus

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

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-24 10:54     ` Dikshita Agarwal
@ 2025-09-24 16:23       ` Dmitry Baryshkov
  2025-10-01  8:24         ` Dikshita Agarwal
  0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2025-09-24 16:23 UTC (permalink / raw)
  To: Dikshita Agarwal
  Cc: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel

On Wed, Sep 24, 2025 at 04:24:07PM +0530, Dikshita Agarwal wrote:
> 
> 
> On 9/19/2025 10:24 PM, Dmitry Baryshkov wrote:
> > On Fri, Sep 19, 2025 at 09:17:16PM +0530, Dikshita Agarwal wrote:
> >> Introduce handling for the QC08C format in the decoder.
> >> Update format checks and configuration to enable decoding of QC08C
> >> streams.
> > 
> > What is QC08C? Is it supported on all devices?
> 
> It's qualcomm specific compressed format, it's defined here
> https://elixir.bootlin.com/linux/v6.17-rc2/source/include/uapi/linux/videodev2.h#L820
> 
> And Yes, it's supported on all Qualcomm devices supported by iris driver.

Is it going to be supported by all platforms that are going to be
migrated from Venus to Iris?

Is it just NV12 + UBWC or something else?

> 
> > 
> >>
> >> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> >> ---
> >>  drivers/media/platform/qcom/iris/iris_buffer.c     |  5 +-
> >>  .../platform/qcom/iris/iris_hfi_gen1_command.c     | 12 +++--
> >>  .../platform/qcom/iris/iris_hfi_gen2_command.c     | 18 ++++++-
> >>  .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
> >>  drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
> >>  .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
> >>  drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
> >>  drivers/media/platform/qcom/iris/iris_vdec.c       | 61 ++++++++++++++++++----
> >>  8 files changed, 89 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
> >> index 5982d7adefeab80905478b32cddba7bd4651a691..11134f75c26dd1f6c92ba72fbf4e56e41a72de64 100644
> >> --- a/drivers/media/platform/qcom/iris/iris_instance.h
> >> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
> >> @@ -15,12 +15,17 @@
> >>  #define DEFAULT_WIDTH 320
> >>  #define DEFAULT_HEIGHT 240
> >>  
> >> -enum iris_fmt_type {
> >> +enum iris_fmt_type_out {
> >>  	IRIS_FMT_H264,
> >>  	IRIS_FMT_HEVC,
> >>  	IRIS_FMT_VP9,
> >>  };
> >>  
> >> +enum iris_fmt_type_cap {
> >> +	IRIS_FMT_NV12,
> >> +	IRIS_FMT_UBWC,
> > 
> > UBWC is not a format on its own, it's a modifier of the format. Please
> > come up with a better naming.
> 
> Sure, will rename this to IRIS_FMT_QC08C inline with v4l2 definition.

Ack.


-- 
With best wishes
Dmitry

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-24 16:23       ` Dmitry Baryshkov
@ 2025-10-01  8:24         ` Dikshita Agarwal
  0 siblings, 0 replies; 20+ messages in thread
From: Dikshita Agarwal @ 2025-10-01  8:24 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, linux-media, linux-arm-msm, linux-kernel



On 9/24/2025 9:53 PM, Dmitry Baryshkov wrote:
> On Wed, Sep 24, 2025 at 04:24:07PM +0530, Dikshita Agarwal wrote:
>>
>>
>> On 9/19/2025 10:24 PM, Dmitry Baryshkov wrote:
>>> On Fri, Sep 19, 2025 at 09:17:16PM +0530, Dikshita Agarwal wrote:
>>>> Introduce handling for the QC08C format in the decoder.
>>>> Update format checks and configuration to enable decoding of QC08C
>>>> streams.
>>>
>>> What is QC08C? Is it supported on all devices?
>>
>> It's qualcomm specific compressed format, it's defined here
>> https://elixir.bootlin.com/linux/v6.17-rc2/source/include/uapi/linux/videodev2.h#L820
>>
>> And Yes, it's supported on all Qualcomm devices supported by iris driver.
> 
> Is it going to be supported by all platforms that are going to be
> migrated from Venus to Iris?

Yes, for all hardware which supports UBWC.

> 
> Is it just NV12 + UBWC or something else?

Yes it's NV12 + UBWC compression.

> 
>>
>>>
>>>>
>>>> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
>>>> ---
>>>>  drivers/media/platform/qcom/iris/iris_buffer.c     |  5 +-
>>>>  .../platform/qcom/iris/iris_hfi_gen1_command.c     | 12 +++--
>>>>  .../platform/qcom/iris/iris_hfi_gen2_command.c     | 18 ++++++-
>>>>  .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>>>>  drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
>>>>  .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
>>>>  drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
>>>>  drivers/media/platform/qcom/iris/iris_vdec.c       | 61 ++++++++++++++++++----
>>>>  8 files changed, 89 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
>>>> index 5982d7adefeab80905478b32cddba7bd4651a691..11134f75c26dd1f6c92ba72fbf4e56e41a72de64 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_instance.h
>>>> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
>>>> @@ -15,12 +15,17 @@
>>>>  #define DEFAULT_WIDTH 320
>>>>  #define DEFAULT_HEIGHT 240
>>>>  
>>>> -enum iris_fmt_type {
>>>> +enum iris_fmt_type_out {
>>>>  	IRIS_FMT_H264,
>>>>  	IRIS_FMT_HEVC,
>>>>  	IRIS_FMT_VP9,
>>>>  };
>>>>  
>>>> +enum iris_fmt_type_cap {
>>>> +	IRIS_FMT_NV12,
>>>> +	IRIS_FMT_UBWC,
>>>
>>> UBWC is not a format on its own, it's a modifier of the format. Please
>>> come up with a better naming.
>>
>> Sure, will rename this to IRIS_FMT_QC08C inline with v4l2 definition.
> 
> Ack.
> 
> 

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-24 13:28   ` Bryan O'Donoghue
@ 2025-10-01  8:36     ` Neil Armstrong
  2025-10-01 15:04       ` Bryan O'Donoghue
  2025-10-01  8:38     ` Dikshita Agarwal
  1 sibling, 1 reply; 20+ messages in thread
From: Neil Armstrong @ 2025-10-01  8:36 UTC (permalink / raw)
  To: Bryan O'Donoghue, Dikshita Agarwal, Vikash Garodia,
	Abhinav Kumar, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel

On 9/24/25 15:28, Bryan O'Donoghue wrote:
> On 19/09/2025 16:47, Dikshita Agarwal wrote:
>> Introduce handling for the QC08C format in the decoder.
>> Update format checks and configuration to enable decoding of QC08C
>> streams.
> 
> Since QC08C is a Qualcomm specific pixel format, you should detail in your commit log exactly what the packing/ordering of pixels is.
> 
> In other words tell the reader more about QC08C.

This has been upstreamed 3y ago for venus, which is the same as iris:
https://lore.kernel.org/all/20220117155559.234026-1-stanimir.varbanov@linaro.org/

No need to re-explain it for iris, the format is the same.

Neil

> 
>>
>> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
>> ---
>>   drivers/media/platform/qcom/iris/iris_buffer.c     |  5 +-
>>   .../platform/qcom/iris/iris_hfi_gen1_command.c     | 12 +++--
>>   .../platform/qcom/iris/iris_hfi_gen2_command.c     | 18 ++++++-
>>   .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>>   drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
>>   .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
>>   drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
>>   drivers/media/platform/qcom/iris/iris_vdec.c       | 61 ++++++++++++++++++----
>>   8 files changed, 89 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
>> index c0900038e7defccf7de3cb60e17c71e36a0e8ead..83dcf49e57ec1473bc4edd26c48ab0b247d23818 100644
>> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
>> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
>> @@ -261,7 +261,10 @@ int iris_get_buffer_size(struct iris_inst *inst,
>>           case BUF_INPUT:
>>               return iris_dec_bitstream_buffer_size(inst);
>>           case BUF_OUTPUT:
>> -            return iris_yuv_buffer_size_nv12(inst);
>> +            if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
> 
> It'd be nice to get a pointer to the final level of indireciton you need generally, instead of these very->long->lists->of.indirections.
> 
> Please consider getting a final pointer as much as possible especially in functions where you end up writing those long chains over and over again.
> 
>> +                return iris_yuv_buffer_size_qc08c(inst);
>> +            else
>> +                return iris_yuv_buffer_size_nv12(inst);
>>           case BUF_DPB:
>>               return iris_yuv_buffer_size_qc08c(inst);
>>           default:
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> index e1788c266bb1080921f17248fd5ee60156b3143d..e458d3349ce09aadb75d056ae84e3aab95f03078 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> @@ -774,20 +774,21 @@ static int iris_hfi_gen1_set_raw_format(struct iris_inst *inst, u32 plane)
>>           pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>>           if (iris_split_mode_enabled(inst)) {
>>               fmt.buffer_type = HFI_BUFFER_OUTPUT;
>> -            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> -                HFI_COLOR_FORMAT_NV12_UBWC : 0;
>> +            fmt.format = HFI_COLOR_FORMAT_NV12_UBWC;
>>
>>               ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>>               if (ret)
>>                   return ret;
>>
>>               fmt.buffer_type = HFI_BUFFER_OUTPUT2;
>> -            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
>> +            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> +                HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
>>
>>               ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>>           } else {
>>               fmt.buffer_type = HFI_BUFFER_OUTPUT;
>> -            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FORMAT_NV12 : 0;
>> +            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> +                HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
>>
>>               ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>>           }
>> @@ -806,6 +807,9 @@ static int iris_hfi_gen1_set_format_constraints(struct iris_inst *inst, u32 plan
>>       const u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_PLANE_ACTUAL_CONSTRAINTS_INFO;
>>       struct hfi_uncompressed_plane_actual_constraints_info pconstraint;
>>
>> +    if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
>> +        return 0;
>> +
>>       pconstraint.buffer_type = HFI_BUFFER_OUTPUT2;
>>       pconstraint.num_planes = 2;
>>       pconstraint.plane_format[0].stride_multiples = 128;
>> 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 4ce71a14250832440099e4cf3835b4aedfb749e8..5ad202d3fcdc57a2b7b43de15763a686ce0f7bd7 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> @@ -422,6 +422,20 @@ static int iris_hfi_gen2_set_level(struct iris_inst *inst, u32 plane)
>>                             sizeof(u32));
>>   }
>>
>> +static int iris_hfi_gen2_set_opb_enable(struct iris_inst *inst, u32 plane)
>> +{
>> +    u32 port = iris_hfi_gen2_get_port(inst, plane);
>> +    u32 opb_enable = iris_split_mode_enabled(inst);
>> +
>> +    return iris_hfi_gen2_session_set_property(inst,
>> +                          HFI_PROP_OPB_ENABLE,
>> +                          HFI_HOST_FLAGS_NONE,
>> +                          port,
>> +                          HFI_PAYLOAD_U32,
>> +                          &opb_enable,
>> +                          sizeof(u32));
>> +}
>> +
>>   static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
>>   {
>>       u32 port = iris_hfi_gen2_get_port(inst, plane);
>> @@ -429,7 +443,8 @@ 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 : 0;
>> +        hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
>> +            HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
>>       } else {
>>           pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
>>           hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ? HFI_COLOR_FMT_NV12 : 0;
>> @@ -527,6 +542,7 @@ static int iris_hfi_gen2_session_set_config_params(struct iris_inst *inst, u32 p
>>           {HFI_PROP_SIGNAL_COLOR_INFO,          iris_hfi_gen2_set_colorspace             },
>>           {HFI_PROP_PROFILE,                    iris_hfi_gen2_set_profile                },
>>           {HFI_PROP_LEVEL,                      iris_hfi_gen2_set_level                  },
>> +        {HFI_PROP_OPB_ENABLE,                 iris_hfi_gen2_set_opb_enable             },
> 
> 
> As I read this code I end up asking myself "what does OPB" mean ?
> 
> For preference the introduction of OPB would be its own patch with its own commit log that explains OPB as an individual thing.
> 
> You can enable your QC08C format as an additional step.
> 
> 
>>           {HFI_PROP_COLOR_FORMAT,               iris_hfi_gen2_set_colorformat            },
>>           {HFI_PROP_LINEAR_STRIDE_SCANLINE,     iris_hfi_gen2_set_linear_stride_scanline },
>>           {HFI_PROP_TIER,                       iris_hfi_gen2_set_tier                   },
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> index aa1f795f5626c1f76a32dd650302633877ce67be..1b6a4dbac828ffea53c1be0d3624a033c283c972 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> @@ -91,6 +91,7 @@ enum hfi_seq_header_mode {
>>   #define HFI_PROP_BUFFER_MARK            0x0300016c
>>   #define HFI_PROP_RAW_RESOLUTION        0x03000178
>>   #define HFI_PROP_TOTAL_PEAK_BITRATE        0x0300017C
>> +#define HFI_PROP_OPB_ENABLE            0x03000184
>>   #define HFI_PROP_COMV_BUFFER_COUNT        0x03000193
>>   #define HFI_PROP_END                0x03FFFFFF
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
>> index 5982d7adefeab80905478b32cddba7bd4651a691..11134f75c26dd1f6c92ba72fbf4e56e41a72de64 100644
>> --- a/drivers/media/platform/qcom/iris/iris_instance.h
>> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
>> @@ -15,12 +15,17 @@
>>   #define DEFAULT_WIDTH 320
>>   #define DEFAULT_HEIGHT 240
>>
>> -enum iris_fmt_type {
>> +enum iris_fmt_type_out {
>>       IRIS_FMT_H264,
>>       IRIS_FMT_HEVC,
>>       IRIS_FMT_VP9,
>>   };
>>
>> +enum iris_fmt_type_cap {
>> +    IRIS_FMT_NV12,
>> +    IRIS_FMT_UBWC,
>> +};
>> +
>>   struct iris_fmt {
>>       u32 pixfmt;
>>       u32 type;
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> index 36d69cc73986b74534a2912524c8553970fd862e..69c952c68e939f305f25511e2e4763487ec8e0de 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> @@ -691,6 +691,7 @@ static const u32 sm8550_venc_input_config_params[] = {
>>   };
>>
>>   static const u32 sm8550_vdec_output_config_params[] = {
>> +    HFI_PROP_OPB_ENABLE,
>>       HFI_PROP_COLOR_FORMAT,
>>       HFI_PROP_LINEAR_STRIDE_SCANLINE,
>>   };
>> diff --git a/drivers/media/platform/qcom/iris/iris_utils.c b/drivers/media/platform/qcom/iris/iris_utils.c
>> index 85c70a62b1fd2c409fc18b28f64771cb0097a7fd..e2f1131de43128254d8211343771e657e425541e 100644
>> --- a/drivers/media/platform/qcom/iris/iris_utils.c
>> +++ b/drivers/media/platform/qcom/iris/iris_utils.c
>> @@ -34,7 +34,8 @@ int iris_get_mbpf(struct iris_inst *inst)
>>
>>   bool iris_split_mode_enabled(struct iris_inst *inst)
>>   {
>> -    return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12;
>> +    return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12 ||
>> +        inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C;
>>   }
>>
>>   void iris_helper_buffers_done(struct iris_inst *inst, unsigned int type,
>> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
>> index ae13c3e1b426bfd81a7b46dc6c3ff5eb5c4860cb..7fa745c6bf8950d192c2d2fc1770c4b6fd7b8c4c 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
>> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
>> @@ -67,7 +67,7 @@ void iris_vdec_inst_deinit(struct iris_inst *inst)
>>       kfree(inst->fmt_src);
>>   }
>>
>> -static const struct iris_fmt iris_vdec_formats[] = {
>> +static const struct iris_fmt iris_vdec_formats_out[] = {
>>       [IRIS_FMT_H264] = {
>>           .pixfmt = V4L2_PIX_FMT_H264,
>>           .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>> @@ -82,12 +82,35 @@ static const struct iris_fmt iris_vdec_formats[] = {
>>       },
>>   };
>>
>> +static const struct iris_fmt iris_vdec_formats_cap[] = {
>> +    [IRIS_FMT_NV12] = {
>> +        .pixfmt = V4L2_PIX_FMT_NV12,
>> +        .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> +    },
>> +    [IRIS_FMT_UBWC] = {
>> +        .pixfmt = V4L2_PIX_FMT_QC08C,
>> +        .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> +    },
>> +};
>> +
>>   static const struct iris_fmt *
>>   find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>>   {
>> -    unsigned int size = ARRAY_SIZE(iris_vdec_formats);
>> -    const struct iris_fmt *fmt = iris_vdec_formats;
>> +    const struct iris_fmt *fmt = NULL;
>> +    unsigned int size = 0;
>>       unsigned int i;
>> +    switch (type) {
>> +    case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>> +        fmt = iris_vdec_formats_out;
>> +        size = ARRAY_SIZE(iris_vdec_formats_out);
>> +        break;
>> +    case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> +        fmt = iris_vdec_formats_cap;
>> +        size = ARRAY_SIZE(iris_vdec_formats_cap);
>> +        break;
>> +    default:
>> +        return NULL;
>> +    }
>>
>>       for (i = 0; i < size; i++) {
>>           if (fmt[i].pixfmt == pixfmt)
>> @@ -103,8 +126,21 @@ find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>>   static const struct iris_fmt *
>>   find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
>>   {
>> -    const struct iris_fmt *fmt = iris_vdec_formats;
>> -    unsigned int size = ARRAY_SIZE(iris_vdec_formats);
>> +    const struct iris_fmt *fmt = NULL;
>> +    unsigned int size = 0;
>> +
>> +    switch (type) {
>> +    case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>> +        fmt = iris_vdec_formats_out;
>> +        size = ARRAY_SIZE(iris_vdec_formats_out);
>> +        break;
>> +    case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> +        fmt = iris_vdec_formats_cap;
>> +        size = ARRAY_SIZE(iris_vdec_formats_cap);
>> +        break;
>> +    default:
>> +        return NULL;
>> +    }
>>
>>       if (index >= size || fmt[index].type != type)
>>           return NULL;
>> @@ -126,9 +162,10 @@ int iris_vdec_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f)
>>           f->flags = V4L2_FMT_FLAG_COMPRESSED | V4L2_FMT_FLAG_DYN_RESOLUTION;
>>           break;
>>       case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> -        if (f->index)
>> +        fmt = find_format_by_index(inst, f->index, f->type);
>> +        if (!fmt)
>>               return -EINVAL;
>> -        f->pixelformat = V4L2_PIX_FMT_NV12;
>> +        f->pixelformat = fmt->pixfmt;
>>           break;
>>       default:
>>           return -EINVAL;
>> @@ -157,7 +194,7 @@ int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f)
>>           }
>>           break;
>>       case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> -        if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) {
>> +        if (!fmt) {
>>               f_inst = inst->fmt_dst;
>>               f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat;
>>               f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width;
>> @@ -238,10 +275,11 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
>>           inst->crop.height = f->fmt.pix_mp.height;
>>           break;
>>       case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> +        if (!(find_format(inst, f->fmt.pix_mp.pixelformat, f->type)))
>> +            return -EINVAL;
>> +
>>           fmt = inst->fmt_dst;
>>           fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>> -        if (fmt->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12)
>> -            return -EINVAL;
>>           fmt->fmt.pix_mp.pixelformat = f->fmt.pix_mp.pixelformat;
>>           fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
>>           fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
>> @@ -268,7 +306,8 @@ int iris_vdec_validate_format(struct iris_inst *inst, u32 pixelformat)
>>   {
>>       const struct iris_fmt *fmt = NULL;
>>
>> -    if (pixelformat != V4L2_PIX_FMT_NV12) {
>> +    fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
>> +    if (!fmt) {
>>           fmt = find_format(inst, pixelformat, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>>           if (!fmt)
>>               return -EINVAL;
>>
>> -- 
>> 2.34.1
>>
> 


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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-09-24 13:28   ` Bryan O'Donoghue
  2025-10-01  8:36     ` Neil Armstrong
@ 2025-10-01  8:38     ` Dikshita Agarwal
  1 sibling, 0 replies; 20+ messages in thread
From: Dikshita Agarwal @ 2025-10-01  8:38 UTC (permalink / raw)
  To: Bryan O'Donoghue, Vikash Garodia, Abhinav Kumar,
	Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel



On 9/24/2025 6:58 PM, Bryan O'Donoghue wrote:
> On 19/09/2025 16:47, Dikshita Agarwal wrote:
>> Introduce handling for the QC08C format in the decoder.
>> Update format checks and configuration to enable decoding of QC08C
>> streams.
> 
> Since QC08C is a Qualcomm specific pixel format, you should detail in your
> commit log exactly what the packing/ordering of pixels is.

I am just enabling support for QC08C format with this patch in iris driver
not defining it. The format is already described in v4l2 spec and the
packing is explained in detail[1]

[1]
https://elixir.bootlin.com/linux/v6.17-rc2/source/drivers/media/platform/qcom/iris/iris_buffer.c#L79

The only difference is, earlier it was enabled for interanl reference
buffers while this patch enables it for OPB (Output picture buffer) which
is used for display.

> 
> In other words tell the reader more about QC08C.
> 
>>
>> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
>> ---
>>   drivers/media/platform/qcom/iris/iris_buffer.c     |  5 +-
>>   .../platform/qcom/iris/iris_hfi_gen1_command.c     | 12 +++--
>>   .../platform/qcom/iris/iris_hfi_gen2_command.c     | 18 ++++++-
>>   .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>>   drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
>>   .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
>>   drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
>>   drivers/media/platform/qcom/iris/iris_vdec.c       | 61
>> ++++++++++++++++++----
>>   8 files changed, 89 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c
>> b/drivers/media/platform/qcom/iris/iris_buffer.c
>> index
>> c0900038e7defccf7de3cb60e17c71e36a0e8ead..83dcf49e57ec1473bc4edd26c48ab0b247d23818 100644
>> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
>> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
>> @@ -261,7 +261,10 @@ int iris_get_buffer_size(struct iris_inst *inst,
>>           case BUF_INPUT:
>>               return iris_dec_bitstream_buffer_size(inst);
>>           case BUF_OUTPUT:
>> -            return iris_yuv_buffer_size_nv12(inst);
>> +            if (inst->fmt_dst->fmt.pix_mp.pixelformat ==
>> V4L2_PIX_FMT_QC08C)
> 
> It'd be nice to get a pointer to the final level of indireciton you need
> generally, instead of these very->long->lists->of.indirections.

yeah, following the same in other parts of the code.
Here in this API, it is used only once that's why used directly.

> 
> Please consider getting a final pointer as much as possible especially in
> functions where you end up writing those long chains over and over again.
> 
>> +                return iris_yuv_buffer_size_qc08c(inst);
>> +            else
>> +                return iris_yuv_buffer_size_nv12(inst);
>>           case BUF_DPB:
>>               return iris_yuv_buffer_size_qc08c(inst);
>>           default:
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> index
>> e1788c266bb1080921f17248fd5ee60156b3143d..e458d3349ce09aadb75d056ae84e3aab95f03078 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> @@ -774,20 +774,21 @@ static int iris_hfi_gen1_set_raw_format(struct
>> iris_inst *inst, u32 plane)
>>           pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>>           if (iris_split_mode_enabled(inst)) {
>>               fmt.buffer_type = HFI_BUFFER_OUTPUT;
>> -            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> -                HFI_COLOR_FORMAT_NV12_UBWC : 0;
>> +            fmt.format = HFI_COLOR_FORMAT_NV12_UBWC;
>>
>>               ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>>               if (ret)
>>                   return ret;
>>
>>               fmt.buffer_type = HFI_BUFFER_OUTPUT2;
>> -            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> HFI_COLOR_FORMAT_NV12 : 0;
>> +            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> +                HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
>>
>>               ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>>           } else {
>>               fmt.buffer_type = HFI_BUFFER_OUTPUT;
>> -            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> HFI_COLOR_FORMAT_NV12 : 0;
>> +            fmt.format = pixelformat == V4L2_PIX_FMT_NV12 ?
>> +                HFI_COLOR_FORMAT_NV12 : HFI_COLOR_FORMAT_NV12_UBWC;
>>
>>               ret = hfi_gen1_set_property(inst, ptype, &fmt, sizeof(fmt));
>>           }
>> @@ -806,6 +807,9 @@ static int
>> iris_hfi_gen1_set_format_constraints(struct iris_inst *inst, u32 plan
>>       const u32 ptype =
>> HFI_PROPERTY_PARAM_UNCOMPRESSED_PLANE_ACTUAL_CONSTRAINTS_INFO;
>>       struct hfi_uncompressed_plane_actual_constraints_info pconstraint;
>>
>> +    if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
>> +        return 0;
>> +
>>       pconstraint.buffer_type = HFI_BUFFER_OUTPUT2;
>>       pconstraint.num_planes = 2;
>>       pconstraint.plane_format[0].stride_multiples = 128;
>> 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
>> 4ce71a14250832440099e4cf3835b4aedfb749e8..5ad202d3fcdc57a2b7b43de15763a686ce0f7bd7 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> @@ -422,6 +422,20 @@ static int iris_hfi_gen2_set_level(struct iris_inst
>> *inst, u32 plane)
>>                             sizeof(u32));
>>   }
>>
>> +static int iris_hfi_gen2_set_opb_enable(struct iris_inst *inst, u32 plane)
>> +{
>> +    u32 port = iris_hfi_gen2_get_port(inst, plane);
>> +    u32 opb_enable = iris_split_mode_enabled(inst);
>> +
>> +    return iris_hfi_gen2_session_set_property(inst,
>> +                          HFI_PROP_OPB_ENABLE,
>> +                          HFI_HOST_FLAGS_NONE,
>> +                          port,
>> +                          HFI_PAYLOAD_U32,
>> +                          &opb_enable,
>> +                          sizeof(u32));
>> +}
>> +
>>   static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32
>> plane)
>>   {
>>       u32 port = iris_hfi_gen2_get_port(inst, plane);
>> @@ -429,7 +443,8 @@ 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 : 0;
>> +        hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
>> +            HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
>>       } else {
>>           pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
>>           hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
>> HFI_COLOR_FMT_NV12 : 0;
>> @@ -527,6 +542,7 @@ static int
>> iris_hfi_gen2_session_set_config_params(struct iris_inst *inst, u32 p
>>           {HFI_PROP_SIGNAL_COLOR_INFO,         
>> iris_hfi_gen2_set_colorspace             },
>>           {HFI_PROP_PROFILE,                   
>> iris_hfi_gen2_set_profile                },
>>           {HFI_PROP_LEVEL,                     
>> iris_hfi_gen2_set_level                  },
>> +        {HFI_PROP_OPB_ENABLE,                
>> iris_hfi_gen2_set_opb_enable             },
> 
> 
> As I read this code I end up asking myself "what does OPB" mean ?
> 
> For preference the introduction of OPB would be its own patch with its own
> commit log that explains OPB as an individual thing.
> 
> You can enable your QC08C format as an additional step.
> 
> 
>>           {HFI_PROP_COLOR_FORMAT,              
>> iris_hfi_gen2_set_colorformat            },
>>           {HFI_PROP_LINEAR_STRIDE_SCANLINE,    
>> iris_hfi_gen2_set_linear_stride_scanline },
>>           {HFI_PROP_TIER,                      
>> iris_hfi_gen2_set_tier                   },
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> index
>> aa1f795f5626c1f76a32dd650302633877ce67be..1b6a4dbac828ffea53c1be0d3624a033c283c972 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> @@ -91,6 +91,7 @@ enum hfi_seq_header_mode {
>>   #define HFI_PROP_BUFFER_MARK            0x0300016c
>>   #define HFI_PROP_RAW_RESOLUTION        0x03000178
>>   #define HFI_PROP_TOTAL_PEAK_BITRATE        0x0300017C
>> +#define HFI_PROP_OPB_ENABLE            0x03000184
>>   #define HFI_PROP_COMV_BUFFER_COUNT        0x03000193
>>   #define HFI_PROP_END                0x03FFFFFF
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h
>> b/drivers/media/platform/qcom/iris/iris_instance.h
>> index
>> 5982d7adefeab80905478b32cddba7bd4651a691..11134f75c26dd1f6c92ba72fbf4e56e41a72de64 100644
>> --- a/drivers/media/platform/qcom/iris/iris_instance.h
>> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
>> @@ -15,12 +15,17 @@
>>   #define DEFAULT_WIDTH 320
>>   #define DEFAULT_HEIGHT 240
>>
>> -enum iris_fmt_type {
>> +enum iris_fmt_type_out {
>>       IRIS_FMT_H264,
>>       IRIS_FMT_HEVC,
>>       IRIS_FMT_VP9,
>>   };
>>
>> +enum iris_fmt_type_cap {
>> +    IRIS_FMT_NV12,
>> +    IRIS_FMT_UBWC,
>> +};
>> +
>>   struct iris_fmt {
>>       u32 pixfmt;
>>       u32 type;
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> index
>> 36d69cc73986b74534a2912524c8553970fd862e..69c952c68e939f305f25511e2e4763487ec8e0de 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> @@ -691,6 +691,7 @@ static const u32 sm8550_venc_input_config_params[] = {
>>   };
>>
>>   static const u32 sm8550_vdec_output_config_params[] = {
>> +    HFI_PROP_OPB_ENABLE,
>>       HFI_PROP_COLOR_FORMAT,
>>       HFI_PROP_LINEAR_STRIDE_SCANLINE,
>>   };
>> diff --git a/drivers/media/platform/qcom/iris/iris_utils.c
>> b/drivers/media/platform/qcom/iris/iris_utils.c
>> index
>> 85c70a62b1fd2c409fc18b28f64771cb0097a7fd..e2f1131de43128254d8211343771e657e425541e 100644
>> --- a/drivers/media/platform/qcom/iris/iris_utils.c
>> +++ b/drivers/media/platform/qcom/iris/iris_utils.c
>> @@ -34,7 +34,8 @@ int iris_get_mbpf(struct iris_inst *inst)
>>
>>   bool iris_split_mode_enabled(struct iris_inst *inst)
>>   {
>> -    return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12;
>> +    return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12 ||
>> +        inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C;
>>   }
>>
>>   void iris_helper_buffers_done(struct iris_inst *inst, unsigned int type,
>> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c
>> b/drivers/media/platform/qcom/iris/iris_vdec.c
>> index
>> ae13c3e1b426bfd81a7b46dc6c3ff5eb5c4860cb..7fa745c6bf8950d192c2d2fc1770c4b6fd7b8c4c 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
>> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
>> @@ -67,7 +67,7 @@ void iris_vdec_inst_deinit(struct iris_inst *inst)
>>       kfree(inst->fmt_src);
>>   }
>>
>> -static const struct iris_fmt iris_vdec_formats[] = {
>> +static const struct iris_fmt iris_vdec_formats_out[] = {
>>       [IRIS_FMT_H264] = {
>>           .pixfmt = V4L2_PIX_FMT_H264,
>>           .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>> @@ -82,12 +82,35 @@ static const struct iris_fmt iris_vdec_formats[] = {
>>       },
>>   };
>>
>> +static const struct iris_fmt iris_vdec_formats_cap[] = {
>> +    [IRIS_FMT_NV12] = {
>> +        .pixfmt = V4L2_PIX_FMT_NV12,
>> +        .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> +    },
>> +    [IRIS_FMT_UBWC] = {
>> +        .pixfmt = V4L2_PIX_FMT_QC08C,
>> +        .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> +    },
>> +};
>> +
>>   static const struct iris_fmt *
>>   find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>>   {
>> -    unsigned int size = ARRAY_SIZE(iris_vdec_formats);
>> -    const struct iris_fmt *fmt = iris_vdec_formats;
>> +    const struct iris_fmt *fmt = NULL;
>> +    unsigned int size = 0;
>>       unsigned int i;
>> +    switch (type) {
>> +    case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>> +        fmt = iris_vdec_formats_out;
>> +        size = ARRAY_SIZE(iris_vdec_formats_out);
>> +        break;
>> +    case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> +        fmt = iris_vdec_formats_cap;
>> +        size = ARRAY_SIZE(iris_vdec_formats_cap);
>> +        break;
>> +    default:
>> +        return NULL;
>> +    }
>>
>>       for (i = 0; i < size; i++) {
>>           if (fmt[i].pixfmt == pixfmt)
>> @@ -103,8 +126,21 @@ find_format(struct iris_inst *inst, u32 pixfmt, u32
>> type)
>>   static const struct iris_fmt *
>>   find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
>>   {
>> -    const struct iris_fmt *fmt = iris_vdec_formats;
>> -    unsigned int size = ARRAY_SIZE(iris_vdec_formats);
>> +    const struct iris_fmt *fmt = NULL;
>> +    unsigned int size = 0;
>> +
>> +    switch (type) {
>> +    case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>> +        fmt = iris_vdec_formats_out;
>> +        size = ARRAY_SIZE(iris_vdec_formats_out);
>> +        break;
>> +    case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> +        fmt = iris_vdec_formats_cap;
>> +        size = ARRAY_SIZE(iris_vdec_formats_cap);
>> +        break;
>> +    default:
>> +        return NULL;
>> +    }
>>
>>       if (index >= size || fmt[index].type != type)
>>           return NULL;
>> @@ -126,9 +162,10 @@ int iris_vdec_enum_fmt(struct iris_inst *inst,
>> struct v4l2_fmtdesc *f)
>>           f->flags = V4L2_FMT_FLAG_COMPRESSED |
>> V4L2_FMT_FLAG_DYN_RESOLUTION;
>>           break;
>>       case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> -        if (f->index)
>> +        fmt = find_format_by_index(inst, f->index, f->type);
>> +        if (!fmt)
>>               return -EINVAL;
>> -        f->pixelformat = V4L2_PIX_FMT_NV12;
>> +        f->pixelformat = fmt->pixfmt;
>>           break;
>>       default:
>>           return -EINVAL;
>> @@ -157,7 +194,7 @@ int iris_vdec_try_fmt(struct iris_inst *inst, struct
>> v4l2_format *f)
>>           }
>>           break;
>>       case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> -        if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) {
>> +        if (!fmt) {
>>               f_inst = inst->fmt_dst;
>>               f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat;
>>               f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width;
>> @@ -238,10 +275,11 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct
>> v4l2_format *f)
>>           inst->crop.height = f->fmt.pix_mp.height;
>>           break;
>>       case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> +        if (!(find_format(inst, f->fmt.pix_mp.pixelformat, f->type)))
>> +            return -EINVAL;
>> +
>>           fmt = inst->fmt_dst;
>>           fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>> -        if (fmt->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12)
>> -            return -EINVAL;
>>           fmt->fmt.pix_mp.pixelformat = f->fmt.pix_mp.pixelformat;
>>           fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
>>           fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
>> @@ -268,7 +306,8 @@ int iris_vdec_validate_format(struct iris_inst *inst,
>> u32 pixelformat)
>>   {
>>       const struct iris_fmt *fmt = NULL;
>>
>> -    if (pixelformat != V4L2_PIX_FMT_NV12) {
>> +    fmt = find_format(inst, pixelformat,
>> V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
>> +    if (!fmt) {
>>           fmt = find_format(inst, pixelformat,
>> V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>>           if (!fmt)
>>               return -EINVAL;
>>
>> -- 
>> 2.34.1
>>
> 

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

* Re: [PATCH 0/2] Add support for QC08C format in iris driver
  2025-09-19 15:47 [PATCH 0/2] Add support for QC08C format in iris driver Dikshita Agarwal
  2025-09-19 15:47 ` [PATCH 1/2] media: iris: Add support for QC08C format for decoder Dikshita Agarwal
  2025-09-19 15:47 ` [PATCH 2/2] media: iris: Add support for QC08C format for encoder Dikshita Agarwal
@ 2025-10-01  8:39 ` Neil Armstrong
  2025-10-01  9:00   ` Dikshita Agarwal
  2025-10-01 13:47   ` Nicolas Dufresne
  2 siblings, 2 replies; 20+ messages in thread
From: Neil Armstrong @ 2025-10-01  8:39 UTC (permalink / raw)
  To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel

Hi,

On 9/19/25 17:47, Dikshita Agarwal wrote:
> Add support for the QC08C color format in both the encoder and decoder
> paths of the iris driver. The changes include:
> 
> - Adding QC08C format handling in the driver for both encoding and
> decoding.
> - Updating format enumeration to properly return supported formats.
> - Ensuring the correct HFI format is set for firmware communication.
> -Making all related changes required for seamless integration of QC08C
> support.
> 
> The changes have been validated using v4l2-ctl, compliance, and GStreamer (GST) tests.
> Both GST and v4l2-ctl tests were performed using the NV12 format, as
> these clients do not support the QCOM-specific QC08C format, and all
> tests passed successfully.

Sorry but this means you didn't test the full decoding and encoding with GST and v4l2-ctl using QC08C ?
So how did you test ?

Thanks,
Neil

> 
> During v4l2-ctl testing, a regression was observed when using the NV12
> color format after adding QC08C support. A fix for this regression has
> also been posted [1].
> 
> [1]: https://lore.kernel.org/linux-media/20250918103235.4066441-1-dikshita.agarwal@oss.qualcomm.com/T/#u
> 
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
> Dikshita Agarwal (2):
>        media: iris: Add support for QC08C format for decoder
>        media: iris: Add support for QC08C format for encoder
> 
>   drivers/media/platform/qcom/iris/iris_buffer.c     | 17 ++++--
>   .../platform/qcom/iris/iris_hfi_gen1_command.c     | 15 ++++--
>   .../platform/qcom/iris/iris_hfi_gen2_command.c     | 21 +++++++-
>   .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>   drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
>   .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
>   drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
>   drivers/media/platform/qcom/iris/iris_vdec.c       | 61 ++++++++++++++++++----
>   drivers/media/platform/qcom/iris/iris_venc.c       | 59 +++++++++++++++++----
>   9 files changed, 152 insertions(+), 33 deletions(-)
> ---
> base-commit: 40b7a19f321e65789612ebaca966472055dab48c
> change-id: 20250918-video-iris-ubwc-enable-87eac6f41fa4
> 
> Best regards,


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

* Re: [PATCH 0/2] Add support for QC08C format in iris driver
  2025-10-01  8:39 ` [PATCH 0/2] Add support for QC08C format in iris driver Neil Armstrong
@ 2025-10-01  9:00   ` Dikshita Agarwal
  2025-10-01 13:47   ` Nicolas Dufresne
  1 sibling, 0 replies; 20+ messages in thread
From: Dikshita Agarwal @ 2025-10-01  9:00 UTC (permalink / raw)
  To: Neil Armstrong, Vikash Garodia, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel



On 10/1/2025 2:09 PM, Neil Armstrong wrote:
> Hi,
> 
> On 9/19/25 17:47, Dikshita Agarwal wrote:
>> Add support for the QC08C color format in both the encoder and decoder
>> paths of the iris driver. The changes include:
>>
>> - Adding QC08C format handling in the driver for both encoding and
>> decoding.
>> - Updating format enumeration to properly return supported formats.
>> - Ensuring the correct HFI format is set for firmware communication.
>> -Making all related changes required for seamless integration of QC08C
>> support.
>>
>> The changes have been validated using v4l2-ctl, compliance, and GStreamer
>> (GST) tests.
>> Both GST and v4l2-ctl tests were performed using the NV12 format, as
>> these clients do not support the QCOM-specific QC08C format, and all
>> tests passed successfully.
> 
> Sorry but this means you didn't test the full decoding and encoding with
> GST and v4l2-ctl using QC08C ?
> So how did you test ?

I have tested the decoding and decoding with QC08C using
https://github.com/quic/v4l-video-test-app

Thanks,
Dikshita

> 
> Thanks,
> Neil
> 
>>
>> During v4l2-ctl testing, a regression was observed when using the NV12
>> color format after adding QC08C support. A fix for this regression has
>> also been posted [1].
>>
>> [1]:
>> https://lore.kernel.org/linux-media/20250918103235.4066441-1-dikshita.agarwal@oss.qualcomm.com/T/#u
>>
>> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
>> ---
>> Dikshita Agarwal (2):
>>        media: iris: Add support for QC08C format for decoder
>>        media: iris: Add support for QC08C format for encoder
>>
>>   drivers/media/platform/qcom/iris/iris_buffer.c     | 17 ++++--
>>   .../platform/qcom/iris/iris_hfi_gen1_command.c     | 15 ++++--
>>   .../platform/qcom/iris/iris_hfi_gen2_command.c     | 21 +++++++-
>>   .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>>   drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
>>   .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
>>   drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
>>   drivers/media/platform/qcom/iris/iris_vdec.c       | 61
>> ++++++++++++++++++----
>>   drivers/media/platform/qcom/iris/iris_venc.c       | 59
>> +++++++++++++++++----
>>   9 files changed, 152 insertions(+), 33 deletions(-)
>> ---
>> base-commit: 40b7a19f321e65789612ebaca966472055dab48c
>> change-id: 20250918-video-iris-ubwc-enable-87eac6f41fa4
>>
>> Best regards,
> 

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

* Re: [PATCH 0/2] Add support for QC08C format in iris driver
  2025-10-01  8:39 ` [PATCH 0/2] Add support for QC08C format in iris driver Neil Armstrong
  2025-10-01  9:00   ` Dikshita Agarwal
@ 2025-10-01 13:47   ` Nicolas Dufresne
  1 sibling, 0 replies; 20+ messages in thread
From: Nicolas Dufresne @ 2025-10-01 13:47 UTC (permalink / raw)
  To: Neil Armstrong, Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel

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

Hi,

Le mercredi 01 octobre 2025 à 10:39 +0200, Neil Armstrong a écrit :
> Hi,
> 
> On 9/19/25 17:47, Dikshita Agarwal wrote:
> > Add support for the QC08C color format in both the encoder and decoder
> > paths of the iris driver. The changes include:
> > 
> > - Adding QC08C format handling in the driver for both encoding and
> > decoding.
> > - Updating format enumeration to properly return supported formats.
> > - Ensuring the correct HFI format is set for firmware communication.
> > -Making all related changes required for seamless integration of QC08C
> > support.
> > 
> > The changes have been validated using v4l2-ctl, compliance, and GStreamer
> > (GST) tests.
> > Both GST and v4l2-ctl tests were performed using the NV12 format, as
> > these clients do not support the QCOM-specific QC08C format, and all
> > tests passed successfully.
> 
> Sorry but this means you didn't test the full decoding and encoding with GST
> and v4l2-ctl using QC08C ?
> So how did you test ?

We've made addition of V4L2/DRM format mapping trivial lately in GStreamer. So
trivial, that my colleague Robert Mader made the changes for you, and this
change is just waiting for someone with the hardware to test.

https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8195

With that you can visually test it by rendering through OpenGL or your display
controller if supported. This gives option to use glimagesink, waylandsink, and
possibly kmssink, but not sure for the later.

What would need to work harder would be fluster testing. Going through GL will
mean converting from YUV to RGB back to YUV, which can damage the pictures
slightly, resulting in different MD5.

regards,
Nicolas

> 
> Thanks,
> Neil
> 
> > 
> > During v4l2-ctl testing, a regression was observed when using the NV12
> > color format after adding QC08C support. A fix for this regression has
> > also been posted [1].
> > 
> > [1]:
> > https://lore.kernel.org/linux-media/20250918103235.4066441-1-dikshita.agarwal@oss.qualcomm.com/T/#u
> > 
> > Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> > ---
> > Dikshita Agarwal (2):
> >        media: iris: Add support for QC08C format for decoder
> >        media: iris: Add support for QC08C format for encoder
> > 
> >   drivers/media/platform/qcom/iris/iris_buffer.c     | 17 ++++--
> >   .../platform/qcom/iris/iris_hfi_gen1_command.c     | 15 ++++--
> >   .../platform/qcom/iris/iris_hfi_gen2_command.c     | 21 +++++++-
> >   .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
> >   drivers/media/platform/qcom/iris/iris_instance.h   |  7 ++-
> >   .../media/platform/qcom/iris/iris_platform_gen2.c  |  1 +
> >   drivers/media/platform/qcom/iris/iris_utils.c      |  3 +-
> >   drivers/media/platform/qcom/iris/iris_vdec.c       | 61
> > ++++++++++++++++++----
> >   drivers/media/platform/qcom/iris/iris_venc.c       | 59 +++++++++++++++++-
> > ---
> >   9 files changed, 152 insertions(+), 33 deletions(-)
> > ---
> > base-commit: 40b7a19f321e65789612ebaca966472055dab48c
> > change-id: 20250918-video-iris-ubwc-enable-87eac6f41fa4
> > 
> > Best regards,
> 

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

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-10-01  8:36     ` Neil Armstrong
@ 2025-10-01 15:04       ` Bryan O'Donoghue
  2025-10-06  6:21         ` Dikshita Agarwal
  0 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2025-10-01 15:04 UTC (permalink / raw)
  To: Neil Armstrong, Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
	Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel

On 01/10/2025 09:36, Neil Armstrong wrote:
> On 9/24/25 15:28, Bryan O'Donoghue wrote:
>> On 19/09/2025 16:47, Dikshita Agarwal wrote:
>>> Introduce handling for the QC08C format in the decoder.
>>> Update format checks and configuration to enable decoding of QC08C
>>> streams.
>>
>> Since QC08C is a Qualcomm specific pixel format, you should detail in your commit log exactly what the packing/ordering of pixels is.
>>
>> In other words tell the reader more about QC08C.
> 
> This has been upstreamed 3y ago for venus, which is the same as iris:
> https://lore.kernel.org/all/20220117155559.234026-1-stanimir.varbanov@linaro.org/
> 
> No need to re-explain it for iris, the format is the same.
> 
> Neil
Yeah no, at a minimum the explanation of NV12 + UBWC should appear in 
the commit log for this format.

thx
---
bod

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-10-01 15:04       ` Bryan O'Donoghue
@ 2025-10-06  6:21         ` Dikshita Agarwal
  2025-10-06 10:18           ` Dmitry Baryshkov
  0 siblings, 1 reply; 20+ messages in thread
From: Dikshita Agarwal @ 2025-10-06  6:21 UTC (permalink / raw)
  To: Bryan O'Donoghue, Neil Armstrong, Vikash Garodia,
	Abhinav Kumar, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel



On 10/1/2025 8:34 PM, Bryan O'Donoghue wrote:
> On 01/10/2025 09:36, Neil Armstrong wrote:
>> On 9/24/25 15:28, Bryan O'Donoghue wrote:
>>> On 19/09/2025 16:47, Dikshita Agarwal wrote:
>>>> Introduce handling for the QC08C format in the decoder.
>>>> Update format checks and configuration to enable decoding of QC08C
>>>> streams.
>>>
>>> Since QC08C is a Qualcomm specific pixel format, you should detail in
>>> your commit log exactly what the packing/ordering of pixels is.
>>>
>>> In other words tell the reader more about QC08C.
>>
>> This has been upstreamed 3y ago for venus, which is the same as iris:
>> https://lore.kernel.org/all/20220117155559.234026-1-stanimir.varbanov@linaro.org/
>>
>> No need to re-explain it for iris, the format is the same.
>>
>> Neil
> Yeah no, at a minimum the explanation of NV12 + UBWC should appear in the
> commit log for this format.

Please see [1] in case it was missed

[1]:
https://lore.kernel.org/linux-media/10bb819d-105b-5471-b3a6-774fce134eb6@oss.qualcomm.com/

Thanks,
Dikshita
> 
> thx
> ---
> bod

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

* Re: [PATCH 1/2] media: iris: Add support for QC08C format for decoder
  2025-10-06  6:21         ` Dikshita Agarwal
@ 2025-10-06 10:18           ` Dmitry Baryshkov
  0 siblings, 0 replies; 20+ messages in thread
From: Dmitry Baryshkov @ 2025-10-06 10:18 UTC (permalink / raw)
  To: Dikshita Agarwal
  Cc: Bryan O'Donoghue, Neil Armstrong, Vikash Garodia,
	Abhinav Kumar, Mauro Carvalho Chehab, linux-media, linux-arm-msm,
	linux-kernel

On Mon, Oct 06, 2025 at 11:51:19AM +0530, Dikshita Agarwal wrote:
> 
> 
> On 10/1/2025 8:34 PM, Bryan O'Donoghue wrote:
> > On 01/10/2025 09:36, Neil Armstrong wrote:
> >> On 9/24/25 15:28, Bryan O'Donoghue wrote:
> >>> On 19/09/2025 16:47, Dikshita Agarwal wrote:
> >>>> Introduce handling for the QC08C format in the decoder.
> >>>> Update format checks and configuration to enable decoding of QC08C
> >>>> streams.
> >>>
> >>> Since QC08C is a Qualcomm specific pixel format, you should detail in
> >>> your commit log exactly what the packing/ordering of pixels is.
> >>>
> >>> In other words tell the reader more about QC08C.
> >>
> >> This has been upstreamed 3y ago for venus, which is the same as iris:
> >> https://lore.kernel.org/all/20220117155559.234026-1-stanimir.varbanov@linaro.org/
> >>
> >> No need to re-explain it for iris, the format is the same.
> >>
> >> Neil
> > Yeah no, at a minimum the explanation of NV12 + UBWC should appear in the
> > commit log for this format.
> 
> Please see [1] in case it was missed

Just mentioning that QC08C is NV12 with UBWC compression wouldn't harm
and it would make everybody's life easier.

> 
> [1]:
> https://lore.kernel.org/linux-media/10bb819d-105b-5471-b3a6-774fce134eb6@oss.qualcomm.com/
> 
> Thanks,
> Dikshita
> > 
> > thx
> > ---
> > bod

-- 
With best wishes
Dmitry

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

end of thread, other threads:[~2025-10-06 10:18 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-19 15:47 [PATCH 0/2] Add support for QC08C format in iris driver Dikshita Agarwal
2025-09-19 15:47 ` [PATCH 1/2] media: iris: Add support for QC08C format for decoder Dikshita Agarwal
2025-09-19 16:54   ` Dmitry Baryshkov
2025-09-24 10:54     ` Dikshita Agarwal
2025-09-24 16:23       ` Dmitry Baryshkov
2025-10-01  8:24         ` Dikshita Agarwal
2025-09-24 12:32   ` Markus Elfring
2025-09-24 15:50     ` Nicolas Dufresne
2025-09-24 13:28   ` Bryan O'Donoghue
2025-10-01  8:36     ` Neil Armstrong
2025-10-01 15:04       ` Bryan O'Donoghue
2025-10-06  6:21         ` Dikshita Agarwal
2025-10-06 10:18           ` Dmitry Baryshkov
2025-10-01  8:38     ` Dikshita Agarwal
2025-09-19 15:47 ` [PATCH 2/2] media: iris: Add support for QC08C format for encoder Dikshita Agarwal
2025-09-24 12:40   ` Markus Elfring
2025-09-24 13:31     ` Bryan O'Donoghue
2025-10-01  8:39 ` [PATCH 0/2] Add support for QC08C format in iris driver Neil Armstrong
2025-10-01  9:00   ` Dikshita Agarwal
2025-10-01 13:47   ` Nicolas Dufresne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox