[PATCH v5 1/3] drm: Implement drm_modeset_lock_all_ctx()

[PATCH v5 1/3] drm: Implement drm_modeset_lock_all_ctx()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This function is like drm_modeset_lock_all(), but it takes the lock
acquisition context as a parameter rather than storing it in the DRM
device's mode_config structure.

Implement drm_modeset_{,un}lock_all() in terms of the new function for
better code reuse, and add a note to the kerneldoc that new code should
use the new functions.

v2: improve kerneldoc
v4: rename drm_modeset_lock_all_crtcs() to drm_modeset_lock_all_ctx()
    and take mode_config's .connection_mutex instead of .mutex lock to
    avoid lock inversion (Daniel Vetter), use drm_modeset_drop_locks()
    which is now the equivalent of drm_modeset_unlock_all_ctx()
v5: do not take the dev->mode_config.connection_mutex in
    drm_atomic_legacy_backoff() since drm_modeset_lock_all_ctx()
    already keeps it, enhance kerneldoc for drm_modeset_lock_all_ctx()
    (Daniel Vetter)

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_atomic.c       |  7 +--
 drivers/gpu/drm/drm_modeset_lock.c | 89 +++++++++++++++++++++++++-------------
 include/drm/drm_modeset_lock.h     |  4 +-
 3 files changed, 63 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 55b4debad79b..ef5f7663a718 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1188,12 +1188,7 @@ void drm_atomic_legacy_backoff(struct drm_atomic_state *state)
 retry:
 	drm_modeset_backoff(state->acquire_ctx);
 
-	ret = drm_modeset_lock(&state->dev->mode_config.connection_mutex,
-			       state->acquire_ctx);
-	if (ret)
-		goto retry;
-	ret = drm_modeset_lock_all_crtcs(state->dev,
-					 state->acquire_ctx);
+	ret = drm_modeset_lock_all_ctx(state->dev, state->acquire_ctx);
 	if (ret)
 		goto retry;
 }
diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 6675b1428410..c2f5971146ba 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -57,11 +57,18 @@
 
 /**
  * drm_modeset_lock_all - take all modeset locks
- * @dev: drm device
+ * @dev: DRM device
  *
  * This function takes all modeset locks, suitable where a more fine-grained
- * scheme isn't (yet) implemented. Locks must be dropped with
- * drm_modeset_unlock_all.
+ * scheme isn't (yet) implemented. Locks must be dropped by calling the
+ * drm_modeset_unlock_all() function.
+ *
+ * This function is deprecated. It allocates a lock acquisition context and
+ * stores it in the DRM device's ->mode_config. This facilitate conversion of
+ * existing code because it removes the need to manually deal with the
+ * acquisition context, but it is also brittle because the context is global
+ * and care must be taken not to nest calls. New code should use the
+ * drm_modeset_lock_all_ctx() function and pass in the context explicitly.
  */
 void drm_modeset_lock_all(struct drm_device *dev)
 {
@@ -78,39 +85,43 @@ void drm_modeset_lock_all(struct drm_device *dev)
 	drm_modeset_acquire_init(ctx, 0);
 
 retry:
-	ret = drm_modeset_lock(&config->connection_mutex, ctx);
-	if (ret)
-		goto fail;
-	ret = drm_modeset_lock_all_crtcs(dev, ctx);
-	if (ret)
-		goto fail;
+	ret = drm_modeset_lock_all_ctx(dev, ctx);
+	if (ret < 0) {
+		if (ret == -EDEADLK) {
+			drm_modeset_backoff(ctx);
+			goto retry;
+		}
+
+		drm_modeset_acquire_fini(ctx);
+		kfree(ctx);
+		return;
+	}
 
 	WARN_ON(config->acquire_ctx);
 
-	/* now we hold the locks, so now that it is safe, stash the
-	 * ctx for drm_modeset_unlock_all():
+	/*
+	 * We hold the locks now, so it is safe to stash the acquisition
+	 * context for drm_modeset_unlock_all().
 	 */
 	config->acquire_ctx = ctx;
 
 	drm_warn_on_modeset_not_all_locked(dev);
-
-	return;
-
-fail:
-	if (ret == -EDEADLK) {
-		drm_modeset_backoff(ctx);
-		goto retry;
-	}
-
-	kfree(ctx);
 }
 EXPORT_SYMBOL(drm_modeset_lock_all);
 
 /**
  * drm_modeset_unlock_all - drop all modeset locks
- * @dev: device
+ * @dev: DRM device
  *
- * This function drop all modeset locks taken by drm_modeset_lock_all.
+ * This function drops all modeset locks taken by a previous call to the
+ * drm_modeset_lock_all() function.
+ *
+ * This function is deprecated. It uses the lock acquisition context stored
+ * in the DRM device's ->mode_config. This facilitates conversion of existing
+ * code because it removes the need to manually deal with the acquisition
+ * context, but it is also brittle because the context is global and care must
+ * be taken not to nest calls. New code should pass the acquisition context
+ * directly to the drm_modeset_drop_locks() function.
  */
 void drm_modeset_unlock_all(struct drm_device *dev)
 {
@@ -431,14 +442,34 @@ void drm_modeset_unlock(struct drm_modeset_lock *lock)
 }
 EXPORT_SYMBOL(drm_modeset_unlock);
 
-/* In some legacy codepaths it's convenient to just grab all the crtc and plane
- * related locks. */
-int drm_modeset_lock_all_crtcs(struct drm_device *dev,
-		struct drm_modeset_acquire_ctx *ctx)
+/**
+ * drm_modeset_lock_all_ctx - take all modeset locks
+ * @dev: DRM device
+ * @ctx: lock acquisition context
+ *
+ * This function takes all modeset locks, suitable where a more fine-grained
+ * scheme isn't (yet) implemented.
+ *
+ * Unlike drm_modeset_lock_all(), it doesn't take the dev->mode_config.mutex
+ * since that lock isn't required for modeset state changes. Callers which
+ * need to grab that lock too need to do so outside of the acquire context
+ * @ctx.
+ *
+ * Locks acquired with this function should be released by calling the
+ * drm_modeset_drop_locks() function on @ctx.
+ *
+ * Returns: 0 on success or a negative error-code on failure.
+ */
+int drm_modeset_lock_all_ctx(struct drm_device *dev,
+			     struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_crtc *crtc;
 	struct drm_plane *plane;
-	int ret = 0;
+	int ret;
+
+	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
+	if (ret)
+		return ret;
 
 	drm_for_each_crtc(crtc, dev) {
 		ret = drm_modeset_lock(&crtc->mutex, ctx);
@@ -454,4 +485,4 @@ int drm_modeset_lock_all_crtcs(struct drm_device *dev,
 
 	return 0;
 }
-EXPORT_SYMBOL(drm_modeset_lock_all_crtcs);
+EXPORT_SYMBOL(drm_modeset_lock_all_ctx);
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 94938d89347c..c5576fbcb909 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -138,7 +138,7 @@ void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
 struct drm_modeset_acquire_ctx *
 drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc);
 
-int drm_modeset_lock_all_crtcs(struct drm_device *dev,
-		struct drm_modeset_acquire_ctx *ctx);
+int drm_modeset_lock_all_ctx(struct drm_device *dev,
+			     struct drm_modeset_acquire_ctx *ctx);
 
 #endif /* DRM_MODESET_LOCK_H_ */
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 2/3] drm/atomic-helper: Implement subsystem-level suspend/resume

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Provide subsystem-level suspend and resume helpers that can be used to
implement suspend/resume on atomic mode-setting enabled drivers.

v2: simplify locking, enhance kerneldoc comments
v3: pass lock acquisition context by parameter, improve kerneldoc
v4: - remove redundant code (already provided by atomic helpers)
      (Maarten Lankhorst)
    - move backoff dance from drm_modeset_lock_all_ctx() into suspend
      helper (Daniel Vetter)
v5: handle potential EDEADLK from drm_atomic_helper_duplicate_state()
    and drm_atomic_helper_disable_all() (Daniel Vetter)

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 162 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_crtc_helper.c   |   6 ++
 include/drm/drm_atomic_helper.h     |   6 ++
 3 files changed, 173 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 3731a26979bc..ce2cfef08742 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1818,6 +1818,161 @@ commit:
 }
 
 /**
+ * drm_atomic_helper_disable_all - disable all currently active outputs
+ * @dev: DRM device
+ * @ctx: lock acquisition context
+ *
+ * Loops through all connectors, finding those that aren't turned off and then
+ * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
+ * that they are connected to.
+ *
+ * This is used for example in suspend/resume to disable all currently active
+ * functions when suspending.
+ *
+ * Note that if callers haven't already acquired all modeset locks this might
+ * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ *
+ * See also:
+ * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
+ */
+int drm_atomic_helper_disable_all(struct drm_device *dev,
+				  struct drm_modeset_acquire_ctx *ctx)
+{
+	struct drm_atomic_state *state;
+	struct drm_connector *conn;
+	int err;
+
+	state = drm_atomic_state_alloc(dev);
+	if (!state)
+		return -ENOMEM;
+
+	state->acquire_ctx = ctx;
+
+	drm_for_each_connector(conn, dev) {
+		struct drm_crtc *crtc = conn->state->crtc;
+		struct drm_crtc_state *crtc_state;
+
+		if (!crtc || conn->dpms != DRM_MODE_DPMS_ON)
+			continue;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state)) {
+			err = PTR_ERR(crtc_state);
+			goto free;
+		}
+
+		crtc_state->active = false;
+	}
+
+	err = drm_atomic_commit(state);
+
+free:
+	if (err < 0)
+		drm_atomic_state_free(state);
+
+	return err;
+}
+EXPORT_SYMBOL(drm_atomic_helper_disable_all);
+
+/**
+ * drm_atomic_helper_suspend - subsystem-level suspend helper
+ * @dev: DRM device
+ *
+ * Duplicates the current atomic state, disables all active outputs and then
+ * returns a pointer to the original atomic state to the caller. Drivers can
+ * pass this pointer to the drm_atomic_helper_resume() helper upon resume to
+ * restore the output configuration that was active at the time the system
+ * entered suspend.
+ *
+ * Note that it is potentially unsafe to use this. The atomic state object
+ * returned by this function is assumed to be persistent. Drivers must ensure
+ * that this holds true. Before calling this function, drivers must make sure
+ * to suspend fbdev emulation so that nothing can be using the device.
+ *
+ * Returns:
+ * A pointer to a copy of the state before suspend on success or an ERR_PTR()-
+ * encoded error code on failure. Drivers should store the returned atomic
+ * state object and pass it to the drm_atomic_helper_resume() helper upon
+ * resume.
+ *
+ * See also:
+ * drm_atomic_helper_duplicate_state(), drm_atomic_helper_disable_all(),
+ * drm_atomic_helper_resume()
+ */
+struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)
+{
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_atomic_state *state;
+	int err;
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+retry:
+	err = drm_modeset_lock_all_ctx(dev, &ctx);
+	if (err < 0) {
+		state = ERR_PTR(err);
+		goto unlock;
+	}
+
+	state = drm_atomic_helper_duplicate_state(dev, &ctx);
+	if (IS_ERR(state))
+		goto unlock;
+
+	err = drm_atomic_helper_disable_all(dev, &ctx);
+	if (err < 0) {
+		drm_atomic_state_free(state);
+		state = ERR_PTR(err);
+		goto unlock;
+	}
+
+unlock:
+	if (PTR_ERR(state) == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+	return state;
+}
+EXPORT_SYMBOL(drm_atomic_helper_suspend);
+
+/**
+ * drm_atomic_helper_resume - subsystem-level resume helper
+ * @dev: DRM device
+ * @state: atomic state to resume to
+ *
+ * Calls drm_mode_config_reset() to synchronize hardware and software states,
+ * grabs all modeset locks and commits the atomic state object. This can be
+ * used in conjunction with the drm_atomic_helper_suspend() helper to
+ * implement suspend/resume for drivers that support atomic mode-setting.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ *
+ * See also:
+ * drm_atomic_helper_suspend()
+ */
+int drm_atomic_helper_resume(struct drm_device *dev,
+			     struct drm_atomic_state *state)
+{
+	struct drm_mode_config *config = &dev->mode_config;
+	int err;
+
+	drm_mode_config_reset(dev);
+	drm_modeset_lock_all(dev);
+	state->acquire_ctx = config->acquire_ctx;
+	err = drm_atomic_commit(state);
+	drm_modeset_unlock_all(dev);
+
+	return err;
+}
+EXPORT_SYMBOL(drm_atomic_helper_resume);
+
+/**
  * drm_atomic_helper_crtc_set_property - helper for crtc properties
  * @crtc: DRM crtc
  * @property: DRM property
@@ -2429,7 +2584,9 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_duplicate_state);
  * @ctx: lock acquisition context
  *
  * Makes a copy of the current atomic state by looping over all objects and
- * duplicating their respective states.
+ * duplicating their respective states. This is used for example by suspend/
+ * resume support code to save the state prior to suspend such that it can
+ * be restored upon resume.
  *
  * Note that this treats atomic state as persistent between save and restore.
  * Drivers must make sure that this is possible and won't result in confusion
@@ -2441,6 +2598,9 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_duplicate_state);
  * Returns:
  * A pointer to the copy of the atomic state object on success or an
  * ERR_PTR()-encoded error code on failure.
+ *
+ * See also:
+ * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
  */
 struct drm_atomic_state *
 drm_atomic_helper_duplicate_state(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 6b4cf25fed12..10d0989db273 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -855,6 +855,12 @@ EXPORT_SYMBOL(drm_helper_mode_fill_fb_struct);
  * due to slight differences in allocating shared resources when the
  * configuration is restored in a different order than when userspace set it up)
  * need to use their own restore logic.
+ *
+ * This function is deprecated. New drivers should implement atomic mode-
+ * setting and use the atomic suspend/resume helpers.
+ *
+ * See also:
+ * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
  */
 void drm_helper_resume_force_mode(struct drm_device *dev)
 {
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 8cba54a2a0a0..8045cdea8cc9 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -81,6 +81,12 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set);
 int __drm_atomic_helper_set_config(struct drm_mode_set *set,
 		struct drm_atomic_state *state);
 
+int drm_atomic_helper_disable_all(struct drm_device *dev,
+				  struct drm_modeset_acquire_ctx *ctx);
+struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev);
+int drm_atomic_helper_resume(struct drm_device *dev,
+			     struct drm_atomic_state *state);
+
 int drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc,
 					struct drm_property *property,
 					uint64_t val);
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 3/3] drm/tegra: Implement subsystem-level suspend/resume

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Use the drm_atomic_helper_suspend() and drm_atomic_helper_resume()
helpers to implement subsystem-level suspend/resume.

v2: suspend framebuffer device to avoid concurrency issues
v3: resume fbdev on failure to suspend (Emil Velikov)

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/drm.c | 12 ++++++++++++
 drivers/gpu/drm/tegra/drm.h |  4 ++++
 drivers/gpu/drm/tegra/fb.c  | 24 ++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index c8a5dce4e46e..639a1041e178 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1021,8 +1021,17 @@ static int host1x_drm_remove(struct host1x_device *dev)
 static int host1x_drm_suspend(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
+	struct tegra_drm *tegra = drm->dev_private;
 
 	drm_kms_helper_poll_disable(drm);
+	tegra_drm_fb_suspend(drm);
+
+	tegra->state = drm_atomic_helper_suspend(drm);
+	if (IS_ERR(tegra->state)) {
+		tegra_drm_fb_resume(drm);
+		drm_kms_helper_poll_enable(drm);
+		return PTR_ERR(tegra->state);
+	}
 
 	return 0;
 }
@@ -1030,7 +1039,10 @@ static int host1x_drm_suspend(struct device *dev)
 static int host1x_drm_resume(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
+	struct tegra_drm *tegra = drm->dev_private;
 
+	drm_atomic_helper_resume(drm, tegra->state);
+	tegra_drm_fb_resume(drm);
 	drm_kms_helper_poll_enable(drm);
 
 	return 0;
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 69566b616f88..dd9bb4816d0b 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -57,6 +57,8 @@ struct tegra_drm {
 		struct work_struct work;
 		struct mutex lock;
 	} commit;
+
+	struct drm_atomic_state *state;
 };
 
 struct tegra_drm_client;
@@ -265,6 +267,8 @@ int tegra_drm_fb_prepare(struct drm_device *drm);
 void tegra_drm_fb_free(struct drm_device *drm);
 int tegra_drm_fb_init(struct drm_device *drm);
 void tegra_drm_fb_exit(struct drm_device *drm);
