Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] PM_PME support on display hotplug
@ 2026-09-03  8:04 Vinod Govindapillai
  2026-09-03  8:04 ` [PATCH v2 1/5] drm/xe/pm: initialize the device's system wakeup capabilities Vinod Govindapillai
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Vinod Govindapillai @ 2026-09-03  8:04 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: vinod.govindapillai, imre.deak, jouni.hogander

Introduce PME support for PME capable devices and avoid
HPD polling on runtime suspend. For PME capable devices
HPD will trigger PME which will wakeup the system. And
rest of the runtime resume call is same as before.

v2: Instead of plumbing keep_hpd and pme_capability to
    the existing interfaces, a separate helper function to
    set and get pme capability is added.

    HPD polling is handled directly inside the
    intel_hpd_poll_enable() based on the pme capability
    flag

Vinod Govindapillai (5):
  drm/xe/pm: initialize the device's system wakeup capabilities
  drm/i915/hotplug: add helpers to track HPDs can generate PME
  drm/i915/irq: conditional HPD IRQ resets based on PME capability
  drm/i915/hotplug: avoid HPD polling if the device is PME capable
  drm/xe/pm: handle the PME capability and runtime pm routines

 .../gpu/drm/i915/display/intel_display_core.h |  2 ++
 .../gpu/drm/i915/display/intel_display_irq.c  | 20 ++++++++++++-------
 drivers/gpu/drm/i915/display/intel_hotplug.c  | 15 ++++++++++++++
 drivers/gpu/drm/i915/display/intel_hotplug.h  |  3 +++
 drivers/gpu/drm/xe/display/xe_display.c       | 10 ++++++++++
 drivers/gpu/drm/xe/display/xe_display.h       |  4 ++++
 drivers/gpu/drm/xe/xe_pci.c                   | 20 ++++++++++++++++++-
 7 files changed, 66 insertions(+), 8 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/5] drm/xe/pm: initialize the device's system wakeup capabilities
  2026-09-03  8:04 [PATCH v2 0/5] PM_PME support on display hotplug Vinod Govindapillai
@ 2026-09-03  8:04 ` Vinod Govindapillai
  2026-09-03  8:15   ` sashiko-bot
  2026-09-03  8:04 ` [PATCH v2 2/5] drm/i915/hotplug: add helpers to track HPDs can generate PME Vinod Govindapillai
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Vinod Govindapillai @ 2026-09-03  8:04 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: vinod.govindapillai, imre.deak, jouni.hogander

NVL+ XE devices are capable of generating PME from HPDs. Enable
the device as a wakeup device so the display hotplugs can
generate PM PME events.

Bspec: 52979, 52980, 68857, 68867, 68970
Assisted-by: GitHub_Copilot:claude-opus-5
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/xe/xe_pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index ab4da1d9a9f1..f8e16aefd2f8 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -1217,6 +1217,10 @@ static int __xe_pci_probe(struct pci_dev *pdev, const struct xe_device_desc *des
 
 	pci_set_master(pdev);
 
+	err = devm_device_init_wakeup(&pdev->dev);
+	if (err)
+		return err;
+
 	err = xe_probe_info_early(xe, desc, &probed_info);
 	if (err)
 		return err;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/5] drm/i915/hotplug: add helpers to track HPDs can generate PME
  2026-09-03  8:04 [PATCH v2 0/5] PM_PME support on display hotplug Vinod Govindapillai
  2026-09-03  8:04 ` [PATCH v2 1/5] drm/xe/pm: initialize the device's system wakeup capabilities Vinod Govindapillai
@ 2026-09-03  8:04 ` Vinod Govindapillai
  2026-09-03  8:04 ` [PATCH v2 3/5] drm/i915/irq: conditional HPD IRQ resets based on PME capability Vinod Govindapillai
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Vinod Govindapillai @ 2026-09-03  8:04 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: vinod.govindapillai, imre.deak, jouni.hogander

As NVL+ devices can generate PME from HPDs, introduce a flag to
keep track of such a capability and helper functions to access
this flag. This flag will be set to true from runtime pm
functions if device is capable of wakeup from HPDs. This is a
place holder for the flag and it will be used for conditional
IRQ resets and HPD polling on suspend.

Bspec: 52979, 52980, 68857, 68867, 68970
Assisted-by: GitHub_Copilot:claude-opus-5
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_core.h |  2 ++
 drivers/gpu/drm/i915/display/intel_hotplug.c      | 10 ++++++++++
 drivers/gpu/drm/i915/display/intel_hotplug.h      |  3 +++
 3 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
index 7e988b7b1fe7..30b6f2524dee 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -220,6 +220,8 @@ struct intel_hotplug {
 	 * cue to ignore the long HPDs and can be set / unset using debugfs.
 	 */
 	bool ignore_long_hpd;
+
+	bool can_generate_pme;
 };
 
 struct intel_vbt_data {
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 970aa95ee344..b24c9f360e24 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -846,6 +846,16 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
 	}
 }
 
+void intel_hpd_set_pme_capable(struct intel_display *display, bool can_generate_pme)
+{
+	display->hotplug.can_generate_pme = can_generate_pme;
+}
+
+bool intel_hpd_can_generate_pme(struct intel_display *display)
+{
+	return display->hotplug.can_generate_pme;
+}
+
 /**
  * intel_hpd_poll_enable - enable polling for connectors with hpd
  * @display: display device instance
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
index edc41c9d3d65..c57e30f8366b 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.h
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
@@ -35,4 +35,7 @@ void intel_hpd_enable_detection_work(struct intel_display *display);
 void intel_hpd_disable_detection_work(struct intel_display *display);
 bool intel_hpd_schedule_detection(struct intel_display *display);
 
+void intel_hpd_set_pme_capable(struct intel_display *display, bool can_generate_pme);
+bool intel_hpd_can_generate_pme(struct intel_display *display);
+
 #endif /* __INTEL_HOTPLUG_H__ */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/5] drm/i915/irq: conditional HPD IRQ resets based on PME capability
  2026-09-03  8:04 [PATCH v2 0/5] PM_PME support on display hotplug Vinod Govindapillai
  2026-09-03  8:04 ` [PATCH v2 1/5] drm/xe/pm: initialize the device's system wakeup capabilities Vinod Govindapillai
  2026-09-03  8:04 ` [PATCH v2 2/5] drm/i915/hotplug: add helpers to track HPDs can generate PME Vinod Govindapillai
@ 2026-09-03  8:04 ` Vinod Govindapillai
  2026-09-03  8:32   ` sashiko-bot
  2026-09-04  9:56   ` Jani Nikula
  2026-09-03  8:04 ` [PATCH v2 4/5] drm/i915/hotplug: avoid HPD polling if the device is PME capable Vinod Govindapillai
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 14+ messages in thread
From: Vinod Govindapillai @ 2026-09-03  8:04 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: vinod.govindapillai, imre.deak, jouni.hogander

If a device supports generating PME from HPDs, resetting HPD IRQs
will be counter productive as HPDs itself will be lost. During
suspend routines, all the IRQs are reset. So if the device is
capable of generating PME rom HPDs, keep the HPD related IRQs from
reset based on the PME capability of the device on a target power
state. PME capability will be assessed and updated separately.

Bspec: 52979, 52980, 68857, 68867, 68970
Assisted-by: GitHub_Copilot:claude-opus-5
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 .../gpu/drm/i915/display/intel_display_irq.c  | 20 ++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
index a59b75830bd1..fc50b19d5ddf 100644
--- a/drivers/gpu/drm/i915/display/intel_display_irq.c
+++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
@@ -22,6 +22,7 @@
 #include "intel_fdi_regs.h"
 #include "intel_fifo_underrun.h"
 #include "intel_gmbus.h"
+#include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"
 #include "intel_lpe_audio.h"
 #include "intel_parent.h"
@@ -2217,8 +2218,11 @@ static void gen11_display_irq_reset(struct intel_display *display)
 	enum pipe pipe;
 	u32 trans_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
 		BIT(TRANSCODER_C) | BIT(TRANSCODER_D);
+	/* If HPD can generate PME, we don't want to reset HPD related IRQs */
+	bool keep_hpd = intel_hpd_can_generate_pme(display);
 
-	intel_de_write(display, GEN11_DISPLAY_INT_CTL, 0);
+	if (!keep_hpd)
+		intel_de_write(display, GEN11_DISPLAY_INT_CTL, 0);
 
 	if (DISPLAY_VER(display) >= 12) {
 		enum transcoder trans;
@@ -2250,13 +2254,15 @@ static void gen11_display_irq_reset(struct intel_display *display)
 	irq_reset(display, GEN8_DE_PORT_IRQ_REGS);
 	irq_reset(display, GEN8_DE_MISC_IRQ_REGS);
 
-	if (DISPLAY_VER(display) >= 14)
-		irq_reset(display, PICAINTERRUPT_IRQ_REGS);
-	else
-		irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
+	if (!keep_hpd) {
+		if (DISPLAY_VER(display) >= 14)
+			irq_reset(display, PICAINTERRUPT_IRQ_REGS);
+		else
+			irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
 
-	if (INTEL_PCH_TYPE(display) >= PCH_ICP)
-		irq_reset(display, SDE_IRQ_REGS);
+		if (INTEL_PCH_TYPE(display) >= PCH_ICP)
+			irq_reset(display, SDE_IRQ_REGS);
+	}
 }
 
 void gen8_irq_power_well_post_enable(struct intel_display *display,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/5] drm/i915/hotplug: avoid HPD polling if the device is PME capable
  2026-09-03  8:04 [PATCH v2 0/5] PM_PME support on display hotplug Vinod Govindapillai
                   ` (2 preceding siblings ...)
  2026-09-03  8:04 ` [PATCH v2 3/5] drm/i915/irq: conditional HPD IRQ resets based on PME capability Vinod Govindapillai
@ 2026-09-03  8:04 ` Vinod Govindapillai
  2026-09-03  8:04 ` [PATCH v2 5/5] drm/xe/pm: handle the PME capability and runtime pm routines Vinod Govindapillai
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Vinod Govindapillai @ 2026-09-03  8:04 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: vinod.govindapillai, imre.deak, jouni.hogander

In supported devices, HPDs can generate PME and which in turn
can invoke runtime resume calls for xe. No need to keep the
HPD polling in such PME capable devices.

Bspec: 52979, 52980, 68857, 68867, 68970
Assisted-by: GitHub_Copilot:claude-opus-5
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hotplug.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index b24c9f360e24..8b7b9f47f049 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -877,6 +877,11 @@ void intel_hpd_poll_enable(struct intel_display *display)
 	if (!HAS_DISPLAY(display) || !intel_display_device_enabled(display))
 		return;
 
+	if (intel_hpd_can_generate_pme(display)) {
+		drm_dbg_kms(display->drm, "PME wake capable device, skipping HPD polling.\n");
+		return;
+	}
+
 	WRITE_ONCE(display->hotplug.poll_enabled, true);
 
 	/*
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/5] drm/xe/pm: handle the PME capability and runtime pm routines
  2026-09-03  8:04 [PATCH v2 0/5] PM_PME support on display hotplug Vinod Govindapillai
                   ` (3 preceding siblings ...)
  2026-09-03  8:04 ` [PATCH v2 4/5] drm/i915/hotplug: avoid HPD polling if the device is PME capable Vinod Govindapillai
@ 2026-09-03  8:04 ` Vinod Govindapillai
  2026-09-03 12:36   ` [PATCH v3 " Vinod Govindapillai
  2026-09-04 10:03   ` [PATCH v2 " Jani Nikula
  2026-09-03 10:26 ` ✓ i915.CI.BAT: success for pm_pme support on display hotplug (rev3) Patchwork
  2026-09-03 14:15 ` ✗ i915.CI.BAT: failure for pm_pme support on display hotplug (rev4) Patchwork
  6 siblings, 2 replies; 14+ messages in thread
From: Vinod Govindapillai @ 2026-09-03  8:04 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: vinod.govindapillai, imre.deak, jouni.hogander

During the runtime suspend, check if device is capable of wakeup
from PME. If yes update the helper so that IRQ reset and HPD
polling can be handled accordingly. For PME capable devices,
HPD related IRQs are not reset during runtime suspend and
do not start polling for HPDs every 10s. Instead PME can be
generated from HPDs and corresponding runtime resume calls
can be invoked by PME.

Bspec: 52979, 52980, 68857, 68867, 68970
Assisted-by: GitHub_Copilot:claude-opus-5
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/xe/display/xe_display.c | 10 ++++++++++
 drivers/gpu/drm/xe/display/xe_display.h |  4 ++++
 drivers/gpu/drm/xe/xe_pci.c             | 16 +++++++++++++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index 7b25c0814674..ff4a74c5c763 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -236,6 +236,16 @@ void xe_display_irq_postinstall(struct xe_device *xe)
 	intel_display_irq_postinstall(display);
 }
 
+void xe_display_set_pme_capable(struct xe_device *xe, bool pme_from_hpd)
+{
+	struct intel_display *display = xe->display;
+
+	if (!xe->info.probe_display)
+		return;
+
+	intel_hpd_set_pme_capable(display, pme_from_hpd);
+}
+
 static bool suspend_to_idle(void)
 {
 #if IS_ENABLED(CONFIG_ACPI_SLEEP)
diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
index 0babb50bfc77..a77a8fa1d802 100644
--- a/drivers/gpu/drm/xe/display/xe_display.h
+++ b/drivers/gpu/drm/xe/display/xe_display.h
@@ -37,6 +37,8 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir);
 void xe_display_irq_reset(struct xe_device *xe);
 void xe_display_irq_postinstall(struct xe_device *xe);
 
+void xe_display_set_pme_capable(struct xe_device *xe, bool pme_from_hpd);
+
 void xe_display_pm_suspend(struct xe_device *xe);
 void xe_display_pm_suspend_late(struct xe_device *xe);
 void xe_display_pm_resume_early(struct xe_device *xe);
@@ -75,6 +77,8 @@ static inline void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir)
 static inline void xe_display_irq_reset(struct xe_device *xe) {}
 static inline void xe_display_irq_postinstall(struct xe_device *xe) {}
 
+static inline void xe_display_set_pme_capable(struct xe_device *xe, bool pme_from_hpd) {}
+
 static inline void xe_display_pm_suspend(struct xe_device *xe) {}
 static inline void xe_display_pm_suspend_late(struct xe_device *xe) {}
 static inline void xe_display_pm_resume_early(struct xe_device *xe) {}
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index f8e16aefd2f8..46a33956c5b4 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -1385,6 +1385,8 @@ static int xe_pci_runtime_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct xe_device *xe = pdev_to_xe_device(pdev);
+	pci_power_t state = xe->d3cold.allowed ? PCI_D3cold : PCI_D3hot;
+	bool pme_capable = pci_enable_wake(pdev, state, true) == 0;
 	int err;
 
 	/*
@@ -1396,9 +1398,17 @@ static int xe_pci_runtime_suspend(struct device *dev)
 	xe_assert(xe, !IS_SRIOV_VF(xe));
 	xe_assert(xe, !pci_num_vf(pdev));
 
+	xe_display_set_pme_capable(xe, pme_capable);
+
 	err = xe_pm_runtime_suspend(xe);
-	if (err)
+	if (err) {
+		if (pme_capable) {
+			pci_enable_wake(pdev, state, false);
+			xe_display_set_pme_capable(xe, false);
+		}
+
 		return err;
+	}
 
 	pci_save_state(pdev);
 
@@ -1419,12 +1429,16 @@ static int xe_pci_runtime_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct xe_device *xe = pdev_to_xe_device(pdev);
+	pci_power_t state = xe->d3cold.allowed ? PCI_D3cold : PCI_D3hot;
 	int err;
 
 	err = pci_set_power_state(pdev, PCI_D0);
 	if (err)
 		return err;
 
+	pci_enable_wake(pdev, state, false);
+	xe_display_set_pme_capable(xe, false);
+
 	pci_restore_state(pdev);
 
 	if (xe->d3cold.allowed) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/5] drm/xe/pm: initialize the device's system wakeup capabilities
  2026-09-03  8:04 ` [PATCH v2 1/5] drm/xe/pm: initialize the device's system wakeup capabilities Vinod Govindapillai
@ 2026-09-03  8:15   ` sashiko-bot
  0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-09-03  8:15 UTC (permalink / raw)
  To: Vinod Govindapillai; +Cc: intel-xe, intel-gfx

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Device wakeup capability is enabled unconditionally for all platforms and VFs, falsely advertising PME capability and breaking HPD polling on unsupported platforms.
--

commit 3d9bcb78036d2ff7f08956912516e90ce3bea87a
Author: Vinod Govindapillai <vinod.govindapillai@intel.com>

drm/xe/pm: initialize the device's system wakeup capabilities

This commit enables the device as a wakeup device to allow display hotplugs
to generate PME events for NVL+ hardware.

> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 1e04e8ef2611f..009271ce532c6 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -1216,6 +1216,10 @@ static int __xe_pci_probe(struct pci_dev *pdev, const struct xe_device_desc *des
>  
>  	pci_set_master(pdev);
>  
> +	err = devm_device_init_wakeup(&pdev->dev);
> +	if (err)
> +		return err;
> +

[Severity: High]
Does enabling wakeup unconditionally here falsely advertise PME capabilities
for unsupported platforms (e.g., DG2, MTL, LNL) and SR-IOV VFs?

By calling devm_device_init_wakeup() in __xe_pci_probe(), the driver sets
dev->power.can_wakeup = true for every xe device. Later, during
xe_pci_runtime_suspend(), the code checks:

    pme_capable = pci_enable_wake(pdev, state, true) == 0;

Because wakeup is enabled by default, pci_enable_wake() will incorrectly
return 0 (success) on older PCIe platforms that report standard PCI PM
capabilities.

This erroneously sets pme_capable = true, which is passed to
xe_display_set_pme_capable(). Since intel_hpd_poll_enable() skips polling
if the device is marked PME capable, could this cause hotplug events to be
completely missed during suspend on pre-NVL platforms that cannot actually
generate PMEs from HPDs?

>  	err = xe_probe_info_early(xe, desc, &probed_info);
>  	if (err)
>  		return err;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903080410.489411-1-vinod.govindapillai@intel.com?part=1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] drm/i915/irq: conditional HPD IRQ resets based on PME capability
  2026-09-03  8:04 ` [PATCH v2 3/5] drm/i915/irq: conditional HPD IRQ resets based on PME capability Vinod Govindapillai
@ 2026-09-03  8:32   ` sashiko-bot
  2026-09-04  9:56   ` Jani Nikula
  1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-09-03  8:32 UTC (permalink / raw)
  To: Vinod Govindapillai; +Cc: intel-xe, intel-gfx

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Lost HPD interrupt on system resume causing broken hotplug detection
- [Medium] Unintentionally leaving non-HPD interrupts unmasked during suspend
--

commit 3a88160970f909255e3f0d82a7735b68d8c9df0d
Author: Vinod Govindapillai <vinod.govindapillai@intel.com>

drm/i915/irq: conditional HPD IRQ resets based on PME capability

This commit conditionally skips resetting HPD-related IRQs during suspend
if the device supports generating PME from HPDs. This aims to keep the
device capable of waking from hotplug events.

> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index a59b75830bd13..fc50b19d5ddfe 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
[ ... ]
> @@ -2250,13 +2254,15 @@ static void gen11_display_irq_reset(struct intel_display *display)
>  	irq_reset(display, GEN8_DE_PORT_IRQ_REGS);
>  	irq_reset(display, GEN8_DE_MISC_IRQ_REGS);
>  
> -	if (DISPLAY_VER(display) >= 14)
> -		irq_reset(display, PICAINTERRUPT_IRQ_REGS);
> -	else
> -		irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
> +	if (!keep_hpd) {
> +		if (DISPLAY_VER(display) >= 14)
> +			irq_reset(display, PICAINTERRUPT_IRQ_REGS);
> +		else
> +			irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
>  
> -	if (INTEL_PCH_TYPE(display) >= PCH_ICP)
> -		irq_reset(display, SDE_IRQ_REGS);
> +		if (INTEL_PCH_TYPE(display) >= PCH_ICP)
> +			irq_reset(display, SDE_IRQ_REGS);
> +	}
>  }

[Severity: Medium]
Since PICAINTERRUPT_IRQ_REGS and SDE_IRQ_REGS contain both HPD and non-HPD
interrupts (like GMBUS, I2C, AUX, and Poison errors), does skipping their
reset here leave those non-HPD interrupts unmasked during suspend? Could
this lead to spurious wakeups or unintended interrupt handling while the
system is suspended?

[Severity: High]
During system resume, if keep_hpd evaluates to false (for example, if PME
capability is cleared before runtime resume), this block will execute
irq_reset() for these registers.

Looking at irq_reset(), it unconditionally clears the IIR:

static void irq_reset(struct intel_display *display, struct intel_irq_regs regs)
{
	intel_de_write(display, regs.imr, 0xffffffff);
	intel_de_posting_read(display, regs.imr);
	...
	/* IIR can theoretically queue up two events. Be paranoid. */
	intel_de_write(display, regs.iir, 0xffffffff);
	intel_de_posting_read(display, regs.iir);
	intel_de_write(display, regs.iir, 0xffffffff);
	intel_de_posting_read(display, regs.iir);
}

Does this permanently destroy the pending HPD event that just woke the system
before the IRQ handlers can process it?

Additionally, even if keep_hpd remained true during resume, wouldn't the
subsequent call to irq_init() during postinstall explicitly clear the IIR
anyway?

static void irq_init(struct intel_display *display, ...)
{
	...
	assert_iir_is_zero(display, regs.iir);
	...
}

Will this cause the hotplug event to be completely ignored, preventing the
display from being brought up after a PME wake?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903080410.489411-1-vinod.govindapillai@intel.com?part=3

^ permalink raw reply	[flat|nested] 14+ messages in thread

* ✓ i915.CI.BAT: success for pm_pme support on display hotplug (rev3)
  2026-09-03  8:04 [PATCH v2 0/5] PM_PME support on display hotplug Vinod Govindapillai
                   ` (4 preceding siblings ...)
  2026-09-03  8:04 ` [PATCH v2 5/5] drm/xe/pm: handle the PME capability and runtime pm routines Vinod Govindapillai
@ 2026-09-03 10:26 ` Patchwork
  2026-09-03 14:15 ` ✗ i915.CI.BAT: failure for pm_pme support on display hotplug (rev4) Patchwork
  6 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2026-09-03 10:26 UTC (permalink / raw)
  To: Vinod Govindapillai; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]

== Series Details ==

Series: pm_pme support on display hotplug (rev3)
URL   : https://patchwork.freedesktop.org/series/172306/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_19081 -> Patchwork_172306v3
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172306v3/index.html

Participating hosts (38 -> 37)
------------------------------

  Missing    (1): bat-dg2-13 

Known issues
------------

  Here are the changes found in Patchwork_172306v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@requests:
    - bat-apl-1:          [PASS][1] -> [DMESG-WARN][2] ([i915#13735]) +39 other tests dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19081/bat-apl-1/igt@i915_selftest@live@requests.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172306v3/bat-apl-1/igt@i915_selftest@live@requests.html

  * igt@kms_flip@basic-flip-vs-modeset@b-dp1:
    - bat-apl-1:          [PASS][3] -> [DMESG-FAIL][4] ([i915#180]) +2 other tests dmesg-fail
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19081/bat-apl-1/igt@kms_flip@basic-flip-vs-modeset@b-dp1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172306v3/bat-apl-1/igt@kms_flip@basic-flip-vs-modeset@b-dp1.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-dp1:
    - bat-apl-1:          [PASS][5] -> [DMESG-WARN][6] ([i915#13735] / [i915#180]) +32 other tests dmesg-warn
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19081/bat-apl-1/igt@kms_flip@basic-flip-vs-wf_vblank@b-dp1.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172306v3/bat-apl-1/igt@kms_flip@basic-flip-vs-wf_vblank@b-dp1.html

  
  [i915#13735]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13735
  [i915#180]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/180


Build changes
-------------

  * Linux: CI_DRM_19081 -> Patchwork_172306v3

  CI-20190529: 20190529
  CI_DRM_19081: 9fde1e4fc48d175c52ecacfd163d4c47b22b1c23 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9081: 9081
  Patchwork_172306v3: 9fde1e4fc48d175c52ecacfd163d4c47b22b1c23 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172306v3/index.html

[-- Attachment #2: Type: text/html, Size: 3204 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 5/5] drm/xe/pm: handle the PME capability and runtime pm routines
  2026-09-03  8:04 ` [PATCH v2 5/5] drm/xe/pm: handle the PME capability and runtime pm routines Vinod Govindapillai
@ 2026-09-03 12:36   ` Vinod Govindapillai
  2026-09-04 10:03   ` [PATCH v2 " Jani Nikula
  1 sibling, 0 replies; 14+ messages in thread
From: Vinod Govindapillai @ 2026-09-03 12:36 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: vinod.govindapillai, imre.deak, jouni.hogander

During the runtime suspend, check if device is capable of wakeup
from PME. If yes update the helper so that IRQ reset and HPD
polling can be handled accordingly. For PME capable devices,
HPD related IRQs are not reset during runtime suspend and
do not start polling for HPDs every 10s. Instead PME can be
generated from HPDs and corresponding runtime resume calls
can be invoked by PME.

v2: ensure that both software policy (device_may_wakeup()) and HW/
    Platform (pci_dev_run_wake()) can support device wakeup

Bspec: 52979, 52980, 68857, 68867, 68970
Assisted-by: GitHub_Copilot:claude-opus-5
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/xe/display/xe_display.c | 10 ++++++++++
 drivers/gpu/drm/xe/display/xe_display.h |  4 ++++
 drivers/gpu/drm/xe/xe_pci.c             | 17 ++++++++++++++++-
 drivers/gpu/drm/xe/xe_pm.c              | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_pm.h              |  1 +
 5 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index 7b25c0814674..ff4a74c5c763 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -236,6 +236,16 @@ void xe_display_irq_postinstall(struct xe_device *xe)
 	intel_display_irq_postinstall(display);
 }
 
+void xe_display_set_pme_capable(struct xe_device *xe, bool pme_from_hpd)
+{
+	struct intel_display *display = xe->display;
+
+	if (!xe->info.probe_display)
+		return;
+
+	intel_hpd_set_pme_capable(display, pme_from_hpd);
+}
+
 static bool suspend_to_idle(void)
 {
 #if IS_ENABLED(CONFIG_ACPI_SLEEP)
diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
index 0babb50bfc77..a77a8fa1d802 100644
--- a/drivers/gpu/drm/xe/display/xe_display.h
+++ b/drivers/gpu/drm/xe/display/xe_display.h
@@ -37,6 +37,8 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir);
 void xe_display_irq_reset(struct xe_device *xe);
 void xe_display_irq_postinstall(struct xe_device *xe);
 
+void xe_display_set_pme_capable(struct xe_device *xe, bool pme_from_hpd);
+
 void xe_display_pm_suspend(struct xe_device *xe);
 void xe_display_pm_suspend_late(struct xe_device *xe);
 void xe_display_pm_resume_early(struct xe_device *xe);
@@ -75,6 +77,8 @@ static inline void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir)
 static inline void xe_display_irq_reset(struct xe_device *xe) {}
 static inline void xe_display_irq_postinstall(struct xe_device *xe) {}
 
+static inline void xe_display_set_pme_capable(struct xe_device *xe, bool pme_from_hpd) {}
+
 static inline void xe_display_pm_suspend(struct xe_device *xe) {}
 static inline void xe_display_pm_suspend_late(struct xe_device *xe) {}
 static inline void xe_display_pm_resume_early(struct xe_device *xe) {}
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index f8e16aefd2f8..e510917490d1 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -1385,6 +1385,9 @@ static int xe_pci_runtime_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct xe_device *xe = pdev_to_xe_device(pdev);
+	pci_power_t state = xe->d3cold.allowed ? PCI_D3cold : PCI_D3hot;
+	bool pme_capable = xe_pm_pme_supported(xe) &&
+			   pci_enable_wake(pdev, state, true) == 0;
 	int err;
 
 	/*
@@ -1396,9 +1399,17 @@ static int xe_pci_runtime_suspend(struct device *dev)
 	xe_assert(xe, !IS_SRIOV_VF(xe));
 	xe_assert(xe, !pci_num_vf(pdev));
 
+	xe_display_set_pme_capable(xe, pme_capable);
+
 	err = xe_pm_runtime_suspend(xe);
-	if (err)
+	if (err) {
+		if (pme_capable) {
+			pci_enable_wake(pdev, state, false);
+			xe_display_set_pme_capable(xe, false);
+		}
+
 		return err;
+	}
 
 	pci_save_state(pdev);
 
@@ -1419,12 +1430,16 @@ static int xe_pci_runtime_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct xe_device *xe = pdev_to_xe_device(pdev);
+	pci_power_t state = xe->d3cold.allowed ? PCI_D3cold : PCI_D3hot;
 	int err;
 
 	err = pci_set_power_state(pdev, PCI_D0);
 	if (err)
 		return err;
 
+	pci_enable_wake(pdev, state, false);
+	xe_display_set_pme_capable(xe, false);
+
 	pci_restore_state(pdev);
 
 	if (xe->d3cold.allowed) {
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index f517bf453b54..e6b18968e486 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -92,6 +92,8 @@ static struct lockdep_map xe_pm_block_lockdep_map = {
 };
 #endif
 
+#define HAS_PM_PME_SUPPORT(xe) (GRAPHICS_VERx100(xe) >= 3500)
+
 static void xe_pm_block_begin_signalling(void)
 {
 	lock_acquire_shared_recursive(&xe_pm_block_lockdep_map, 0, 1, NULL, _RET_IP_);
@@ -738,6 +740,28 @@ int xe_pm_runtime_resume(struct xe_device *xe)
 	return err;
 }
 
+/**
+ * xe_pm_pme_supported - Can the device signal PME from its suspend target state?
+ * @xe: xe device instance
+ *
+ * Determine whether the device can generate a Power Management Event while
+ * runtime suspended.
+ *
+ * Return: true if PME is supported from the target state, false otherwise.
+ */
+bool xe_pm_pme_supported(struct xe_device *xe)
+{
+	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
+
+	if (!HAS_PM_PME_SUPPORT(xe))
+		return false;
+
+	if (!device_may_wakeup(&pdev->dev))
+		return false;
+
+	return pci_dev_run_wake(pdev);
+}
+
 /*
  * For places where resume is synchronous it can be quite easy to deadlock
  * if we are not careful. Also in practice it might be quite timing
diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
index 6d5ab09cb769..16b8699e59ec 100644
--- a/drivers/gpu/drm/xe/xe_pm.h
+++ b/drivers/gpu/drm/xe/xe_pm.h
@@ -30,6 +30,7 @@ bool xe_pm_runtime_get_if_active(struct xe_device *xe);
 bool xe_pm_runtime_get_if_in_use(struct xe_device *xe);
 void xe_pm_runtime_get_noresume(struct xe_device *xe);
 bool xe_pm_runtime_resume_and_get(struct xe_device *xe);
+bool xe_pm_pme_supported(struct xe_device *xe);
 void xe_pm_assert_unbounded_bridge(struct xe_device *xe);
 int xe_pm_set_vram_threshold(struct xe_device *xe, u32 threshold);
 void xe_pm_d3cold_allowed_toggle(struct xe_device *xe);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* ✗ i915.CI.BAT: failure for pm_pme support on display hotplug (rev4)
  2026-09-03  8:04 [PATCH v2 0/5] PM_PME support on display hotplug Vinod Govindapillai
                   ` (5 preceding siblings ...)
  2026-09-03 10:26 ` ✓ i915.CI.BAT: success for pm_pme support on display hotplug (rev3) Patchwork
@ 2026-09-03 14:15 ` Patchwork
  6 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2026-09-03 14:15 UTC (permalink / raw)
  To: Vinod Govindapillai; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]

== Series Details ==

Series: pm_pme support on display hotplug (rev4)
URL   : https://patchwork.freedesktop.org/series/172306/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_19083 -> Patchwork_172306v4
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_172306v4 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_172306v4, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172306v4/index.html

Participating hosts (40 -> 38)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_172306v4:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live:
    - bat-arlh-3:         [PASS][1] -> [INCOMPLETE][2] +1 other test incomplete
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19083/bat-arlh-3/igt@i915_selftest@live.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172306v4/bat-arlh-3/igt@i915_selftest@live.html

  


Build changes
-------------

  * Linux: CI_DRM_19083 -> Patchwork_172306v4

  CI-20190529: 20190529
  CI_DRM_19083: b2fada7902896bce389c2b033688d18df36855fc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9082: 2d61f578d998115259b87f9fa27f597ce11a0c89 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_172306v4: b2fada7902896bce389c2b033688d18df36855fc @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172306v4/index.html

[-- Attachment #2: Type: text/html, Size: 2499 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] drm/i915/irq: conditional HPD IRQ resets based on PME capability
  2026-09-03  8:04 ` [PATCH v2 3/5] drm/i915/irq: conditional HPD IRQ resets based on PME capability Vinod Govindapillai
  2026-09-03  8:32   ` sashiko-bot
@ 2026-09-04  9:56   ` Jani Nikula
  2026-09-04 12:12     ` Govindapillai, Vinod
  1 sibling, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2026-09-04  9:56 UTC (permalink / raw)
  To: Vinod Govindapillai, intel-xe, intel-gfx
  Cc: vinod.govindapillai, imre.deak, jouni.hogander

On Thu, 03 Sep 2026, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote:
> If a device supports generating PME from HPDs, resetting HPD IRQs
> will be counter productive as HPDs itself will be lost. During
> suspend routines, all the IRQs are reset. So if the device is
> capable of generating PME rom HPDs, keep the HPD related IRQs from
> reset based on the PME capability of the device on a target power
> state. PME capability will be assessed and updated separately.
>
> Bspec: 52979, 52980, 68857, 68867, 68970
> Assisted-by: GitHub_Copilot:claude-opus-5
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_display_irq.c  | 20 ++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index a59b75830bd1..fc50b19d5ddf 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -22,6 +22,7 @@
>  #include "intel_fdi_regs.h"
>  #include "intel_fifo_underrun.h"
>  #include "intel_gmbus.h"
> +#include "intel_hotplug.h"
>  #include "intel_hotplug_irq.h"
>  #include "intel_lpe_audio.h"
>  #include "intel_parent.h"
> @@ -2217,8 +2218,11 @@ static void gen11_display_irq_reset(struct intel_display *display)
>  	enum pipe pipe;
>  	u32 trans_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
>  		BIT(TRANSCODER_C) | BIT(TRANSCODER_D);
> +	/* If HPD can generate PME, we don't want to reset HPD related IRQs */

Please don't add comments in the middle of the declarations. IMO it's a
distraction rather than helpful.

To be fair, one comment like this here or there would be fine, but it
takes only a few to set an example both for humans and the language
models to start littering the declarations with comments, and I don't
want that.

> +	bool keep_hpd = intel_hpd_can_generate_pme(display);

	bool reset_hpd = !intel_hpd_can_generate_pme(display);

IMO this would be more clear.

> -	intel_de_write(display, GEN11_DISPLAY_INT_CTL, 0);
> +	if (!keep_hpd)
> +		intel_de_write(display, GEN11_DISPLAY_INT_CTL, 0);
>  
>  	if (DISPLAY_VER(display) >= 12) {
>  		enum transcoder trans;
> @@ -2250,13 +2254,15 @@ static void gen11_display_irq_reset(struct intel_display *display)
>  	irq_reset(display, GEN8_DE_PORT_IRQ_REGS);
>  	irq_reset(display, GEN8_DE_MISC_IRQ_REGS);
>  
> -	if (DISPLAY_VER(display) >= 14)
> -		irq_reset(display, PICAINTERRUPT_IRQ_REGS);
> -	else
> -		irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
> +	if (!keep_hpd) {
> +		if (DISPLAY_VER(display) >= 14)
> +			irq_reset(display, PICAINTERRUPT_IRQ_REGS);
> +		else
> +			irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
>  
> -	if (INTEL_PCH_TYPE(display) >= PCH_ICP)
> -		irq_reset(display, SDE_IRQ_REGS);
> +		if (INTEL_PCH_TYPE(display) >= PCH_ICP)
> +			irq_reset(display, SDE_IRQ_REGS);
> +	}
>  }
>  
>  void gen8_irq_power_well_post_enable(struct intel_display *display,

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/5] drm/xe/pm: handle the PME capability and runtime pm routines
  2026-09-03  8:04 ` [PATCH v2 5/5] drm/xe/pm: handle the PME capability and runtime pm routines Vinod Govindapillai
  2026-09-03 12:36   ` [PATCH v3 " Vinod Govindapillai
@ 2026-09-04 10:03   ` Jani Nikula
  1 sibling, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2026-09-04 10:03 UTC (permalink / raw)
  To: Vinod Govindapillai, intel-xe, intel-gfx
  Cc: vinod.govindapillai, imre.deak, jouni.hogander

On Thu, 03 Sep 2026, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote:
> During the runtime suspend, check if device is capable of wakeup
> from PME. If yes update the helper so that IRQ reset and HPD
> polling can be handled accordingly. For PME capable devices,
> HPD related IRQs are not reset during runtime suspend and
> do not start polling for HPDs every 10s. Instead PME can be
> generated from HPDs and corresponding runtime resume calls
> can be invoked by PME.
>
> Bspec: 52979, 52980, 68857, 68867, 68970
> Assisted-by: GitHub_Copilot:claude-opus-5
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/xe/display/xe_display.c | 10 ++++++++++
>  drivers/gpu/drm/xe/display/xe_display.h |  4 ++++
>  drivers/gpu/drm/xe/xe_pci.c             | 16 +++++++++++++++-
>  3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 7b25c0814674..ff4a74c5c763 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -236,6 +236,16 @@ void xe_display_irq_postinstall(struct xe_device *xe)
>  	intel_display_irq_postinstall(display);
>  }
>  
> +void xe_display_set_pme_capable(struct xe_device *xe, bool pme_from_hpd)
> +{
> +	struct intel_display *display = xe->display;
> +
> +	if (!xe->info.probe_display)
> +		return;
> +
> +	intel_hpd_set_pme_capable(display, pme_from_hpd);

The downside with this is that I've been trying hard to *reduce* the
number of direct low-level calls from i915 and xe core to display.

There are 100+ calls already. IMO we need to drop them to the ballpark
of less than half.

This would go directly to a TODO list of things to refactor and clean
up.

Feels like it would be more clean with a parent interface for the
display to ask if pme is available, when needed, even though the parent
interface is also too big...

BR,
Jani.



> +}
> +
>  static bool suspend_to_idle(void)
>  {
>  #if IS_ENABLED(CONFIG_ACPI_SLEEP)
> diff --git a/drivers/gpu/drm/xe/display/xe_display.h b/drivers/gpu/drm/xe/display/xe_display.h
> index 0babb50bfc77..a77a8fa1d802 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.h
> +++ b/drivers/gpu/drm/xe/display/xe_display.h
> @@ -37,6 +37,8 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir);
>  void xe_display_irq_reset(struct xe_device *xe);
>  void xe_display_irq_postinstall(struct xe_device *xe);
>  
> +void xe_display_set_pme_capable(struct xe_device *xe, bool pme_from_hpd);
> +
>  void xe_display_pm_suspend(struct xe_device *xe);
>  void xe_display_pm_suspend_late(struct xe_device *xe);
>  void xe_display_pm_resume_early(struct xe_device *xe);
> @@ -75,6 +77,8 @@ static inline void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir)
>  static inline void xe_display_irq_reset(struct xe_device *xe) {}
>  static inline void xe_display_irq_postinstall(struct xe_device *xe) {}
>  
> +static inline void xe_display_set_pme_capable(struct xe_device *xe, bool pme_from_hpd) {}
> +
>  static inline void xe_display_pm_suspend(struct xe_device *xe) {}
>  static inline void xe_display_pm_suspend_late(struct xe_device *xe) {}
>  static inline void xe_display_pm_resume_early(struct xe_device *xe) {}
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index f8e16aefd2f8..46a33956c5b4 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -1385,6 +1385,8 @@ static int xe_pci_runtime_suspend(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct xe_device *xe = pdev_to_xe_device(pdev);
> +	pci_power_t state = xe->d3cold.allowed ? PCI_D3cold : PCI_D3hot;
> +	bool pme_capable = pci_enable_wake(pdev, state, true) == 0;
>  	int err;
>  
>  	/*
> @@ -1396,9 +1398,17 @@ static int xe_pci_runtime_suspend(struct device *dev)
>  	xe_assert(xe, !IS_SRIOV_VF(xe));
>  	xe_assert(xe, !pci_num_vf(pdev));
>  
> +	xe_display_set_pme_capable(xe, pme_capable);
> +
>  	err = xe_pm_runtime_suspend(xe);
> -	if (err)
> +	if (err) {
> +		if (pme_capable) {
> +			pci_enable_wake(pdev, state, false);
> +			xe_display_set_pme_capable(xe, false);
> +		}
> +
>  		return err;
> +	}
>  
>  	pci_save_state(pdev);
>  
> @@ -1419,12 +1429,16 @@ static int xe_pci_runtime_resume(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct xe_device *xe = pdev_to_xe_device(pdev);
> +	pci_power_t state = xe->d3cold.allowed ? PCI_D3cold : PCI_D3hot;
>  	int err;
>  
>  	err = pci_set_power_state(pdev, PCI_D0);
>  	if (err)
>  		return err;
>  
> +	pci_enable_wake(pdev, state, false);
> +	xe_display_set_pme_capable(xe, false);
> +
>  	pci_restore_state(pdev);
>  
>  	if (xe->d3cold.allowed) {

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] drm/i915/irq: conditional HPD IRQ resets based on PME capability
  2026-09-04  9:56   ` Jani Nikula
@ 2026-09-04 12:12     ` Govindapillai, Vinod
  0 siblings, 0 replies; 14+ messages in thread
From: Govindapillai, Vinod @ 2026-09-04 12:12 UTC (permalink / raw)
  To: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	jani.nikula@linux.intel.com
  Cc: Deak, Imre, Hogander, Jouni

On Fri, 2026-09-04 at 12:56 +0300, Jani Nikula wrote:
> On Thu, 03 Sep 2026, Vinod Govindapillai
> <vinod.govindapillai@intel.com> wrote:
> > If a device supports generating PME from HPDs, resetting HPD IRQs
> > will be counter productive as HPDs itself will be lost. During
> > suspend routines, all the IRQs are reset. So if the device is
> > capable of generating PME rom HPDs, keep the HPD related IRQs from
> > reset based on the PME capability of the device on a target power
> > state. PME capability will be assessed and updated separately.
> > 
> > Bspec: 52979, 52980, 68857, 68867, 68970
> > Assisted-by: GitHub_Copilot:claude-opus-5
> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> > ---
> >  .../gpu/drm/i915/display/intel_display_irq.c  | 20 ++++++++++++---
> > ----
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c
> > b/drivers/gpu/drm/i915/display/intel_display_irq.c
> > index a59b75830bd1..fc50b19d5ddf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> > @@ -22,6 +22,7 @@
> >  #include "intel_fdi_regs.h"
> >  #include "intel_fifo_underrun.h"
> >  #include "intel_gmbus.h"
> > +#include "intel_hotplug.h"
> >  #include "intel_hotplug_irq.h"
> >  #include "intel_lpe_audio.h"
> >  #include "intel_parent.h"
> > @@ -2217,8 +2218,11 @@ static void gen11_display_irq_reset(struct
> > intel_display *display)
> >  	enum pipe pipe;
> >  	u32 trans_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
> >  		BIT(TRANSCODER_C) | BIT(TRANSCODER_D);
> > +	/* If HPD can generate PME, we don't want to reset HPD
> > related IRQs */
> 
> Please don't add comments in the middle of the declarations. IMO it's
> a
> distraction rather than helpful.

Ack. Forgot to remove that!


> 
> To be fair, one comment like this here or there would be fine, but it
> takes only a few to set an example both for humans and the language
> models to start littering the declarations with comments, and I don't
> want that.
> 
> > +	bool keep_hpd = intel_hpd_can_generate_pme(display);
> 
> 	bool reset_hpd = !intel_hpd_can_generate_pme(display);
> 
> IMO this would be more clear.

Okay. Will update.

BR
vinod

> 
> > -	intel_de_write(display, GEN11_DISPLAY_INT_CTL, 0);
> > +	if (!keep_hpd)
> > +		intel_de_write(display, GEN11_DISPLAY_INT_CTL, 0);
> >  
> >  	if (DISPLAY_VER(display) >= 12) {
> >  		enum transcoder trans;
> > @@ -2250,13 +2254,15 @@ static void gen11_display_irq_reset(struct
> > intel_display *display)
> >  	irq_reset(display, GEN8_DE_PORT_IRQ_REGS);
> >  	irq_reset(display, GEN8_DE_MISC_IRQ_REGS);
> >  
> > -	if (DISPLAY_VER(display) >= 14)
> > -		irq_reset(display, PICAINTERRUPT_IRQ_REGS);
> > -	else
> > -		irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
> > +	if (!keep_hpd) {
> > +		if (DISPLAY_VER(display) >= 14)
> > +			irq_reset(display,
> > PICAINTERRUPT_IRQ_REGS);
> > +		else
> > +			irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
> >  
> > -	if (INTEL_PCH_TYPE(display) >= PCH_ICP)
> > -		irq_reset(display, SDE_IRQ_REGS);
> > +		if (INTEL_PCH_TYPE(display) >= PCH_ICP)
> > +			irq_reset(display, SDE_IRQ_REGS);
> > +	}
> >  }
> >  
> >  void gen8_irq_power_well_post_enable(struct intel_display
> > *display,
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-09-04 12:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03  8:04 [PATCH v2 0/5] PM_PME support on display hotplug Vinod Govindapillai
2026-09-03  8:04 ` [PATCH v2 1/5] drm/xe/pm: initialize the device's system wakeup capabilities Vinod Govindapillai
2026-09-03  8:15   ` sashiko-bot
2026-09-03  8:04 ` [PATCH v2 2/5] drm/i915/hotplug: add helpers to track HPDs can generate PME Vinod Govindapillai
2026-09-03  8:04 ` [PATCH v2 3/5] drm/i915/irq: conditional HPD IRQ resets based on PME capability Vinod Govindapillai
2026-09-03  8:32   ` sashiko-bot
2026-09-04  9:56   ` Jani Nikula
2026-09-04 12:12     ` Govindapillai, Vinod
2026-09-03  8:04 ` [PATCH v2 4/5] drm/i915/hotplug: avoid HPD polling if the device is PME capable Vinod Govindapillai
2026-09-03  8:04 ` [PATCH v2 5/5] drm/xe/pm: handle the PME capability and runtime pm routines Vinod Govindapillai
2026-09-03 12:36   ` [PATCH v3 " Vinod Govindapillai
2026-09-04 10:03   ` [PATCH v2 " Jani Nikula
2026-09-03 10:26 ` ✓ i915.CI.BAT: success for pm_pme support on display hotplug (rev3) Patchwork
2026-09-03 14:15 ` ✗ i915.CI.BAT: failure for pm_pme support on display hotplug (rev4) Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox