On Wed, Aug 24, 2016 at 07:10:15AM +0100, Chris Wilson 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
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 3 +-
> drivers/gpu/drm/drm_atomic.c | 23 +++----
> 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 | 32 ++++-----
> 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 | 12 +++-
> include/drm/drm_crtc.h | 3 +
> 18 files changed, 85 insertions(+), 128 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index d4a3d61b7b06..15a9f9d3ef9a 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 5cb2e22d5d55..349c2f0de900 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
> + * __drm_atomic_state_free - free all memory for an atomic state
> * @state: atomic state to deallocate
Doc line needs updating as well.
Other than that, this looks good to me:
Reviewed-by: Eric Engestrom <eric.engestrom@xxxxxxxxxx>
> *
> * 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)
> {
[snip]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
