On Fri, 2026-09-04 at 13:03 +0300, Jani Nikula wrote:
> On Thu, 03 Sep 2026, Vinod Govindapillai
> <[email protected]> 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 <[email protected]>
> > ---
> > 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) {
>
