AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: <sunpeng.li-5C7GfCeVMHo@public.gmane.org>
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: "Leo (Sunpeng) Li" <sunpeng.li-5C7GfCeVMHo@public.gmane.org>,
	michel-otUistvHUpPR7s880joybQ@public.gmane.org,
	harry.wentland-5C7GfCeVMHo@public.gmane.org
Subject: [PATCH xf86-video-amdgpu 00/13] Enabling Color Management - Round 2
Date: Thu, 3 May 2018 14:31:42 -0400	[thread overview]
Message-ID: <1525372315-28462-1-git-send-email-sunpeng.li@amd.com> (raw)

From: "Leo (Sunpeng) Li" <sunpeng.li@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(-)

-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

             reply	other threads:[~2018-05-03 18:31 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-03 18:31 sunpeng.li-5C7GfCeVMHo [this message]
     [not found] ` <1525372315-28462-1-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 01/13] Add color management properties to driver-private CRTC object sunpeng.li-5C7GfCeVMHo
     [not found]     ` <1525372315-28462-2-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
2018-05-16 17:07       ` Michel Dänzer
     [not found]         ` <0dda7c80-2bdc-52a7-8dcf-ad5a7f6e9346-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-05-17 21:43           ` Leo Li
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 02/13] Push color properties to kernel DRM on CRTC init sunpeng.li-5C7GfCeVMHo
     [not found]     ` <1525372315-28462-3-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
2018-05-16 17:08       ` Michel Dänzer
     [not found]         ` <a2b8ff58-edcc-bb95-f630-1926b1a8874b-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-05-17 21:43           ` Leo Li
     [not found]             ` <db641905-9dea-015d-24d2-fa25d136abfc-5C7GfCeVMHo@public.gmane.org>
2018-05-18  7:52               ` Michel Dänzer
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 03/13] List disabled color properties on RandR outputs without a CRTC sunpeng.li-5C7GfCeVMHo
     [not found]     ` <1525372315-28462-4-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
2018-05-16 17:09       ` Michel Dänzer
     [not found]         ` <62d1f2e3-d271-658a-7bf4-88b41b080266-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-05-17 21:43           ` Leo Li
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 04/13] Use CRTC's color properties if output has a CRTC attached sunpeng.li-5C7GfCeVMHo
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 05/13] Enable setting of color properties though RandR sunpeng.li-5C7GfCeVMHo
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 06/13] Compose non-legacy with legacy gamma LUT before pushing to kernel DRM sunpeng.li-5C7GfCeVMHo
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 07/13] Also compose LUT when setting legacy gamma sunpeng.li-5C7GfCeVMHo
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 08/13] Set driver-private CRTC's dpms mode on disable sunpeng.li-5C7GfCeVMHo
     [not found]     ` <1525372315-28462-9-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
2018-05-16 17:09       ` Michel Dänzer
     [not found]         ` <12553d4f-c171-112b-bad6-6f53d47aa8e3-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-05-17 21:43           ` Leo Li
     [not found]             ` <6cf602ed-b43d-d446-74a0-a39d35042f5a-5C7GfCeVMHo@public.gmane.org>
2018-05-18 10:35               ` Michel Dänzer
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 09/13] Move drmmode_do_crtc_dpms sunpeng.li-5C7GfCeVMHo
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 10/13] Push staged color properties when DPMS state toggles On sunpeng.li-5C7GfCeVMHo
     [not found]     ` <1525372315-28462-11-git-send-email-sunpeng.li-5C7GfCeVMHo@public.gmane.org>
2018-05-16 17:10       ` Michel Dänzer
     [not found]         ` <23a9fe2a-df1c-6ecb-b60e-bdcb22d8179c-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-05-17 21:44           ` Leo Li
     [not found]             ` <f323d1de-d517-805f-d373-a7310ec6496c-5C7GfCeVMHo@public.gmane.org>
2018-05-18  8:01               ` Michel Dänzer
     [not found]                 ` <6500b993-624a-45aa-6310-958ca8c1f21b-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-05-24 19:01                   ` Leo Li
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 11/13] Push staged color properties on output detect sunpeng.li-5C7GfCeVMHo
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 12/13] Update color properties on modeset major sunpeng.li-5C7GfCeVMHo
2018-05-03 18:31   ` [PATCH xf86-video-amdgpu 13/13] Refactor pushing color management properties into a function sunpeng.li-5C7GfCeVMHo
2018-05-14 13:38   ` [PATCH xf86-video-amdgpu 00/13] Enabling Color Management - Round 2 Leo Li
2018-05-14 14:17   ` Michel Dänzer
2018-05-16 17:06   ` Michel Dänzer
     [not found]     ` <b3a7fcde-fe69-5944-1857-c2b43fa068de-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-05-17 21:43       ` Leo Li
     [not found]         ` <62c350eb-8634-5bab-e14e-6d75315e3f56-5C7GfCeVMHo@public.gmane.org>
2018-05-18  8:10           ` Michel Dänzer
     [not found]             ` <8fe3423f-8d9e-120c-32e1-7a6a7fd81606-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-05-24 20:29               ` Leo Li
     [not found]                 ` <7c183caf-d2de-cf56-ed31-7922b24d3a8e-5C7GfCeVMHo@public.gmane.org>
2018-05-25  7:51                   ` Michel Dänzer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1525372315-28462-1-git-send-email-sunpeng.li@amd.com \
    --to=sunpeng.li-5c7gfcevmho@public.gmane.org \
    --cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=harry.wentland-5C7GfCeVMHo@public.gmane.org \
    --cc=michel-otUistvHUpPR7s880joybQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox