| Message ID | 1439293065-15941-2-git-send-email-thierry.reding@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 11-08-15 om 13:37 schreef 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. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 191 ++++++++++++++++++++++++++++++++++++ > include/drm/drm_atomic_helper.h | 5 + > 2 files changed, 196 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index f842499ca79e..aface21b713e 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1751,6 +1751,197 @@ backoff: > EXPORT_SYMBOL(drm_atomic_helper_set_config); > > /** > + * drm_atomic_helper_disable_all - disable all currently active outputs > + * @dev: DRM device > + * > + * 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. > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +int drm_atomic_helper_disable_all(struct drm_device *dev) > +{ > + struct drm_mode_config *config = &dev->mode_config; > + struct drm_connector_state *conn_state; > + struct drm_modeset_acquire_ctx ctx; > + struct drm_atomic_state *state; > + struct drm_connector *conn; > + unsigned int i; > + int err; > + > + state = drm_atomic_state_alloc(dev); > + if (!state) > + return -ENOMEM; > + > + drm_modeset_acquire_init(&ctx, 0); > + > + state->allow_modeset = true; > + state->acquire_ctx = &ctx; > + > +retry: > + err = drm_modeset_lock(&config->connection_mutex, &ctx); > + if (err < 0) > + goto free; > + > + 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 unlock; > + } > + > + err = drm_atomic_add_affected_connectors(state, crtc); > + if (err < 0) > + goto unlock; > + > + conn->dpms = DRM_MODE_DPMS_OFF; > + crtc_state->active = false; > + } > + > + err = drm_atomic_commit(state); > + if (err < 0) { > + /* restore DPMS mode */ > + for_each_connector_in_state(state, conn, conn_state, i) > + conn->dpms = DRM_MODE_DPMS_ON; > + > + if (err == -EDEADLK) { > + drm_atomic_state_clear(state); > + drm_modeset_backoff(&ctx); > + goto retry; > + } > + } > + > +unlock: > + drm_modeset_drop_locks(&ctx); > +free: > + drm_modeset_acquire_fini(&ctx); > + > + if (err < 0) > + drm_atomic_state_free(state); > + > + return err; > +} > +EXPORT_SYMBOL(drm_atomic_helper_disable_all); Use lock_all again. And this version is not handling -EDEADLK from get_*_state so it doesn't work as intended. :-) In case of doubt, try with CONFIG_DEBUG_WW_MUTEX_SLOWPATH. > +/** > + * drm_atomic_helper_restore_commit - restores an atomic state object > + * @state: atomic state object > + * > + * Takes an atomic state object for which no locks have been taken (such as > + * atomic state objects obtained from drm_atomic_helper_duplicate_state()), > + * grabs all the necessary locks, commits the state and releases the locks > + * while gracefully handling deadlocks. > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +static int drm_atomic_helper_restore_commit(struct drm_atomic_state *state) > +{ > + struct drm_mode_config *config = &state->dev->mode_config; > + struct drm_plane_state *plane_state; > + struct drm_modeset_acquire_ctx ctx; > + struct drm_crtc_state *crtc_state; > + struct drm_plane *plane; > + struct drm_crtc *crtc; > + unsigned int i; > + int err; > + > + drm_modeset_acquire_init(&ctx, 0); > + state->allow_modeset = true; > + state->acquire_ctx = &ctx; > + > +retry: > + err = drm_modeset_lock(&config->connection_mutex, &ctx); > + if (err < 0) > + goto fini; > + > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + err = drm_modeset_lock(&crtc->mutex, &ctx); > + if (err < 0) > + goto unlock; > + } > + > + for_each_plane_in_state(state, plane, plane_state, i) { > + err = drm_modeset_lock(&plane->mutex, &ctx); > + if (err < 0) > + goto unlock; > + } > + > + err = drm_atomic_commit(state); > + if (err == -EDEADLK) { > + drm_modeset_backoff(&ctx); > + goto retry; > + } > + > +unlock: > + drm_modeset_drop_locks(&ctx); > +fini: > + drm_modeset_acquire_fini(&ctx); > + > + return err; > +} ^^ drm_modeset_lock_all(dev); state->acquire_ctx = ...; ret = drm_atomic_commit(state); drm_modeset_unlock_all(dev); and fold it into atomic_helper_resume > +/** > + * 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. > + * > + * 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. > + */ > +struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev) > +{ > + struct drm_atomic_state *state; > + int err; drm_modeset_lock_all(); > + state = drm_atomic_helper_duplicate_state(dev); > + if (IS_ERR(state)) > + return state; > + > + err = drm_atomic_helper_disable_all(dev); > + if (err < 0) { > + drm_atomic_state_free(state); > + return ERR_PTR(err); > + } drm_modeset_lock_unlock_all(); > + 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 > + * > + * Takes an atomic state object, grabs all the necessary locks and commits it. > + * This can be used in conjunction with the drm_atomic_helper_suspend() helper > + * to implement suspend/resume for atomic mode-setting enabled drivers. > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +int drm_atomic_helper_resume(struct drm_device *dev, > + struct drm_atomic_state *state) > +{ > + drm_mode_config_reset(dev); > + > + return drm_atomic_helper_restore_commit(state); > +} > +EXPORT_SYMBOL(drm_atomic_helper_resume); > + > +/** > * drm_atomic_helper_crtc_set_property - helper for crtc properties > * @crtc: DRM crtc > * @property: DRM property > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index 5d27612997ce..080f131c9f2e 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -75,6 +75,11 @@ int drm_atomic_helper_update_plane(struct drm_plane *plane, > int drm_atomic_helper_disable_plane(struct drm_plane *plane); > int drm_atomic_helper_set_config(struct drm_mode_set *set); > > +int drm_atomic_helper_disable_all(struct drm_device *dev); > +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, Aug 11, 2015 at 01:37:44PM +0200, 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. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 191 ++++++++++++++++++++++++++++++++++++ > include/drm/drm_atomic_helper.h | 5 + > 2 files changed, 196 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index f842499ca79e..aface21b713e 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1751,6 +1751,197 @@ backoff: > EXPORT_SYMBOL(drm_atomic_helper_set_config); > > /** > + * drm_atomic_helper_disable_all - disable all currently active outputs > + * @dev: DRM device > + * > + * 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. Imo the subordinate helpers here should also mention how they're used in the suspend/resume helpers and reference them (soonish all those function references will be hyperlinks, making them even more useful). Same for the other subordinate helpers (also duplicate_state from the previous patch). > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +int drm_atomic_helper_disable_all(struct drm_device *dev) > +{ > + struct drm_mode_config *config = &dev->mode_config; > + struct drm_connector_state *conn_state; > + struct drm_modeset_acquire_ctx ctx; > + struct drm_atomic_state *state; > + struct drm_connector *conn; > + unsigned int i; > + int err; > + > + state = drm_atomic_state_alloc(dev); > + if (!state) > + return -ENOMEM; > + > + drm_modeset_acquire_init(&ctx, 0); > + > + state->allow_modeset = true; > + state->acquire_ctx = &ctx; > + > +retry: > + err = drm_modeset_lock(&config->connection_mutex, &ctx); > + if (err < 0) > + goto free; Same comment about locking as with the duplicate_state helper. > + > + 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 unlock; > + } > + > + err = drm_atomic_add_affected_connectors(state, crtc); > + if (err < 0) > + goto unlock; > + > + conn->dpms = DRM_MODE_DPMS_OFF; > + crtc_state->active = false; > + } > + > + err = drm_atomic_commit(state); > + if (err < 0) { > + /* restore DPMS mode */ > + for_each_connector_in_state(state, conn, conn_state, i) > + conn->dpms = DRM_MODE_DPMS_ON; > + > + if (err == -EDEADLK) { > + drm_atomic_state_clear(state); > + drm_modeset_backoff(&ctx); > + goto retry; > + } > + } > + > +unlock: > + drm_modeset_drop_locks(&ctx); > +free: > + drm_modeset_acquire_fini(&ctx); > + > + if (err < 0) > + drm_atomic_state_free(state); > + > + return err; > +} > +EXPORT_SYMBOL(drm_atomic_helper_disable_all); > + > +/** > + * drm_atomic_helper_restore_commit - restores an atomic state object > + * @state: atomic state object > + * > + * Takes an atomic state object for which no locks have been taken (such as > + * atomic state objects obtained from drm_atomic_helper_duplicate_state()), > + * grabs all the necessary locks, commits the state and releases the locks > + * while gracefully handling deadlocks. I think this also needs some warnings that reusing atomic states while locks have been droppped could be risky, similar to duplicate_state(). > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +static int drm_atomic_helper_restore_commit(struct drm_atomic_state *state) > +{ > + struct drm_mode_config *config = &state->dev->mode_config; > + struct drm_plane_state *plane_state; > + struct drm_modeset_acquire_ctx ctx; > + struct drm_crtc_state *crtc_state; > + struct drm_plane *plane; > + struct drm_crtc *crtc; > + unsigned int i; > + int err; > + > + drm_modeset_acquire_init(&ctx, 0); > + state->allow_modeset = true; > + state->acquire_ctx = &ctx; > + > +retry: > + err = drm_modeset_lock(&config->connection_mutex, &ctx); > + if (err < 0) > + goto fini; > + > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + err = drm_modeset_lock(&crtc->mutex, &ctx); > + if (err < 0) > + goto unlock; > + } > + > + for_each_plane_in_state(state, plane, plane_state, i) { > + err = drm_modeset_lock(&plane->mutex, &ctx); > + if (err < 0) > + goto unlock; > + } > + > + err = drm_atomic_commit(state); > + if (err == -EDEADLK) { > + drm_modeset_backoff(&ctx); > + goto retry; > + } > + > +unlock: > + drm_modeset_drop_locks(&ctx); > +fini: > + drm_modeset_acquire_fini(&ctx); > + > + return err; > +} > + > +/** > + * 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. > + * > + * 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. Style-nit: There should be a newline after "Returns:" for consistency with other kerneldoc in drm. > + */ > +struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev) > +{ > + struct drm_atomic_state *state; > + int err; > + > + state = drm_atomic_helper_duplicate_state(dev); > + if (IS_ERR(state)) > + return state; > + > + err = drm_atomic_helper_disable_all(dev); > + if (err < 0) { > + drm_atomic_state_free(state); This will hit a locking WARN_ON. Maybe better to pull out the drm_modset_lock all from these two functions and just pass them the acquire ctx? Otoh drm-misc already has a patch from Maarten to remove that locking check since its violated with drm dp mst stuff iirc. > + return ERR_PTR(err); > + } > + > + 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 > + * > + * Takes an atomic state object, grabs all the necessary locks and commits it. > + * This can be used in conjunction with the drm_atomic_helper_suspend() helper > + * to implement suspend/resume for atomic mode-setting enabled drivers. Kerneldoc should also mention that it calls reset() first to make sure sw and hw state agree again. Otherwise the atomic_commit might go "well there's nothing to do here", so it's a crucial step. Looks great otherwise. -Daniel > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +int drm_atomic_helper_resume(struct drm_device *dev, > + struct drm_atomic_state *state) > +{ > + drm_mode_config_reset(dev); > + > + return drm_atomic_helper_restore_commit(state); > +} > +EXPORT_SYMBOL(drm_atomic_helper_resume); > + > +/** > * drm_atomic_helper_crtc_set_property - helper for crtc properties > * @crtc: DRM crtc > * @property: DRM property > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index 5d27612997ce..080f131c9f2e 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -75,6 +75,11 @@ int drm_atomic_helper_update_plane(struct drm_plane *plane, > int drm_atomic_helper_disable_plane(struct drm_plane *plane); > int drm_atomic_helper_set_config(struct drm_mode_set *set); > > +int drm_atomic_helper_disable_all(struct drm_device *dev); > +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.4.5 >
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index f842499ca79e..aface21b713e 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1751,6 +1751,197 @@ backoff: EXPORT_SYMBOL(drm_atomic_helper_set_config); /** + * drm_atomic_helper_disable_all - disable all currently active outputs + * @dev: DRM device + * + * 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. + * + * Returns: 0 on success or a negative error code on failure. + */ +int drm_atomic_helper_disable_all(struct drm_device *dev) +{ + struct drm_mode_config *config = &dev->mode_config; + struct drm_connector_state *conn_state; + struct drm_modeset_acquire_ctx ctx; + struct drm_atomic_state *state; + struct drm_connector *conn; + unsigned int i; + int err; + + state = drm_atomic_state_alloc(dev); + if (!state) + return -ENOMEM; + + drm_modeset_acquire_init(&ctx, 0); + + state->allow_modeset = true; + state->acquire_ctx = &ctx; + +retry: + err = drm_modeset_lock(&config->connection_mutex, &ctx); + if (err < 0) + goto free; + + 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 unlock; + } + + err = drm_atomic_add_affected_connectors(state, crtc); + if (err < 0) + goto unlock; + + conn->dpms = DRM_MODE_DPMS_OFF; + crtc_state->active = false; + } + + err = drm_atomic_commit(state); + if (err < 0) { + /* restore DPMS mode */ + for_each_connector_in_state(state, conn, conn_state, i) + conn->dpms = DRM_MODE_DPMS_ON; + + if (err == -EDEADLK) { + drm_atomic_state_clear(state); + drm_modeset_backoff(&ctx); + goto retry; + } + } + +unlock: + drm_modeset_drop_locks(&ctx); +free: + drm_modeset_acquire_fini(&ctx); + + if (err < 0) + drm_atomic_state_free(state); + + return err; +} +EXPORT_SYMBOL(drm_atomic_helper_disable_all); + +/** + * drm_atomic_helper_restore_commit - restores an atomic state object + * @state: atomic state object + * + * Takes an atomic state object for which no locks have been taken (such as + * atomic state objects obtained from drm_atomic_helper_duplicate_state()), + * grabs all the necessary locks, commits the state and releases the locks + * while gracefully handling deadlocks. + * + * Returns: 0 on success or a negative error code on failure. + */ +static int drm_atomic_helper_restore_commit(struct drm_atomic_state *state) +{ + struct drm_mode_config *config = &state->dev->mode_config; + struct drm_plane_state *plane_state; + struct drm_modeset_acquire_ctx ctx; + struct drm_crtc_state *crtc_state; + struct drm_plane *plane; + struct drm_crtc *crtc; + unsigned int i; + int err; + + drm_modeset_acquire_init(&ctx, 0); + state->allow_modeset = true; + state->acquire_ctx = &ctx; + +retry: + err = drm_modeset_lock(&config->connection_mutex, &ctx); + if (err < 0) + goto fini; + + for_each_crtc_in_state(state, crtc, crtc_state, i) { + err = drm_modeset_lock(&crtc->mutex, &ctx); + if (err < 0) + goto unlock; + } + + for_each_plane_in_state(state, plane, plane_state, i) { + err = drm_modeset_lock(&plane->mutex, &ctx); + if (err < 0) + goto unlock; + } + + err = drm_atomic_commit(state); + if (err == -EDEADLK) { + drm_modeset_backoff(&ctx); + goto retry; + } + +unlock: + drm_modeset_drop_locks(&ctx); +fini: + drm_modeset_acquire_fini(&ctx); + + return err; +} + +/** + * 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. + * + * 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. + */ +struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev) +{ + struct drm_atomic_state *state; + int err; + + state = drm_atomic_helper_duplicate_state(dev); + if (IS_ERR(state)) + return state; + + err = drm_atomic_helper_disable_all(dev); + if (err < 0) { + drm_atomic_state_free(state); + return ERR_PTR(err); + } + + 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 + * + * Takes an atomic state object, grabs all the necessary locks and commits it. + * This can be used in conjunction with the drm_atomic_helper_suspend() helper + * to implement suspend/resume for atomic mode-setting enabled drivers. + * + * Returns: 0 on success or a negative error code on failure. + */ +int drm_atomic_helper_resume(struct drm_device *dev, + struct drm_atomic_state *state) +{ + drm_mode_config_reset(dev); + + return drm_atomic_helper_restore_commit(state); +} +EXPORT_SYMBOL(drm_atomic_helper_resume); + +/** * drm_atomic_helper_crtc_set_property - helper for crtc properties * @crtc: DRM crtc * @property: DRM property diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 5d27612997ce..080f131c9f2e 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -75,6 +75,11 @@ int drm_atomic_helper_update_plane(struct drm_plane *plane, int drm_atomic_helper_disable_plane(struct drm_plane *plane); int drm_atomic_helper_set_config(struct drm_mode_set *set); +int drm_atomic_helper_disable_all(struct drm_device *dev); +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);