> -----Original Message----- > From: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx> > Sent: Saturday, July 22, 2023 12:30 AM > To: Gupta, Anshuman <anshuman.gupta@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 1/4] drm/xe: Only set PCI d3cold_allowed when we are > really allowing. > > On Fri, Jul 21, 2023 at 03:39:35AM -0400, Gupta, Anshuman wrote: > > > > > > > -----Original Message----- > > > From: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx> > > > Sent: Friday, July 21, 2023 2:34 AM > > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Gupta, Anshuman > > > <anshuman.gupta@xxxxxxxxx> > > > Subject: [PATCH 1/4] drm/xe: Only set PCI d3cold_allowed when we are > > > really allowing. > > > > > > First of all it was strange to see: > > > if (allowed) { > > > ... > > > } else { > > > D3COLD_ENABLE > > > } > > > > > > But besides this misalignment, let's also use the pci d3cold_allowed > > > useful to us and know that we are not really allowing d3cold. > > > > > > Cc: Anshuman Gupta <anshuman.gupta@xxxxxxxxx> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > --- > > > drivers/gpu/drm/xe/xe_pci.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/xe/xe_pci.c > > > b/drivers/gpu/drm/xe/xe_pci.c index 78df43c20cd2..0c4051f4f746 > > > 100644 > > > --- a/drivers/gpu/drm/xe/xe_pci.c > > > +++ b/drivers/gpu/drm/xe/xe_pci.c > > > @@ -794,6 +794,7 @@ static int xe_pci_runtime_suspend(struct device > > > *dev) > > > pci_save_state(pdev); > > > > > > if (xe->d3cold.allowed) { > > > + d3cold_toggle(pdev, D3COLD_ENABLE); > > > pci_disable_device(pdev); > > > pci_ignore_hotplug(pdev); > > > pci_set_power_state(pdev, PCI_D3cold); @@ -823,8 +824,6 > @@ static > > > int xe_pci_runtime_resume(struct device *dev) > > > return err; > > > > > > pci_set_master(pdev); > > > - } else { > > > - d3cold_toggle(pdev, D3COLD_ENABLE); > > > } > > During s2idle , d3cold may get disabled if won't restore it's state in runtime > resume. > > I always forget about that case... please disregard this patch. I think , we can have this patch. I realized that system wide suspend/resume d3cold need some brainstorming. If device is already in runtime suspend state during system wide suspend, PM core may smartly skip device suspend/resume callback on Xe driver. This wasn't the case on I915 driver as it explicitly disables those smart optimization by dev_pm_set_driver_flags(kdev, DPM_FLAG_NO_DIRECT_COMPLETE). Thanks, Anshuman. Thanks, Anshuman Gupta. > > > Thanks, > > Anshuman Gupta. > > > > > > return xe_pm_runtime_resume(xe); > > > -- > > > 2.41.0 > >