[PATCH] drm/msm/dp: add .shutdown to mask HPD IRQ before system shutdown

[PATCH] drm/msm/dp: add .shutdown to mask HPD IRQ before system shutdown

[Mahadevan P]

On reboot, a late DP hot-plug-detect (HPD) IRQ can fire after
apps_smmu has already disabled translation for the display
subsystem, causing the HPD thread to kick off a new modeset that
drives DPU/DP hardware and DMA through a stale IOMMU mapping.

DPU's own .shutdown disables all CRTCs first, but a pending HPD IRQ
thread wakes up afterwards, reads the DPCD, and fires an unsolicited
hotplug event that triggers a second atomic commit turning the
display back on -- right as the IOMMU is disabling translation:

  systemd-shutdown[1]: Rebooting.
  msm_dpu: drm_atomic_commit: committing (shutdown disabling CRTCs)
  arm-smmu 3da0000.iommu: disabling translation
  msm_dpu: drm_dp_read_dpcd_caps (late HPD IRQ thread wakes up)
  msm_dpu: drm_sysfs_connector_hotplug_event: DP-1 hotplug event
  msm_dpu: drm_client_modeset_probe: DP-1 found preferred mode
  msm_dpu: drm_atomic_commit: committing (unsolicited, re-enables display)
  dpu_crtc_commit_kickoff: crtc94 first commit
  arm-smmu 15200000.iommu: disabling translation

Mask and flush the IRQ in .shutdown so no HPD event can retrigger a
modeset once shutdown has started.
Reported on lemans-evk and monaco-evk during reboot stress testing.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Mahadevan P <mahadevan.p@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index dc6f33809ca5..b4fbdeebf952 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1207,6 +1207,14 @@ static void msm_dp_display_remove(struct platform_device *pdev)
 	platform_set_drvdata(pdev, NULL);
 }
 
+static void msm_dp_display_shutdown(struct platform_device *pdev)
+{
+	struct msm_dp_display_private *dp = dev_get_dp_display_private(&pdev->dev);
+
+	disable_irq(dp->irq);
+	synchronize_irq(dp->irq);
+}
+
 static int msm_dp_pm_runtime_suspend(struct device *dev)
 {
 	struct msm_dp_display_private *dp = dev_get_dp_display_private(dev);
@@ -1253,6 +1261,7 @@ static const struct dev_pm_ops msm_dp_pm_ops = {
 static struct platform_driver msm_dp_display_driver = {
 	.probe  = msm_dp_display_probe,
 	.remove = msm_dp_display_remove,
+	.shutdown = msm_dp_display_shutdown,
 	.driver = {
 		.name = "msm-dp-display",
 		.of_match_table = msm_dp_dt_match,

---
base-commit: 1a1757b76427f6201bfe0bf1bea9f7574f332a93
change-id: 20260717-dpshutdown-047912fcf60b

Best regards,
-- 
Mahadevan P <mahadevan.p@oss.qualcomm.com>

Re: [PATCH] drm/msm/dp: add .shutdown to mask HPD IRQ before system shutdown

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] Redundant synchronize_irq() call.

Pre-existing issues:
- [High] Incomplete fix: Race window in system shutdown still allows IOMMU faults.
- [High] Use-after-free of dp->aux in IRQ handler during driver unbind.
--

--- Patch [1]: [PATCH] drm/msm/dp: add .shutdown to mask HPD IRQ before system shutdown ---
commit d07c91cbfabdf117c1daa68222fb3889fa6527d8
Author: Mahadevan P <mahadevan.p@oss.qualcomm.com>

drm/msm/dp: add .shutdown to mask HPD IRQ before system shutdown

This commit adds a .shutdown callback to mask and flush the HPD IRQ before
system shutdown. This is intended to prevent late hotplug events from
triggering a new modeset that drives hardware through a stale IOMMU mapping
during reboot.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index dc6f33809ca5f..b4fbdeebf952c 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1207,6 +1207,14 @@ static void msm_dp_display_remove(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, NULL);
>  }

[Severity: High]
This is a pre-existing issue, but can this lead to a use-after-free during
driver unbind?

Looking at msm_dp_display_remove(), msm_dp_display_deinit_sub_modules() is
called right before platform_set_drvdata():

drivers/gpu/drm/msm/dp/dp_display.c:msm_dp_display_remove() {
    ...
    msm_dp_display_deinit_sub_modules(dp);
    platform_set_drvdata(pdev, NULL);
}

This manually frees dp->aux. However, since the IRQ is managed by
devm_request_threaded_irq, it remains registered and enabled until the
devres release phase occurs after the remove callback returns.

If a hotplug interrupt fires during this window, wouldn't the handler
dereference the freed pointer?

msm_dp_display_irq_handler()
    ...
    hpd_isr_status = msm_dp_aux_get_hpd_intr_status(dp->aux);

>  
> +static void msm_dp_display_shutdown(struct platform_device *pdev)
> +{
> +	struct msm_dp_display_private *dp = dev_get_dp_display_private(&pdev->dev);
> +
> +	disable_irq(dp->irq);

[Severity: High]
This is a pre-existing issue, but does this fully resolve the race condition
during system shutdown?

If a hotplug interrupt fires after the DRM master .shutdown callback has
disabled CRTCs but before msm_dp_display_shutdown(), disable_irq() will wait
for the scheduled msm_dp_display_irq_thread to finish. 

If that thread executes drm_client_dev_hotplug() and performs an atomic
commit to turn the display back on, disable_irq() waits for it to complete.

Doesn't this guarantee the display is active when .shutdown returns,
allowing the IOMMU shutdown to trigger the same fatal fault?

> +	synchronize_irq(dp->irq);

[Severity: Low]
Is this call to synchronize_irq() redundant?

Since disable_irq() is not called from an atomic context here, it
inherently calls synchronize_irq() internally to wait for any currently
executing handlers to complete.

> +}
> +
>  static int msm_dp_pm_runtime_suspend(struct device *dev)

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-dpshutdown-v1-1-b062c2f7dfb1@oss.qualcomm.com?part=1