Re: [PATCH v2] drm/atomic-helper: tear down HPD/polling in drm_atomic_helper_shutdown()

[sashiko-bot@kernel.org]

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