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); 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. + * + * 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_ */
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);
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
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 +}
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
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Thierry Reding