| Summary: | [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2" | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | DRI | Reporter: | Jaime Velasco Juan <jsagarribay> | ||||||||||
| Component: | DRM/Radeon | Assignee: | Default DRI bug account <dri-devel> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||
| Severity: | normal | ||||||||||||
| Priority: | medium | ||||||||||||
| Version: | DRI git | ||||||||||||
| Hardware: | x86-64 (AMD64) | ||||||||||||
| OS: | Linux (All) | ||||||||||||
| Whiteboard: | |||||||||||||
| i915 platform: | i915 features: | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 97607 [details]
dmesg of working version
Created attachment 97608 [details]
dmesg of broken version
Looks like the new pll computation is not respecting the RADEON_PLL_USE_FRAC_FB_DIV flag. Older asics shouldn't use fraction fb dividers. (In reply to comment #3) > Looks like the new pll computation is not respecting the > RADEON_PLL_USE_FRAC_FB_DIV flag. Older asics shouldn't use fraction fb > dividers. Nevermind, we always set the RADEON_PLL_USE_FRAC_FB_DIV for LVDS if spread spectrum is enabled. Looks like we aren't respecting RADEON_PLL_USE_REF_DIV properly. Created attachment 97609 [details] [review] Possible fix. Please try the attached patch. We now always use the fixed reference divider if both RADEON_PLL_USE_REF_DIV and RADEON_PLL_USE_FRAC_FB_DIV are set. If only RADEON_PLL_USE_REF_DIV is set then we only use the fixed value as minimum limit. (In reply to comment #6) > Please try the attached patch. > > We now always use the fixed reference divider if both RADEON_PLL_USE_REF_DIV > and RADEON_PLL_USE_FRAC_FB_DIV are set. > > If only RADEON_PLL_USE_REF_DIV is set then we only use the fixed value as > minimum limit. The path fixes the issue. Thanks! (In reply to comment #6) > Please try the attached patch. > > We now always use the fixed reference divider if both RADEON_PLL_USE_REF_DIV > and RADEON_PLL_USE_FRAC_FB_DIV are set. > > If only RADEON_PLL_USE_REF_DIV is set then we only use the fixed value as > minimum limit. Does that make sense? Wouldn't we want to use use the fixed value if RADEON_PLL_USE_REF_DIV regardless of whether RADEON_PLL_USE_FRAC_FB_DIV is set? (In reply to comment #8) > (In reply to comment #6) > > Please try the attached patch. > > > > We now always use the fixed reference divider if both RADEON_PLL_USE_REF_DIV > > and RADEON_PLL_USE_FRAC_FB_DIV are set. > > > > If only RADEON_PLL_USE_REF_DIV is set then we only use the fixed value as > > minimum limit. > > Does that make sense? Wouldn't we want to use use the fixed value if > RADEON_PLL_USE_REF_DIV regardless of whether RADEON_PLL_USE_FRAC_FB_DIV is > set? I don't really know, it's just the way how the old code handled this. If RADEON_PLL_USE_REF_DIV was set the RADEON_PLL_USE_FRAC_FB_DIV branch just used the value as provided, but the none RADEON_PLL_USE_FRAC_FB_DIV branch still tried to increment the vale. Marking as fixed. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Created attachment 97606 [details] Output of 'xrandr --prop' Hello, I'm testing drm-next and have found this problem. The display shows some kind of moving wavy pattern, noticeable in midtones and grandients. I've bisected it to the mentioned commit. The display is the internal LVDS panel, I haven't tested external displays. $ git bisect log git bisect start # good: [aa3a38dd40b610de206fb04777cc5469ce057de8] drm/radeon: update CI DPM powertune settings git bisect good aa3a38dd40b610de206fb04777cc5469ce057de8 # bad: [bc00aa0ebfed7edc1ca91ec2d9b0d5b04c5e06a8] drm: bochs: drop unused struct fields git bisect bad bc00aa0ebfed7edc1ca91ec2d9b0d5b04c5e06a8 # bad: [8bf5dda3777f2a477e3c448b38e18f5e1b502eae] drm/radeon/si: make sure mc ucode is loaded before checking the size git bisect bad 8bf5dda3777f2a477e3c448b38e18f5e1b502eae # good: [8a6a70662fc93c2c43ba48de3e46bf7e860e18cc] drm/radeon: re-enable mclk dpm on R7 260X asics git bisect good 8a6a70662fc93c2c43ba48de3e46bf7e860e18cc # good: [aeb0c2d8d3fc60ea931f807275166ea8abe1feeb] drm/radeon: memory leak on bo reservation failure. v2 git bisect good aeb0c2d8d3fc60ea931f807275166ea8abe1feeb # bad: [9b17c2afd74b5f56497ffacbcac85ee2cf0a1c1b] drm/radeon: improve PLL params if we don't match exactly v2 git bisect bad 9b17c2afd74b5f56497ffacbcac85ee2cf0a1c1b # first bad commit: [9b17c2afd74b5f56497ffacbcac85ee2cf0a1c1b] drm/radeon: improve PLL params if we don't match exactly v2 (the commit ID are local, as I've rebased drm-next and drm-fixes on v3.14). I attach two kernel logs booting and doing: echo 0xe >/sys/module/drm/parameters/debug xrand --rate 50 xrand --rate 60 I think the relevant difference is (first line is good, next is buggy): - [drm:radeon_compute_pll_avivo] 122000 - 13725, pll dividers - fb: 122.0 ref: 6, post 4 + [drm:radeon_compute_pll_avivo] 122000 - 12198, pll dividers - fb: 126.5 ref: 7, post 4