Re: [PATCH] media: ipu6: Don't stop or close a stream that failed to start

[Sakari Ailus]

Hi Jake,

Please be careful with git send-email arguments in the future.

On Fri, Nov 06, 2026 at 07:19:05AM -0500, Jake Steinman wrote:
> When ipu6_isys_start_stream_firmware() fails, the firmware stream is
> either not open or has already been closed by its own error path. Since
> commit eb68e9f1c6eb ("media: ipu6: Bridge the gap between streams in
> V4L2 and IPU6 firmware"), the err_requeue_buffers label in
> ipu6_isys_csi2_enable_streams() falls through to
> err_stop_stream_firmware, so the driver then sends STREAM_STOP and
> STREAM_CLOSE for that stream as well. Each of those waits for a
> response that never comes.
> 
> On Panther Lake (IPU7.5) I have seen this after the firmware rejected a
> stream start: stop and close both timed out, a later stream open timed
> out too, and the ISYS needed a power cycle before anything could stream
> again.
> 

I wonder if any firmware communication failure, a timeout or other error
should trigger the IPU reset (once the users are gone). The functions
communicating with the firmware should also probably check that condition.
I just sent a PR on IPU reset changes, they probably don't address this
problem though.

> Requeue the buffers and go straight to freeing the stream on this path.
> The CSI-2 receiver error path still stops and closes the stream, which
> is open there.
> 
> Fixes: eb68e9f1c6eb ("media: ipu6: Bridge the gap between streams in V4L2 and IPU6 firmware")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Jake Steinman <j@xxxxxxxxxxxxxxxx>
> ---
>  drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> index 1525454..ed4e4f6 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> @@ -678,8 +678,11 @@ static int ipu6_isys_csi2_enable_streams(struct v4l2_subdev *sd,
>  	}
>  
>  	ret = ipu6_isys_start_stream_firmware(stream, &bl, &desc);
> -	if (ret)
> -		goto err_requeue_buffers;
> +	if (ret) {
> +		/* The firmware has no open stream to stop or close here. */
> +		ipu6_isys_buffer_list_queue(&bl, IPU6_ISYS_BUFFER_LIST_FL_INCOMING, 0);
> +		goto err_free_stream_firmware;
> +	}
>  
>  	if (!csi2->streaming_vc) {
>  		ret = (IS_IPU7(isp) || IS_IPU8(isp)) ?
> @@ -704,7 +707,6 @@ err_stop_stream_csi2:
>  	else
>  		ipu6_isys_csi2_stream_disable(csi2);
>  
> -err_requeue_buffers:
>  	ipu6_isys_buffer_list_queue(&bl, IPU6_ISYS_BUFFER_LIST_FL_INCOMING, 0);
>  
>  err_stop_stream_firmware:

-- 
Regards,

Sakari Ailus