Commit 18dddadc authored by Daniel Vetter's avatar Daniel Vetter

drm/atomic: Introduce drm_atomic_helper_shutdown

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).

v3: Correct kerneldoc to point at _disable_all().
Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Acked-by: default avatarNoralf Trønnes <noralf@tronnes.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Ben Skeggs <bskeggs@redhat.com>
Tested-by: default avatarTomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170321164149.31531-1-daniel.vetter@ffwll.ch
parent 79b85d2b
...@@ -2443,7 +2443,8 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, ...@@ -2443,7 +2443,8 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
* that they are connected to. * that they are connected to.
* *
* This is used for example in suspend/resume to disable all currently active * 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 * Note that if callers haven't already acquired all modeset locks this might
* return -EDEADLK, which must be handled by calling drm_modeset_backoff(). * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
...@@ -2452,7 +2453,8 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, ...@@ -2452,7 +2453,8 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
* 0 on success or a negative error code on failure. * 0 on success or a negative error code on failure.
* *
* See also: * 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, int drm_atomic_helper_disable_all(struct drm_device *dev,
struct drm_modeset_acquire_ctx *ctx) struct drm_modeset_acquire_ctx *ctx)
...@@ -2516,6 +2518,42 @@ int drm_atomic_helper_disable_all(struct drm_device *dev, ...@@ -2516,6 +2518,42 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
EXPORT_SYMBOL(drm_atomic_helper_disable_all); 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_all().
*/
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);
/** /**
* drm_atomic_helper_suspend - subsystem-level suspend helper * drm_atomic_helper_suspend - subsystem-level suspend helper
* @dev: DRM device * @dev: DRM device
......
...@@ -94,6 +94,8 @@ EXPORT_SYMBOL(drm_crtc_from_index); ...@@ -94,6 +94,8 @@ EXPORT_SYMBOL(drm_crtc_from_index);
* drm_crtc_force_disable - Forcibly turn off a CRTC * drm_crtc_force_disable - Forcibly turn off a CRTC
* @crtc: CRTC to turn off * @crtc: CRTC to turn off
* *
* Note: This should only be used by non-atomic legacy drivers.
*
* Returns: * Returns:
* Zero on success, error code on failure. * Zero on success, error code on failure.
*/ */
...@@ -103,6 +105,8 @@ int drm_crtc_force_disable(struct drm_crtc *crtc) ...@@ -103,6 +105,8 @@ int drm_crtc_force_disable(struct drm_crtc *crtc)
.crtc = crtc, .crtc = crtc,
}; };
WARN_ON(drm_drv_uses_atomic_modeset(crtc->dev));
return drm_mode_set_config_internal(&set); return drm_mode_set_config_internal(&set);
} }
EXPORT_SYMBOL(drm_crtc_force_disable); EXPORT_SYMBOL(drm_crtc_force_disable);
...@@ -114,6 +118,9 @@ EXPORT_SYMBOL(drm_crtc_force_disable); ...@@ -114,6 +118,9 @@ EXPORT_SYMBOL(drm_crtc_force_disable);
* Drivers may want to call this on unload to ensure that all displays are * Drivers may want to call this on unload to ensure that all displays are
* unlit and the GPU is in a consistent, low power state. Takes modeset locks. * unlit and the GPU is in a consistent, low power state. Takes modeset locks.
* *
* Note: This should only be used by non-atomic legacy drivers. For an atomic
* version look at drm_atomic_helper_shutdown().
*
* Returns: * Returns:
* Zero on success, error code on failure. * Zero on success, error code on failure.
*/ */
......
...@@ -1307,8 +1307,6 @@ void i915_driver_unload(struct drm_device *dev) ...@@ -1307,8 +1307,6 @@ void i915_driver_unload(struct drm_device *dev)
{ {
struct drm_i915_private *dev_priv = to_i915(dev); struct drm_i915_private *dev_priv = to_i915(dev);
struct pci_dev *pdev = dev_priv->drm.pdev; struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_modeset_acquire_ctx ctx;
int ret;
intel_fbdev_fini(dev); intel_fbdev_fini(dev);
...@@ -1317,23 +1315,7 @@ void i915_driver_unload(struct drm_device *dev) ...@@ -1317,23 +1315,7 @@ void i915_driver_unload(struct drm_device *dev)
intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
drm_modeset_acquire_init(&ctx, 0); drm_atomic_helper_shutdown(dev);
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);
intel_gvt_cleanup(dev_priv); intel_gvt_cleanup(dev_priv);
......
...@@ -436,8 +436,12 @@ nouveau_display_fini(struct drm_device *dev, bool suspend) ...@@ -436,8 +436,12 @@ nouveau_display_fini(struct drm_device *dev, bool suspend)
struct drm_connector *connector; struct drm_connector *connector;
struct drm_crtc *crtc; struct drm_crtc *crtc;
if (!suspend) if (!suspend) {
if (drm_drv_uses_atomic_modeset(dev))
drm_atomic_helper_shutdown(dev);
else
drm_crtc_force_disable_all(dev); drm_crtc_force_disable_all(dev);
}
/* Make sure that drm and hw vblank irqs get properly disabled. */ /* Make sure that drm and hw vblank irqs get properly disabled. */
drm_for_each_crtc(crtc, dev) drm_for_each_crtc(crtc, dev)
......
...@@ -236,7 +236,7 @@ static void tinydrm_unregister(struct tinydrm_device *tdev) ...@@ -236,7 +236,7 @@ static void tinydrm_unregister(struct tinydrm_device *tdev)
{ {
struct drm_fbdev_cma *fbdev_cma = tdev->fbdev_cma; struct drm_fbdev_cma *fbdev_cma = tdev->fbdev_cma;
drm_crtc_force_disable_all(tdev->drm); drm_atomic_helper_shutdown(tdev->drm);
/* don't restore fbdev in lastclose, keep pipeline disabled */ /* don't restore fbdev in lastclose, keep pipeline disabled */
tdev->fbdev_cma = NULL; tdev->fbdev_cma = NULL;
drm_dev_unregister(tdev->drm); drm_dev_unregister(tdev->drm);
...@@ -287,7 +287,7 @@ EXPORT_SYMBOL(devm_tinydrm_register); ...@@ -287,7 +287,7 @@ EXPORT_SYMBOL(devm_tinydrm_register);
*/ */
void tinydrm_shutdown(struct tinydrm_device *tdev) void tinydrm_shutdown(struct tinydrm_device *tdev)
{ {
drm_crtc_force_disable_all(tdev->drm); drm_atomic_helper_shutdown(tdev->drm);
} }
EXPORT_SYMBOL(tinydrm_shutdown); EXPORT_SYMBOL(tinydrm_shutdown);
......
...@@ -104,6 +104,7 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, ...@@ -104,6 +104,7 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
int drm_atomic_helper_disable_all(struct drm_device *dev, int drm_atomic_helper_disable_all(struct drm_device *dev,
struct drm_modeset_acquire_ctx *ctx); struct drm_modeset_acquire_ctx *ctx);
void drm_atomic_helper_shutdown(struct drm_device *dev);
struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev); struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev);
int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state, int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
struct drm_modeset_acquire_ctx *ctx); struct drm_modeset_acquire_ctx *ctx);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment