[PATCH v4 2/5] drm/atomic: Make drm_atomic_plane_disabling easier to understand.

[Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>]

This function becomes a lot simpler when having passed both the old and
new state to it. Looking at all callers, it seems that old_plane_state
is never NULL so the check can be dropped.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
---
 drivers/gpu/drm/drm_atomic_helper.c |  7 ++++---
 drivers/gpu/drm/drm_plane_helper.c  |  2 +-
 include/drm/drm_atomic_helper.h     | 26 ++++++++------------------
 3 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 7d432d9a18cf..ea544bddc29b 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1757,7 +1757,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 		if (!funcs)
 			continue;
 
-		disabling = drm_atomic_plane_disabling(plane, old_plane_state);
+		disabling = drm_atomic_plane_disabling(old_plane_state,
+						       new_plane_state);
 
 		if (active_only) {
 			/*
@@ -1852,11 +1853,11 @@ drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state)
 
 		WARN_ON(plane->state->crtc && plane->state->crtc != crtc);
 
-		if (drm_atomic_plane_disabling(plane, old_plane_state) &&
+		if (drm_atomic_plane_disabling(old_plane_state, plane->state) &&
 		    plane_funcs->atomic_disable)
 			plane_funcs->atomic_disable(plane, old_plane_state);
 		else if (plane->state->crtc ||
-			 drm_atomic_plane_disabling(plane, old_plane_state))
+			 drm_atomic_plane_disabling(old_plane_state, plane->state))
 			plane_funcs->atomic_update(plane, old_plane_state);
 	}
 
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 148688fb920a..f4d70dd5e3e4 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -470,7 +470,7 @@ int drm_plane_helper_commit(struct drm_plane *plane,
 	 * Drivers may optionally implement the ->atomic_disable callback, so
 	 * special-case that here.
 	 */
-	if (drm_atomic_plane_disabling(plane, plane_state) &&
+	if (drm_atomic_plane_disabling(plane_state, plane->state) &&
 	    plane_funcs->atomic_disable)
 		plane_funcs->atomic_disable(plane, plane_state);
 	else
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 9ceda379ce58..dc16274987c7 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -220,10 +220,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 			      __drm_atomic_get_current_plane_state((crtc_state)->state, \
 								   plane)))
 
-/*
+/**
  * drm_atomic_plane_disabling - check whether a plane is being disabled
- * @plane: plane object
- * @old_state: previous atomic state
+ * @old_plane_state: old atomic plane state
+ * @new_plane_state: new atomic plane state
  *
  * Checks the atomic state of a plane to determine whether it's being disabled
  * or not. This also WARNs if it detects an invalid state (both CRTC and FB
@@ -233,28 +233,18 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
  * True if the plane is being disabled, false otherwise.
  */
 static inline bool
-drm_atomic_plane_disabling(struct drm_plane *plane,
-			   struct drm_plane_state *old_state)
+drm_atomic_plane_disabling(struct drm_plane_state *old_plane_state,
+			   struct drm_plane_state *new_plane_state)
 {
 	/*
 	 * When disabling a plane, CRTC and FB should always be NULL together.
 	 * Anything else should be considered a bug in the atomic core, so we
 	 * gently warn about it.
 	 */
-	WARN_ON((plane->state->crtc == NULL && plane->state->fb != NULL) ||
-		(plane->state->crtc != NULL && plane->state->fb == NULL));
+	WARN_ON((new_plane_state->crtc == NULL && new_plane_state->fb != NULL) ||
+		(new_plane_state->crtc != NULL && new_plane_state->fb == NULL));
 
-	/*
-	 * When using the transitional helpers, old_state may be NULL. If so,
-	 * we know nothing about the current state and have to assume that it
-	 * might be enabled.
-	 *
-	 * When using the atomic helpers, old_state won't be NULL. Therefore
-	 * this check assumes that either the driver will have reconstructed
-	 * the correct state in ->reset() or that the driver will have taken
-	 * appropriate measures to disable all planes.
-	 */
-	return (!old_state || old_state->crtc) && !plane->state->crtc;
+	return old_plane_state->crtc && !new_plane_state->crtc;
 }
 
 #endif /* DRM_ATOMIC_HELPER_H_ */
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel