Re: [PATCH] drm: Convert all helpers to drm_connector_list_iter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 15, 2016 at 10:58 AM, Daniel Vetter <daniel.vetter@xxxxxxxx> 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@xxxxxxx>


Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>

> Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
>  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)
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux