Re: [PATCH 02/19] drm/i915: Call intel_update_watermarks() in specific place during modeset

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Aug 30, 2013 at 11:49:52PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 30, 2013 at 05:26:29PM -0300, Paulo Zanoni wrote:
> > 2013/8/30  <ville.syrjala@linux.intel.com>:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Make the call to intel_update_watermarks() just once or twice during
> > > modeset. Ideally it should happen independently when each plane gets
> > > enabled/disabled, but for now it seems better to keep it in central
> > > place. We can improve things when we get all the planes sorted out
> > > in a better way.
> > >
> > > When enabling set up the watermarks just before the pipe is enabled.
> > > And when disabling we need to wait until we've marked the crtc as
> > > inactive.
> > 
> > Why do we need to wait until we've marked the CRTC as inactive?
> > (Daniel/Ville should put the answer in the commit message)
> 
> Because the watermark compute code looks at intel_crtc->active. If we
> compute the watermarks before, the code thinks the pipe is active.
> 
> Hmm. BTW now that I look at intel_crtc_active() I start to wonder why it
> looks at the clock in the user specified mode. In fact most (maybe all?)
> of the pre-hsw watermark code is fscked up and it looks at the wrong
> mode. Sigh. Suppose I need to make a quick for all that as well...

That was a fallout from the partial modeset state recovery on boot-up -
we've ended up doing divide-by-zeros since the pipe was marked as active
but we've lacked the dotclock required to compute the new watermarks. So
for those we've just skipped this.

Since hsw still has no dotclock readout support we need to keep onto this
hack for a while longer. But adding an XXX comment here would be useful.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch