On Wed, May 08, 2013 at 09:13:54PM +0200, Daniel Vetter wrote: > On Wed, May 08, 2013 at 01:07:23PM -0300, Paulo Zanoni wrote: > > Hi > > > > 2013/5/8 Damien Lespiau <damien.lesp...@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.zan...@intel.com> > > Tested-by: Paulo Zanoni <paulo.r.zan...@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.
Oops, blows up on 32bit machines. I guess the igt will be useful once more to check that the 32bit version is solid, too. Dropped from dinq for now. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
