* [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state.
@ 2017-02-16 14:47 Maarten Lankhorst
2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
Fifth iteration. Instead of trying to convert all drivers straight away,
implement all macros that are required to get state working.
Old situation:
Use obj->state, which can refer to old or new state.
Use drm_atomic_get_(existing_)obj_state, which can refer to new or old state.
Use for_each_obj_in_state, which refers to new or old state.
New situation:
During atomic check:
- Use drm_atomic_get_obj_state to add a object to the atomic state,
or get the new state.
- Use drm_atomic_get_(old/new)_obj_state to peek at the new/old state,
without adding the object. This will return NULL if the object is
not part of the state. For planes and connectors the relevant crtc_state
is added, so this will work to get the crtc_state from obj_state->crtc
too, this means not having to write some error handling.
During atomic commit:
- Do not use drm_atomic_get_obj_state, obj->state or drm_atomic_get_(existing_)obj_state
any more, replace with drm_atomic_get_old/new_obj_state calls as required.
During both:
- Use for_each_(new,old,oldnew)_obj_in_state to get the old or new state as needed.
oldnew will be renamed to for_each_obj_in_state after all callers are converted
to the new api.
When not doing atomic updates:
Look at obj->state for now. I have some patches to fix this but I was asked to
make it return a const state. This breaks a lot of users though so I skipped that
patch in this iteration.
This series will return the correct state regardless of swapping.
Maarten Lankhorst (5):
drm/atomic: Fix atomic helpers to use the new iterator macros, v2.
drm/atomic: Make drm_atomic_plane_disabling easier to understand.
drm/atomic: Add macros to access existing old/new state, v2.
drm/atomic: Convert get_existing_state callers to get_old/new_state,
v3.
drm/blend: Use new atomic iterator macros.
drivers/gpu/drm/drm_atomic.c | 6 +-
drivers/gpu/drm/drm_atomic_helper.c | 469 ++++++++++++++++----------------
drivers/gpu/drm/drm_blend.c | 23 +-
drivers/gpu/drm/drm_plane_helper.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 4 +-
include/drm/drm_atomic.h | 108 ++++++++
include/drm/drm_atomic_helper.h | 26 +-
7 files changed, 375 insertions(+), 263 deletions(-)
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2. 2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst @ 2017-02-16 14:47 ` Maarten Lankhorst 2017-03-01 0:49 ` Laurent Pinchart 2017-02-16 14:47 ` [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand Maarten Lankhorst ` (3 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx There are new iterator macros that annotate whether the new or old state should be used. This is better than using a state that depends on whether it's called before or after swap. For clarity, also rename the variables from $obj_state to (old,new)_$obj_state as well. Changes since v1: - Use old/new_*_state for variable names as much as possible. (pinchartl) - Expand commit message. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/drm_atomic_helper.c | 431 +++++++++++++++++++----------------- 1 file changed, 222 insertions(+), 209 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 9203f3e933f7..7d432d9a18cf 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -63,14 +63,15 @@ */ static void drm_atomic_helper_plane_changed(struct drm_atomic_state *state, + struct drm_plane_state *old_plane_state, struct drm_plane_state *plane_state, struct drm_plane *plane) { struct drm_crtc_state *crtc_state; - if (plane->state->crtc) { + if (old_plane_state->crtc) { crtc_state = drm_atomic_get_existing_crtc_state(state, - plane->state->crtc); + old_plane_state->crtc); if (WARN_ON(!crtc_state)) return; @@ -92,7 +93,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, static int handle_conflicting_encoders(struct drm_atomic_state *state, bool disable_conflicting_encoders) { - struct drm_connector_state *conn_state; + struct drm_connector_state *new_conn_state; struct drm_connector *connector; struct drm_connector_list_iter conn_iter; struct drm_encoder *encoder; @@ -104,15 +105,15 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state, * part of the state. If the same encoder is assigned to multiple * connectors bail out. */ - for_each_connector_in_state(state, connector, conn_state, i) { + for_each_new_connector_in_state(state, connector, new_conn_state, i) { const struct drm_connector_helper_funcs *funcs = connector->helper_private; struct drm_encoder *new_encoder; - if (!conn_state->crtc) + if (!new_conn_state->crtc) continue; if (funcs->atomic_best_encoder) - new_encoder = funcs->atomic_best_encoder(connector, conn_state); + new_encoder = funcs->atomic_best_encoder(connector, new_conn_state); else if (funcs->best_encoder) new_encoder = funcs->best_encoder(connector); else @@ -166,20 +167,20 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state, goto out; } - conn_state = drm_atomic_get_connector_state(state, connector); - if (IS_ERR(conn_state)) { - ret = PTR_ERR(conn_state); + new_conn_state = drm_atomic_get_connector_state(state, connector); + if (IS_ERR(new_conn_state)) { + ret = PTR_ERR(new_conn_state); goto out; } DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], disabling [CONNECTOR:%d:%s]\n", encoder->base.id, encoder->name, - conn_state->crtc->base.id, conn_state->crtc->name, + new_conn_state->crtc->base.id, new_conn_state->crtc->name, connector->base.id, connector->name); - crtc_state = drm_atomic_get_existing_crtc_state(state, conn_state->crtc); + crtc_state = drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); - ret = drm_atomic_set_crtc_for_connector(conn_state, NULL); + ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL); if (ret) goto out; @@ -245,22 +246,22 @@ steal_encoder(struct drm_atomic_state *state, { struct drm_crtc_state *crtc_state; struct drm_connector *connector; - struct drm_connector_state *connector_state; + struct drm_connector_state *old_connector_state, *new_connector_state; int i; - for_each_connector_in_state(state, connector, connector_state, i) { + for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) { struct drm_crtc *encoder_crtc; - if (connector_state->best_encoder != encoder) + if (new_connector_state->best_encoder != encoder) continue; - encoder_crtc = connector->state->crtc; + encoder_crtc = old_connector_state->crtc; DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing it\n", encoder->base.id, encoder->name, encoder_crtc->base.id, encoder_crtc->name); - set_best_encoder(state, connector_state, NULL); + set_best_encoder(state, new_connector_state, NULL); crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc); crtc_state->connectors_changed = true; @@ -272,7 +273,8 @@ steal_encoder(struct drm_atomic_state *state, static int update_connector_routing(struct drm_atomic_state *state, struct drm_connector *connector, - struct drm_connector_state *connector_state) + struct drm_connector_state *old_connector_state, + struct drm_connector_state *new_connector_state) { const struct drm_connector_helper_funcs *funcs; struct drm_encoder *new_encoder; @@ -282,24 +284,24 @@ update_connector_routing(struct drm_atomic_state *state, connector->base.id, connector->name); - if (connector->state->crtc != connector_state->crtc) { - if (connector->state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, connector->state->crtc); + if (old_connector_state->crtc != new_connector_state->crtc) { + if (old_connector_state->crtc) { + crtc_state = drm_atomic_get_existing_crtc_state(state, old_connector_state->crtc); crtc_state->connectors_changed = true; } - if (connector_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc); + if (new_connector_state->crtc) { + crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc); crtc_state->connectors_changed = true; } } - if (!connector_state->crtc) { + if (!new_connector_state->crtc) { DRM_DEBUG_ATOMIC("Disabling [CONNECTOR:%d:%s]\n", connector->base.id, connector->name); - set_best_encoder(state, connector_state, NULL); + set_best_encoder(state, new_connector_state, NULL); return 0; } @@ -308,7 +310,7 @@ update_connector_routing(struct drm_atomic_state *state, if (funcs->atomic_best_encoder) new_encoder = funcs->atomic_best_encoder(connector, - connector_state); + new_connector_state); else if (funcs->best_encoder) new_encoder = funcs->best_encoder(connector); else @@ -321,34 +323,34 @@ update_connector_routing(struct drm_atomic_state *state, return -EINVAL; } - if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) { + if (!drm_encoder_crtc_ok(new_encoder, new_connector_state->crtc)) { DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n", new_encoder->base.id, new_encoder->name, - connector_state->crtc->base.id, - connector_state->crtc->name); + new_connector_state->crtc->base.id, + new_connector_state->crtc->name); return -EINVAL; } - if (new_encoder == connector_state->best_encoder) { - set_best_encoder(state, connector_state, new_encoder); + if (new_encoder == new_connector_state->best_encoder) { + set_best_encoder(state, new_connector_state, new_encoder); DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] keeps [ENCODER:%d:%s], now on [CRTC:%d:%s]\n", connector->base.id, connector->name, new_encoder->base.id, new_encoder->name, - connector_state->crtc->base.id, - connector_state->crtc->name); + new_connector_state->crtc->base.id, + new_connector_state->crtc->name); return 0; } steal_encoder(state, new_encoder); - set_best_encoder(state, connector_state, new_encoder); + set_best_encoder(state, new_connector_state, new_encoder); - crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc); + crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc); crtc_state->connectors_changed = true; DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n", @@ -356,8 +358,8 @@ update_connector_routing(struct drm_atomic_state *state, connector->name, new_encoder->base.id, new_encoder->name, - connector_state->crtc->base.id, - connector_state->crtc->name); + new_connector_state->crtc->base.id, + new_connector_state->crtc->name); return 0; } @@ -366,57 +368,57 @@ static int mode_fixup(struct drm_atomic_state *state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *conn_state; + struct drm_connector_state *new_conn_state; int i; int ret; - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (!crtc_state->mode_changed && - !crtc_state->connectors_changed) + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { + if (!new_crtc_state->mode_changed && + !new_crtc_state->connectors_changed) continue; - drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode); + drm_mode_copy(&new_crtc_state->adjusted_mode, &new_crtc_state->mode); } - for_each_connector_in_state(state, connector, conn_state, i) { + for_each_new_connector_in_state(state, connector, new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; struct drm_encoder *encoder; - WARN_ON(!!conn_state->best_encoder != !!conn_state->crtc); + WARN_ON(!!new_conn_state->best_encoder != !!new_conn_state->crtc); - if (!conn_state->crtc || !conn_state->best_encoder) + if (!new_conn_state->crtc || !new_conn_state->best_encoder) continue; - crtc_state = drm_atomic_get_existing_crtc_state(state, - conn_state->crtc); + new_crtc_state = + drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); /* * Each encoder has at most one connector (since we always steal * it away), so we won't call ->mode_fixup twice. */ - encoder = conn_state->best_encoder; + encoder = new_conn_state->best_encoder; funcs = encoder->helper_private; - ret = drm_bridge_mode_fixup(encoder->bridge, &crtc_state->mode, - &crtc_state->adjusted_mode); + ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state->mode, + &new_crtc_state->adjusted_mode); if (!ret) { DRM_DEBUG_ATOMIC("Bridge fixup failed\n"); return -EINVAL; } if (funcs && funcs->atomic_check) { - ret = funcs->atomic_check(encoder, crtc_state, - conn_state); + ret = funcs->atomic_check(encoder, new_crtc_state, + new_conn_state); if (ret) { DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n", encoder->base.id, encoder->name); return ret; } } else if (funcs && funcs->mode_fixup) { - ret = funcs->mode_fixup(encoder, &crtc_state->mode, - &crtc_state->adjusted_mode); + ret = funcs->mode_fixup(encoder, &new_crtc_state->mode, + &new_crtc_state->adjusted_mode); if (!ret) { DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n", encoder->base.id, encoder->name); @@ -425,22 +427,22 @@ mode_fixup(struct drm_atomic_state *state) } } - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; - if (!crtc_state->enable) + if (!new_crtc_state->enable) continue; - if (!crtc_state->mode_changed && - !crtc_state->connectors_changed) + if (!new_crtc_state->mode_changed && + !new_crtc_state->connectors_changed) continue; funcs = crtc->helper_private; if (!funcs->mode_fixup) continue; - ret = funcs->mode_fixup(crtc, &crtc_state->mode, - &crtc_state->adjusted_mode); + ret = funcs->mode_fixup(crtc, &new_crtc_state->mode, + &new_crtc_state->adjusted_mode); if (!ret) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n", crtc->base.id, crtc->name); @@ -487,19 +489,19 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *connector_state; + struct drm_connector_state *old_connector_state, *new_connector_state; int i, ret; - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode)) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { + if (!drm_mode_equal(&old_crtc_state->mode, &new_crtc_state->mode)) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode changed\n", crtc->base.id, crtc->name); - crtc_state->mode_changed = true; + new_crtc_state->mode_changed = true; } - if (crtc->state->enable != crtc_state->enable) { + if (old_crtc_state->enable != new_crtc_state->enable) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enable changed\n", crtc->base.id, crtc->name); @@ -511,8 +513,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, * The other way around is true as well. enable != 0 * iff connectors are attached and a mode is set. */ - crtc_state->mode_changed = true; - crtc_state->connectors_changed = true; + new_crtc_state->mode_changed = true; + new_crtc_state->connectors_changed = true; } } @@ -520,14 +522,15 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (ret) return ret; - for_each_connector_in_state(state, connector, connector_state, i) { + for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) { /* * This only sets crtc->connectors_changed for routing changes, * drivers must set crtc->connectors_changed themselves when * connector properties need to be updated. */ ret = update_connector_routing(state, connector, - connector_state); + old_connector_state, + new_connector_state); if (ret) return ret; } @@ -538,28 +541,28 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, * configuration. This must be done before calling mode_fixup in case a * crtc only changed its mode but has the same set of connectors. */ - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { bool has_connectors = - !!crtc_state->connector_mask; + !!new_crtc_state->connector_mask; /* * We must set ->active_changed after walking connectors for * otherwise an update that only changes active would result in * a full modeset because update_connector_routing force that. */ - if (crtc->state->active != crtc_state->active) { + if (old_crtc_state->active != new_crtc_state->active) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active changed\n", crtc->base.id, crtc->name); - crtc_state->active_changed = true; + new_crtc_state->active_changed = true; } - if (!drm_atomic_crtc_needs_modeset(crtc_state)) + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) continue; DRM_DEBUG_ATOMIC("[CRTC:%d:%s] needs all connectors, enable: %c, active: %c\n", crtc->base.id, crtc->name, - crtc_state->enable ? 'y' : 'n', - crtc_state->active ? 'y' : 'n'); + new_crtc_state->enable ? 'y' : 'n', + new_crtc_state->active ? 'y' : 'n'); ret = drm_atomic_add_affected_connectors(state, crtc); if (ret != 0) @@ -569,7 +572,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (ret != 0) return ret; - if (crtc_state->enable != has_connectors) { + if (new_crtc_state->enable != has_connectors) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n", crtc->base.id, crtc->name); @@ -602,22 +605,22 @@ drm_atomic_helper_check_planes(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *new_plane_state, *old_plane_state; int i, ret = 0; - for_each_plane_in_state(state, plane, plane_state, i) { + for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; funcs = plane->helper_private; - drm_atomic_helper_plane_changed(state, plane_state, plane); + drm_atomic_helper_plane_changed(state, old_plane_state, new_plane_state, plane); if (!funcs || !funcs->atomic_check) continue; - ret = funcs->atomic_check(plane, plane_state); + ret = funcs->atomic_check(plane, new_plane_state); if (ret) { DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n", plane->base.id, plane->name); @@ -625,7 +628,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev, } } - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; funcs = crtc->helper_private; @@ -633,7 +636,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev, if (!funcs || !funcs->atomic_check) continue; - ret = funcs->atomic_check(crtc, crtc_state); + ret = funcs->atomic_check(crtc, new_crtc_state); if (ret) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n", crtc->base.id, crtc->name); @@ -687,12 +690,12 @@ static void disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; int i; - for_each_connector_in_state(old_state, connector, old_conn_state, i) { + for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; struct drm_encoder *encoder; @@ -729,7 +732,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) /* Right function depends upon target state. */ if (funcs) { - if (connector->state->crtc && funcs->prepare) + if (new_conn_state->crtc && funcs->prepare) funcs->prepare(encoder); else if (funcs->disable) funcs->disable(encoder); @@ -740,11 +743,11 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) drm_bridge_post_disable(encoder->bridge); } - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; /* Shut down everything that needs a full modeset. */ - if (!drm_atomic_crtc_needs_modeset(crtc->state)) + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) continue; if (!old_crtc_state->active) @@ -757,7 +760,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) /* Right function depends upon target state. */ - if (crtc->state->enable && funcs->prepare) + if (new_crtc_state->enable && funcs->prepare) funcs->prepare(crtc); else if (funcs->atomic_disable) funcs->atomic_disable(crtc, old_crtc_state); @@ -786,13 +789,13 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *new_crtc_state; int i; /* clear out existing links and update dpms */ - for_each_connector_in_state(old_state, connector, old_conn_state, i) { + for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) { if (connector->encoder) { WARN_ON(!connector->encoder->crtc); @@ -800,7 +803,7 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, connector->encoder = NULL; } - crtc = connector->state->crtc; + crtc = new_conn_state->crtc; if ((!crtc && old_conn_state->crtc) || (crtc && drm_atomic_crtc_needs_modeset(crtc->state))) { struct drm_property *dpms_prop = @@ -817,23 +820,23 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, } /* set new links */ - for_each_connector_in_state(old_state, connector, old_conn_state, i) { - if (!connector->state->crtc) + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { + if (!new_conn_state->crtc) continue; - if (WARN_ON(!connector->state->best_encoder)) + if (WARN_ON(!new_conn_state->best_encoder)) continue; - connector->encoder = connector->state->best_encoder; - connector->encoder->crtc = connector->state->crtc; + connector->encoder = new_conn_state->best_encoder; + connector->encoder->crtc = new_conn_state->crtc; } /* set legacy state in the crtc structure */ - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { struct drm_plane *primary = crtc->primary; - crtc->mode = crtc->state->mode; - crtc->enabled = crtc->state->enable; + crtc->mode = new_crtc_state->mode; + crtc->enabled = new_crtc_state->enable; if (drm_atomic_get_existing_plane_state(old_state, primary) && primary->state->crtc == crtc) { @@ -841,9 +844,9 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, crtc->y = primary->state->src_y >> 16; } - if (crtc->state->enable) + if (new_crtc_state->enable) drm_calc_timestamping_constants(crtc, - &crtc->state->adjusted_mode); + &new_crtc_state->adjusted_mode); } } EXPORT_SYMBOL(drm_atomic_helper_update_legacy_modeset_state); @@ -852,20 +855,20 @@ static void crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *new_conn_state; int i; - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; - if (!crtc->state->mode_changed) + if (!new_crtc_state->mode_changed) continue; funcs = crtc->helper_private; - if (crtc->state->enable && funcs->mode_set_nofb) { + if (new_crtc_state->enable && funcs->mode_set_nofb) { DRM_DEBUG_ATOMIC("modeset on [CRTC:%d:%s]\n", crtc->base.id, crtc->name); @@ -873,18 +876,17 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) } } - for_each_connector_in_state(old_state, connector, old_conn_state, i) { + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; - struct drm_crtc_state *new_crtc_state; struct drm_encoder *encoder; struct drm_display_mode *mode, *adjusted_mode; - if (!connector->state->best_encoder) + if (!new_conn_state->best_encoder) continue; - encoder = connector->state->best_encoder; + encoder = new_conn_state->best_encoder; funcs = encoder->helper_private; - new_crtc_state = connector->state->crtc->state; + new_crtc_state = new_conn_state->crtc->state; mode = &new_crtc_state->mode; adjusted_mode = &new_crtc_state->adjusted_mode; @@ -900,7 +902,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) */ if (funcs && funcs->atomic_mode_set) { funcs->atomic_mode_set(encoder, new_crtc_state, - connector->state); + new_conn_state); } else if (funcs && funcs->mode_set) { funcs->mode_set(encoder, mode, adjusted_mode); } @@ -952,24 +954,24 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *new_conn_state; int i; - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; /* Need to filter out CRTCs where only planes change. */ - if (!drm_atomic_crtc_needs_modeset(crtc->state)) + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) continue; - if (!crtc->state->active) + if (!new_crtc_state->active) continue; funcs = crtc->helper_private; - if (crtc->state->enable) { + if (new_crtc_state->enable) { DRM_DEBUG_ATOMIC("enabling [CRTC:%d:%s]\n", crtc->base.id, crtc->name); @@ -980,18 +982,18 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, } } - for_each_connector_in_state(old_state, connector, old_conn_state, i) { + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; struct drm_encoder *encoder; - if (!connector->state->best_encoder) + if (!new_conn_state->best_encoder) continue; - if (!connector->state->crtc->state->active || - !drm_atomic_crtc_needs_modeset(connector->state->crtc->state)) + if (!new_conn_state->crtc->state->active || + !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state)) continue; - encoder = connector->state->best_encoder; + encoder = new_conn_state->best_encoder; funcs = encoder->helper_private; DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n", @@ -1041,29 +1043,26 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev, bool pre_swap) { struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *new_plane_state; int i, ret; - for_each_plane_in_state(state, plane, plane_state, i) { - if (!pre_swap) - plane_state = plane->state; - - if (!plane_state->fence) + for_each_new_plane_in_state(state, plane, new_plane_state, i) { + if (!new_plane_state->fence) continue; - WARN_ON(!plane_state->fb); + WARN_ON(!new_plane_state->fb); /* * If waiting for fences pre-swap (ie: nonblock), userspace can * still interrupt the operation. Instead of blocking until the * timer expires, make the wait interruptible. */ - ret = dma_fence_wait(plane_state->fence, pre_swap); + ret = dma_fence_wait(new_plane_state->fence, pre_swap); if (ret) return ret; - dma_fence_put(plane_state->fence); - plane_state->fence = NULL; + dma_fence_put(new_plane_state->fence); + new_plane_state->fence = NULL; } return 0; @@ -1086,7 +1085,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; int i, ret; unsigned crtc_mask = 0; @@ -1097,9 +1096,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, if (old_state->legacy_cursor_update) return; - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { - struct drm_crtc_state *new_crtc_state = crtc->state; - + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { if (!new_crtc_state->active || !new_crtc_state->planes_changed) continue; @@ -1111,7 +1108,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, old_state->crtcs[i].last_vblank_count = drm_crtc_vblank_count(crtc); } - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { if (!(crtc_mask & drm_crtc_mask(crtc))) continue; @@ -1420,11 +1417,11 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, bool nonblock) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_crtc_commit *commit; int i, ret; - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { commit = kzalloc(sizeof(*commit), GFP_KERNEL); if (!commit) return -ENOMEM; @@ -1445,7 +1442,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, /* Drivers only send out events when at least either current or * new CRTC state is active. Complete right away if everything * stays off. */ - if (!crtc->state->active && !crtc_state->active) { + if (!old_crtc_state->active && !new_crtc_state->active) { complete_all(&commit->flip_done); continue; } @@ -1456,17 +1453,17 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, continue; } - if (!crtc_state->event) { + if (!new_crtc_state->event) { commit->event = kzalloc(sizeof(*commit->event), GFP_KERNEL); if (!commit->event) return -ENOMEM; - crtc_state->event = commit->event; + new_crtc_state->event = commit->event; } - crtc_state->event->base.completion = &commit->flip_done; - crtc_state->event->base.completion_release = release_crtc_commit; + new_crtc_state->event->base.completion = &commit->flip_done; + new_crtc_state->event->base.completion_release = release_crtc_commit; drm_crtc_commit_get(commit); } @@ -1505,12 +1502,12 @@ static struct drm_crtc_commit *preceeding_commit(struct drm_crtc *crtc) void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_crtc_commit *commit; int i; long ret; - for_each_crtc_in_state(old_state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { spin_lock(&crtc->commit_lock); commit = preceeding_commit(crtc); if (commit) @@ -1557,17 +1554,17 @@ 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_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_crtc_commit *commit; int i; - for_each_crtc_in_state(old_state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { commit = old_state->crtcs[i].commit; if (!commit) continue; /* backend must have consumed any event by now */ - WARN_ON(crtc->state->event); + WARN_ON(new_crtc_state->event); spin_lock(&crtc->commit_lock); complete_all(&commit->hw_done); spin_unlock(&crtc->commit_lock); @@ -1589,12 +1586,12 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_hw_done); void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_crtc_commit *commit; int i; long ret; - for_each_crtc_in_state(old_state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { commit = old_state->crtcs[i].commit; if (WARN_ON(!commit)) continue; @@ -1645,16 +1642,16 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *new_plane_state; int ret, i, j; - for_each_plane_in_state(state, plane, plane_state, i) { + for_each_new_plane_in_state(state, plane, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; funcs = plane->helper_private; if (funcs->prepare_fb) { - ret = funcs->prepare_fb(plane, plane_state); + ret = funcs->prepare_fb(plane, new_plane_state); if (ret) goto fail; } @@ -1663,7 +1660,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev, return 0; fail: - for_each_plane_in_state(state, plane, plane_state, j) { + for_each_new_plane_in_state(state, plane, new_plane_state, j) { const struct drm_plane_helper_funcs *funcs; if (j >= i) @@ -1672,7 +1669,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev, funcs = plane->helper_private; if (funcs->cleanup_fb) - funcs->cleanup_fb(plane, plane_state); + funcs->cleanup_fb(plane, new_plane_state); } return ret; @@ -1730,14 +1727,14 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, uint32_t flags) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; - struct drm_plane_state *old_plane_state; + struct drm_plane_state *old_plane_state, *new_plane_state; int i; bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY; bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET; - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; funcs = crtc->helper_private; @@ -1745,13 +1742,13 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, if (!funcs || !funcs->atomic_begin) continue; - if (active_only && !crtc->state->active) + if (active_only && !new_crtc_state->active) continue; funcs->atomic_begin(crtc, old_crtc_state); } - for_each_plane_in_state(old_state, plane, old_plane_state, i) { + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; bool disabling; @@ -1770,7 +1767,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, * CRTC to avoid skipping planes being disabled on an * active CRTC. */ - if (!disabling && !plane_crtc_active(plane->state)) + if (!disabling && !plane_crtc_active(new_plane_state)) continue; if (disabling && !plane_crtc_active(old_plane_state)) continue; @@ -1789,12 +1786,12 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, continue; funcs->atomic_disable(plane, old_plane_state); - } else if (plane->state->crtc || disabling) { + } else if (new_plane_state->crtc || disabling) { funcs->atomic_update(plane, old_plane_state); } } - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; funcs = crtc->helper_private; @@ -1802,7 +1799,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, if (!funcs || !funcs->atomic_flush) continue; - if (active_only && !crtc->state->active) + if (active_only && !new_crtc_state->active) continue; funcs->atomic_flush(crtc, old_crtc_state); @@ -1929,11 +1926,21 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *old_plane_state, *new_plane_state; int i; - for_each_plane_in_state(old_state, plane, plane_state, i) { + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; + struct drm_plane_state *plane_state; + + /* + * This might be called before swapping when commit is aborted, + * in which case we have to free the new state. + */ + if (old_plane_state == plane->state) + plane_state = new_plane_state; + else + plane_state = old_plane_state; funcs = plane->helper_private; @@ -1979,15 +1986,15 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, int i; long ret; struct drm_connector *connector; - struct drm_connector_state *conn_state, *old_conn_state; + struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state, *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; - struct drm_plane_state *plane_state, *old_plane_state; + struct drm_plane_state *old_plane_state, *new_plane_state; struct drm_crtc_commit *commit; if (stall) { - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { spin_lock(&crtc->commit_lock); commit = list_first_entry_or_null(&crtc->commit_list, struct drm_crtc_commit, commit_entry); @@ -2007,20 +2014,24 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, } } - for_each_oldnew_connector_in_state(state, connector, old_conn_state, conn_state, i) { + for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_conn_state, i) { WARN_ON(connector->state != old_conn_state); - connector->state->state = state; - swap(state->connectors[i].state, connector->state); - connector->state->state = NULL; + old_conn_state->state = state; + new_conn_state->state = NULL; + + state->connectors[i].state = old_conn_state; + connector->state = new_conn_state; } - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { WARN_ON(crtc->state != old_crtc_state); - crtc->state->state = state; - swap(state->crtcs[i].state, crtc->state); - crtc->state->state = NULL; + old_crtc_state->state = state; + new_crtc_state->state = NULL; + + state->crtcs[i].state = old_crtc_state; + crtc->state = new_crtc_state; if (state->crtcs[i].commit) { spin_lock(&crtc->commit_lock); @@ -2032,12 +2043,14 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, } } - for_each_oldnew_plane_in_state(state, plane, old_plane_state, plane_state, i) { + for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { WARN_ON(plane->state != old_plane_state); - plane->state->state = state; - swap(state->planes[i].state, plane->state); - plane->state->state = NULL; + old_plane_state->state = state; + new_plane_state->state = NULL; + + state->planes[i].state = old_plane_state; + plane->state = new_plane_state; } } EXPORT_SYMBOL(drm_atomic_helper_swap_state); @@ -2220,9 +2233,9 @@ static int update_output_state(struct drm_atomic_state *state, { struct drm_device *dev = set->crtc->dev; struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *conn_state; + struct drm_connector_state *new_conn_state; int ret, i; ret = drm_modeset_lock(&dev->mode_config.connection_mutex, @@ -2235,9 +2248,9 @@ static int update_output_state(struct drm_atomic_state *state, if (ret) return ret; - for_each_connector_in_state(state, connector, conn_state, i) { - if (conn_state->crtc == set->crtc) { - ret = drm_atomic_set_crtc_for_connector(conn_state, + for_each_new_connector_in_state(state, connector, new_conn_state, i) { + if (new_conn_state->crtc == set->crtc) { + ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL); if (ret) return ret; @@ -2246,18 +2259,18 @@ static int update_output_state(struct drm_atomic_state *state, /* Then set all connectors from set->connectors on the target crtc */ for (i = 0; i < set->num_connectors; i++) { - conn_state = drm_atomic_get_connector_state(state, + new_conn_state = drm_atomic_get_connector_state(state, set->connectors[i]); - if (IS_ERR(conn_state)) - return PTR_ERR(conn_state); + if (IS_ERR(new_conn_state)) + return PTR_ERR(new_conn_state); - ret = drm_atomic_set_crtc_for_connector(conn_state, + ret = drm_atomic_set_crtc_for_connector(new_conn_state, set->crtc); if (ret) return ret; } - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { /* Don't update ->enable for the CRTC in the set_config request, * since a mismatch would indicate a bug in the upper layers. * The actual modeset code later on will catch any @@ -2265,13 +2278,13 @@ static int update_output_state(struct drm_atomic_state *state, if (crtc == set->crtc) continue; - if (!crtc_state->connector_mask) { - ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, + if (!new_crtc_state->connector_mask) { + ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state, NULL); if (ret < 0) return ret; - crtc_state->active = false; + new_crtc_state->active = false; } } @@ -2545,21 +2558,21 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state, { int i; struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *new_plane_state; struct drm_connector *connector; - struct drm_connector_state *conn_state; + struct drm_connector_state *new_conn_state; struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; state->acquire_ctx = ctx; - for_each_new_plane_in_state(state, plane, plane_state, i) + for_each_new_plane_in_state(state, plane, new_plane_state, i) state->planes[i].old_state = plane->state; - for_each_new_crtc_in_state(state, crtc, crtc_state, i) + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) state->crtcs[i].old_state = crtc->state; - for_each_new_connector_in_state(state, connector, conn_state, i) + for_each_new_connector_in_state(state, connector, new_conn_state, i) state->connectors[i].old_state = connector->state; return drm_atomic_commit(state); -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2. 2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst @ 2017-03-01 0:49 ` Laurent Pinchart 2017-03-01 9:09 ` Maarten Lankhorst 2017-03-01 9:21 ` [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3 Maarten Lankhorst 0 siblings, 2 replies; 15+ messages in thread From: Laurent Pinchart @ 2017-03-01 0:49 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx Hi Maarten, Thank you for the patch. On Thursday 16 Feb 2017 15:47:06 Maarten Lankhorst wrote: > There are new iterator macros that annotate whether the new or old > state should be used. This is better than using a state that depends on > whether it's called before or after swap. For clarity, also rename the > variables from $obj_state to (old,new)_$obj_state as well. > > Changes since v1: > - Use old/new_*_state for variable names as much as possible. (pinchartl) > - Expand commit message. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 431 +++++++++++++++++---------------- > 1 file changed, 222 insertions(+), 209 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index 9203f3e933f7..7d432d9a18cf > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c [snip] > @@ -1929,11 +1926,21 @@ void drm_atomic_helper_cleanup_planes(struct > drm_device *dev, struct drm_atomic_state *old_state) > { > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *old_plane_state, *new_plane_state; > int i; > > - for_each_plane_in_state(old_state, plane, plane_state, i) { > + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, > new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; > + struct drm_plane_state *plane_state; > + > + /* > + * This might be called before swapping when commit is aborted, > + * in which case we have to free the new state. s/free/cleanup/ Apart from that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> You will however need to rebase the series on top of the latest drm-misc as it conflicts (at compile time) with commit 40ee6fbef75fe6452dc9e69e6f9f1a2c7808ed67 Author: Manasi Navare <manasi.d.navare@intel.com> Date: Fri Dec 16 12:29:06 2016 +0200 drm: Add a new connector atomic property for link status > + */ > + if (old_plane_state == plane->state) > + plane_state = new_plane_state; > + else > + plane_state = old_plane_state; > > funcs = plane->helper_private; > [snip] -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2. 2017-03-01 0:49 ` Laurent Pinchart @ 2017-03-01 9:09 ` Maarten Lankhorst 2017-03-01 9:21 ` [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3 Maarten Lankhorst 1 sibling, 0 replies; 15+ messages in thread From: Maarten Lankhorst @ 2017-03-01 9:09 UTC (permalink / raw) To: Laurent Pinchart, dri-devel; +Cc: intel-gfx Op 01-03-17 om 01:49 schreef Laurent Pinchart: > Hi Maarten, > > Thank you for the patch. > > On Thursday 16 Feb 2017 15:47:06 Maarten Lankhorst wrote: >> There are new iterator macros that annotate whether the new or old >> state should be used. This is better than using a state that depends on >> whether it's called before or after swap. For clarity, also rename the >> variables from $obj_state to (old,new)_$obj_state as well. >> >> Changes since v1: >> - Use old/new_*_state for variable names as much as possible. (pinchartl) >> - Expand commit message. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/drm_atomic_helper.c | 431 +++++++++++++++++---------------- >> 1 file changed, 222 insertions(+), 209 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c >> b/drivers/gpu/drm/drm_atomic_helper.c index 9203f3e933f7..7d432d9a18cf >> 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c > [snip] > >> @@ -1929,11 +1926,21 @@ void drm_atomic_helper_cleanup_planes(struct >> drm_device *dev, struct drm_atomic_state *old_state) >> { >> struct drm_plane *plane; >> - struct drm_plane_state *plane_state; >> + struct drm_plane_state *old_plane_state, *new_plane_state; >> int i; >> >> - for_each_plane_in_state(old_state, plane, plane_state, i) { >> + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, >> new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; >> + struct drm_plane_state *plane_state; >> + >> + /* >> + * This might be called before swapping when commit is > aborted, >> + * in which case we have to free the new state. > s/free/cleanup/ > > Apart from that, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > You will however need to rebase the series on top of the latest drm-misc as it > conflicts (at compile time) with Yeah I noticed, patch 1 and 5 are affected, will resend those. Thanks for the review, ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3. 2017-03-01 0:49 ` Laurent Pinchart 2017-03-01 9:09 ` Maarten Lankhorst @ 2017-03-01 9:21 ` Maarten Lankhorst 2017-03-01 10:06 ` Laurent Pinchart 1 sibling, 1 reply; 15+ messages in thread From: Maarten Lankhorst @ 2017-03-01 9:21 UTC (permalink / raw) To: Laurent Pinchart, dri-devel; +Cc: intel-gfx There are new iterator macros that annotate whether the new or old state should be used. This is better than using a state that depends on whether it's called before or after swap. For clarity, also rename the variables from $obj_state to (old,new)_$obj_state as well. Changes since v1: - Use old/new_*_state for variable names as much as possible. (pinchartl) - Expand commit message. Changes since v2: - Rebase on top of link training patches. - free -> cleanup (pinchartl) Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/gpu/drm/drm_atomic_helper.c | 446 +++++++++++++++++++----------------- 1 file changed, 230 insertions(+), 216 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 6b12396f718b..aa1e7609b024 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -63,14 +63,15 @@ */ static void drm_atomic_helper_plane_changed(struct drm_atomic_state *state, + struct drm_plane_state *old_plane_state, struct drm_plane_state *plane_state, struct drm_plane *plane) { struct drm_crtc_state *crtc_state; - if (plane->state->crtc) { + if (old_plane_state->crtc) { crtc_state = drm_atomic_get_existing_crtc_state(state, - plane->state->crtc); + old_plane_state->crtc); if (WARN_ON(!crtc_state)) return; @@ -92,7 +93,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, static int handle_conflicting_encoders(struct drm_atomic_state *state, bool disable_conflicting_encoders) { - struct drm_connector_state *conn_state; + struct drm_connector_state *new_conn_state; struct drm_connector *connector; struct drm_connector_list_iter conn_iter; struct drm_encoder *encoder; @@ -104,15 +105,15 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state, * part of the state. If the same encoder is assigned to multiple * connectors bail out. */ - for_each_connector_in_state(state, connector, conn_state, i) { + for_each_new_connector_in_state(state, connector, new_conn_state, i) { const struct drm_connector_helper_funcs *funcs = connector->helper_private; struct drm_encoder *new_encoder; - if (!conn_state->crtc) + if (!new_conn_state->crtc) continue; if (funcs->atomic_best_encoder) - new_encoder = funcs->atomic_best_encoder(connector, conn_state); + new_encoder = funcs->atomic_best_encoder(connector, new_conn_state); else if (funcs->best_encoder) new_encoder = funcs->best_encoder(connector); else @@ -166,20 +167,20 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state, goto out; } - conn_state = drm_atomic_get_connector_state(state, connector); - if (IS_ERR(conn_state)) { - ret = PTR_ERR(conn_state); + new_conn_state = drm_atomic_get_connector_state(state, connector); + if (IS_ERR(new_conn_state)) { + ret = PTR_ERR(new_conn_state); goto out; } DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], disabling [CONNECTOR:%d:%s]\n", encoder->base.id, encoder->name, - conn_state->crtc->base.id, conn_state->crtc->name, + new_conn_state->crtc->base.id, new_conn_state->crtc->name, connector->base.id, connector->name); - crtc_state = drm_atomic_get_existing_crtc_state(state, conn_state->crtc); + crtc_state = drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); - ret = drm_atomic_set_crtc_for_connector(conn_state, NULL); + ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL); if (ret) goto out; @@ -245,22 +246,22 @@ steal_encoder(struct drm_atomic_state *state, { struct drm_crtc_state *crtc_state; struct drm_connector *connector; - struct drm_connector_state *connector_state; + struct drm_connector_state *old_connector_state, *new_connector_state; int i; - for_each_connector_in_state(state, connector, connector_state, i) { + for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) { struct drm_crtc *encoder_crtc; - if (connector_state->best_encoder != encoder) + if (new_connector_state->best_encoder != encoder) continue; - encoder_crtc = connector->state->crtc; + encoder_crtc = old_connector_state->crtc; DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing it\n", encoder->base.id, encoder->name, encoder_crtc->base.id, encoder_crtc->name); - set_best_encoder(state, connector_state, NULL); + set_best_encoder(state, new_connector_state, NULL); crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc); crtc_state->connectors_changed = true; @@ -272,7 +273,8 @@ steal_encoder(struct drm_atomic_state *state, static int update_connector_routing(struct drm_atomic_state *state, struct drm_connector *connector, - struct drm_connector_state *connector_state) + struct drm_connector_state *old_connector_state, + struct drm_connector_state *new_connector_state) { const struct drm_connector_helper_funcs *funcs; struct drm_encoder *new_encoder; @@ -282,24 +284,24 @@ update_connector_routing(struct drm_atomic_state *state, connector->base.id, connector->name); - if (connector->state->crtc != connector_state->crtc) { - if (connector->state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, connector->state->crtc); + if (old_connector_state->crtc != new_connector_state->crtc) { + if (old_connector_state->crtc) { + crtc_state = drm_atomic_get_existing_crtc_state(state, old_connector_state->crtc); crtc_state->connectors_changed = true; } - if (connector_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc); + if (new_connector_state->crtc) { + crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc); crtc_state->connectors_changed = true; } } - if (!connector_state->crtc) { + if (!new_connector_state->crtc) { DRM_DEBUG_ATOMIC("Disabling [CONNECTOR:%d:%s]\n", connector->base.id, connector->name); - set_best_encoder(state, connector_state, NULL); + set_best_encoder(state, new_connector_state, NULL); return 0; } @@ -308,7 +310,7 @@ update_connector_routing(struct drm_atomic_state *state, if (funcs->atomic_best_encoder) new_encoder = funcs->atomic_best_encoder(connector, - connector_state); + new_connector_state); else if (funcs->best_encoder) new_encoder = funcs->best_encoder(connector); else @@ -321,34 +323,34 @@ update_connector_routing(struct drm_atomic_state *state, return -EINVAL; } - if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) { + if (!drm_encoder_crtc_ok(new_encoder, new_connector_state->crtc)) { DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n", new_encoder->base.id, new_encoder->name, - connector_state->crtc->base.id, - connector_state->crtc->name); + new_connector_state->crtc->base.id, + new_connector_state->crtc->name); return -EINVAL; } - if (new_encoder == connector_state->best_encoder) { - set_best_encoder(state, connector_state, new_encoder); + if (new_encoder == new_connector_state->best_encoder) { + set_best_encoder(state, new_connector_state, new_encoder); DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] keeps [ENCODER:%d:%s], now on [CRTC:%d:%s]\n", connector->base.id, connector->name, new_encoder->base.id, new_encoder->name, - connector_state->crtc->base.id, - connector_state->crtc->name); + new_connector_state->crtc->base.id, + new_connector_state->crtc->name); return 0; } steal_encoder(state, new_encoder); - set_best_encoder(state, connector_state, new_encoder); + set_best_encoder(state, new_connector_state, new_encoder); - crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc); + crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc); crtc_state->connectors_changed = true; DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n", @@ -356,8 +358,8 @@ update_connector_routing(struct drm_atomic_state *state, connector->name, new_encoder->base.id, new_encoder->name, - connector_state->crtc->base.id, - connector_state->crtc->name); + new_connector_state->crtc->base.id, + new_connector_state->crtc->name); return 0; } @@ -366,57 +368,57 @@ static int mode_fixup(struct drm_atomic_state *state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *conn_state; + struct drm_connector_state *new_conn_state; int i; int ret; - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (!crtc_state->mode_changed && - !crtc_state->connectors_changed) + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { + if (!new_crtc_state->mode_changed && + !new_crtc_state->connectors_changed) continue; - drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode); + drm_mode_copy(&new_crtc_state->adjusted_mode, &new_crtc_state->mode); } - for_each_connector_in_state(state, connector, conn_state, i) { + for_each_new_connector_in_state(state, connector, new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; struct drm_encoder *encoder; - WARN_ON(!!conn_state->best_encoder != !!conn_state->crtc); + WARN_ON(!!new_conn_state->best_encoder != !!new_conn_state->crtc); - if (!conn_state->crtc || !conn_state->best_encoder) + if (!new_conn_state->crtc || !new_conn_state->best_encoder) continue; - crtc_state = drm_atomic_get_existing_crtc_state(state, - conn_state->crtc); + new_crtc_state = + drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); /* * Each encoder has at most one connector (since we always steal * it away), so we won't call ->mode_fixup twice. */ - encoder = conn_state->best_encoder; + encoder = new_conn_state->best_encoder; funcs = encoder->helper_private; - ret = drm_bridge_mode_fixup(encoder->bridge, &crtc_state->mode, - &crtc_state->adjusted_mode); + ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state->mode, + &new_crtc_state->adjusted_mode); if (!ret) { DRM_DEBUG_ATOMIC("Bridge fixup failed\n"); return -EINVAL; } if (funcs && funcs->atomic_check) { - ret = funcs->atomic_check(encoder, crtc_state, - conn_state); + ret = funcs->atomic_check(encoder, new_crtc_state, + new_conn_state); if (ret) { DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n", encoder->base.id, encoder->name); return ret; } } else if (funcs && funcs->mode_fixup) { - ret = funcs->mode_fixup(encoder, &crtc_state->mode, - &crtc_state->adjusted_mode); + ret = funcs->mode_fixup(encoder, &new_crtc_state->mode, + &new_crtc_state->adjusted_mode); if (!ret) { DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n", encoder->base.id, encoder->name); @@ -425,22 +427,22 @@ mode_fixup(struct drm_atomic_state *state) } } - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; - if (!crtc_state->enable) + if (!new_crtc_state->enable) continue; - if (!crtc_state->mode_changed && - !crtc_state->connectors_changed) + if (!new_crtc_state->mode_changed && + !new_crtc_state->connectors_changed) continue; funcs = crtc->helper_private; if (!funcs->mode_fixup) continue; - ret = funcs->mode_fixup(crtc, &crtc_state->mode, - &crtc_state->adjusted_mode); + ret = funcs->mode_fixup(crtc, &new_crtc_state->mode, + &new_crtc_state->adjusted_mode); if (!ret) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n", crtc->base.id, crtc->name); @@ -487,19 +489,19 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *connector_state; + struct drm_connector_state *old_connector_state, *new_connector_state; int i, ret; - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode)) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { + if (!drm_mode_equal(&old_crtc_state->mode, &new_crtc_state->mode)) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode changed\n", crtc->base.id, crtc->name); - crtc_state->mode_changed = true; + new_crtc_state->mode_changed = true; } - if (crtc->state->enable != crtc_state->enable) { + if (old_crtc_state->enable != new_crtc_state->enable) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enable changed\n", crtc->base.id, crtc->name); @@ -511,8 +513,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, * The other way around is true as well. enable != 0 * iff connectors are attached and a mode is set. */ - crtc_state->mode_changed = true; - crtc_state->connectors_changed = true; + new_crtc_state->mode_changed = true; + new_crtc_state->connectors_changed = true; } } @@ -520,22 +522,23 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (ret) return ret; - for_each_connector_in_state(state, connector, connector_state, i) { + for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) { /* * This only sets crtc->connectors_changed for routing changes, * drivers must set crtc->connectors_changed themselves when * connector properties need to be updated. */ ret = update_connector_routing(state, connector, - connector_state); + old_connector_state, + new_connector_state); if (ret) return ret; - if (connector->state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, - connector->state->crtc); - if (connector->state->link_status != - connector_state->link_status) - crtc_state->connectors_changed = true; + if (old_connector_state->crtc) { + new_crtc_state = drm_atomic_get_existing_crtc_state(state, + old_connector_state->crtc); + if (old_connector_state->link_status != + new_connector_state->link_status) + new_crtc_state->connectors_changed = true; } } @@ -545,28 +548,28 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, * configuration. This must be done before calling mode_fixup in case a * crtc only changed its mode but has the same set of connectors. */ - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { bool has_connectors = - !!crtc_state->connector_mask; + !!new_crtc_state->connector_mask; /* * We must set ->active_changed after walking connectors for * otherwise an update that only changes active would result in * a full modeset because update_connector_routing force that. */ - if (crtc->state->active != crtc_state->active) { + if (old_crtc_state->active != new_crtc_state->active) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active changed\n", crtc->base.id, crtc->name); - crtc_state->active_changed = true; + new_crtc_state->active_changed = true; } - if (!drm_atomic_crtc_needs_modeset(crtc_state)) + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) continue; DRM_DEBUG_ATOMIC("[CRTC:%d:%s] needs all connectors, enable: %c, active: %c\n", crtc->base.id, crtc->name, - crtc_state->enable ? 'y' : 'n', - crtc_state->active ? 'y' : 'n'); + new_crtc_state->enable ? 'y' : 'n', + new_crtc_state->active ? 'y' : 'n'); ret = drm_atomic_add_affected_connectors(state, crtc); if (ret != 0) @@ -576,7 +579,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (ret != 0) return ret; - if (crtc_state->enable != has_connectors) { + if (new_crtc_state->enable != has_connectors) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n", crtc->base.id, crtc->name); @@ -609,22 +612,22 @@ drm_atomic_helper_check_planes(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *new_plane_state, *old_plane_state; int i, ret = 0; - for_each_plane_in_state(state, plane, plane_state, i) { + for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; funcs = plane->helper_private; - drm_atomic_helper_plane_changed(state, plane_state, plane); + drm_atomic_helper_plane_changed(state, old_plane_state, new_plane_state, plane); if (!funcs || !funcs->atomic_check) continue; - ret = funcs->atomic_check(plane, plane_state); + ret = funcs->atomic_check(plane, new_plane_state); if (ret) { DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n", plane->base.id, plane->name); @@ -632,7 +635,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev, } } - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; funcs = crtc->helper_private; @@ -640,7 +643,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev, if (!funcs || !funcs->atomic_check) continue; - ret = funcs->atomic_check(crtc, crtc_state); + ret = funcs->atomic_check(crtc, new_crtc_state); if (ret) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n", crtc->base.id, crtc->name); @@ -694,12 +697,12 @@ static void disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; int i; - for_each_connector_in_state(old_state, connector, old_conn_state, i) { + for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; struct drm_encoder *encoder; @@ -736,7 +739,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) /* Right function depends upon target state. */ if (funcs) { - if (connector->state->crtc && funcs->prepare) + if (new_conn_state->crtc && funcs->prepare) funcs->prepare(encoder); else if (funcs->disable) funcs->disable(encoder); @@ -747,11 +750,11 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) drm_bridge_post_disable(encoder->bridge); } - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; /* Shut down everything that needs a full modeset. */ - if (!drm_atomic_crtc_needs_modeset(crtc->state)) + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) continue; if (!old_crtc_state->active) @@ -764,7 +767,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) /* Right function depends upon target state. */ - if (crtc->state->enable && funcs->prepare) + if (new_crtc_state->enable && funcs->prepare) funcs->prepare(crtc); else if (funcs->atomic_disable) funcs->atomic_disable(crtc, old_crtc_state); @@ -793,13 +796,13 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *new_crtc_state; int i; /* clear out existing links and update dpms */ - for_each_connector_in_state(old_state, connector, old_conn_state, i) { + for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) { if (connector->encoder) { WARN_ON(!connector->encoder->crtc); @@ -807,7 +810,7 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, connector->encoder = NULL; } - crtc = connector->state->crtc; + crtc = new_conn_state->crtc; if ((!crtc && old_conn_state->crtc) || (crtc && drm_atomic_crtc_needs_modeset(crtc->state))) { struct drm_property *dpms_prop = @@ -824,23 +827,23 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, } /* set new links */ - for_each_connector_in_state(old_state, connector, old_conn_state, i) { - if (!connector->state->crtc) + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { + if (!new_conn_state->crtc) continue; - if (WARN_ON(!connector->state->best_encoder)) + if (WARN_ON(!new_conn_state->best_encoder)) continue; - connector->encoder = connector->state->best_encoder; - connector->encoder->crtc = connector->state->crtc; + connector->encoder = new_conn_state->best_encoder; + connector->encoder->crtc = new_conn_state->crtc; } /* set legacy state in the crtc structure */ - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { struct drm_plane *primary = crtc->primary; - crtc->mode = crtc->state->mode; - crtc->enabled = crtc->state->enable; + crtc->mode = new_crtc_state->mode; + crtc->enabled = new_crtc_state->enable; if (drm_atomic_get_existing_plane_state(old_state, primary) && primary->state->crtc == crtc) { @@ -848,9 +851,9 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, crtc->y = primary->state->src_y >> 16; } - if (crtc->state->enable) + if (new_crtc_state->enable) drm_calc_timestamping_constants(crtc, - &crtc->state->adjusted_mode); + &new_crtc_state->adjusted_mode); } } EXPORT_SYMBOL(drm_atomic_helper_update_legacy_modeset_state); @@ -859,20 +862,20 @@ static void crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *new_conn_state; int i; - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; - if (!crtc->state->mode_changed) + if (!new_crtc_state->mode_changed) continue; funcs = crtc->helper_private; - if (crtc->state->enable && funcs->mode_set_nofb) { + if (new_crtc_state->enable && funcs->mode_set_nofb) { DRM_DEBUG_ATOMIC("modeset on [CRTC:%d:%s]\n", crtc->base.id, crtc->name); @@ -880,18 +883,17 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) } } - for_each_connector_in_state(old_state, connector, old_conn_state, i) { + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; - struct drm_crtc_state *new_crtc_state; struct drm_encoder *encoder; struct drm_display_mode *mode, *adjusted_mode; - if (!connector->state->best_encoder) + if (!new_conn_state->best_encoder) continue; - encoder = connector->state->best_encoder; + encoder = new_conn_state->best_encoder; funcs = encoder->helper_private; - new_crtc_state = connector->state->crtc->state; + new_crtc_state = new_conn_state->crtc->state; mode = &new_crtc_state->mode; adjusted_mode = &new_crtc_state->adjusted_mode; @@ -907,7 +909,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) */ if (funcs && funcs->atomic_mode_set) { funcs->atomic_mode_set(encoder, new_crtc_state, - connector->state); + new_conn_state); } else if (funcs && funcs->mode_set) { funcs->mode_set(encoder, mode, adjusted_mode); } @@ -959,24 +961,24 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *new_conn_state; int i; - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; /* Need to filter out CRTCs where only planes change. */ - if (!drm_atomic_crtc_needs_modeset(crtc->state)) + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) continue; - if (!crtc->state->active) + if (!new_crtc_state->active) continue; funcs = crtc->helper_private; - if (crtc->state->enable) { + if (new_crtc_state->enable) { DRM_DEBUG_ATOMIC("enabling [CRTC:%d:%s]\n", crtc->base.id, crtc->name); @@ -987,18 +989,18 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, } } - for_each_connector_in_state(old_state, connector, old_conn_state, i) { + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; struct drm_encoder *encoder; - if (!connector->state->best_encoder) + if (!new_conn_state->best_encoder) continue; - if (!connector->state->crtc->state->active || - !drm_atomic_crtc_needs_modeset(connector->state->crtc->state)) + if (!new_conn_state->crtc->state->active || + !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state)) continue; - encoder = connector->state->best_encoder; + encoder = new_conn_state->best_encoder; funcs = encoder->helper_private; DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n", @@ -1048,29 +1050,26 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev, bool pre_swap) { struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *new_plane_state; int i, ret; - for_each_plane_in_state(state, plane, plane_state, i) { - if (!pre_swap) - plane_state = plane->state; - - if (!plane_state->fence) + for_each_new_plane_in_state(state, plane, new_plane_state, i) { + if (!new_plane_state->fence) continue; - WARN_ON(!plane_state->fb); + WARN_ON(!new_plane_state->fb); /* * If waiting for fences pre-swap (ie: nonblock), userspace can * still interrupt the operation. Instead of blocking until the * timer expires, make the wait interruptible. */ - ret = dma_fence_wait(plane_state->fence, pre_swap); + ret = dma_fence_wait(new_plane_state->fence, pre_swap); if (ret) return ret; - dma_fence_put(plane_state->fence); - plane_state->fence = NULL; + dma_fence_put(new_plane_state->fence); + new_plane_state->fence = NULL; } return 0; @@ -1093,7 +1092,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; int i, ret; unsigned crtc_mask = 0; @@ -1104,9 +1103,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, if (old_state->legacy_cursor_update) return; - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { - struct drm_crtc_state *new_crtc_state = crtc->state; - + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { if (!new_crtc_state->active || !new_crtc_state->planes_changed) continue; @@ -1118,7 +1115,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, old_state->crtcs[i].last_vblank_count = drm_crtc_vblank_count(crtc); } - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { if (!(crtc_mask & drm_crtc_mask(crtc))) continue; @@ -1427,11 +1424,11 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, bool nonblock) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_crtc_commit *commit; int i, ret; - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { commit = kzalloc(sizeof(*commit), GFP_KERNEL); if (!commit) return -ENOMEM; @@ -1452,7 +1449,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, /* Drivers only send out events when at least either current or * new CRTC state is active. Complete right away if everything * stays off. */ - if (!crtc->state->active && !crtc_state->active) { + if (!old_crtc_state->active && !new_crtc_state->active) { complete_all(&commit->flip_done); continue; } @@ -1463,17 +1460,17 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, continue; } - if (!crtc_state->event) { + if (!new_crtc_state->event) { commit->event = kzalloc(sizeof(*commit->event), GFP_KERNEL); if (!commit->event) return -ENOMEM; - crtc_state->event = commit->event; + new_crtc_state->event = commit->event; } - crtc_state->event->base.completion = &commit->flip_done; - crtc_state->event->base.completion_release = release_crtc_commit; + new_crtc_state->event->base.completion = &commit->flip_done; + new_crtc_state->event->base.completion_release = release_crtc_commit; drm_crtc_commit_get(commit); } @@ -1512,12 +1509,12 @@ static struct drm_crtc_commit *preceeding_commit(struct drm_crtc *crtc) void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_crtc_commit *commit; int i; long ret; - for_each_crtc_in_state(old_state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { spin_lock(&crtc->commit_lock); commit = preceeding_commit(crtc); if (commit) @@ -1564,17 +1561,17 @@ 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_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_crtc_commit *commit; int i; - for_each_crtc_in_state(old_state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { commit = old_state->crtcs[i].commit; if (!commit) continue; /* backend must have consumed any event by now */ - WARN_ON(crtc->state->event); + WARN_ON(new_crtc_state->event); spin_lock(&crtc->commit_lock); complete_all(&commit->hw_done); spin_unlock(&crtc->commit_lock); @@ -1596,12 +1593,12 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_hw_done); void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_crtc_commit *commit; int i; long ret; - for_each_crtc_in_state(old_state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { commit = old_state->crtcs[i].commit; if (WARN_ON(!commit)) continue; @@ -1652,16 +1649,16 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *new_plane_state; int ret, i, j; - for_each_plane_in_state(state, plane, plane_state, i) { + for_each_new_plane_in_state(state, plane, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; funcs = plane->helper_private; if (funcs->prepare_fb) { - ret = funcs->prepare_fb(plane, plane_state); + ret = funcs->prepare_fb(plane, new_plane_state); if (ret) goto fail; } @@ -1670,7 +1667,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev, return 0; fail: - for_each_plane_in_state(state, plane, plane_state, j) { + for_each_new_plane_in_state(state, plane, new_plane_state, j) { const struct drm_plane_helper_funcs *funcs; if (j >= i) @@ -1679,7 +1676,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev, funcs = plane->helper_private; if (funcs->cleanup_fb) - funcs->cleanup_fb(plane, plane_state); + funcs->cleanup_fb(plane, new_plane_state); } return ret; @@ -1737,14 +1734,14 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, uint32_t flags) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; - struct drm_plane_state *old_plane_state; + struct drm_plane_state *old_plane_state, *new_plane_state; int i; bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY; bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET; - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; funcs = crtc->helper_private; @@ -1752,13 +1749,13 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, if (!funcs || !funcs->atomic_begin) continue; - if (active_only && !crtc->state->active) + if (active_only && !new_crtc_state->active) continue; funcs->atomic_begin(crtc, old_crtc_state); } - for_each_plane_in_state(old_state, plane, old_plane_state, i) { + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; bool disabling; @@ -1777,7 +1774,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, * CRTC to avoid skipping planes being disabled on an * active CRTC. */ - if (!disabling && !plane_crtc_active(plane->state)) + if (!disabling && !plane_crtc_active(new_plane_state)) continue; if (disabling && !plane_crtc_active(old_plane_state)) continue; @@ -1796,12 +1793,12 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, continue; funcs->atomic_disable(plane, old_plane_state); - } else if (plane->state->crtc || disabling) { + } else if (new_plane_state->crtc || disabling) { funcs->atomic_update(plane, old_plane_state); } } - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; funcs = crtc->helper_private; @@ -1809,7 +1806,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, if (!funcs || !funcs->atomic_flush) continue; - if (active_only && !crtc->state->active) + if (active_only && !new_crtc_state->active) continue; funcs->atomic_flush(crtc, old_crtc_state); @@ -1936,11 +1933,21 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *old_plane_state, *new_plane_state; int i; - for_each_plane_in_state(old_state, plane, plane_state, i) { + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; + struct drm_plane_state *plane_state; + + /* + * This might be called before swapping when commit is aborted, + * in which case we have to cleanup the new state. + */ + if (old_plane_state == plane->state) + plane_state = new_plane_state; + else + plane_state = old_plane_state; funcs = plane->helper_private; @@ -1986,15 +1993,15 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, int i; long ret; struct drm_connector *connector; - struct drm_connector_state *conn_state, *old_conn_state; + struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state, *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; - struct drm_plane_state *plane_state, *old_plane_state; + struct drm_plane_state *old_plane_state, *new_plane_state; struct drm_crtc_commit *commit; if (stall) { - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { spin_lock(&crtc->commit_lock); commit = list_first_entry_or_null(&crtc->commit_list, struct drm_crtc_commit, commit_entry); @@ -2014,20 +2021,24 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, } } - for_each_oldnew_connector_in_state(state, connector, old_conn_state, conn_state, i) { + for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_conn_state, i) { WARN_ON(connector->state != old_conn_state); - connector->state->state = state; - swap(state->connectors[i].state, connector->state); - connector->state->state = NULL; + old_conn_state->state = state; + new_conn_state->state = NULL; + + state->connectors[i].state = old_conn_state; + connector->state = new_conn_state; } - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { WARN_ON(crtc->state != old_crtc_state); - crtc->state->state = state; - swap(state->crtcs[i].state, crtc->state); - crtc->state->state = NULL; + old_crtc_state->state = state; + new_crtc_state->state = NULL; + + state->crtcs[i].state = old_crtc_state; + crtc->state = new_crtc_state; if (state->crtcs[i].commit) { spin_lock(&crtc->commit_lock); @@ -2039,12 +2050,14 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, } } - for_each_oldnew_plane_in_state(state, plane, old_plane_state, plane_state, i) { + for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { WARN_ON(plane->state != old_plane_state); - plane->state->state = state; - swap(state->planes[i].state, plane->state); - plane->state->state = NULL; + old_plane_state->state = state; + new_plane_state->state = NULL; + + state->planes[i].state = old_plane_state; + plane->state = new_plane_state; } } EXPORT_SYMBOL(drm_atomic_helper_swap_state); @@ -2227,9 +2240,9 @@ static int update_output_state(struct drm_atomic_state *state, { struct drm_device *dev = set->crtc->dev; struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_connector *connector; - struct drm_connector_state *conn_state; + struct drm_connector_state *new_conn_state; int ret, i; ret = drm_modeset_lock(&dev->mode_config.connection_mutex, @@ -2242,31 +2255,32 @@ static int update_output_state(struct drm_atomic_state *state, if (ret) return ret; - for_each_connector_in_state(state, connector, conn_state, i) { - if (conn_state->crtc == set->crtc) { - ret = drm_atomic_set_crtc_for_connector(conn_state, + for_each_new_connector_in_state(state, connector, new_conn_state, i) { + if (new_conn_state->crtc == set->crtc) { + ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL); if (ret) return ret; + /* Make sure legacy setCrtc always re-trains */ - conn_state->link_status = DRM_LINK_STATUS_GOOD; + new_conn_state->link_status = DRM_LINK_STATUS_GOOD; } } /* Then set all connectors from set->connectors on the target crtc */ for (i = 0; i < set->num_connectors; i++) { - conn_state = drm_atomic_get_connector_state(state, + new_conn_state = drm_atomic_get_connector_state(state, set->connectors[i]); - if (IS_ERR(conn_state)) - return PTR_ERR(conn_state); + if (IS_ERR(new_conn_state)) + return PTR_ERR(new_conn_state); - ret = drm_atomic_set_crtc_for_connector(conn_state, + ret = drm_atomic_set_crtc_for_connector(new_conn_state, set->crtc); if (ret) return ret; } - for_each_crtc_in_state(state, crtc, crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { /* Don't update ->enable for the CRTC in the set_config request, * since a mismatch would indicate a bug in the upper layers. * The actual modeset code later on will catch any @@ -2274,13 +2288,13 @@ static int update_output_state(struct drm_atomic_state *state, if (crtc == set->crtc) continue; - if (!crtc_state->connector_mask) { - ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, + if (!new_crtc_state->connector_mask) { + ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state, NULL); if (ret < 0) return ret; - crtc_state->active = false; + new_crtc_state->active = false; } } @@ -2583,21 +2597,21 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state, { int i; struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *new_plane_state; struct drm_connector *connector; - struct drm_connector_state *conn_state; + struct drm_connector_state *new_conn_state; struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; state->acquire_ctx = ctx; - for_each_new_plane_in_state(state, plane, plane_state, i) + for_each_new_plane_in_state(state, plane, new_plane_state, i) state->planes[i].old_state = plane->state; - for_each_new_crtc_in_state(state, crtc, crtc_state, i) + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) state->crtcs[i].old_state = crtc->state; - for_each_new_connector_in_state(state, connector, conn_state, i) + for_each_new_connector_in_state(state, connector, new_conn_state, i) state->connectors[i].old_state = connector->state; return drm_atomic_commit(state); -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3. 2017-03-01 9:21 ` [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3 Maarten Lankhorst @ 2017-03-01 10:06 ` Laurent Pinchart 0 siblings, 0 replies; 15+ messages in thread From: Laurent Pinchart @ 2017-03-01 10:06 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel Hi Maarten, Thank you for the resend. For the whole series, Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> On Wednesday 01 Mar 2017 10:21:26 Maarten Lankhorst wrote: > There are new iterator macros that annotate whether the new or old > state should be used. This is better than using a state that depends on > whether it's called before or after swap. For clarity, also rename the > variables from $obj_state to (old,new)_$obj_state as well. > > Changes since v1: > - Use old/new_*_state for variable names as much as possible. (pinchartl) > - Expand commit message. > Changes since v2: > - Rebase on top of link training patches. > - free -> cleanup (pinchartl) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 446 ++++++++++++++++----------------- > 1 file changed, 230 insertions(+), 216 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index 6b12396f718b..aa1e7609b024 > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -63,14 +63,15 @@ > */ > static void > drm_atomic_helper_plane_changed(struct drm_atomic_state *state, > + struct drm_plane_state *old_plane_state, > struct drm_plane_state *plane_state, > struct drm_plane *plane) > { > struct drm_crtc_state *crtc_state; > > - if (plane->state->crtc) { > + if (old_plane_state->crtc) { > crtc_state = drm_atomic_get_existing_crtc_state(state, > - plane->state- >crtc); > + old_plane_state->crtc); > > if (WARN_ON(!crtc_state)) > return; > @@ -92,7 +93,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state > *state, static int handle_conflicting_encoders(struct drm_atomic_state > *state, bool disable_conflicting_encoders) > { > - struct drm_connector_state *conn_state; > + struct drm_connector_state *new_conn_state; > struct drm_connector *connector; > struct drm_connector_list_iter conn_iter; > struct drm_encoder *encoder; > @@ -104,15 +105,15 @@ static int handle_conflicting_encoders(struct > drm_atomic_state *state, * part of the state. If the same encoder is > assigned to multiple * connectors bail out. > */ > - for_each_connector_in_state(state, connector, conn_state, i) { > + for_each_new_connector_in_state(state, connector, new_conn_state, i) { > const struct drm_connector_helper_funcs *funcs = > connector->helper_private; struct drm_encoder *new_encoder; > > - if (!conn_state->crtc) > + if (!new_conn_state->crtc) > continue; > > if (funcs->atomic_best_encoder) > - new_encoder = funcs->atomic_best_encoder(connector, conn_state); > + new_encoder = funcs->atomic_best_encoder(connector, new_conn_state); > else if (funcs->best_encoder) > new_encoder = funcs->best_encoder(connector); > else > @@ -166,20 +167,20 @@ static int handle_conflicting_encoders(struct > drm_atomic_state *state, goto out; > } > > - conn_state = drm_atomic_get_connector_state(state, connector); > - if (IS_ERR(conn_state)) { > - ret = PTR_ERR(conn_state); > + new_conn_state = drm_atomic_get_connector_state(state, connector); > + if (IS_ERR(new_conn_state)) { > + ret = PTR_ERR(new_conn_state); > goto out; > } > > DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], disabling > [CONNECTOR:%d:%s]\n", encoder->base.id, encoder->name, > - conn_state->crtc->base.id, conn_state->crtc- >name, > + new_conn_state->crtc->base.id, new_conn_state->crtc->name, > connector->base.id, connector->name); > > - crtc_state = drm_atomic_get_existing_crtc_state(state, conn_state->crtc); > + crtc_state = drm_atomic_get_existing_crtc_state(state, > new_conn_state->crtc); > > - ret = drm_atomic_set_crtc_for_connector(conn_state, NULL); > + ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL); > if (ret) > goto out; > > @@ -245,22 +246,22 @@ steal_encoder(struct drm_atomic_state *state, > { > struct drm_crtc_state *crtc_state; > struct drm_connector *connector; > - struct drm_connector_state *connector_state; > + struct drm_connector_state *old_connector_state, *new_connector_state; > int i; > > - for_each_connector_in_state(state, connector, connector_state, i) { > + for_each_oldnew_connector_in_state(state, connector, old_connector_state, > new_connector_state, i) { struct drm_crtc *encoder_crtc; > > - if (connector_state->best_encoder != encoder) > + if (new_connector_state->best_encoder != encoder) > continue; > > - encoder_crtc = connector->state->crtc; > + encoder_crtc = old_connector_state->crtc; > > DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing it\n", > encoder->base.id, encoder->name, > encoder_crtc->base.id, encoder_crtc->name); > > - set_best_encoder(state, connector_state, NULL); > + set_best_encoder(state, new_connector_state, NULL); > > crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc); > crtc_state->connectors_changed = true; > @@ -272,7 +273,8 @@ steal_encoder(struct drm_atomic_state *state, > static int > update_connector_routing(struct drm_atomic_state *state, > struct drm_connector *connector, > - struct drm_connector_state *connector_state) > + struct drm_connector_state *old_connector_state, > + struct drm_connector_state *new_connector_state) > { > const struct drm_connector_helper_funcs *funcs; > struct drm_encoder *new_encoder; > @@ -282,24 +284,24 @@ update_connector_routing(struct drm_atomic_state > *state, connector->base.id, > connector->name); > > - if (connector->state->crtc != connector_state->crtc) { > - if (connector->state->crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(state, > connector->state->crtc); + if (old_connector_state->crtc != > new_connector_state->crtc) { > + if (old_connector_state->crtc) { > + crtc_state = drm_atomic_get_existing_crtc_state(state, > old_connector_state->crtc); crtc_state->connectors_changed = true; > } > > - if (connector_state->crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(state, > connector_state->crtc); + if (new_connector_state->crtc) { > + crtc_state = drm_atomic_get_existing_crtc_state(state, > new_connector_state->crtc); crtc_state->connectors_changed = true; > } > } > > - if (!connector_state->crtc) { > + if (!new_connector_state->crtc) { > DRM_DEBUG_ATOMIC("Disabling [CONNECTOR:%d:%s]\n", > connector->base.id, > connector->name); > > - set_best_encoder(state, connector_state, NULL); > + set_best_encoder(state, new_connector_state, NULL); > > return 0; > } > @@ -308,7 +310,7 @@ update_connector_routing(struct drm_atomic_state *state, > > if (funcs->atomic_best_encoder) > new_encoder = funcs->atomic_best_encoder(connector, > - connector_state); > + new_connector_state); > else if (funcs->best_encoder) > new_encoder = funcs->best_encoder(connector); > else > @@ -321,34 +323,34 @@ update_connector_routing(struct drm_atomic_state > *state, return -EINVAL; > } > > - if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) { > + if (!drm_encoder_crtc_ok(new_encoder, new_connector_state->crtc)) { > DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d: %s]\n", > new_encoder->base.id, > new_encoder->name, > - connector_state->crtc->base.id, > - connector_state->crtc->name); > + new_connector_state->crtc->base.id, > + new_connector_state->crtc->name); > return -EINVAL; > } > > - if (new_encoder == connector_state->best_encoder) { > - set_best_encoder(state, connector_state, new_encoder); > + if (new_encoder == new_connector_state->best_encoder) { > + set_best_encoder(state, new_connector_state, new_encoder); > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] keeps [ENCODER:%d:%s], now on > [CRTC:%d:%s]\n", connector->base.id, > connector->name, > new_encoder->base.id, > new_encoder->name, > - connector_state->crtc->base.id, > - connector_state->crtc->name); > + new_connector_state->crtc->base.id, > + new_connector_state->crtc->name); > > return 0; > } > > steal_encoder(state, new_encoder); > > - set_best_encoder(state, connector_state, new_encoder); > + set_best_encoder(state, new_connector_state, new_encoder); > > - crtc_state = drm_atomic_get_existing_crtc_state(state, > connector_state->crtc); + crtc_state = > drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc); > crtc_state->connectors_changed = true; > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on > [CRTC:%d:%s]\n", @@ -356,8 +358,8 @@ update_connector_routing(struct > drm_atomic_state *state, connector->name, > new_encoder->base.id, > new_encoder->name, > - connector_state->crtc->base.id, > - connector_state->crtc->name); > + new_connector_state->crtc->base.id, > + new_connector_state->crtc->name); > > return 0; > } > @@ -366,57 +368,57 @@ static int > mode_fixup(struct drm_atomic_state *state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *new_crtc_state; > struct drm_connector *connector; > - struct drm_connector_state *conn_state; > + struct drm_connector_state *new_conn_state; > int i; > int ret; > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > - if (!crtc_state->mode_changed && > - !crtc_state->connectors_changed) > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { > + if (!new_crtc_state->mode_changed && > + !new_crtc_state->connectors_changed) > continue; > > - drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode); > + drm_mode_copy(&new_crtc_state->adjusted_mode, &new_crtc_state- >mode); > } > > - for_each_connector_in_state(state, connector, conn_state, i) { > + for_each_new_connector_in_state(state, connector, new_conn_state, i) { > const struct drm_encoder_helper_funcs *funcs; > struct drm_encoder *encoder; > > - WARN_ON(!!conn_state->best_encoder != !!conn_state->crtc); > + WARN_ON(!!new_conn_state->best_encoder != !!new_conn_state- >crtc); > > - if (!conn_state->crtc || !conn_state->best_encoder) > + if (!new_conn_state->crtc || !new_conn_state->best_encoder) > continue; > > - crtc_state = drm_atomic_get_existing_crtc_state(state, > - conn_state- >crtc); > + new_crtc_state = > + drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); > > /* > * Each encoder has at most one connector (since we always steal > * it away), so we won't call ->mode_fixup twice. > */ > - encoder = conn_state->best_encoder; > + encoder = new_conn_state->best_encoder; > funcs = encoder->helper_private; > > - ret = drm_bridge_mode_fixup(encoder->bridge, &crtc_state- >mode, > - &crtc_state->adjusted_mode); > + ret = drm_bridge_mode_fixup(encoder->bridge, &new_crtc_state- >mode, > + &new_crtc_state->adjusted_mode); > if (!ret) { > DRM_DEBUG_ATOMIC("Bridge fixup failed\n"); > return -EINVAL; > } > > if (funcs && funcs->atomic_check) { > - ret = funcs->atomic_check(encoder, crtc_state, > - conn_state); > + ret = funcs->atomic_check(encoder, new_crtc_state, > + new_conn_state); > if (ret) { > DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] check failed\n", > encoder->base.id, encoder- >name); > return ret; > } > } else if (funcs && funcs->mode_fixup) { > - ret = funcs->mode_fixup(encoder, &crtc_state->mode, > - &crtc_state->adjusted_mode); > + ret = funcs->mode_fixup(encoder, &new_crtc_state- >mode, > + &new_crtc_state- >adjusted_mode); > if (!ret) { > DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n", > encoder->base.id, encoder- >name); > @@ -425,22 +427,22 @@ mode_fixup(struct drm_atomic_state *state) > } > } > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { > const struct drm_crtc_helper_funcs *funcs; > > - if (!crtc_state->enable) > + if (!new_crtc_state->enable) > continue; > > - if (!crtc_state->mode_changed && > - !crtc_state->connectors_changed) > + if (!new_crtc_state->mode_changed && > + !new_crtc_state->connectors_changed) > continue; > > funcs = crtc->helper_private; > if (!funcs->mode_fixup) > continue; > > - ret = funcs->mode_fixup(crtc, &crtc_state->mode, > - &crtc_state->adjusted_mode); > + ret = funcs->mode_fixup(crtc, &new_crtc_state->mode, > + &new_crtc_state->adjusted_mode); > if (!ret) { > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] fixup failed\n", > crtc->base.id, crtc->name); > @@ -487,19 +489,19 @@ drm_atomic_helper_check_modeset(struct drm_device > *dev, struct drm_atomic_state *state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct drm_connector *connector; > - struct drm_connector_state *connector_state; > + struct drm_connector_state *old_connector_state, *new_connector_state; > int i, ret; > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > - if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode)) { > + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, > i) { + if (!drm_mode_equal(&old_crtc_state->mode, &new_crtc_state->mode)) > { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] mode changed\n", > crtc->base.id, crtc->name); > - crtc_state->mode_changed = true; > + new_crtc_state->mode_changed = true; > } > > - if (crtc->state->enable != crtc_state->enable) { > + if (old_crtc_state->enable != new_crtc_state->enable) { > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enable changed\n", > crtc->base.id, crtc->name); > > @@ -511,8 +513,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > * The other way around is true as well. enable != 0 > * iff connectors are attached and a mode is set. > */ > - crtc_state->mode_changed = true; > - crtc_state->connectors_changed = true; > + new_crtc_state->mode_changed = true; > + new_crtc_state->connectors_changed = true; > } > } > > @@ -520,22 +522,23 @@ drm_atomic_helper_check_modeset(struct drm_device > *dev, if (ret) > return ret; > > - for_each_connector_in_state(state, connector, connector_state, i) { > + for_each_oldnew_connector_in_state(state, connector, old_connector_state, > new_connector_state, i) { /* > * This only sets crtc->connectors_changed for routing changes, > * drivers must set crtc->connectors_changed themselves when > * connector properties need to be updated. > */ > ret = update_connector_routing(state, connector, > - connector_state); > + old_connector_state, > + new_connector_state); > if (ret) > return ret; > - if (connector->state->crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(state, > - connector->state->crtc); > - if (connector->state->link_status != > - connector_state->link_status) > - crtc_state->connectors_changed = true; > + if (old_connector_state->crtc) { > + new_crtc_state = drm_atomic_get_existing_crtc_state(state, > + old_connector_state->crtc); > + if (old_connector_state->link_status != > + new_connector_state->link_status) > + new_crtc_state->connectors_changed = true; > } > } > > @@ -545,28 +548,28 @@ drm_atomic_helper_check_modeset(struct drm_device > *dev, * configuration. This must be done before calling mode_fixup in case > a * crtc only changed its mode but has the same set of connectors. */ > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, > i) { bool has_connectors = > - !!crtc_state->connector_mask; > + !!new_crtc_state->connector_mask; > > /* > * We must set ->active_changed after walking connectors for > * otherwise an update that only changes active would result in > * a full modeset because update_connector_routing force that. > */ > - if (crtc->state->active != crtc_state->active) { > + if (old_crtc_state->active != new_crtc_state->active) { > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active changed\n", > crtc->base.id, crtc->name); > - crtc_state->active_changed = true; > + new_crtc_state->active_changed = true; > } > > - if (!drm_atomic_crtc_needs_modeset(crtc_state)) > + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) > continue; > > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] needs all connectors, enable: %c, active: > %c\n", crtc->base.id, crtc->name, > - crtc_state->enable ? 'y' : 'n', > - crtc_state->active ? 'y' : 'n'); > + new_crtc_state->enable ? 'y' : 'n', > + new_crtc_state->active ? 'y' : 'n'); > > ret = drm_atomic_add_affected_connectors(state, crtc); > if (ret != 0) > @@ -576,7 +579,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > if (ret != 0) > return ret; > > - if (crtc_state->enable != has_connectors) { > + if (new_crtc_state->enable != has_connectors) { > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n", > crtc->base.id, crtc->name); > > @@ -609,22 +612,22 @@ drm_atomic_helper_check_planes(struct drm_device *dev, > struct drm_atomic_state *state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *new_crtc_state; > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *new_plane_state, *old_plane_state; > int i, ret = 0; > > - for_each_plane_in_state(state, plane, plane_state, i) { > + for_each_oldnew_plane_in_state(state, plane, old_plane_state, > new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; > > funcs = plane->helper_private; > > - drm_atomic_helper_plane_changed(state, plane_state, plane); > + drm_atomic_helper_plane_changed(state, old_plane_state, new_plane_state, > plane); > > if (!funcs || !funcs->atomic_check) > continue; > > - ret = funcs->atomic_check(plane, plane_state); > + ret = funcs->atomic_check(plane, new_plane_state); > if (ret) { > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic driver check failed\n", > plane->base.id, plane->name); > @@ -632,7 +635,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev, > } > } > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { > const struct drm_crtc_helper_funcs *funcs; > > funcs = crtc->helper_private; > @@ -640,7 +643,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev, > if (!funcs || !funcs->atomic_check) > continue; > > - ret = funcs->atomic_check(crtc, crtc_state); > + ret = funcs->atomic_check(crtc, new_crtc_state); > if (ret) { > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n", > crtc->base.id, crtc->name); > @@ -694,12 +697,12 @@ static void > disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) > { > struct drm_connector *connector; > - struct drm_connector_state *old_conn_state; > + struct drm_connector_state *old_conn_state, *new_conn_state; > struct drm_crtc *crtc; > - struct drm_crtc_state *old_crtc_state; > + struct drm_crtc_state *old_crtc_state, *new_crtc_state; > int i; > > - for_each_connector_in_state(old_state, connector, old_conn_state, i) { > + for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, > new_conn_state, i) { const struct drm_encoder_helper_funcs *funcs; > struct drm_encoder *encoder; > > @@ -736,7 +739,7 @@ disable_outputs(struct drm_device *dev, struct > drm_atomic_state *old_state) > > /* Right function depends upon target state. */ > if (funcs) { > - if (connector->state->crtc && funcs->prepare) > + if (new_conn_state->crtc && funcs->prepare) > funcs->prepare(encoder); > else if (funcs->disable) > funcs->disable(encoder); > @@ -747,11 +750,11 @@ disable_outputs(struct drm_device *dev, struct > drm_atomic_state *old_state) drm_bridge_post_disable(encoder->bridge); > } > > - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { > + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, > new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; > > /* Shut down everything that needs a full modeset. */ > - if (!drm_atomic_crtc_needs_modeset(crtc->state)) > + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) > continue; > > if (!old_crtc_state->active) > @@ -764,7 +767,7 @@ disable_outputs(struct drm_device *dev, struct > drm_atomic_state *old_state) > > > /* Right function depends upon target state. */ > - if (crtc->state->enable && funcs->prepare) > + if (new_crtc_state->enable && funcs->prepare) > funcs->prepare(crtc); > else if (funcs->atomic_disable) > funcs->atomic_disable(crtc, old_crtc_state); > @@ -793,13 +796,13 @@ drm_atomic_helper_update_legacy_modeset_state(struct > drm_device *dev, struct drm_atomic_state *old_state) > { > struct drm_connector *connector; > - struct drm_connector_state *old_conn_state; > + struct drm_connector_state *old_conn_state, *new_conn_state; > struct drm_crtc *crtc; > - struct drm_crtc_state *old_crtc_state; > + struct drm_crtc_state *new_crtc_state; > int i; > > /* clear out existing links and update dpms */ > - for_each_connector_in_state(old_state, connector, old_conn_state, i) { > + for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, > new_conn_state, i) { if (connector->encoder) { > WARN_ON(!connector->encoder->crtc); > > @@ -807,7 +810,7 @@ drm_atomic_helper_update_legacy_modeset_state(struct > drm_device *dev, connector->encoder = NULL; > } > > - crtc = connector->state->crtc; > + crtc = new_conn_state->crtc; > if ((!crtc && old_conn_state->crtc) || > (crtc && drm_atomic_crtc_needs_modeset(crtc->state))) { > struct drm_property *dpms_prop = > @@ -824,23 +827,23 @@ drm_atomic_helper_update_legacy_modeset_state(struct > drm_device *dev, } > > /* set new links */ > - for_each_connector_in_state(old_state, connector, old_conn_state, i) { > - if (!connector->state->crtc) > + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { > + if (!new_conn_state->crtc) > continue; > > - if (WARN_ON(!connector->state->best_encoder)) > + if (WARN_ON(!new_conn_state->best_encoder)) > continue; > > - connector->encoder = connector->state->best_encoder; > - connector->encoder->crtc = connector->state->crtc; > + connector->encoder = new_conn_state->best_encoder; > + connector->encoder->crtc = new_conn_state->crtc; > } > > /* set legacy state in the crtc structure */ > - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { > + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > struct drm_plane *primary = crtc->primary; > > - crtc->mode = crtc->state->mode; > - crtc->enabled = crtc->state->enable; > + crtc->mode = new_crtc_state->mode; > + crtc->enabled = new_crtc_state->enable; > > if (drm_atomic_get_existing_plane_state(old_state, primary) && > primary->state->crtc == crtc) { > @@ -848,9 +851,9 @@ drm_atomic_helper_update_legacy_modeset_state(struct > drm_device *dev, crtc->y = primary->state->src_y >> 16; > } > > - if (crtc->state->enable) > + if (new_crtc_state->enable) > drm_calc_timestamping_constants(crtc, > - &crtc->state- >adjusted_mode); > + &new_crtc_state- >adjusted_mode); > } > } > EXPORT_SYMBOL(drm_atomic_helper_update_legacy_modeset_state); > @@ -859,20 +862,20 @@ static void > crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *old_crtc_state; > + struct drm_crtc_state *new_crtc_state; > struct drm_connector *connector; > - struct drm_connector_state *old_conn_state; > + struct drm_connector_state *new_conn_state; > int i; > > - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { > + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > const struct drm_crtc_helper_funcs *funcs; > > - if (!crtc->state->mode_changed) > + if (!new_crtc_state->mode_changed) > continue; > > funcs = crtc->helper_private; > > - if (crtc->state->enable && funcs->mode_set_nofb) { > + if (new_crtc_state->enable && funcs->mode_set_nofb) { > DRM_DEBUG_ATOMIC("modeset on [CRTC:%d:%s]\n", > crtc->base.id, crtc->name); > > @@ -880,18 +883,17 @@ crtc_set_mode(struct drm_device *dev, struct > drm_atomic_state *old_state) } > } > > - for_each_connector_in_state(old_state, connector, old_conn_state, i) { > + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { > const struct drm_encoder_helper_funcs *funcs; > - struct drm_crtc_state *new_crtc_state; > struct drm_encoder *encoder; > struct drm_display_mode *mode, *adjusted_mode; > > - if (!connector->state->best_encoder) > + if (!new_conn_state->best_encoder) > continue; > > - encoder = connector->state->best_encoder; > + encoder = new_conn_state->best_encoder; > funcs = encoder->helper_private; > - new_crtc_state = connector->state->crtc->state; > + new_crtc_state = new_conn_state->crtc->state; > mode = &new_crtc_state->mode; > adjusted_mode = &new_crtc_state->adjusted_mode; > > @@ -907,7 +909,7 @@ crtc_set_mode(struct drm_device *dev, struct > drm_atomic_state *old_state) */ > if (funcs && funcs->atomic_mode_set) { > funcs->atomic_mode_set(encoder, new_crtc_state, > - connector->state); > + new_conn_state); > } else if (funcs && funcs->mode_set) { > funcs->mode_set(encoder, mode, adjusted_mode); > } > @@ -959,24 +961,24 @@ void drm_atomic_helper_commit_modeset_enables(struct > drm_device *dev, struct drm_atomic_state *old_state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *old_crtc_state; > + struct drm_crtc_state *new_crtc_state; > struct drm_connector *connector; > - struct drm_connector_state *old_conn_state; > + struct drm_connector_state *new_conn_state; > int i; > > - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { > + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > const struct drm_crtc_helper_funcs *funcs; > > /* Need to filter out CRTCs where only planes change. */ > - if (!drm_atomic_crtc_needs_modeset(crtc->state)) > + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) > continue; > > - if (!crtc->state->active) > + if (!new_crtc_state->active) > continue; > > funcs = crtc->helper_private; > > - if (crtc->state->enable) { > + if (new_crtc_state->enable) { > DRM_DEBUG_ATOMIC("enabling [CRTC:%d:%s]\n", > crtc->base.id, crtc->name); > > @@ -987,18 +989,18 @@ void drm_atomic_helper_commit_modeset_enables(struct > drm_device *dev, } > } > > - for_each_connector_in_state(old_state, connector, old_conn_state, i) { > + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { > const struct drm_encoder_helper_funcs *funcs; > struct drm_encoder *encoder; > > - if (!connector->state->best_encoder) > + if (!new_conn_state->best_encoder) > continue; > > - if (!connector->state->crtc->state->active || > - !drm_atomic_crtc_needs_modeset(connector->state->crtc- >state)) > + if (!new_conn_state->crtc->state->active || > + !drm_atomic_crtc_needs_modeset(new_conn_state->crtc- >state)) > continue; > > - encoder = connector->state->best_encoder; > + encoder = new_conn_state->best_encoder; > funcs = encoder->helper_private; > > DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n", > @@ -1048,29 +1050,26 @@ int drm_atomic_helper_wait_for_fences(struct > drm_device *dev, bool pre_swap) > { > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *new_plane_state; > int i, ret; > > - for_each_plane_in_state(state, plane, plane_state, i) { > - if (!pre_swap) > - plane_state = plane->state; > - > - if (!plane_state->fence) > + for_each_new_plane_in_state(state, plane, new_plane_state, i) { > + if (!new_plane_state->fence) > continue; > > - WARN_ON(!plane_state->fb); > + WARN_ON(!new_plane_state->fb); > > /* > * If waiting for fences pre-swap (ie: nonblock), userspace can > * still interrupt the operation. Instead of blocking until the > * timer expires, make the wait interruptible. > */ > - ret = dma_fence_wait(plane_state->fence, pre_swap); > + ret = dma_fence_wait(new_plane_state->fence, pre_swap); > if (ret) > return ret; > > - dma_fence_put(plane_state->fence); > - plane_state->fence = NULL; > + dma_fence_put(new_plane_state->fence); > + new_plane_state->fence = NULL; > } > > return 0; > @@ -1093,7 +1092,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device > *dev, struct drm_atomic_state *old_state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *old_crtc_state; > + struct drm_crtc_state *old_crtc_state, *new_crtc_state; > int i, ret; > unsigned crtc_mask = 0; > > @@ -1104,9 +1103,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device > *dev, if (old_state->legacy_cursor_update) > return; > > - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { > - struct drm_crtc_state *new_crtc_state = crtc->state; > - > + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, > new_crtc_state, i) { if (!new_crtc_state->active || > !new_crtc_state->planes_changed) continue; > > @@ -1118,7 +1115,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device > *dev, old_state->crtcs[i].last_vblank_count = drm_crtc_vblank_count(crtc); > } > > - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { > + for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { > if (!(crtc_mask & drm_crtc_mask(crtc))) > continue; > > @@ -1427,11 +1424,11 @@ int drm_atomic_helper_setup_commit(struct > drm_atomic_state *state, bool nonblock) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct drm_crtc_commit *commit; > int i, ret; > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, > i) { commit = kzalloc(sizeof(*commit), GFP_KERNEL); > if (!commit) > return -ENOMEM; > @@ -1452,7 +1449,7 @@ int drm_atomic_helper_setup_commit(struct > drm_atomic_state *state, /* Drivers only send out events when at least > either current or * new CRTC state is active. Complete right away if > everything > * stays off. */ > - if (!crtc->state->active && !crtc_state->active) { > + if (!old_crtc_state->active && !new_crtc_state->active) { > complete_all(&commit->flip_done); > continue; > } > @@ -1463,17 +1460,17 @@ int drm_atomic_helper_setup_commit(struct > drm_atomic_state *state, continue; > } > > - if (!crtc_state->event) { > + if (!new_crtc_state->event) { > commit->event = kzalloc(sizeof(*commit->event), > GFP_KERNEL); > if (!commit->event) > return -ENOMEM; > > - crtc_state->event = commit->event; > + new_crtc_state->event = commit->event; > } > > - crtc_state->event->base.completion = &commit->flip_done; > - crtc_state->event->base.completion_release = release_crtc_commit; > + new_crtc_state->event->base.completion = &commit->flip_done; > + new_crtc_state->event->base.completion_release = release_crtc_commit; > drm_crtc_commit_get(commit); > } > > @@ -1512,12 +1509,12 @@ static struct drm_crtc_commit > *preceeding_commit(struct drm_crtc *crtc) void > drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *new_crtc_state; > struct drm_crtc_commit *commit; > int i; > long ret; > > - for_each_crtc_in_state(old_state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > spin_lock(&crtc->commit_lock); > commit = preceeding_commit(crtc); > if (commit) > @@ -1564,17 +1561,17 @@ > 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_crtc_state *crtc_state; > + struct drm_crtc_state *new_crtc_state; > struct drm_crtc_commit *commit; > int i; > > - for_each_crtc_in_state(old_state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > commit = old_state->crtcs[i].commit; > if (!commit) > continue; > > /* backend must have consumed any event by now */ > - WARN_ON(crtc->state->event); > + WARN_ON(new_crtc_state->event); > spin_lock(&crtc->commit_lock); > complete_all(&commit->hw_done); > spin_unlock(&crtc->commit_lock); > @@ -1596,12 +1593,12 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_hw_done); > void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state > *old_state) { > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *new_crtc_state; > struct drm_crtc_commit *commit; > int i; > long ret; > > - for_each_crtc_in_state(old_state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > commit = old_state->crtcs[i].commit; > if (WARN_ON(!commit)) > continue; > @@ -1652,16 +1649,16 @@ int drm_atomic_helper_prepare_planes(struct > drm_device *dev, struct drm_atomic_state *state) > { > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *new_plane_state; > int ret, i, j; > > - for_each_plane_in_state(state, plane, plane_state, i) { > + for_each_new_plane_in_state(state, plane, new_plane_state, i) { > const struct drm_plane_helper_funcs *funcs; > > funcs = plane->helper_private; > > if (funcs->prepare_fb) { > - ret = funcs->prepare_fb(plane, plane_state); > + ret = funcs->prepare_fb(plane, new_plane_state); > if (ret) > goto fail; > } > @@ -1670,7 +1667,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device > *dev, return 0; > > fail: > - for_each_plane_in_state(state, plane, plane_state, j) { > + for_each_new_plane_in_state(state, plane, new_plane_state, j) { > const struct drm_plane_helper_funcs *funcs; > > if (j >= i) > @@ -1679,7 +1676,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device > *dev, funcs = plane->helper_private; > > if (funcs->cleanup_fb) > - funcs->cleanup_fb(plane, plane_state); > + funcs->cleanup_fb(plane, new_plane_state); > } > > return ret; > @@ -1737,14 +1734,14 @@ void drm_atomic_helper_commit_planes(struct > drm_device *dev, uint32_t flags) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *old_crtc_state; > + struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct drm_plane *plane; > - struct drm_plane_state *old_plane_state; > + struct drm_plane_state *old_plane_state, *new_plane_state; > int i; > bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY; > bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET; > > - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { > + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, > new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; > > funcs = crtc->helper_private; > @@ -1752,13 +1749,13 @@ void drm_atomic_helper_commit_planes(struct > drm_device *dev, if (!funcs || !funcs->atomic_begin) > continue; > > - if (active_only && !crtc->state->active) > + if (active_only && !new_crtc_state->active) > continue; > > funcs->atomic_begin(crtc, old_crtc_state); > } > > - for_each_plane_in_state(old_state, plane, old_plane_state, i) { > + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, > new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; > bool disabling; > > @@ -1777,7 +1774,7 @@ void drm_atomic_helper_commit_planes(struct drm_device > *dev, * CRTC to avoid skipping planes being disabled on an > * active CRTC. > */ > - if (!disabling && !plane_crtc_active(plane->state)) > + if (!disabling && !plane_crtc_active(new_plane_state)) > continue; > if (disabling && !plane_crtc_active(old_plane_state)) > continue; > @@ -1796,12 +1793,12 @@ void drm_atomic_helper_commit_planes(struct > drm_device *dev, continue; > > funcs->atomic_disable(plane, old_plane_state); > - } else if (plane->state->crtc || disabling) { > + } else if (new_plane_state->crtc || disabling) { > funcs->atomic_update(plane, old_plane_state); > } > } > > - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { > + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, > new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; > > funcs = crtc->helper_private; > @@ -1809,7 +1806,7 @@ void drm_atomic_helper_commit_planes(struct drm_device > *dev, if (!funcs || !funcs->atomic_flush) > continue; > > - if (active_only && !crtc->state->active) > + if (active_only && !new_crtc_state->active) > continue; > > funcs->atomic_flush(crtc, old_crtc_state); > @@ -1936,11 +1933,21 @@ void drm_atomic_helper_cleanup_planes(struct > drm_device *dev, struct drm_atomic_state *old_state) > { > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *old_plane_state, *new_plane_state; > int i; > > - for_each_plane_in_state(old_state, plane, plane_state, i) { > + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, > new_plane_state, i) { const struct drm_plane_helper_funcs *funcs; > + struct drm_plane_state *plane_state; > + > + /* > + * This might be called before swapping when commit is aborted, > + * in which case we have to cleanup the new state. > + */ > + if (old_plane_state == plane->state) > + plane_state = new_plane_state; > + else > + plane_state = old_plane_state; > > funcs = plane->helper_private; > > @@ -1986,15 +1993,15 @@ void drm_atomic_helper_swap_state(struct > drm_atomic_state *state, int i; > long ret; > struct drm_connector *connector; > - struct drm_connector_state *conn_state, *old_conn_state; > + struct drm_connector_state *old_conn_state, *new_conn_state; > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state, *old_crtc_state; > + struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct drm_plane *plane; > - struct drm_plane_state *plane_state, *old_plane_state; > + struct drm_plane_state *old_plane_state, *new_plane_state; > struct drm_crtc_commit *commit; > > if (stall) { > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { > spin_lock(&crtc->commit_lock); > commit = list_first_entry_or_null(&crtc->commit_list, > struct drm_crtc_commit, commit_entry); > @@ -2014,20 +2021,24 @@ void drm_atomic_helper_swap_state(struct > drm_atomic_state *state, } > } > > - for_each_oldnew_connector_in_state(state, connector, old_conn_state, > conn_state, i) { + for_each_oldnew_connector_in_state(state, connector, > old_conn_state, new_conn_state, i) { WARN_ON(connector->state != > old_conn_state); > > - connector->state->state = state; > - swap(state->connectors[i].state, connector->state); > - connector->state->state = NULL; > + old_conn_state->state = state; > + new_conn_state->state = NULL; > + > + state->connectors[i].state = old_conn_state; > + connector->state = new_conn_state; > } > > - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) > { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { WARN_ON(crtc->state != old_crtc_state); > > - crtc->state->state = state; > - swap(state->crtcs[i].state, crtc->state); > - crtc->state->state = NULL; > + old_crtc_state->state = state; > + new_crtc_state->state = NULL; > + > + state->crtcs[i].state = old_crtc_state; > + crtc->state = new_crtc_state; > > if (state->crtcs[i].commit) { > spin_lock(&crtc->commit_lock); > @@ -2039,12 +2050,14 @@ void drm_atomic_helper_swap_state(struct > drm_atomic_state *state, } > } > > - for_each_oldnew_plane_in_state(state, plane, old_plane_state, plane_state, > i) { + for_each_oldnew_plane_in_state(state, plane, old_plane_state, > new_plane_state, i) { WARN_ON(plane->state != old_plane_state); > > - plane->state->state = state; > - swap(state->planes[i].state, plane->state); > - plane->state->state = NULL; > + old_plane_state->state = state; > + new_plane_state->state = NULL; > + > + state->planes[i].state = old_plane_state; > + plane->state = new_plane_state; > } > } > EXPORT_SYMBOL(drm_atomic_helper_swap_state); > @@ -2227,9 +2240,9 @@ static int update_output_state(struct drm_atomic_state > *state, { > struct drm_device *dev = set->crtc->dev; > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *new_crtc_state; > struct drm_connector *connector; > - struct drm_connector_state *conn_state; > + struct drm_connector_state *new_conn_state; > int ret, i; > > ret = drm_modeset_lock(&dev->mode_config.connection_mutex, > @@ -2242,31 +2255,32 @@ static int update_output_state(struct > drm_atomic_state *state, if (ret) > return ret; > > - for_each_connector_in_state(state, connector, conn_state, i) { > - if (conn_state->crtc == set->crtc) { > - ret = drm_atomic_set_crtc_for_connector(conn_state, > + for_each_new_connector_in_state(state, connector, new_conn_state, i) { > + if (new_conn_state->crtc == set->crtc) { > + ret = drm_atomic_set_crtc_for_connector(new_conn_state, > NULL); > if (ret) > return ret; > + > /* Make sure legacy setCrtc always re-trains */ > - conn_state->link_status = DRM_LINK_STATUS_GOOD; > + new_conn_state->link_status = DRM_LINK_STATUS_GOOD; > } > } > > /* Then set all connectors from set->connectors on the target crtc */ > for (i = 0; i < set->num_connectors; i++) { > - conn_state = drm_atomic_get_connector_state(state, > + new_conn_state = drm_atomic_get_connector_state(state, > set- >connectors[i]); > - if (IS_ERR(conn_state)) > - return PTR_ERR(conn_state); > + if (IS_ERR(new_conn_state)) > + return PTR_ERR(new_conn_state); > > - ret = drm_atomic_set_crtc_for_connector(conn_state, > + ret = drm_atomic_set_crtc_for_connector(new_conn_state, > set->crtc); > if (ret) > return ret; > } > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { > /* Don't update ->enable for the CRTC in the set_config request, > * since a mismatch would indicate a bug in the upper layers. > * The actual modeset code later on will catch any > @@ -2274,13 +2288,13 @@ static int update_output_state(struct > drm_atomic_state *state, if (crtc == set->crtc) > continue; > > - if (!crtc_state->connector_mask) { > - ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, > + if (!new_crtc_state->connector_mask) { > + ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state, > NULL); > if (ret < 0) > return ret; > > - crtc_state->active = false; > + new_crtc_state->active = false; > } > } > > @@ -2583,21 +2597,21 @@ int drm_atomic_helper_commit_duplicated_state(struct > drm_atomic_state *state, { > int i; > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *new_plane_state; > struct drm_connector *connector; > - struct drm_connector_state *conn_state; > + struct drm_connector_state *new_conn_state; > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *new_crtc_state; > > state->acquire_ctx = ctx; > > - for_each_new_plane_in_state(state, plane, plane_state, i) > + for_each_new_plane_in_state(state, plane, new_plane_state, i) > state->planes[i].old_state = plane->state; > > - for_each_new_crtc_in_state(state, crtc, crtc_state, i) > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) > state->crtcs[i].old_state = crtc->state; > > - for_each_new_connector_in_state(state, connector, conn_state, i) > + for_each_new_connector_in_state(state, connector, new_conn_state, i) > state->connectors[i].old_state = connector->state; > > return drm_atomic_commit(state); -- Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand. 2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst 2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst @ 2017-02-16 14:47 ` Maarten Lankhorst 2017-03-01 0:53 ` Laurent Pinchart 2017-02-16 14:47 ` [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2 Maarten Lankhorst ` (2 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx This function becomes a lot simpler when having passed both the old and new state to it. Looking at all callers, it seems that old_plane_state is never NULL so the check can be dropped. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/drm_atomic_helper.c | 7 ++++--- drivers/gpu/drm/drm_plane_helper.c | 2 +- include/drm/drm_atomic_helper.h | 26 ++++++++------------------ 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 7d432d9a18cf..ea544bddc29b 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1757,7 +1757,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, if (!funcs) continue; - disabling = drm_atomic_plane_disabling(plane, old_plane_state); + disabling = drm_atomic_plane_disabling(old_plane_state, + new_plane_state); if (active_only) { /* @@ -1852,11 +1853,11 @@ drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state) WARN_ON(plane->state->crtc && plane->state->crtc != crtc); - if (drm_atomic_plane_disabling(plane, old_plane_state) && + if (drm_atomic_plane_disabling(old_plane_state, plane->state) && plane_funcs->atomic_disable) plane_funcs->atomic_disable(plane, old_plane_state); else if (plane->state->crtc || - drm_atomic_plane_disabling(plane, old_plane_state)) + drm_atomic_plane_disabling(old_plane_state, plane->state)) plane_funcs->atomic_update(plane, old_plane_state); } diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index 148688fb920a..f4d70dd5e3e4 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -470,7 +470,7 @@ int drm_plane_helper_commit(struct drm_plane *plane, * Drivers may optionally implement the ->atomic_disable callback, so * special-case that here. */ - if (drm_atomic_plane_disabling(plane, plane_state) && + if (drm_atomic_plane_disabling(plane_state, plane->state) && plane_funcs->atomic_disable) plane_funcs->atomic_disable(plane, plane_state); else diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 9ceda379ce58..dc16274987c7 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -220,10 +220,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, __drm_atomic_get_current_plane_state((crtc_state)->state, \ plane))) -/* +/** * drm_atomic_plane_disabling - check whether a plane is being disabled - * @plane: plane object - * @old_state: previous atomic state + * @old_plane_state: old atomic plane state + * @new_plane_state: new atomic plane state * * Checks the atomic state of a plane to determine whether it's being disabled * or not. This also WARNs if it detects an invalid state (both CRTC and FB @@ -233,28 +233,18 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, * True if the plane is being disabled, false otherwise. */ static inline bool -drm_atomic_plane_disabling(struct drm_plane *plane, - struct drm_plane_state *old_state) +drm_atomic_plane_disabling(struct drm_plane_state *old_plane_state, + struct drm_plane_state *new_plane_state) { /* * When disabling a plane, CRTC and FB should always be NULL together. * Anything else should be considered a bug in the atomic core, so we * gently warn about it. */ - WARN_ON((plane->state->crtc == NULL && plane->state->fb != NULL) || - (plane->state->crtc != NULL && plane->state->fb == NULL)); + WARN_ON((new_plane_state->crtc == NULL && new_plane_state->fb != NULL) || + (new_plane_state->crtc != NULL && new_plane_state->fb == NULL)); - /* - * When using the transitional helpers, old_state may be NULL. If so, - * we know nothing about the current state and have to assume that it - * might be enabled. - * - * When using the atomic helpers, old_state won't be NULL. Therefore - * this check assumes that either the driver will have reconstructed - * the correct state in ->reset() or that the driver will have taken - * appropriate measures to disable all planes. - */ - return (!old_state || old_state->crtc) && !plane->state->crtc; + return old_plane_state->crtc && !new_plane_state->crtc; } #endif /* DRM_ATOMIC_HELPER_H_ */ -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand. 2017-02-16 14:47 ` [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand Maarten Lankhorst @ 2017-03-01 0:53 ` Laurent Pinchart 0 siblings, 0 replies; 15+ messages in thread From: Laurent Pinchart @ 2017-03-01 0:53 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx Hi Maarten, Thank you for the patch. On Thursday 16 Feb 2017 15:47:07 Maarten Lankhorst wrote: > This function becomes a lot simpler when having passed both the old and > new state to it. Looking at all callers, it seems that old_plane_state > is never NULL so the check can be dropped. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 7 ++++--- > drivers/gpu/drm/drm_plane_helper.c | 2 +- > include/drm/drm_atomic_helper.h | 26 ++++++++------------------ > 3 files changed, 13 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index 7d432d9a18cf..ea544bddc29b > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1757,7 +1757,8 @@ void drm_atomic_helper_commit_planes(struct drm_device > *dev, if (!funcs) > continue; > > - disabling = drm_atomic_plane_disabling(plane, old_plane_state); > + disabling = drm_atomic_plane_disabling(old_plane_state, > + new_plane_state); > > if (active_only) { > /* > @@ -1852,11 +1853,11 @@ drm_atomic_helper_commit_planes_on_crtc(struct > drm_crtc_state *old_crtc_state) > > WARN_ON(plane->state->crtc && plane->state->crtc != crtc); > > - if (drm_atomic_plane_disabling(plane, old_plane_state) && > + if (drm_atomic_plane_disabling(old_plane_state, plane->state) && > plane_funcs->atomic_disable) > plane_funcs->atomic_disable(plane, old_plane_state); > else if (plane->state->crtc || > - drm_atomic_plane_disabling(plane, old_plane_state)) > + drm_atomic_plane_disabling(old_plane_state, plane- >state)) > plane_funcs->atomic_update(plane, old_plane_state); > } > > diff --git a/drivers/gpu/drm/drm_plane_helper.c > b/drivers/gpu/drm/drm_plane_helper.c index 148688fb920a..f4d70dd5e3e4 > 100644 > --- a/drivers/gpu/drm/drm_plane_helper.c > +++ b/drivers/gpu/drm/drm_plane_helper.c > @@ -470,7 +470,7 @@ int drm_plane_helper_commit(struct drm_plane *plane, > * Drivers may optionally implement the ->atomic_disable callback, so > * special-case that here. > */ > - if (drm_atomic_plane_disabling(plane, plane_state) && > + if (drm_atomic_plane_disabling(plane_state, plane->state) && > plane_funcs->atomic_disable) > plane_funcs->atomic_disable(plane, plane_state); > else > diff --git a/include/drm/drm_atomic_helper.h > b/include/drm/drm_atomic_helper.h index 9ceda379ce58..dc16274987c7 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -220,10 +220,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc > *crtc, __drm_atomic_get_current_plane_state((crtc_state)->state, \ plane))) > > -/* > +/** > * drm_atomic_plane_disabling - check whether a plane is being disabled > - * @plane: plane object > - * @old_state: previous atomic state > + * @old_plane_state: old atomic plane state > + * @new_plane_state: new atomic plane state > * > * Checks the atomic state of a plane to determine whether it's being > disabled * or not. This also WARNs if it detects an invalid state (both > CRTC and FB @@ -233,28 +233,18 @@ int > drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, * True if the > plane is being disabled, false otherwise. > */ > static inline bool > -drm_atomic_plane_disabling(struct drm_plane *plane, > - struct drm_plane_state *old_state) > +drm_atomic_plane_disabling(struct drm_plane_state *old_plane_state, > + struct drm_plane_state *new_plane_state) > { > /* > * When disabling a plane, CRTC and FB should always be NULL together. > * Anything else should be considered a bug in the atomic core, so we > * gently warn about it. > */ > - WARN_ON((plane->state->crtc == NULL && plane->state->fb != NULL) || > - (plane->state->crtc != NULL && plane->state->fb == NULL)); > + WARN_ON((new_plane_state->crtc == NULL && new_plane_state->fb != NULL) || > + (new_plane_state->crtc != NULL && new_plane_state->fb == NULL)); > > - /* > - * When using the transitional helpers, old_state may be NULL. If so, > - * we know nothing about the current state and have to assume that it > - * might be enabled. > - * > - * When using the atomic helpers, old_state won't be NULL. Therefore > - * this check assumes that either the driver will have reconstructed > - * the correct state in ->reset() or that the driver will have taken > - * appropriate measures to disable all planes. > - */ > - return (!old_state || old_state->crtc) && !plane->state->crtc; > + return old_plane_state->crtc && !new_plane_state->crtc; > } > > #endif /* DRM_ATOMIC_HELPER_H_ */ -- Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2. 2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst 2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst 2017-02-16 14:47 ` [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand Maarten Lankhorst @ 2017-02-16 14:47 ` Maarten Lankhorst 2017-03-01 0:56 ` Laurent Pinchart 2017-02-16 14:47 ` [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3 Maarten Lankhorst 2017-02-16 14:47 ` [PATCH v4 5/5] drm/blend: Use new atomic iterator macros Maarten Lankhorst 4 siblings, 1 reply; 15+ messages in thread From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx After atomic commit, these macros should be used in place of get_existing_state. Also after commit get_xx_state should no longer be used because it may not have the required locks. The calls to drm_atomic_get_existing_$obj_state should no longer be used, and converted over to these new calls. Changes since v1: - Expand commit message. - Deprecate get_existing_*_state functions in the documentation. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- include/drm/drm_atomic.h | 108 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index c6f355a970d2..0147a047878d 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -277,6 +277,9 @@ int drm_atomic_connector_set_property(struct drm_connector *connector, * * This function returns the crtc state for the given crtc, or NULL * if the crtc is not part of the global atomic state. + * + * This function is deprecated, @drm_atomic_get_old_crtc_state or + * @drm_atomic_get_new_crtc_state should be used instead. */ static inline struct drm_crtc_state * drm_atomic_get_existing_crtc_state(struct drm_atomic_state *state, @@ -286,12 +289,44 @@ drm_atomic_get_existing_crtc_state(struct drm_atomic_state *state, } /** + * drm_atomic_get_old_crtc_state - get old crtc state, if it exists + * @state: global atomic state object + * @crtc: crtc to grab + * + * This function returns the old crtc state for the given crtc, or + * NULL if the crtc is not part of the global atomic state. + */ +static inline struct drm_crtc_state * +drm_atomic_get_old_crtc_state(struct drm_atomic_state *state, + struct drm_crtc *crtc) +{ + return state->crtcs[drm_crtc_index(crtc)].old_state; +} +/** + * drm_atomic_get_new_crtc_state - get new crtc state, if it exists + * @state: global atomic state object + * @crtc: crtc to grab + * + * This function returns the new crtc state for the given crtc, or + * NULL if the crtc is not part of the global atomic state. + */ +static inline struct drm_crtc_state * +drm_atomic_get_new_crtc_state(struct drm_atomic_state *state, + struct drm_crtc *crtc) +{ + return state->crtcs[drm_crtc_index(crtc)].new_state; +} + +/** * drm_atomic_get_existing_plane_state - get plane state, if it exists * @state: global atomic state object * @plane: plane to grab * * This function returns the plane state for the given plane, or NULL * if the plane is not part of the global atomic state. + * + * This function is deprecated, @drm_atomic_get_old_plane_state or + * @drm_atomic_get_new_plane_state should be used instead. */ static inline struct drm_plane_state * drm_atomic_get_existing_plane_state(struct drm_atomic_state *state, @@ -301,12 +336,45 @@ drm_atomic_get_existing_plane_state(struct drm_atomic_state *state, } /** + * drm_atomic_get_old_plane_state - get plane state, if it exists + * @state: global atomic state object + * @plane: plane to grab + * + * This function returns the old plane state for the given plane, or + * NULL if the plane is not part of the global atomic state. + */ +static inline struct drm_plane_state * +drm_atomic_get_old_plane_state(struct drm_atomic_state *state, + struct drm_plane *plane) +{ + return state->planes[drm_plane_index(plane)].old_state; +} + +/** + * drm_atomic_get_new_plane_state - get plane state, if it exists + * @state: global atomic state object + * @plane: plane to grab + * + * This function returns the new plane state for the given plane, or + * NULL if the plane is not part of the global atomic state. + */ +static inline struct drm_plane_state * +drm_atomic_get_new_plane_state(struct drm_atomic_state *state, + struct drm_plane *plane) +{ + return state->planes[drm_plane_index(plane)].new_state; +} + +/** * drm_atomic_get_existing_connector_state - get connector state, if it exists * @state: global atomic state object * @connector: connector to grab * * This function returns the connector state for the given connector, * or NULL if the connector is not part of the global atomic state. + * + * This function is deprecated, @drm_atomic_get_old_connector_state or + * @drm_atomic_get_new_connector_state should be used instead. */ static inline struct drm_connector_state * drm_atomic_get_existing_connector_state(struct drm_atomic_state *state, @@ -321,6 +389,46 @@ drm_atomic_get_existing_connector_state(struct drm_atomic_state *state, } /** + * drm_atomic_get_old_connector_state - get connector state, if it exists + * @state: global atomic state object + * @connector: connector to grab + * + * This function returns the old connector state for the given connector, + * or NULL if the connector is not part of the global atomic state. + */ +static inline struct drm_connector_state * +drm_atomic_get_old_connector_state(struct drm_atomic_state *state, + struct drm_connector *connector) +{ + int index = drm_connector_index(connector); + + if (index >= state->num_connector) + return NULL; + + return state->connectors[index].old_state; +} + +/** + * drm_atomic_get_new_connector_state - get connector state, if it exists + * @state: global atomic state object + * @connector: connector to grab + * + * This function returns the new connector state for the given connector, + * or NULL if the connector is not part of the global atomic state. + */ +static inline struct drm_connector_state * +drm_atomic_get_new_connector_state(struct drm_atomic_state *state, + struct drm_connector *connector) +{ + int index = drm_connector_index(connector); + + if (index >= state->num_connector) + return NULL; + + return state->connectors[index].new_state; +} + +/** * __drm_atomic_get_current_plane_state - get current plane state * @state: global atomic state object * @plane: plane to grab -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2. 2017-02-16 14:47 ` [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2 Maarten Lankhorst @ 2017-03-01 0:56 ` Laurent Pinchart 0 siblings, 0 replies; 15+ messages in thread From: Laurent Pinchart @ 2017-03-01 0:56 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx Hi Maarten, Thank you for the patch. On Thursday 16 Feb 2017 15:47:08 Maarten Lankhorst wrote: > After atomic commit, these macros should be used in place of > get_existing_state. Also after commit get_xx_state should no longer > be used because it may not have the required locks. > > The calls to drm_atomic_get_existing_$obj_state should no longer be > used, and converted over to these new calls. > > Changes since v1: > - Expand commit message. > - Deprecate get_existing_*_state functions in the documentation. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/drm/drm_atomic.h | 108 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 108 insertions(+) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index c6f355a970d2..0147a047878d 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -277,6 +277,9 @@ int drm_atomic_connector_set_property(struct > drm_connector *connector, * > * This function returns the crtc state for the given crtc, or NULL > * if the crtc is not part of the global atomic state. > + * > + * This function is deprecated, @drm_atomic_get_old_crtc_state or > + * @drm_atomic_get_new_crtc_state should be used instead. > */ > static inline struct drm_crtc_state * > drm_atomic_get_existing_crtc_state(struct drm_atomic_state *state, > @@ -286,12 +289,44 @@ drm_atomic_get_existing_crtc_state(struct > drm_atomic_state *state, } > > /** > + * drm_atomic_get_old_crtc_state - get old crtc state, if it exists > + * @state: global atomic state object > + * @crtc: crtc to grab > + * > + * This function returns the old crtc state for the given crtc, or > + * NULL if the crtc is not part of the global atomic state. > + */ > +static inline struct drm_crtc_state * > +drm_atomic_get_old_crtc_state(struct drm_atomic_state *state, > + struct drm_crtc *crtc) > +{ > + return state->crtcs[drm_crtc_index(crtc)].old_state; > +} > +/** > + * drm_atomic_get_new_crtc_state - get new crtc state, if it exists > + * @state: global atomic state object > + * @crtc: crtc to grab > + * > + * This function returns the new crtc state for the given crtc, or > + * NULL if the crtc is not part of the global atomic state. > + */ > +static inline struct drm_crtc_state * > +drm_atomic_get_new_crtc_state(struct drm_atomic_state *state, > + struct drm_crtc *crtc) > +{ > + return state->crtcs[drm_crtc_index(crtc)].new_state; > +} > + > +/** > * drm_atomic_get_existing_plane_state - get plane state, if it exists > * @state: global atomic state object > * @plane: plane to grab > * > * This function returns the plane state for the given plane, or NULL > * if the plane is not part of the global atomic state. > + * > + * This function is deprecated, @drm_atomic_get_old_plane_state or > + * @drm_atomic_get_new_plane_state should be used instead. > */ > static inline struct drm_plane_state * > drm_atomic_get_existing_plane_state(struct drm_atomic_state *state, > @@ -301,12 +336,45 @@ drm_atomic_get_existing_plane_state(struct > drm_atomic_state *state, } > > /** > + * drm_atomic_get_old_plane_state - get plane state, if it exists > + * @state: global atomic state object > + * @plane: plane to grab > + * > + * This function returns the old plane state for the given plane, or > + * NULL if the plane is not part of the global atomic state. > + */ > +static inline struct drm_plane_state * > +drm_atomic_get_old_plane_state(struct drm_atomic_state *state, > + struct drm_plane *plane) > +{ > + return state->planes[drm_plane_index(plane)].old_state; > +} > + > +/** > + * drm_atomic_get_new_plane_state - get plane state, if it exists > + * @state: global atomic state object > + * @plane: plane to grab > + * > + * This function returns the new plane state for the given plane, or > + * NULL if the plane is not part of the global atomic state. > + */ > +static inline struct drm_plane_state * > +drm_atomic_get_new_plane_state(struct drm_atomic_state *state, > + struct drm_plane *plane) > +{ > + return state->planes[drm_plane_index(plane)].new_state; > +} > + > +/** > * drm_atomic_get_existing_connector_state - get connector state, if it > exists * @state: global atomic state object > * @connector: connector to grab > * > * This function returns the connector state for the given connector, > * or NULL if the connector is not part of the global atomic state. > + * > + * This function is deprecated, @drm_atomic_get_old_connector_state or > + * @drm_atomic_get_new_connector_state should be used instead. > */ > static inline struct drm_connector_state * > drm_atomic_get_existing_connector_state(struct drm_atomic_state *state, > @@ -321,6 +389,46 @@ drm_atomic_get_existing_connector_state(struct > drm_atomic_state *state, } > > /** > + * drm_atomic_get_old_connector_state - get connector state, if it exists > + * @state: global atomic state object > + * @connector: connector to grab > + * > + * This function returns the old connector state for the given connector, > + * or NULL if the connector is not part of the global atomic state. > + */ > +static inline struct drm_connector_state * > +drm_atomic_get_old_connector_state(struct drm_atomic_state *state, > + struct drm_connector *connector) > +{ > + int index = drm_connector_index(connector); > + > + if (index >= state->num_connector) > + return NULL; > + > + return state->connectors[index].old_state; > +} > + > +/** > + * drm_atomic_get_new_connector_state - get connector state, if it exists > + * @state: global atomic state object > + * @connector: connector to grab > + * > + * This function returns the new connector state for the given connector, > + * or NULL if the connector is not part of the global atomic state. > + */ > +static inline struct drm_connector_state * > +drm_atomic_get_new_connector_state(struct drm_atomic_state *state, > + struct drm_connector *connector) > +{ > + int index = drm_connector_index(connector); > + > + if (index >= state->num_connector) > + return NULL; > + > + return state->connectors[index].new_state; > +} > + > +/** > * __drm_atomic_get_current_plane_state - get current plane state > * @state: global atomic state object > * @plane: plane to grab -- Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3. 2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst ` (2 preceding siblings ...) 2017-02-16 14:47 ` [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2 Maarten Lankhorst @ 2017-02-16 14:47 ` Maarten Lankhorst 2017-03-01 1:04 ` Laurent Pinchart 2017-02-16 14:47 ` [PATCH v4 5/5] drm/blend: Use new atomic iterator macros Maarten Lankhorst 4 siblings, 1 reply; 15+ messages in thread From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx This is a straightforward conversion that converts all the users of get_existing_state in atomic core to use get_old_state or get_new_state Changes since v1: - Fix using the wrong state in drm_atomic_helper_update_legacy_modeset_state. Changes since v2: - Use the correct state in disable_outputs() Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/drm_atomic.c | 6 ++--- drivers/gpu/drm/drm_atomic_helper.c | 43 +++++++++++++++++---------------- drivers/gpu/drm/drm_blend.c | 3 +-- drivers/gpu/drm/drm_simple_kms_helper.c | 4 +-- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index afec53832145..c9aac9cbf111 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1358,8 +1358,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, return 0; if (conn_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(conn_state->state, - conn_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(conn_state->state, + conn_state->crtc); crtc_state->connector_mask &= ~(1 << drm_connector_index(conn_state->connector)); @@ -1476,7 +1476,7 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state, { struct drm_plane *plane; - WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc)); + WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc)); drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) { struct drm_plane_state *plane_state = diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ea544bddc29b..6296ae6e6aff 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -70,8 +70,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, struct drm_crtc_state *crtc_state; if (old_plane_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, - old_plane_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, + old_plane_state->crtc); if (WARN_ON(!crtc_state)) return; @@ -80,8 +80,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, } if (plane_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, - plane_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc); if (WARN_ON(!crtc_state)) return; @@ -150,7 +149,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state, drm_for_each_connector_iter(connector, &conn_iter) { struct drm_crtc_state *crtc_state; - if (drm_atomic_get_existing_connector_state(state, connector)) + if (drm_atomic_get_new_connector_state(state, connector)) continue; encoder = connector->state->best_encoder; @@ -178,7 +177,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state, new_conn_state->crtc->base.id, new_conn_state->crtc->name, connector->base.id, connector->name); - crtc_state = drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL); if (ret) @@ -219,7 +218,7 @@ set_best_encoder(struct drm_atomic_state *state, */ WARN_ON(!crtc && encoder != conn_state->best_encoder); if (crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); crtc_state->encoder_mask &= ~(1 << drm_encoder_index(conn_state->best_encoder)); @@ -230,7 +229,7 @@ set_best_encoder(struct drm_atomic_state *state, crtc = conn_state->crtc; WARN_ON(!crtc); if (crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); crtc_state->encoder_mask |= 1 << drm_encoder_index(encoder); @@ -263,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state, set_best_encoder(state, new_connector_state, NULL); - crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, encoder_crtc); crtc_state->connectors_changed = true; return; @@ -286,12 +285,12 @@ update_connector_routing(struct drm_atomic_state *state, if (old_connector_state->crtc != new_connector_state->crtc) { if (old_connector_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, old_connector_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, old_connector_state->crtc); crtc_state->connectors_changed = true; } if (new_connector_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, new_connector_state->crtc); crtc_state->connectors_changed = true; } } @@ -350,7 +349,7 @@ update_connector_routing(struct drm_atomic_state *state, set_best_encoder(state, new_connector_state, new_encoder); - crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, new_connector_state->crtc); crtc_state->connectors_changed = true; DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n", @@ -392,7 +391,7 @@ mode_fixup(struct drm_atomic_state *state) continue; new_crtc_state = - drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); + drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); /* * Each encoder has at most one connector (since we always steal @@ -704,8 +703,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) if (!old_conn_state->crtc) continue; - old_crtc_state = drm_atomic_get_existing_crtc_state(old_state, - old_conn_state->crtc); + old_crtc_state = drm_atomic_get_old_crtc_state(old_state, old_conn_state->crtc); if (!old_crtc_state->active || !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state)) @@ -834,14 +832,17 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, /* set legacy state in the crtc structure */ for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { struct drm_plane *primary = crtc->primary; + struct drm_plane_state *new_plane_state; crtc->mode = new_crtc_state->mode; crtc->enabled = new_crtc_state->enable; - if (drm_atomic_get_existing_plane_state(old_state, primary) && - primary->state->crtc == crtc) { - crtc->x = primary->state->src_x >> 16; - crtc->y = primary->state->src_y >> 16; + new_plane_state = + drm_atomic_get_new_plane_state(old_state, primary); + + if (new_plane_state && new_plane_state->crtc == crtc) { + crtc->x = new_plane_state->src_x >> 16; + crtc->y = new_plane_state->src_y >> 16; } if (new_crtc_state->enable) @@ -1843,7 +1844,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state) drm_for_each_plane_mask(plane, crtc->dev, plane_mask) { struct drm_plane_state *old_plane_state = - drm_atomic_get_existing_plane_state(old_state, plane); + drm_atomic_get_old_plane_state(old_state, plane); const struct drm_plane_helper_funcs *plane_funcs; plane_funcs = plane->helper_private; @@ -2914,7 +2915,7 @@ int drm_atomic_helper_page_flip_target( if (ret != 0) goto fail; - crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); if (WARN_ON(!crtc_state)) { ret = -EINVAL; goto fail; diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index 665aafc6ad68..d7053bb67db6 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -387,8 +387,7 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, if (!crtc) continue; if (plane->state->zpos != plane_state->zpos) { - crtc_state = - drm_atomic_get_existing_crtc_state(state, crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); crtc_state->zpos_changed = true; } } diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 35c5d99296b9..16789faa9291 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -86,8 +86,8 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, int ret; pipe = container_of(plane, struct drm_simple_display_pipe, plane); - crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state, - &pipe->crtc); + crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, + &pipe->crtc); if (crtc_state->enable != !!plane_state->crtc) return -EINVAL; /* plane must match crtc enable state */ -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3. 2017-02-16 14:47 ` [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3 Maarten Lankhorst @ 2017-03-01 1:04 ` Laurent Pinchart 2017-03-01 9:22 ` [PATCH v4.1 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v4 Maarten Lankhorst 0 siblings, 1 reply; 15+ messages in thread From: Laurent Pinchart @ 2017-03-01 1:04 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx Hi Maarten, Thank you for the patch. On Thursday 16 Feb 2017 15:47:09 Maarten Lankhorst wrote: > This is a straightforward conversion that converts all the users of > get_existing_state in atomic core to use get_old_state or get_new_state > > Changes since v1: > - Fix using the wrong state in > drm_atomic_helper_update_legacy_modeset_state. Changes since v2: > - Use the correct state in disable_outputs() > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/drm_atomic.c | 6 ++--- > drivers/gpu/drm/drm_atomic_helper.c | 43 ++++++++++++++---------------- > drivers/gpu/drm/drm_blend.c | 3 +-- > drivers/gpu/drm/drm_simple_kms_helper.c | 4 +-- > 4 files changed, 28 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index afec53832145..c9aac9cbf111 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1358,8 +1358,8 @@ drm_atomic_set_crtc_for_connector(struct > drm_connector_state *conn_state, return 0; > > if (conn_state->crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(conn_state- >state, > - conn_state- >crtc); > + crtc_state = drm_atomic_get_new_crtc_state(conn_state->state, > + conn_state->crtc); I believe more *_state -> *_(new|old)_state renames could be useful, but that can be addressed as a subsequent patch, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > crtc_state->connector_mask &= > ~(1 << drm_connector_index(conn_state->connector)); > @@ -1476,7 +1476,7 @@ drm_atomic_add_affected_planes(struct drm_atomic_state > *state, { > struct drm_plane *plane; > > - WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc)); > + WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc)); > > drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) { > struct drm_plane_state *plane_state = > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index ea544bddc29b..6296ae6e6aff > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -70,8 +70,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state > *state, struct drm_crtc_state *crtc_state; > > if (old_plane_state->crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(state, > - old_plane_state->crtc); > + crtc_state = drm_atomic_get_new_crtc_state(state, > + old_plane_state- >crtc); > > if (WARN_ON(!crtc_state)) > return; > @@ -80,8 +80,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state > *state, } > > if (plane_state->crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(state, > - plane_state- >crtc); > + crtc_state = drm_atomic_get_new_crtc_state(state, plane_state- >crtc); > > if (WARN_ON(!crtc_state)) > return; > @@ -150,7 +149,7 @@ static int handle_conflicting_encoders(struct > drm_atomic_state *state, drm_for_each_connector_iter(connector, &conn_iter) > { > struct drm_crtc_state *crtc_state; > > - if (drm_atomic_get_existing_connector_state(state, connector)) > + if (drm_atomic_get_new_connector_state(state, connector)) > continue; > > encoder = connector->state->best_encoder; > @@ -178,7 +177,7 @@ static int handle_conflicting_encoders(struct > drm_atomic_state *state, new_conn_state->crtc->base.id, > new_conn_state->crtc->name, > connector->base.id, connector->name); > > - crtc_state = drm_atomic_get_existing_crtc_state(state, > new_conn_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, > new_conn_state->crtc); > > ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL); > if (ret) > @@ -219,7 +218,7 @@ set_best_encoder(struct drm_atomic_state *state, > */ > WARN_ON(!crtc && encoder != conn_state->best_encoder); > if (crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); > + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > > crtc_state->encoder_mask &= > ~(1 << drm_encoder_index(conn_state- >best_encoder)); > @@ -230,7 +229,7 @@ set_best_encoder(struct drm_atomic_state *state, > crtc = conn_state->crtc; > WARN_ON(!crtc); > if (crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); > + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > > crtc_state->encoder_mask |= > 1 << drm_encoder_index(encoder); > @@ -263,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state, > > set_best_encoder(state, new_connector_state, NULL); > > - crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc); > + crtc_state = drm_atomic_get_new_crtc_state(state, encoder_crtc); > crtc_state->connectors_changed = true; > > return; > @@ -286,12 +285,12 @@ update_connector_routing(struct drm_atomic_state > *state, > > if (old_connector_state->crtc != new_connector_state->crtc) { > if (old_connector_state->crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(state, > old_connector_state->crtc); + crtc_state = > drm_atomic_get_new_crtc_state(state, old_connector_state->crtc); > crtc_state->connectors_changed = true; > } > > if (new_connector_state->crtc) { > - crtc_state = drm_atomic_get_existing_crtc_state(state, > new_connector_state->crtc); + crtc_state = > drm_atomic_get_new_crtc_state(state, new_connector_state->crtc); > crtc_state->connectors_changed = true; > } > } > @@ -350,7 +349,7 @@ update_connector_routing(struct drm_atomic_state *state, > > set_best_encoder(state, new_connector_state, new_encoder); > > - crtc_state = drm_atomic_get_existing_crtc_state(state, > new_connector_state->crtc); + crtc_state = > drm_atomic_get_new_crtc_state(state, new_connector_state->crtc); > crtc_state->connectors_changed = true; > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on > [CRTC:%d:%s]\n", @@ -392,7 +391,7 @@ mode_fixup(struct drm_atomic_state > *state) > continue; > > new_crtc_state = > - drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); > + drm_atomic_get_new_crtc_state(state, new_conn_state- >crtc); > > /* > * Each encoder has at most one connector (since we always steal > @@ -704,8 +703,7 @@ disable_outputs(struct drm_device *dev, struct > drm_atomic_state *old_state) if (!old_conn_state->crtc) > continue; > > - old_crtc_state = drm_atomic_get_existing_crtc_state(old_state, > - old_conn_state->crtc); > + old_crtc_state = drm_atomic_get_old_crtc_state(old_state, > old_conn_state->crtc); > > if (!old_crtc_state->active || > !drm_atomic_crtc_needs_modeset(old_conn_state->crtc- >state)) > @@ -834,14 +832,17 @@ drm_atomic_helper_update_legacy_modeset_state(struct > drm_device *dev, /* set legacy state in the crtc structure */ > for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > struct drm_plane *primary = crtc->primary; > + struct drm_plane_state *new_plane_state; > > crtc->mode = new_crtc_state->mode; > crtc->enabled = new_crtc_state->enable; > > - if (drm_atomic_get_existing_plane_state(old_state, primary) && > - primary->state->crtc == crtc) { > - crtc->x = primary->state->src_x >> 16; > - crtc->y = primary->state->src_y >> 16; > + new_plane_state = > + drm_atomic_get_new_plane_state(old_state, primary); > + > + if (new_plane_state && new_plane_state->crtc == crtc) { > + crtc->x = new_plane_state->src_x >> 16; > + crtc->y = new_plane_state->src_y >> 16; > } > > if (new_crtc_state->enable) > @@ -1843,7 +1844,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct > drm_crtc_state *old_crtc_state) > > drm_for_each_plane_mask(plane, crtc->dev, plane_mask) { > struct drm_plane_state *old_plane_state = > - drm_atomic_get_existing_plane_state(old_state, plane); > + drm_atomic_get_old_plane_state(old_state, plane); > const struct drm_plane_helper_funcs *plane_funcs; > > plane_funcs = plane->helper_private; > @@ -2914,7 +2915,7 @@ int drm_atomic_helper_page_flip_target( > if (ret != 0) > goto fail; > > - crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); > + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > if (WARN_ON(!crtc_state)) { > ret = -EINVAL; > goto fail; > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c > index 665aafc6ad68..d7053bb67db6 100644 > --- a/drivers/gpu/drm/drm_blend.c > +++ b/drivers/gpu/drm/drm_blend.c > @@ -387,8 +387,7 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, > if (!crtc) > continue; > if (plane->state->zpos != plane_state->zpos) { > - crtc_state = > - drm_atomic_get_existing_crtc_state(state, crtc); > + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > crtc_state->zpos_changed = true; > } > } > diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c > b/drivers/gpu/drm/drm_simple_kms_helper.c index 35c5d99296b9..16789faa9291 > 100644 > --- a/drivers/gpu/drm/drm_simple_kms_helper.c > +++ b/drivers/gpu/drm/drm_simple_kms_helper.c > @@ -86,8 +86,8 @@ static int drm_simple_kms_plane_atomic_check(struct > drm_plane *plane, int ret; > > pipe = container_of(plane, struct drm_simple_display_pipe, plane); > - crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state, > - &pipe->crtc); > + crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, > + &pipe->crtc); > if (crtc_state->enable != !!plane_state->crtc) > return -EINVAL; /* plane must match crtc enable state */ -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4.1 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v4. 2017-03-01 1:04 ` Laurent Pinchart @ 2017-03-01 9:22 ` Maarten Lankhorst 0 siblings, 0 replies; 15+ messages in thread From: Maarten Lankhorst @ 2017-03-01 9:22 UTC (permalink / raw) To: Laurent Pinchart, dri-devel; +Cc: intel-gfx This is a straightforward conversion that converts all the users of get_existing_state in atomic core to use get_old_state or get_new_state Changes since v1: - Fix using the wrong state in drm_atomic_helper_update_legacy_modeset_state. Changes since v2: - Use the correct state in disable_outputs() Changes since v3: - Rebase for link status training. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/drm_atomic.c | 6 ++--- drivers/gpu/drm/drm_atomic_helper.c | 47 +++++++++++++++++---------------- drivers/gpu/drm/drm_blend.c | 3 +-- drivers/gpu/drm/drm_simple_kms_helper.c | 4 +-- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 236d947011f9..9b892af7811a 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1374,8 +1374,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, return 0; if (conn_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(conn_state->state, - conn_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(conn_state->state, + conn_state->crtc); crtc_state->connector_mask &= ~(1 << drm_connector_index(conn_state->connector)); @@ -1492,7 +1492,7 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state, { struct drm_plane *plane; - WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc)); + WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc)); drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) { struct drm_plane_state *plane_state = diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index bc96d31bd388..4e26b73bb0d5 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -70,8 +70,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, struct drm_crtc_state *crtc_state; if (old_plane_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, - old_plane_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, + old_plane_state->crtc); if (WARN_ON(!crtc_state)) return; @@ -80,8 +80,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, } if (plane_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, - plane_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc); if (WARN_ON(!crtc_state)) return; @@ -150,7 +149,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state, drm_for_each_connector_iter(connector, &conn_iter) { struct drm_crtc_state *crtc_state; - if (drm_atomic_get_existing_connector_state(state, connector)) + if (drm_atomic_get_new_connector_state(state, connector)) continue; encoder = connector->state->best_encoder; @@ -178,7 +177,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state, new_conn_state->crtc->base.id, new_conn_state->crtc->name, connector->base.id, connector->name); - crtc_state = drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); ret = drm_atomic_set_crtc_for_connector(new_conn_state, NULL); if (ret) @@ -219,7 +218,7 @@ set_best_encoder(struct drm_atomic_state *state, */ WARN_ON(!crtc && encoder != conn_state->best_encoder); if (crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); crtc_state->encoder_mask &= ~(1 << drm_encoder_index(conn_state->best_encoder)); @@ -230,7 +229,7 @@ set_best_encoder(struct drm_atomic_state *state, crtc = conn_state->crtc; WARN_ON(!crtc); if (crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); crtc_state->encoder_mask |= 1 << drm_encoder_index(encoder); @@ -263,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state, set_best_encoder(state, new_connector_state, NULL); - crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, encoder_crtc); crtc_state->connectors_changed = true; return; @@ -286,12 +285,12 @@ update_connector_routing(struct drm_atomic_state *state, if (old_connector_state->crtc != new_connector_state->crtc) { if (old_connector_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, old_connector_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, old_connector_state->crtc); crtc_state->connectors_changed = true; } if (new_connector_state->crtc) { - crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, new_connector_state->crtc); crtc_state->connectors_changed = true; } } @@ -350,7 +349,7 @@ update_connector_routing(struct drm_atomic_state *state, set_best_encoder(state, new_connector_state, new_encoder); - crtc_state = drm_atomic_get_existing_crtc_state(state, new_connector_state->crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, new_connector_state->crtc); crtc_state->connectors_changed = true; DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n", @@ -392,7 +391,7 @@ mode_fixup(struct drm_atomic_state *state) continue; new_crtc_state = - drm_atomic_get_existing_crtc_state(state, new_conn_state->crtc); + drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); /* * Each encoder has at most one connector (since we always steal @@ -534,8 +533,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (ret) return ret; if (old_connector_state->crtc) { - new_crtc_state = drm_atomic_get_existing_crtc_state(state, - old_connector_state->crtc); + new_crtc_state = drm_atomic_get_new_crtc_state(state, + old_connector_state->crtc); if (old_connector_state->link_status != new_connector_state->link_status) new_crtc_state->connectors_changed = true; @@ -711,8 +710,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) if (!old_conn_state->crtc) continue; - old_crtc_state = drm_atomic_get_existing_crtc_state(old_state, - old_conn_state->crtc); + old_crtc_state = drm_atomic_get_old_crtc_state(old_state, old_conn_state->crtc); if (!old_crtc_state->active || !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state)) @@ -841,14 +839,17 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev, /* set legacy state in the crtc structure */ for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { struct drm_plane *primary = crtc->primary; + struct drm_plane_state *new_plane_state; crtc->mode = new_crtc_state->mode; crtc->enabled = new_crtc_state->enable; - if (drm_atomic_get_existing_plane_state(old_state, primary) && - primary->state->crtc == crtc) { - crtc->x = primary->state->src_x >> 16; - crtc->y = primary->state->src_y >> 16; + new_plane_state = + drm_atomic_get_new_plane_state(old_state, primary); + + if (new_plane_state && new_plane_state->crtc == crtc) { + crtc->x = new_plane_state->src_x >> 16; + crtc->y = new_plane_state->src_y >> 16; } if (new_crtc_state->enable) @@ -1850,7 +1851,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state) drm_for_each_plane_mask(plane, crtc->dev, plane_mask) { struct drm_plane_state *old_plane_state = - drm_atomic_get_existing_plane_state(old_state, plane); + drm_atomic_get_old_plane_state(old_state, plane); const struct drm_plane_helper_funcs *plane_funcs; plane_funcs = plane->helper_private; @@ -2953,7 +2954,7 @@ int drm_atomic_helper_page_flip_target( if (ret != 0) goto fail; - crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); if (WARN_ON(!crtc_state)) { ret = -EINVAL; goto fail; diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index 665aafc6ad68..d7053bb67db6 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -387,8 +387,7 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, if (!crtc) continue; if (plane->state->zpos != plane_state->zpos) { - crtc_state = - drm_atomic_get_existing_crtc_state(state, crtc); + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); crtc_state->zpos_changed = true; } } diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 35c5d99296b9..16789faa9291 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -86,8 +86,8 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, int ret; pipe = container_of(plane, struct drm_simple_display_pipe, plane); - crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state, - &pipe->crtc); + crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, + &pipe->crtc); if (crtc_state->enable != !!plane_state->crtc) return -EINVAL; /* plane must match crtc enable state */ -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4 5/5] drm/blend: Use new atomic iterator macros. 2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst ` (3 preceding siblings ...) 2017-02-16 14:47 ` [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3 Maarten Lankhorst @ 2017-02-16 14:47 ` Maarten Lankhorst 2017-03-01 0:58 ` Laurent Pinchart 4 siblings, 1 reply; 15+ messages in thread From: Maarten Lankhorst @ 2017-02-16 14:47 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx There are new iterator macros that annotate whether the new or old state should be used. This is better than using a state that depends on whether it's called before or after swap. For clarity, also rename the variables from $obj_state to (old,new)_$obj_state as well. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/drm_blend.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d7053bb67db6..a0d0d6843288 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -377,26 +377,26 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, struct drm_atomic_state *state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; - struct drm_plane_state *plane_state; + struct drm_plane_state *old_plane_state, *new_plane_state; int i, ret = 0; - for_each_plane_in_state(state, plane, plane_state, i) { - crtc = plane_state->crtc; + for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { + crtc = new_plane_state->crtc; if (!crtc) continue; - if (plane->state->zpos != plane_state->zpos) { - crtc_state = drm_atomic_get_new_crtc_state(state, crtc); - crtc_state->zpos_changed = true; + if (old_plane_state->zpos != new_plane_state->zpos) { + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + new_crtc_state->zpos_changed = true; } } - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (crtc_state->plane_mask != crtc->state->plane_mask || - crtc_state->zpos_changed) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { + if (old_crtc_state->plane_mask != new_crtc_state->plane_mask || + new_crtc_state->zpos_changed) { ret = drm_atomic_helper_crtc_normalize_zpos(crtc, - crtc_state); + new_crtc_state); if (ret) return ret; } -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 5/5] drm/blend: Use new atomic iterator macros. 2017-02-16 14:47 ` [PATCH v4 5/5] drm/blend: Use new atomic iterator macros Maarten Lankhorst @ 2017-03-01 0:58 ` Laurent Pinchart 0 siblings, 0 replies; 15+ messages in thread From: Laurent Pinchart @ 2017-03-01 0:58 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx Hi Maarten, Thank you for the patch. On Thursday 16 Feb 2017 15:47:10 Maarten Lankhorst wrote: > There are new iterator macros that annotate whether the new or old > state should be used. This is better than using a state that depends on > whether it's called before or after swap. For clarity, also rename the > variables from $obj_state to (old,new)_$obj_state as well. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/drm_blend.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c > index d7053bb67db6..a0d0d6843288 100644 > --- a/drivers/gpu/drm/drm_blend.c > +++ b/drivers/gpu/drm/drm_blend.c > @@ -377,26 +377,26 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, > struct drm_atomic_state *state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *old_plane_state, *new_plane_state; > int i, ret = 0; > > - for_each_plane_in_state(state, plane, plane_state, i) { > - crtc = plane_state->crtc; > + for_each_oldnew_plane_in_state(state, plane, old_plane_state, > new_plane_state, i) { + crtc = new_plane_state->crtc; > if (!crtc) > continue; > - if (plane->state->zpos != plane_state->zpos) { > - crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > - crtc_state->zpos_changed = true; > + if (old_plane_state->zpos != new_plane_state->zpos) { > + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > + new_crtc_state->zpos_changed = true; > } > } > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > - if (crtc_state->plane_mask != crtc->state->plane_mask || > - crtc_state->zpos_changed) { > + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, > i) { + if (old_crtc_state->plane_mask != new_crtc_state- >plane_mask || + > new_crtc_state->zpos_changed) { > ret = drm_atomic_helper_crtc_normalize_zpos(crtc, > - crtc_state); > + new_crtc_state); > if (ret) > return ret; > } -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-03-01 10:06 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst 2017-02-16 14:47 ` [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2 Maarten Lankhorst 2017-03-01 0:49 ` Laurent Pinchart 2017-03-01 9:09 ` Maarten Lankhorst 2017-03-01 9:21 ` [PATCH v4.1 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v3 Maarten Lankhorst 2017-03-01 10:06 ` Laurent Pinchart 2017-02-16 14:47 ` [PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand Maarten Lankhorst 2017-03-01 0:53 ` Laurent Pinchart 2017-02-16 14:47 ` [PATCH v4 3/5] drm/atomic: Add macros to access existing old/new state, v2 Maarten Lankhorst 2017-03-01 0:56 ` Laurent Pinchart 2017-02-16 14:47 ` [PATCH v4 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v3 Maarten Lankhorst 2017-03-01 1:04 ` Laurent Pinchart 2017-03-01 9:22 ` [PATCH v4.1 4/5] drm/atomic: Convert get_existing_state callers to get_old/new_state, v4 Maarten Lankhorst 2017-02-16 14:47 ` [PATCH v4 5/5] drm/blend: Use new atomic iterator macros Maarten Lankhorst 2017-03-01 0:58 ` Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).