[Intel-gfx] [PATCH 2/9] drm/i915: fix linetime_watermarks code

[Chris Wilson]

On Mon, May 06, 2013 at 10:13:33AM -0300, Paulo Zanoni wrote:
> 2013/5/5 Chris Wilson <chris at chris-wilson.co.uk>:
> > On Fri, May 03, 2013 at 05:23:38PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >>
> >> The spec says the linetime watermarks must be programmed before
> >> enabling any display low power watermarks, but we're currently
> >> updating the linetime watermarks after we call intel_update_watermarks
> >> (and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
> >> best way guarantee the linetime watermarks will be updated before the
> >> low power watermarks is inside the update_wm function, because it's
> >> the function that enables low power watermarks. And since Haswell is
> >> the only platform that has linetime watermarks, let's completely kill
> >> the "intel_update_linetime_watermarks" abstraction and just use the
> >> intel_update_watermarks abstraction by creating haswell_update_wm.
> >
> >> +static void haswell_update_wm(struct drm_device *dev)
> >> +{
> >> +     struct drm_i915_private *dev_priv = dev->dev_private;
> >> +     struct drm_crtc *crtc;
> >> +     enum pipe pipe;
> >> +
> >> +     for_each_pipe(pipe) {
> >> +             crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> >> +             haswell_update_linetime_wm(dev, crtc);
> >> +     }
> >
> > The LP watermarks are still enabled at this point.
> 
> Did you mean "they're not disabled yet"? Well, at least now we're
> programming the linetime registers _before_ we update the LP
> registers, and every time. If we want to be 100% spec-compliant
> without any intermediate steps then I guess we can discard this
> linetime series and merge it all inside the single patch that will
> fully implement haswell_update_wm. But it will be one single big
> patch. Just tell me which one you prefer.

Or you can just disable the 3 LP wm first before updating the linetime
wm.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre