[Intel-gfx] [PATCH 2/2] drm/i915: Use crtc_state->active in primary check_plane func

[Daniel Vetter]

On Wed, Jul 08, 2015 at 12:24:07PM +0200, Maarten Lankhorst wrote:
> Op 07-07-15 om 11:15 schreef Daniel Vetter:
> > Since
> >
> > commit 8c7b5ccb729870e606321b3703e2c2e698c49a95
> > Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
> > Date:   Tue Apr 21 17:13:19 2015 +0300
> >
> >     drm/i915: Use atomic helpers for computing changed flags
> >
> > we compute the plane state for a modeset before actually committing
> > any changes, which means crtc->active won't be correct yet. Looking at
> > future work in the modeset conversion targetting 4.3 the only places
> > where crtc_state->active isn't accurate is when disabling other CRTCs
> > than the one the modeset is for (when stealing connectors). Which
> > isn't the case here. And that's also confirmed by an audit, we do
> > unconditionally update crtc_state->active for the current pipe.
> >
> > We also don't need to update any other plane check functions since we
> > only ever add the primary state to the modeset update right now.
> >
> > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Cc: Jani Nikula <jani.nikula at linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 647b1404c441..ba9321998a41 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13276,7 +13276,7 @@ intel_check_primary_plane(struct drm_plane *plane,
> >  	if (ret)
> >  		return ret;
> >  
> > -	if (intel_crtc->active) {
> > +	if (crtc_state->base.active) {
> >  		struct intel_plane_state *old_state =
> >  			to_intel_plane_state(plane->state);
> >  
> I think this was probably a feature, not a bug. Since full atomic planes won't be part of v4.2
> both patches look ok to me.

I wasn't sure - in dinq we didn't convert his as part of the patches which
switched to check state->active in check functions, but only later on was
removed in some seemingly unrelated refactor. Not sure why this didn't
blow up more ...

> Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>

Both applied, thanks for review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch