[Intel-gfx] [PATCH] drm/i915: Compute WR PLL dividers dynamically

[Daniel Vetter]

On Wed, May 08, 2013 at 01:07:23PM -0300, Paulo Zanoni wrote:
> Hi
> 
> 2013/5/8 Damien Lespiau <damien.lespiau at intel.com>:
> > Up to now, we were using a static table to match the clock frequency
> > with a (r2,n2,p) triplet. Despite this table being big, it's by no mean
> > comprehensive and we had to fall back to the closest frequency when the
> > requested TMDS clock wasn't in the table.
> >
> > This patch computes (r2,n2,p) dynamically and get rid of The Big Table.
> >
> 
> There are 2 minor optional bikesheds below. I "reviewed" your patch,
> it looks correct, but I have to admit I didn't really check the math
> too hard (and there's no definitive documentation we can check your
> math against...).
> 
> So the real work I did was testing. I discovered I don't have any
> monitors with modes that are not on the big wrpll_tmds_clock_table, so
> I couldn't test that case. In one of my monitors I tested all the
> modes and they all work. In another monitor I tested some modes, they
> work, and I also checked whether the p/n/r values match the table and
> they do match. I also checked on the monitor's menu if it thinks its
> frequency is the correct one. All looks correct.
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Tested-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

Queued for -next, thanks for the patch.
 
> I also looked briefly to your i-g-t patches. They look fine, but my
> concern is that the code inside the Kernel will get out-of-sync with
> the code in i-g-t, so we won't really be able to catch regressions.
> OTOH, I do have an idea for a different i-g-t test: you read our
> registers (WRPLL_1, WRPLL_2, Link M) in order to check which
> frequency/r/n/p we're using inside the Kernel, then you look at the
> old wrpll_tmds_clock_table and check if the values used on the Kernel
> match the values on the table. Maybe this should be incorporated at
> testdisplay. Maybe my idea is just a bad idea and should be ignored.
> Feel free to do whatever you prefer for the i-g-t patches.

Yeah, I'm not too sure about what we should do with the i-g-t. Commit it
as-is certainly won't hurt, but I'm not sure whether it can provide
additional value now that the conversion is done.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch