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

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

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