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

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

Hi

I noticed some lockdep splat in kernel log in some scenarios during
tesing and figured out that is coming from the pci pme apis which do
kmalloc. So I updated the series to handle that. Also tried to avoid
any explicit cross display calls and all the relevant calls are wrapped
to the intel parent interfaces.

BR
Vinod

On Tue, 2026-09-08 at 10:41 +0300, Govindapillai, Vinod wrote:
> 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