Re: [PATCH 2/9] drm/i915: fix linetime_watermarks code

[Paulo Zanoni <przanoni@gmail.com>]

2013/5/5 Chris Wilson <chris@chris-wilson.co.uk>:
> On Fri, May 03, 2013 at 05:23:38PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@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.

>> +
>> +     sandybridge_update_wm(dev);
>> +}
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre



-- 
Paulo Zanoni