All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state.
@ 2017-01-10 10:21 Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 1/7] drm/atomic: Add new iterators over all state Maarten Lankhorst
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-10 10:21 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Third version. 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 add that
patch in this iteration.

This will give the correct state regardless of swapping.

Maarten Lankhorst (7):
  drm/atomic: Add new iterators over all state
  drm/atomic: Make add_affected_connectors look at crtc_state.
  drm/atomic: Use new atomic iterator macros.
  drm/atomic: Fix atomic helpers to use the new iterator macros.
  drm/atomic: Add macros to access existing old/new state
  drm/atomic: Convert get_existing_state callers to get_old/new_state
  drm/blend: Use new atomic iterator macros.

 drivers/gpu/drm/drm_atomic.c         |  39 ++--
 drivers/gpu/drm/drm_atomic_helper.c  | 376 ++++++++++++++++++++---------------
 drivers/gpu/drm/drm_blend.c          |  23 +--
 drivers/gpu/drm/i915/intel_display.c |  11 +-
 include/drm/drm_atomic.h             | 180 ++++++++++++++++-
 include/drm/drm_atomic_helper.h      |   3 +
 6 files changed, 437 insertions(+), 195 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] 13+ messages in thread

* [PATCH v2 1/7] drm/atomic: Add new iterators over all state
  2017-01-10 10:21 [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
@ 2017-01-10 10:21 ` Maarten Lankhorst
  2017-01-10 13:46   ` [PATCH v2.1 1/7] drm/atomic: Add new iterators over all state, v3 Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 2/7] drm/atomic: Make add_affected_connectors look at crtc_state Maarten Lankhorst
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-10 10:21 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Add for_each_(old)(new)_(plane,connector,crtc)_in_state iterators to
replace the old for_each_xxx_in_state ones. This is useful for >1 flip
depth and getting rid of all xxx->state dereferences.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c         |  6 +++
 drivers/gpu/drm/drm_atomic_helper.c  | 52 +++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_display.c | 11 ++---
 include/drm/drm_atomic.h             | 81 ++++++++++++++++++++++++++++++++++--
 include/drm/drm_atomic_helper.h      |  3 ++
 5 files changed, 142 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 6414bcf7f41b..1c1cbf436717 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -275,6 +275,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
 		return ERR_PTR(-ENOMEM);
 
 	state->crtcs[index].state = crtc_state;
+	state->crtcs[index].old_state = crtc->state;
+	state->crtcs[index].new_state = crtc_state;
 	state->crtcs[index].ptr = crtc;
 	crtc_state->state = state;
 
@@ -691,6 +693,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
 
 	state->planes[index].state = plane_state;
 	state->planes[index].ptr = plane;
+	state->planes[index].old_state = plane->state;
+	state->planes[index].new_state = plane_state;
 	plane_state->state = state;
 
 	DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n",
@@ -1031,6 +1035,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
 
 	drm_connector_reference(connector);
 	state->connectors[index].state = connector_state;
+	state->connectors[index].old_state = connector->state;
+	state->connectors[index].new_state = connector_state;
 	state->connectors[index].ptr = connector;
 	connector_state->state = state;
 
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 7b71ac48b8a4..c468194934a8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2471,7 +2471,7 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_all);
  *
  * See also:
  * drm_atomic_helper_duplicate_state(), drm_atomic_helper_disable_all(),
- * drm_atomic_helper_resume()
+ * drm_atomic_helper_resume(), drm_atomic_helper_commit_duplicated_state()
  */
 struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)
 {
@@ -2512,6 +2512,52 @@ struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)
 EXPORT_SYMBOL(drm_atomic_helper_suspend);
 
 /**
+ * drm_atomic_helper_commit_duplicated_state - commit duplicated state
+ * @state: duplicated atomic state to commit
+ * @ctx: pointer to acquire_ctx to use for commit.
+ * @nonblock: commit the state non-blocking.
+ *
+ * The state returned by drm_atomic_helper_duplicate_state() and
+ * drm_atomic_helper_suspend() is partially invalid, and needs to
+ * be fixed up before commit.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ *
+ * See also:
+ * drm_atomic_helper_suspend()
+ */
+int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
+					      struct drm_modeset_acquire_ctx *ctx,
+					      bool nonblock)
+{
+	int i;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
+	struct drm_connector *connector;
+	struct drm_connector_state *conn_state;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+
+	state->acquire_ctx = ctx;
+
+	for_each_new_plane_in_state(state, plane, plane_state, i)
+		state->planes[i].old_state = plane->state;
+
+	for_each_new_crtc_in_state(state, crtc, crtc_state, i)
+		state->crtcs[i].old_state = crtc->state;
+
+	for_each_new_connector_in_state(state, connector, conn_state, i)
+		state->connectors[i].old_state = connector->state;
+
+	if (nonblock)
+		return drm_atomic_nonblocking_commit(state);
+	else
+		return drm_atomic_commit(state);
+}
+EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);
+
+/**
  * drm_atomic_helper_resume - subsystem-level resume helper
  * @dev: DRM device
  * @state: atomic state to resume to
@@ -2534,9 +2580,9 @@ int drm_atomic_helper_resume(struct drm_device *dev,
 	int err;
 
 	drm_mode_config_reset(dev);
+
 	drm_modeset_lock_all(dev);
-	state->acquire_ctx = config->acquire_ctx;
-	err = drm_atomic_commit(state);
+	err = drm_atomic_helper_commit_duplicated_state(state, config->acquire_ctx, false);
 	drm_modeset_unlock_all(dev);
 
 	return err;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e2150a64860c..0c1499f18041 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3489,7 +3489,8 @@ static void intel_update_primary_planes(struct drm_device *dev)
 
 static int
 __intel_display_resume(struct drm_device *dev,
-		       struct drm_atomic_state *state)
+		       struct drm_atomic_state *state,
+		       struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_crtc_state *crtc_state;
 	struct drm_crtc *crtc;
@@ -3513,7 +3514,7 @@ __intel_display_resume(struct drm_device *dev,
 	/* ignore any reset values/BIOS leftovers in the WM registers */
 	to_intel_atomic_state(state)->skip_intermediate_wm = true;
 
-	ret = drm_atomic_commit(state);
+	ret = drm_atomic_helper_commit_duplicated_state(state, ctx, false);
 
 	WARN_ON(ret == -EDEADLK);
 	return ret;
@@ -3607,7 +3608,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 			 */
 			intel_update_primary_planes(dev);
 		} else {
-			ret = __intel_display_resume(dev, state);
+			ret = __intel_display_resume(dev, state, ctx);
 			if (ret)
 				DRM_ERROR("Restoring old state failed with %i\n", ret);
 		}
@@ -3627,7 +3628,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 			dev_priv->display.hpd_irq_setup(dev_priv);
 		spin_unlock_irq(&dev_priv->irq_lock);
 
-		ret = __intel_display_resume(dev, state);
+		ret = __intel_display_resume(dev, state, ctx);
 		if (ret)
 			DRM_ERROR("Restoring old state failed with %i\n", ret);
 
@@ -17210,7 +17211,7 @@ void intel_display_resume(struct drm_device *dev)
 	}
 
 	if (!ret)
