Re: [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2)

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, May 08, 2015 at 10:56:03AM +0300, Ander Conselvan De Oliveira wrote:
> On Thu, 2015-05-07 at 14:31 -0700, Matt Roper wrote:
> > With the recent modeset internal rework, we wind up setting
> > crtc_state->enable to false, but leave crtc_state->active as true, which
> > is incorrect.  This mismatch gets caught by drm_atomic_crtc_check() and
> > causes subsequent atomic operations (such as plane updates while the
> > CRTC is disabled) to fail.
> > 
> > Bisect points to
> > 
> >         commit dad9a7d6d96630182fb52aae7c3856e9e7285e13
> >         Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >         Date:   Tue Apr 21 17:13:19 2015 +0300
> > 
> >             drm/i915: Use atomic helpers for computing changed flags
> > 
> > as the commit that actually triggers the regression.
> > 
> > v2: Update to alter in-flight state rather than already-committed state
> >     (first version was accidentally based on a midpoint of Ander's
> >     modeset rework series, before his final patches that add proper
> >     state swapping to the legacy modeset path).
> > 
> > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Testcase: igt/kms_universal_plane
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c297cdc..981478a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12340,6 +12340,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
> >  			continue;
> >  
> >  		if (!crtc_state->enable) {
> > +			crtc_state->active = false;
> 
> I believe a more appropriate fix would be to set crtc_state->active when
> we set crtc_state->enable to false in intel_modeset_stage_output_state()
> and the HW state read out. But I think Maarten does that in his later
> patch series, so as a stopgap solution this is fine.
> 
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx