Op 13-07-17 om 14:33 schreef Daniel Vetter: > On Wed, Jul 12, 2017 at 10:13:43AM +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. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> Cc: Ben Skeggs <bskeggs@xxxxxxxxxx> >> Cc: nouveau@xxxxxxxxxxxxxxxxxxxxx >> --- >> drivers/gpu/drm/nouveau/nv50_display.c | 71 +++++++++++++++++----------------- >> 1 file changed, 36 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c >> index 42a85c14aea0..b1ce8f1f58dc 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; >> @@ -3904,9 +3904,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); >> @@ -3925,8 +3925,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, >> @@ -3939,8 +3939,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, >> @@ -4005,8 +4005,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, >> @@ -4018,14 +4018,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, >> @@ -4055,23 +4055,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); >> } >> } >> @@ -4096,7 +4096,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; >> @@ -4122,8 +4122,8 @@ nv50_disp_atomic_commit(struct drm_device *dev, >> goto done; >> } >> >> - 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; >> @@ -4185,18 +4185,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_new_crtc_state(&atom->state, crtc); >> + new_crtc_state = drm_atomic_get_new_crtc_state(&atom->state, crtc); > I think you have a mixup here of the crtc states for old and new crtc, > both are get_new_crtc_state. Ah indeed, thanks for catching. :) > Otherwise lgtm. > -Daniel > >> + 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); >> @@ -4217,15 +4218,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); >> @@ -4241,7 +4242,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; >> >> @@ -4249,12 +4250,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 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel