[01/26] drm/atomic-helper: use for_each_*_in_state more

Commit Message

Daniel Vetter May 29, 2016, 6:34 p.m. UTC

This avois leaking drm_atomic_state internals into the helpers. The
only place where this still happens after this patch is drm_atomic_helper_swap_state().
It's unavoidable there, and maybe a good indicator we should actually
move that function into drm_atomic.c.

Signed-off-by: Daniel Vetter <[e-mail]>
---
 drivers/gpu/drm/drm_atomic_helper.c | 47 +++++++++++++------------------------
 1 file changed, 16 insertions(+), 31 deletions(-)

Comments

Maarten Lankhorst May 30, 2016, 12:17 p.m. UTC | #1

Op 29-05-16 om 20:34 schreef Daniel Vetter:
> This avois leaking drm_atomic_state internals into the helpers. The
> only place where this still happens after this patch is drm_atomic_helper_swap_state().
> It's unavoidable there, and maybe a good indicator we should actually
> move that function into drm_atomic.c.
Would be a good idea, commit is part of atomic core and there's really only one way to do swap_state.
Drivers can swap any required internal state before or after the call.

~Maarten.

Daniel Vetter May 30, 2016, 3:02 p.m. UTC | #2

On Mon, May 30, 2016 at 02:17:06PM +0200, Maarten Lankhorst wrote:
> Op 29-05-16 om 20:34 schreef Daniel Vetter:
> > This avois leaking drm_atomic_state internals into the helpers. The
> > only place where this still happens after this patch is drm_atomic_helper_swap_state().
> > It's unavoidable there, and maybe a good indicator we should actually
> > move that function into drm_atomic.c.
> Would be a good idea, commit is part of atomic core and there's really only one way to do swap_state.
> Drivers can swap any required internal state before or after the call.

Later on I'm adding some of the nonblocking stuff to swap_state, and I
think at least the nonblocking support should be part of the helpers.

I'm torn again about where to put it ;-)
-Daniel

Patch

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ddfa0d120e39..464541c87c40 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -611,7 +611,7 @@  drm_atomic_helper_check_planes(struct drm_device *dev,
 		if (!funcs || !funcs->atomic_check)
 			continue;
 
-		ret = funcs->atomic_check(crtc, state->crtc_states[i]);
+		ret = funcs->atomic_check(crtc, crtc_state);
 		if (ret) {
 			DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic driver check failed\n",
 					 crtc->base.id, crtc->name);
@@ -1249,16 +1249,12 @@  EXPORT_SYMBOL(drm_atomic_helper_commit);
 int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state)
 {
-	int nplanes = dev->mode_config.num_total_plane;
-	int ret, i;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
+	int ret, i, j;
 
-	for (i = 0; i < nplanes; i++) {
+	for_each_plane_in_state(state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
-		struct drm_plane *plane = state->planes[i];
-		struct drm_plane_state *plane_state = state->plane_states[i];
-
-		if (!plane)
-			continue;
 
 		funcs = plane->helper_private;
 
@@ -1272,12 +1268,10 @@  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 	return 0;
 
 fail:
-	for (i--; i >= 0; i--) {
+	for_each_plane_in_state(state, plane, plane_state, j) {
 		const struct drm_plane_helper_funcs *funcs;
-		struct drm_plane *plane = state->planes[i];
-		struct drm_plane_state *plane_state = state->plane_states[i];
 
-		if (!plane)
+		if (j >= i)
 			continue;
 
 		funcs = plane->helper_private;
@@ -1564,35 +1558,26 @@  void drm_atomic_helper_swap_state(struct drm_device *dev,
 				  struct drm_atomic_state *state)
 {
 	int i;
+	struct drm_connector *connector;
+	struct drm_connector_state *conn_state;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
 
-	for (i = 0; i < state->num_connector; i++) {
-		struct drm_connector *connector = state->connectors[i];
-
-		if (!connector)
-			continue;
-
+	for_each_connector_in_state(state, connector, conn_state, i) {
 		connector->state->state = state;
 		swap(state->connector_states[i], connector->state);
 		connector->state->state = NULL;
 	}
 
-	for (i = 0; i < dev->mode_config.num_crtc; i++) {
-		struct drm_crtc *crtc = state->crtcs[i];
-
-		if (!crtc)
-			continue;
-
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		crtc->state->state = state;
 		swap(state->crtc_states[i], crtc->state);
 		crtc->state->state = NULL;
 	}
 
-	for (i = 0; i < dev->mode_config.num_total_plane; i++) {
-		struct drm_plane *plane = state->planes[i];
-
-		if (!plane)
-			continue;
-
+	for_each_plane_in_state(state, plane, plane_state, i) {
 		plane->state->state = state;
 		swap(state->plane_states[i], plane->state);
 		plane->state->state = NULL;