Bug 28932 - new avivo PLL calculation in Radeon driver fails with certain modelines
Summary: new avivo PLL calculation in Radeon driver fails with certain modelines
Alias: None
Product: Drivers
Classification: Unclassified
Component: Video(DRI - non Intel) (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_video-dri
Depends on:
Blocks: 27352
  Show dependency tree
Reported: 2011-02-12 07:49 UTC by Chris Kennedy
Modified: 2011-02-21 08:42 UTC (History)
3 users (show)

See Also:
Kernel Version: 2.6.38-rc4
Tree: Mainline
Regression: Yes

possible fix (1.41 KB, patch)
2011-02-12 08:51 UTC, Alex Deucher
Details | Diff
alternate patch (1.67 KB, patch)
2011-02-13 23:26 UTC, Alex Deucher
Details | Diff

Description Chris Kennedy 2011-02-12 07:49:08 UTC
This patch https://bugs.freedesktop.org/attachment.cgi?id=41943 fails to calculate the PLL values for certain lower dotclock modelines.  I can change the code to use the legacy PLL calculation and things work again.  

Here's an example modeline that fails now, used to work flawless...

# mario 256x224@59.19 15.2699Khz
	ModeLine          "256x224x59.19" 5.130681 256 272 296 336 224 233 236 258 -HSync -VSync

This modeline now with the newer avivo PLL calculation ends up being 133Hz instead of 59.19 and the Horizontal frequency is 39khz or so.  It's basically completely wrong, way off of what it should be.

Things work fine for anything that seems to have a dotclock above 12 Mhz or so, from what I suspect the newer method isn't taking into account possible lower dotclock and Horizontal frequency modelines.  

Comment 1 Alex Deucher 2011-02-12 08:51:26 UTC
Created attachment 47492 [details]
possible fix

This patch should fix it.  The post divider was overflowing.
Comment 2 Chris Kennedy 2011-02-12 16:53:50 UTC
This fixes the issue, now these lower dotclock modelines work again.

Comment 3 Alex Deucher 2011-02-13 23:26:28 UTC
Created attachment 47672 [details]
alternate patch

Please try this patch instead of the previous one and let me know which works better or if they both work ok.
Comment 4 Chris Kennedy 2011-02-14 07:31:36 UTC
This works works good too, basically same as the last one, can't tell any difference.
Comment 5 Rafael J. Wysocki 2011-02-14 23:45:22 UTC
Handled-By : Alex Deucher <alexdeucher@gmail.com>
Patch : https://bugzilla.kernel.org/attachment.cgi?id=47672
Comment 6 Chris Kennedy 2011-02-18 13:36:01 UTC
I think there's an issue with some legacy chips plus low dotclocks and that same patch, oddly the only little part I can see which could do this is...

@@ -849,7 +951,7 @@ void radeon_compute_pll(struct radeon_pll *pll,
 		max_fractional_feed_div = pll->max_frac_feedback_div;
-	for (post_div = max_post_div; post_div >= min_post_div; --post_div) {
+	for (post_div = min_post_div; post_div <= max_post_div; ++post_div) {
 		uint32_t ref_div;
 		if ((pll->flags & RADEON_PLL_NO_ODD_POST_DIV) && (post_div & 1))

Although it's basically looking like that must be the issue.  A person has a ATI Radeon 9200SE 5964 (AGP)and with 2.6.38-rc4-git4+ it works while with 2.6.38-rc5 there are certain (not all) modelines with low dotclocks that end up being totally out of sync.

Here's an example of a non-working modeline for him with the newer kernel, vs. the older one step back before the patch with the above change...

# toki 256x224@59.61 15.6774Khz
	ModeLine          "256x224x59.61" 5.518455 256 272 304 352 224 235 238 263 -HSync -VSync

There's a lot of other ones thought that can work with the same lower dotclocks, so I'm not sure why that is, but I can have him test with that change in the legacy pll computation reversed.   Yet from what I can tell, that is the only code his Radeon should be hitting that changed in the patch.

Comment 7 Florian Mickler 2011-02-20 00:03:57 UTC
merged in .38-rc5: 
commit a4b40d5d97f5c9ad0b7f4bf2818291ca184bb433
Author: Alex Deucher <alexdeucher@gmail.com>
Date:   Mon Feb 14 11:43:10 2011 -0500

    drm/radeon/kms: add bounds checking to avivo pll algo
Comment 8 Alex Deucher 2011-02-21 04:51:56 UTC
Comment 6 is not relevant to this bug and has been opened as a separate issue: bug 29502.
Comment 9 Florian Mickler 2011-02-21 08:42:00 UTC
Thx, closing.

Note You need to log in before you can comment on or make changes to this bug.