search for: drm_kms_helper_is_poll_worker

...nnectors.c @@ -736,9 +736,11 @@ amdgpu_connector_lvds_detect(struct drm_connector *connector, bool force) enum drm_connector_status ret = connector_status_disconnected; int r; - r = pm_runtime_get_sync(connector->dev->dev); - if (r < 0) - return connector_status_disconnected; + if (!drm_kms_helper_is_poll_worker()) { + r = pm_runtime_get_sync(connector->dev->dev); + if (r < 0) + return connector_status_disconnected; + } if (encoder) { struct amdgpu_encoder *amdgpu_encoder = to_amdgpu_encoder(encoder); @@ -757,8 +759,12 @@ amdgpu_connector_lvds_detect(struct drm_connector *connector, boo...

...m/drm_probe_helper.c index 6dc2dde5b672..7a6b2dc08913 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -654,6 +654,26 @@ static void output_poll_execute(struct work_struct *work) schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD); } +/** + * drm_kms_helper_is_poll_worker - is %current task an output poll worker? + * + * Determine if %current task is an output poll worker. This can be used + * to select distinct code paths for output polling versus other contexts. + * + * One use case is to avoid a deadlock between the output poll worker and + * the autosuspend wor...

...ce1 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -653,6 +653,20 @@ static void output_poll_execute(struct work_struct > *work) > schedule_delayed_work(delayed_work, > DRM_OUTPUT_POLL_PERIOD); > } > > +/** > + * drm_kms_helper_is_poll_worker - is %current task an output poll worker? > + * > + * Determine if %current task is an output poll worker. This can be used > + * to select distinct code paths for output polling versus other contexts. > + */ For this, it would be worth explicitly noting in the comments herethat this s...

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,

...m/drm_probe_helper.c index 555fbe54d6e2..019881d15ce1 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -653,6 +653,20 @@ static void output_poll_execute(struct work_struct *work) schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD); } +/** + * drm_kms_helper_is_poll_worker - is %current task an output poll worker? + * + * Determine if %current task is an output poll worker. This can be used + * to select distinct code paths for output polling versus other contexts. + */ +bool drm_kms_helper_is_poll_worker(void) +{ + struct work_struct *work = current_work(); + + ret...

...spend request while the pm core waits for the > > > output poll thread to finish > > > > 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_helper_is_poll_worker()). First of all, I was mistaken when I wrote above that a check for !drm_kms_helper_is_poll_worker() would solve the problem. Sorry! It doesn't because the call to pm_runtime_get_sync() is not happening in output_poll_execute() but in drm_dp_mst_link_probe_work(). Looking once more at the t...

...s are only polled while runtime active, so resuming the + * device here is unnecessary (and would deadlock upon runtime suspend + * because it waits for polling to finish). We do however, want to + * prevent the autosuspend timer from elapsing during this operation + * if possible. */ - if (!drm_kms_helper_is_poll_worker()) { - ret = pm_runtime_get_sync(connector->dev->dev); + if (drm_kms_helper_is_poll_worker()) { + pm_runtime_get_noresume(dev->dev); + } else { + ret = pm_runtime_get_sync(dev->dev); if (ret < 0 && ret != -EACCES) return conn_status; } @@ -655,10 +659,8 @@ nouveau...

...> > +++ b/drivers/gpu/drm/drm_probe_helper.c > > @@ -653,6 +653,20 @@ static void output_poll_execute(struct work_struct > > *work) > > schedule_delayed_work(delayed_work, > > DRM_OUTPUT_POLL_PERIOD); > > } > > > > +/** > > + * drm_kms_helper_is_poll_worker - is %current task an output poll worker? > > + * > > + * Determine if %current task is an output poll worker. This can be used > > + * to select distinct code paths for output polling versus other contexts. > > + */ > For this, it would be worth explicitly noting in the...

...e_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_helper_is_poll_worker()). Now I realize I sent you down the wrong path when I suggested to introduce a DRM helper here. My apologies, I didn't fully appreciate what is going awry here! Anything that happens in nouveau's poll worker never needs to acquire a runtime PM ref because polling is only enabled while...

