[Bug 77673] New: [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[Bug 77673] New: [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 2602 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

          Priority: medium
            Bug ID: 77673
          Assignee: dri-devel@lists.freedesktop.org
           Summary: [bisected] moving moire starting with "drm/radeon:
                    improve PLL params if we don't match exactly v2"
          Severity: normal
    Classification: Unclassified
                OS: Linux (All)
          Reporter: jsagarribay@gmail.com
          Hardware: x86-64 (AMD64)
            Status: NEW
           Version: DRI CVS
         Component: DRM/Radeon
           Product: DRI

Created attachment 97606
  --> https://bugs.freedesktop.org/attachment.cgi?id=97606&action=edit
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

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 4101 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 316 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

--- Comment #1 from Jaime Velasco Juan <jsagarribay@gmail.com> ---
Created attachment 97607
  --> https://bugs.freedesktop.org/attachment.cgi?id=97607&action=edit
dmesg of working version

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1356 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 315 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

--- Comment #2 from Jaime Velasco Juan <jsagarribay@gmail.com> ---
Created attachment 97608
  --> https://bugs.freedesktop.org/attachment.cgi?id=97608&action=edit
dmesg of broken version

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1353 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 322 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

--- Comment #3 from Alex Deucher <agd5f@yahoo.com> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1211 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 516 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

--- Comment #4 from Alex Deucher <agd5f@yahoo.com> ---
(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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1481 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 315 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

--- Comment #5 from Christian König <deathsimple@vodafone.de> ---
Created attachment 97609
  --> https://bugs.freedesktop.org/attachment.cgi?id=97609&action=edit
Possible fix.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1436 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 448 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

--- Comment #6 from Christian König <deathsimple@vodafone.de> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1369 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 508 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

--- Comment #7 from Jaime Velasco Juan <jsagarribay@gmail.com> ---
(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!

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1497 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 612 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

--- Comment #8 from Alex Deucher <agd5f@yahoo.com> ---
(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?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1589 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 958 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

--- Comment #9 from Christian König <deathsimple@vodafone.de> ---
(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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 2042 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 77673] [bisected] moving moire starting with "drm/radeon: improve PLL params if we don't match exactly v2"

[bugzilla-daemon]

[-- Attachment #1.1: Type: text/plain, Size: 528 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=77673

Christian König <deathsimple@vodafone.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Christian König <deathsimple@vodafone.de> ---
Marking as fixed.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 2227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel