Re: [PATCH v4 12/16] 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] Calling a synchronous HFI command from the ISR response thread causes a deadlock.
--

commit 960bbe7059755e03232b477bd06562c01b126fdd
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_vpu_buffer.c b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> index fb6f1016415e2..4a39b8fef52b0 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> @@ -2194,6 +2194,19 @@ 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)
> +{
> +	const struct iris_hfi_session_ops *hfi_ops = inst->hfi_session_ops;
> +	int ret;
> +
> +	/* return 0 on error to let the driver cope */
> +	ret = hfi_ops->session_get_property(inst, HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS);

[Severity: High]
Could this lead to a regression by waiting for a response in the response
handler thread?

When iris_vpu_ar50lt_gen1_buf_size() calls hfi_ops->session_get_property(),
it synchronously blocks until the firmware responds:

drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c:iris_hfi_gen1_session_get_property() {
    ...
    return iris_wait_for_session_response(inst, false);
}

However, iris_vpu_ar50lt_gen1_buf_size() (via iris_get_buffer_size()) is
invoked from iris_hfi_gen1_read_changed_params() during event processing:

drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c:iris_hfi_gen1_read_changed_params() {
    ...
    pixmp_op->plane_fmt[0].sizeimage = iris_get_buffer_size(inst, BUF_OUTPUT);
    ...
}

This event processing runs inside the driver's response handler:

drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c:iris_hfi_gen1_response_handler() {
    ...
        iris_hfi_gen1_handle_response(core, core->response_packet);
    ...
}

If the response handler thread blocks itself waiting for a response, how will
that response ever be dequeued and processed? It appears this will stall the
response polling loop until the wait times out.

> +	if (ret)
> +		return 0;
> +
> +	return inst->buffers[buffer_type].size;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-iris-ar50lt-v4-0-0abfb74d5b3c@oss.qualcomm.com?part=12