Op 21-03-17 om 16:26 schreef Daniel Vetter: > The trouble here is that it does multiple atomic commits under one > drm_modeset_lock_all, which breaks the behind-the-scenes acquire > context magic that function pulls off. It's much better to have one > overall atomic commit. That we still have multiple atomic commits > prevents us from adding some pretty useful debug checks to the atomic > machinery. > > Hence it is really a bad idea to call the legacy > drm_crtc_force_disable_all() function. There's 2 atomic drivers using > this still, nouveau and tinydrm. To fix this, introduce a new > drm_atomic_helper_shutdown() by extracting the code from i915. > > While at it improve kernel-doc and catch future offenders by > sprinkling a WARN_ON into the legacy function. We should probably move > those into the legacy modeset helpers, too ... > > v2: Make it compile on arm drivers too (Noralf). > > Acked-by: Noralf Trønnes <noralf@xxxxxxxxxxx> > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Cc: Noralf Trønnes <noralf@xxxxxxxxxxx> > Cc: Ben Skeggs <bskeggs@xxxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > --- > drivers/gpu/drm/drm_atomic_helper.c | 42 +++++++++++++++++++++++++++-- > drivers/gpu/drm/drm_crtc.c | 7 +++++ > drivers/gpu/drm/i915/i915_drv.c | 20 +------------- > drivers/gpu/drm/nouveau/nouveau_display.c | 8 ++++-- > drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 4 +-- > include/drm/drm_atomic_helper.h | 1 + > 6 files changed, 57 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index b4dcb2559e09..5f401519d03c 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2444,7 +2444,8 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, > * that they are connected to. > * > * This is used for example in suspend/resume to disable all currently active > - * functions when suspending. > + * functions when suspending. If you just want to shut down everything at e.g. > + * driver unload, look at drm_atomic_helper_shutdown(). > * > * Note that if callers haven't already acquired all modeset locks this might > * return -EDEADLK, which must be handled by calling drm_modeset_backoff(). > @@ -2453,7 +2454,8 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, > * 0 on success or a negative error code on failure. > * > * See also: > - * drm_atomic_helper_suspend(), drm_atomic_helper_resume() > + * drm_atomic_helper_suspend(), drm_atomic_helper_resume() and > + * drm_atomic_helper_shutdown(). > */ > int drm_atomic_helper_disable_all(struct drm_device *dev, > struct drm_modeset_acquire_ctx *ctx) > @@ -2518,6 +2520,42 @@ int drm_atomic_helper_disable_all(struct drm_device *dev, > EXPORT_SYMBOL(drm_atomic_helper_disable_all); > > /** > + * drm_atomic_helper_shutdown - shutdown all CRTC > + * @dev: DRM device > + * > + * This shuts down all CRTC, which is useful for driver unloading. Shutdown on > + * suspend should instead be handled with drm_atomic_helper_suspend(), since > + * that also takes a snapshot of the modeset state to be restored on resume. > + * > + * This is just a convenience wrapper around drm_atomic_helper_disable_all(), > + * and it is the atomic version of drm_crtc_force_disable(). > + */ > +void drm_atomic_helper_shutdown(struct drm_device *dev) > +{ > + struct drm_modeset_acquire_ctx ctx; > + int ret; > + > + drm_modeset_acquire_init(&ctx, 0); > + while (1) { > + ret = drm_modeset_lock_all_ctx(dev, &ctx); > + if (!ret) > + ret = drm_atomic_helper_disable_all(dev, &ctx); > + > + if (ret != -EDEADLK) > + break; > + > + drm_modeset_backoff(&ctx); > + } > + > + if (ret) > + DRM_ERROR("Disabling all crtc's during unload failed with %i\n", ret); > + > + drm_modeset_drop_locks(&ctx); > + drm_modeset_acquire_fini(&ctx); > +} > +EXPORT_SYMBOL(drm_atomic_helper_shutdown); Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel