On Fri, Sep 16, 2016 at 06:07:36PM -0400, Alex Deucher wrote: > On Fri, Sep 16, 2016 at 5:38 PM, Lukas Wunner <lukas@xxxxxxxxx> wrote: > > On Fri, Sep 16, 2016 at 04:42:43PM -0400, Alex Deucher wrote: > >> drm/amdgpu: skip suspend/resume on DRM_SWITCH_POWER_DYNAMIC_OFF > >> drm/radeon: skip suspend/resume on DRM_SWITCH_POWER_DYNAMIC_OFF > > > > Those two are unnecessary, it can't happen that the ->suspend hook > > is executed with the device runtime suspended. > > > > Since commit d14d2a8453d6 ("drm: Remove dev_pm_ops from drm_class"), > > DRM devices are afforded direct_complete, i.e. if the GPU is runtime > > suspended upon system sleep, it is left in this state. The only > > callbacks that are executed are the ->prepare and the ->complete hook. > > All the callbacks in-between, like ->suspend, are skipped. > > > > Even if direct_complete is not afforded for some reason, the PM core > > will automatically runtime resume the device before executing ->suspend. > > > > That ->suspend is skipped in the DRM_SWITCH_POWER_OFF case was done > > because the device is suspended behind the PM core's back if runpm=0 > > is set. (And it doesn't work properly because the PCI core will > > invalidate the saved_state during ->resume_noirq. That could be > > solved by returning 1 from the ->prepare hook in the DRM_SWITCH_POWER_OFF > > case, but that's a different story.) > > > > (Sorry for not raising this earlier, I'm not subscribed to amd-gfx.) > > Thanks for the heads up. They shouldn't hurt anything and it matches > what other drivers do. If you feel strongly about it, I can revert > them later. Well, superfluous code shouldn't be included just because it doesn't hurt or because others didn't know better either, or removed because of someone else's feelings. ;-) You can find the runtime resume before execution of the driver ->suspend callback in drivers/pci/pci-driver.c:pci_pm_suspend(): const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; ... pm_runtime_resume(dev); ... if (pm->suspend) { ... error = pm->suspend(dev); The device is prevented from auto-suspending because of a runtime PM ref acquired beforehand in drivers/base/power/main.c:device_prepare(). Best regards, Lukas _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel