* [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
` (5 more replies)
0 siblings, 6 replies; 16+ 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] 16+ 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 ` (4 subsequent siblings) 5 siblings, 1 reply; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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 ` (3 subsequent siblings) 5 siblings, 1 reply; 16+ 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] 16+ 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; 16+ 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] 16+ 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 ` (2 subsequent siblings) 5 siblings, 1 reply; 16+ 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] 16+ 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; 16+ 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] 16+ 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 2017-02-16 19:22 ` ✓ Fi.CI.BAT: success for drm/atomic: Add accessor macros for all atomic state. (rev5) Patchwork 5 siblings, 1 reply; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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 2017-02-16 19:22 ` ✓ Fi.CI.BAT: success for drm/atomic: Add accessor macros for all atomic state. (rev5) Patchwork 5 siblings, 1 reply; 16+ 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] 16+ 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; 16+ 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] 16+ messages in thread
* ✓ Fi.CI.BAT: success for drm/atomic: Add accessor macros for all atomic state. (rev5) 2017-02-16 14:47 [PATCH v4 0/5] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst ` (4 preceding siblings ...) 2017-02-16 14:47 ` [PATCH v4 5/5] drm/blend: Use new atomic iterator macros Maarten Lankhorst @ 2017-02-16 19:22 ` Patchwork 5 siblings, 0 replies; 16+ messages in thread From: Patchwork @ 2017-02-16 19:22 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx == Series Details == Series: drm/atomic: Add accessor macros for all atomic state. (rev5) URL : https://patchwork.freedesktop.org/series/17745/ State : success == Summary == Series 17745v5 drm/atomic: Add accessor macros for all atomic state. https://patchwork.freedesktop.org/api/1.0/series/17745/revisions/5/mbox/ fi-bdw-5557u total:252 pass:241 dwarn:0 dfail:0 fail:0 skip:11 fi-bsw-n3050 total:252 pass:213 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:252 pass:233 dwarn:0 dfail:0 fail:0 skip:19 fi-bxt-t5700 total:83 pass:70 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:252 pass:225 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:252 pass:221 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:252 pass:236 dwarn:0 dfail:0 fail:0 skip:16 fi-hsw-4770r total:252 pass:236 dwarn:0 dfail:0 fail:0 skip:16 fi-ilk-650 total:252 pass:202 dwarn:0 dfail:0 fail:0 skip:50 fi-ivb-3520m total:252 pass:234 dwarn:0 dfail:0 fail:0 skip:18 fi-ivb-3770 total:252 pass:234 dwarn:0 dfail:0 fail:0 skip:18 fi-kbl-7500u total:252 pass:234 dwarn:0 dfail:0 fail:0 skip:18 fi-skl-6260u total:252 pass:242 dwarn:0 dfail:0 fail:0 skip:10 fi-skl-6700hq total:252 pass:235 dwarn:0 dfail:0 fail:0 skip:17 fi-skl-6700k total:252 pass:230 dwarn:4 dfail:0 fail:0 skip:18 fi-skl-6770hq total:252 pass:242 dwarn:0 dfail:0 fail:0 skip:10 fi-snb-2520m total:252 pass:224 dwarn:0 dfail:0 fail:0 skip:28 fi-snb-2600 total:252 pass:223 dwarn:0 dfail:0 fail:0 skip:29 b29710e319e4b58d994065f5e776f6f66674d7c9 drm-tip: 2017y-02m-16d-15h-35m-40s UTC integration manifest 7df9cb5 drm/blend: Use new atomic iterator macros. 7e55380 drm/atomic: Convert get_existing_state callers to get_old/new_state, v3. 2be25d3 drm/atomic: Add macros to access existing old/new state, v2. c3cbb76 drm/atomic: Make drm_atomic_plane_disabling easier to understand. 2024186 drm/atomic: Fix atomic helpers to use the new iterator macros, v2. == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3860/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-03-01 10:06 UTC | newest] Thread overview: 16+ 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 2017-02-16 19:22 ` ✓ Fi.CI.BAT: success for drm/atomic: Add accessor macros for all atomic state. (rev5) Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.