-		ret = __intel_display_resume(dev, state);
+		ret = __intel_display_resume(dev, state, &ctx);
 
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index f96220ed4004..6062e7f27325 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -137,12 +137,12 @@ struct drm_crtc_commit {
 
 struct __drm_planes_state {
 	struct drm_plane *ptr;
-	struct drm_plane_state *state;
+	struct drm_plane_state *state, *old_state, *new_state;
 };
 
 struct __drm_crtcs_state {
 	struct drm_crtc *ptr;
-	struct drm_crtc_state *state;
+	struct drm_crtc_state *state, *old_state, *new_state;
 	struct drm_crtc_commit *commit;
 	s64 __user *out_fence_ptr;
 	unsigned last_vblank_count;
@@ -150,7 +150,7 @@ struct __drm_crtcs_state {
 
 struct __drm_connnectors_state {
 	struct drm_connector *ptr;
-	struct drm_connector_state *state;
+	struct drm_connector_state *state, *old_state, *new_state;
 };
 
 /**
@@ -397,6 +397,31 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
 	     (__i)++)							\
 		for_each_if (connector)
 
+#define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \
+	for ((__i) = 0;								\
+	     (__i) < (__state)->num_connector &&				\
+	     ((connector) = (__state)->connectors[__i].ptr,			\
+	     (old_connector_state) = (__state)->connectors[__i].old_state,	\
+	     (new_connector_state) = (__state)->connectors[__i].new_state, 1); 	\
+	     (__i)++)							\
+		for_each_if (connector)
+
+#define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \
+	for ((__i) = 0;								\
+	     (__i) < (__state)->num_connector &&				\
+	     ((connector) = (__state)->connectors[__i].ptr,			\
+	     (old_connector_state) = (__state)->connectors[__i].old_state, 1); 	\
+	     (__i)++)							\
+		for_each_if (connector)
+
+#define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \
+	for ((__i) = 0;								\
+	     (__i) < (__state)->num_connector &&				\
+	     ((connector) = (__state)->connectors[__i].ptr,			\
+	     (new_connector_state) = (__state)->connectors[__i].new_state, 1); 	\
+	     (__i)++)							\
+		for_each_if (connector)
+
 #define for_each_crtc_in_state(__state, crtc, crtc_state, __i)	\
 	for ((__i) = 0;						\
 	     (__i) < (__state)->dev->mode_config.num_crtc &&	\
@@ -405,6 +430,31 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
 	     (__i)++)						\
 		for_each_if (crtc_state)
 
+#define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_crtc &&		\
+	     ((crtc) = (__state)->crtcs[__i].ptr,			\
+	     (old_crtc_state) = (__state)->crtcs[__i].old_state,	\
+	     (new_crtc_state) = (__state)->crtcs[__i].new_state, 1);	\
+	     (__i)++)							\
+		for_each_if (crtc)
+
+#define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i)	\
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_crtc &&		\
+	     ((crtc) = (__state)->crtcs[__i].ptr,			\
+	     (old_crtc_state) = (__state)->crtcs[__i].old_state, 1);	\
+	     (__i)++)							\
+		for_each_if (crtc)
+
+#define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i)	\
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_crtc &&		\
+	     ((crtc) = (__state)->crtcs[__i].ptr,			\
+	     (new_crtc_state) = (__state)->crtcs[__i].new_state, 1);	\
+	     (__i)++)							\
+		for_each_if (crtc)
+
 #define for_each_plane_in_state(__state, plane, plane_state, __i)		\
 	for ((__i) = 0;							\
 	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
@@ -413,6 +463,31 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
 	     (__i)++)							\
 		for_each_if (plane_state)
 
+#define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
+	     ((plane) = (__state)->planes[__i].ptr,			\
+	     (old_plane_state) = (__state)->planes[__i].old_state,	\
+	     (new_plane_state) = (__state)->planes[__i].new_state, 1);	\
+	     (__i)++)							\
+		for_each_if (plane)
+
+#define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
+	     ((plane) = (__state)->planes[__i].ptr,			\
+	     (old_plane_state) = (__state)->planes[__i].old_state, 1);	\
+	     (__i)++)							\
+		for_each_if (plane)
+
+#define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
+	     ((plane) = (__state)->planes[__i].ptr,			\
+	     (new_plane_state) = (__state)->planes[__i].new_state, 1);	\
+	     (__i)++)							\
+		for_each_if (plane)
+
 /**
  * drm_atomic_crtc_needs_modeset - compute combined modeset need
  * @state: &drm_crtc_state for the CRTC
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 4b2353dc34ba..c6a9436598f9 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -105,6 +105,9 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
 int drm_atomic_helper_disable_all(struct drm_device *dev,
 				  struct drm_modeset_acquire_ctx *ctx);
 struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev);
+int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
+					      struct drm_modeset_acquire_ctx *ctx,
+					      bool nonblock);
 int drm_atomic_helper_resume(struct drm_device *dev,
 			     struct drm_atomic_state *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] 13+ messages in thread

* [PATCH v2 2/7] drm/atomic: Make add_affected_connectors look at crtc_state.
  2017-01-10 10:21 [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 1/7] drm/atomic: Add new iterators over all state Maarten Lankhorst
@ 2017-01-10 10:21 ` Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 3/7] drm/atomic: Use new atomic iterator macros Maarten Lankhorst
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-10 10:21 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

This kills another dereference of connector->state. connector_mask
holds all unchanged connectors at least and any changed connectors
are already in state anyway.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1c1cbf436717..18cdf2c956c6 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1419,8 +1419,13 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state,
 	struct drm_connector *connector;
 	struct drm_connector_state *conn_state;
 	struct drm_connector_list_iter conn_iter;
+	struct drm_crtc_state *crtc_state;
 	int ret;
 
+	crtc_state = drm_atomic_get_crtc_state(state, crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
 	ret = drm_modeset_lock(&config->connection_mutex, state->acquire_ctx);
 	if (ret)
 		return ret;
@@ -1429,12 +1434,12 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state,
 			 crtc->base.id, crtc->name, state);
 
 	/*
-	 * Changed connectors are already in @state, so only need to look at the
-	 * current configuration.
+	 * Changed connectors are already in @state, so only need to look
+	 * at the connector_mask in crtc_state.
 	 */
 	drm_connector_list_iter_get(state->dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
-		if (connector->state->crtc != crtc)
+		if (!(crtc_state->connector_mask & (1 << drm_connector_index(connector))))
 			continue;
 
 		conn_state = drm_atomic_get_connector_state(state, connector);
-- 
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] 13+ messages in thread

* [PATCH v2 3/7] drm/atomic: Use new atomic iterator macros.
  2017-01-10 10:21 [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 1/7] drm/atomic: Add new iterators over all state Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 2/7] drm/atomic: Make add_affected_connectors look at crtc_state Maarten Lankhorst
@ 2017-01-10 10:21 ` Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 4/7] drm/atomic: Fix atomic helpers to use the new " Maarten Lankhorst
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-10 10:21 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 18cdf2c956c6..7b61e0da9ace 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1562,7 +1562,7 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
 
 	DRM_DEBUG_ATOMIC("checking %p\n", state);
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
+	for_each_new_plane_in_state(state, plane, plane_state, i) {
 		ret = drm_atomic_plane_check(plane, plane_state);
 		if (ret) {
 			DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check failed\n",
@@ -1571,7 +1571,7 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
 		}
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
 		ret = drm_atomic_crtc_check(crtc, crtc_state);
 		if (ret) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check failed\n",
@@ -1584,7 +1584,7 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
 		ret = config->funcs->atomic_check(state->dev, state);
 
 	if (!state->allow_modeset) {
-		for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
 			if (drm_atomic_crtc_needs_modeset(crtc_state)) {
 				DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full modeset\n",
 						 crtc->base.id, crtc->name);
@@ -1668,13 +1668,13 @@ static void drm_atomic_print_state(const struct drm_atomic_state *state)
 
 	DRM_DEBUG_ATOMIC("checking %p\n", state);
 
-	for_each_plane_in_state(state, plane, plane_state, i)
+	for_each_new_plane_in_state(state, plane, plane_state, i)
 		drm_atomic_plane_print_state(&p, plane_state);
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i)
+	for_each_new_crtc_in_state(state, crtc, crtc_state, i)
 		drm_atomic_crtc_print_state(&p, crtc_state);
 
-	for_each_connector_in_state(state, connector, connector_state, i)
+	for_each_new_connector_in_state(state, connector, connector_state, i)
 		drm_atomic_connector_print_state(&p, connector_state);
 }
 
@@ -1961,7 +1961,7 @@ static int prepare_crtc_signaling(struct drm_device *dev,
 	if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
 		return 0;
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
 		u64 __user *fence_ptr;
 
 		fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
@@ -2041,7 +2041,7 @@ static void complete_crtc_signaling(struct drm_device *dev,
 		return;
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
 		/*
 		 * TEST_ONLY and PAGE_FLIP_EVENT are mutually
 		 * exclusive, if they weren't, this code should be
-- 
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] 13+ messages in thread

* [PATCH v2 4/7] drm/atomic: Fix atomic helpers to use the new iterator macros.
  2017-01-10 10:21 [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
                   ` (2 preceding siblings ...)
  2017-01-10 10:21 ` [PATCH v2 3/7] drm/atomic: Use new atomic iterator macros Maarten Lankhorst
@ 2017-01-10 10:21 ` Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 5/7] drm/atomic: Add macros to access existing old/new state Maarten Lankhorst
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-10 10:21 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 293 +++++++++++++++++++-----------------
 1 file changed, 152 insertions(+), 141 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index c468194934a8..b577977322d0 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;
@@ -104,7 +105,7 @@ 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, conn_state, i) {
 		const struct drm_connector_helper_funcs *funcs = connector->helper_private;
 		struct drm_encoder *new_encoder;
 
@@ -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,33 +323,33 @@ 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]\n",
 				 new_encoder->base.id,
 				 new_encoder->name,
