From: Yifei Gao <gyf161023@gmail.com>
To: Bin Du <bin.du@amd.com>,
Nirujogi Pratap <pratap.nirujogi@amd.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Sultan Alsawaf <sultan@kerneltoast.com>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
Yifei Gao <gyf161023@gmail.com>
Subject: [PATCH v3 2/2] media: amd: isp4: fix self-deadlock in isp4sd_pwron_and_init() error path
Date: Tue, 28 Jul 2026 14:16:50 +0000 [thread overview]
Message-ID: <20260728141659.62310-3-gyf161023@gmail.com> (raw)
In-Reply-To: <20260728141659.62310-1-gyf161023@gmail.com>
isp4sd_pwron_and_init() holds ops_mutex through guard(mutex) for the
whole function. On any initialization failure it jumps to the err_deinit
label and calls isp4sd_pwroff_and_deinit(), which takes the same
ops_mutex through its own guard(mutex). Because the guard in
isp4sd_pwron_and_init() still holds the lock at err_deinit, the call
re-acquires a non-recursive mutex held by the same thread and deadlocks
on any initialization failure.
The error path cannot simply drop the lock and keep calling the full
teardown, because that teardown is not valid at the earlier failure
points. In particular, pm_runtime_resume_and_get() drops the usage
counter again on failure (via pm_runtime_put_noidle()), so no PM
reference is held when it returns an error; calling pm_runtime_put_sync()
unconditionally would underflow the usage count, and the teardown would
also touch ISP MMIO while the device is not powered. This was previously
masked by the deadlock, since the thread never reached the teardown body.
Replace the single teardown call with staged labels that unwind only the
resources actually acquired at each failure point. The error path no
longer calls the locked isp4sd_pwroff_and_deinit(), which removes the
deadlock, and each failure now skips the steps it never reached, keeping
the runtime-PM count balanced and not accessing MMIO while unpowered.
isp4if_start() and isp4sd_start_resp_proc_threads() already clean up
after themselves on failure, so their resources are not unwound again.
Fixes: 4e5e7a7ddb4a ("media: platform: amd: isp4 subdev and firmware loading handling added")
Assisted-by: Claude:claude-opus-4-8 smatch
Signed-off-by: Yifei Gao <gyf161023@gmail.com>
---
drivers/media/platform/amd/isp4/isp4_subdev.c | 28 +++++++++++++++----
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.c b/drivers/media/platform/amd/isp4/isp4_subdev.c
index 48deea79ce6c..868d1c74d35e 100644
--- a/drivers/media/platform/amd/isp4/isp4_subdev.c
+++ b/drivers/media/platform/amd/isp4/isp4_subdev.c
@@ -687,7 +687,7 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
if (ret) {
dev_err(dev, "fail to power on isp_subdev ret %d\n",
ret);
- goto err_deinit;
+ goto err_module_disable;
}
/* ISPPG ISP Power Status */
@@ -697,7 +697,7 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
dev_err(dev,
"fail to set performance state %u, ret %d\n",
perf_state, ret);
- goto err_deinit;
+ goto err_power_off;
}
ispif->status = ISP4IF_STATUS_PWR_ON;
@@ -709,12 +709,12 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
ret = isp4if_start(ispif);
if (ret) {
dev_err(dev, "fail to start isp_subdev interface\n");
- goto err_deinit;
+ goto err_perf_restore;
}
if (isp4sd_start_resp_proc_threads(isp_subdev)) {
dev_err(dev, "isp_start_resp_proc_threads fail\n");
- goto err_deinit;
+ goto err_stop_interface;
}
dev_dbg(dev, "create resp threads ok\n");
@@ -724,8 +724,24 @@ int isp4sd_pwron_and_init(struct v4l2_subdev *sd)
isp_subdev->irq_enabled = true;
return 0;
-err_deinit:
- isp4sd_pwroff_and_deinit(sd);
+
+err_stop_interface:
+ isp4if_stop(ispif);
+err_perf_restore:
+ ret = dev_pm_genpd_set_performance_state(dev, ISP4SD_PERFORMANCE_STATE_LOW);
+ if (ret)
+ dev_err(dev, "fail to set performance state %u, ret %d\n",
+ ISP4SD_PERFORMANCE_STATE_LOW, ret);
+err_power_off:
+ isp4hw_wreg(isp_subdev->mmio, ISP_SOFT_RESET, 0);
+ isp4hw_wreg(isp_subdev->mmio, ISP_POWER_STATUS, 0);
+ ret = pm_runtime_put_sync(dev);
+ if (ret)
+ dev_err(dev, "power off isp_subdev fail %d\n", ret);
+ ispif->status = ISP4IF_STATUS_PWR_OFF;
+err_module_disable:
+ isp4sd_module_enable(isp_subdev, false);
+ msleep(20);
return -EINVAL;
}
--
2.43.0
next prev parent reply other threads:[~2026-07-28 14:17 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-25 20:36 [PATCH] media: amd: isp4: fix self-deadlock in power-on " Yifei Gao
2026-07-27 3:32 ` Bin Du
2026-07-27 18:34 ` [PATCH v2] media: amd: isp4: fix self-deadlock in isp4sd_pwron_and_init() " Yifei Gao
2026-07-28 10:17 ` Bin Du
2026-07-28 14:16 ` [PATCH v3 0/2] media: amd: isp4: fix error handling in isp4sd_pwron_and_init() Yifei Gao
2026-07-28 14:16 ` [PATCH v3 1/2] media: amd: isp4: release partial allocations in isp4if_alloc_fw_gpumem() Yifei Gao
2026-07-28 14:16 ` Yifei Gao [this message]
2026-07-28 17:35 ` [PATCH v3 2/2] media: amd: isp4: fix self-deadlock in isp4sd_pwron_and_init() error path Sakari Ailus
2026-07-28 19:07 ` [PATCH v4 0/2] media: amd: isp4: fix error handling in isp4sd_pwron_and_init() Yifei Gao
2026-07-28 19:07 ` [PATCH v4 1/2] media: amd: isp4: release partial allocations in isp4if_alloc_fw_gpumem() Yifei Gao
2026-07-30 9:57 ` Bin Du
2026-07-28 19:07 ` [PATCH v4 2/2] media: amd: isp4: fix self-deadlock in isp4sd_pwron_and_init() error path Yifei Gao
2026-07-30 14:14 ` [PATCH v5 0/2] media: amd: isp4: fix error handling in isp4sd_pwron_and_init() Yifei Gao
2026-07-30 14:14 ` [PATCH v5 1/2] media: amd: isp4: release partial allocations in isp4if_alloc_fw_gpumem() Yifei Gao
2026-08-03 7:22 ` Bin Du
2026-07-30 14:14 ` [PATCH v5 2/2] media: amd: isp4: fix self-deadlock in isp4sd_pwron_and_init() error path Yifei Gao
2026-08-03 7:22 ` Bin Du
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260728141659.62310-3-gyf161023@gmail.com \
--to=gyf161023@gmail.com \
--cc=bin.du@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=pratap.nirujogi@amd.com \
--cc=sakari.ailus@linux.intel.com \
--cc=sultan@kerneltoast.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®