search for: runtime_suspend

...e to keep > the device awake for whenever possible. So, grab a PM ref safely in our > hotplug handlers using pm_runtime_get_noresume() and mark the device as > busy once we're finished. Patch looks fine in principle, but doesn't seem to be sufficient to fix the following race: 1. runtime_suspend commences 2. user plugs in a display before the runtime_suspend worker disables hotplug interrupts in nouveau_display_fini() 3. hotplug is handled, display is lit up 4. runtime_suspend worker waits for hotplug handler to finish 5. GPU is runtime suspended and the newly plugged in display goes bl...

Fix a deadlock on hybrid graphics laptops that's been present since 2013: DRM drivers poll connectors in 10 sec intervals. The poll worker is stopped on ->runtime_suspend with cancel_delayed_work_sync(). However the poll worker invokes the DRM drivers' ->detect callbacks, which call pm_runtime_get_sync(). If the poll worker starts after runtime suspend has begun, pm_runtime_get_sync() will wait for runtime suspend to finish with the intention of runtime res...

On 11 February 2018 at 09:38, Lukas Wunner <lukas at wunner.de> wrote: > Fix a deadlock on hybrid graphics laptops that's been present since 2013: > > DRM drivers poll connectors in 10 sec intervals. The poll worker is > stopped on ->runtime_suspend with cancel_delayed_work_sync(). However > the poll worker invokes the DRM drivers' ->detect callbacks, which call > pm_runtime_get_sync(). If the poll worker starts after runtime suspend > has begun, pm_runtime_get_sync() will wait for runtime suspend to finish > with the inte...

Hi Lukas, On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > Fix a deadlock on hybrid graphics laptops that's been present since 2013: > > DRM drivers poll connectors in 10 sec intervals. The poll worker is > stopped on ->runtime_suspend with cancel_delayed_work_sync(). However > the poll worker invokes the DRM drivers' ->detect callbacks, which call > pm_runtime_get_sync(). If the poll worker starts after runtime suspend > has begun, pm_runtime_get_sync() will wait for runtime suspend to finish > with the inte...

...poll_execute(), and focus on the latter two. As said I'm unfamiliar with MST but browsing through drm_dp_mst_topology.c I notice that drm_dp_mst_link_probe_work() is the ->work element in drm_dp_mst_topology_mgr() and is queued on HPD. I further notice that the work item is flushed on ->runtime_suspend: nouveau_pmops_runtime_suspend() nouveau_do_suspend() nouveau_display_suspend() nouveau_display_fini() disp->fini() == nv50_display_fini() nv50_mstm_fini() drm_dp_mst_topology_mgr_suspend() flush_work(&mgr->work); And before the work item is flushed, t...

On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > Fix a deadlock on hybrid graphics laptops that's been present since 2013: > > DRM drivers poll connectors in 10 sec intervals. The poll worker is > stopped on ->runtime_suspend with cancel_delayed_work_sync(). However > the poll worker invokes the DRM drivers' ->detect callbacks, which call > pm_runtime_get_sync(). If the poll worker starts after runtime suspend > has begun, pm_runtime_get_sync() will wait for runtime suspend to finish > with the inte...

...018 at 09:38, Lukas Wunner <lukas at wunner.de> wrote: > > The patches for radeon and amdgpu are compile-tested only, I only have a > > MacBook Pro with an Nvidia GK107 to test. To test the patches, add an > > "msleep(12*1000);" at the top of the driver's ->runtime_suspend hook. > > This ensures that the poll worker runs after ->runtime_suspend has begun. > > Wait 12 sec after the GPU has begun runtime suspend, then check > > /sys/bus/pci/devices/0000:01:00.0/power/runtime_status. Without this > > series, the status will be stuck at "...

On Sun, 2018-02-11 at 10:38 +0100, Lukas Wunner wrote: > Introduce a helper to determine if the current task is an output poll > worker. > > This allows us to fix a long-standing deadlock in several DRM drivers > wherein the ->runtime_suspend callback waits for the output poll worker > to finish and the worker in turn calls a ->detect callback which waits > for runtime suspend to finish. The ->detect callback is invoked from > multiple call sites and waiting for runtime suspend to finish is the > correct thing to do e...

...ev))) { > > ret = pm_runtime_get_sync(dev); > > if (ret < 0 && ret != -EACCES) > > return ret; > > } > > > > But here's the catch: This only works for an *async* runtime suspend. > > It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc, > > because then the runtime suspend is executed in the context of the caller, > > not in the context of dev->power.work. > > > > So it's not a full solution, but hopefully something that gets you > > going. I'm not really familiar with the code pat...

...triggers > an atomic commit which triggers a call to pm_runtime_get_sync() > - the output poll thread deadlocks the pm core waiting for the pm core > to finish the autosuspend request while the pm core waits for the > output poll thread to finish This will still deadlock if ->runtime_suspend commences before the hotplug event and the hotplug event occurs before polling has been disabled by ->runtime_suspend. The correct fix is to call pm_runtime_get_sync() *conditionally* in the atomic commit which enables the display, using the same conditional as d61a5c106351, i.e. if (!drm_kms_h...

nouveau's ->runtime_suspend hook calls drm_kms_helper_poll_disable(), which waits for the output poll worker to finish if it's running. The output poll worker meanwhile calls pm_runtime_get_sync() in nouveau_connector_detect() which waits for the ongoing suspend to finish, causing a deadlock. Fix by not acquiring a runt...

amdgpu's ->runtime_suspend hook calls drm_kms_helper_poll_disable(), which waits for the output poll worker to finish if it's running. The output poll worker meanwhile calls pm_runtime_get_sync() in amdgpu's ->detect hooks, which waits for the ongoing suspend to finish, causing a deadlock. Fix by not acquiring a...

...ukas Wunner <lukas at wunner.de> wrote: > > > The patches for radeon and amdgpu are compile-tested only, I only have a > > > MacBook Pro with an Nvidia GK107 to test. To test the patches, add an > > > "msleep(12*1000);" at the top of the driver's ->runtime_suspend hook. > > > This ensures that the poll worker runs after ->runtime_suspend has begun. > > > Wait 12 sec after the GPU has begun runtime suspend, then check > > > /sys/bus/pci/devices/0000:01:00.0/power/runtime_status. Without this > > > series, the status wil...

On Tue, Feb 13, 2018 at 10:55:06AM +0000, Liviu Dudau wrote: > On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > > DRM drivers poll connectors in 10 sec intervals. The poll worker is > > stopped on ->runtime_suspend with cancel_delayed_work_sync(). However > > the poll worker invokes the DRM drivers' ->detect callbacks, which call > > pm_runtime_get_sync(). If the poll worker starts after runtime suspend > > has begun, pm_runtime_get_sync() will wait for runtime suspend to finish &gt...

On Fri, Feb 24, 2017 at 1:19 PM, Lukas Wunner <lukas at wunner.de> wrote: > An external Thunderbolt GPU can neither drive the laptop's panel nor be > powered off by the platform, so there's no point in registering it with > vga_switcheroo. > In fact, when the external GPU is runtime suspended, > vga_switcheroo will cut power to the internal discrete GPU, resulting in

Introduce a helper to determine if the current task is an output poll worker. This allows us to fix a long-standing deadlock in several DRM drivers wherein the ->runtime_suspend callback waits for the output poll worker to finish and the worker in turn calls a ->detect callback which waits for runtime suspend to finish. The ->detect callback is invoked from multiple call sites and waiting for runtime suspend to finish is the correct thing to do except if it's ex...

...side nouveau_pmops_runtime_resume(). > > > This includes modesetting locks, i2c bus locks, etc. > > > > [snip] > > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > > > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *dev) > > > return -EBUSY; > > > } > > > > > > + dev->power.disable_depth++; > > > + > > > > Anyway, if I understand the commit message correctly, you're hitting a > > pm_runtime_get_sync() in a code path th...

...c commit which triggers a call to pm_runtime_get_sync() > > - the output poll thread deadlocks the pm core waiting for the pm core > > to finish the autosuspend request while the pm core waits for the > > output poll thread to finish > > This will still deadlock if ->runtime_suspend commences before the > hotplug event and the hotplug event occurs before polling has been > disabled by ->runtime_suspend. > > The correct fix is to call pm_runtime_get_sync() *conditionally* in > the atomic commit which enables the display, using the same conditional > as d61...

...+0100, Daniel Vetter wrote: > On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > > Fix a deadlock on hybrid graphics laptops that's been present since 2013: > > > > DRM drivers poll connectors in 10 sec intervals. The poll worker is > > stopped on ->runtime_suspend with cancel_delayed_work_sync(). However > > the poll worker invokes the DRM drivers' ->detect callbacks, which call > > pm_runtime_get_sync(). If the poll worker starts after runtime suspend > > has begun, pm_runtime_get_sync() will wait for runtime suspend to finish &gt...

...as "inflating the code"; I > > would think of it as "having fewer special cases", i.e., we always use > > the same restore path instead of having the main one plus a special > > one for unbound devices. > > That is a fair point, but if you look at pci_pm_runtime_suspend(), it > doesn't actually save anything for devices without drivers, so we'd > just restore the original initial state for them every time. > > If we are to restore the entire state on runtime resume, > pci_pm_runtime_suspend() should be changed to call pci_save_state() >...