Op 01-11-16 om 14:41 schreef Ville Syrjälä: > On Tue, Nov 01, 2016 at 02:34:00PM +0100, Maarten Lankhorst wrote: >> Op 01-11-16 om 14:09 schreef Ville Syrjälä: >>> On Mon, Oct 17, 2016 at 02:37:00PM +0200, Maarten Lankhorst wrote: >>>> Add for_each_(old)(new)_(plane,connector,crtc)_in_state iterators to >>>> replace the old for_each_xxx_in_state ones. This is useful for >1 flip >>>> depth and getting rid of all xxx->state dereferences. >>>> >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >>>> --- >>>> drivers/gpu/drm/drm_atomic.c | 6 +++ >>>> drivers/gpu/drm/drm_atomic_helper.c | 52 +++++++++++++++++++++-- >>>> drivers/gpu/drm/i915/intel_display.c | 11 ++--- >>>> include/drm/drm_atomic.h | 81 ++++++++++++++++++++++++++++++++++-- >>>> include/drm/drm_atomic_helper.h | 3 ++ >>>> 5 files changed, 142 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c >>>> index 5dd70540219c..120775fcfb70 100644 >>>> --- a/drivers/gpu/drm/drm_atomic.c >>>> +++ b/drivers/gpu/drm/drm_atomic.c >>>> @@ -278,6 +278,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state, >>>> return ERR_PTR(-ENOMEM); >>>> >>>> state->crtcs[index].state = crtc_state; >>>> + state->crtcs[index].old_state = crtc->state; >>>> + state->crtcs[index].new_state = crtc_state; >>>> state->crtcs[index].ptr = crtc; >>>> crtc_state->state = state; >>>> >>>> @@ -640,6 +642,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state, >>>> >>>> state->planes[index].state = plane_state; >>>> state->planes[index].ptr = plane; >>>> + state->planes[index].old_state = plane->state; >>>> + state->planes[index].new_state = plane_state; >>>> plane_state->state = state; >>>> >>>> DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n", >>>> @@ -931,6 +935,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, >>>> >>>> drm_connector_reference(connector); >>>> state->connectors[index].state = connector_state; >>>> + state->connectors[index].old_state = connector->state; >>>> + state->connectors[index].new_state = connector_state; >>>> state->connectors[index].ptr = connector; >>>> connector_state->state = state; >>>> >>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c >>>> index 07b432f43b98..fcb6e5b55217 100644 >>>> --- a/drivers/gpu/drm/drm_atomic_helper.c >>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c >>>> @@ -2495,7 +2495,7 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_all); >>>> * >>>> * See also: >>>> * drm_atomic_helper_duplicate_state(), drm_atomic_helper_disable_all(), >>>> - * drm_atomic_helper_resume() >>>> + * drm_atomic_helper_resume(), drm_atomic_helper_commit_duplicated_state() >>>> */ >>>> struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev) >>>> { >>>> @@ -2536,6 +2536,52 @@ unlock: >>>> EXPORT_SYMBOL(drm_atomic_helper_suspend); >>>> >>>> /** >>>> + * drm_atomic_helper_commit_duplicated_state - commit duplicated state >>>> + * @state: duplicated atomic state to commit >>>> + * @ctx: pointer to acquire_ctx to use for commit. >>>> + * @nonblock: commit the state non-blocking. >>>> + * >>>> + * The state returned by drm_atomic_helper_duplicate_state() and >>>> + * drm_atomic_helper_suspend() is partially invalid, and needs to >>>> + * be fixed up before commit. >>> So the old_state pointers are potentially invalid because whatever->state >>> may have changed since we duplicated the state? >> Yes when you use drm_atomic_helper_duplicate_state, and commit a different state before committing the duplicated state. >> This only happens during suspend/resume and gpu reset. > Should we maybe have something like drm_atomic_refresh_old_state(state)? > Would allow the caller then to check something in the old vs. new state > before committing? iirc that was the first approach I took, but then you shouldn't do anything with a duplicated state after creating a except commit it, so creating a commit_duplicated_state should be enough. Can you think of any case where you do the following? Duplicate state commit different state Look at duplicated state, tinker with it, commit it. ~Maarten _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel