Ping :) Leo On 2018-05-03 02:31 PM, sunpeng.li at amd.com wrote: > From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com> > > > This patchset ended up looking quite different from the first. To address some > fundamental issues, the design had to be reworked. > > Things gathered from previous review: > > 1. User client should not have to handle DRM blob objects. That should be the > job of the DDX driver. > > 2. Since legacy gamma sets the same properties within DRM as non-legacy gamma, > the previous implementation created conflicts when setting both. > > * We should at least support this use-case: Nightlight enabled (uses legacy > gamma), with monitor correction enabled (non-legacy gamma) > > 3. Since color management properties are attached to the CRTC, the previous > revision has the properties hooked onto the CRTC life-cycle, not the output > life-cycle. This is problematic, since clients expect properties on an > output to stay consistent throughout its life. > > 4. Although not mentioned during review, the color properties did not persist > across certain events, such as DPMS state changes or hotplugs. > > > To address the above, the following was done: > > 1. XRandR allows setting of array-like properties. This is used to directly > pass LUT/CTM data from the client to the DDX driver. > > 2. Legacy and non-legacy gamma LUTs are now merged via composition. This will > allow both to be in effect, while only programming one LUT in kernel driver. > > 3. The three color management properties (Degamma LUT, Color Transform Matrix > (CTM), and Gamma LUT) are hard-coded into the DDX driver, to be listed (as > disabled) regardless of whether a CRTC is attached on the output, or whether > the kernel driver supports it. > > * If kernel driver does not support color management, the properties will > remain disabled. A `xrandr --set` will then error. > > 4. Color properties are now *staged* inside the driver-private CRTC object. > This allows us to *push* the properties into kernel DRM (and consequently > into hardware) whenever there is a need. > > * Along with staging and pushing, an *update* function is used to notify > RandR to update the color properties listed on outputs. This can be used > when `xrandr --auto` enables a CRTC on an output, and the output need to > reflect the CRTC's color properties. > > > However, there are some things being done that aren't quite nice, to which I > don't yet have a solution. Any thoughts and suggestions are welcome: > > * When using libXrandr to set the CTM property, 16bit format is used. Ideally > we should use 64bit format, since the CTM consists of 9x64bit fixed-point > values. However, it isn't recognized by XRRChangeOutputProperty as a valid > format. 32 bit could work, since the CTM values are S31.32 fixed-point. > However, using this format corrupts the data once it gets to xserver. On > first glance, it may be the cast to long within XRRChangeOutputProperty, in > addition to compiling 64 bit (shouldn't it use int32_t instead?). > > * Since LUTs can be quite long, the output of `xrandr --prop` isn't very nice. > > * Setting these properties through the xrandr app isn't supported without > modifications to the app, due to the length of these properties. However, > setting through libXrandr works. > > Support on the xrandr app side can come as a seperate patch set. For now, > testing the new API can be done via libXrandr. I've made a sample application > here: git://people.freedesktop.org/~hwentland/color-demo-app > Clone, make, and it's ready to go. > > > Leo (Sunpeng) Li (13): > Add color management properties to driver-private CRTC object > Push color properties to kernel DRM on CRTC init > List disabled color properties on RandR outputs without a CRTC > Use CRTC's color properties if output has a CRTC attached > Enable setting of color properties though RandR > Compose non-legacy with legacy gamma LUT before pushing to kernel DRM > Also compose LUT when setting legacy gamma. > Set driver-private CRTC's dpms mode on disable > Move drmmode_do_crtc_dpms > Push staged color properties when DPMS state toggles On > Push staged color properties on output detect > Update color properties on modeset major > Refactor pushing color management properties into a function > > src/drmmode_display.c | 878 ++++++++++++++++++++++++++++++++++++++++++++++---- > src/drmmode_display.h | 8 + > 2 files changed, 824 insertions(+), 62 deletions(-) >
