Re: [RFC PATCH 00/40] drm/amd/display: add AMD driver-specific properties for color mgmt

[Harry Wentland]

On 5/9/23 12:52, Melissa Wen wrote:
> On 05/08, Harry Wentland wrote:
>>
>>
>> On 4/23/23 10:10, Melissa Wen wrote:
>>> Hi all,
>>>
>>> Joshua Ashton and I (with the great collaboration of Harry Wentland -
>>> thanks) have been working on KMS color pipeline enhancement for Steam
>>> Deck/SteamOS by exposing the large set of color caps available in AMD
>>> display HW.
>>>
>>
>> Thank you for your work on this.
>>
>>> This patchset results from this full-stack work, including pre-blending
>>> and post-blending new color properties. The first two patches fix
>>> quantization issues on shaper LUT programming. Just after, we have one
>>> patch that adds a config option to restrict AMD colo feature usage. The
>>> following 13 patches implement AMD driver-private color properties
>>> (pending detachment of property counter and plane color_mgmt_changed
>>> from DRM). Finally, the last 24 patches rework the AMD display manager
>>> and color management to support the properties exposed.
>>>
>>> In short, for pre-blending, we added the following:
>>> - plane degamma LUT and predefined transfer function;
>>> - plane HDR multiplier
>>> - plane shaper LUT/transfer function;
>>> - plane 3D LUT; and finally,
>>> - plane blend LUT/transfer function, just before blending.
>>>
>>> After blending, we already have DRM CRTC degamma/gamma LUTs and CTM,
>>> therefore, we extend CRTC color pipeline with the following:
>>> - CRTC shaper LUT/transfer function;
>>> - CRTC 3D LUT; and
>>> - CRTC gamma transfer function.
>>>
>>> You can already find the AMD color capabilities and color management
>>> pipeline documented here:
>>> https://dri.freedesktop.org/docs/drm/gpu/amdgpu/display/display-manager.html#color-management-properties
>>>
>>> In previous iterations, we tried to provide a generic solution for
>>> post-blending shaper and 3D LUT [1][2][3], and also Alex Hung worked on
>>> a pre-blending 3D LUT solution[4] extending plane color mgmt proposal
>>> from Uma Shankar [5]. However, we identified during our work [6] that
>>> AMD provides many other valuable capabilities that we don't find in
>>> other vendors, so we started to work on AMD driver-private color
>>> properties that better describe its color pipeline, enabling us to
>>> expose full AMD color capabilities on Deck HW.
>>>
>>> Our primary purpose is to avoid usage limitations of color calibration
>>> features provided by HW just because we don't have an interface for
>>> that. At the same time, a generic solution doesn't fit well since some
>>> of these capabilities seem AMD HW specific, such as hardcoded
>>> curve/predefined transfer function and shaper 1D LUTs sandwiching 3D
>>> LUT.
>>>
>>> So far, we keep these properties' usage under an AMD display config
>>> option (STEAM_DECK). However, we are fine with having them fully
>>> available to other DCN HW generations. In the current proposal, we are
>>> already checking ASICs before exposing a color feature. We can work on
>>> 3D LUT resource acquisition details to fit them to DCN 3+ families that
>>> support them. Indeed, before moving to these config boundaries, we
>>> started working on an open solution for any AMD HW [7].
>>>
>>
>> The problem with a CONFIG_XYZ option is that it becomes uAPI and can't be
>> removed. I feel we have a good proposal going for the generic solution.
>> Would it work for you if we don't make this a CONFIG_ option? What I mean
>> is using
>>
>> #define AMD_PRIVATE_COLOR
>>
>> around the interface bits, which are only compiled when building with
>> -DAMD_PRIVATE_COLOR
> 
> I think we can go with this approach for the properties already in use
> by Gamescope/SteamOS.
>>
>> That way we have the option to rip the driver-private stuff out later
>> while still allowing for experimentation now.
>>
>> Or, alternatively, we can merge everything but the stuff currently
>> guarded by CONFIG_STEAM_DECK, so that custom kernels can enable this
>> functionality by simply merging one patch that includes all the
>> CONFIG_STEAM_DECK stuff.
> 
> An then we can drop the interface of things that Gamescope is not
> managing, but keep those things already programmed on DM color for any
> future usage. What do you think?
> 

Sure.

Harry

> Melissa
> 
>>
>> This will allow us to merge the vast majority of the code without
>> having to maintain it in downstream repo.
>>
>>> The userspace case here is Gamescope which is the compositor for
>>> SteamOS. It's already using all of this functionality (although with a
>>> VALVE1_ prefix instead of AMD) to implement its color management
>>> pipeline right now:
>>> https://github.com/ValveSoftware/gamescope
>>>
>>> We are planning on shipping our color management support with gamut
>>> mapping, HDR, SDR on HDR, HDR on SDR, and much more in Steam OS 3.5. A
>>> brief overview of our color pipeline can be found here:
>>> https://github.com/ValveSoftware/gamescope/blob/master/src/docs/Steam%20Deck%20Display%20Pipeline.png
>>>
>>> We have also had some other userspace interests from Xaver Hugl (KDE) in
>>> experimenting with these properties for their HDR/color bring-up before
>>> a generic interface is settled on also.
>>>
>>> It still needs AMD-specific IGT tests; we are working on documentation
>>> and adding plane CTM support too. 
>>>
>>> We decided first to share our work to collect thoughts and open for
>>> discussion, even with missing refinements, since driver-private
>>> properties are not the usual DMR/KMS color management approach.
>>>
>>> Please, let us know your thoughts.
>>>
>>
>> As discussed at the hackfest I think it's a good idea to have something
>> that's easy to enable for the purposes of experimentation (and to
>> help downstream users that help us figure out how this all fits
>> together, i.e. SteamOS).
>>
>> Harry
>>
>>> Best Regards,
>>>
>>> Signed-off-by: Joshua Ashton <joshua@xxxxxxxxx>
>>> Signed-off-by: Melissa Wen<mwen@xxxxxxxxxx>
>>>
>>> [1] https://lore.kernel.org/dri-devel/20220619223104.667413-1-mwen@xxxxxxxxxx/
>>> [2] https://lore.kernel.org/amd-gfx/20220906164628.2361811-1-mwen@xxxxxxxxxx/
>>> [3] https://lore.kernel.org/dri-devel/20230109143846.1966301-1-mwen@xxxxxxxxxx/
>>> [4] https://lore.kernel.org/dri-devel/20221004211451.1475215-1-alex.hung@xxxxxxx/
>>> [5] https://lore.kernel.org/dri-devel/20210906213904.27918-1-uma.shankar@xxxxxxxxx/
>>> [6] https://gitlab.freedesktop.org/mwen/linux-amd/-/commits/amd-color-mgmt
>>> [7] https://gitlab.freedesktop.org/mwen/linux-amd/-/commits/amd-private-color-mgmt
>>>
>>> Harry Wentland (2):
>>>   drm/amd/display: fix segment distribution for linear LUTs
>>>   drm/amd/display: fix the delta clamping for shaper LUT
>>>
>>> Joshua Ashton (15):
>>>   drm/amd/display: add CRTC gamma TF to driver-private props
>>>   drm/amd/display: add plane degamma LUT driver-private props
>>>   drm/amd/display: add plane degamma TF driver-private property
>>>   drm/amd/display: add plane HDR multiplier driver-private property
>>>   drm/amd/display: add plane blend LUT and TF driver-private properties
>>>   drm/amd/display: copy 3D LUT settings from crtc state to stream_update
>>>   drm/amd/display: dynamically acquire 3DLUT resources for color changes
>>>   drm/amd/display: add CRTC regamma TF support
>>>   drm/amd/display: set sdr_ref_white_level to 80 for out_transfer_func
>>>   drm/amd/display: add support for plane degamma TF and LUT properties
>>>   drm/amd/display: add dc_fixpt_from_s3132 helper
>>>   drm/adm/display: add HDR multiplier support
>>>   drm/amd/display: handle empty LUTs in __set_input_tf
>>>   drm/amd/display: add DRM plane blend LUT and TF support
>>>   drm/amd/display: allow newer DC hardware to use degamma ROM for PQ/HLG
>>>
>>> Melissa Wen (23):
>>>   drm/amd/display: introduce Steam Deck color features to AMD display
>>>     driver
>>>   drm/drm_mode_object: increase max objects to accommodate new color
>>>     props
>>>   drm/amd/display: add shaper LUT driver-private props
>>>   drm/amd/display: add 3D LUT driver-private props
>>>   drm/drm_plane: track color mgmt changes per plane
>>>   drm/amd/display: move replace blob func to dm plane
>>>   drm/amd/display: add plane 3D LUT driver-private properties
>>>   drm/amd/display: add plane shaper LUT driver-private properties
>>>   drm/amd/display: add plane shaper TF driver-private property
>>>   drm/amd/display: add comments to describe DM crtc color mgmt behavior
>>>   drm/amd/display: encapsulate atomic regamma operation
>>>   drm/amd/display: update lut3d and shaper lut to stream
>>>   drm/amd/display: allow BYPASS 3D LUT but keep shaper LUT settings
>>>   drm/amd/display: handle MPC 3D LUT resources for a given context
>>>   drm/amd/display: add CRTC 3D LUT support to amd color pipeline
>>>   drm/amd/display: decouple steps to reuse in CRTC shaper LUT support
>>>   drm/amd/display: add CRTC shaper LUT support to amd color pipeline
>>>   drm/amd/display: add CRTC shaper TF support
>>>   drm/amd/display: mark plane as needing reset if plane color mgmt
>>>     changes
>>>   drm/amd/display: decouple steps for mapping CRTC degamma to DC plane
>>>   drm/amd/display: reject atomic commit if setting both plane and CRTC
>>>     degamma
>>>   drm/amd/display: add plane shaper/3D LUT and shaper TF support
>>>   drm/amd/display: copy dc_plane color settings to surface_updates
>>>
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 153 +++++
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h      |  92 +++
>>>  drivers/gpu/drm/amd/display/Kconfig           |   6 +
>>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  31 +-
>>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 120 +++-
>>>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 613 ++++++++++++++++--
>>>  .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c    | 124 +++-
>>>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 238 +++++++
>>>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.h   |   7 +
>>>  drivers/gpu/drm/amd/display/dc/core/dc.c      |  49 +-
>>>  drivers/gpu/drm/amd/display/dc/dc.h           |   8 +
>>>  .../amd/display/dc/dcn10/dcn10_cm_common.c    | 109 +++-
>>>  .../drm/amd/display/dc/dcn20/dcn20_hwseq.c    |   5 +-
>>>  .../drm/amd/display/dc/dcn30/dcn30_hwseq.c    |   9 +-
>>>  .../amd/display/dc/dcn301/dcn301_resource.c   |  26 +-
>>>  .../gpu/drm/amd/display/include/fixed31_32.h  |  12 +
>>>  drivers/gpu/drm/drm_atomic.c                  |   1 +
>>>  drivers/gpu/drm/drm_atomic_state_helper.c     |   1 +
>>>  include/drm/drm_mode_object.h                 |   2 +-
>>>  include/drm/drm_plane.h                       |   7 +
>>>  20 files changed, 1509 insertions(+), 104 deletions(-)
>>>
>>
>>