linux-kernel - Re: [PATCH v3] media: iris: Refine internal buffer reconfiguration logic for resolution change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <vt77gtk5fjwd4z4g5ppuwa7552y27nydnniikz64khzgif4qbg@rbzu3alfbg6x>
Date: Mon, 10 Nov 2025 14:42:35 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>
Cc: Vikash Garodia <vikash.garodia@....qualcomm.com>,
        Abhinav Kumar <abhinav.kumar@...ux.dev>,
        Bryan O'Donoghue <bod@...nel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Hans Verkuil <hverkuil@...nel.org>,
        Stefan Schmidt <stefan.schmidt@...aro.org>,
        linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Val Packett <val@...kett.cool>
Subject: Re: [PATCH v3] media: iris: Refine internal buffer reconfiguration
 logic for resolution change

On Mon, Nov 10, 2025 at 02:31:20PM +0530, Dikshita Agarwal wrote:
> 
> 
> On 11/10/2025 2:46 AM, Dmitry Baryshkov wrote:
> > On Wed, Nov 05, 2025 at 11:17:37AM +0530, Dikshita Agarwal wrote:
> >> Improve the condition used to determine when input internal buffers need
> >> to be reconfigured during streamon on the capture port. Previously, the
> >> check relied on the INPUT_PAUSE sub-state, which was also being set
> >> during seek operations. This led to input buffers being queued multiple
> >> times to the firmware, causing session errors due to duplicate buffer
> >> submissions.
> >>
> >> This change introduces a more accurate check using the FIRST_IPSC and
> >> DRC sub-states to ensure that input buffer reconfiguration is triggered
> >> only during resolution change scenarios, such as streamoff/on on the
> >> capture port. This avoids duplicate buffer queuing during seek
> >> operations.
> >>
> >> Fixes: c1f8b2cc72ec ("media: iris: handle streamoff/on from client in dynamic resolution change")
> >> Cc: stable@...r.kernel.org
> >> Reported-by: Val Packett <val@...kett.cool>
> >> Closes: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4700
> >> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>
> >> ---
> >> Changes in v3:
> >> - Fixed the compilation issue
> >> - Added stable@...r.kernel.org in Cc
> >> - Link to v2: https://lore.kernel.org/r/20251104-iris-seek-fix-v2-1-c9dace39b43d@oss.qualcomm.com
> >>
> >> Changes in v2:
> >> - Removed spurious space and addressed other comments (Nicolas)
> >> - Remove the unnecessary initializations (Self) 
> >> - Link to v1: https://lore.kernel.org/r/20251103-iris-seek-fix-v1-1-6db5f5e17722@oss.qualcomm.com
> >> ---
> >>  drivers/media/platform/qcom/iris/iris_common.c | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/qcom/iris/iris_common.c b/drivers/media/platform/qcom/iris/iris_common.c
> >> index 9fc663bdaf3fc989fe1273b4d4280a87f68de85d..7f1c7fe144f707accc2e3da65ce37cd6d9dfeaff 100644
> >> --- a/drivers/media/platform/qcom/iris/iris_common.c
> >> +++ b/drivers/media/platform/qcom/iris/iris_common.c
> >> @@ -91,12 +91,14 @@ int iris_process_streamon_input(struct iris_inst *inst)
> >>  int iris_process_streamon_output(struct iris_inst *inst)
> >>  {
> >>  	const struct iris_hfi_command_ops *hfi_ops = inst->core->hfi_ops;
> >> -	bool drain_active = false, drc_active = false;
> >>  	enum iris_inst_sub_state clear_sub_state = 0;
> >> +	bool drain_active, drc_active, first_ipsc;
> >>  	int ret = 0;
> >>  
> >>  	iris_scale_power(inst);
> >>  
> >> +	first_ipsc = inst->sub_state & IRIS_INST_SUB_FIRST_IPSC;
> >> +
> >>  	drain_active = inst->sub_state & IRIS_INST_SUB_DRAIN &&
> >>  		inst->sub_state & IRIS_INST_SUB_DRAIN_LAST;
> >>  
> >> @@ -108,7 +110,8 @@ int iris_process_streamon_output(struct iris_inst *inst)
> >>  	else if (drain_active)
> >>  		clear_sub_state = IRIS_INST_SUB_DRAIN | IRIS_INST_SUB_DRAIN_LAST;
> >>  
> >> -	if (inst->domain == DECODER && inst->sub_state & IRIS_INST_SUB_INPUT_PAUSE) {
> >> +	/* Input internal buffer reconfiguration required in case of resolution change */
> >> +	if (first_ipsc || drc_active) {
> >>  		ret = iris_alloc_and_queue_input_int_bufs(inst);
> >>  		if (ret)
> >>  			return ret;
> > 
> > I will repeat my (unanswered) question from v2:
> > 
> > After this line comes manual writing of STAGE and PIPE. Could you please
> > point out where is the driver updating the resolution in the firmware?
> > And if it does, why do we need to write STAGE and PIPE again?
> 
> Sorry for late reply,
> 
> During streamon on the output port, the driver sets the resolution in the
> firmware. However, during Dynamic Resolution Change (DRC), the resolution
> update originates from the firmware and is communicated to the driver. As a
> result, the driver does not proactively update the resolution in the
> firmware during DRC.
> 
> STAGE parameter depends on the resolution, the driver must update the
> firmware with the new STAGE value after a resolution change to ensure
> proper operation.
> 
> On the other hand, the PIPE value is independent of resolution. It is
> typically updated to 1 for interlaced content, which is identified during
> the sequence change. Currently, the Iris driver does not support interlaced
> content, so updating the PIPE value during DRC handling is redundant.
> However, this update is harmless and will be necessary once interlace
> support is added in the future.

Ack, thanks for the explanation.

-- 
With best wishes
Dmitry