-				 connector_state->crtc->base.id);
+				 new_connector_state->crtc->base.id);
 		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",
@@ -355,8 +357,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;
 }
@@ -371,7 +373,7 @@ mode_fixup(struct drm_atomic_state *state)
 	int i;
 	bool ret;
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
 		if (!crtc_state->mode_changed &&
 		    !crtc_state->connectors_changed)
 			continue;
@@ -379,7 +381,7 @@ mode_fixup(struct drm_atomic_state *state)
 		drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode);
 	}
 
-	for_each_connector_in_state(state, connector, conn_state, i) {
+	for_each_new_connector_in_state(state, connector, conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
 
@@ -424,7 +426,7 @@ 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, crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		if (!crtc_state->enable)
@@ -483,19 +485,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);
 
@@ -507,8 +509,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;
 		}
 	}
 
@@ -516,14 +518,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;
 	}
@@ -534,28 +537,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)
@@ -565,7 +568,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);
 
@@ -599,15 +602,15 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *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, 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, plane_state, plane);
 
 		if (!funcs || !funcs->atomic_check)
 			continue;
@@ -620,7 +623,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, crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
 		funcs = crtc->helper_private;
@@ -681,12 +684,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;
 
@@ -723,7 +726,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);
@@ -734,11 +737,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)
@@ -751,7 +754,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);
@@ -780,13 +783,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 *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);
 
@@ -794,7 +797,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 =
@@ -811,23 +814,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, crtc_state, i) {
 		struct drm_plane *primary = crtc->primary;
 
-		crtc->mode = crtc->state->mode;
-		crtc->enabled = crtc->state->enable;
+		crtc->mode = crtc_state->mode;
+		crtc->enabled = crtc_state->enable;
 
 		if (drm_atomic_get_existing_plane_state(old_state, primary) &&
 		    primary->state->crtc == crtc) {
@@ -835,9 +838,9 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 			crtc->y = primary->state->src_y >> 16;
 		}
 
-		if (crtc->state->enable)
+		if (crtc_state->enable)
 			drm_calc_timestamping_constants(crtc,
-							&crtc->state->adjusted_mode);
+							&crtc_state->adjusted_mode);
 	}
 }
 EXPORT_SYMBOL(drm_atomic_helper_update_legacy_modeset_state);
@@ -846,20 +849,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 *crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *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, crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
-		if (!crtc->state->mode_changed)
+		if (!crtc_state->mode_changed)
 			continue;
 
 		funcs = crtc->helper_private;
 
-		if (crtc->state->enable && funcs->mode_set_nofb) {
+		if (crtc_state->enable && funcs->mode_set_nofb) {
 			DRM_DEBUG_ATOMIC("modeset on [CRTC:%d:%s]\n",
 					 crtc->base.id, crtc->name);
 
@@ -867,18 +870,18 @@ 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, 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 (!conn_state->best_encoder)
 			continue;
 
-		encoder = connector->state->best_encoder;
+		encoder = conn_state->best_encoder;
 		funcs = encoder->helper_private;
-		new_crtc_state = connector->state->crtc->state;
+		new_crtc_state = conn_state->crtc->state;
 		mode = &new_crtc_state->mode;
 		adjusted_mode = &new_crtc_state->adjusted_mode;
 
@@ -894,7 +897,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);
+					       conn_state);
 		} else if (funcs && funcs->mode_set) {
 			funcs->mode_set(encoder, mode, adjusted_mode);
 		}
@@ -946,24 +949,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 *crtc_state;
 	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state;
+	struct drm_connector_state *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, 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(crtc_state))
 			continue;
 
-		if (!crtc->state->active)
+		if (!crtc_state->active)
 			continue;
 
 		funcs = crtc->helper_private;
 
-		if (crtc->state->enable) {
+		if (crtc_state->enable) {
 			DRM_DEBUG_ATOMIC("enabling [CRTC:%d:%s]\n",
 					 crtc->base.id, crtc->name);
 
@@ -974,18 +977,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, conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
 
-		if (!connector->state->best_encoder)
+		if (!conn_state->best_encoder)
 			continue;
 
-		if (!connector->state->crtc->state->active ||
-		    !drm_atomic_crtc_needs_modeset(connector->state->crtc->state))
+		if (!conn_state->crtc->state->active ||
+		    !drm_atomic_crtc_needs_modeset(conn_state->crtc->state))
 			continue;
 
-		encoder = connector->state->best_encoder;
+		encoder = conn_state->best_encoder;
 		funcs = encoder->helper_private;
 
 		DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n",
@@ -1038,10 +1041,7 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
 	struct drm_plane_state *plane_state;
 	int i, ret;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		if (!pre_swap)
-			plane_state = plane->state;
-
+	for_each_new_plane_in_state(state, plane, plane_state, i) {
 		if (!plane_state->fence)
 			continue;
 
@@ -1080,7 +1080,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;
 
@@ -1091,9 +1091,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;
 
@@ -1105,7 +1103,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;
 
@@ -1412,11 +1410,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, *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, crtc_state, i) {
 		commit = kzalloc(sizeof(*commit), GFP_KERNEL);
 		if (!commit)
 			return -ENOMEM;
@@ -1437,7 +1435,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 && !crtc_state->active) {
 			complete_all(&commit->flip_done);
 			continue;
 		}
@@ -1503,7 +1501,7 @@ void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state)
 	int i;
 	long ret;
 
-	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, crtc_state, i) {
 		spin_lock(&crtc->commit_lock);
 		commit = preceeding_commit(crtc);
 		if (commit)
@@ -1554,13 +1552,13 @@ void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_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, 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(crtc_state->event);
 		spin_lock(&crtc->commit_lock);
 		complete_all(&commit->hw_done);
 		spin_unlock(&crtc->commit_lock);
@@ -1587,7 +1585,7 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *old_state)
 	int i;
 	long ret;
 
-	for_each_crtc_in_state(old_state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(old_state, crtc, crtc_state, i) {
 		commit = old_state->crtcs[i].commit;
 		if (WARN_ON(!commit))
 			continue;
@@ -1640,7 +1638,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 	struct drm_plane_state *plane_state;
 	int ret, i, j;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
+	for_each_new_plane_in_state(state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
 		funcs = plane->helper_private;
@@ -1655,7 +1653,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, plane_state, j) {
 		const struct drm_plane_helper_funcs *funcs;
 
 		if (j >= i)
@@ -1722,14 +1720,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;
@@ -1737,13 +1735,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;
 
@@ -1762,7 +1760,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;
@@ -1781,12 +1779,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;
@@ -1794,7 +1792,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);
@@ -1921,12 +1919,19 @@ 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 *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, plane_state, new_plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
+		/*
+		 * This might be called before swapping when commit is aborted,
+		 * in which case we have to free the new state.
+		 */
+		if (plane_state == plane->state)
+			plane_state = new_plane_state;
+
 		funcs = plane->helper_private;
 
 		if (funcs->cleanup_fb)
@@ -1971,15 +1976,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;
+	struct drm_connector_state *old_conn_state, *new_conn_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;
 	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);
@@ -1999,16 +2004,20 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 		}
 	}
 
-	for_each_connector_in_state(state, connector, conn_state, i) {
-		connector->state->state = state;
-		swap(state->connectors[i].state, connector->state);
-		connector->state->state = NULL;
+	for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_conn_state, i) {
+		old_conn_state->state = state;
+		new_conn_state->state = NULL;
+
+		state->connectors[i].state = old_conn_state;
+		connector->state = new_conn_state;
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		crtc->state->state = state;
-		swap(state->crtcs[i].state, crtc->state);
-		crtc->state->state = NULL;
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		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);
@@ -2020,10 +2029,12 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 		}
 	}
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		plane->state->state = state;
-		swap(state->planes[i].state, plane->state);
-		plane->state->state = NULL;
+	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
+		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);
@@ -2221,7 +2232,7 @@ static int update_output_state(struct drm_atomic_state *state,
 	if (ret)
 		return ret;
 
-	for_each_connector_in_state(state, connector, conn_state, i) {
+	for_each_new_connector_in_state(state, connector, conn_state, i) {
 		if (conn_state->crtc == set->crtc) {
 			ret = drm_atomic_set_crtc_for_connector(conn_state,
 								NULL);
@@ -2243,7 +2254,7 @@ static int update_output_state(struct drm_atomic_state *state,
 			return ret;
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_new_crtc_in_state(state, crtc, 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
-- 
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] 13+ messages in thread

* [PATCH v2 5/7] drm/atomic: Add macros to access existing old/new state
  2017-01-10 10:21 [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
                   ` (3 preceding siblings ...)
  2017-01-10 10:21 ` [PATCH v2 4/7] drm/atomic: Fix atomic helpers to use the new " Maarten Lankhorst
@ 2017-01-10 10:21 ` Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state Maarten Lankhorst
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-10 10:21 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.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 include/drm/drm_atomic.h | 99 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 6062e7f27325..2e6bb7acc837 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -285,6 +285,35 @@ 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
@@ -300,6 +329,36 @@ 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
@@ -320,6 +379,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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state
  2017-01-10 10:21 [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
                   ` (4 preceding siblings ...)
  2017-01-10 10:21 ` [PATCH v2 5/7] drm/atomic: Add macros to access existing old/new state Maarten Lankhorst
@ 2017-01-10 10:21 ` Maarten Lankhorst
  2017-01-12  9:27   ` [PATCH v2.1 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state, v2 Maarten Lankhorst
  2017-01-10 10:21 ` [PATCH v2 7/7] drm/blend: Use new atomic iterator macros Maarten Lankhorst
  2017-01-10 12:23 ` ✗ Fi.CI.BAT: warning for drm/atomic: Add accessor macros for all atomic state Patchwork
  7 siblings, 1 reply; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-10 10:21 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c        |  6 +++---
 drivers/gpu/drm/drm_atomic_helper.c | 39 +++++++++++++++++--------------------
 drivers/gpu/drm/drm_blend.c         |  3 +--
 3 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7b61e0da9ace..6428e9469607 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1362,8 +1362,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));
@@ -1480,7 +1480,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 b577977322d0..0b770eee0958 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -70,8 +70,7 @@ 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 +79,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 +148,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 +176,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 				 conn_state->crtc->base.id, 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_new_crtc_state(state, conn_state->crtc);
 
 		ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
 		if (ret)
@@ -219,7 +217,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 +228,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 +261,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 +284,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;
 		}
 	}
@@ -349,7 +347,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",
@@ -390,8 +388,7 @@ mode_fixup(struct drm_atomic_state *state)
 		if (!conn_state->crtc || !conn_state->best_encoder)
 			continue;
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								conn_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
 
 		/*
 		 * Each encoder has at most one connector (since we always steal
@@ -698,8 +695,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_new_crtc_state(old_state, old_conn_state->crtc);
 
 		if (!old_crtc_state->active ||
 		    !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state))
@@ -828,14 +824,15 @@ 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, crtc_state, i) {
 		struct drm_plane *primary = crtc->primary;
+		struct drm_plane_state *old_plane_state;
 
 		crtc->mode = crtc_state->mode;
 		crtc->enabled = 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;
+		old_plane_state = drm_atomic_get_old_plane_state(old_state, primary);
+		if (old_plane_state && old_plane_state->crtc == crtc) {
+			crtc->x = old_plane_state->src_x >> 16;
+			crtc->y = old_plane_state->src_y >> 16;
 		}
 
 		if (crtc_state->enable)
@@ -1835,7 +1832,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;
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 1f2412c7ccfd..5c45d0852608 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -388,8 +388,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;
 		}
 	}
-- 
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] 13+ messages in thread

* [PATCH v2 7/7] drm/blend: Use new atomic iterator macros.
  2017-01-10 10:21 [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
                   ` (5 preceding siblings ...)
  2017-01-10 10:21 ` [PATCH v2 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state Maarten Lankhorst
@ 2017-01-10 10:21 ` Maarten Lankhorst
  2017-01-10 12:23 ` ✗ Fi.CI.BAT: warning for drm/atomic: Add accessor macros for all atomic state Patchwork
  7 siblings, 0 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-10 10:21 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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 5c45d0852608..78cf9f6cae08 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -378,26 +378,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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* ✗ Fi.CI.BAT: warning for drm/atomic: Add accessor macros for all atomic state.
  2017-01-10 10:21 [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
                   ` (6 preceding siblings ...)
  2017-01-10 10:21 ` [PATCH v2 7/7] drm/blend: Use new atomic iterator macros Maarten Lankhorst
@ 2017-01-10 12:23 ` Patchwork
  7 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2017-01-10 12:23 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: drm/atomic: Add accessor macros for all atomic state.
URL   : https://patchwork.freedesktop.org/series/17745/
State : warning

== Summary ==

Series 17745v1 drm/atomic: Add accessor macros for all atomic state.
https://patchwork.freedesktop.org/api/1.0/series/17745/revisions/1/mbox/

Test kms_force_connector_basic:
        Subgroup force-load-detect:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup prune-stale-modes:
                pass       -> DMESG-WARN (fi-byt-j1900)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-snb-2520m)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-snb-2520m)

fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:216  dwarn:3   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:212  dwarn:3   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:211  dwarn:3   dfail:0   fail:0   skip:32 

36f4533ad92a375ca696d437f99a303683efd5d8 drm-tip: 2017y-01m-10d-10h-14m-49s UTC integration manifest
bb3410c drm/blend: Use new atomic iterator macros.
bafaf77 drm/atomic: Convert get_existing_state callers to get_old/new_state
69153ed drm/atomic: Add macros to access existing old/new state
247c28c drm/atomic: Fix atomic helpers to use the new iterator macros.
6ca2a9cb drm/atomic: Use new atomic iterator macros.
9eede92 drm/atomic: Make add_affected_connectors look at crtc_state.
eae69b1 drm/atomic: Add new iterators over all state

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3462/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2.1 1/7] drm/atomic: Add new iterators over all state, v3.
  2017-01-10 10:21 ` [PATCH v2 1/7] drm/atomic: Add new iterators over all state Maarten Lankhorst
@ 2017-01-10 13:46   ` Maarten Lankhorst
  2017-01-15 19:56     ` Laurent Pinchart
  0 siblings, 1 reply; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-10 13:46 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Add for_each_(old)(new)_(plane,connector,crtc)_in_state iterators to
replace the old for_each_xxx_in_state ones. This is useful for >1 flip
depth and getting rid of all xxx->state dereferences.

This requires extra fixups done when committing a state after
duplicating, which in general isn't valid but is used by suspend/resume.
To handle these, introduce drm_atomic_helper_commit_duplicated_state
which performs those fixups before checking & committing the state.

Changes since v1:
- Remove nonblock parameter for commit_duplicated_state.
Changes since v2:
- Use commit_duplicated_state for i915 load detection.
- Add WARN_ON(old_state != obj->state) before swapping.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
Forgot that there was a v2. It still broke on igt load detect tests, this should be the fix.

I also added a big WARN_ON before swapping to ensure old state is correct. At this point we
can no longer fix it because atomic_check is called with the wrong items, but it will help
limit the damage.

 drivers/gpu/drm/drm_atomic.c         |  6 +++
 drivers/gpu/drm/drm_atomic_helper.c  | 65 +++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_display.c | 13 +++---
 include/drm/drm_atomic.h             | 81 ++++++++++++++++++++++++++++++++++--
 include/drm/drm_atomic_helper.h      |  2 +
 5 files changed, 149 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 6414bcf7f41b..1c1cbf436717 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -275,6 +275,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
 		return ERR_PTR(-ENOMEM);
 
 	state->crtcs[index].state = crtc_state;
+	state->crtcs[index].old_state = crtc->state;
+	state->crtcs[index].new_state = crtc_state;
 	state->crtcs[index].ptr = crtc;
 	crtc_state->state = state;
 
@@ -691,6 +693,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
 
 	state->planes[index].state = plane_state;
 	state->planes[index].ptr = plane;
+	state->planes[index].old_state = plane->state;
+	state->planes[index].new_state = plane_state;
 	plane_state->state = state;
 
 	DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n",
@@ -1031,6 +1035,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
 
 	drm_connector_reference(connector);
 	state->connectors[index].state = connector_state;
+	state->connectors[index].old_state = connector->state;
+	state->connectors[index].new_state = connector_state;
 	state->connectors[index].ptr = connector;
 	connector_state->state = state;
 
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 7b71ac48b8a4..b652ef1a25b1 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1971,11 +1971,11 @@ 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;
+	struct drm_connector_state *conn_state, *old_conn_state;
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *crtc_state, *old_crtc_state;
 	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *plane_state, *old_plane_state;
 	struct drm_crtc_commit *commit;
 
 	if (stall) {
@@ -1999,13 +1999,17 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 		}
 	}
 
-	for_each_connector_in_state(state, connector, conn_state, i) {
+	for_each_oldnew_connector_in_state(state, connector, old_conn_state, 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;
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 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;
@@ -2020,7 +2024,9 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 		}
 	}
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
+	for_each_oldnew_plane_in_state(state, plane, old_plane_state, 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;
@@ -2471,7 +2477,7 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_all);
  *
  * See also:
  * drm_atomic_helper_duplicate_state(), drm_atomic_helper_disable_all(),
- * drm_atomic_helper_resume()
+ * drm_atomic_helper_resume(), drm_atomic_helper_commit_duplicated_state()
  */
 struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)
 {
@@ -2512,6 +2518,47 @@ struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)
 EXPORT_SYMBOL(drm_atomic_helper_suspend);
 
 /**
+ * drm_atomic_helper_commit_duplicated_state - commit duplicated state
+ * @state: duplicated atomic state to commit
+ * @ctx: pointer to acquire_ctx to use for commit.
+ *
+ * The state returned by drm_atomic_helper_duplicate_state() and
+ * drm_atomic_helper_suspend() is partially invalid, and needs to
+ * be fixed up before commit.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ *
+ * See also:
+ * drm_atomic_helper_suspend()
+ */
+int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
+					      struct drm_modeset_acquire_ctx *ctx)
+{
+	int i;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
+	struct drm_connector *connector;
+	struct drm_connector_state *conn_state;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+
+	state->acquire_ctx = ctx;
+
+	for_each_new_plane_in_state(state, plane, plane_state, i)
+		state->planes[i].old_state = plane->state;
+
+	for_each_new_crtc_in_state(state, crtc, crtc_state, i)
+		state->crtcs[i].old_state = crtc->state;
+
+	for_each_new_connector_in_state(state, connector, conn_state, i)
+		state->connectors[i].old_state = connector->state;
+
+	return drm_atomic_commit(state);
+}
+EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);
+
+/**
  * drm_atomic_helper_resume - subsystem-level resume helper
  * @dev: DRM device
  * @state: atomic state to resume to
@@ -2534,9 +2581,9 @@ int drm_atomic_helper_resume(struct drm_device *dev,
 	int err;
 
 	drm_mode_config_reset(dev);
+
 	drm_modeset_lock_all(dev);
-	state->acquire_ctx = config->acquire_ctx;
-	err = drm_atomic_commit(state);
+	err = drm_atomic_helper_commit_duplicated_state(state, config->acquire_ctx);
 	drm_modeset_unlock_all(dev);
 
 	return err;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e2150a64860c..5364724df57f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3489,7 +3489,8 @@ static void intel_update_primary_planes(struct drm_device *dev)
 
 static int
 __intel_display_resume(struct drm_device *dev,
-		       struct drm_atomic_state *state)
+		       struct drm_atomic_state *state,
+		       struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_crtc_state *crtc_state;
 	struct drm_crtc *crtc;
@@ -3513,7 +3514,7 @@ __intel_display_resume(struct drm_device *dev,
 	/* ignore any reset values/BIOS leftovers in the WM registers */
 	to_intel_atomic_state(state)->skip_intermediate_wm = true;
 
-	ret = drm_atomic_commit(state);
+	ret = drm_atomic_helper_commit_duplicated_state(state, ctx);
 
 	WARN_ON(ret == -EDEADLK);
 	return ret;
@@ -3607,7 +3608,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 			 */
 			intel_update_primary_planes(dev);
 		} else {
-			ret = __intel_display_resume(dev, state);
+			ret = __intel_display_resume(dev, state, ctx);
 			if (ret)
 				DRM_ERROR("Restoring old state failed with %i\n", ret);
 		}
@@ -3627,7 +3628,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 			dev_priv->display.hpd_irq_setup(dev_priv);
 		spin_unlock_irq(&dev_priv->irq_lock);
 
-		ret = __intel_display_resume(dev, state);
+		ret = __intel_display_resume(dev, state, ctx);
 		if (ret)
 			DRM_ERROR("Restoring old state failed with %i\n", ret);
 
@@ -11328,7 +11329,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 	if (!state)
 		return;
 
-	ret = drm_atomic_commit(state);
+	ret = drm_atomic_helper_commit_duplicated_state(state, ctx);
 	if (ret)
 		DRM_DEBUG_KMS("Couldn't release load detect pipe: %i\n", ret);
 	drm_atomic_state_put(state);
@@ -17210,7 +17211,7 @@ void intel_display_resume(struct drm_device *dev)
 	}
 
 	if (!ret)
-		ret = __intel_display_resume(dev, state);
+		ret = __intel_display_resume(dev, state, &ctx);
 
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index f96220ed4004..6062e7f27325 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -137,12 +137,12 @@ struct drm_crtc_commit {
 
 struct __drm_planes_state {
 	struct drm_plane *ptr;
-	struct drm_plane_state *state;
+	struct drm_plane_state *state, *old_state, *new_state;
 };
 
 struct __drm_crtcs_state {
 	struct drm_crtc *ptr;
-	struct drm_crtc_state *state;
+	struct drm_crtc_state *state, *old_state, *new_state;
 	struct drm_crtc_commit *commit;
 	s64 __user *out_fence_ptr;
 	unsigned last_vblank_count;
@@ -150,7 +150,7 @@ struct __drm_crtcs_state {
 
 struct __drm_connnectors_state {
 	struct drm_connector *ptr;
-	struct drm_connector_state *state;
+	struct drm_connector_state *state, *old_state, *new_state;
 };
 
 /**
@@ -397,6 +397,31 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
 	     (__i)++)							\
 		for_each_if (connector)
 
+#define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \
+	for ((__i) = 0;								\
+	     (__i) < (__state)->num_connector &&				\
+	     ((connector) = (__state)->connectors[__i].ptr,			\
+	     (old_connector_state) = (__state)->connectors[__i].old_state,	\
+	     (new_connector_state) = (__state)->connectors[__i].new_state, 1); 	\
+	     (__i)++)							\
+		for_each_if (connector)
+
+#define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \
+	for ((__i) = 0;								\
+	     (__i) < (__state)->num_connector &&				\
+	     ((connector) = (__state)->connectors[__i].ptr,			\
+	     (old_connector_state) = (__state)->connectors[__i].old_state, 1); 	\
+	     (__i)++)							\
+		for_each_if (connector)
+
+#define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \
+	for ((__i) = 0;								\
+	     (__i) < (__state)->num_connector &&				\
+	     ((connector) = (__state)->connectors[__i].ptr,			\
+	     (new_connector_state) = (__state)->connectors[__i].new_state, 1); 	\
+	     (__i)++)							\
+		for_each_if (connector)
+
 #define for_each_crtc_in_state(__state, crtc, crtc_state, __i)	\
 	for ((__i) = 0;						\
 	     (__i) < (__state)->dev->mode_config.num_crtc &&	\
@@ -405,6 +430,31 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
 	     (__i)++)						\
 		for_each_if (crtc_state)
 
+#define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_crtc &&		\
+	     ((crtc) = (__state)->crtcs[__i].ptr,			\
+	     (old_crtc_state) = (__state)->crtcs[__i].old_state,	\
+	     (new_crtc_state) = (__state)->crtcs[__i].new_state, 1);	\
+	     (__i)++)							\
+		for_each_if (crtc)
+
+#define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i)	\
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_crtc &&		\
+	     ((crtc) = (__state)->crtcs[__i].ptr,			\
+	     (old_crtc_state) = (__state)->crtcs[__i].old_state, 1);	\
+	     (__i)++)							\
+		for_each_if (crtc)
+
+#define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i)	\
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_crtc &&		\
+	     ((crtc) = (__state)->crtcs[__i].ptr,			\
+	     (new_crtc_state) = (__state)->crtcs[__i].new_state, 1);	\
+	     (__i)++)							\
+		for_each_if (crtc)
+
 #define for_each_plane_in_state(__state, plane, plane_state, __i)		\
 	for ((__i) = 0;							\
 	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
@@ -413,6 +463,31 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
 	     (__i)++)							\
 		for_each_if (plane_state)
 
+#define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
+	     ((plane) = (__state)->planes[__i].ptr,			\
+	     (old_plane_state) = (__state)->planes[__i].old_state,	\
+	     (new_plane_state) = (__state)->planes[__i].new_state, 1);	\
+	     (__i)++)							\
+		for_each_if (plane)
+
+#define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
+	     ((plane) = (__state)->planes[__i].ptr,			\
+	     (old_plane_state) = (__state)->planes[__i].old_state, 1);	\
+	     (__i)++)							\
+		for_each_if (plane)
+
+#define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \
+	for ((__i) = 0;							\
+	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
+	     ((plane) = (__state)->planes[__i].ptr,			\
+	     (new_plane_state) = (__state)->planes[__i].new_state, 1);	\
+	     (__i)++)							\
+		for_each_if (plane)
+
 /**
  * drm_atomic_crtc_needs_modeset - compute combined modeset need
  * @state: &drm_crtc_state for the CRTC
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 4b2353dc34ba..e43d4de2ff8f 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -105,6 +105,8 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
 int drm_atomic_helper_disable_all(struct drm_device *dev,
 				  struct drm_modeset_acquire_ctx *ctx);
 struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev);
+int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
+					      struct drm_modeset_acquire_ctx *ctx);
 int drm_atomic_helper_resume(struct drm_device *dev,
 			     struct drm_atomic_state *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] 13+ messages in thread

* [PATCH v2.1 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state, v2.
  2017-01-10 10:21 ` [PATCH v2 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state Maarten Lankhorst
@ 2017-01-12  9:27   ` Maarten Lankhorst
  0 siblings, 0 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-12  9:27 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.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c        |  6 +++---
 drivers/gpu/drm/drm_atomic_helper.c | 39 +++++++++++++++++--------------------
 drivers/gpu/drm/drm_blend.c         |  3 +--
 3 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7b61e0da9ace..6428e9469607 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1362,8 +1362,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));
@@ -1480,7 +1480,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 d2d4e7eefa1a..2e117d0acf2f 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -70,8 +70,7 @@ 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 +79,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 +148,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 +176,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 				 conn_state->crtc->base.id, 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_new_crtc_state(state, conn_state->crtc);
 
 		ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
 		if (ret)
@@ -219,7 +217,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 +228,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 +261,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 +284,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;
 		}
 	}
@@ -349,7 +347,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",
@@ -390,8 +388,7 @@ mode_fixup(struct drm_atomic_state *state)
 		if (!conn_state->crtc || !conn_state->best_encoder)
 			continue;
 
-		crtc_state = drm_atomic_get_existing_crtc_state(state,
-								conn_state->crtc);
+		crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
 
 		/*
 		 * Each encoder has at most one connector (since we always steal
@@ -698,8 +695,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_new_crtc_state(old_state, old_conn_state->crtc);
 
 		if (!old_crtc_state->active ||
 		    !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state))
@@ -828,14 +824,15 @@ 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, crtc_state, i) {
 		struct drm_plane *primary = crtc->primary;
+		struct drm_plane_state *plane_state;
 
 		crtc->mode = crtc_state->mode;
 		crtc->enabled = 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;
+		plane_state = drm_atomic_get_new_plane_state(old_state, primary);
+		if (plane_state && plane_state->crtc == crtc) {
+			crtc->x = plane_state->src_x >> 16;
+			crtc->y = plane_state->src_y >> 16;
 		}
 
 		if (crtc_state->enable)
@@ -1835,7 +1832,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;
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 1f2412c7ccfd..5c45d0852608 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -388,8 +388,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;
 		}
 	}
-- 
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] 13+ messages in thread

* Re: [PATCH v2.1 1/7] drm/atomic: Add new iterators over all state, v3.
  2017-01-10 13:46   ` [PATCH v2.1 1/7] drm/atomic: Add new iterators over all state, v3 Maarten Lankhorst
@ 2017-01-15 19:56     ` Laurent Pinchart
  2017-01-16  9:26       ` Maarten Lankhorst
  0 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2017-01-15 19:56 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Hi Maarten,

Thank you for the patch.

On Tuesday 10 Jan 2017 14:46:58 Maarten Lankhorst wrote:
> Add for_each_(old)(new)_(plane,connector,crtc)_in_state iterators to
> replace the old for_each_xxx_in_state ones. This is useful for >1 flip
> depth and getting rid of all xxx->state dereferences.
> 
> This requires extra fixups done when committing a state after
> duplicating, which in general isn't valid but is used by suspend/resume.
> To handle these, introduce drm_atomic_helper_commit_duplicated_state
> which performs those fixups before checking & committing the state.
> 
> Changes since v1:
> - Remove nonblock parameter for commit_duplicated_state.
> Changes since v2:
> - Use commit_duplicated_state for i915 load detection.
> - Add WARN_ON(old_state != obj->state) before swapping.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> Forgot that there was a v2. It still broke on igt load detect tests, this
> should be the fix.
> 
> I also added a big WARN_ON before swapping to ensure old state is correct.
> At this point we can no longer fix it because atomic_check is called with
> the wrong items, but it will help limit the damage.

This patch now conflicts with "[PATCH v2 4/7] drm/atomic: Fix atomic helpers 
to use the new iterator macros.". Could you fix that and repost the whole 
series ?

>  drivers/gpu/drm/drm_atomic.c         |  6 +++
>  drivers/gpu/drm/drm_atomic_helper.c  | 65 +++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_display.c | 13 +++---
>  include/drm/drm_atomic.h             | 81
> ++++++++++++++++++++++++++++++++++-- include/drm/drm_atomic_helper.h      |
>  2 +
>  5 files changed, 149 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 6414bcf7f41b..1c1cbf436717 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -275,6 +275,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state
> *state, return ERR_PTR(-ENOMEM);
> 
>  	state->crtcs[index].state = crtc_state;
> +	state->crtcs[index].old_state = crtc->state;
> +	state->crtcs[index].new_state = crtc_state;
>  	state->crtcs[index].ptr = crtc;
>  	crtc_state->state = state;
> 
> @@ -691,6 +693,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state
> *state,
> 
>  	state->planes[index].state = plane_state;
>  	state->planes[index].ptr = plane;
> +	state->planes[index].old_state = plane->state;
> +	state->planes[index].new_state = plane_state;
>  	plane_state->state = state;
> 
>  	DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n",
> @@ -1031,6 +1035,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state
> *state,
> 
>  	drm_connector_reference(connector);
>  	state->connectors[index].state = connector_state;
> +	state->connectors[index].old_state = connector->state;
> +	state->connectors[index].new_state = connector_state;
>  	state->connectors[index].ptr = connector;
>  	connector_state->state = state;
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index 7b71ac48b8a4..b652ef1a25b1
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1971,11 +1971,11 @@ 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;
> +	struct drm_connector_state *conn_state, *old_conn_state;
>  	struct drm_crtc *crtc;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *crtc_state, *old_crtc_state;
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *plane_state, *old_plane_state;
>  	struct drm_crtc_commit *commit;
> 
>  	if (stall) {
> @@ -1999,13 +1999,17 @@ void drm_atomic_helper_swap_state(struct
> drm_atomic_state *state, }
>  	}
> 
> -	for_each_connector_in_state(state, connector, conn_state, i) {
> +	for_each_oldnew_connector_in_state(state, connector, old_conn_state,
> 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;
>  	}
> 
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 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;
> @@ -2020,7 +2024,9 @@ void drm_atomic_helper_swap_state(struct
> drm_atomic_state *state, }
>  	}
> 
> -	for_each_plane_in_state(state, plane, plane_state, i) {
> +	for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
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;
> @@ -2471,7 +2477,7 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_all);
>   *
>   * See also:
>   * drm_atomic_helper_duplicate_state(), drm_atomic_helper_disable_all(),
> - * drm_atomic_helper_resume()
> + * drm_atomic_helper_resume(), drm_atomic_helper_commit_duplicated_state()
>   */
>  struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev)
>  {
> @@ -2512,6 +2518,47 @@ struct drm_atomic_state
> *drm_atomic_helper_suspend(struct drm_device *dev)
> EXPORT_SYMBOL(drm_atomic_helper_suspend);
> 
>  /**
> + * drm_atomic_helper_commit_duplicated_state - commit duplicated state
> + * @state: duplicated atomic state to commit
> + * @ctx: pointer to acquire_ctx to use for commit.
> + *
> + * The state returned by drm_atomic_helper_duplicate_state() and
> + * drm_atomic_helper_suspend() is partially invalid, and needs to
> + * be fixed up before commit.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + *
> + * See also:
> + * drm_atomic_helper_suspend()
> + */
> +int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state
> *state, +					      struct 
drm_modeset_acquire_ctx *ctx)
> +{
> +	int i;
> +	struct drm_plane *plane;
> +	struct drm_plane_state *plane_state;
> +	struct drm_connector *connector;
> +	struct drm_connector_state *conn_state;
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
> +
> +	state->acquire_ctx = ctx;
> +
> +	for_each_new_plane_in_state(state, plane, plane_state, i)
> +		state->planes[i].old_state = plane->state;
> +
> +	for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> +		state->crtcs[i].old_state = crtc->state;
> +
> +	for_each_new_connector_in_state(state, connector, conn_state, i)
> +		state->connectors[i].old_state = connector->state;
> +
> +	return drm_atomic_commit(state);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);
> +
> +/**
>   * drm_atomic_helper_resume - subsystem-level resume helper
>   * @dev: DRM device
>   * @state: atomic state to resume to
> @@ -2534,9 +2581,9 @@ int drm_atomic_helper_resume(struct drm_device *dev,
>  	int err;
> 
>  	drm_mode_config_reset(dev);
> +
>  	drm_modeset_lock_all(dev);
> -	state->acquire_ctx = config->acquire_ctx;
> -	err = drm_atomic_commit(state);
> +	err = drm_atomic_helper_commit_duplicated_state(state,
> config->acquire_ctx); drm_modeset_unlock_all(dev);
> 
>  	return err;
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index e2150a64860c..5364724df57f
> 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3489,7 +3489,8 @@ static void intel_update_primary_planes(struct
> drm_device *dev)
> 
>  static int
>  __intel_display_resume(struct drm_device *dev,
> -		       struct drm_atomic_state *state)
> +		       struct drm_atomic_state *state,
> +		       struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_crtc *crtc;
> @@ -3513,7 +3514,7 @@ __intel_display_resume(struct drm_device *dev,
>  	/* ignore any reset values/BIOS leftovers in the WM registers */
>  	to_intel_atomic_state(state)->skip_intermediate_wm = true;
> 
> -	ret = drm_atomic_commit(state);
> +	ret = drm_atomic_helper_commit_duplicated_state(state, ctx);
> 
>  	WARN_ON(ret == -EDEADLK);
>  	return ret;
> @@ -3607,7 +3608,7 @@ void intel_finish_reset(struct drm_i915_private
> *dev_priv) */
>  			intel_update_primary_planes(dev);
>  		} else {
> -			ret = __intel_display_resume(dev, state);
> +			ret = __intel_display_resume(dev, state, ctx);
>  			if (ret)
>  				DRM_ERROR("Restoring old state failed with 
%i\n", ret);
>  		}
> @@ -3627,7 +3628,7 @@ void intel_finish_reset(struct drm_i915_private
> *dev_priv) dev_priv->display.hpd_irq_setup(dev_priv);
>  		spin_unlock_irq(&dev_priv->irq_lock);
> 
> -		ret = __intel_display_resume(dev, state);
> +		ret = __intel_display_resume(dev, state, ctx);
>  		if (ret)
>  			DRM_ERROR("Restoring old state failed with %i\n", 
ret);
> 
> @@ -11328,7 +11329,7 @@ void intel_release_load_detect_pipe(struct
> drm_connector *connector, if (!state)
>  		return;
> 
> -	ret = drm_atomic_commit(state);
> +	ret = drm_atomic_helper_commit_duplicated_state(state, ctx);
>  	if (ret)
>  		DRM_DEBUG_KMS("Couldn't release load detect pipe: %i\n", ret);
>  	drm_atomic_state_put(state);
> @@ -17210,7 +17211,7 @@ void intel_display_resume(struct drm_device *dev)
>  	}
> 
>  	if (!ret)
> -		ret = __intel_display_resume(dev, state);
> +		ret = __intel_display_resume(dev, state, &ctx);
> 
>  	drm_modeset_drop_locks(&ctx);
>  	drm_modeset_acquire_fini(&ctx);
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index f96220ed4004..6062e7f27325 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -137,12 +137,12 @@ struct drm_crtc_commit {
> 
>  struct __drm_planes_state {
>  	struct drm_plane *ptr;
> -	struct drm_plane_state *state;
> +	struct drm_plane_state *state, *old_state, *new_state;
>  };
> 
>  struct __drm_crtcs_state {
>  	struct drm_crtc *ptr;
> -	struct drm_crtc_state *state;
> +	struct drm_crtc_state *state, *old_state, *new_state;
>  	struct drm_crtc_commit *commit;
>  	s64 __user *out_fence_ptr;
>  	unsigned last_vblank_count;
> @@ -150,7 +150,7 @@ struct __drm_crtcs_state {
> 
>  struct __drm_connnectors_state {
>  	struct drm_connector *ptr;
> -	struct drm_connector_state *state;
> +	struct drm_connector_state *state, *old_state, *new_state;
>  };
> 
>  /**
> @@ -397,6 +397,31 @@ void drm_state_dump(struct drm_device *dev, struct
> drm_printer *p); (__i)++)							
\
>  		for_each_if (connector)
> 
> +#define for_each_oldnew_connector_in_state(__state, connector,
> old_connector_state, new_connector_state, __i) \ +	for ((__i) = 0;		
						\
> +	     (__i) < (__state)->num_connector &&				
\
> +	     ((connector) = (__state)->connectors[__i].ptr,			
\
> +	     (old_connector_state) = (__state)->connectors[__i].old_state,	
\
> +	     (new_connector_state) = (__state)->connectors[__i].new_state, 1); 
	\
> +	     (__i)++)							\
> +		for_each_if (connector)
> +
> +#define for_each_old_connector_in_state(__state, connector,
> old_connector_state, __i) \ +	for ((__i) = 0;					
			\
> +	     (__i) < (__state)->num_connector &&				
\
> +	     ((connector) = (__state)->connectors[__i].ptr,			
\
> +	     (old_connector_state) = (__state)->connectors[__i].old_state, 1); 
	\
> +	     (__i)++)							\
> +		for_each_if (connector)
> +
> +#define for_each_new_connector_in_state(__state, connector,
> new_connector_state, __i) \ +	for ((__i) = 0;					
			\
> +	     (__i) < (__state)->num_connector &&				
\
> +	     ((connector) = (__state)->connectors[__i].ptr,			
\
> +	     (new_connector_state) = (__state)->connectors[__i].new_state, 1); 
	\
> +	     (__i)++)							\
> +		for_each_if (connector)
> +
>  #define for_each_crtc_in_state(__state, crtc, crtc_state, __i)	\
>  	for ((__i) = 0;						\
>  	     (__i) < (__state)->dev->mode_config.num_crtc &&	\
> @@ -405,6 +430,31 @@ void drm_state_dump(struct drm_device *dev, struct
> drm_printer *p); (__i)++)						\
>  		for_each_if (crtc_state)
> 
> +#define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state,
> new_crtc_state, __i) \ +	for ((__i) = 0;					
		\
> +	     (__i) < (__state)->dev->mode_config.num_crtc &&		\
> +	     ((crtc) = (__state)->crtcs[__i].ptr,			\
> +	     (old_crtc_state) = (__state)->crtcs[__i].old_state,	\
> +	     (new_crtc_state) = (__state)->crtcs[__i].new_state, 1);	\
> +	     (__i)++)							\
> +		for_each_if (crtc)
> +
> +#define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i)	
\
> +	for ((__i) = 0;							\
> +	     (__i) < (__state)->dev->mode_config.num_crtc &&		\
> +	     ((crtc) = (__state)->crtcs[__i].ptr,			\
> +	     (old_crtc_state) = (__state)->crtcs[__i].old_state, 1);	\
> +	     (__i)++)							\
> +		for_each_if (crtc)
> +
> +#define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i)	
\
> +	for ((__i) = 0;							\
> +	     (__i) < (__state)->dev->mode_config.num_crtc &&		\
> +	     ((crtc) = (__state)->crtcs[__i].ptr,			\
> +	     (new_crtc_state) = (__state)->crtcs[__i].new_state, 1);	\
> +	     (__i)++)							\
> +		for_each_if (crtc)
> +
>  #define for_each_plane_in_state(__state, plane, plane_state, __i)		
\
>  	for ((__i) = 0;							\
>  	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
> @@ -413,6 +463,31 @@ void drm_state_dump(struct drm_device *dev, struct
> drm_printer *p); (__i)++)							
\
>  		for_each_if (plane_state)
> 
> +#define for_each_oldnew_plane_in_state(__state, plane, old_plane_state,
> new_plane_state, __i) \ +	for ((__i) = 0;					
		\
> +	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
> +	     ((plane) = (__state)->planes[__i].ptr,			\
> +	     (old_plane_state) = (__state)->planes[__i].old_state,	\
> +	     (new_plane_state) = (__state)->planes[__i].new_state, 1);	\
> +	     (__i)++)							\
> +		for_each_if (plane)
> +
> +#define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \
> +	for ((__i) = 0;							\
> +	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
> +	     ((plane) = (__state)->planes[__i].ptr,			\
> +	     (old_plane_state) = (__state)->planes[__i].old_state, 1);	\
> +	     (__i)++)							\
> +		for_each_if (plane)
> +
> +#define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \
> +	for ((__i) = 0;							\
> +	     (__i) < (__state)->dev->mode_config.num_total_plane &&	\
> +	     ((plane) = (__state)->planes[__i].ptr,			\
> +	     (new_plane_state) = (__state)->planes[__i].new_state, 1);	\
> +	     (__i)++)							\
> +		for_each_if (plane)
> +
>  /**
>   * drm_atomic_crtc_needs_modeset - compute combined modeset need
>   * @state: &drm_crtc_state for the CRTC
> diff --git a/include/drm/drm_atomic_helper.h
> b/include/drm/drm_atomic_helper.h index 4b2353dc34ba..e43d4de2ff8f 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -105,6 +105,8 @@ int __drm_atomic_helper_set_config(struct drm_mode_set
> *set, int drm_atomic_helper_disable_all(struct drm_device *dev,
>  				  struct drm_modeset_acquire_ctx *ctx);
>  struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev);
> +int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state
> *state, +					      struct 
drm_modeset_acquire_ctx *ctx);
>  int drm_atomic_helper_resume(struct drm_device *dev,
>  			     struct drm_atomic_state *state);
> 
> -- 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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] 13+ messages in thread

* Re: [PATCH v2.1 1/7] drm/atomic: Add new iterators over all state, v3.
  2017-01-15 19:56     ` Laurent Pinchart
@ 2017-01-16  9:26       ` Maarten Lankhorst
  0 siblings, 0 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2017-01-16  9:26 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel; +Cc: intel-gfx

Op 15-01-17 om 20:56 schreef Laurent Pinchart:
> Hi Maarten,
>
> Thank you for the patch.
>
> On Tuesday 10 Jan 2017 14:46:58 Maarten Lankhorst wrote:
>> Add for_each_(old)(new)_(plane,connector,crtc)_in_state iterators to
>> replace the old for_each_xxx_in_state ones. This is useful for >1 flip
>> depth and getting rid of all xxx->state dereferences.
>>
>> This requires extra fixups done when committing a state after
>> duplicating, which in general isn't valid but is used by suspend/resume.
>> To handle these, introduce drm_atomic_helper_commit_duplicated_state
>> which performs those fixups before checking & committing the state.
>>
>> Changes since v1:
>> - Remove nonblock parameter for commit_duplicated_state.
>> Changes since v2:
>> - Use commit_duplicated_state for i915 load detection.
>> - Add WARN_ON(old_state != obj->state) before swapping.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> Forgot that there was a v2. It still broke on igt load detect tests, this
>> should be the fix.
>>
>> I also added a big WARN_ON before swapping to ensure old state is correct.
>> At this point we can no longer fix it because atomic_check is called with
>> the wrong items, but it will help limit the damage.
> This patch now conflicts with "[PATCH v2 4/7] drm/atomic: Fix atomic helpers 
> to use the new iterator macros.". Could you fix that and repost the whole 
> series ?
Yeah I noticed. I wanted to resend because patch 7 had a small bug too, but trybot was failing on some tests.
After doing some bisection by sending patches it turns out the error went away by itself. The failing test also didn't seem related to the series.
With that issue gone, I'll resend.

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-01-16  9:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-10 10:21 [PATCH v2 0/7] drm/atomic: Add accessor macros for all atomic state Maarten Lankhorst
2017-01-10 10:21 ` [PATCH v2 1/7] drm/atomic: Add new iterators over all state Maarten Lankhorst
2017-01-10 13:46   ` [PATCH v2.1 1/7] drm/atomic: Add new iterators over all state, v3 Maarten Lankhorst
2017-01-15 19:56     ` Laurent Pinchart
2017-01-16  9:26       ` Maarten Lankhorst
2017-01-10 10:21 ` [PATCH v2 2/7] drm/atomic: Make add_affected_connectors look at crtc_state Maarten Lankhorst
2017-01-10 10:21 ` [PATCH v2 3/7] drm/atomic: Use new atomic iterator macros Maarten Lankhorst
2017-01-10 10:21 ` [PATCH v2 4/7] drm/atomic: Fix atomic helpers to use the new " Maarten Lankhorst
2017-01-10 10:21 ` [PATCH v2 5/7] drm/atomic: Add macros to access existing old/new state Maarten Lankhorst
2017-01-10 10:21 ` [PATCH v2 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state Maarten Lankhorst
2017-01-12  9:27   ` [PATCH v2.1 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state, v2 Maarten Lankhorst
2017-01-10 10:21 ` [PATCH v2 7/7] drm/blend: Use new atomic iterator macros Maarten Lankhorst
2017-01-10 12:23 ` ✗ Fi.CI.BAT: warning for drm/atomic: Add accessor macros for all atomic state 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.