Reviewed-by: Harry Wentland: <harry.wentland at amd.com>
On 2016-12-15 10:58 AM, Daniel Vetter wrote:
> Mostly nothing special (except making sure that really all error paths
> and friends call iter_put).
>
> v2: Don't forget the raw connector_list walking in
> drm_helper_move_panel_connectors_to_head. That one unfortunately can't
> be converted to the iterator helpers, but since it's just some list
> splicing best to just wrap the entire thing up in one critical
> section.
>
> v3: Bail out after iter_put (Harry).
>
> Cc: Harry Wentland <harry.wentland at amd.com>
> Reviewed-by: Harry Wentland <harry.wentland at amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 39 ++++++++++++++++++++--------
> drivers/gpu/drm/drm_crtc_helper.c | 49
> ++++++++++++++++++++++++++++--------
> drivers/gpu/drm/drm_fb_helper.c | 12 ++++++---
> drivers/gpu/drm/drm_modeset_helper.c | 2 ++
> drivers/gpu/drm/drm_plane_helper.c | 5 +++-
> drivers/gpu/drm/drm_probe_helper.c | 18 ++++++++-----
> 6 files changed, 92 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 23767df72615..e2e15a9903a9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -94,9 +94,10 @@ static int handle_conflicting_encoders(struct
> drm_atomic_state *state,
> {
> struct drm_connector_state *conn_state;
> struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> struct drm_encoder *encoder;
> unsigned encoder_mask = 0;
> - int i, ret;
> + int i, ret = 0;
>
> /*
> * First loop, find all newly assigned encoders from the connectors
> @@ -144,7 +145,8 @@ static int handle_conflicting_encoders(struct
> drm_atomic_state *state,
> * and the crtc is disabled if no encoder is left. This preserves
> * compatibility with the legacy set_config behavior.
> */
> - drm_for_each_connector(connector, state->dev) {
> + drm_connector_list_iter_get(state->dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> struct drm_crtc_state *crtc_state;
>
> if (drm_atomic_get_existing_connector_state(state, connector))
> @@ -160,12 +162,15 @@ static int handle_conflicting_encoders(struct
> drm_atomic_state *state,
> connector->state->crtc->base.id,
> connector->state->crtc->name,
> connector->base.id, connector->name);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out;
> }
>
> conn_state = drm_atomic_get_connector_state(state, connector);
> - if (IS_ERR(conn_state))
> - return PTR_ERR(conn_state);
> + if (IS_ERR(conn_state)) {
> + ret = PTR_ERR(conn_state);
> + goto out;
> + }
>
> DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s],
> disabling [CONNECTOR:%d:%s]\n",
> encoder->base.id, encoder->name,
> @@ -176,19 +181,21 @@ static int handle_conflicting_encoders(struct
> drm_atomic_state *state,
>
> ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
> if (ret)
> - return ret;
> + goto out;
>
> if (!crtc_state->connector_mask) {
> ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
> NULL);
> if (ret < 0)
> - return ret;
> + goto out;
>
> crtc_state->active = false;
> }
> }
> +out:
> + drm_connector_list_iter_put(&conn_iter);
>
> - return 0;
> + return ret;
> }
>
> static void
> @@ -2442,6 +2449,7 @@ int drm_atomic_helper_disable_all(struct drm_device
> *dev,
> {
> struct drm_atomic_state *state;
> struct drm_connector *conn;
> + struct drm_connector_list_iter conn_iter;
> int err;
>
> state = drm_atomic_state_alloc(dev);
> @@ -2450,7 +2458,8 @@ int drm_atomic_helper_disable_all(struct drm_device
> *dev,
>
> state->acquire_ctx = ctx;
>
> - drm_for_each_connector(conn, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(conn, &conn_iter) {
> struct drm_crtc *crtc = conn->state->crtc;
> struct drm_crtc_state *crtc_state;
>
> @@ -2468,6 +2477,7 @@ int drm_atomic_helper_disable_all(struct drm_device
> *dev,
>
> err = drm_atomic_commit(state);
> free:
> + drm_connector_list_iter_put(&conn_iter);
> drm_atomic_state_put(state);
> return err;
> }
> @@ -2840,6 +2850,7 @@ int drm_atomic_helper_connector_dpms(struct
> drm_connector *connector,
> struct drm_crtc_state *crtc_state;
> struct drm_crtc *crtc;
> struct drm_connector *tmp_connector;
> + struct drm_connector_list_iter conn_iter;
> int ret;
> bool active = false;
> int old_mode = connector->dpms;
> @@ -2867,7 +2878,8 @@ int drm_atomic_helper_connector_dpms(struct
> drm_connector *connector,
>
> WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
>
> - drm_for_each_connector(tmp_connector, connector->dev) {
> + drm_connector_list_iter_get(connector->dev, &conn_iter);
> + drm_for_each_connector_iter(tmp_connector, &conn_iter) {
> if (tmp_connector->state->crtc != crtc)
> continue;
>
> @@ -2876,6 +2888,7 @@ int drm_atomic_helper_connector_dpms(struct
> drm_connector *connector,
> break;
> }
> }
> + drm_connector_list_iter_put(&conn_iter);
> crtc_state->active = active;
>
> ret = drm_atomic_commit(state);
> @@ -3253,6 +3266,7 @@ drm_atomic_helper_duplicate_state(struct drm_device
> *dev,
> {
> struct drm_atomic_state *state;
> struct drm_connector *conn;
> + struct drm_connector_list_iter conn_iter;
> struct drm_plane *plane;
> struct drm_crtc *crtc;
> int err = 0;
> @@ -3283,15 +3297,18 @@ drm_atomic_helper_duplicate_state(struct drm_device
> *dev,
> }
> }
>
> - drm_for_each_connector(conn, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(conn, &conn_iter) {
> struct drm_connector_state *conn_state;
>
> conn_state = drm_atomic_get_connector_state(state, conn);
> if (IS_ERR(conn_state)) {
> err = PTR_ERR(conn_state);
> + drm_connector_list_iter_put(&conn_iter);
> goto free;
> }
> }
> + drm_connector_list_iter_put(&conn_iter);
>
> /* clear the acquire context so that it isn't accidentally reused */
> state->acquire_ctx = NULL;
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c
> index 9d007f5f9732..ad154325a0fd 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -88,6 +88,7 @@
> bool drm_helper_encoder_in_use(struct drm_encoder *encoder)
> {
> struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> struct drm_device *dev = encoder->dev;
>
> /*
> @@ -99,9 +100,15 @@ bool drm_helper_encoder_in_use(struct drm_encoder
> *encoder)
>
> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> }
>
> - drm_for_each_connector(connector, dev)
> - if (connector->encoder == encoder)
> +
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> + if (connector->encoder == encoder) {
> + drm_connector_list_iter_put(&conn_iter);
> return true;
> + }
> + }
> + drm_connector_list_iter_put(&conn_iter);
> return false;
> }
> EXPORT_SYMBOL(drm_helper_encoder_in_use);
> @@ -436,10 +443,13 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
>
> /* Decouple all encoders and their attached connectors from this crtc */
> drm_for_each_encoder(encoder, dev) {
> + struct drm_connector_list_iter conn_iter;
> +
> if (encoder->crtc != crtc)
> continue;
>
> - drm_for_each_connector(connector, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> if (connector->encoder != encoder)
> continue;
>
> @@ -456,6 +466,7 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
> /* we keep a reference while the encoder is bound */
> drm_connector_unreference(connector);
> }
> + drm_connector_list_iter_put(&conn_iter);
> }
>
> __drm_helper_disable_unused_functions(dev);
> @@ -507,6 +518,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
> bool mode_changed = false; /* if true do a full mode set */
> bool fb_changed = false; /* if true and !mode_changed just do a flip */
> struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> int count = 0, ro, fail = 0;
> const struct drm_crtc_helper_funcs *crtc_funcs;
> struct drm_mode_set save_set;
> @@ -571,9 +583,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
> }
>
> count = 0;
> - drm_for_each_connector(connector, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter)
> save_connector_encoders[count++] = connector->encoder;
> - }
> + drm_connector_list_iter_put(&conn_iter);
>
> save_set.crtc = set->crtc;
> save_set.mode = &set->crtc->mode;
> @@ -615,7 +628,8 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
>
> /* a) traverse passed in connector list and get encoders for them */
> count = 0;
> - drm_for_each_connector(connector, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> const struct drm_connector_helper_funcs *connector_funcs =
> connector->helper_private;
> new_encoder = connector->encoder;
> @@ -648,6 +662,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
> connector->encoder = new_encoder;
> }
> }
> + drm_connector_list_iter_put(&conn_iter);
>
> if (fail) {
> ret = -EINVAL;
> @@ -655,7 +670,8 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
> }
>
> count = 0;
> - drm_for_each_connector(connector, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> if (!connector->encoder)
> continue;
>
> @@ -673,6 +689,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
> if (new_crtc &&
> !drm_encoder_crtc_ok(connector->encoder, new_crtc)) {
> ret = -EINVAL;
> + drm_connector_list_iter_put(&conn_iter);
> goto fail;
> }
> if (new_crtc != connector->encoder->crtc) {
> @@ -689,6 +706,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
> connector->base.id, connector->name);
> }
> }
> + drm_connector_list_iter_put(&conn_iter);
>
> /* mode_set_base is not a required function */
> if (fb_changed && !crtc_funcs->mode_set_base)
> @@ -743,9 +761,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
> }
>
> count = 0;
> - drm_for_each_connector(connector, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter)
> connector->encoder = save_connector_encoders[count++];
> - }
> + drm_connector_list_iter_put(&conn_iter);
>
> /* after fail drop reference on all unbound connectors in set, let
> * bound connectors keep their reference
> @@ -772,12 +791,16 @@ static int drm_helper_choose_encoder_dpms(struct
> drm_encoder *encoder)
> {
> int dpms = DRM_MODE_DPMS_OFF;
> struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> struct drm_device *dev = encoder->dev;
>
> - drm_for_each_connector(connector, dev)
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter)
> if (connector->encoder == encoder)
> if (connector->dpms < dpms)
> dpms = connector->dpms;
> + drm_connector_list_iter_put(&conn_iter);
> +
> return dpms;
> }
>
> @@ -809,12 +832,16 @@ static int drm_helper_choose_crtc_dpms(struct drm_crtc
> *crtc)
> {
> int dpms = DRM_MODE_DPMS_OFF;
> struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> struct drm_device *dev = crtc->dev;
>
> - drm_for_each_connector(connector, dev)
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter)
> if (connector->encoder && connector->encoder->crtc == crtc)
> if (connector->dpms < dpms)
> dpms = connector->dpms;
> + drm_connector_list_iter_put(&conn_iter);
> +
> return dpms;
> }
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index bee5e4149a1c..145d55fef69e 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -120,20 +120,22 @@ int drm_fb_helper_single_add_all_connectors(struct
> drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
> struct drm_connector *connector;
> - int i, ret;
> + struct drm_connector_list_iter conn_iter;
> + int i, ret = 0;
>
> if (!drm_fbdev_emulation)
> return 0;
>
> mutex_lock(&dev->mode_config.mutex);
> - drm_for_each_connector(connector, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> ret = drm_fb_helper_add_one_connector(fb_helper, connector);
>
> if (ret)
> goto fail;
> }
> - mutex_unlock(&dev->mode_config.mutex);
> - return 0;
> + goto out;
> +
> fail:
> drm_fb_helper_for_each_connector(fb_helper, i) {
> struct drm_fb_helper_connector *fb_helper_connector =
> @@ -145,6 +147,8 @@ int drm_fb_helper_single_add_all_connectors(struct
> drm_fb_helper *fb_helper)
> fb_helper->connector_info[i] = NULL;
> }
> fb_helper->connector_count = 0;
> +out:
> + drm_connector_list_iter_put(&conn_iter);
> mutex_unlock(&dev->mode_config.mutex);
>
> return ret;
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c
> b/drivers/gpu/drm/drm_modeset_helper.c
> index 5b051859b8d3..5d8fa791fff5 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -48,6 +48,7 @@ void drm_helper_move_panel_connectors_to_head(struct
> drm_device *dev)
>
> INIT_LIST_HEAD(&panel_list);
>
> + spin_lock_irq(&dev->mode_config.connector_list_lock);
> list_for_each_entry_safe(connector, tmp,
> &dev->mode_config.connector_list, head) {
> if (connector->connector_type == DRM_MODE_CONNECTOR_LVDS ||
> @@ -57,6 +58,7 @@ void drm_helper_move_panel_connectors_to_head(struct
> drm_device *dev)
> }
>
> list_splice(&panel_list, &dev->mode_config.connector_list);
> + spin_unlock_irq(&dev->mode_config.connector_list_lock);
> }
> EXPORT_SYMBOL(drm_helper_move_panel_connectors_to_head);
>
> diff --git a/drivers/gpu/drm/drm_plane_helper.c
> b/drivers/gpu/drm/drm_plane_helper.c
> index 7a7dddf604d7..bc9f97422cd1 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -74,6 +74,7 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> {
> struct drm_device *dev = crtc->dev;
> struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> int count = 0;
>
> /*
> @@ -83,7 +84,8 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> */
> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>
> - drm_for_each_connector(connector, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> if (connector->encoder && connector->encoder->crtc == crtc) {
> if (connector_list != NULL && count < num_connectors)
> *(connector_list++) = connector;
> @@ -91,6 +93,7 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> count++;
> }
> }
> + drm_connector_list_iter_put(&conn_iter);
>
> return count;
> }
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index ac953f037be7..7cff91e7497f 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -129,6 +129,7 @@ void drm_kms_helper_poll_enable_locked(struct drm_device
> *dev)
> {
> bool poll = false;
> struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> unsigned long delay = DRM_OUTPUT_POLL_PERIOD;
>
> WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
> @@ -136,11 +137,13 @@ void drm_kms_helper_poll_enable_locked(struct
> drm_device *dev)
> if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
> return;
>
> - drm_for_each_connector(connector, dev) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT |
> DRM_CONNECTOR_POLL_DISCONNECT))
> poll = true;
> }
> + drm_connector_list_iter_put(&conn_iter);
>
> if (dev->mode_config.delayed_event) {
> poll = true;
> @@ -382,6 +385,7 @@ static void output_poll_execute(struct work_struct *work)
> struct delayed_work *delayed_work = to_delayed_work(work);
> struct drm_device *dev = container_of(delayed_work, struct drm_device,
> mode_config.output_poll_work);
> struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> enum drm_connector_status old_status;
> bool repoll = false, changed;
>
> @@ -397,8 +401,8 @@ static void output_poll_execute(struct work_struct *work)
> goto out;
> }
>
> - drm_for_each_connector(connector, dev) {
> -
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> /* Ignore forced connectors. */
> if (connector->force)
> continue;
> @@ -451,6 +455,7 @@ static void output_poll_execute(struct work_struct *work)
> changed = true;
> }
> }
> + drm_connector_list_iter_put(&conn_iter);
>
> mutex_unlock(&dev->mode_config.mutex);
>
> @@ -562,6 +567,7 @@ EXPORT_SYMBOL(drm_kms_helper_poll_fini);
> bool drm_helper_hpd_irq_event(struct drm_device *dev)
> {
> struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> enum drm_connector_status old_status;
> bool changed = false;
>
> @@ -569,8 +575,8 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
> return false;
>
> mutex_lock(&dev->mode_config.mutex);
> - drm_for_each_connector(connector, dev) {
> -
> + drm_connector_list_iter_get(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> /* Only handle HPD capable connectors. */
> if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
> continue;
> @@ -586,7 +592,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
> if (old_status != connector->status)
> changed = true;
> }
> -
> + drm_connector_list_iter_put(&conn_iter);
> mutex_unlock(&dev->mode_config.mutex);
>
> if (changed)
>
