Re: [PATCH] drm/xe/d3cold: Set power state to D3Cold during s2idle/s3

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Mar 27, 2025 at 10:02:29PM +0530, Nilawar, Badal wrote:
> 
> On 27-03-2025 21:49, Badal Nilawar wrote:
> Hi Rodrigo,
> 
> > According to pci core guidelines, pci_save_config is recommended when the
> > driver explicitly needs to set the pci power state. As of now xe kmd is
> > only doing pci_save_config while entering to s2idle/s3 state, which makes
> > pci core think that device driver has already applied required pci power
> > state. This leads to GPU remain in D0 state. To fix the issue setting
> > the pci power state to D3Cold.
> > 
> > Fixes:dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_pci.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > index 7046e7e9a6c7..3317d475be79 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -932,6 +932,7 @@ static int xe_pci_suspend(struct device *dev)
> >   	pci_save_state(pdev);
> >   	pci_disable_device(pdev);
> > +	pci_set_power_state(pdev, PCI_D3cold);
> 
> Another approach to avoid calling pci_save_state and pci_set_power_state,
> allowing the PCI core to manage this.
> Currently, the above change aligns with the Xe RPM suspend flow.

Either way is fine it seems. Or we don't save the state and let pci subsystem
handle that for us or we save and set explicitly. So, let's move quickly
with this option here that is already fixing our current issue.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Regards,
> Badal
> 
> >   	return 0;
> >   }