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
