Op 17-01-17 om 02:27 schreef Laurent Pinchart: > Hi Maarten, > > One more thing. > > On Monday 16 Jan 2017 10:37:43 Maarten Lankhorst wrote: >> This is a straightforward conversion that converts all the users of >> get_existing_state in atomic core to use get_old_state or get_new_state >> >> Changes since v1: >> - Fix using the wrong state in >> drm_atomic_helper_update_legacy_modeset_state. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/drm_atomic.c | 6 +++--- >> drivers/gpu/drm/drm_atomic_helper.c | 39 ++++++++++++++-------------------- >> drivers/gpu/drm/drm_blend.c | 3 +-- >> 3 files changed, 22 insertions(+), 26 deletions(-) > [snip] > >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c >> b/drivers/gpu/drm/drm_atomic_helper.c index b26cf786ce12..1de8d5fbc8d3 >> 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c > [snip] > >> @@ -698,8 +695,7 @@ disable_outputs(struct drm_device *dev, struct >> drm_atomic_state *old_state) if (!old_conn_state->crtc) >> continue; >> >> - old_crtc_state = drm_atomic_get_existing_crtc_state(old_state, >> - > old_conn_state->crtc); >> + old_crtc_state = drm_atomic_get_new_crtc_state(old_state, >> old_conn_state->crtc); > This looks wrong. I believe you should call drm_atomic_get_old_crtc_state() > here. If I'm not mistaken drm_atomic_get_existing_crtc_state() did the right > thing as old_state->crtcs[*].state was set to the old state by the state swap > operation. This is wrong, even. Fixed in next version. At least looking at the code made it obvious. :) > On the other hand, drm_atomic_helper_update_legacy_modeset_state() uses > drm_atomic_get_new_plane_state() the same way you do here, even though it > operates after state swap, and your changelog specifically mentions that > you've fixed that in v2. It's getting too late to properly wrap my head around > this, I'll let you check which option is correct (but I reserve the right to > challenge it if your explanation isn't convincing enough ;-)). update_legacy_modeset_state was looking at primary->state (new state) before, but was using get_existing_state to check whether it's part of the commit, so it's valid to look at plane->state. This was fixed to get the new state, so it makes sense there. >> if (!old_crtc_state->active || >> !drm_atomic_crtc_needs_modeset(old_conn_state->crtc- >> state)) _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