...onnector->dev->dev); - if (ret < 0 && ret != -EACCES) - return conn_status; + /* Outputs are only polled while runtime active, so acquiring a + * runtime PM ref here is unnecessary (and would deadlock upon + * runtime suspend because it waits for polling to finish). + */ + if (!drm_kms_helper_is_poll_worker()) { + ret = pm_runtime_get_sync(connector->dev->dev); + if (ret < 0 && ret != -EACCES) + return conn_status; + } nv_encoder = nouveau_connector_ddc_detect(connector); if (nv_encoder && (i2c = nv_encoder->i2c) != NULL) { @@ -647,8 +653,10 @@ nouveau_connector_d...

...me active, so > + * resuming the device here is unnecessary (and would deadlock upon > + * runtime suspend because it waits for polling to finish). We do > + * however, want to prevent the autosuspend timer from elapsing during > + * this operation if possible. > */ > - if (drm_kms_helper_is_poll_worker()) { > + if (drm_kms_helper_is_poll_worker() || Uh, this entirely slipped through my rader. Looks very much like a hack. If you want to avoid doing runtime pm from the poll worker imo the right thing to do would be to use pm_runtime_get_if_in_use() and skip the entire detection (returning the c...

This is the latest version of https://patchwork.freedesktop.org/series/46815/ I moved everything out of fb_helper and back into nouveau, because it seems that other drivers actually do have this handled already as far as I can tell. Lyude Paul (13): drm/nouveau: Fix bogus drm_kms_helper_poll_enable() placement drm/nouveau: Remove duplicate poll_enable() in pmops_runtime_suspend()

...> 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_helper_is_poll_worker()). > > Now I realize I sent you down the wrong path when I suggested to > introduce a DRM helper here. My apologies, I didn't fully appreciate > what is going awry here! > > Anything that happens in nouveau's poll worker never needs to acquire > a runtime PM ref bec...

...happens while we're runtime active, so + * resuming the device here is unnecessary (and would deadlock upon + * runtime suspend because it waits for polling to finish). We do + * however, want to prevent the autosuspend timer from elapsing during + * this operation if possible. */ - if (drm_kms_helper_is_poll_worker()) { + if (drm_kms_helper_is_poll_worker() || + nv_connector->hpd_task == current) { pm_runtime_get_noresume(dev->dev); } else { ret = pm_runtime_get_sync(dev->dev); @@ -1151,6 +1152,8 @@ nouveau_connector_hotplug(struct nvif_notify *notify) const char *name = connector->n...

This is the latest version of https://patchwork.freedesktop.org/series/46815/ With a bunch of fixes to the new fb_helper to prevent it from breaking module loading/unloading with nouveau. Also; lots of documentation fixes and one fix in response to a kbuild bot. Lyude Paul (8): drm/nouveau: Fix bogus drm_kms_helper_poll_enable() placement drm/nouveau: Enable polling even if we have runtime

From: Ville Syrjälä <ville.syrjala at linux.intel.com> Use drm_for_each_connector_encoder_ids() for iterating connector->encoder_ids[]. A bit more convenient not having to deal with the implementation details. Cc: Ben Skeggs <bskeggs at redhat.com> Cc: nouveau at lists.freedesktop.org Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com> ---

Next version of https://patchwork.freedesktop.org/series/46815/ Same as previous version, but some small changes made to commit messages and acks/rbs have been added Lyude Paul (5): drm/nouveau: Fix bogus drm_kms_helper_poll_enable() placement drm/nouveau: Remove duplicate poll_enable() in pmops_runtime_suspend() drm/nouveau: Fix deadlock with fb_helper with async RPM requests

Latest version of https://patchwork.freedesktop.org/series/46815/ with some significant improvements: - I finally figured out a clean way to do this entirely with runtime PM helpers, no avoiding grabbing refs required! - Since this new method removes the need for a lot of the other changes I made (although we probably still want those changes, but not for fixing these deadlocks)

...ini(struct drm_device *dev); > -bool drm_helper_hpd_irq_event(struct drm_device *dev); > -void drm_kms_helper_hotplug_event(struct drm_device *dev); > - > -void drm_kms_helper_poll_disable(struct drm_device *dev); > -void drm_kms_helper_poll_enable(struct drm_device *dev); > -bool drm_kms_helper_is_poll_worker(void); > - > #endif > diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h > new file mode 100644 > index 000000000000..96c060c16a1e > --- /dev/null > +++ b/include/drm/drm_probe_helper.h > @@ -0,0 +1,50 @@ > +/* > + * Copyright © 2006 Keith Pa...

...ev); >> -bool drm_helper_hpd_irq_event(struct drm_device *dev); >> -void drm_kms_helper_hotplug_event(struct drm_device *dev); >> - >> -void drm_kms_helper_poll_disable(struct drm_device *dev); >> -void drm_kms_helper_poll_enable(struct drm_device *dev); >> -bool drm_kms_helper_is_poll_worker(void); >> - >> #endif >> diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h >> new file mode 100644 >> index 000000000000..96c060c16a1e >> --- /dev/null >> +++ b/include/drm/drm_probe_helper.h >> @@ -0,0 +1,50 @@ >> +...