| Message ID | 20170830121752.31291-4-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Maarten, Thank you for the patch. On Wednesday, 30 August 2017 15:17:50 EEST Maarten Lankhorst wrote: > Most code only cares about the current commit or previous commit. > Fortuantely we already have a place to track those. Move it to > drm_crtc_state where it belongs. :) > > The per-crtc commit_list is kept for places where we have to look > deeper than the current or previous commit for checking whether to stall > on unpin. This is used in drm_atomic_helper_setup_commit and > intel_has_pending_fb_unpin. > > Changes since v1: > - Update kerneldoc for drm_crtc.commit_list. (danvet) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_atomic.c | 7 --- > drivers/gpu/drm/drm_atomic_helper.c | 92 +++++++++------------------------- > include/drm/drm_atomic.h | 1 - > include/drm/drm_crtc.h | 23 ++++++++-- > 4 files changed, 42 insertions(+), 81 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 2fd383d7253a..2cce48f203e0 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -163,13 +163,6 @@ void drm_atomic_state_default_clear(struct > drm_atomic_state *state) crtc->funcs->atomic_destroy_state(crtc, > state->crtcs[i].state); > > - if (state->crtcs[i].commit) { > - kfree(state->crtcs[i].commit->event); > - state->crtcs[i].commit->event = NULL; > - drm_crtc_commit_put(state->crtcs[i].commit); > - } > - > - state->crtcs[i].commit = NULL; > state->crtcs[i].ptr = NULL; > state->crtcs[i].state = NULL; > } > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index 11d0e94a2181..8ccb8b6536c0 > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1262,12 +1262,12 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks); > void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev, > struct drm_atomic_state *old_state) > { > - struct drm_crtc_state *unused; > + struct drm_crtc_state *new_crtc_state; > struct drm_crtc *crtc; > int i; > > - for_each_new_crtc_in_state(old_state, crtc, unused, i) { > - struct drm_crtc_commit *commit = old_state->crtcs[i].commit; > + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > + struct drm_crtc_commit *commit = new_crtc_state->commit; > int ret; > > if (!commit) > @@ -1388,11 +1388,10 @@ int drm_atomic_helper_async_check(struct drm_device > *dev, { > struct drm_crtc *crtc; > struct drm_crtc_state *crtc_state; > - struct drm_crtc_commit *commit; > struct drm_plane *__plane, *plane = NULL; > struct drm_plane_state *__plane_state, *plane_state = NULL; > const struct drm_plane_helper_funcs *funcs; > - int i, j, n_planes = 0; > + int i, n_planes = 0; > > for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > if (drm_atomic_crtc_needs_modeset(crtc_state)) > @@ -1420,33 +1419,10 @@ int drm_atomic_helper_async_check(struct drm_device > *dev, return -EINVAL; > > /* > - * Don't do an async update if there is an outstanding commit modifying > + * TODO: Don't do an async update if there is an outstanding commit > modifying > * the plane. This prevents our async update's changes from getting > * overridden by a previous synchronous update's state. > */ As mentioned in a comment to your previous patch, this is unrelated to $SUBJECT. You should mention and explain this change in the commit message (possibly splitting it out in a separate commit if you think that would make more sense, up to you). > - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > - if (plane->crtc != crtc) > - continue; > - > - spin_lock(&crtc->commit_lock); > - commit = list_first_entry_or_null(&crtc->commit_list, > - struct drm_crtc_commit, > - commit_entry); > - if (!commit) { > - spin_unlock(&crtc->commit_lock); > - continue; > - } > - spin_unlock(&crtc->commit_lock); > - > - if (!crtc->state->state) > - continue; > - > - for_each_plane_in_state(crtc->state->state, __plane, > - __plane_state, j) { > - if (__plane == plane) > - return -EINVAL; > - } > - } > > return funcs->atomic_async_check(plane, plane_state); > } > @@ -1731,7 +1707,7 @@ int drm_atomic_helper_setup_commit(struct > drm_atomic_state *state, kref_init(&commit->ref); > commit->crtc = crtc; > > - state->crtcs[i].commit = commit; > + new_crtc_state->commit = commit; > > ret = stall_checks(crtc, nonblock); > if (ret) > @@ -1769,22 +1745,6 @@ int drm_atomic_helper_setup_commit(struct > drm_atomic_state *state, } > EXPORT_SYMBOL(drm_atomic_helper_setup_commit); > > - > -static struct drm_crtc_commit *preceeding_commit(struct drm_crtc *crtc) > -{ > - struct drm_crtc_commit *commit; > - int i = 0; > - > - list_for_each_entry(commit, &crtc->commit_list, commit_entry) { > - /* skip the first entry, that's the current commit */ > - if (i == 1) > - return commit; > - i++; > - } > - > - return NULL; > -} > - > /** > * drm_atomic_helper_wait_for_dependencies - wait for required preceeding > commits * @old_state: atomic state object with old state structures > @@ -1800,17 +1760,13 @@ static struct drm_crtc_commit > *preceeding_commit(struct drm_crtc *crtc) void > drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *new_crtc_state; > + struct drm_crtc_state *old_crtc_state; > struct drm_crtc_commit *commit; > int i; > long ret; > > - for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > - spin_lock(&crtc->commit_lock); > - commit = preceeding_commit(crtc); > - if (commit) > - drm_crtc_commit_get(commit); > - spin_unlock(&crtc->commit_lock); > + for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { > + commit = old_crtc_state->commit; > > if (!commit) > continue; > @@ -1828,8 +1784,6 @@ void drm_atomic_helper_wait_for_dependencies(struct > drm_atomic_state *old_state) if (ret == 0) > DRM_ERROR("[CRTC:%d:%s] flip_done timed out\n", > crtc->base.id, crtc->name); > - > - drm_crtc_commit_put(commit); > } > } > EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies); > @@ -1857,7 +1811,7 @@ void drm_atomic_helper_commit_hw_done(struct > drm_atomic_state *old_state) int i; > > for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > - commit = old_state->crtcs[i].commit; > + commit = new_crtc_state->commit; > if (!commit) > continue; > > @@ -1887,7 +1841,7 @@ void drm_atomic_helper_commit_cleanup_done(struct > drm_atomic_state *old_state) int i; > > for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { > - commit = old_state->crtcs[i].commit; > + commit = new_crtc_state->commit; > if (WARN_ON(!commit)) > continue; > > @@ -2277,20 +2231,13 @@ int drm_atomic_helper_swap_state(struct > drm_atomic_state *state, struct drm_private_state *old_obj_state, > *new_obj_state; > > if (stall) { > - 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); > - if (commit) > - drm_crtc_commit_get(commit); > - spin_unlock(&crtc->commit_lock); > + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { > + commit = old_crtc_state->commit; > > if (!commit) > continue; > > ret = wait_for_completion_interruptible(&commit->hw_done); > - drm_crtc_commit_put(commit); > - > if (ret) > return ret; > } > @@ -2315,13 +2262,13 @@ int drm_atomic_helper_swap_state(struct > drm_atomic_state *state, state->crtcs[i].state = old_crtc_state; > crtc->state = new_crtc_state; > > - if (state->crtcs[i].commit) { > + if (new_crtc_state->commit) { > spin_lock(&crtc->commit_lock); > - list_add(&state->crtcs[i].commit->commit_entry, > + list_add(&new_crtc_state->commit->commit_entry, > &crtc->commit_list); > spin_unlock(&crtc->commit_lock); > > - state->crtcs[i].commit->event = NULL; > + new_crtc_state->commit->event = NULL; > } > } > > @@ -3169,6 +3116,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct > drm_crtc *crtc, state->connectors_changed = false; > state->color_mgmt_changed = false; > state->zpos_changed = false; > + state->commit = NULL; > state->event = NULL; > state->pageflip_flags = 0; > } > @@ -3207,6 +3155,12 @@ > EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state); */ > void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state) > { > + if (state->commit) { > + kfree(state->commit->event); > + state->commit->event = NULL; > + drm_crtc_commit_put(state->commit); > + } > + > drm_property_blob_put(state->mode_blob); > drm_property_blob_put(state->degamma_lut); > drm_property_blob_put(state->ctm); > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 8a5808eb5628..e76d9f95c191 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -144,7 +144,6 @@ struct __drm_planes_state { > struct __drm_crtcs_state { > struct drm_crtc *ptr; > struct drm_crtc_state *state, *old_state, *new_state; > - struct drm_crtc_commit *commit; > s32 __user *out_fence_ptr; > unsigned last_vblank_count; > }; > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 1a642020e306..1a01ff4ea023 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -253,6 +253,15 @@ struct drm_crtc_state { > */ > struct drm_pending_vblank_event *event; > > + /** > + * @commit: > + * > + * This tracks how the commit for this update proceeds through the > + * various phases. This is never cleared, except when we destroy the > + * state, so that subsequent commits can synchronize with previous ones. > + */ > + struct drm_crtc_commit *commit; > + > struct drm_atomic_state *state; > }; > > @@ -808,10 +817,16 @@ struct drm_crtc { > * @commit_list: > * > * List of &drm_crtc_commit structures tracking pending commits. > - * Protected by @commit_lock. This list doesn't hold its own full > - * reference, but burrows it from the ongoing commit. Commit entries > - * must be removed from this list once the commit is fully completed, > - * but before it's correspoding &drm_atomic_state gets destroyed. > + * Protected by @commit_lock. This list holds its own full reference, > + * as does the ongoing commit. > + * > + * "Note that the commit for a state change is also tracked in > + * &drm_crtc_state.commit. For accessing the immediately preceeding > + * commit in an atomic update it is recommended to just use that > + * pointer in the old CRTC state, since accessing that doesn't need > + * any locking or list-walking. @commit_list should only be used to > + * stall for framebuffer cleanup that's signalled through > + * &drm_crtc_commit.cleanup_done." > */ > struct list_head commit_list;
Op 30-08-17 om 15:09 schreef Laurent Pinchart: > Hi Maarten, > > Thank you for the patch. > > On Wednesday, 30 August 2017 15:17:50 EEST Maarten Lankhorst wrote: >> Most code only cares about the current commit or previous commit. >> Fortuantely we already have a place to track those. Move it to >> drm_crtc_state where it belongs. :) >> >> The per-crtc commit_list is kept for places where we have to look >> deeper than the current or previous commit for checking whether to stall >> on unpin. This is used in drm_atomic_helper_setup_commit and >> intel_has_pending_fb_unpin. >> >> Changes since v1: >> - Update kerneldoc for drm_crtc.commit_list. (danvet) >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> --- >> drivers/gpu/drm/drm_atomic.c | 7 --- >> drivers/gpu/drm/drm_atomic_helper.c | 92 +++++++++------------------------- >> include/drm/drm_atomic.h | 1 - >> include/drm/drm_crtc.h | 23 ++++++++-- >> 4 files changed, 42 insertions(+), 81 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c >> index 2fd383d7253a..2cce48f203e0 100644 >> --- a/drivers/gpu/drm/drm_atomic.c >> +++ b/drivers/gpu/drm/drm_atomic.c >> @@ -163,13 +163,6 @@ void drm_atomic_state_default_clear(struct >> drm_atomic_state *state) crtc->funcs->atomic_destroy_state(crtc, >> state->crtcs[i].state); >> >> - if (state->crtcs[i].commit) { >> - kfree(state->crtcs[i].commit->event); >> - state->crtcs[i].commit->event = NULL; >> - drm_crtc_commit_put(state->crtcs[i].commit); >> - } >> - >> - state->crtcs[i].commit = NULL; >> state->crtcs[i].ptr = NULL; >> state->crtcs[i].state = NULL; >> } >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c >> b/drivers/gpu/drm/drm_atomic_helper.c index 11d0e94a2181..8ccb8b6536c0 >> 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c >> @@ -1262,12 +1262,12 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks); >> void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev, >> struct drm_atomic_state *old_state) >> { >> - struct drm_crtc_state *unused; >> + struct drm_crtc_state *new_crtc_state; >> struct drm_crtc *crtc; >> int i; >> >> - for_each_new_crtc_in_state(old_state, crtc, unused, i) { >> - struct drm_crtc_commit *commit = old_state->crtcs[i].commit; >> + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { >> + struct drm_crtc_commit *commit = new_crtc_state->commit; >> int ret; >> >> if (!commit) >> @@ -1388,11 +1388,10 @@ int drm_atomic_helper_async_check(struct drm_device >> *dev, { >> struct drm_crtc *crtc; >> struct drm_crtc_state *crtc_state; >> - struct drm_crtc_commit *commit; >> struct drm_plane *__plane, *plane = NULL; >> struct drm_plane_state *__plane_state, *plane_state = NULL; >> const struct drm_plane_helper_funcs *funcs; >> - int i, j, n_planes = 0; >> + int i, n_planes = 0; >> >> for_each_new_crtc_in_state(state, crtc, crtc_state, i) { >> if (drm_atomic_crtc_needs_modeset(crtc_state)) >> @@ -1420,33 +1419,10 @@ int drm_atomic_helper_async_check(struct drm_device >> *dev, return -EINVAL; >> >> /* >> - * Don't do an async update if there is an outstanding commit modifying >> + * TODO: Don't do an async update if there is an outstanding commit >> modifying >> * the plane. This prevents our async update's changes from getting >> * overridden by a previous synchronous update's state. >> */ > As mentioned in a comment to your previous patch, this is unrelated to > $SUBJECT. You should mention and explain this change in the commit message > (possibly splitting it out in a separate commit if you think that would make > more sense, up to you). I'm all for removing this, it doesn't work.. But I g uess it can be kept in for now and I'll drop this hunk.
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 2fd383d7253a..2cce48f203e0 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -163,13 +163,6 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) crtc->funcs->atomic_destroy_state(crtc, state->crtcs[i].state); - if (state->crtcs[i].commit) { - kfree(state->crtcs[i].commit->event); - state->crtcs[i].commit->event = NULL; - drm_crtc_commit_put(state->crtcs[i].commit); - } - - state->crtcs[i].commit = NULL; state->crtcs[i].ptr = NULL; state->crtcs[i].state = NULL; } diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 11d0e94a2181..8ccb8b6536c0 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1262,12 +1262,12 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks); void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev, struct drm_atomic_state *old_state) { - struct drm_crtc_state *unused; + struct drm_crtc_state *new_crtc_state; struct drm_crtc *crtc; int i; - for_each_new_crtc_in_state(old_state, crtc, unused, i) { - struct drm_crtc_commit *commit = old_state->crtcs[i].commit; + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { + struct drm_crtc_commit *commit = new_crtc_state->commit; int ret; if (!commit) @@ -1388,11 +1388,10 @@ int drm_atomic_helper_async_check(struct drm_device *dev, { struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; - struct drm_crtc_commit *commit; struct drm_plane *__plane, *plane = NULL; struct drm_plane_state *__plane_state, *plane_state = NULL; const struct drm_plane_helper_funcs *funcs; - int i, j, n_planes = 0; + int i, n_planes = 0; for_each_new_crtc_in_state(state, crtc, crtc_state, i) { if (drm_atomic_crtc_needs_modeset(crtc_state)) @@ -1420,33 +1419,10 @@ int drm_atomic_helper_async_check(struct drm_device *dev, return -EINVAL; /* - * Don't do an async update if there is an outstanding commit modifying + * TODO: Don't do an async update if there is an outstanding commit modifying * the plane. This prevents our async update's changes from getting * overridden by a previous synchronous update's state. */ - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { - if (plane->crtc != crtc) - continue; - - spin_lock(&crtc->commit_lock); - commit = list_first_entry_or_null(&crtc->commit_list, - struct drm_crtc_commit, - commit_entry); - if (!commit) { - spin_unlock(&crtc->commit_lock); - continue; - } - spin_unlock(&crtc->commit_lock); - - if (!crtc->state->state) - continue; - - for_each_plane_in_state(crtc->state->state, __plane, - __plane_state, j) { - if (__plane == plane) - return -EINVAL; - } - } return funcs->atomic_async_check(plane, plane_state); } @@ -1731,7 +1707,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, kref_init(&commit->ref); commit->crtc = crtc; - state->crtcs[i].commit = commit; + new_crtc_state->commit = commit; ret = stall_checks(crtc, nonblock); if (ret) @@ -1769,22 +1745,6 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_atomic_helper_setup_commit); - -static struct drm_crtc_commit *preceeding_commit(struct drm_crtc *crtc) -{ - struct drm_crtc_commit *commit; - int i = 0; - - list_for_each_entry(commit, &crtc->commit_list, commit_entry) { - /* skip the first entry, that's the current commit */ - if (i == 1) - return commit; - i++; - } - - return NULL; -} - /** * drm_atomic_helper_wait_for_dependencies - wait for required preceeding commits * @old_state: atomic state object with old state structures @@ -1800,17 +1760,13 @@ static struct drm_crtc_commit *preceeding_commit(struct drm_crtc *crtc) void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state) { struct drm_crtc *crtc; - struct drm_crtc_state *new_crtc_state; + struct drm_crtc_state *old_crtc_state; struct drm_crtc_commit *commit; int i; long ret; - for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { - spin_lock(&crtc->commit_lock); - commit = preceeding_commit(crtc); - if (commit) - drm_crtc_commit_get(commit); - spin_unlock(&crtc->commit_lock); + for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { + commit = old_crtc_state->commit; if (!commit) continue; @@ -1828,8 +1784,6 @@ void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state) if (ret == 0) DRM_ERROR("[CRTC:%d:%s] flip_done timed out\n", crtc->base.id, crtc->name); - - drm_crtc_commit_put(commit); } } EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies); @@ -1857,7 +1811,7 @@ void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state) int i; for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { - commit = old_state->crtcs[i].commit; + commit = new_crtc_state->commit; if (!commit) continue; @@ -1887,7 +1841,7 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *old_state) int i; for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { - commit = old_state->crtcs[i].commit; + commit = new_crtc_state->commit; if (WARN_ON(!commit)) continue; @@ -2277,20 +2231,13 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, struct drm_private_state *old_obj_state, *new_obj_state; if (stall) { - 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); - if (commit) - drm_crtc_commit_get(commit); - spin_unlock(&crtc->commit_lock); + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { + commit = old_crtc_state->commit; if (!commit) continue; ret = wait_for_completion_interruptible(&commit->hw_done); - drm_crtc_commit_put(commit); - if (ret) return ret; } @@ -2315,13 +2262,13 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, state->crtcs[i].state = old_crtc_state; crtc->state = new_crtc_state; - if (state->crtcs[i].commit) { + if (new_crtc_state->commit) { spin_lock(&crtc->commit_lock); - list_add(&state->crtcs[i].commit->commit_entry, + list_add(&new_crtc_state->commit->commit_entry, &crtc->commit_list); spin_unlock(&crtc->commit_lock); - state->crtcs[i].commit->event = NULL; + new_crtc_state->commit->event = NULL; } } @@ -3169,6 +3116,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, state->connectors_changed = false; state->color_mgmt_changed = false; state->zpos_changed = false; + state->commit = NULL; state->event = NULL; state->pageflip_flags = 0; } @@ -3207,6 +3155,12 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state); */ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state) { + if (state->commit) { + kfree(state->commit->event); + state->commit->event = NULL; + drm_crtc_commit_put(state->commit); + } + drm_property_blob_put(state->mode_blob); drm_property_blob_put(state->degamma_lut); drm_property_blob_put(state->ctm); diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 8a5808eb5628..e76d9f95c191 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -144,7 +144,6 @@ struct __drm_planes_state { struct __drm_crtcs_state { struct drm_crtc *ptr; struct drm_crtc_state *state, *old_state, *new_state; - struct drm_crtc_commit *commit; s32 __user *out_fence_ptr; unsigned last_vblank_count; }; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 1a642020e306..1a01ff4ea023 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -253,6 +253,15 @@ struct drm_crtc_state { */ struct drm_pending_vblank_event *event; + /** + * @commit: + * + * This tracks how the commit for this update proceeds through the + * various phases. This is never cleared, except when we destroy the + * state, so that subsequent commits can synchronize with previous ones. + */ + struct drm_crtc_commit *commit; + struct drm_atomic_state *state; }; @@ -808,10 +817,16 @@ struct drm_crtc { * @commit_list: * * List of &drm_crtc_commit structures tracking pending commits. - * Protected by @commit_lock. This list doesn't hold its own full - * reference, but burrows it from the ongoing commit. Commit entries - * must be removed from this list once the commit is fully completed, - * but before it's correspoding &drm_atomic_state gets destroyed. + * Protected by @commit_lock. This list holds its own full reference, + * as does the ongoing commit. + * + * "Note that the commit for a state change is also tracked in + * &drm_crtc_state.commit. For accessing the immediately preceeding + * commit in an atomic update it is recommended to just use that + * pointer in the old CRTC state, since accessing that doesn't need + * any locking or list-walking. @commit_list should only be used to + * stall for framebuffer cleanup that's signalled through + * &drm_crtc_commit.cleanup_done." */ struct list_head commit_list;