[PATCH v3 21/22] drm/atomic: Introduce drm_atomic_helper_duplicate_commited_state()
ville.syrjala at linux.intel.com
ville.syrjala at linux.intel.com
Thu Jul 6 20:24:41 UTC 2017
From: Ville Syrjälä <ville.syrjala at linux.intel.com>
For i915 GPU reset handling we'll want to be able to duplicate the state
that was last commited to the hardware. For that purpose let's start to
track the commited state for each object and provide a way to duplicate
the commmited state into a new drm_atomic_state. The locking for
.commited_state must to be provided by the driver.
drm_atomic_helper_duplicate_commited_state() duplicates the state
to both old_state and new_state. For the purposes of i915 GPU reset we
would only need one of them, but we actually need two top level states;
one for disabling everything (which would need the duplicated state to
be old_state), and another to reenable everything (which would need the
duplicated state to be new_state). So to make it less comples I figured
I'd just always duplicate both. Might want to rethink this if for no
other reason that reducing the chances of memory allocation failure.
Due to the double state duplication we need
drm_atomic_helper_clean_commited_state() to clean up the duplicated
old_state since that's not handled by the normal drm_atomic_state
cleanup code.
TODO: do we want this in the helper, or maybe it should be just in i915?
v2: s/commited/committed/ everywhere (checkpatch)
Handle state duplication errors better
v3: Even more care in dealing with memory allocation errors
Handle private objs too
Deal with the potential ordering issues between swap_state()
and hw_done() by keeping track of which state was swapped in
last
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 1 +
drivers/gpu/drm/drm_atomic_helper.c | 231 ++++++++++++++++++++++++++++++++++++
include/drm/drm_atomic.h | 4 +
include/drm/drm_atomic_helper.h | 8 ++
include/drm/drm_connector.h | 11 ++
include/drm/drm_crtc.h | 11 ++
include/drm/drm_plane.h | 11 ++
7 files changed, 277 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 56925b93f598..e1578d50d66f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1020,6 +1020,7 @@ drm_atomic_private_obj_init(struct drm_private_obj *obj,
memset(obj, 0, sizeof(*obj));
obj->state = state;
+ obj->committed_state = state;
obj->funcs = funcs;
}
EXPORT_SYMBOL(drm_atomic_private_obj_init);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index f0887f231fb8..c3d02f12cd5d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1815,6 +1815,11 @@ void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state)
}
EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies);
+static bool state_seqno_after(unsigned int a, unsigned int b)
+{
+ return (int)(b - a) < 0;
+}
+
/**
* drm_atomic_helper_commit_hw_done - setup possible nonblocking commit
* @old_state: atomic state object with old state structures
@@ -1833,11 +1838,39 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies);
void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
{
struct drm_crtc *crtc;
+ struct drm_plane *plane;
+ struct drm_connector *connector;
+ struct drm_private_obj *obj;
struct drm_crtc_state *new_crtc_state;
+ struct drm_plane_state *new_plane_state;
+ struct drm_connector_state *new_connector_state;
+ struct drm_private_state *new_obj_state;
struct drm_crtc_commit *commit;
int i;
+ static DEFINE_SPINLOCK(committed_state_lock);
+
+ spin_lock(&committed_state_lock);
+
+ for_each_new_plane_in_state(old_state, plane, new_plane_state, i) {
+ if (plane->committed_state &&
+ state_seqno_after(new_plane_state->seqno,
+ plane->committed_state->seqno))
+ plane->committed_state = new_plane_state;
+ }
+
+ for_each_new_connector_in_state(old_state, connector, new_connector_state, i) {
+ if (connector->committed_state &&
+ state_seqno_after(new_connector_state->seqno,
+ connector->committed_state->seqno))
+ connector->committed_state = new_connector_state;
+ }
for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
+ if (crtc->committed_state &&
+ state_seqno_after(new_crtc_state->seqno,
+ crtc->committed_state->seqno))
+ crtc->committed_state = new_crtc_state;
+
commit = old_state->crtcs[i].commit;
if (!commit)
continue;
@@ -1846,6 +1879,15 @@ void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state)
WARN_ON(new_crtc_state->event);
complete_all(&commit->hw_done);
}
+
+ for_each_new_private_obj_in_state(old_state, obj, new_obj_state, i) {
+ if (obj->committed_state &&
+ state_seqno_after(new_obj_state->seqno,
+ obj->committed_state->seqno))
+ obj->committed_state = new_obj_state;
+ }
+
+ spin_unlock(&committed_state_lock);
}
EXPORT_SYMBOL(drm_atomic_helper_commit_hw_done);
@@ -2296,6 +2338,7 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
old_conn_state->state = state;
new_conn_state->state = NULL;
+ new_conn_state->seqno = ++connector->state_seqno;
__drm_atomic_state_connector(state, i)->state = old_conn_state;
connector->state = new_conn_state;
@@ -2309,6 +2352,7 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
state->crtcs[i].state = old_crtc_state;
crtc->state = new_crtc_state;
+ new_crtc_state->seqno = ++crtc->state_seqno;
if (state->crtcs[i].commit) {
spin_lock(&crtc->commit_lock);
@@ -2325,6 +2369,7 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
old_plane_state->state = state;
new_plane_state->state = NULL;
+ new_plane_state->seqno = ++plane->state_seqno;
state->planes[i].state = old_plane_state;
plane->state = new_plane_state;
@@ -2335,6 +2380,7 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
old_obj_state->state = state;
new_obj_state->state = NULL;
+ new_obj_state->seqno = ++obj->state_seqno;
__drm_atomic_state_private_obj(state, i)->state = old_obj_state;
obj->state = new_obj_state;
@@ -3582,6 +3628,7 @@ __drm_atomic_helper_connector_reset(struct drm_connector *connector,
conn_state->connector = connector;
connector->state = conn_state;
+ connector->committed_state = conn_state;
}
EXPORT_SYMBOL(__drm_atomic_helper_connector_reset);
@@ -3740,6 +3787,189 @@ drm_atomic_helper_duplicate_state(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_atomic_helper_duplicate_state);
+struct drm_atomic_state *
+drm_atomic_helper_duplicate_committed_state(struct drm_device *dev)
+{
+ struct drm_atomic_state *state;
+ struct drm_connector_list_iter conn_iter;
+ struct drm_connector *conn;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ int err = 0;
+
+ state = drm_atomic_state_alloc(dev);
+ if (!state)
+ return ERR_PTR(-ENOMEM);
+
+ drm_for_each_plane(plane, dev) {
+ struct drm_plane_state *old_state, *new_state;
+ int i = drm_plane_index(plane);
+
+ old_state = plane->funcs->atomic_duplicate_state(plane, plane->committed_state);
+ if (!old_state) {
+ err = -ENOMEM;
+ goto free;
+ }
+ new_state = plane->funcs->atomic_duplicate_state(plane, plane->committed_state);
+ if (!new_state) {
+ plane->funcs->atomic_destroy_state(plane, old_state);
+ err = -ENOMEM;
+ goto free;
+ }
+
+ state->planes[i].state = new_state;
+ state->planes[i].old_state = old_state;
+ state->planes[i].new_state = new_state;
+ state->planes[i].ptr = plane;
+
+ old_state->state = state;
+ }
+
+ drm_for_each_crtc(crtc, dev) {
+ struct drm_crtc_state *old_state, *new_state;
+ int i = drm_crtc_index(crtc);
+
+ old_state = crtc->funcs->atomic_duplicate_state(crtc, crtc->committed_state);
+ if (!old_state) {
+ err = -ENOMEM;
+ goto free;
+ }
+ new_state = crtc->funcs->atomic_duplicate_state(crtc, crtc->committed_state);
+ if (!new_state) {
+ crtc->funcs->atomic_destroy_state(crtc, old_state);
+ err = -ENOMEM;
+ goto free;
+ }
+
+ state->crtcs[i].state = new_state;
+ state->crtcs[i].old_state = old_state;
+ state->crtcs[i].new_state = new_state;
+ state->crtcs[i].ptr = crtc;
+
+ old_state->state = state;
+ }
+
+ drm_connector_list_iter_begin(dev, &conn_iter);
+ drm_for_each_connector_iter(conn, &conn_iter) {
+ struct drm_connector_state *old_state, *new_state;
+ struct __drm_connectors_state *c;
+ int i = drm_connector_index(conn);
+
+ err = drm_dynarray_reserve(&state->connectors, i);
+ if (err)
+ break;
+
+ old_state = conn->funcs->atomic_duplicate_state(conn, conn->committed_state);
+ if (!old_state) {
+ err = -ENOMEM;
+ break;
+ }
+ new_state = conn->funcs->atomic_duplicate_state(conn, conn->committed_state);
+ if (!new_state) {
+ conn->funcs->atomic_destroy_state(conn, old_state);
+ err = -ENOMEM;
+ break;
+ }
+
+ state->num_connector = state->connectors.num_elems;
+
+ c = __drm_atomic_state_connector(state, i);
+
+ c->state = new_state;
+ c->old_state = old_state;
+ c->new_state = new_state;
+ c->ptr = drm_connector_get(conn);
+
+ old_state->state = state;
+ }
+ drm_connector_list_iter_end(&conn_iter);
+
+free:
+ if (err < 0) {
+ drm_atomic_helper_clean_committed_state(state);
+ drm_atomic_state_put(state);
+ state = ERR_PTR(err);
+ }
+
+ return state;
+}
+EXPORT_SYMBOL(drm_atomic_helper_duplicate_committed_state);
+
+int drm_atomic_helper_duplicate_private_obj_committed_state(struct drm_atomic_state *state,
+ struct drm_private_obj *obj)
+{
+ struct drm_private_state *old_state, *new_state;
+ struct __drm_private_objs_state *p;
+ int ret, i = state->num_private_objs;
+
+ ret = drm_dynarray_reserve(&state->private_objs, i);
+ if (ret)
+ return ret;
+
+ old_state = obj->funcs->atomic_duplicate_state(obj, obj->committed_state);
+ if (!old_state)
+ return -ENOMEM;
+
+ new_state = obj->funcs->atomic_duplicate_state(obj, obj->committed_state);
+ if (!new_state) {
+ obj->funcs->atomic_destroy_state(obj, obj->committed_state);
+ return -ENOMEM;
+ }
+
+ state->num_private_objs = state->private_objs.num_elems;
+
+ p = __drm_atomic_state_private_obj(state, i);
+
+ p->state = new_state;
+ p->old_state = old_state;
+ p->new_state = new_state;
+ p->ptr = obj;
+
+ old_state->state = state;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_duplicate_private_obj_committed_state);
+
+void
+drm_atomic_helper_clean_committed_state(struct drm_atomic_state *state)
+{
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_connector *conn;
+ struct drm_plane_state *plane_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_connector_state *conn_state;
+ struct drm_private_obj *obj;
+ struct drm_private_state *obj_state;
+ int i;
+
+ /* restore the correct state->state */
+ for_each_new_plane_in_state(state, plane, plane_state, i) {
+ plane->funcs->atomic_destroy_state(plane, state->planes[i].old_state);
+ state->planes[i].old_state = NULL;
+ }
+ for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+ crtc->funcs->atomic_destroy_state(crtc, state->crtcs[i].old_state);
+ state->crtcs[i].old_state = NULL;
+ }
+ for_each_new_connector_in_state(state, conn, conn_state, i) {
+ struct __drm_connectors_state *c =
+ __drm_atomic_state_connector(state, i);
+
+ conn->funcs->atomic_destroy_state(conn, c->old_state);
+ c->old_state = NULL;
+ }
+ for_each_new_private_obj_in_state(state, obj, obj_state, i) {
+ struct __drm_private_objs_state *p =
+ __drm_atomic_state_private_obj(state, i);
+
+ obj->funcs->atomic_destroy_state(obj, p->old_state);
+ p->old_state = NULL;
+ }
+}
+EXPORT_SYMBOL(drm_atomic_helper_clean_committed_state);
+
/**
* __drm_atomic_helper_connector_destroy_state - release connector state
* @state: connector state object to release
@@ -3856,6 +4086,7 @@ EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set);
* __drm_atomic_helper_private_duplicate_state - copy atomic private state
* @obj: CRTC object
* @state: new private object state
+ * @old_state: old private object state
*
* Copies atomic state from a private objects's current state and resets inferred values.
* This is useful for drivers that subclass the private state.
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 0e6c54b3e0af..9ff4fb46d500 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -194,12 +194,16 @@ struct drm_private_state_funcs {
struct drm_private_obj {
struct drm_private_state *state;
+ struct drm_private_state *committed_state;
const struct drm_private_state_funcs *funcs;
+
+ unsigned int state_seqno;
};
struct drm_private_state {
struct drm_atomic_state *state;
+ unsigned int seqno;
};
struct __drm_private_objs_state {
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index b799687a4b09..3a609bbb5818 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -185,6 +185,14 @@ drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
struct drm_atomic_state *
drm_atomic_helper_duplicate_state(struct drm_device *dev,
struct drm_modeset_acquire_ctx *ctx);
+struct drm_atomic_state *
+drm_atomic_helper_duplicate_committed_state(struct drm_device *dev);
+struct drm_private_obj;
+int
+drm_atomic_helper_duplicate_private_obj_committed_state(struct drm_atomic_state *state,
+ struct drm_private_obj *obj);
+void
+drm_atomic_helper_clean_committed_state(struct drm_atomic_state *state);
void
__drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state);
void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8f26166f78b4..a38ba05db4fa 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -306,6 +306,8 @@ struct drm_tv_connector_state {
struct drm_connector_state {
struct drm_connector *connector;
+ unsigned int seqno;
+
/**
* @crtc: CRTC to connect connector to, NULL if disabled.
*
@@ -707,6 +709,8 @@ struct drm_connector {
char *name;
+ unsigned int state_seqno;
+
/**
* @mutex: Lock for general connector state, but currently only protects
* @registered. Most of the connector state is still protected by
@@ -868,6 +872,13 @@ struct drm_connector {
*/
struct drm_connector_state *state;
+ /**
+ * @committed_state:
+ *
+ * Current committed atomic state for this connector.
+ */
+ struct drm_connector_state *committed_state;
+
/* DisplayID bits */
bool has_tile;
struct drm_tile_group *tile_group;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8bfbc54660ab..2a1897d76c71 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -108,6 +108,8 @@ struct drm_plane_helper_funcs;
struct drm_crtc_state {
struct drm_crtc *crtc;
+ unsigned int seqno;
+
bool enable;
bool active;
@@ -750,6 +752,8 @@ struct drm_crtc {
char *name;
+ unsigned int state_seqno;
+
/**
* @mutex:
*
@@ -816,6 +820,13 @@ struct drm_crtc {
struct drm_crtc_state *state;
/**
+ * @committed_state:
+ *
+ * Current committed atomic state for this CRTC.
+ */
+ struct drm_crtc_state *committed_state;
+
+ /**
* @commit_list:
*
* List of &drm_crtc_commit structures tracking pending commits.
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 08ad4b58adbe..ff3495705e63 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -59,6 +59,8 @@ struct drm_modeset_acquire_ctx;
struct drm_plane_state {
struct drm_plane *plane;
+ unsigned int seqno;
+
/**
* @crtc:
*
@@ -470,6 +472,8 @@ struct drm_plane {
char *name;
+ unsigned int state_seqno;
+
/**
* @mutex:
*
@@ -522,6 +526,13 @@ struct drm_plane {
*/
struct drm_plane_state *state;
+ /**
+ * @committed_state:
+ *
+ * Current committed atomic state for this plane.
+ */
+ struct drm_plane_state *committed_state;
+
struct drm_property *zpos_property;
struct drm_property *rotation_property;
};
--
2.13.0
More information about the dri-devel
mailing list