[PATCH 10/24] drm/amd/display: Rework CRTC color management

Kazlauskas, Nicholas Nicholas.Kazlauskas at amd.com
Fri Jun 7 16:06:28 UTC 2019

On 6/7/19 11:51 AM, Michel Dänzer wrote:
> On 2019-06-07 2:26 p.m., Kazlauskas, Nicholas wrote:
>> On 6/7/19 3:58 AM, Michel Dänzer wrote:
>>> On 2019-06-06 10:54 p.m., Bhawanpreet Lakha wrote:
>>>> From: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>>> [Why]
>>>> To prepare for the upcoming DRM plane color management properties
>>>> we need to correct a lot of wrong behavior and assumptions made for
>>>> CRTC color management.
>>>> The documentation added by this commit in amdgpu_dm_color explains
>>>> how the HW color pipeline works and its limitations with the DRM
>>>> interface.
>>>> The current implementation does the following wrong:
>>>> - Implicit sRGB DGM when no CRTC DGM is set
>>>> - Implicit sRGB RGM when no CRTC RGM is set
>>>> - No way to specify a non-linear DGM matrix that produces correct output
>>>> - No way to specify a correct RGM when a linear DGM is used
>>>> We had workarounds for passing kms_color tests but not all of the
>>>> behavior we had wrong was covered by these tests (especially when
>>>> it comes to non-linear DGM). Testing both DGM and RGM at the same time
>>>> isn't something kms_color tests well either.
>>>> [How]
>>>> The specifics for how color management works in AMDGPU and the new
>>>> behavior can be found by reading the documentation added to
>>>> amdgpu_dm_color.c from this patch.
>>>> All of the incorrect cases from the old implementation have been
>>>> addressed for the atomic interface, but there still a few TODOs for
>>>> the legacy one.
>>>> Note: this does cause regressions for kms_color at pipe-a-ctm-* over HDMI.
>>>> The result looks correct from visual inspection but the CRC no longer
>>>> matches. For reference, the test was previously doing the following:
>>>> linear degamma -> CTM -> sRGB regamma -> RGB to YUV (709) -> ...
>>>> Now the test is doing:
>>>> linear degamma -> CTM -> linear regamma -> RGB to YUV (709) -> ...
>>>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>>> Reviewed-by: Sun peng Li <Sunpeng.Li at amd.com>
>>>> Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha at amd.com>
>>> Does this address https://bugs.freedesktop.org/110677 ? If so, can you
>>> add a reference to the commit log?
>> It unfortunately does not. There are still some remaining issues with
>> legacy gamma support that I intend to address at some point - which I
>> left in this patch as TODOs.
> Note that the bug reporter is using xf86-video-amdgpu, which no longer
> uses legacy gamma with DC, unless I misunderstand what you mean by that.

While it can use the full LUT, my guess is that we were still getting 
256 entries in DM/DC, which we interpret as legacy at the moment.

FWIW I did actually tried reproducing the issue after applying the patch 
during development and it gave me the same results before and after for 
that specific issue.

Nicholas Kazlauskas

More information about the amd-gfx mailing list