Skip to content

Commit

Permalink
drm: Add reference counting to drm_atomic_state
Browse files Browse the repository at this point in the history 
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@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20161014121833.439-27-chris@chris-wilson.co.uk
  • Loading branch information
Chris Wilson authored and Daniel Vetter committed Oct 17, 2016
1 parent 1dfdb0e commit 0853695
Show file tree
Hide file tree
Showing 18 changed files with 102 additions and 131 deletions.
3 changes: 2 additions & 1 deletion drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
25 changes: 10 additions & 15 deletions drivers/gpu/drm/drm_atomic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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?
*/
Expand Down Expand Up @@ -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);

Expand All @@ -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
Expand Down Expand Up @@ -1742,7 +1738,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
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) {
Expand Down Expand Up @@ -1775,8 +1771,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
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);
Expand Down
98 changes: 24 additions & 74 deletions drivers/gpu/drm/drm_atomic_helper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,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)
Expand Down Expand Up @@ -1290,6 +1290,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
Expand Down Expand Up @@ -1591,7 +1592,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.
Expand Down Expand Up @@ -2114,18 +2115,13 @@ int drm_atomic_helper_update_plane(struct drm_plane *plane,
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);
Expand Down Expand Up @@ -2187,18 +2183,13 @@ int drm_atomic_helper_disable_plane(struct drm_plane *plane)
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);
Expand Down Expand Up @@ -2327,18 +2318,13 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set)
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);
Expand Down Expand Up @@ -2480,11 +2466,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);
Expand Down Expand Up @@ -2535,7 +2518,7 @@ struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)

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;
}
Expand Down Expand Up @@ -2624,18 +2607,13 @@ drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc,
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);
Expand Down Expand Up @@ -2684,18 +2662,13 @@ drm_atomic_helper_plane_set_property(struct drm_plane *plane,
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);
Expand Down Expand Up @@ -2744,18 +2717,13 @@ drm_atomic_helper_connector_set_property(struct drm_connector *connector,
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);
Expand Down Expand Up @@ -2828,18 +2796,13 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
}

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);
Expand Down Expand Up @@ -2915,19 +2878,14 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector,
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);
Expand Down Expand Up @@ -3334,7 +3292,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);
}

Expand Down Expand Up @@ -3449,22 +3407,14 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
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);
Expand Down
9 changes: 2 additions & 7 deletions drivers/gpu/drm/drm_fb_helper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,9 +367,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
if (ret == -EDEADLK)
goto backoff;

if (ret != 0)
drm_atomic_state_free(state);

drm_atomic_state_put(state);
return ret;

backoff:
Expand Down Expand Up @@ -1361,16 +1359,13 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
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:
Expand Down
3 changes: 2 additions & 1 deletion drivers/gpu/drm/exynos/exynos_drm_drv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,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;
Expand Down Expand Up @@ -254,6 +254,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
Expand Down
5 changes: 2 additions & 3 deletions drivers/gpu/drm/i915/i915_debugfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3941,10 +3941,9 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_i915_private *dev_priv,

ret = drm_atomic_commit(state);
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_modeset_unlock_all(dev);
drm_atomic_state_put(state);
}

static int ivb_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
Expand Down
Loading

0 comments on commit 0853695

Please sign in to comment.