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

["Hogander, Jouni" <jouni.hogander@intel.com>]

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.
> 
> 
> 
> > +}
> > +
> >  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) {
>