* [PATCH v2] drm/atomic-helper: tear down HPD/polling in drm_atomic_helper_shutdown()
@ 2026-07-30 12:46 Mahadevan P
2026-07-30 13:01 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Mahadevan P @ 2026-07-30 12:46 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Mahadevan P
drm_atomic_helper_shutdown() disables all CRTCs but leaves output
polling and IRQ-driven hot-plug detection running. 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.
drm_atomic_helper_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
drm_kms_helper_poll_fini() tears down this: it stops the output poll
worker and calls each connector's &drm_connector_helper_funcs.disable_hpd,
which for HPD-capable bridges masks the interrupt in hardware.
Reported on Qualcomm platforms such as 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>
---
Changes in v2:
- Dropped the drm/msm/dp local .shutdown patch (disable_irq()/
synchronize_irq() in dp_display.c)., fixed this at the DRM core
level instead so every driver benefits, not just msm_dp.(Dmitry).
- Link to v1: https://lore.kernel.org/r/20260717-dpshutdown-v1-1-b062c2f7dfb1@oss.qualcomm.com
---
drivers/gpu/drm/drm_atomic_helper.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 285aac3554df..80c67623180a 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -43,6 +43,7 @@
#include <drm/drm_gem_atomic_helper.h>
#include <drm/drm_panic.h>
#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
#include <drm/drm_self_refresh_helper.h>
#include <drm/drm_vblank.h>
#include <drm/drm_writeback.h>
@@ -3676,6 +3677,8 @@ EXPORT_SYMBOL(drm_atomic_helper_reset_crtc);
*
* This is just a convenience wrapper around drm_atomic_helper_disable_all(),
* and it is the atomic version of drm_helper_force_disable_all().
+ *
+ * This also tears down output polling and HPD via drm_kms_helper_poll_fini().
*/
void drm_atomic_helper_shutdown(struct drm_device *dev)
{
@@ -3685,6 +3688,8 @@ void drm_atomic_helper_shutdown(struct drm_device *dev)
if (dev == NULL)
return;
+ drm_kms_helper_poll_fini(dev);
+
DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
ret = drm_atomic_helper_disable_all(dev, &ctx);
---
base-commit: 1a1757b76427f6201bfe0bf1bea9f7574f332a93
change-id: 20260717-dpshutdown-047912fcf60b
Best regards,
--
Mahadevan P <mahadevan.p@oss.qualcomm.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] drm/atomic-helper: tear down HPD/polling in drm_atomic_helper_shutdown()
2026-07-30 12:46 [PATCH v2] drm/atomic-helper: tear down HPD/polling in drm_atomic_helper_shutdown() Mahadevan P
@ 2026-07-30 13:01 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-30 13:01 UTC (permalink / raw)
To: Mahadevan P; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm_kms_helper_poll_fini() fails to synchronize with in-flight driver HPD threads and introduces a TOCTOU race with userspace probing, leaving the original late-modeset bug unfixed.
- [Medium] Calling drm_kms_helper_poll_fini() inside drm_atomic_helper_shutdown() introduces a regression causing a kernel warning stack trace (drm_WARN_ON) during driver unload.
--
--- Patch [1]: [PATCH v2] drm/atomic-helper: tear down HPD/polling in drm_atomic_helper_shutdown() ---
commit e1bd39c97d78edddf053e357bf0f99d72ae55494
Author: Mahadevan P <mahadevan.p@oss.qualcomm.com>
drm/atomic-helper: tear down HPD/polling in drm_atomic_helper_shutdown()
This commit adds a call to drm_kms_helper_poll_fini() within
drm_atomic_helper_shutdown() to tear down output polling and HPD interrupts.
The goal is to prevent late hotplug events from triggering modesets after the
display hardware and IOMMU mappings have been disabled during shutdown.
Link: https://lore.kernel.org/r/20260717-dpshutdown-v1-1-b062c2f7dfb1@oss.qualcomm.com
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 285aac3554dfd..80c67623180a2 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
[ ... ]
> @@ -3676,6 +3677,8 @@ EXPORT_SYMBOL(drm_atomic_helper_reset_crtc);
> *
> * This is just a convenience wrapper around drm_atomic_helper_disable_all(),
> * and it is the atomic version of drm_helper_force_disable_all().
> + *
> + * This also tears down output polling and HPD via drm_kms_helper_poll_fini().
> */
> void drm_atomic_helper_shutdown(struct drm_device *dev)
> {
> @@ -3685,6 +3688,8 @@ void drm_atomic_helper_shutdown(struct drm_device *dev)
> if (dev == NULL)
> return;
>
> + drm_kms_helper_poll_fini(dev);
[Severity: High]
Does calling drm_kms_helper_poll_fini() here fully synchronize with in-flight
driver HPD threads?
While this masks hardware interrupts, it doesn't appear to flush pending IRQ
threads since there is no synchronize_irq() equivalent here. If an HPD
interrupt fires just before shutdown and schedules a driver's threaded IRQ
handler (like msm_dp_display_irq_thread), it seems the thread could wake up,
block on mode_config.mutex, and then trigger an unsolicited modeset after
drm_atomic_helper_shutdown() finishes and releases the locks.
Also, can this introduce a race with userspace probing?
drm_kms_helper_poll_fini() modifies poll_enabled locklessly. A concurrent
GETCONNECTOR ioctl holding mode_config.mutex might read poll_enabled as true
and call drm_kms_helper_poll_enable(), which could turn HPD back on during
the shutdown sequence.
[Severity: Medium]
Could this cause a kernel warning during driver unload?
drm_kms_helper_poll_fini() sets dev->mode_config.poll_enabled to false.
However, many drivers explicitly call drm_kms_helper_poll_disable() during
their own teardown sequences after calling drm_atomic_helper_shutdown()
(for example, in nouveau_display_fini() in the nouveau driver).
Since drm_kms_helper_poll_disable() has a guard check:
if (drm_WARN_ON(dev, !dev->mode_config.poll_enabled))
return;
It looks like drivers performing standard unload routines will hit this
warning stack trace because the core already silently finalized the polling
state.
> +
> DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>
> ret = drm_atomic_helper_disable_all(dev, &ctx);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-dpshutdown-v2-1-441fc5543bed@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-30 13:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 12:46 [PATCH v2] drm/atomic-helper: tear down HPD/polling in drm_atomic_helper_shutdown() Mahadevan P
2026-07-30 13:01 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.