[Intel-gfx] [PATCH v2] drm/i915: Don't forget to mark crtc as inactive after disable

[Patrik Jakobsson]

On Fri, Jul 10, 2015 at 12:22:51PM +0100, Damien Lespiau wrote:
> Hi Patrik,
> 
> Please do Cc the patch author and reviewer when finding a regression,
> they are superb candidates for the review, especially when they are busy
> rewriting the display code.

Hmm, I figured they would be picked up from the commit msg by git-send-email but
maybe that doesn't work on indented lines.

Anyways, CCing them now.

Thanks
Patrik

> 
> On Wed, Jul 08, 2015 at 03:31:52PM +0200, Patrik Jakobsson wrote:
> > Watermark calculations depend on the intel_crtc->active flag to be set
> > properly. Suspend/resume is broken on SKL and we also get DDB mismatches
> > without this patch.
> > 
> > The regression was introduced in:
> > 
> > commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6
> > Author: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Date:   Mon Jun 15 12:33:53 2015 +0200
> > 
> >     drm/i915: Update less state during modeset.
> > 
> >     No need to repeatedly call update_watermarks, or update_fbc.
> >     Down to a single call to update_watermarks in .crtc_enable
> > 
> >     Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> >     Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
> >     Tested-by(IVB): Matt Roper <matthew.d.roper at intel.com>
> >     Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > 
> > v2: Don't touch disable_shared_dpll()
> > 
> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> 
> We do need to update the watermarks in disable() as well, to repartition
> the DDB and update the watermarks based on the new allocation.
> 
> Maarten, Matt, I've no clue where you want the crtc state update, where
> the atomic WM work is at, could you comment?
> 
> Thanks,
> 
> -- 
> Damien
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3c2425f..b4f7a4f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5046,6 +5046,9 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> >  
> >  		ironlake_fdi_pll_disable(intel_crtc);
> >  	}
> > +
> > +	intel_crtc->active = false;
> > +	intel_update_watermarks(crtc);
> >  }
> >  
> >  static void haswell_crtc_disable(struct drm_crtc *crtc)
> > @@ -5091,6 +5094,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> >  	for_each_encoder_on_crtc(dev, crtc, encoder)
> >  		if (encoder->post_disable)
> >  			encoder->post_disable(encoder);
> > +
> > +	intel_crtc->active = false;
> > +	intel_update_watermarks(crtc);
> >  }
> >  
> >  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> > @@ -6155,6 +6161,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> >  
> >  	if (!IS_GEN2(dev))
> >  		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> > +
> > +	intel_crtc->active = false;
> > +	intel_update_watermarks(crtc);
> >  }
> >  
> >  static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx