[Intel-gfx] [PATCH 25/38] drm: Add reference counting to drm_atomic_state

Sean Paul seanpaul at chromium.org
Wed Sep 21 07:24:24 UTC 2016


On Tue, Sep 20, 2016 at 11:29 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> drm_atomic_state has a complicated single owner model that tracks the
> single reference from allocation through to destruction on another
> thread - or perhaps on a local error path. We can simplify this tracking
> by using reference counting (at a cost of a few more atomics). This is
> even more beneficial when the lifetime of the state becomes more
> convoluted than being passed to a single worker thread for the commit.
>
> v2: Double check !intel atomic_commit functions for missing gets
> v3: Update kerneldocs
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: dri-devel at lists.freedesktop.org
> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

This looks good to me, it'll hopefully give drivers a bit more
flexibility with respect to keeping state around during/directly after
commit.

Reviewed-by: Sean Paul <seanpaul at chromium.org>

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  3 +-
>  drivers/gpu/drm/drm_atomic.c                 | 25 +++----
>  drivers/gpu/drm/drm_atomic_helper.c          | 98 +++++++---------------------
>  drivers/gpu/drm/drm_fb_helper.c              |  9 +--
>  drivers/gpu/drm/exynos/exynos_drm_drv.c      |  3 +-
>  drivers/gpu/drm/i915/i915_debugfs.c          |  3 +-
>  drivers/gpu/drm/i915/intel_display.c         | 31 +++++----
>  drivers/gpu/drm/i915/intel_sprite.c          |  4 +-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c       |  3 +-
>  drivers/gpu/drm/msm/msm_atomic.c             |  3 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c           |  3 +-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c        |  3 +-
>  drivers/gpu/drm/sti/sti_drv.c                |  3 +-
>  drivers/gpu/drm/tegra/drm.c                  |  3 +-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  2 -
>  drivers/gpu/drm/vc4/vc4_kms.c                |  3 +-
>  include/drm/drm_atomic.h                     | 28 +++++++-
>  include/drm/drm_crtc.h                       |  3 +
>  18 files changed, 101 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 8e7483d90c47..c9e645fa1233 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -464,7 +464,7 @@ atmel_hlcdc_dc_atomic_complete(struct atmel_hlcdc_dc_commit *commit)
>
>         drm_atomic_helper_cleanup_planes(dev, old_state);
>
> -       drm_atomic_state_free(old_state);
> +       drm_atomic_state_put(old_state);
>
>         /* Complete the commit, wake up any waiter. */
>         spin_lock(&dc->commit.wait.lock);
> @@ -521,6 +521,7 @@ static int atmel_hlcdc_dc_atomic_commit(struct drm_device *dev,
>         /* Swap the state, this is the point of no return. */
>         drm_atomic_helper_swap_state(state, true);
>
> +       drm_atomic_state_get(state);
>         if (async)
>                 queue_work(dc->wq, &commit->work);
>         else
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 23739609427d..5dd70540219c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -74,6 +74,8 @@ EXPORT_SYMBOL(drm_atomic_state_default_release);
>  int
>  drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
>  {
> +       kref_init(&state->ref);
> +
>         /* TODO legacy paths should maybe do a better job about
>          * setting this appropriately?
>          */
> @@ -215,22 +217,16 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)
>  EXPORT_SYMBOL(drm_atomic_state_clear);
>
>  /**
> - * drm_atomic_state_free - free all memory for an atomic state
> - * @state: atomic state to deallocate
> + * __drm_atomic_state_free - free all memory for an atomic state
> + * @ref: This atomic state to deallocate
>   *
>   * This frees all memory associated with an atomic state, including all the
>   * per-object state for planes, crtcs and connectors.
>   */
> -void drm_atomic_state_free(struct drm_atomic_state *state)
> +void __drm_atomic_state_free(struct kref *ref)
>  {
> -       struct drm_device *dev;
> -       struct drm_mode_config *config;
> -
> -       if (!state)
> -               return;
> -
> -       dev = state->dev;
> -       config = &dev->mode_config;
> +       struct drm_atomic_state *state = container_of(ref, typeof(*state), ref);
> +       struct drm_mode_config *config = &state->dev->mode_config;
>
>         drm_atomic_state_clear(state);
>
> @@ -243,7 +239,7 @@ void drm_atomic_state_free(struct drm_atomic_state *state)
>                 kfree(state);
>         }
>  }
> -EXPORT_SYMBOL(drm_atomic_state_free);
> +EXPORT_SYMBOL(__drm_atomic_state_free);
>
>  /**
>   * drm_atomic_get_crtc_state - get crtc state
> @@ -1742,7 +1738,7 @@ retry:
>         if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
>                 /*
>                  * Unlike commit, check_only does not clean up state.
> -                * Below we call drm_atomic_state_free for it.
> +                * Below we call drm_atomic_state_put for it.
>                  */
>                 ret = drm_atomic_check_only(state);
>         } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
> @@ -1775,8 +1771,7 @@ out:
>                 goto retry;
>         }
>
> -       if (ret || arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
> -               drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>
>         drm_modeset_drop_locks(&ctx);
>         drm_modeset_acquire_fini(&ctx);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index c3f83476f996..df6e9b7c377c 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1207,7 +1207,7 @@ static void commit_tail(struct drm_atomic_state *state)
>
>         drm_atomic_helper_commit_cleanup_done(state);
>
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>  }
>
>  static void commit_work(struct work_struct *work)
> @@ -1289,6 +1289,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>          * make sure work items don't artifically stall on each another.
>          */
>
> +       drm_atomic_state_get(state);
>         if (nonblock)
>                 queue_work(system_unbound_wq, &state->commit_work);
>         else
> @@ -1590,7 +1591,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_hw_done);
>   *
>   * This signals completion of the atomic update @state, including any cleanup
>   * work. If used, it must be called right before calling
> - * drm_atomic_state_free().
> + * drm_atomic_state_put().
>   *
>   * This is part of the atomic helper support for nonblocking commits, see
>   * drm_atomic_helper_setup_commit() for an overview.
> @@ -2113,18 +2114,13 @@ retry:
>                 state->legacy_cursor_update = true;
>
>         ret = drm_atomic_commit(state);
> -       if (ret != 0)
> -               goto fail;
> -
> -       /* Driver takes ownership of state on successful commit. */
> -       return 0;
>  fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
> +
>  backoff:
>         drm_atomic_state_clear(state);
>         drm_atomic_legacy_backoff(state);
> @@ -2186,18 +2182,13 @@ retry:
>                 goto fail;
>
>         ret = drm_atomic_commit(state);
> -       if (ret != 0)
> -               goto fail;
> -
> -       /* Driver takes ownership of state on successful commit. */
> -       return 0;
>  fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
> +
>  backoff:
>         drm_atomic_state_clear(state);
>         drm_atomic_legacy_backoff(state);
> @@ -2326,18 +2317,13 @@ retry:
>                 goto fail;
>
>         ret = drm_atomic_commit(state);
> -       if (ret != 0)
> -               goto fail;
> -
> -       /* Driver takes ownership of state on successful commit. */
> -       return 0;
>  fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
> +
>  backoff:
>         drm_atomic_state_clear(state);
>         drm_atomic_legacy_backoff(state);
> @@ -2479,11 +2465,8 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
>         }
>
>         err = drm_atomic_commit(state);
> -
>  free:
> -       if (err < 0)
> -               drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return err;
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_disable_all);
> @@ -2534,7 +2517,7 @@ retry:
>
>         err = drm_atomic_helper_disable_all(dev, &ctx);
>         if (err < 0) {
> -               drm_atomic_state_free(state);
> +               drm_atomic_state_put(state);
>                 state = ERR_PTR(err);
>                 goto unlock;
>         }
> @@ -2623,18 +2606,13 @@ retry:
>                 goto fail;
>
>         ret = drm_atomic_commit(state);
> -       if (ret != 0)
> -               goto fail;
> -
> -       /* Driver takes ownership of state on successful commit. */
> -       return 0;
>  fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
> +
>  backoff:
>         drm_atomic_state_clear(state);
>         drm_atomic_legacy_backoff(state);
> @@ -2683,18 +2661,13 @@ retry:
>                 goto fail;
>
>         ret = drm_atomic_commit(state);
> -       if (ret != 0)
> -               goto fail;
> -
> -       /* Driver takes ownership of state on successful commit. */
> -       return 0;
>  fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
> +
>  backoff:
>         drm_atomic_state_clear(state);
>         drm_atomic_legacy_backoff(state);
> @@ -2743,18 +2716,13 @@ retry:
>                 goto fail;
>
>         ret = drm_atomic_commit(state);
> -       if (ret != 0)
> -               goto fail;
> -
> -       /* Driver takes ownership of state on successful commit. */
> -       return 0;
>  fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
> +
>  backoff:
>         drm_atomic_state_clear(state);
>         drm_atomic_legacy_backoff(state);
> @@ -2827,18 +2795,13 @@ retry:
>         }
>
>         ret = drm_atomic_nonblocking_commit(state);
> -       if (ret != 0)
> -               goto fail;
> -
> -       /* Driver takes ownership of state on successful commit. */
> -       return 0;
>  fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
> +
>  backoff:
>         drm_atomic_state_clear(state);
>         drm_atomic_legacy_backoff(state);
> @@ -2914,19 +2877,14 @@ retry:
>         crtc_state->active = active;
>
>         ret = drm_atomic_commit(state);
> -       if (ret != 0)
> -               goto fail;
> -
> -       /* Driver takes ownership of state on successful commit. */
> -       return 0;
>  fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
>         connector->dpms = old_mode;
> -       drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
> +
>  backoff:
>         drm_atomic_state_clear(state);
>         drm_atomic_legacy_backoff(state);
> @@ -3333,7 +3291,7 @@ drm_atomic_helper_duplicate_state(struct drm_device *dev,
>
>  free:
>         if (err < 0) {
> -               drm_atomic_state_free(state);
> +               drm_atomic_state_put(state);
>                 state = ERR_PTR(err);
>         }
>
> @@ -3448,22 +3406,14 @@ retry:
>                 goto fail;
>
>         ret = drm_atomic_commit(state);
> -       if (ret)
> -               goto fail;
> -
> -       /* Driver takes ownership of state on successful commit. */
> -
> -       drm_property_unreference_blob(blob);
> -
> -       return 0;
>  fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>         drm_property_unreference_blob(blob);
> -
>         return ret;
> +
>  backoff:
>         drm_atomic_state_clear(state);
>         drm_atomic_legacy_backoff(state);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 03414bde1f15..22d4f0e22101 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -367,9 +367,7 @@ fail:
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       if (ret != 0)
> -               drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
>
>  backoff:
> @@ -1361,16 +1359,13 @@ retry:
>         info->var.xoffset = var->xoffset;
>         info->var.yoffset = var->yoffset;
>
> -
>  fail:
>         drm_atomic_clean_old_fb(dev, plane_mask, ret);
>
>         if (ret == -EDEADLK)
>                 goto backoff;
>
> -       if (ret != 0)
> -               drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>         return ret;
>
>  backoff:
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 486943e70f70..f11ba20781ce 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -111,7 +111,7 @@ static void exynos_atomic_commit_complete(struct exynos_atomic_commit *commit)
>
>         drm_atomic_helper_cleanup_planes(dev, state);
>
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>
>         spin_lock(&priv->lock);
>         priv->pending &= ~commit->crtcs;
> @@ -296,6 +296,7 @@ int exynos_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state,
>
>         drm_atomic_helper_swap_state(state, true);
>
> +       drm_atomic_state_get(state);
>         if (nonblock)
>                 schedule_work(&commit->work);
>         else
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7ecdd5cc27dd..8614bf4e479a 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3956,8 +3956,7 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_i915_private *dev_priv,
>  out:
>         drm_modeset_unlock_all(dev);
>         WARN(ret, "Toggling workaround to %i returns %i\n", enable, ret);
> -       if (ret)
> -               drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>  }
>
>  static int ivb_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ea697160591a..c4b28a30b282 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3581,7 +3581,7 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv)
>         return;
>
>  err:
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>  }
>
>  void intel_finish_reset(struct drm_i915_private *dev_priv)
> @@ -3640,6 +3640,8 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
>                 intel_hpd_init(dev_priv);
>         }
>
> +       if (state)
> +               drm_atomic_state_put(state);
>         drm_modeset_drop_locks(ctx);
>         drm_modeset_acquire_fini(ctx);
>         mutex_unlock(&dev->mode_config.mutex);
> @@ -6879,7 +6881,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
>
>         dev_priv->display.crtc_disable(crtc_state, state);
>
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>
>         DRM_DEBUG_KMS("[CRTC:%d:%s] hw state adjusted, was enabled, now disabled\n",
>                       crtc->base.id, crtc->name);
> @@ -11246,8 +11248,8 @@ found:
>         return true;
>
>  fail:
> -       drm_atomic_state_free(state);
> -       drm_atomic_state_free(restore_state);
> +       drm_atomic_state_put(state);
> +       drm_atomic_state_put(restore_state);
>         restore_state = state = NULL;
>
>         if (ret == -EDEADLK) {
> @@ -11276,10 +11278,9 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>                 return;
>
>         ret = drm_atomic_commit(state);
> -       if (ret) {
> +       if (ret)
>                 DRM_DEBUG_KMS("Couldn't release load detect pipe: %i\n", ret);
> -               drm_atomic_state_free(state);
> -       }
> +       drm_atomic_state_put(state);
>  }
>
>  static int i9xx_pll_refclk(struct drm_device *dev,
> @@ -12344,8 +12345,7 @@ retry:
>                         goto retry;
>                 }
>
> -               if (ret)
> -                       drm_atomic_state_free(state);
> +               drm_atomic_state_put(state);
>
>                 if (ret == 0 && event) {
>                         spin_lock_irq(&dev->event_lock);
> @@ -14390,7 +14390,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>
>         drm_atomic_helper_commit_cleanup_done(state);
>
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>
>         /* As one of the primary mmio accessors, KMS has a high likelihood
>          * of triggering bugs in unclaimed access. After we finish
> @@ -14473,6 +14473,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>         intel_shared_dpll_commit(state);
>         intel_atomic_track_fbs(state);
>
> +       drm_atomic_state_get(state);
>         if (nonblock)
>                 queue_work(system_unbound_wq, &state->commit_work);
>         else
> @@ -14514,9 +14515,8 @@ retry:
>                 goto retry;
>         }
>
> -       if (ret)
>  out:
> -               drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>  }
>
>  /*
> @@ -16254,8 +16254,8 @@ retry:
>                 dev_priv->display.optimize_watermarks(cs);
>         }
>
> -       drm_atomic_state_free(state);
>  fail:
> +       drm_atomic_state_put(state);
>         drm_modeset_drop_locks(&ctx);
>         drm_modeset_acquire_fini(&ctx);
>  }
> @@ -16892,10 +16892,9 @@ void intel_display_resume(struct drm_device *dev)
>         drm_modeset_acquire_fini(&ctx);
>         mutex_unlock(&dev->mode_config.mutex);
>
> -       if (ret) {
> +       if (ret)
>                 DRM_ERROR("Restoring old state failed with %i\n", ret);
> -               drm_atomic_state_free(state);
> -       }
> +       drm_atomic_state_put(state);
>  }
>
>  void intel_modeset_gem_init(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 73a521fdf1bd..be3e04623e2a 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -987,9 +987,7 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>                 drm_modeset_backoff(&ctx);
>         }
>
> -       if (ret)
> -               drm_atomic_state_free(state);
> -
> +       drm_atomic_state_put(state);
>  out:
>         drm_modeset_drop_locks(&ctx);
>         drm_modeset_acquire_fini(&ctx);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 72c1ae4e02d4..5ab6017575ea 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -83,7 +83,7 @@ static void mtk_atomic_complete(struct mtk_drm_private *private,
>         drm_atomic_helper_wait_for_vblanks(drm, state);
>
>         drm_atomic_helper_cleanup_planes(drm, state);
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>  }
>
>  static void mtk_atomic_work(struct work_struct *work)
> @@ -110,6 +110,7 @@ static int mtk_atomic_commit(struct drm_device *drm,
>
>         drm_atomic_helper_swap_state(state, true);
>
> +       drm_atomic_state_get(state);
>         if (async)
>                 mtk_atomic_schedule(private, state);
>         else
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
> index 73bae382eac3..db193f835298 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -141,7 +141,7 @@ static void complete_commit(struct msm_commit *c, bool async)
>
>         kms->funcs->complete_commit(kms, state);
>
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>
>         commit_destroy(c);
>  }
> @@ -256,6 +256,7 @@ int msm_atomic_commit(struct drm_device *dev,
>          * current layout.
>          */
>
> +       drm_atomic_state_get(state);
>         if (nonblock) {
>                 queue_work(priv->atomic_wq, &c->work);
>                 return 0;
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index e1cfba51cff6..1735c7accf72 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -105,7 +105,7 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
>
>         dispc_runtime_put();
>
> -       drm_atomic_state_free(old_state);
> +       drm_atomic_state_put(old_state);
>
>         /* Complete the commit, wake up any waiter. */
>         spin_lock(&priv->commit.lock);
> @@ -176,6 +176,7 @@ static int omap_atomic_commit(struct drm_device *dev,
>         /* Swap the state, this is the point of no return. */
>         drm_atomic_helper_swap_state(state, true);
>
> +       drm_atomic_state_get(state);
>         if (nonblock)
>                 schedule_work(&commit->work);
>         else
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index bd9c3bb9252c..c76ed9ee6a01 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -264,7 +264,7 @@ static void rcar_du_atomic_complete(struct rcar_du_commit *commit)
>
>         drm_atomic_helper_cleanup_planes(dev, old_state);
>
> -       drm_atomic_state_free(old_state);
> +       drm_atomic_state_put(old_state);
>
>         /* Complete the commit, wake up any waiter. */
>         spin_lock(&rcdu->commit.wait.lock);
> @@ -330,6 +330,7 @@ static int rcar_du_atomic_commit(struct drm_device *dev,
>         /* Swap the state, this is the point of no return. */
>         drm_atomic_helper_swap_state(state, true);
>
> +       drm_atomic_state_get(state);
>         if (nonblock)
>                 schedule_work(&commit->work);
>         else
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index 7cd3804c6dee..7ad8d42dba28 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -184,7 +184,7 @@ static void sti_atomic_complete(struct sti_private *private,
>         drm_atomic_helper_wait_for_vblanks(drm, state);
>
>         drm_atomic_helper_cleanup_planes(drm, state);
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>  }
>
>  static void sti_atomic_work(struct work_struct *work)
> @@ -217,6 +217,7 @@ static int sti_atomic_commit(struct drm_device *drm,
>
>         drm_atomic_helper_swap_state(state, true);
>
> +       drm_atomic_state_get(state);
>         if (nonblock)
>                 sti_atomic_schedule(private, state);
>         else
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 4b9f1c79cd7b..4ca8f2f3c46b 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -63,7 +63,7 @@ static void tegra_atomic_complete(struct tegra_drm *tegra,
>         drm_atomic_helper_wait_for_vblanks(drm, state);
>
>         drm_atomic_helper_cleanup_planes(drm, state);
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>  }
>
>  static void tegra_atomic_work(struct work_struct *work)
> @@ -96,6 +96,7 @@ static int tegra_atomic_commit(struct drm_device *drm,
>
>         drm_atomic_helper_swap_state(state, true);
>
> +       drm_atomic_state_get(state);
>         if (nonblock)
>                 tegra_atomic_schedule(tegra, state);
>         else
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index f8892e9ad169..19abf65f0ff3 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -143,8 +143,6 @@ static int tilcdc_commit(struct drm_device *dev,
>
>         drm_atomic_helper_cleanup_planes(dev, state);
>
> -       drm_atomic_state_free(state);
> -
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index c1f65c6c8e60..f31f72af8551 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -61,7 +61,7 @@ vc4_atomic_complete_commit(struct vc4_commit *c)
>
>         drm_atomic_helper_cleanup_planes(dev, state);
>
> -       drm_atomic_state_free(state);
> +       drm_atomic_state_put(state);
>
>         up(&vc4->async_modeset);
>
> @@ -173,6 +173,7 @@ static int vc4_atomic_commit(struct drm_device *dev,
>          * current layout.
>          */
>
> +       drm_atomic_state_get(state);
>         if (nonblock) {
>                 vc4_queue_seqno_cb(dev, &c->cb, wait_seqno,
>                                    vc4_atomic_complete_commit_seqno_cb);
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 856a9c85a838..1c6210e7a791 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -39,7 +39,33 @@ static inline void drm_crtc_commit_get(struct drm_crtc_commit *commit)
>  struct drm_atomic_state * __must_check
>  drm_atomic_state_alloc(struct drm_device *dev);
>  void drm_atomic_state_clear(struct drm_atomic_state *state);
> -void drm_atomic_state_free(struct drm_atomic_state *state);
> +
> +/**
> + * drm_atomic_state_get - acquire a reference to the atomic state
> + * @state: The atomic state
> + *
> + * Returns a new reference to the @state
> + */
> +static inline struct drm_atomic_state *
> +drm_atomic_state_get(struct drm_atomic_state *state)
> +{
> +       kref_get(&state->ref);
> +       return state;
> +}
> +
> +void __drm_atomic_state_free(struct kref *ref);
> +
> +/**
> + * drm_atomic_state_put - release a reference to the atomic state
> + * @state: The atomic state
> + *
> + * This releases a reference to @state which is freed after removing the
> + * final reference. No locking required and callable from any context.
> + */
> +static inline void drm_atomic_state_put(struct drm_atomic_state *state)
> +{
> +       kref_put(&state->ref, __drm_atomic_state_free);
> +}
>
>  int  __must_check
>  drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 35b13fc6bbc1..82ed80e97e8b 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1143,6 +1143,7 @@ struct __drm_connnectors_state {
>
>  /**
>   * struct drm_atomic_state - the global state object for atomic updates
> + * @ref: count of all references to this state
>   * @dev: parent DRM device
>   * @allow_modeset: allow full modeset
>   * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics
> @@ -1154,6 +1155,8 @@ struct __drm_connnectors_state {
>   * @acquire_ctx: acquire context for this atomic modeset state update
>   */
>  struct drm_atomic_state {
> +       struct kref ref;
> +
>         struct drm_device *dev;
>         bool allow_modeset : 1;
>         bool legacy_cursor_update : 1;
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


More information about the dri-devel mailing list