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>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
