Hi Ben, On Tue, Nov 08, 2016 at 09:29:38PM +0100, Peter Wu wrote: > On Tue, Nov 08, 2016 at 12:57:00PM +0100, Lukas Wunner wrote: > > nouveau's ->suspend and ->resume callbacks are currently skipped if the > > device's status is either DRM_SWITCH_POWER_OFF (powered off by > > vga_switcheroo manual power control) or DRM_SWITCH_POWER_DYNAMIC_OFF > > (runtime suspended). > > > > In the former case this makes sense since the device is powered off > > behind the PM core's back: It will try to execute the ->suspend and > > ->resume callbacks upon system sleep, not knowing that the device is > > inaccessible. Therefore the callbacks have to become no-ops. > > > > However the latter case doesn't make any sense because the PM core > > never calls the ->suspend and ->resume callbacks of runtime suspended > > devices: Such devices are either runtime resumed before going to system > > sleep (see call to pm_runtime_resume() in drivers/pci/pci-driver: > > pci_pm_suspend()) or they are left runtime suspended over the entire > > system suspend/resume process (search for "direct_complete" in > > drivers/base/power/main.c). > > > > Consequently the DRM_SWITCH_POWER_DYNAMIC_OFF checks are superfluous. > > Drop them. > > > > Signed-off-by: Lukas Wunner <[email protected]>
Just a gentle ping: This patch was posted a month ago and I'm not seeing it in your tree. Do you have objections? Should I repost with Peter's Reviewed-by? Thanks, Lukas > > It is better to rely on the official documentation rather than the > implementation. Luckily, Documentation/power/pci.txt supports the claim: > > 2.4.1. System Suspend > > When the system is going into a sleep state in which the contents of > memory will > be preserved, such as one of the ACPI sleep states S1-S3, the phases are: > > prepare, suspend, suspend_noirq. > [..] > The pci_pm_prepare() routine first puts the device into the "fully > functional" > state with the help of pm_runtime_resume(). [..] > > So indeed we can be sure that the device is runtime-resumed before > suspend. System resume is not documented explicitly, but it seems > reasonable that the device is not runtime-suspended between system > suspend and resume. > > Reviewed-by: Peter Wu <[email protected]> > > > --- > > drivers/gpu/drm/nouveau/nouveau_drm.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c > > b/drivers/gpu/drm/nouveau/nouveau_drm.c > > index 9876e6f..d91d092 100644 > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > > @@ -666,8 +666,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev, > > struct drm_device *drm_dev = pci_get_drvdata(pdev); > > int ret; > > > > - if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF || > > - drm_dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF) > > + if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF) > > return 0; > > > > ret = nouveau_do_suspend(drm_dev, false); > > @@ -688,8 +687,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev, > > struct drm_device *drm_dev = pci_get_drvdata(pdev); > > int ret; > > > > - if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF || > > - drm_dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF) > > + if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF) > > return 0; > > > > pci_set_power_state(pdev, PCI_D0); > > -- > > 2.10.1 _______________________________________________ Nouveau mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/nouveau