+void tegra_drm_fb_suspend(struct drm_device *drm);
+void tegra_drm_fb_resume(struct drm_device *drm);
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev);
 void tegra_fb_output_poll_changed(struct drm_device *drm);
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index ede9e94f3312..a7213f30fb20 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -10,6 +10,8 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/console.h>
+
 #include "drm.h"
 #include "gem.h"
 
@@ -413,3 +415,25 @@ void tegra_drm_fb_exit(struct drm_device *drm)
 	tegra_fbdev_exit(tegra->fbdev);
 #endif
 }
+
+void tegra_drm_fb_suspend(struct drm_device *drm)
+{
+#ifdef CONFIG_DRM_FBDEV_EMULATION
+	struct tegra_drm *tegra = drm->dev_private;
+
+	console_lock();
+	drm_fb_helper_set_suspend(&tegra->fbdev->base, 1);
+	console_unlock();
+#endif
+}
+
+void tegra_drm_fb_resume(struct drm_device *drm)
+{
+#ifdef CONFIG_DRM_FBDEV_EMULATION
+	struct tegra_drm *tegra = drm->dev_private;
+
+	console_lock();
+	drm_fb_helper_set_suspend(&tegra->fbdev->base, 0);
+	console_unlock();
+#endif
+}
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 2/3] drm/atomic-helper: Implement subsystem-level suspend/resume

[Daniel Vetter]

On Wed, Dec 02, 2015 at 05:50:04PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Provide subsystem-level suspend and resume helpers that can be used to
> implement suspend/resume on atomic mode-setting enabled drivers.
> 
> v2: simplify locking, enhance kerneldoc comments
> v3: pass lock acquisition context by parameter, improve kerneldoc
> v4: - remove redundant code (already provided by atomic helpers)
>       (Maarten Lankhorst)
>     - move backoff dance from drm_modeset_lock_all_ctx() into suspend
>       helper (Daniel Vetter)
> v5: handle potential EDEADLK from drm_atomic_helper_duplicate_state()
>     and drm_atomic_helper_disable_all() (Daniel Vetter)
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Patches 1&2 applied to drm-misc, thanks a lot. If you want I can also
apply the tegra one there, or you'll just wait until this lands in
drm-next and rebase - I plan to send another drm-misc pull tomorrow anyway
so I can get at these goodies for i915.

