Nouveau - Jul 2017 - [PATCH v2 6/7] drm/nouveau: Convert nouveau to use new iterator macros, v2.

Use the new atomic iterator macros, the old ones are about to be
removed. With the new macros, it's more easy to get old and new state so
get them from the macros instead of from obj->state.

Changes since v1:
- Don't mix up old and new state. (danvet)
- Rebase on top of interruptible swap_state changes.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Cc: Ben Skeggs <bskeggs at redhat.com>
Cc: nouveau at lists.freedesktop.org
---
 drivers/gpu/drm/nouveau/nv50_display.c | 72 +++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
b/drivers/gpu/drm/nouveau/nv50_display.c
index 747c99c1e474..7abfb561b00c 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2103,7 +2103,7 @@ nv50_head_atomic_check(struct drm_crtc *crtc, struct
drm_crtc_state *state)
 
 	NV_ATOMIC(drm, "%s atomic_check %d\n", crtc->name,
asyh->state.active);
 	if (asyh->state.active) {
-		for_each_connector_in_state(asyh->state.state, conn, conns, i) {
+		for_each_new_connector_in_state(asyh->state.state, conn, conns, i) {
 			if (conns->crtc == crtc) {
 				asyc = nouveau_conn_atom(conns);
 				break;
@@ -3905,9 +3905,9 @@ static void
 nv50_disp_atomic_commit_tail(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_crtc *crtc;
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *new_plane_state;
 	struct drm_plane *plane;
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nv50_disp *disp = nv50_disp(dev);
@@ -3926,8 +3926,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
 		mutex_lock(&disp->mutex);
 
 	/* Disable head(s). */
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		struct nv50_head_atom *asyh = nv50_head_atom(crtc->state);
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+		struct nv50_head_atom *asyh = nv50_head_atom(new_crtc_state);
 		struct nv50_head *head = nv50_head(crtc);
 
 		NV_ATOMIC(drm, "%s: clr %04x (set %04x)\n", crtc->name,
@@ -3940,8 +3940,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
 	}
 
 	/* Disable plane(s). */
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
+	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+		struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
 		struct nv50_wndw *wndw = nv50_wndw(plane);
 
 		NV_ATOMIC(drm, "%s: clr %02x (set %02x)\n", plane->name,
@@ -4006,8 +4006,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
 	}
 
 	/* Update head(s). */
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		struct nv50_head_atom *asyh = nv50_head_atom(crtc->state);
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+		struct nv50_head_atom *asyh = nv50_head_atom(new_crtc_state);
 		struct nv50_head *head = nv50_head(crtc);
 
 		NV_ATOMIC(drm, "%s: set %04x (clr %04x)\n", crtc->name,
@@ -4019,14 +4019,14 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
 		}
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (crtc->state->event)
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+		if (new_crtc_state->event)
 			drm_crtc_vblank_get(crtc);
 	}
 
 	/* Update plane(s). */
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
+	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+		struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
 		struct nv50_wndw *wndw = nv50_wndw(plane);
 
 		NV_ATOMIC(drm, "%s: set %02x (clr %02x)\n", plane->name,
@@ -4056,23 +4056,23 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
 		mutex_unlock(&disp->mutex);
 
 	/* Wait for HW to signal completion. */
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
+	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+		struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
 		struct nv50_wndw *wndw = nv50_wndw(plane);
 		int ret = nv50_wndw_wait_armed(wndw, asyw);
 		if (ret)
 			NV_ERROR(drm, "%s: timeout\n", plane->name);
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (crtc->state->event) {
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+		if (new_crtc_state->event) {
 			unsigned long flags;
 			/* Get correct count/ts if racing with vblank irq */
 			drm_crtc_accurate_vblank_count(crtc);
 			spin_lock_irqsave(&crtc->dev->event_lock, flags);
-			drm_crtc_send_vblank_event(crtc, crtc->state->event);
+			drm_crtc_send_vblank_event(crtc, new_crtc_state->event);
 			spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
-			crtc->state->event = NULL;
+			new_crtc_state->event = NULL;
 			drm_crtc_vblank_put(crtc);
 		}
 	}
@@ -4097,7 +4097,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
 {
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nv50_disp *disp = nv50_disp(dev);
-	struct drm_plane_state *plane_state;
+	struct drm_plane_state *old_plane_state;
 	struct drm_plane *plane;
 	struct drm_crtc *crtc;
 	bool active = false;
@@ -4127,9 +4127,10 @@ nv50_disp_atomic_commit(struct drm_device *dev,
 	if (ret)
 		goto err_cleanup;
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane_state);
+	for_each_old_plane_in_state(state, plane, old_plane_state, i) {
+		struct nv50_wndw_atom *asyw = nv50_wndw_atom(old_plane_state);
 		struct nv50_wndw *wndw = nv50_wndw(plane);
+
 		if (asyw->set.image) {
 			asyw->ntfy.handle = wndw->dmac->sync.handle;
 			asyw->ntfy.offset = wndw->ntfy;
@@ -4192,18 +4193,19 @@ nv50_disp_outp_atomic_add(struct nv50_atom *atom, struct
drm_encoder *encoder)
 
 static int
 nv50_disp_outp_atomic_check_clr(struct nv50_atom *atom,
-				struct drm_connector *connector)
+				struct drm_connector_state *old_connector_state)
 {
-	struct drm_encoder *encoder = connector->state->best_encoder;
-	struct drm_crtc_state *crtc_state;
+	struct drm_encoder *encoder = old_connector_state->best_encoder;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_crtc *crtc;
 	struct nv50_outp_atom *outp;
 
-	if (!(crtc = connector->state->crtc))
+	if (!(crtc = old_connector_state->crtc))
 		return 0;
 
-	crtc_state = drm_atomic_get_existing_crtc_state(&atom->state, crtc);
-	if (crtc->state->active &&
drm_atomic_crtc_needs_modeset(crtc_state)) {
+	old_crtc_state = drm_atomic_get_old_crtc_state(&atom->state, crtc);
+	new_crtc_state = drm_atomic_get_new_crtc_state(&atom->state, crtc);
+	if (old_crtc_state->active &&
drm_atomic_crtc_needs_modeset(new_crtc_state)) {
 		outp = nv50_disp_outp_atomic_add(atom, encoder);
 		if (IS_ERR(outp))
 			return PTR_ERR(outp);
@@ -4224,15 +4226,15 @@ nv50_disp_outp_atomic_check_set(struct nv50_atom *atom,
 				struct drm_connector_state *connector_state)
 {
 	struct drm_encoder *encoder = connector_state->best_encoder;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *new_crtc_state;
 	struct drm_crtc *crtc;
 	struct nv50_outp_atom *outp;
 
 	if (!(crtc = connector_state->crtc))
 		return 0;
 
-	crtc_state = drm_atomic_get_existing_crtc_state(&atom->state, crtc);
-	if (crtc_state->active &&
drm_atomic_crtc_needs_modeset(crtc_state)) {
+	new_crtc_state = drm_atomic_get_new_crtc_state(&atom->state, crtc);
+	if (new_crtc_state->active &&
drm_atomic_crtc_needs_modeset(new_crtc_state)) {
 		outp = nv50_disp_outp_atomic_add(atom, encoder);
 		if (IS_ERR(outp))
 			return PTR_ERR(outp);
@@ -4248,7 +4250,7 @@ static int
 nv50_disp_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
 {
 	struct nv50_atom *atom = nv50_atom(state);
-	struct drm_connector_state *connector_state;
+	struct drm_connector_state *old_connector_state, *new_connector_state;
 	struct drm_connector *connector;
 	int ret, i;
 
@@ -4256,12 +4258,12 @@ nv50_disp_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
 	if (ret)
 		return ret;
 
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		ret = nv50_disp_outp_atomic_check_clr(atom, connector);
+	for_each_oldnew_connector_in_state(state, connector, old_connector_state,
new_connector_state, i) {
+		ret = nv50_disp_outp_atomic_check_clr(atom, old_connector_state);
 		if (ret)
 			return ret;
 
-		ret = nv50_disp_outp_atomic_check_set(atom, connector_state);
+		ret = nv50_disp_outp_atomic_check_set(atom, new_connector_state);
 		if (ret)
 			return ret;
 	}
-- 
2.11.0

On Wed, Jul 19, 2017 at 04:39:19PM +0200, Maarten Lankhorst
wrote:
> Use the new atomic iterator macros, the old ones are about to be
> removed. With the new macros, it's more easy to get old and new state
so
> get them from the macros instead of from obj->state.
> 
> Changes since v1:
> - Don't mix up old and new state. (danvet)
> - Rebase on top of interruptible swap_state changes.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at
linux.intel.com>
> Cc: Ben Skeggs <bskeggs at redhat.com>
> Cc: nouveau at lists.freedesktop.org
Reviewed-by: Daniel Vetter <daniel.vetter at
ffwll.ch>
> ---
>  drivers/gpu/drm/nouveau/nv50_display.c | 72
+++++++++++++++++-----------------
>  1 file changed, 37 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
b/drivers/gpu/drm/nouveau/nv50_display.c
> index 747c99c1e474..7abfb561b00c 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -2103,7 +2103,7 @@ nv50_head_atomic_check(struct drm_crtc *crtc, struct
drm_crtc_state *state)
>  
>  	NV_ATOMIC(drm, "%s atomic_check %d\n", crtc->name,
asyh->state.active);
>  	if (asyh->state.active) {
> -		for_each_connector_in_state(asyh->state.state, conn, conns, i) {
> +		for_each_new_connector_in_state(asyh->state.state, conn, conns, i) {
>  			if (conns->crtc == crtc) {
>  				asyc = nouveau_conn_atom(conns);
>  				break;
> @@ -3905,9 +3905,9 @@ static void
>  nv50_disp_atomic_commit_tail(struct drm_atomic_state *state)
>  {
>  	struct drm_device *dev = state->dev;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_crtc *crtc;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *new_plane_state;
>  	struct drm_plane *plane;
>  	struct nouveau_drm *drm = nouveau_drm(dev);
>  	struct nv50_disp *disp = nv50_disp(dev);
> @@ -3926,8 +3926,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
>  		mutex_lock(&disp->mutex);
>  
>  	/* Disable head(s). */
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		struct nv50_head_atom *asyh = nv50_head_atom(crtc->state);
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		struct nv50_head_atom *asyh = nv50_head_atom(new_crtc_state);
>  		struct nv50_head *head = nv50_head(crtc);
>  
>  		NV_ATOMIC(drm, "%s: clr %04x (set %04x)\n", crtc->name,
> @@ -3940,8 +3940,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
>  	}
>  
>  	/* Disable plane(s). */
> -	for_each_plane_in_state(state, plane, plane_state, i) {
> -		struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
> +	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
> +		struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
>  		struct nv50_wndw *wndw = nv50_wndw(plane);
>  
>  		NV_ATOMIC(drm, "%s: clr %02x (set %02x)\n", plane->name,
> @@ -4006,8 +4006,8 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state
*state)
>  	}
>  
>  	/* Update head(s). */
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		struct nv50_head_atom *asyh = nv50_head_atom(crtc->state);
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		struct nv50_head_atom *asyh = nv50_head_atom(new_crtc_state);
>  		struct nv50_head *head = nv50_head(crtc);
>  
>  		NV_ATOMIC(drm, "%s: set %04x (clr %04x)\n", crtc->name,
> @@ -4019,14 +4019,14 @@ nv50_disp_atomic_commit_tail(struct
drm_atomic_state *state)
>  		}
>  	}
>  
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		if (crtc->state->event)
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (new_crtc_state->event)
>  			drm_crtc_vblank_get(crtc);
>  	}
>  
>  	/* Update plane(s). */
> -	for_each_plane_in_state(state, plane, plane_state, i) {
> -		struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
> +	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
> +		struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
>  		struct nv50_wndw *wndw = nv50_wndw(plane);
>  
>  		NV_ATOMIC(drm, "%s: set %02x (clr %02x)\n", plane->name,
> @@ -4056,23 +4056,23 @@ nv50_disp_atomic_commit_tail(struct
drm_atomic_state *state)
>  		mutex_unlock(&disp->mutex);
>  
>  	/* Wait for HW to signal completion. */
> -	for_each_plane_in_state(state, plane, plane_state, i) {
> -		struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane->state);
> +	for_each_new_plane_in_state(state, plane, new_plane_state, i) {
> +		struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
>  		struct nv50_wndw *wndw = nv50_wndw(plane);
>  		int ret = nv50_wndw_wait_armed(wndw, asyw);
>  		if (ret)
>  			NV_ERROR(drm, "%s: timeout\n", plane->name);
>  	}
>  
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		if (crtc->state->event) {
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (new_crtc_state->event) {
>  			unsigned long flags;
>  			/* Get correct count/ts if racing with vblank irq */
>  			drm_crtc_accurate_vblank_count(crtc);
>  			spin_lock_irqsave(&crtc->dev->event_lock, flags);
> -			drm_crtc_send_vblank_event(crtc, crtc->state->event);
> +			drm_crtc_send_vblank_event(crtc, new_crtc_state->event);
>  			spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> -			crtc->state->event = NULL;
> +			new_crtc_state->event = NULL;
>  			drm_crtc_vblank_put(crtc);
>  		}
>  	}
> @@ -4097,7 +4097,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
>  {
>  	struct nouveau_drm *drm = nouveau_drm(dev);
>  	struct nv50_disp *disp = nv50_disp(dev);
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *old_plane_state;
>  	struct drm_plane *plane;
>  	struct drm_crtc *crtc;
>  	bool active = false;
> @@ -4127,9 +4127,10 @@ nv50_disp_atomic_commit(struct drm_device *dev,
>  	if (ret)
>  		goto err_cleanup;
>  
> -	for_each_plane_in_state(state, plane, plane_state, i) {
> -		struct nv50_wndw_atom *asyw = nv50_wndw_atom(plane_state);
> +	for_each_old_plane_in_state(state, plane, old_plane_state, i) {
> +		struct nv50_wndw_atom *asyw = nv50_wndw_atom(old_plane_state);
>  		struct nv50_wndw *wndw = nv50_wndw(plane);
> +
>  		if (asyw->set.image) {
>  			asyw->ntfy.handle = wndw->dmac->sync.handle;
>  			asyw->ntfy.offset = wndw->ntfy;
> @@ -4192,18 +4193,19 @@ nv50_disp_outp_atomic_add(struct nv50_atom *atom,
struct drm_encoder *encoder)
>  
>  static int
>  nv50_disp_outp_atomic_check_clr(struct nv50_atom *atom,
> -				struct drm_connector *connector)
> +				struct drm_connector_state *old_connector_state)
>  {
> -	struct drm_encoder *encoder = connector->state->best_encoder;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_encoder *encoder = old_connector_state->best_encoder;
> +	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct drm_crtc *crtc;
>  	struct nv50_outp_atom *outp;
>  
> -	if (!(crtc = connector->state->crtc))
> +	if (!(crtc = old_connector_state->crtc))
>  		return 0;
>  
> -	crtc_state = drm_atomic_get_existing_crtc_state(&atom->state,
crtc);
> -	if (crtc->state->active &&
drm_atomic_crtc_needs_modeset(crtc_state)) {
> +	old_crtc_state = drm_atomic_get_old_crtc_state(&atom->state,
crtc);
> +	new_crtc_state = drm_atomic_get_new_crtc_state(&atom->state,
crtc);
> +	if (old_crtc_state->active &&
drm_atomic_crtc_needs_modeset(new_crtc_state)) {
>  		outp = nv50_disp_outp_atomic_add(atom, encoder);
>  		if (IS_ERR(outp))
>  			return PTR_ERR(outp);
> @@ -4224,15 +4226,15 @@ nv50_disp_outp_atomic_check_set(struct nv50_atom
*atom,
>  				struct drm_connector_state *connector_state)
>  {
>  	struct drm_encoder *encoder = connector_state->best_encoder;
> -	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc_state *new_crtc_state;
>  	struct drm_crtc *crtc;
>  	struct nv50_outp_atom *outp;
>  
>  	if (!(crtc = connector_state->crtc))
>  		return 0;
>  
> -	crtc_state = drm_atomic_get_existing_crtc_state(&atom->state,
crtc);
> -	if (crtc_state->active &&
drm_atomic_crtc_needs_modeset(crtc_state)) {
> +	new_crtc_state = drm_atomic_get_new_crtc_state(&atom->state,
crtc);
> +	if (new_crtc_state->active &&
drm_atomic_crtc_needs_modeset(new_crtc_state)) {
>  		outp = nv50_disp_outp_atomic_add(atom, encoder);
>  		if (IS_ERR(outp))
>  			return PTR_ERR(outp);
> @@ -4248,7 +4250,7 @@ static int
>  nv50_disp_atomic_check(struct drm_device *dev, struct drm_atomic_state
*state)
>  {
>  	struct nv50_atom *atom = nv50_atom(state);
> -	struct drm_connector_state *connector_state;
> +	struct drm_connector_state *old_connector_state, *new_connector_state;
>  	struct drm_connector *connector;
>  	int ret, i;
>  
> @@ -4256,12 +4258,12 @@ nv50_disp_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
>  	if (ret)
>  		return ret;
>  
> -	for_each_connector_in_state(state, connector, connector_state, i) {
> -		ret = nv50_disp_outp_atomic_check_clr(atom, connector);
> +	for_each_oldnew_connector_in_state(state, connector, old_connector_state,
new_connector_state, i) {
> +		ret = nv50_disp_outp_atomic_check_clr(atom, old_connector_state);
>  		if (ret)
>  			return ret;
>  
> -		ret = nv50_disp_outp_atomic_check_set(atom, connector_state);
> +		ret = nv50_disp_outp_atomic_check_set(atom, new_connector_state);
>  		if (ret)
>  			return ret;
>  	}
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch