On Thu, Jun 11, 2015 at 06:23:09AM +0200, Maarten Lankhorst wrote: > Op 11-06-15 om 03:35 schreef Matt Roper: > > On Thu, Jun 04, 2015 at 02:47:44PM +0200, Maarten Lankhorst wrote: > >> By passing crtc_state to the check_plane functions a lot of duplicated > >> code can be removed. And now that the transitional helpers are gone the > >> crtc_state can be reliably obtained. > >> > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > >> --- > >> drivers/gpu/drm/i915/intel_atomic_plane.c | 4 ++- > >> drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++--------------------- > >> drivers/gpu/drm/i915/intel_drv.h | 1 + > >> drivers/gpu/drm/i915/intel_sprite.c | 13 +++------ > >> 4 files changed, 23 insertions(+), 43 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > >> index aa2128369a0a..4d8cacbca777 100644 > >> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > >> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > >> @@ -119,6 +119,8 @@ static int intel_plane_atomic_check(struct drm_plane *plane, > >> crtc = crtc ? crtc : plane->crtc; > >> intel_crtc = to_intel_crtc(crtc); > >> > >> + intel_state->visible = false; > >> + > > What do we need this change for? Primary and cursor check functions > > immediately overwrite state->visible, so setting this here has no > > effect. The sprite case where fb==NULL is the only case where this > > would matter, but moving the assignment from the sprite check function > > to here doesn't seem like it gains us anything. > > > I was using it to clear intel_state->visible even if no crtc is set. In that case state->visible > should already be false, but a bit of paranoia never hunts. :-) Resetting of state should be done in the duplicate functions. This makes sure we never ever forget to compute it correctly ;-) Sprinkling clearing code all over (and especially over the compute code) is imo fragile and should be avoided if possible. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx