Op 15-06-15 om 14:05 schreef Daniel Vetter: > 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 Good point! ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
