* [PATCH] drm: Add reference counting to drm_atomic_state
@ 2016-08-23 21:43 Chris Wilson
2016-08-24 0:29 ` kbuild test robot
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Chris Wilson @ 2016-08-23 21:43 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx
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.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +-
drivers/gpu/drm/drm_atomic.c | 23 +++----
drivers/gpu/drm/drm_atomic_helper.c | 98 +++++++---------------------
drivers/gpu/drm/drm_fb_helper.c | 8 +--
drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +-
drivers/gpu/drm/i915/i915_debugfs.c | 3 +-
drivers/gpu/drm/i915/intel_display.c | 25 +++----
drivers/gpu/drm/i915/intel_sprite.c | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +-
drivers/gpu/drm/msm/msm_atomic.c | 2 +-
drivers/gpu/drm/omapdrm/omap_drv.c | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
drivers/gpu/drm/sti/sti_drv.c | 2 +-
drivers/gpu/drm/tegra/drm.c | 2 +-
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
include/drm/drm_atomic.h | 12 +++-
include/drm/drm_crtc.h | 2 +
18 files changed, 73 insertions(+), 122 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..00f30fbd2336 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);
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
*
* 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 9abe0a242f96..14dbae947a12 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1187,7 +1187,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)
@@ -1263,6 +1263,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
@@ -1564,7 +1565,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.
@@ -2067,18 +2068,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);
@@ -2140,18 +2136,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);
@@ -2280,18 +2271,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);
@@ -2433,11 +2419,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);
@@ -2488,7 +2471,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;
}
@@ -2577,18 +2560,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);
@@ -2637,18 +2615,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);
@@ -2697,18 +2670,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);
@@ -2781,18 +2749,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);
@@ -2868,19 +2831,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);
@@ -3287,7 +3245,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);
}
@@ -3402,22 +3360,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 dd8e3b68fd53..3a98b51058d5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -365,9 +365,7 @@ fail:
if (ret == -EDEADLK)
goto backoff;
- if (ret != 0)
- drm_atomic_state_free(state);
-
+ drm_atomic_state_put(state);
return ret;
backoff:
@@ -1366,9 +1364,7 @@ fail:
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 877d2efa28e2..7ea75cb1fdad 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;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7d7b4d9280e9..6e0bd07039d3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3954,8 +3954,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 b10bea630a0d..7cdb3bba83a4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3577,7 +3577,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)
@@ -6885,7 +6885,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);
@@ -11268,11 +11268,13 @@ found:
/* let the connector get through one full cycle before testing */
intel_wait_for_vblank(dev, intel_crtc->pipe);
+ drm_atomic_state_put(state);
+ drm_atomic_state_put(restore_state);
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) {
@@ -11303,7 +11305,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
ret = drm_atomic_commit(state);
if (ret) {
DRM_DEBUG_KMS("Couldn't release load detect pipe: %i\n", ret);
- drm_atomic_state_free(state);
+ drm_atomic_state_put(state);
}
}
@@ -12375,8 +12377,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);
@@ -14381,7 +14382,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
@@ -14464,6 +14465,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
@@ -14505,9 +14507,8 @@ retry:
goto retry;
}
- if (ret)
out:
- drm_atomic_state_free(state);
+ drm_atomic_state_put(state);
}
#undef for_each_intel_crtc_masked
@@ -16246,7 +16247,7 @@ retry:
dev_priv->display.optimize_watermarks(cs);
}
- drm_atomic_state_free(state);
+ drm_atomic_state_put(state);
fail:
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
@@ -16881,7 +16882,7 @@ void intel_display_resume(struct drm_device *dev)
if (ret) {
DRM_ERROR("Restoring old state failed with %i\n", ret);
- drm_atomic_state_free(state);
+ drm_atomic_state_put(state);
}
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0df783a66bfb..be819d86beae 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -982,9 +982,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 0e769abd0c2c..64c60d2b9246 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -81,7 +81,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)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 4a8a6f1f1151..032211e90d66 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);
}
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 3dd78f2045f9..e29f7051bb2c 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);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index f03eb55318c1..6ac768f54123 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -263,7 +263,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);
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index f8311b2bc84e..a17e4e9a8037 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)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 755264d9db22..5a39ed4e817b 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -62,7 +62,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)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 3404d245d28d..6142d7039c95 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -129,7 +129,7 @@ static int tilcdc_commit(struct drm_device *dev,
drm_atomic_helper_cleanup_planes(dev, state);
- drm_atomic_state_free(state);
+ drm_atomic_state_put(state);
return 0;
}
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 4ac894d993cd..2329e7ddb378 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);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 856a9c85a838..68d2b5d17ed3 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -39,7 +39,17 @@ 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);
+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);
+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 3fa0275e509f..823df1bc9248 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1395,6 +1395,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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: Add reference counting to drm_atomic_state
2016-08-23 21:43 [PATCH] drm: Add reference counting to drm_atomic_state Chris Wilson
@ 2016-08-24 0:29 ` kbuild test robot
2016-08-24 3:59 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-08-24 6:10 ` [PATCH v2] " Chris Wilson
2 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2016-08-24 0:29 UTC (permalink / raw)
To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, kbuild-all, dri-devel
[-- Attachment #1: Type: text/plain, Size: 8326 bytes --]
Hi Chris,
[auto build test WARNING on drm-intel/for-linux-next]
[cannot apply to drm/drm-next v4.8-rc3 next-20160823]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-Add-reference-counting-to-drm_atomic_state/20160824-054553
base: git://anongit.freedesktop.org/drm-intel for-linux-next
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
drivers/gpu/drm/drm_modes.c:693: warning: No description found for parameter 'bus_flags'
>> drivers/gpu/drm/drm_atomic.c:228: warning: No description found for parameter 'ref'
>> drivers/gpu/drm/drm_atomic.c:228: warning: Excess function parameter 'state' description in '__drm_atomic_state_free'
>> drivers/gpu/drm/drm_atomic.c:228: warning: No description found for parameter 'ref'
>> drivers/gpu/drm/drm_atomic.c:228: warning: Excess function parameter 'state' description in '__drm_atomic_state_free'
>> include/drm/drm_crtc.h:2063: warning: No description found for parameter 'ref'
drivers/gpu/drm/drm_plane_helper.c:248: warning: No description found for parameter 'dst'
drivers/gpu/drm/drm_plane_helper.c:248: warning: Excess function parameter 'dest' description in 'drm_plane_helper_check_update'
drivers/gpu/drm/drm_plane_helper.c:247: warning: No description found for parameter 'dst'
drivers/gpu/drm/drm_plane_helper.c:247: warning: Excess function parameter 'dest' description in 'drm_plane_helper_check_update'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: Excess function parameter 'dev' description in 'intel_dp_set_drrs_state'
drivers/gpu/drm/i915/intel_dp.c:5158: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5188: warning: No description found for parameter 'old_crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: Excess function parameter 'dev' description in 'intel_dp_set_drrs_state'
drivers/gpu/drm/i915/intel_dp.c:5158: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5188: warning: No description found for parameter 'old_crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: Excess function parameter 'dev' description in 'intel_dp_set_drrs_state'
drivers/gpu/drm/i915/intel_dp.c:5158: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5188: warning: No description found for parameter 'old_crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: Excess function parameter 'dev' description in 'intel_dp_set_drrs_state'
drivers/gpu/drm/i915/intel_dp.c:5158: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5188: warning: No description found for parameter 'old_crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: Excess function parameter 'dev' description in 'intel_dp_set_drrs_state'
drivers/gpu/drm/i915/intel_dp.c:5158: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5188: warning: No description found for parameter 'old_crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: Excess function parameter 'dev' description in 'intel_dp_set_drrs_state'
drivers/gpu/drm/i915/intel_dp.c:5158: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5188: warning: No description found for parameter 'old_crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5063: warning: Excess function parameter 'dev' description in 'intel_dp_set_drrs_state'
drivers/gpu/drm/i915/intel_dp.c:5158: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_dp.c:5188: warning: No description found for parameter 'old_crtc_state'
drivers/gpu/drm/drm_crtc.c:1270: WARNING: Inline literal start-string without end-string.
drivers/gpu/drm/drm_crtc.c:1385: WARNING: Inline literal start-string without end-string.
include/drm/drm_crtc.h:1202: WARNING: Inline literal start-string without end-string.
include/drm/drm_crtc.h:1255: WARNING: Inline literal start-string without end-string.
include/drm/drm_crtc.h:1268: WARNING: Inline literal start-string without end-string.
include/drm/drm_crtc.h:1272: WARNING: Inline literal start-string without end-string.
drivers/gpu/drm/drm_irq.c:718: WARNING: Option list ends without a blank line; unexpected unindent.
drivers/gpu/drm/drm_fb_helper.c:2191: WARNING: Inline emphasis start-string without end-string.
drivers/gpu/drm/drm_simple_kms_helper.c:141: WARNING: Inline literal start-string without end-string.
include/drm/drm_gem.h:212: WARNING: Inline emphasis start-string without end-string.
drivers/gpu/drm/i915/i915_vgpu.c:179: WARNING: Literal block ends without a blank line; unexpected unindent.
drivers/gpu/drm/i915/intel_audio.c:54: WARNING: Inline emphasis start-string without end-string.
drivers/gpu/drm/i915/intel_audio.c:54: WARNING: Inline emphasis start-string without end-string.
drivers/gpu/drm/i915/intel_guc_fwif.h:159: WARNING: Block quote ends without a blank line; unexpected unindent.
drivers/gpu/drm/i915/intel_guc_fwif.h:178: WARNING: Enumerated list ends without a blank line; unexpected unindent.
Documentation/gpu/drm-kms.rst:13: WARNING: Could not lex literal_block as "C". Highlighting skipped.
Documentation/gpu/drm-kms-helpers.rst:16: WARNING: Could not lex literal_block as "C". Highlighting skipped.
Documentation/gpu/i915.rst:57: WARNING: Could not lex literal_block as "C". Highlighting skipped.
vim +/ref +228 drivers/gpu/drm/drm_atomic.c
212 if (config->funcs->atomic_state_clear)
213 config->funcs->atomic_state_clear(state);
214 else
215 drm_atomic_state_default_clear(state);
216 }
217 EXPORT_SYMBOL(drm_atomic_state_clear);
218
219 /**
220 * __drm_atomic_state_free - free all memory for an atomic state
221 * @state: atomic state to deallocate
222 *
223 * This frees all memory associated with an atomic state, including all the
224 * per-object state for planes, crtcs and connectors.
225 */
226 void __drm_atomic_state_free(struct kref *ref)
227 {
> 228 struct drm_atomic_state *state = container_of(ref, typeof(*state), ref);
229 struct drm_mode_config *config = &state->dev->mode_config;
230
231 drm_atomic_state_clear(state);
232
233 DRM_DEBUG_ATOMIC("Freeing atomic state %p\n", state);
234
235 if (config->funcs->atomic_state_free) {
236 config->funcs->atomic_state_free(state);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6422 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* ✗ Fi.CI.BAT: failure for drm: Add reference counting to drm_atomic_state
2016-08-23 21:43 [PATCH] drm: Add reference counting to drm_atomic_state Chris Wilson
2016-08-24 0:29 ` kbuild test robot
@ 2016-08-24 3:59 ` Patchwork
2016-08-24 6:10 ` [PATCH v2] " Chris Wilson
2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2016-08-24 3:59 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm: Add reference counting to drm_atomic_state
URL : https://patchwork.freedesktop.org/series/11494/
State : failure
== Summary ==
Series 11494v1 drm: Add reference counting to drm_atomic_state
http://patchwork.freedesktop.org/api/1.0/series/11494/revisions/1/mbox/
Test kms_cursor_legacy:
Subgroup basic-flip-vs-cursor-legacy:
pass -> FAIL (fi-skl-6700k)
Subgroup basic-flip-vs-cursor-varying-size:
pass -> FAIL (fi-skl-6700k)
fail -> PASS (fi-hsw-4770r)
Test kms_force_connector_basic:
Subgroup force-load-detect:
pass -> INCOMPLETE (fi-hsw-4770k)
pass -> INCOMPLETE (fi-ivb-3520m)
pass -> INCOMPLETE (fi-snb-2600)
pass -> INCOMPLETE (fi-snb-2520m)
fi-bdw-5557u total:249 pass:232 dwarn:0 dfail:0 fail:1 skip:16
fi-bsw-n3050 total:249 pass:202 dwarn:0 dfail:0 fail:3 skip:44
fi-byt-n2820 total:249 pass:204 dwarn:0 dfail:0 fail:4 skip:41
fi-hsw-4770k total:25 pass:22 dwarn:0 dfail:0 fail:0 skip:2
fi-hsw-4770r total:249 pass:222 dwarn:0 dfail:0 fail:1 skip:26
fi-ivb-3520m total:25 pass:21 dwarn:0 dfail:0 fail:0 skip:3
fi-skl-6260u total:249 pass:234 dwarn:0 dfail:0 fail:1 skip:14
fi-skl-6700k total:249 pass:214 dwarn:4 dfail:0 fail:3 skip:28
fi-snb-2520m total:25 pass:20 dwarn:0 dfail:0 fail:0 skip:4
fi-snb-2600 total:25 pass:20 dwarn:0 dfail:0 fail:0 skip:4
Results at /archive/results/CI_IGT_test/Patchwork_2415/
74607c2ff95614c1ace6a7207a2960ecc1cec95c drm-intel-nightly: 2016y-08m-23d-19h-46m-16s UTC integration manifest
a87e70f drm: Add reference counting to drm_atomic_state
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] drm: Add reference counting to drm_atomic_state
2016-08-23 21:43 [PATCH] drm: Add reference counting to drm_atomic_state Chris Wilson
2016-08-24 0:29 ` kbuild test robot
2016-08-24 3:59 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-08-24 6:10 ` Chris Wilson
2016-08-24 9:17 ` Eric Engestrom
2 siblings, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2016-08-24 6:10 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx
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@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
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
*
* 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 9abe0a242f96..14dbae947a12 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1187,7 +1187,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)
@@ -1263,6 +1263,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
@@ -1564,7 +1565,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.
@@ -2067,18 +2068,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);
@@ -2140,18 +2136,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);
@@ -2280,18 +2271,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);
@@ -2433,11 +2419,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);
@@ -2488,7 +2471,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;
}
@@ -2577,18 +2560,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);
@@ -2637,18 +2615,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);
@@ -2697,18 +2670,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);
@@ -2781,18 +2749,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);
@@ -2868,19 +2831,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);
@@ -3287,7 +3245,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);
}
@@ -3402,22 +3360,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 dd8e3b68fd53..05a711ecf97a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -365,9 +365,7 @@ fail:
if (ret == -EDEADLK)
goto backoff;
- if (ret != 0)
- drm_atomic_state_free(state);
-
+ drm_atomic_state_put(state);
return ret;
backoff:
@@ -1359,16 +1357,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 877d2efa28e2..6a4a08ea293d 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 7d7b4d9280e9..6e0bd07039d3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3954,8 +3954,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 b10bea630a0d..d61c264d5a37 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3577,7 +3577,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)
@@ -3638,6 +3638,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
intel_hpd_init(dev_priv);
}
+ drm_atomic_state_put(state);
drm_modeset_drop_locks(ctx);
drm_modeset_acquire_fini(ctx);
mutex_unlock(&dev->mode_config.mutex);
@@ -6885,7 +6886,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);
@@ -11268,11 +11269,13 @@ found:
/* let the connector get through one full cycle before testing */
intel_wait_for_vblank(dev, intel_crtc->pipe);
+ drm_atomic_state_put(state);
+ drm_atomic_state_put(restore_state);
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) {
@@ -11301,10 +11304,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,
@@ -12375,8 +12377,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);
@@ -14381,7 +14382,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
@@ -14464,6 +14465,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
@@ -14505,9 +14507,8 @@ retry:
goto retry;
}
- if (ret)
out:
- drm_atomic_state_free(state);
+ drm_atomic_state_put(state);
}
#undef for_each_intel_crtc_masked
@@ -16246,8 +16247,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);
}
@@ -16879,10 +16880,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 0df783a66bfb..be819d86beae 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -982,9 +982,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 0e769abd0c2c..0e8fe8cab2b4 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -81,7 +81,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)
@@ -108,6 +108,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 4a8a6f1f1151..9b2ceaa987be 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 3dd78f2045f9..5726077cbe28 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 f03eb55318c1..ba84548d083e 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -263,7 +263,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);
@@ -329,6 +329,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 f8311b2bc84e..4f1c366ca18d 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 755264d9db22..b8623faa447e 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -62,7 +62,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)
@@ -95,6 +95,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 3404d245d28d..d5d88085dde9 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -129,8 +129,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 4ac894d993cd..3815894aafa0 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..68d2b5d17ed3 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -39,7 +39,17 @@ 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);
+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);
+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 3fa0275e509f..4c7b56381989 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1384,6 +1384,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
@@ -1395,6 +1396,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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm: Add reference counting to drm_atomic_state
2016-08-24 6:10 ` [PATCH v2] " Chris Wilson
@ 2016-08-24 9:17 ` Eric Engestrom
0 siblings, 0 replies; 5+ messages in thread
From: Eric Engestrom @ 2016-08-24 9:17 UTC (permalink / raw)
To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, dri-devel
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@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> 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@imgtec.com>
> *
> * 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]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-08-24 9:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-23 21:43 [PATCH] drm: Add reference counting to drm_atomic_state Chris Wilson
2016-08-24 0:29 ` kbuild test robot
2016-08-24 3:59 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-08-24 6:10 ` [PATCH v2] " Chris Wilson
2016-08-24 9:17 ` Eric Engestrom
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.