Re: [PATCH v2 5/5] drm/xe/pm: handle the PME capability and runtime pm routines

["Govindapillai, Vinod" <vinod.govindapillai@intel.com>]

On Tue, 2026-09-08 at 06:19 +0000, Hogander, Jouni wrote:
> On Fri, 2026-09-04 at 13:03 +0300, Jani Nikula wrote:
> > 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...
> 
> Would it be ok to solve this by doing all this in display runtime
> callbacks?
> 
> Possible d3cold entry could be found out by using pci_target_state.
> 
> Is it ok to acces pci device (pci_enable_wake and pci_target_state)
> in
> display pm runtime callbacks.
> 
> BR,
> Jouni Högander
> 
> > 
> > BR,
> > Jani.
> > 
> > 
> > 

> > 
Btw, there is an updated version of this patch.. the diff is 

+#define HAS_PM_PME_SUPPORT(xe) (GRAPHICS_VERx100(xe) >= 3500)
+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);
+}

Because I noticed that we need to call "pci_dev_run_wake" to see the hw
policy also supports PME. device_may_wakeup() check the any user
controlled changes to the wakeup policy through debugfs.
HAS_PM_PME_SUPPORT() for the bspec.. which is not really needed I think
- pci_dev_run_wake() should handle that. But anyway this gives a
clarity when this feature was introduced and if any other code
variations are required in future.

Also note, pci_target_state() is a static function. But anyway
pci_dev_run_wake() use that internally to see return if PME if
supported.

Trying to solve two things here..

1. if PME is supported we need to avoid HPD IRQ resets during xe IRQ
resets which is part of the suspend routine.

2. Avoid HPD polling

Well.. I wasn't super happy about this intel_hpd_set_pme_capable()
interface. The alternative is "keep_Hpd" plumbed in to the IRQ reset
calls in the previous iteration, to avoid resetting HPD IRQs for PME.

So far intel_hpd_init, intel_hpd_poll_enable/disable are being called
from xedisplay and have common implementation for xe and i915. Thats
the reason I didnt move this to the intel_parent interface.

Also I wanted to keep a single point check for the pme_capablity in
xe_pci_runtime_suspend() so that the source of the logic for avoiding
HPD IRQ resets and avoiding HPD polling from a single call to check pme
capability - as /sys/bus/pci/devices/0000:00:02.0/power/wakeup could
affect the wakeup policy.

BR
Vinod