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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 08 Sep 2026, "Hogander, Jouni" <jouni.hogander@intel.com> 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.

Full disclosure: I really don't know how this xe d3cold runtime
suspend/resume path is supposed to work. i915 disables PCI d3cold before
runtime suspend, and enables it again at runtime resume.

The display part should be unified for both (though its fine if i915
disables it like it does now), and xe really should stop calling
low-level display functions directly from the xe_display_enable_d3cold()
and xe_display_disable_d3cold() functions.

BR,
Jani.









>
> 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) {
>> 
>

-- 
Jani Nikula, Intel