* Re: [PATCH] media: amd: isp4: fix self-deadlock in isp4sd_start_stream() error path
2026-10-28 14:05 [PATCH] media: amd: isp4: fix self-deadlock in isp4sd_start_stream() error path Caspar Adriani via B4 Relay
@ 2026-10-28 18:52 ` Sultan Alsawaf
2026-10-30 2:09 ` Bin Du
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Sultan Alsawaf @ 2026-10-28 18:52 UTC (permalink / raw)
To: caspar.adriani
Cc: Bin Du, Nirujogi Pratap, Mauro Carvalho Chehab, Sakari Ailus,
Benjamin Chan, King Li, Svetoslav Stoilov, Yifei Gao,
Seifelden Abdelmonem, linux-media, linux-kernel, stable
On Wed, Oct 28, 2026 at 03:05:41PM +0100, Caspar Adriani via B4 Relay wrote:
> From: Caspar Adriani <caspar.adriani@proton.me>
>
> isp4sd_start_stream() takes ops_mutex with guard() and, when
> isp4sd_init_stream() or isp4sd_setup_output() fails, jumps to
> err_stop_stream and calls isp4sd_stop_stream(). That function takes
> ops_mutex again, so the task blocks on a mutex it already holds. Every
> later open, close or ioctl on the device then blocks behind it, and
> only a reboot recovers:
>
> INFO: task .v4l2-ctl-wrapp:38440 blocked for more than 122 seconds.
> isp4sd_stop_stream.isra.0+0x43/0x1d0 [amd_isp4_capture]
> isp4sd_enable_streams.cold+0x8b/0x26c [amd_isp4_capture]
> v4l2_subdev_enable_streams+0x1ff/0x360 [videodev]
> isp4vid_qops_start_streaming+0xf2/0x160 [amd_isp4_capture]
> INFO: task .v4l2-ctl-wrapp:38440 is blocked on a mutex likely owned
> by task .v4l2-ctl-wrapp:38440.
>
> One way to reach the error path: start and stop a stream, then start
> it again without setting the format. isp4sd_uninit_stream() clears the
> subdev pad format on stop, so isp4sd_get_str_out_prop() fails on the
> second start with "fail to get out prop" and "fail to setup output".
>
> Move the body of isp4sd_stop_stream() into __isp4sd_stop_stream(),
> which expects ops_mutex to be held, and call it from the error path.
>
> Fixes: 4e5e7a7ddb4a ("media: platform: amd: isp4 subdev and firmware loading handling added")
> Cc: stable@vger.kernel.org
> Signed-off-by: Caspar Adriani <caspar.adriani@proton.me>
> ---
> Tested on an ASUS ROG Flow Z13 (GZ302EA) on v7.3, with a local,
> not-for-upstream change that adds its OMNI13B1 sensor to the ISP4 ACPI
> tables so the capture device is created. The stock firmware does not
> support that sensor, so no frames arrive, but the start and stop paths
> run. Sequence: v4l2-ctl --set-fmt-video=1920x1080 NV12 --stream-mmap,
> stopped after 10 s; the same without --set-fmt-video; then the first
> command again.
>
> Without this patch (v7.2.8, same setup) this failure hung the task as
> shown above. With it, VIDIOC_STREAMON returns -EINVAL after "fail to
> get out prop" / "fail to setup output", v4l2-ctl exits, no task is left
> blocked, and the third run starts and stops the stream normally.
> Not run: lockdep (CONFIG_PROVE_LOCKING is off in this kernel).
> ---
> drivers/media/platform/amd/isp4/isp4_subdev.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.c b/drivers/media/platform/amd/isp4/isp4_subdev.c
> index a108d90ace..08512f060a 100644
> --- a/drivers/media/platform/amd/isp4/isp4_subdev.c
> +++ b/drivers/media/platform/amd/isp4/isp4_subdev.c
> @@ -745,15 +745,15 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
> return -EINVAL;
> }
>
> -static int isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> - struct v4l2_subdev_state *state, u32 pad)
> +static void __isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> + struct v4l2_subdev_state *state, u32 pad)
> {
> struct isp4sd_sensor_info *sensor_info = &isp_subdev->sensor_info;
> struct isp4sd_output_info *output_info = &sensor_info->output_info;
> struct isp4_interface *ispif = &isp_subdev->ispif;
> struct device *dev = isp_subdev->dev;
>
> - guard(mutex)(&isp_subdev->ops_mutex);
> + lockdep_assert_held(&isp_subdev->ops_mutex);
> dev_dbg(dev, "status %i\n", output_info->start_status);
>
> if (output_info->start_status == ISP4SD_START_STATUS_STARTED) {
> @@ -786,6 +786,13 @@ static int isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> }
>
> isp4sd_uninit_stream(isp_subdev, state, pad);
> +}
> +
> +static int isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> + struct v4l2_subdev_state *state, u32 pad)
> +{
> + guard(mutex)(&isp_subdev->ops_mutex);
> + __isp4sd_stop_stream(isp_subdev, state, pad);
>
> /*
> * Return success to ensure the stop process proceeds,
> @@ -836,7 +843,7 @@ static int isp4sd_start_stream(struct isp4_subdev *isp_subdev,
> return 0;
>
> err_stop_stream:
> - isp4sd_stop_stream(isp_subdev, state, pad);
> + __isp4sd_stop_stream(isp_subdev, state, pad);
> return ret;
> }
>
>
> ---
> base-commit: 65ba8f34ec886ec72b2c07ddc8cf3d815c32869d
> change-id: 20261028-isp4-start-stream-deadlock-6c33a97d3297
>
> Best regards,
> --
> Caspar Adriani <caspar.adriani@proton.me>
Reviewed-by: Sultan Alsawaf <sultan@kerneltoast.com>
Funny coindicidence, I ran into this independently 2 days ago on v7.3 when
Chromium restarted a stalled stream, and I made a similar fix locally. The
trigger you mentioned is a another bug: the subdev format shouldn't be cleared
when streaming stops, since the video node keeps its copy and userspace can
restart streaming without VIDIOC_S_FMT. I have a fix for that, plus one to power
off the ISP when starting the stream fails, which I'll send on top of this.
Sultan
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: amd: isp4: fix self-deadlock in isp4sd_start_stream() error path
2026-10-28 14:05 [PATCH] media: amd: isp4: fix self-deadlock in isp4sd_start_stream() error path Caspar Adriani via B4 Relay
2026-10-28 18:52 ` Sultan Alsawaf
@ 2026-10-30 2:09 ` Bin Du
2026-11-02 19:19 ` Nirujogi, Pratap
2026-11-03 15:27 ` Seifelden Abdelmonem
3 siblings, 0 replies; 5+ messages in thread
From: Bin Du @ 2026-10-30 2:09 UTC (permalink / raw)
To: caspar.adriani, Nirujogi Pratap
Cc: Mauro Carvalho Chehab, Sakari Ailus, Benjamin Chan, King Li,
Sultan Alsawaf, Svetoslav Stoilov, Yifei Gao,
Seifelden Abdelmonem, linux-media, linux-kernel, stable
Reviewed-by: Bin Du <bin.du@amd.com>
On 10/28/2026 10:05 PM, Caspar Adriani via B4 Relay wrote:
>
> From: Caspar Adriani <caspar.adriani@proton.me>
>
> isp4sd_start_stream() takes ops_mutex with guard() and, when
> isp4sd_init_stream() or isp4sd_setup_output() fails, jumps to
> err_stop_stream and calls isp4sd_stop_stream(). That function takes
> ops_mutex again, so the task blocks on a mutex it already holds. Every
> later open, close or ioctl on the device then blocks behind it, and
> only a reboot recovers:
>
> INFO: task .v4l2-ctl-wrapp:38440 blocked for more than 122 seconds.
> isp4sd_stop_stream.isra.0+0x43/0x1d0 [amd_isp4_capture]
> isp4sd_enable_streams.cold+0x8b/0x26c [amd_isp4_capture]
> v4l2_subdev_enable_streams+0x1ff/0x360 [videodev]
> isp4vid_qops_start_streaming+0xf2/0x160 [amd_isp4_capture]
> INFO: task .v4l2-ctl-wrapp:38440 is blocked on a mutex likely owned
> by task .v4l2-ctl-wrapp:38440.
>
> One way to reach the error path: start and stop a stream, then start
> it again without setting the format. isp4sd_uninit_stream() clears the
> subdev pad format on stop, so isp4sd_get_str_out_prop() fails on the
> second start with "fail to get out prop" and "fail to setup output".
>
> Move the body of isp4sd_stop_stream() into __isp4sd_stop_stream(),
> which expects ops_mutex to be held, and call it from the error path.
>
> Fixes: 4e5e7a7ddb4a ("media: platform: amd: isp4 subdev and firmware loading handling added")
> Cc: stable@vger.kernel.org
> Signed-off-by: Caspar Adriani <caspar.adriani@proton.me>
> ---
> Tested on an ASUS ROG Flow Z13 (GZ302EA) on v7.3, with a local,
> not-for-upstream change that adds its OMNI13B1 sensor to the ISP4 ACPI
> tables so the capture device is created. The stock firmware does not
> support that sensor, so no frames arrive, but the start and stop paths
> run. Sequence: v4l2-ctl --set-fmt-video=1920x1080 NV12 --stream-mmap,
> stopped after 10 s; the same without --set-fmt-video; then the first
> command again.
>
> Without this patch (v7.2.8, same setup) this failure hung the task as
> shown above. With it, VIDIOC_STREAMON returns -EINVAL after "fail to
> get out prop" / "fail to setup output", v4l2-ctl exits, no task is left
> blocked, and the third run starts and stops the stream normally.
> Not run: lockdep (CONFIG_PROVE_LOCKING is off in this kernel).
> ---
> drivers/media/platform/amd/isp4/isp4_subdev.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.c b/drivers/media/platform/amd/isp4/isp4_subdev.c
> index a108d90ace..08512f060a 100644
> --- a/drivers/media/platform/amd/isp4/isp4_subdev.c
> +++ b/drivers/media/platform/amd/isp4/isp4_subdev.c
> @@ -745,15 +745,15 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
> return -EINVAL;
> }
>
> -static int isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> - struct v4l2_subdev_state *state, u32 pad)
> +static void __isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> + struct v4l2_subdev_state *state, u32 pad)
> {
> struct isp4sd_sensor_info *sensor_info = &isp_subdev->sensor_info;
> struct isp4sd_output_info *output_info = &sensor_info->output_info;
> struct isp4_interface *ispif = &isp_subdev->ispif;
> struct device *dev = isp_subdev->dev;
>
> - guard(mutex)(&isp_subdev->ops_mutex);
> + lockdep_assert_held(&isp_subdev->ops_mutex);
> dev_dbg(dev, "status %i\n", output_info->start_status);
>
> if (output_info->start_status == ISP4SD_START_STATUS_STARTED) {
> @@ -786,6 +786,13 @@ static int isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> }
>
> isp4sd_uninit_stream(isp_subdev, state, pad);
> +}
> +
> +static int isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> + struct v4l2_subdev_state *state, u32 pad)
> +{
> + guard(mutex)(&isp_subdev->ops_mutex);
> + __isp4sd_stop_stream(isp_subdev, state, pad);
>
> /*
> * Return success to ensure the stop process proceeds,
> @@ -836,7 +843,7 @@ static int isp4sd_start_stream(struct isp4_subdev *isp_subdev,
> return 0;
>
> err_stop_stream:
> - isp4sd_stop_stream(isp_subdev, state, pad);
> + __isp4sd_stop_stream(isp_subdev, state, pad);
> return ret;
> }
>
>
> ---
> base-commit: 65ba8f34ec886ec72b2c07ddc8cf3d815c32869d
> change-id: 20261028-isp4-start-stream-deadlock-6c33a97d3297
>
> Best regards,
> --
> Caspar Adriani <caspar.adriani@proton.me>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: amd: isp4: fix self-deadlock in isp4sd_start_stream() error path
2026-10-28 14:05 [PATCH] media: amd: isp4: fix self-deadlock in isp4sd_start_stream() error path Caspar Adriani via B4 Relay
2026-10-28 18:52 ` Sultan Alsawaf
2026-10-30 2:09 ` Bin Du
@ 2026-11-02 19:19 ` Nirujogi, Pratap
2026-11-03 15:27 ` Seifelden Abdelmonem
3 siblings, 0 replies; 5+ messages in thread
From: Nirujogi, Pratap @ 2026-11-02 19:19 UTC (permalink / raw)
To: caspar.adriani, Bin Du, Nirujogi Pratap
Cc: Mauro Carvalho Chehab, Sakari Ailus, Benjamin Chan, King Li,
Sultan Alsawaf, Svetoslav Stoilov, Yifei Gao,
Seifelden Abdelmonem, linux-media, linux-kernel, stable
On 10/28/2026 10:05 AM, Caspar Adriani via B4 Relay wrote:
> [Some people who received this message don't often get email from devnull+caspar.adriani.proton.me@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> From: Caspar Adriani <caspar.adriani@proton.me>
>
> isp4sd_start_stream() takes ops_mutex with guard() and, when
> isp4sd_init_stream() or isp4sd_setup_output() fails, jumps to
> err_stop_stream and calls isp4sd_stop_stream(). That function takes
> ops_mutex again, so the task blocks on a mutex it already holds. Every
> later open, close or ioctl on the device then blocks behind it, and
> only a reboot recovers:
>
> INFO: task .v4l2-ctl-wrapp:38440 blocked for more than 122 seconds.
> isp4sd_stop_stream.isra.0+0x43/0x1d0 [amd_isp4_capture]
> isp4sd_enable_streams.cold+0x8b/0x26c [amd_isp4_capture]
> v4l2_subdev_enable_streams+0x1ff/0x360 [videodev]
> isp4vid_qops_start_streaming+0xf2/0x160 [amd_isp4_capture]
> INFO: task .v4l2-ctl-wrapp:38440 is blocked on a mutex likely owned
> by task .v4l2-ctl-wrapp:38440.
>
> One way to reach the error path: start and stop a stream, then start
> it again without setting the format. isp4sd_uninit_stream() clears the
> subdev pad format on stop, so isp4sd_get_str_out_prop() fails on the
> second start with "fail to get out prop" and "fail to setup output".
>
> Move the body of isp4sd_stop_stream() into __isp4sd_stop_stream(),
> which expects ops_mutex to be held, and call it from the error path.
>
> Fixes: 4e5e7a7ddb4a ("media: platform: amd: isp4 subdev and firmware loading handling added")
> Cc: stable@vger.kernel.org
> Signed-off-by: Caspar Adriani <caspar.adriani@proton.me>
> ---
> Tested on an ASUS ROG Flow Z13 (GZ302EA) on v7.3, with a local,
> not-for-upstream change that adds its OMNI13B1 sensor to the ISP4 ACPI
> tables so the capture device is created. The stock firmware does not
> support that sensor, so no frames arrive, but the start and stop paths
> run. Sequence: v4l2-ctl --set-fmt-video=1920x1080 NV12 --stream-mmap,
> stopped after 10 s; the same without --set-fmt-video; then the first
> command again.
>
> Without this patch (v7.2.8, same setup) this failure hung the task as
> shown above. With it, VIDIOC_STREAMON returns -EINVAL after "fail to
> get out prop" / "fail to setup output", v4l2-ctl exits, no task is left
> blocked, and the third run starts and stops the stream normally.
> Not run: lockdep (CONFIG_PROVE_LOCKING is off in this kernel).
> ---
> drivers/media/platform/amd/isp4/isp4_subdev.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.c b/drivers/media/platform/amd/isp4/isp4_subdev.c
> index a108d90ace..08512f060a 100644
> --- a/drivers/media/platform/amd/isp4/isp4_subdev.c
> +++ b/drivers/media/platform/amd/isp4/isp4_subdev.c
> @@ -745,15 +745,15 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
> return -EINVAL;
> }
>
> -static int isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> - struct v4l2_subdev_state *state, u32 pad)
> +static void __isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> + struct v4l2_subdev_state *state, u32 pad)
> {
> struct isp4sd_sensor_info *sensor_info = &isp_subdev->sensor_info;
> struct isp4sd_output_info *output_info = &sensor_info->output_info;
> struct isp4_interface *ispif = &isp_subdev->ispif;
> struct device *dev = isp_subdev->dev;
>
> - guard(mutex)(&isp_subdev->ops_mutex);
> + lockdep_assert_held(&isp_subdev->ops_mutex);
> dev_dbg(dev, "status %i\n", output_info->start_status);
>
> if (output_info->start_status == ISP4SD_START_STATUS_STARTED) {
> @@ -786,6 +786,13 @@ static int isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> }
>
> isp4sd_uninit_stream(isp_subdev, state, pad);
> +}
> +
> +static int isp4sd_stop_stream(struct isp4_subdev *isp_subdev,
> + struct v4l2_subdev_state *state, u32 pad)
> +{
> + guard(mutex)(&isp_subdev->ops_mutex);
> + __isp4sd_stop_stream(isp_subdev, state, pad);
>
> /*
> * Return success to ensure the stop process proceeds,
> @@ -836,7 +843,7 @@ static int isp4sd_start_stream(struct isp4_subdev *isp_subdev,
> return 0;
>
> err_stop_stream:
> - isp4sd_stop_stream(isp_subdev, state, pad);
> + __isp4sd_stop_stream(isp_subdev, state, pad);
> return ret;
> }
>
>
> ---
> base-commit: 65ba8f34ec886ec72b2c07ddc8cf3d815c32869d
> change-id: 20261028-isp4-start-stream-deadlock-6c33a97d3297
>
> Best regards,
> --
> Caspar Adriani <caspar.adriani@proton.me>
>
>
Reviewed-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
Thanks,
Pratap
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: amd: isp4: fix self-deadlock in isp4sd_start_stream() error path
2026-10-28 14:05 [PATCH] media: amd: isp4: fix self-deadlock in isp4sd_start_stream() error path Caspar Adriani via B4 Relay
` (2 preceding siblings ...)
2026-11-02 19:19 ` Nirujogi, Pratap
@ 2026-11-03 15:27 ` Seifelden Abdelmonem
3 siblings, 0 replies; 5+ messages in thread
From: Seifelden Abdelmonem @ 2026-11-03 15:27 UTC (permalink / raw)
To: Caspar Adriani
Cc: Bin Du, Nirujogi Pratap, Mauro Carvalho Chehab, Sakari Ailus,
Benjamin Chan, King Li, Yifei Gao, Sultan Alsawaf, linux-media,
linux-kernel, stable, Seifelden Abdelmonem
Tested on an ASUS ROG Flow Z13 (GZ302EA) with v7.2.10 and an
out-of-tree setup in which the OV13B10 streams (NV12 1920x1080), with
this patch applied on top of my two isp4 fixes [1]:
- stream with the format set: 90 frames;
- stop, then stream again without setting the format: VIDIOC_STREAMON
returns -EINVAL after "fail to get out prop" and "fail to setup
output", v4l2-ctl exits and no task is left blocked;
- stream with the format set again: 90 frames.
No hung task, oops or WARN in the kernel log. I did not run the
sequence without the patch on this setup.
Tested-by: Seifelden Abdelmonem <saif.elden.hatim@gmail.com>
[1] https://lore.kernel.org/all/20261026182442.229869-1-saif.elden.hatim@gmail.com/
Regards,
Seifelden
^ permalink raw reply [flat|nested] 5+ messages in thread