Re: [PATCH] drm/xe: Restore pci state upon resume

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

On Thu, Sep 12, 2024 at 03:11:12PM -0500, Lucas De Marchi wrote:
> On Thu, Sep 12, 2024 at 03:05:30PM GMT, Rodrigo Vivi wrote:
> > The pci state was saved, but not restored. Restore
> > right after the power state transition request like
> > every other driver.
> > 
> > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> 
> oh, no
> 
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_pci.c | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > index 5ba4ec229494..6d29ef4b396f 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -949,6 +949,8 @@ static int xe_pci_resume(struct device *dev)
> > 	if (err)
> > 		return err;
> > 
> > +	pci_restore_state(pdev);
> > +
> 
> something bad happened... I was sure this line was here before. Indeed.
> git log -L:xe_pci_resume:drivers/gpu/drm/xe/xe_pci.c to the rescue
> 
> See commit f6761c68c0ac ("drm/xe/display: Improve s2idle handling.")

hmmm...
Perhaps

Fixes: f6761c68c0ac ("drm/xe/display: Improve s2idle handling.")
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

then?!

The restoration shouldn't have anything to do with that case.
and it pairs with pci_save_state(pdev) is still there...

It is not in i915, but I have an i915 patch to add the restore
there as well.

Also, I might as well in a follow up bring back the
pci_set_power_state(pdev, PCI_D3hot) to Xe.
This is probably the right way, aligns with all other drivers
and i915. So that shouldn't be the cause of the power drainage
on suspend anyway, or i915 would be also draining it.

> 
> Lucas De Marchi
> 
> > 	err = pci_enable_device(pdev);
> > 	if (err)
> > 		return err;
> > -- 
> > 2.46.0
> >