Re: [PATCH v2 04/23] media: iris: Avoid updating frame size to firmware during reconfig

[Vikash Garodia <quic_vgarodia@quicinc.com>]

On 4/28/2025 2:58 PM, Dikshita Agarwal wrote:
> During reconfig, the firmware sends the resolution aligned to 8 bytes.
> If the driver sends the same resolution back to the firmware the resolution
> will be aligned to 16 bytes not 8.
> 
> The alignment mismatch would then subsequently cause the firmware to
> send another redundant sequence change.
change event.
> 
> Fix this by not setting the resolution property during reconfig.
> 
> Cc: stable@vger.kernel.org
> Fixes: 3a19d7b9e08b ("media: iris: implement set properties to firmware during streamon")
> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> ---
>  drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c  | 15 ++++++++-------
>  drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c |  1 +
>  drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c |  1 +
>  drivers/media/platform/qcom/iris/iris_instance.h          |  2 ++
>  drivers/media/platform/qcom/iris/iris_vdec.c              |  1 +
>  5 files changed, 13 insertions(+), 7 deletions(-)
> 
> 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 64f887d9a17d..2239708d2d7e 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -546,14 +546,15 @@ static int iris_hfi_gen1_set_resolution(struct iris_inst *inst)
>  	struct hfi_framesize fs;
>  	int ret;
>  
> -	fs.buffer_type = HFI_BUFFER_INPUT;
> -	fs.width = inst->fmt_src->fmt.pix_mp.width;
> -	fs.height = inst->fmt_src->fmt.pix_mp.height;
> -
> -	ret = hfi_gen1_set_property(inst, ptype, &fs, sizeof(fs));
> -	if (ret)
> -		return ret;
> +	if (!inst->in_reconfig) {
> +		fs.buffer_type = HFI_BUFFER_INPUT;
> +		fs.width = inst->fmt_src->fmt.pix_mp.width;
> +		fs.height = inst->fmt_src->fmt.pix_mp.height;
>  
> +		ret = hfi_gen1_set_property(inst, ptype, &fs, sizeof(fs));
> +		if (ret)
> +			return ret;
> +	}
>  	fs.buffer_type = HFI_BUFFER_OUTPUT2;
>  	fs.width = inst->fmt_dst->fmt.pix_mp.width;
>  	fs.height = inst->fmt_dst->fmt.pix_mp.height;
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> index 91d95eed68aa..6576496fdbdf 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> @@ -155,6 +155,7 @@ static void iris_hfi_gen1_read_changed_params(struct iris_inst *inst,
>  		inst->crop.height = event.height;
>  	}
>  
> +	inst->in_reconfig = true;
>  	inst->fw_min_count = event.buf_count;
>  	inst->buffers[BUF_OUTPUT].min_count = iris_vpu_buf_count(inst, BUF_OUTPUT);
>  	inst->buffers[BUF_OUTPUT].size = pixmp_op->plane_fmt[0].sizeimage;
> 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 b75a01641d5d..0e889d07e997 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> @@ -508,6 +508,7 @@ static void iris_hfi_gen2_read_input_subcr_params(struct iris_inst *inst)
>  	struct vb2_queue *dst_q;
>  	struct v4l2_ctrl *ctrl;
>  
> +	inst->in_reconfig = true;
>  	subsc_params = inst_hfi_gen2->src_subcr_params;
>  	width = (subsc_params.bitstream_resolution &
>  		HFI_BITMASK_BITSTREAM_WIDTH) >> 16;
> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
> index caa3c6507006..a893751766ca 100644
> --- a/drivers/media/platform/qcom/iris/iris_instance.h
> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
> @@ -42,6 +42,7 @@
>   * @sequence_out: a sequence counter for output queue
>   * @tss: timestamp metadata
>   * @metadata_idx: index for metadata buffer
> + * @in_reconfig: a flag raised by decoder when the stream resolution changes
>   */
use the DRC sub-state instead of in_reconfig flag ?

Regards,
Vikash
>  
>  struct iris_inst {
> @@ -72,6 +73,7 @@ struct iris_inst {
>  	u32				sequence_out;
>  	struct iris_ts_metadata		tss[VIDEO_MAX_FRAME];
>  	u32				metadata_idx;
> +	bool				in_reconfig;
>  };
>  
>  #endif
> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index 71751365b000..46abdc47420f 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
> @@ -487,6 +487,7 @@ static int iris_vdec_process_streamon_output(struct iris_inst *inst)
>  	if (ret)
>  		return ret;
>  
> +	inst->in_reconfig = false;
>  	return iris_inst_change_sub_state(inst, clear_sub_state, 0);
>  }
>  
>