Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Skipping `intel_hpd_init()` on runtime resume for PME-capable devices leaves hardware HPD interrupts permanently disabled, breaking hotplug detection while the device is awake. --
--- Patch [5]: [RFC PATCH 5/5] drm/i915/display: skip HPD polling if PME is supported --- commit f6a80f590c41d181052f2d40a190e9c030e50eb2 Author: Vinod Govindapillai <[email protected]> drm/i915/display: skip HPD polling if PME is supported This commit prevents the display driver from enabling HPD polling during runtime suspend if the device supports PM_PME. It also updates the resume path to selectively initialize HPD only if polling was actually enabled. > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c > b/drivers/gpu/drm/i915/display/intel_display_driver.c > index 70d112fd4287a..b7e25b7ea32ed 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c [ ... ] > @@ -951,10 +956,20 @@ void intel_display_driver_pm_runtime_resume(struct > intel_display *display) > * power well, so hpd is reinitialized from there. For > * everyone else do it here. > */ > - if (!display->platform.valleyview && !display->platform.cherryview) { > + if (display->platform.valleyview || display->platform.cherryview) > + goto out; > + > + /* For PME capable devices we would not have resorted into HPD polling > */ > + if (intel_hpd_polling_enabled(display)) > intel_hpd_init(display); [Severity: High] Does skipping intel_hpd_init() here permanently break hardware hotplug detection for PME-capable devices? When a PME-capable device enters runtime suspend, it skips intel_hpd_poll_enable(), so intel_hpd_polling_enabled() will return false. On resume, this condition causes intel_hpd_init() to be skipped. However, looking at intel_hpd_init() in intel_hotplug.c, it does more than just polling setup. It also programs the hardware registers to re-enable hardware interrupts: void intel_hpd_init(struct intel_display *display) { ... spin_lock_irq(&display->irq.lock); intel_hpd_irq_setup(display); spin_unlock_irq(&display->irq.lock); } Without calling intel_hpd_irq_setup(), the hardware HPD interrupts will remain masked. Will any subsequent monitor hotplugs or unplugs that happen while the device is awake be silently ignored? > - intel_hpd_poll_disable(display); > - } > > + /* > + * Regardless of PME capable path, call the HPD polling disable, the > + * poll_init_work, i915_hpd_poll_detect_connectors reprobe catches the > + * hotplug that occurred while suspended > + */ > + intel_hpd_poll_disable(display); > + > +out: > skl_watermark_ipc_update(display); > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