Cheers, Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 162 +++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/drm_crtc_helper.c   |   6 ++
>  include/drm/drm_atomic_helper.h     |   6 ++
>  3 files changed, 173 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 3731a26979bc..ce2cfef08742 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1818,6 +1818,161 @@ commit:
>  }
>  
>  /**
> + * drm_atomic_helper_disable_all - disable all currently active outputs
> + * @dev: DRM device
> + * @ctx: lock acquisition context
> + *
> + * Loops through all connectors, finding those that aren't turned off and then
> + * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
> + * that they are connected to.
> + *
> + * This is used for example in suspend/resume to disable all currently active
> + * functions when suspending.
> + *
> + * Note that if callers haven't already acquired all modeset locks this might
> + * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + *
> + * See also:
> + * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
> + */
> +int drm_atomic_helper_disable_all(struct drm_device *dev,
> +				  struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_atomic_state *state;
> +	struct drm_connector *conn;
> +	int err;
> +
> +	state = drm_atomic_state_alloc(dev);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	state->acquire_ctx = ctx;
> +
> +	drm_for_each_connector(conn, dev) {
> +		struct drm_crtc *crtc = conn->state->crtc;
> +		struct drm_crtc_state *crtc_state;
> +
> +		if (!crtc || conn->dpms != DRM_MODE_DPMS_ON)
> +			continue;
> +
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		if (IS_ERR(crtc_state)) {
> +			err = PTR_ERR(crtc_state);
> +			goto free;
> +		}
> +
> +		crtc_state->active = false;
> +	}
> +
> +	err = drm_atomic_commit(state);
> +
> +free:
> +	if (err < 0)
> +		drm_atomic_state_free(state);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_disable_all);
> +
> +/**
> + * drm_atomic_helper_suspend - subsystem-level suspend helper
> + * @dev: DRM device
> + *
> + * Duplicates the current atomic state, disables all active outputs and then
> + * returns a pointer to the original atomic state to the caller. Drivers can
> + * pass this pointer to the drm_atomic_helper_resume() helper upon resume to
> + * restore the output configuration that was active at the time the system
> + * entered suspend.
> + *
> + * Note that it is potentially unsafe to use this. The atomic state object
> + * returned by this function is assumed to be persistent. Drivers must ensure
> + * that this holds true. Before calling this function, drivers must make sure
> + * to suspend fbdev emulation so that nothing can be using the device.
> + *
> + * Returns:
> + * A pointer to a copy of the state before suspend on success or an ERR_PTR()-
> + * encoded error code on failure. Drivers should store the returned atomic
> + * state object and pass it to the drm_atomic_helper_resume() helper upon
> + * resume.
> + *
> + * See also:
> + * drm_atomic_helper_duplicate_state(), drm_atomic_helper_disable_all(),
> + * drm_atomic_helper_resume()
> + */
> +struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)
> +{
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_state *state;
> +	int err;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +retry:
> +	err = drm_modeset_lock_all_ctx(dev, &ctx);
> +	if (err < 0) {
> +		state = ERR_PTR(err);
> +		goto unlock;
> +	}
> +
> +	state = drm_atomic_helper_duplicate_state(dev, &ctx);
> +	if (IS_ERR(state))
> +		goto unlock;
> +
> +	err = drm_atomic_helper_disable_all(dev, &ctx);
> +	if (err < 0) {
> +		drm_atomic_state_free(state);
> +		state = ERR_PTR(err);
> +		goto unlock;
> +	}
> +
> +unlock:
> +	if (PTR_ERR(state) == -EDEADLK) {
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +	return state;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_suspend);
> +
> +/**
> + * drm_atomic_helper_resume - subsystem-level resume helper
> + * @dev: DRM device
> + * @state: atomic state to resume to
> + *
> + * Calls drm_mode_config_reset() to synchronize hardware and software states,
> + * grabs all modeset locks and commits the atomic state object. This can be
> + * used in conjunction with the drm_atomic_helper_suspend() helper to
> + * implement suspend/resume for drivers that support atomic mode-setting.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + *
> + * See also:
> + * drm_atomic_helper_suspend()
> + */
> +int drm_atomic_helper_resume(struct drm_device *dev,
> +			     struct drm_atomic_state *state)
> +{
> +	struct drm_mode_config *config = &dev->mode_config;
> +	int err;
> +
> +	drm_mode_config_reset(dev);
> +	drm_modeset_lock_all(dev);
> +	state->acquire_ctx = config->acquire_ctx;
> +	err = drm_atomic_commit(state);
> +	drm_modeset_unlock_all(dev);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_resume);
> +
> +/**
>   * drm_atomic_helper_crtc_set_property - helper for crtc properties
>   * @crtc: DRM crtc
>   * @property: DRM property
> @@ -2429,7 +2584,9 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_duplicate_state);
>   * @ctx: lock acquisition context
>   *
>   * Makes a copy of the current atomic state by looping over all objects and
> - * duplicating their respective states.
> + * duplicating their respective states. This is used for example by suspend/
> + * resume support code to save the state prior to suspend such that it can
> + * be restored upon resume.
>   *
>   * Note that this treats atomic state as persistent between save and restore.
>   * Drivers must make sure that this is possible and won't result in confusion
> @@ -2441,6 +2598,9 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_duplicate_state);
>   * Returns:
>   * A pointer to the copy of the atomic state object on success or an
>   * ERR_PTR()-encoded error code on failure.
> + *
> + * See also:
> + * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
>   */
>  struct drm_atomic_state *
>  drm_atomic_helper_duplicate_state(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 6b4cf25fed12..10d0989db273 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -855,6 +855,12 @@ EXPORT_SYMBOL(drm_helper_mode_fill_fb_struct);
>   * due to slight differences in allocating shared resources when the
>   * configuration is restored in a different order than when userspace set it up)
>   * need to use their own restore logic.
> + *
> + * This function is deprecated. New drivers should implement atomic mode-
> + * setting and use the atomic suspend/resume helpers.
> + *
> + * See also:
> + * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
>   */
>  void drm_helper_resume_force_mode(struct drm_device *dev)
>  {
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 8cba54a2a0a0..8045cdea8cc9 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -81,6 +81,12 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set);
>  int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>  		struct drm_atomic_state *state);
>  
> +int drm_atomic_helper_disable_all(struct drm_device *dev,
> +				  struct drm_modeset_acquire_ctx *ctx);
> +struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev);
> +int drm_atomic_helper_resume(struct drm_device *dev,
> +			     struct drm_atomic_state *state);
> +
>  int drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc,
>  					struct drm_property *property,
>  					uint64_t val);
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 2/3] drm/atomic-helper: Implement subsystem-level suspend/resume

[Matt Roper]

On Wed, Dec 02, 2015 at 05:50:04PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Provide subsystem-level suspend and resume helpers that can be used to
> implement suspend/resume on atomic mode-setting enabled drivers.
> 
> v2: simplify locking, enhance kerneldoc comments
> v3: pass lock acquisition context by parameter, improve kerneldoc
> v4: - remove redundant code (already provided by atomic helpers)
>       (Maarten Lankhorst)
>     - move backoff dance from drm_modeset_lock_all_ctx() into suspend
>       helper (Daniel Vetter)
> v5: handle potential EDEADLK from drm_atomic_helper_duplicate_state()
>     and drm_atomic_helper_disable_all() (Daniel Vetter)
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 162 +++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/drm_crtc_helper.c   |   6 ++
>  include/drm/drm_atomic_helper.h     |   6 ++
>  3 files changed, 173 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 3731a26979bc..ce2cfef08742 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1818,6 +1818,161 @@ commit:
>  }
>  
>  /**
> + * drm_atomic_helper_disable_all - disable all currently active outputs
> + * @dev: DRM device
> + * @ctx: lock acquisition context
> + *
> + * Loops through all connectors, finding those that aren't turned off and then
> + * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
> + * that they are connected to.
> + *
> + * This is used for example in suspend/resume to disable all currently active
> + * functions when suspending.
> + *
> + * Note that if callers haven't already acquired all modeset locks this might
> + * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + *
> + * See also:
> + * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
> + */
> +int drm_atomic_helper_disable_all(struct drm_device *dev,
> +				  struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_atomic_state *state;
> +	struct drm_connector *conn;
> +	int err;
> +
> +	state = drm_atomic_state_alloc(dev);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	state->acquire_ctx = ctx;
> +
> +	drm_for_each_connector(conn, dev) {

This iterator will raise a WARN() if the caller isn't already holding
either mode_config->mutex or mode_config->connection_mutex.  Should a
note about that be added to the kerneldoc above where you're talking
about locks?  Ditto for the iterator in
drm_atomic_helper_duplicate_state().


Matt

> +		struct drm_crtc *crtc = conn->state->crtc;
> +		struct drm_crtc_state *crtc_state;
> +
> +		if (!crtc || conn->dpms != DRM_MODE_DPMS_ON)
> +			continue;
> +
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		if (IS_ERR(crtc_state)) {
> +			err = PTR_ERR(crtc_state);
> +			goto free;
> +		}
> +
> +		crtc_state->active = false;
> +	}
> +
> +	err = drm_atomic_commit(state);
> +
> +free:
> +	if (err < 0)
> +		drm_atomic_state_free(state);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_disable_all);
> +
> +/**
> + * drm_atomic_helper_suspend - subsystem-level suspend helper
> + * @dev: DRM device
> + *
> + * Duplicates the current atomic state, disables all active outputs and then
> + * returns a pointer to the original atomic state to the caller. Drivers can
> + * pass this pointer to the drm_atomic_helper_resume() helper upon resume to
> + * restore the output configuration that was active at the time the system
> + * entered suspend.
> + *
> + * Note that it is potentially unsafe to use this. The atomic state object
> + * returned by this function is assumed to be persistent. Drivers must ensure
> + * that this holds true. Before calling this function, drivers must make sure
> + * to suspend fbdev emulation so that nothing can be using the device.
> + *
> + * Returns:
> + * A pointer to a copy of the state before suspend on success or an ERR_PTR()-
> + * encoded error code on failure. Drivers should store the returned atomic
> + * state object and pass it to the drm_atomic_helper_resume() helper upon
> + * resume.
> + *
> + * See also:
> + * drm_atomic_helper_duplicate_state(), drm_atomic_helper_disable_all(),
> + * drm_atomic_helper_resume()
> + */
> +struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)
> +{
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_state *state;
> +	int err;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +retry:
> +	err = drm_modeset_lock_all_ctx(dev, &ctx);
> +	if (err < 0) {
> +		state = ERR_PTR(err);
> +		goto unlock;
> +	}
> +
> +	state = drm_atomic_helper_duplicate_state(dev, &ctx);
> +	if (IS_ERR(state))
> +		goto unlock;
> +
> +	err = drm_atomic_helper_disable_all(dev, &ctx);
> +	if (err < 0) {
> +		drm_atomic_state_free(state);
> +		state = ERR_PTR(err);
> +		goto unlock;
> +	}
> +
> +unlock:
> +	if (PTR_ERR(state) == -EDEADLK) {
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +	return state;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_suspend);
> +
> +/**
> + * drm_atomic_helper_resume - subsystem-level resume helper
> + * @dev: DRM device
> + * @state: atomic state to resume to
> + *
> + * Calls drm_mode_config_reset() to synchronize hardware and software states,
> + * grabs all modeset locks and commits the atomic state object. This can be
> + * used in conjunction with the drm_atomic_helper_suspend() helper to
> + * implement suspend/resume for drivers that support atomic mode-setting.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + *
> + * See also:
> + * drm_atomic_helper_suspend()
> + */
> +int drm_atomic_helper_resume(struct drm_device *dev,
> +			     struct drm_atomic_state *state)
> +{
> +	struct drm_mode_config *config = &dev->mode_config;
> +	int err;
> +
> +	drm_mode_config_reset(dev);
> +	drm_modeset_lock_all(dev);
> +	state->acquire_ctx = config->acquire_ctx;
> +	err = drm_atomic_commit(state);
> +	drm_modeset_unlock_all(dev);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_resume);
> +
> +/**
>   * drm_atomic_helper_crtc_set_property - helper for crtc properties
>   * @crtc: DRM crtc
>   * @property: DRM property
> @@ -2429,7 +2584,9 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_duplicate_state);
>   * @ctx: lock acquisition context
>   *
>   * Makes a copy of the current atomic state by looping over all objects and
> - * duplicating their respective states.
> + * duplicating their respective states. This is used for example by suspend/
> + * resume support code to save the state prior to suspend such that it can
> + * be restored upon resume.
>   *
>   * Note that this treats atomic state as persistent between save and restore.
>   * Drivers must make sure that this is possible and won't result in confusion
> @@ -2441,6 +2598,9 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_duplicate_state);
>   * Returns:
>   * A pointer to the copy of the atomic state object on success or an
>   * ERR_PTR()-encoded error code on failure.
> + *
> + * See also:
> + * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
>   */
>  struct drm_atomic_state *
>  drm_atomic_helper_duplicate_state(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 6b4cf25fed12..10d0989db273 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -855,6 +855,12 @@ EXPORT_SYMBOL(drm_helper_mode_fill_fb_struct);
>   * due to slight differences in allocating shared resources when the
>   * configuration is restored in a different order than when userspace set it up)
>   * need to use their own restore logic.
> + *
> + * This function is deprecated. New drivers should implement atomic mode-
> + * setting and use the atomic suspend/resume helpers.
> + *
> + * See also:
> + * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
>   */
>  void drm_helper_resume_force_mode(struct drm_device *dev)
>  {
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 8cba54a2a0a0..8045cdea8cc9 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -81,6 +81,12 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set);
>  int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>  		struct drm_atomic_state *state);
>  
> +int drm_atomic_helper_disable_all(struct drm_device *dev,
> +				  struct drm_modeset_acquire_ctx *ctx);
> +struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev);
> +int drm_atomic_helper_resume(struct drm_device *dev,
> +			     struct drm_atomic_state *state);
> +
>  int drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc,
>  					struct drm_property *property,
>  					uint64_t val);
> -- 
> 2.5.0
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel