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@xxxxxxx> >>>> >>>> [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@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@xxxxxxx> >>>> Reviewed-by: Sun peng Li <Sunpeng.Li@xxxxxxx> >>>> Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@xxxxxxx> >>> >>> 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 _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
