Re: [PATCH v6] drm/xe/pm: Disable RPM for SR-IOV VFs

[Matthew Brost <matthew.brost@intel.com>]

On Sun, Aug 17, 2025 at 12:40:55PM -0400, Rodrigo Vivi wrote:
> On Tue, Aug 12, 2025 at 10:06:13PM +0530, Satyanarayana K V P wrote:
> > VFs without native PCIe Power Management (PM) capabilities inherit their
> > PF's power state as per PCIe specifications(§5.10.1 PCIe Base Spec 7.0).
> > Enabling Runtime Power Management (RPM) for these VFs trigger unnecessary
> > driver suspend/resume operations that ultimately perform no PCI-level power
> > transition.
> > 
> > Since VFs without PM capabilities cannot independently enter low-power
> > states, the existing RPM workflow becomes redundant:
> > 1. Driver executes full suspend/resume sequence
> > 2. PCI PM transition step becomes no-op
> > 3. VF power state remains tied to PF's status
> > 
> > Disabling RPM for VFs eliminates this redundant processing while
> > maintaining proper power management through PF dependency. This
> > optimization ensures VFs follow their PF's power state without superfluous
> > runtime handling.
> > 
> > Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Maarten Lankhorst <dev@lankhorst.se>
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> and pushing
> 

This might need a little more thought(?). What are the full consequences
of this patch? For example, will xe_pm_suspend()/xe_pm_resume() never be
called on a VF? I think that’s correct and desirable, but consider code
paths like:

if (xe_pm_runtime_get_if_active())
        /* Take action assuming GPU state is valid */
else
        /* Take action assuming GPU state has been lost */


On a VF, this is now broken, since we don’t lose GPU state—unless I’m
missing something, or xe_pm_runtime_get_if_active() always returns true
on a VF which in that case we should be good.

Matt

> > 
> > ---
> > V5 -> V6:
> > - Updated comment (Rodrigo).
> > 
> > V4 -> V5:
> > - Disable RPM for VFs without checking pm_cap (Rodrigo).
> > - Updated commit title.
> > 
> > V3 -> V4:
> > - Fixed review comments (Rodrigo & Maarten)
> > 
> > V2 -> V3:
> > - Fixed review comments (Michal Wajdeczko).
> > 
> > V1 -> V2:
> > - Disable RPM only for VF devices when PM cap is not implemented.
> > ---
> >  drivers/gpu/drm/xe/xe_pm.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index 5e8126ca8e27..51934d5dc44f 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -247,6 +247,10 @@ static void xe_pm_runtime_init(struct xe_device *xe)
> >  {
> >  	struct device *dev = xe->drm.dev;
> >  
> > +	/* Our current VFs do not support RPM. so, disable it */
> > +	if (IS_SRIOV_VF(xe))
> > +		return;
> > +
> >  	/*
> >  	 * Disable the system suspend direct complete optimization.
> >  	 * We need to ensure that the regular device suspend/resume functions
> > @@ -367,6 +371,10 @@ static void xe_pm_runtime_fini(struct xe_device *xe)
> >  {
> >  	struct device *dev = xe->drm.dev;
> >  
> > +	/* Our current VFs do not support RPM. so, disable it */
> > +	if (IS_SRIOV_VF(xe))
> > +		return;
> > +
> >  	pm_runtime_get_sync(dev);
> >  	pm_runtime_forbid(dev);
> >  }
> > -- 
> > 2.43.0
> >