Re: [PATCH v6 14/18] media: iris: implement support for the Agatti platform

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The AR50Lt encoder will fail to allocate its required internal buffers because it never queries their sizes from the firmware, resulting in a size of 0 and firmware initialization failure.
--

--- Patch [14]: [PATCH v6 14/18] media: iris: implement support for the Agatti platform ---
commit 4c7933dee146c178bb97d589bc58051909ba6ca9
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

media: iris: implement support for the Agatti platform

Port support for the AR50Lt video codec core (present for example on the
Agatti platform) to the Iris driver. Unlike more recent cores this
generation doesn't have the PIPE property (as it always has only one
pipe). Also, unlike newer platforms, buffer sizes are requested from the
firmware instead of being calculated by the driver.

> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1.c
> index ca1545d28b531..4e2f1804ca66f 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1.c
[ ... ]
> +const struct iris_firmware_data iris_hfi_gen1_ar50lt_data = {
> +	.init_hfi_ops = &iris_hfi_gen1_sys_ops_init,
> +
> +	.inst_fw_caps_dec = iris_inst_fw_cap_gen1_ar50lt_dec,
> +	.inst_fw_caps_dec_size = ARRAY_SIZE(iris_inst_fw_cap_gen1_ar50lt_dec),
> +	.inst_fw_caps_enc = inst_fw_cap_gen1_ar50lt_enc,
> +	.inst_fw_caps_enc_size = ARRAY_SIZE(inst_fw_cap_gen1_ar50lt_enc),
> +
> +	.dec_input_config_params_default =
> +		sm8250_vdec_input_config_param_default,
> +	.dec_input_config_params_default_size =
> +		ARRAY_SIZE(sm8250_vdec_input_config_param_default),
> +	.enc_input_config_params = sm8250_venc_input_config_param,
> +	.enc_input_config_params_size =
> +		ARRAY_SIZE(sm8250_venc_input_config_param),

[Severity: High]
Will the AR50Lt encoder fail to allocate its required internal buffers because
it never queries their sizes from the firmware?

Since the encoder reuses sm8250_venc_input_config_param, it lacks
HFI_PROPERTY_PARAM_BUFFER_SIZE_ACTUAL. Without this property being requested,
and because venc_prop_type_handle_inp_arr in iris_hfi_gen1_command.c lacks a
handler for it, iris_hfi_gen1_set_bufsize() is never called for the encoder.

As a result, the newly added firmware query for
HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS won't execute, leaving
inst->buffers[buffer_type].size as 0. This means iris_vpu_ar50lt_gen1_buf_size()
will also return 0.

Does the encoder need its own input config array and an update to the property
handler array so that it can correctly fetch buffer sizes?

> +
> +	.dec_ip_int_buf_tbl = iris_hfi_gen1_ar50lt_dec_ip_int_buf_tbl,
> +	.dec_ip_int_buf_tbl_size = ARRAY_SIZE(iris_hfi_gen1_ar50lt_dec_ip_int_buf_tbl),
> +	.dec_op_int_buf_tbl = sm8250_dec_op_int_buf_tbl,
> +	.dec_op_int_buf_tbl_size = ARRAY_SIZE(sm8250_dec_op_int_buf_tbl),
> +
> +	.enc_ip_int_buf_tbl = sm8250_enc_ip_int_buf_tbl,
> +	.enc_ip_int_buf_tbl_size = ARRAY_SIZE(sm8250_enc_ip_int_buf_tbl),
> +};

> 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 99e82e5510abe..5c267e9aef6ea 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -970,6 +970,11 @@ static int iris_hfi_gen1_set_bufsize(struct iris_inst *inst, u32 plane)
>  	struct hfi_buffer_size_actual bufsz;
>  	int ret;
>  
> +	ret = inst->hfi_session_ops->session_get_property(inst,
> +							  HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS);
> +	if (ret)
> +		return ret;
> +
>  	if (iris_split_mode_enabled(inst)) {
>  		bufsz.type = HFI_BUFFER_OUTPUT;
>  		bufsz.size = inst->core->iris_firmware_desc->get_vpu_buffer_size(inst, BUF_DPB);

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> index fb6f1016415e2..511502d4ea6c3 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
[ ... ]
> @@ -2194,6 +2194,11 @@ u32 iris_vpu4x_buf_size(struct iris_inst *inst, enum iris_buffer_type buffer_typ
>  	return size;
>  }
>  
> +u32 iris_vpu_ar50lt_gen1_buf_size(struct iris_inst *inst, enum iris_buffer_type buffer_type)
> +{
> +	return inst->buffers[buffer_type].size;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-iris-ar50lt-v6-0-374f0a46c23b@oss.qualcomm.com?part=14