Bug 108825

Summary: Regression Raven Ridge: Banding on eDP display
Product: DRI Reporter: Samantha McVey <samantham>
Component: DRM/AMDgpuAssignee: Default DRI bug account <dri-devel>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: harry.wentland, nicholas.kazlauskas
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
dmesg
none
xorg log
none
dmesg with drm.debug=0x1
none
0001-drm-amd-display-Fix-unintialized-max_bpc-state-value.patch
none
v2-0001-drm-amd-display-Fix-unintialized-max_bpc-state-va.patch none

Description Samantha McVey 2018-11-21 23:17:06 UTC
Using this test there are 64 distinct bands rather than any smoothness:
http://www.lagom.nl/lcd-test/gradient.php

It looks like this except there are 64 bands whereas the picture shows 32 bands: http://www.lagom.nl/lcd-test/img/gradient-h-32col.png

Computer is Lenovo A485 with Ryzen PRO 2700U
Comment 1 Samantha McVey 2018-11-22 02:28:14 UTC
Created attachment 142561 [details]
dmesg

bpc seems to be detected as 6, but visually there does not appear to be any kind of dithering going on, resulting in bad banding during movies etc in addition to the test I posted in the last message.
Comment 2 Samantha McVey 2018-11-22 02:29:52 UTC
Created attachment 142562 [details]
xorg log
Comment 3 Samantha McVey 2018-11-22 02:31:04 UTC
Created attachment 142563 [details]
dmesg with drm.debug=0x1
Comment 4 Samantha McVey 2018-11-22 03:53:34 UTC
Looks like this is a regression. Dithering works in 4.19.2 but does not work in amd-staging-drm-next.
Comment 5 Samantha McVey 2018-11-28 07:20:59 UTC
I have bisected it to commit: drm/amd/display: Support amdgpu "max bpc" connector property
307638884f721b02b6a54ee8b351c5b4434bd4a6 Author: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Comment 6 Nicholas Kazlauskas 2018-11-28 14:34:43 UTC
(In reply to Samantha McVey from comment #5)
> I have bisected it to commit: drm/amd/display: Support amdgpu "max bpc"
> connector property
> 307638884f721b02b6a54ee8b351c5b4434bd4a6 Author: Nicholas Kazlauskas
> <nicholas.kazlauskas@amd.com>

This isn't a regression from 4.19 but rather a fix for 4.18 where bpc was uncapped from 8 unconditionally. It locked out high refresh modes for high resolution panels with max bpc greater than 8.

The reported 6bpc in your dmesg log comes from DRM itself and not amdgpu. It's what's being reported as being supported by your panel from its EDID. I'm not sure if this is the panel itself reporting itself wrong or if there's a quirk affecting it from the edid qurik list in drm_edid.c.

You can try reverting my patch on amd-staging-drm-next to see if you still get the banding.

Or you could try setting the "max bpc" property to something higher (like 16). You should be able to add this as an "Option" to a conf file in /etc/X11/xorg.conf.d for your Monitor.
Comment 7 Samantha McVey 2018-11-28 19:59:23 UTC
Nicholas,

Reverting that commit (307638884f721b02b6a54ee8b351c5b4434bd4a6) on the top of amd-staging-drm-next resolves the banding issues. So it is that commit that is causing the problem.
Comment 8 Nicholas Kazlauskas 2018-11-28 21:24:32 UTC
Created attachment 142652 [details] [review]
0001-drm-amd-display-Fix-unintialized-max_bpc-state-value.patch

Does this help fix the issue?
Comment 9 Samantha McVey 2018-11-28 21:31:05 UTC
It doesn't compile with that patch:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function ‘dm_crtc_duplicate_state’:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:2991:7: error: ‘struct dm_crtc_state’ has no member named ‘max_bpc’
  state->max_bpc = cur->max_bpc;
       ^~
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:2991:22: error: ‘struct dm_crtc_state’ has no member named ‘max_bpc’
  state->max_bpc = cur->max_bpc;
                      ^~
Comment 10 Nicholas Kazlauskas 2018-11-28 21:33:43 UTC
Created attachment 142653 [details] [review]
v2-0001-drm-amd-display-Fix-unintialized-max_bpc-state-va.patch

I meant to copy the property for the dm connector state, sorry.

Try this patch instead.
Comment 11 Samantha McVey 2018-11-28 21:54:30 UTC
new_state->max_bpc = cur->max_bpc;

cur doesn't exist here (fails to compile with v2 patch). Did you mean state->max_bpc?
Comment 12 Samantha McVey 2018-11-28 23:09:34 UTC
Nicholas,

Your last patch but with `new_state->max_bpc = state->max_bpc;` instead of `new_state->max_bpc = cur->max_bpc;` resolves the issue.
Comment 13 Samantha McVey 2018-12-16 00:14:28 UTC
Going to close this one 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.