Re: [PATCH 03/13] drm/i915: Split watermark level computation from the code

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Aug 06, 2013 at 09:56:31PM +0100, Chris Wilson wrote:
> On Tue, Aug 06, 2013 at 10:24:02PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Refactor the watermarks computation for one level to a separate
> > function. This function will now set the ->enable flat to true,
> 
> s/flat/flag/
> 
> Though I did think you meant a flatten value at one point.
> 
> > even if the watermark level wasn't actually checked yet. In the
> > future we will delay the checking so we must consider all unchecked
> > watermarks as possibly valid.
> > 
> > v2: Preserve comment about latency units
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I don't have the context here to confirm passing level to
> ilk_compute_pri_wm is exactly what you want, and it seems that level is
> implicitly greater than 0 in the original code.
> 
> The transformation looks fine by itself. If you can calm my quirms over
> the value of level (in the changelog is fine),

Looks like Daniel already took this one. But yeah, I guess we should really
pass 'level > 0' to make it clearer that it's a boolean. If you recall I
had a patch in the original series to pass level all the way down, but I
dropped it since it didn't actually do any good.

As far as level=0, I didn't convert the hsw_compute_wm_pipe() to use the
new function. I could do that as a followup patch.

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC