[Intel-gfx] [PATCH 13/13] drm/i915/skl: Prefer even dividers for SKL DPLLs

[Damien Lespiau]

On Wed, May 27, 2015 at 07:08:38PM -0300, Paulo Zanoni wrote:
> 2015-05-27 18:39 GMT-03:00 Paulo Zanoni <przanoni at gmail.com>:
> > 2015-05-07 14:38 GMT-03:00 Damien Lespiau <damien.lespiau at intel.com>:
> >> Currently, if an odd divider improves the deviation (minimizes it), we
> >> take that divider. The recommendation is to prefer even dividers.
> >
> > The doc says "It is preferred to get as close to the DCO central
> > frequency as possible, but using an even divider value takes
> > precedence.", but I'm wondering if they meant "prefer even over odd in
> > case they have the same deviation" or just "even divider is preferred
> > as long as it's on the deviation threshold, even if there's an odd
> > divider with minimal/no deviation". I see you implement the last
> > option - if you don't count the possible bug mentioned on my review of
> > patch 12.
> >
> > Assuming the loop order will be fixed on patch 12, and assuming you
> > are correctly interpreting the spec, then your patch does what it
> > says, so: Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>.
> 
> I kept looking at the spec, and the section that describes the 4 steps
> for the algorithm totally clarifies what's the correct interpretation.
> Please see step 4. An "even" divider with any acceptable deviation is
> preferred over any possible "odd" divider, it doesn't matter what is
> the best deviation of the "odd" dividers.

Right, too bad I read that last, but that's indeed what I think and well
and answered a previous comment with that already.

> Considering this, I think we really should invert the loops as I
> suggested on patch 12, and then we should modify this patch so that it
> breaks the loop only after we've also iterated over all DCOs. I guess
> these changes are a requirement for the R-B tags on patches 12 and 13
> as long as you don't have counter arguments.

Yup, agreed.

> We should still consider breaking the loop earlier in case we reach 0
> deviation, and we should still consider comparing the pdeviation with
> the ndeviation before picking central_freq, dco_freq and divider.
> These things are not requirements for the R-B tags, but, as I said,
> i'd like to see your opinion.

With v2, the break on deviation 0 would just be a "fast path" because we
can't improve on that. Still worth doing as a follow up (I like having
real diffs rather than v2/v3 with all the changes squashed).

-- 
Damien