Hi Leo, On 2018-06-01 06:03 PM, sunpeng.li at amd.com wrote: > From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com> > > 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. Can you look into making xf86-video-amdgpu set the LUT values at depth 30 as well? Ideally in a way which doesn't require all patches in this series, so that it could be backported for a 18.0.2 point release if necessary. (Similarly for skipping drmmode_cursor_gamma when the kernel supports the new colour management properties, to fix https://bugs.freedesktop.org/106578) Trying to run Xorg in depth 16 or 8[0] results in failure to set any mode: [ 56.138] (EE) AMDGPU(0): failed to set mode: Invalid argument [ 56.138] (WW) AMDGPU(0): Failed to set mode on CRTC 0 [ 56.172] (EE) AMDGPU(0): Failed to enable any CRTC Works fine with amdgpu.dc=0. This has been broken at least since the 4.16 development cycle. [0] You can change Xorg's colour depth either via -depth on its command line, or via the xorg.conf screen section: Section "Screen" Identifier "Screen0" DefaultDepth 30 # or 16 or 8 EndSection -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
