On 2018-06-06 01:03 PM, Michel Dänzer wrote:
On 2018-06-06 06:01 PM, Michel Dänzer wrote:
On 2018-06-01 06:03 PM, [email protected] wrote:
From: "Leo (Sunpeng) Li" <[email protected]>

This ended up being different enough from v2 to warrant a new patchset. Per
Michel's suggestions, there have been various optimizations and cleanups.
Here's what's changed:

* Cache DRM color management property IDs at pre-init,
     * instead of querying DRM each time we need to modify color properties.

* Remove drmmode_update_cm_props().
     * Update color properties in drmmode_output_get_property() instead.
     * This also makes old calls to update_cm_props redundant.

* Get rid of fake CRTCs.
     * Previously, we were allocating a fake CRTC to configure color props on
       outputs that don't have a CRTC.
     * Instead, rr_configure_and_change_cm_property() can be easily modified to
       accept NULL CRTCs.

* Drop patches to persist color properties across DPMS events.
     * Kernel driver should be patched instead:
       https://lists.freedesktop.org/archives/amd-gfx/2018-May/022744.html
     * Color props including legacy gamma now persist across crtc dpms.
     * Non-legacy props now persist across output dpms and hotplug, as long
       as the same CRTC remains attached to that output.

And some smaller improvements:

* Change CTM to be 32-bit format instead of 16-bit.
     * This requires clients to ensure that each 32-bit element is padded to be
       long-sized, since libXrandr parses 32-bit format as long-typed.

* Optimized color management init during CRTC init.
     * Query DRM once for the list of properties, instead of twice.

Sounds good. I'll be going through the patches in detail from now on,
but I don't know yet when I'll be able to finish the review.


Meanwhile, heads up on two issues I discovered while smoke-testing the
series (which are sort of related, but occur even without this series):


Running Xorg in depth 30[0] results in completely wrong colours
(everything has a red tint) with current kernels. I think this is
because DC now preserves the gamma LUT values, but xf86-video-amdgpu
never sets them at depth 30, so the hardware is still using values for
24-bit RGB.

Actually, looks like I made a mistake in my testing before; this issue
only occurs as of patch 6 of this series.


It seems to be caused by legacy gamma being disabled on depth 30. When
that's the case, the gamma_set() hook is set to null. RandR won't
compute the legacy LUT, causing LUT interpolation/composition to spit
out an invalid LUT. I verified this by commenting out the `pScrn->depth
== 30` conditionals guarding the legacy gamma features (in pre_init, and
mode_major).

I'm not certain of the best way to fix this. Would it make sense to
enable legacy gamma on 30bpp if non-legacy is supported? We can do that
since legacy gamma gets interpolated/composed to non-legacy.

However, it doesn't make sense when you look at the LUT size, since
legacy gamma supports only 256 elements (should be 1024 on depth 30?)
In which case we can skip interpolation/composition altogether.

Leo
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to