[PATCH] media: iris: Retain firmware confirmed video_format across GOP restarts

Vishnu Reddy posted 1 patch 3 weeks, 3 days ago
There is a newer version of this series
drivers/media/platform/qcom/iris/iris_hfi_common.h        | 1 +
drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c  | 3 ++-
drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 2 ++
3 files changed, 5 insertions(+), 1 deletion(-)
[PATCH] media: iris: Retain firmware confirmed video_format across GOP restarts
Posted by Vishnu Reddy 3 weeks, 3 days ago
During speed-based rewind, the client restarts the decoder queues once
per group of pictures (GOP), because playing a GOP-based in reverse
means decoding each group forward first and then showing its frames in
reverse order, and every time this happens the driver resends the colour
info property on the bitstream port, which firmware always treats as a
sign that the stream's properties may have changed. The real problem was
that the driver never stored the video_format value that firmware had
last confirmed, so each time colour info was resent, it used a fixed
unspecified value instead of the real one, and value firmware received
kept differing from what it already had. On every restart during rewind,
this looked to firmware like a change on the bitstream port, so firmware
sent a settings-change notification, the driver treated it as a dynamic
resolution change and paused the port, and the client removed its buffers
and built new ones for a resolution that had not actually changed, causing
playback to stall once per group of pictures. By storing and sending back
the same value firmware already confirmed, both sides stay in agreement
across every restart, so a normal restart no longer looks like a real
change, firmware does not send a false settings-change notification, and
rewind can continue without stalling.

Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_hfi_common.h        | 1 +
 drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c  | 3 ++-
 drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.h b/drivers/media/platform/qcom/iris/iris_hfi_common.h
index a27447eb2519..439197e9e199 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_common.h
+++ b/drivers/media/platform/qcom/iris/iris_hfi_common.h
@@ -141,6 +141,7 @@ struct hfi_subscription_params {
 	u32	fw_min_count;
 	u32	pic_order_cnt;
 	u32	color_info;
+	u32	video_format;
 	u32	profile;
 	u32	level;
 	u32	tier;
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 388a36ff2b07..a43a9d7687ad 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
@@ -358,7 +358,7 @@ static int iris_hfi_gen2_set_colorspace(struct iris_inst *inst, u32 plane)
 	struct v4l2_pix_format_mplane *pixmp = &inst->fmt_src->fmt.pix_mp;
 	u32 video_signal_type_present_flag = 0, color_info;
 	u32 matrix_coeff = HFI_MATRIX_COEFF_RESERVED;
-	u32 video_format = UNSPECIFIED_COLOR_FORMAT;
+	u32 video_format = inst_hfi_gen2->src_subcr_params.video_format;
 	u32 full_range = V4L2_QUANTIZATION_DEFAULT;
 	u32 transfer_char = HFI_TRANSFER_RESERVED;
 	u32 port = iris_hfi_gen2_get_port(inst, plane);
@@ -806,6 +806,7 @@ static int iris_hfi_gen2_session_open(struct iris_inst *inst)
 
 	inst_hfi_gen2->ipsc_properties_set = false;
 	inst_hfi_gen2->opsc_properties_set = false;
+	inst_hfi_gen2->src_subcr_params.video_format = UNSPECIFIED_COLOR_FORMAT;
 
 	inst_hfi_gen2->packet = kzalloc(4096, GFP_KERNEL);
 	if (!inst_hfi_gen2->packet)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
index 8c2644c7f6e8..8b1ff253a560 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
@@ -581,6 +581,8 @@ static void iris_hfi_gen2_read_input_subcr_params(struct iris_inst *inst)
 	full_range = (subsc_params.color_info & 0x2000000) >> 25;
 	video_signal_type_present_flag =
 		(subsc_params.color_info & 0x20000000) >> 29;
+	inst_hfi_gen2->src_subcr_params.video_format =
+		(subsc_params.color_info & 0x1C000000) >> 26;
 
 	pixmp_op->colorspace = V4L2_COLORSPACE_DEFAULT;
 	pixmp_op->xfer_func = V4L2_XFER_FUNC_DEFAULT;

---
base-commit: 89c07d98716a13454ec3fd9f97689e812cc71bd4
change-id: 20260901-read_video_format_from_firmware-e5341e334412

Best regards,
--  
Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Re: [PATCH] media: iris: Retain firmware confirmed video_format across GOP restarts
Posted by Konrad Dybcio 3 weeks, 3 days ago
On 9/1/26 3:31 PM, Vishnu Reddy wrote:
> During speed-based rewind, the client restarts the decoder queues once
> per group of pictures (GOP), because playing a GOP-based in reverse

"playing a GOP-based [missing word?] in reverse"

> means decoding each group forward first and then showing its frames in
> reverse order, and every time this happens the driver resends the colour
> info property on the bitstream port, which firmware always treats as a
> sign that the stream's properties may have changed. 

This is a single sentence and it's way too long to parse.

> The real problem was
> that the driver never stored the video_format value that firmware had
> last confirmed, so each time colour info was resent, it used a fixed
> unspecified value instead of the real one, and value firmware received
> kept differing from what it already had. 

[newline]

> On every restart during rewind,
> this looked to firmware like a change on the bitstream port, so firmware
> sent a settings-change notification, the driver treated it as a dynamic
> resolution change and paused the port, 

[full stop]

> and the client removed its buffers

"The client then removed its buffers.."

> and built new ones for a resolution that had not actually changed, causing
> playback to stall once per group of pictures. By storing and sending back

Please use imperative mood for describing the actual action you're taking
(per Documentation/process/submitting-patches.rst)

Konrad