* [PATCH v4 2/3] drm/atomic-helper: Implement subsystem-level suspend/resume
2015-12-01 9:56 [PATCH v4 1/3] drm: Implement drm_modeset_lock_all_ctx() Thierry Reding
@ 2015-12-01 9:57 ` Thierry Reding
2015-12-01 10:09 ` Daniel Vetter
2015-12-01 9:57 ` [PATCH v4 3/3] drm/tegra: " Thierry Reding
2015-12-01 10:02 ` [PATCH v4 1/3] drm: Implement drm_modeset_lock_all_ctx() Daniel Vetter
2 siblings, 1 reply; 5+ messages in thread
From: Thierry Reding @ 2015-12-01 9:57 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
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)
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 163 +++++++++++++++++++++++++++++++++++-
drivers/gpu/drm/drm_crtc_helper.c | 6 ++
include/drm/drm_atomic_helper.h | 6 ++
3 files changed, 174 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 3731a26979bc..1b51722b2a00 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1818,6 +1818,162 @@ 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) {
+ if (err == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry;
+ }
+
+ state = ERR_PTR(err);
+ goto fini;
+ }
+
+ 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:
+ drm_modeset_drop_locks(&ctx);
+fini:
+ 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 +2585,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 +2599,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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v4 2/3] drm/atomic-helper: Implement subsystem-level suspend/resume
2015-12-01 9:57 ` [PATCH v4 2/3] drm/atomic-helper: Implement subsystem-level suspend/resume Thierry Reding
@ 2015-12-01 10:09 ` Daniel Vetter
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2015-12-01 10:09 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, dri-devel
On Tue, Dec 01, 2015 at 10:57:00AM +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)
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 163 +++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/drm_crtc_helper.c | 6 ++
> include/drm/drm_atomic_helper.h | 6 ++
> 3 files changed, 174 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 3731a26979bc..1b51722b2a00 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1818,6 +1818,162 @@ 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) {
> + if (err == -EDEADLK) {
> + drm_modeset_backoff(&ctx);
> + goto retry;
> + }
> +
> + state = ERR_PTR(err);
> + goto fini;
> + }
> +
> + state = drm_atomic_helper_duplicate_state(dev, &ctx);
> + if (IS_ERR(state))
> + goto unlock;
> +
> + err = drm_atomic_helper_disable_all(dev, &ctx);
If the driver has some internal state protected with it's own locking the
above two function might return -EDEADLK too. Kerneldoc for that function
even mentions that. But since we don't currently have such a driver it's
not critical, but still would be good to fix.
Either way this looks really good now imo.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> + if (err < 0) {
> + drm_atomic_state_free(state);
> + state = ERR_PTR(err);
> + goto unlock;
> + }
> +
> +unlock:
> + drm_modeset_drop_locks(&ctx);
> +fini:
> + 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 +2585,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 +2599,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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 3/3] drm/tegra: Implement subsystem-level suspend/resume
2015-12-01 9:56 [PATCH v4 1/3] drm: Implement drm_modeset_lock_all_ctx() Thierry Reding
2015-12-01 9:57 ` [PATCH v4 2/3] drm/atomic-helper: Implement subsystem-level suspend/resume Thierry Reding
@ 2015-12-01 9:57 ` Thierry Reding
2015-12-01 10:02 ` [PATCH v4 1/3] drm: Implement drm_modeset_lock_all_ctx() Daniel Vetter
2 siblings, 0 replies; 5+ messages in thread
From: Thierry Reding @ 2015-12-01 9:57 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v4 1/3] drm: Implement drm_modeset_lock_all_ctx()
2015-12-01 9:56 [PATCH v4 1/3] drm: Implement drm_modeset_lock_all_ctx() Thierry Reding
2015-12-01 9:57 ` [PATCH v4 2/3] drm/atomic-helper: Implement subsystem-level suspend/resume Thierry Reding
2015-12-01 9:57 ` [PATCH v4 3/3] drm/tegra: " Thierry Reding
@ 2015-12-01 10:02 ` Daniel Vetter
2 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2015-12-01 10:02 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, dri-devel
On Tue, Dec 01, 2015 at 10:56:59AM +0100, Thierry Reding wrote:
> 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()
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 3 +-
> drivers/gpu/drm/drm_modeset_lock.c | 82 ++++++++++++++++++++++++--------------
> include/drm/drm_modeset_lock.h | 4 +-
> 3 files changed, 56 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 55b4debad79b..4cbe7c07231c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1192,8 +1192,7 @@ retry:
> state->acquire_ctx);
You forgot to remove the connection_mutex right above here
> 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..341158c92027 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 drops all modeset locks taken by a previous call to the
> + * drm_modeset_lock_all() function.
> *
> - * This function drop all modeset locks taken by drm_modeset_lock_all.
> + * 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,27 @@ 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. Locks must be dropped by calling the
> + * drm_modeset_drop_locks() function.
Imo this needs a bit more explanation.
Note that compared to drm_modeset_lock_all() it does not take
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
themselves, outside of the acquire context @ctx.
Locks acquired with this function should be released with
drm_modeset_drop_locks() called on @ctx.
With the kerneldoc augmented and atomic_legacy_backoff fixed this is:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> + *
> + * 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 +478,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
>
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread