mbox series

[v5,00/44] Color Pipeline API w/ VKMS

Message ID 20240819205714.316380-1-harry.wentland@amd.com (mailing list archive)
Headers show
Series Color Pipeline API w/ VKMS | expand

Message

Harry Wentland Aug. 19, 2024, 8:56 p.m. UTC
This is an RFC set for a color pipeline API, along with implementations
in VKMS and amdgpu. It is tested with a set of IGT tests that can be
found at [1]. The IGT tests run a pixel-by-pixel comparison with an
allowable delta variation as the goal for these transformations is
perceptual correctness, not complete pixel accuracy.

v5 of this patchset fleshed out documentation for colorops and the
various defines that are being introduced.

VKMS supports two named transfer function colorops and two matrix
colorops.

Amdgpu advertises the following pipeline for GPUs with DCN 3 or newer:

1. 1D Curve EOTF
2. 3x4 CTM
3. Multiplier
4. 1D Curve Inverse EOTF
5. 1D LUT
6. 3D LUT
7. 1D Curve EOTF
8. 1D LUT

The supported curves for the 1D Curve type are:
- sRGB EOTF and its inverse
- PQ EOTF, scaled to [0.0, 125.0] and its inverse
- BT.2020/BT.709 OETF and its inverse

Note that the 1st and 5th colorops take the EOTF or Inverse
OETF while the 3rd colorop takes the Inverse EOTF or OETF.

The 3D LUT is a 17^3 tetrahedrally interpolated LUT but the mechanism
exists for other drivers to describe their own 3D LUT capability.

This mirrors the color pipeline used by gamescope and presented by
Melissa Wen, with the exception of the DEGAM LUT, which is not currently
used. See [1]
https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf

At this point we're hoping to see gamescope and kwin implementations
take shape. The existing pipeline should be enough to satisfy the
gamescope use-cases on the drm_plane.

In order to support YUV we'll need to add COLOR_ENCODING and COLOR_RANGE
support to the color pipeline. I have sketched these out already but
don't have it all hooked up yet. This should not hinder adoption of this
API for gaming use-cases.

We'll also want to advertise IN_FORMATS on a color pipeline as some
color pipelines won't be able to work for all IN_FORMATS on a plane.
Again, I have a sketch but no full implementation yet. This is not
currently required by the AMD color pipeline and could be added after
the merge of this set.

VKMS patches could still be improved in a few ways, though the
payoff might be limited and I would rather focus on other work
at the moment. The most obvious thing to improve would be to
eliminate the hard-coded LUTs for identity, and sRGB, and replace
them with fixed-point math instead.

There are plenty of things that I would like to see, but they could
be added after the merge of this patchset:
 - COLOR_ENCODING and COLOR_RANGE
 - IN_FORMATS for a color pipeline
 - Is it possible to support HW which can't bypass entire pipeline?
 - Can we do a LOAD / COMMIT model for LUTs (and other properties)?
 - read-only scaling colorop which defines scaling taps and position
 - named matrices, for things like converting YUV to RGB
 - Add custom LUT colorops to VKMS

IGT tests can be found at [1] or on the igt-dev mailing list.

A kernel branch can be found at [2].

I've also rebased Uma and Chaitanya's patches for the Intel color
pipeline on top of this to show how I envision them to mesh with
my changes. The relevant branches can be found at [3] for the kernel
and [4] for IGT. There were some rebase conflicts in i915 and I'm
not entirely sure I've resolved all of them correctly, but the branch
compiles and shows my thoughts for the new DRM concepts to support
Intel's pipeline.

[1] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-color-pipeline-v5
[2] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-color-pipeline-v5
[3] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-intel-color-pipeline-v5
[4] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-intel-color-pipeline-v5


v5:
 - amdgpu 3D LUT
 - Don't require BYPASS
 - update RFC docs and add to TOC tree
 - add drm_colorop and COLOR_PIPELINE kernel docs (non-RFC)
 - add amdgpu color pipeline doc
 - define SIZE property similar to drm_crtc's GAMMA_SIZE
 - various minor fixes and cleanups

v4:
 - Add amdgpu color pipeline (WIP)
 - Don't block setting of deprecated properties, instead pass client cap
   to atomic check so drivers can ignore these props
 - Drop IOCTL definitions (Pekka)
 - Use enum property for colorop TYPE (Pekka)
 - A few cleanups to the docs (Pekka)
 - Rework the TYPE enum to name relation to avoid code duplication (Pekka)
 - Add missing function declarations (Chaitanya Kumar Borah)
 - Allow setting of NEXT property to NULL in _set_ function (Chaitanya Kumar Borah)
 - Add helper for creation of pipeline drm_plane property (Pekka)
 - Always create Bypass pipeline (Pekka)
 - A bunch of changes to VKMS kunit tests (Pekka)
 - Fix index in CTM doc (Pekka)

v3:
 - Abandon IOCTLs and discover colorops as clients iterate the pipeline
 - Remove need for libdrm
 - Add color_pipeline client cap and make mutually exclusive with
   COLOR_RANGE and COLOR_ENCODING properties
 - add CTM colorop to VKMS
 - Use include way for kunit testing static functions (Arthur)
 - Make TYPE a range property
 - Move enum drm_colorop_type to uapi header
 - and a bunch of smaller bits that are highlighted in the relevant commit
   description

v2:
 - Rebased on drm-misc-next
 - Introduce a VKMS Kunit so we can test LUT functionality in vkms_composer
 - Incorporate feedback in color_pipeline.rst doc
 - Add support for sRGB inverse EOTF
 - Add 2nd enumerated TF colorop to VKMS
 - Fix LUTs and some issues with applying LUTs in VKMS

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
Cc: Simon Ser <contact@emersion.fr>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Melissa Wen <mwen@igalia.com>
Cc: Jonas Ådahl <jadahl@redhat.com>
Cc: Sebastian Wick <sebastian.wick@redhat.com>
Cc: Shashank Sharma <shashank.sharma@amd.com>
Cc: Alexander Goins <agoins@nvidia.com>
Cc: Joshua Ashton <joshua@froggi.es>
Cc: Michel Dänzer <mdaenzer@redhat.com>
Cc: Aleix Pol <aleixpol@kde.org>
Cc: Xaver Hugl <xaver.hugl@gmail.com>
Cc: Victoria Brekenfeld <victoria@system76.com>
Cc: Sima <daniel@ffwll.ch>
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Naseer Ahmed <quic_naseer@quicinc.com>
Cc: Christopher Braga <quic_cbraga@quicinc.com>
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Arthur Grillo <arthurgrillo@riseup.net>
Cc: Hector Martin <marcan@marcan.st>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Sasha McIntosh <sashamcintosh@google.com>
Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

Alex Hung (13):
  drm/colorop: define a new macro for_each_new_colorop_in_state
  drm/amd/display: Skip color pipeline initialization for cursor plane
  drm/amd/display: Add support for sRGB EOTF in DEGAM block
  drm/amd/display: Add support for sRGB Inverse EOTF in SHAPER block
  drm/amd/display: Add support for sRGB EOTF in BLND block
  drm/colorop: Add 1D Curve Custom LUT type
  drm/amd/display: add shaper and blend colorops for 1D Curve Custom LUT
  drm/amd/display: add 3x4 matrix colorop
  drm/colorop: Add mutliplier type
  drm/amd/display: add multiplier colorop
  drm/amd/display: Swap matrix and multiplier
  drm/colorop: Add 3D LUT supports to color pipeline
  drm/amd/display: add 3D LUT colorop

Harry Wentland (31):
  drm: Add helper for conversion from signed-magnitude
  drm/vkms: Round fixp2int conversion in lerp_u16
  drm/vkms: Add kunit tests for VKMS LUT handling
  drm/doc/rfc: Describe why prescriptive color pipeline is needed
  drm/colorop: Introduce new drm_colorop mode object
  drm/colorop: Add TYPE property
  drm/colorop: Add 1D Curve subtype
  Documentation/gpu: document drm_colorop
  drm/colorop: Add BYPASS property
  drm/colorop: Add NEXT property
  drm/colorop: Add atomic state print for drm_colorop
  drm/plane: Add COLOR PIPELINE property
  drm/colorop: Add NEXT to colorop state print
  drm/vkms: Add enumerated 1D curve colorop
  drm/vkms: Add kunit tests for linear and sRGB LUTs
  drm/colorop: Introduce DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
  drm/colorop: Add 3x4 CTM type
  drm/vkms: Use s32 for internal color pipeline precision
  drm/vkms: add 3x4 matrix in color pipeline
  drm/tests: Add a few tests around drm_fixed.h
  drm/vkms: Add tests for CTM handling
  drm/colorop: pass plane_color_pipeline client cap to atomic check
  drm/amd/display: Ignore deprecated props when plane_color_pipeline set
  drm/amd/display: Add bypass COLOR PIPELINE
  drm/colorop: Add PQ 125 EOTF and its inverse
  drm/amd/display: Enable support for PQ 125 EOTF and Inverse
  drm/colorop: add BT2020/BT709 OETF and Inverse OETF
  drm/amd/display: Add support for BT.709 and BT.2020 TFs
  drm/colorop: Define LUT_1D interpolation
  drm/colorop: allow non-bypass colorops
  drm/amd/display: Add AMD color pipeline doc

 Documentation/gpu/drm-kms.rst                 |  15 +
 Documentation/gpu/rfc/color_pipeline.rst      | 376 ++++++++
 Documentation/gpu/rfc/index.rst               |   3 +
 drivers/gpu/drm/Makefile                      |   1 +
 .../gpu/drm/amd/display/amdgpu_dm/Makefile    |   3 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   4 +
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 581 ++++++++++++-
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 196 +++++
 .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |  36 +
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  32 +
 drivers/gpu/drm/drm_atomic.c                  | 184 +++-
 drivers/gpu/drm/drm_atomic_helper.c           |  12 +
 drivers/gpu/drm/drm_atomic_state_helper.c     |   5 +
 drivers/gpu/drm/drm_atomic_uapi.c             | 175 ++++
 drivers/gpu/drm/drm_colorop.c                 | 562 ++++++++++++
 drivers/gpu/drm/drm_ioctl.c                   |   7 +
 drivers/gpu/drm/drm_mode_config.c             |   7 +
 drivers/gpu/drm/drm_plane.c                   |  52 ++
 drivers/gpu/drm/tests/Makefile                |   3 +-
 drivers/gpu/drm/tests/drm_fixp_test.c         |  69 ++
 drivers/gpu/drm/vkms/Kconfig                  |   5 +
 drivers/gpu/drm/vkms/Makefile                 |   4 +-
 drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
 drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 449 ++++++++++
 drivers/gpu/drm/vkms/vkms_colorop.c           | 102 +++
 drivers/gpu/drm/vkms/vkms_composer.c          | 115 ++-
 drivers/gpu/drm/vkms/vkms_drv.h               |   8 +
 drivers/gpu/drm/vkms/vkms_luts.c              | 802 ++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_luts.h              |  12 +
 drivers/gpu/drm/vkms/vkms_plane.c             |   2 +
 include/drm/drm_atomic.h                      | 130 +++
 include/drm/drm_atomic_uapi.h                 |   3 +
 include/drm/drm_colorop.h                     | 427 ++++++++++
 include/drm/drm_file.h                        |   7 +
 include/drm/drm_fixed.h                       |  18 +
 include/drm/drm_mode_config.h                 |  18 +
 include/drm/drm_plane.h                       |  19 +
 include/uapi/drm/amdgpu_drm.h                 |   9 -
 include/uapi/drm/drm.h                        |  16 +
 include/uapi/drm/drm_mode.h                   | 156 ++++
 40 files changed, 4595 insertions(+), 34 deletions(-)
 create mode 100644 Documentation/gpu/rfc/color_pipeline.rst
 create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
 create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
 create mode 100644 drivers/gpu/drm/drm_colorop.c
 create mode 100644 drivers/gpu/drm/tests/drm_fixp_test.c
 create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
 create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
 create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c
 create mode 100644 drivers/gpu/drm/vkms/vkms_luts.c
 create mode 100644 drivers/gpu/drm/vkms/vkms_luts.h
 create mode 100644 include/drm/drm_colorop.h

--
2.46.0

Comments

Louis Chauvet Aug. 27, 2024, 5:49 p.m. UTC | #1
Le 19/08/24 - 16:56, Harry Wentland a écrit :
> This is an RFC set for a color pipeline API, along with implementations
> in VKMS and amdgpu. It is tested with a set of IGT tests that can be
> found at [1]. The IGT tests run a pixel-by-pixel comparison with an
> allowable delta variation as the goal for these transformations is
> perceptual correctness, not complete pixel accuracy.
> 
> v5 of this patchset fleshed out documentation for colorops and the
> various defines that are being introduced.
> 
> VKMS supports two named transfer function colorops and two matrix
> colorops.
> 
> Amdgpu advertises the following pipeline for GPUs with DCN 3 or newer:
> 
> 1. 1D Curve EOTF
> 2. 3x4 CTM
> 3. Multiplier
> 4. 1D Curve Inverse EOTF
> 5. 1D LUT
> 6. 3D LUT
> 7. 1D Curve EOTF
> 8. 1D LUT
> 
> The supported curves for the 1D Curve type are:
> - sRGB EOTF and its inverse
> - PQ EOTF, scaled to [0.0, 125.0] and its inverse
> - BT.2020/BT.709 OETF and its inverse
> 
> Note that the 1st and 5th colorops take the EOTF or Inverse
> OETF while the 3rd colorop takes the Inverse EOTF or OETF.
> 
> The 3D LUT is a 17^3 tetrahedrally interpolated LUT but the mechanism
> exists for other drivers to describe their own 3D LUT capability.
> 
> This mirrors the color pipeline used by gamescope and presented by
> Melissa Wen, with the exception of the DEGAM LUT, which is not currently
> used. See [1]
> https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf
> 
> At this point we're hoping to see gamescope and kwin implementations
> take shape. The existing pipeline should be enough to satisfy the
> gamescope use-cases on the drm_plane.
> 
> In order to support YUV we'll need to add COLOR_ENCODING and COLOR_RANGE
> support to the color pipeline. I have sketched these out already but
> don't have it all hooked up yet. This should not hinder adoption of this
> API for gaming use-cases.
> 
> We'll also want to advertise IN_FORMATS on a color pipeline as some
> color pipelines won't be able to work for all IN_FORMATS on a plane.
> Again, I have a sketch but no full implementation yet. This is not
> currently required by the AMD color pipeline and could be added after
> the merge of this set.
> 
> VKMS patches could still be improved in a few ways, though the
> payoff might be limited and I would rather focus on other work
> at the moment. The most obvious thing to improve would be to
> eliminate the hard-coded LUTs for identity, and sRGB, and replace
> them with fixed-point math instead.

Hi!

I reviewed your VKMS patches and added a few comments in your series. This 
series looks very good.

Thanks for this work,
Louis Chauvet
 
> There are plenty of things that I would like to see, but they could
> be added after the merge of this patchset:
>  - COLOR_ENCODING and COLOR_RANGE
>  - IN_FORMATS for a color pipeline
>  - Is it possible to support HW which can't bypass entire pipeline?
>  - Can we do a LOAD / COMMIT model for LUTs (and other properties)?
>  - read-only scaling colorop which defines scaling taps and position
>  - named matrices, for things like converting YUV to RGB
>  - Add custom LUT colorops to VKMS
> 
> IGT tests can be found at [1] or on the igt-dev mailing list.
> 
> A kernel branch can be found at [2].
> 
> I've also rebased Uma and Chaitanya's patches for the Intel color
> pipeline on top of this to show how I envision them to mesh with
> my changes. The relevant branches can be found at [3] for the kernel
> and [4] for IGT. There were some rebase conflicts in i915 and I'm
> not entirely sure I've resolved all of them correctly, but the branch
> compiles and shows my thoughts for the new DRM concepts to support
> Intel's pipeline.
> 
> [1] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-color-pipeline-v5
> [2] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-color-pipeline-v5
> [3] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-intel-color-pipeline-v5
> [4] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-intel-color-pipeline-v5
> 
> 
> v5:
>  - amdgpu 3D LUT
>  - Don't require BYPASS
>  - update RFC docs and add to TOC tree
>  - add drm_colorop and COLOR_PIPELINE kernel docs (non-RFC)
>  - add amdgpu color pipeline doc
>  - define SIZE property similar to drm_crtc's GAMMA_SIZE
>  - various minor fixes and cleanups
> 
> v4:
>  - Add amdgpu color pipeline (WIP)
>  - Don't block setting of deprecated properties, instead pass client cap
>    to atomic check so drivers can ignore these props
>  - Drop IOCTL definitions (Pekka)
>  - Use enum property for colorop TYPE (Pekka)
>  - A few cleanups to the docs (Pekka)
>  - Rework the TYPE enum to name relation to avoid code duplication (Pekka)
>  - Add missing function declarations (Chaitanya Kumar Borah)
>  - Allow setting of NEXT property to NULL in _set_ function (Chaitanya Kumar Borah)
>  - Add helper for creation of pipeline drm_plane property (Pekka)
>  - Always create Bypass pipeline (Pekka)
>  - A bunch of changes to VKMS kunit tests (Pekka)
>  - Fix index in CTM doc (Pekka)
> 
> v3:
>  - Abandon IOCTLs and discover colorops as clients iterate the pipeline
>  - Remove need for libdrm
>  - Add color_pipeline client cap and make mutually exclusive with
>    COLOR_RANGE and COLOR_ENCODING properties
>  - add CTM colorop to VKMS
>  - Use include way for kunit testing static functions (Arthur)
>  - Make TYPE a range property
>  - Move enum drm_colorop_type to uapi header
>  - and a bunch of smaller bits that are highlighted in the relevant commit
>    description
> 
> v2:
>  - Rebased on drm-misc-next
>  - Introduce a VKMS Kunit so we can test LUT functionality in vkms_composer
>  - Incorporate feedback in color_pipeline.rst doc
>  - Add support for sRGB inverse EOTF
>  - Add 2nd enumerated TF colorop to VKMS
>  - Fix LUTs and some issues with applying LUTs in VKMS
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> Cc: Simon Ser <contact@emersion.fr>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Melissa Wen <mwen@igalia.com>
> Cc: Jonas Ådahl <jadahl@redhat.com>
> Cc: Sebastian Wick <sebastian.wick@redhat.com>
> Cc: Shashank Sharma <shashank.sharma@amd.com>
> Cc: Alexander Goins <agoins@nvidia.com>
> Cc: Joshua Ashton <joshua@froggi.es>
> Cc: Michel Dänzer <mdaenzer@redhat.com>
> Cc: Aleix Pol <aleixpol@kde.org>
> Cc: Xaver Hugl <xaver.hugl@gmail.com>
> Cc: Victoria Brekenfeld <victoria@system76.com>
> Cc: Sima <daniel@ffwll.ch>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Naseer Ahmed <quic_naseer@quicinc.com>
> Cc: Christopher Braga <quic_cbraga@quicinc.com>
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Arthur Grillo <arthurgrillo@riseup.net>
> Cc: Hector Martin <marcan@marcan.st>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: Sasha McIntosh <sashamcintosh@google.com>
> Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> 
> Alex Hung (13):
>   drm/colorop: define a new macro for_each_new_colorop_in_state
>   drm/amd/display: Skip color pipeline initialization for cursor plane
>   drm/amd/display: Add support for sRGB EOTF in DEGAM block
>   drm/amd/display: Add support for sRGB Inverse EOTF in SHAPER block
>   drm/amd/display: Add support for sRGB EOTF in BLND block
>   drm/colorop: Add 1D Curve Custom LUT type
>   drm/amd/display: add shaper and blend colorops for 1D Curve Custom LUT
>   drm/amd/display: add 3x4 matrix colorop
>   drm/colorop: Add mutliplier type
>   drm/amd/display: add multiplier colorop
>   drm/amd/display: Swap matrix and multiplier
>   drm/colorop: Add 3D LUT supports to color pipeline
>   drm/amd/display: add 3D LUT colorop
> 
> Harry Wentland (31):
>   drm: Add helper for conversion from signed-magnitude
>   drm/vkms: Round fixp2int conversion in lerp_u16
>   drm/vkms: Add kunit tests for VKMS LUT handling
>   drm/doc/rfc: Describe why prescriptive color pipeline is needed
>   drm/colorop: Introduce new drm_colorop mode object
>   drm/colorop: Add TYPE property
>   drm/colorop: Add 1D Curve subtype
>   Documentation/gpu: document drm_colorop
>   drm/colorop: Add BYPASS property
>   drm/colorop: Add NEXT property
>   drm/colorop: Add atomic state print for drm_colorop
>   drm/plane: Add COLOR PIPELINE property
>   drm/colorop: Add NEXT to colorop state print
>   drm/vkms: Add enumerated 1D curve colorop
>   drm/vkms: Add kunit tests for linear and sRGB LUTs
>   drm/colorop: Introduce DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
>   drm/colorop: Add 3x4 CTM type
>   drm/vkms: Use s32 for internal color pipeline precision
>   drm/vkms: add 3x4 matrix in color pipeline
>   drm/tests: Add a few tests around drm_fixed.h
>   drm/vkms: Add tests for CTM handling
>   drm/colorop: pass plane_color_pipeline client cap to atomic check
>   drm/amd/display: Ignore deprecated props when plane_color_pipeline set
>   drm/amd/display: Add bypass COLOR PIPELINE
>   drm/colorop: Add PQ 125 EOTF and its inverse
>   drm/amd/display: Enable support for PQ 125 EOTF and Inverse
>   drm/colorop: add BT2020/BT709 OETF and Inverse OETF
>   drm/amd/display: Add support for BT.709 and BT.2020 TFs
>   drm/colorop: Define LUT_1D interpolation
>   drm/colorop: allow non-bypass colorops
>   drm/amd/display: Add AMD color pipeline doc
> 
>  Documentation/gpu/drm-kms.rst                 |  15 +
>  Documentation/gpu/rfc/color_pipeline.rst      | 376 ++++++++
>  Documentation/gpu/rfc/index.rst               |   3 +
>  drivers/gpu/drm/Makefile                      |   1 +
>  .../gpu/drm/amd/display/amdgpu_dm/Makefile    |   3 +-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   4 +
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 581 ++++++++++++-
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 196 +++++
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |  36 +
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  32 +
>  drivers/gpu/drm/drm_atomic.c                  | 184 +++-
>  drivers/gpu/drm/drm_atomic_helper.c           |  12 +
>  drivers/gpu/drm/drm_atomic_state_helper.c     |   5 +
>  drivers/gpu/drm/drm_atomic_uapi.c             | 175 ++++
>  drivers/gpu/drm/drm_colorop.c                 | 562 ++++++++++++
>  drivers/gpu/drm/drm_ioctl.c                   |   7 +
>  drivers/gpu/drm/drm_mode_config.c             |   7 +
>  drivers/gpu/drm/drm_plane.c                   |  52 ++
>  drivers/gpu/drm/tests/Makefile                |   3 +-
>  drivers/gpu/drm/tests/drm_fixp_test.c         |  69 ++
>  drivers/gpu/drm/vkms/Kconfig                  |   5 +
>  drivers/gpu/drm/vkms/Makefile                 |   4 +-
>  drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
>  drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 449 ++++++++++
>  drivers/gpu/drm/vkms/vkms_colorop.c           | 102 +++
>  drivers/gpu/drm/vkms/vkms_composer.c          | 115 ++-
>  drivers/gpu/drm/vkms/vkms_drv.h               |   8 +
>  drivers/gpu/drm/vkms/vkms_luts.c              | 802 ++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_luts.h              |  12 +
>  drivers/gpu/drm/vkms/vkms_plane.c             |   2 +
>  include/drm/drm_atomic.h                      | 130 +++
>  include/drm/drm_atomic_uapi.h                 |   3 +
>  include/drm/drm_colorop.h                     | 427 ++++++++++
>  include/drm/drm_file.h                        |   7 +
>  include/drm/drm_fixed.h                       |  18 +
>  include/drm/drm_mode_config.h                 |  18 +
>  include/drm/drm_plane.h                       |  19 +
>  include/uapi/drm/amdgpu_drm.h                 |   9 -
>  include/uapi/drm/drm.h                        |  16 +
>  include/uapi/drm/drm_mode.h                   | 156 ++++
>  40 files changed, 4595 insertions(+), 34 deletions(-)
>  create mode 100644 Documentation/gpu/rfc/color_pipeline.rst
>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
>  create mode 100644 drivers/gpu/drm/drm_colorop.c
>  create mode 100644 drivers/gpu/drm/tests/drm_fixp_test.c
>  create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
>  create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.h
>  create mode 100644 include/drm/drm_colorop.h
> 
> --
> 2.46.0
>
Xaver Hugl Aug. 29, 2024, 2:55 p.m. UTC | #2
Hi,

I have a WIP implementation of this for KWin at
https://invent.kde.org/plasma/kwin/-/commits/work/zamundaaa/drm-colorop.
It maps KWin's color pipeline to the drm one to get (primary plane
only, for now) direct scanout of HDR content on SDR displays while
doing tone mapping (or SDR on HDR, but that's been possible before).
It currently uses the two 1D custom luts and the 3D lut for this; once
I add support for named 1D curves, it should also make use of that and
the matrix. I haven't tested the Intel version yet, but if it can
match the pipeline, it should work the same there.

In testing your amd-color-pipeline-v5 branch I get two crashes in the
kernel though: https://invent.kde.org/-/snippets/3217
The null pointer dereference only happens sometimes, seemingly
randomly, but if I make KWin generate new blobs each frame, I can 100%
reliably reproduce the list_add corruption problem by just opening
glxgears in fullscreen while the color profile of the display is set
to "built in" (which makes it use the 3D LUT for gamut mapping).
Here's a branch that generates the color pipeline each frame and
triggers this: https://invent.kde.org/plasma/kwin/-/commits/work/zamundaaa/drm-colorop-list-add-corruption



Am Mo., 19. Aug. 2024 um 22:57 Uhr schrieb Harry Wentland
<harry.wentland@amd.com>:
>
> This is an RFC set for a color pipeline API, along with implementations
> in VKMS and amdgpu. It is tested with a set of IGT tests that can be
> found at [1]. The IGT tests run a pixel-by-pixel comparison with an
> allowable delta variation as the goal for these transformations is
> perceptual correctness, not complete pixel accuracy.
>
> v5 of this patchset fleshed out documentation for colorops and the
> various defines that are being introduced.
>
> VKMS supports two named transfer function colorops and two matrix
> colorops.
>
> Amdgpu advertises the following pipeline for GPUs with DCN 3 or newer:
>
> 1. 1D Curve EOTF
> 2. 3x4 CTM
> 3. Multiplier
> 4. 1D Curve Inverse EOTF
> 5. 1D LUT
> 6. 3D LUT
> 7. 1D Curve EOTF
> 8. 1D LUT
>
> The supported curves for the 1D Curve type are:
> - sRGB EOTF and its inverse
> - PQ EOTF, scaled to [0.0, 125.0] and its inverse
> - BT.2020/BT.709 OETF and its inverse
>
> Note that the 1st and 5th colorops take the EOTF or Inverse
> OETF while the 3rd colorop takes the Inverse EOTF or OETF.
>
> The 3D LUT is a 17^3 tetrahedrally interpolated LUT but the mechanism
> exists for other drivers to describe their own 3D LUT capability.
>
> This mirrors the color pipeline used by gamescope and presented by
> Melissa Wen, with the exception of the DEGAM LUT, which is not currently
> used. See [1]
> https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf
>
> At this point we're hoping to see gamescope and kwin implementations
> take shape. The existing pipeline should be enough to satisfy the
> gamescope use-cases on the drm_plane.
>
> In order to support YUV we'll need to add COLOR_ENCODING and COLOR_RANGE
> support to the color pipeline. I have sketched these out already but
> don't have it all hooked up yet. This should not hinder adoption of this
> API for gaming use-cases.
>
> We'll also want to advertise IN_FORMATS on a color pipeline as some
> color pipelines won't be able to work for all IN_FORMATS on a plane.
> Again, I have a sketch but no full implementation yet. This is not
> currently required by the AMD color pipeline and could be added after
> the merge of this set.
>
> VKMS patches could still be improved in a few ways, though the
> payoff might be limited and I would rather focus on other work
> at the moment. The most obvious thing to improve would be to
> eliminate the hard-coded LUTs for identity, and sRGB, and replace
> them with fixed-point math instead.
>
> There are plenty of things that I would like to see, but they could
> be added after the merge of this patchset:
>  - COLOR_ENCODING and COLOR_RANGE
>  - IN_FORMATS for a color pipeline
>  - Is it possible to support HW which can't bypass entire pipeline?
>  - Can we do a LOAD / COMMIT model for LUTs (and other properties)?
>  - read-only scaling colorop which defines scaling taps and position
>  - named matrices, for things like converting YUV to RGB
>  - Add custom LUT colorops to VKMS
>
> IGT tests can be found at [1] or on the igt-dev mailing list.
>
> A kernel branch can be found at [2].
>
> I've also rebased Uma and Chaitanya's patches for the Intel color
> pipeline on top of this to show how I envision them to mesh with
> my changes. The relevant branches can be found at [3] for the kernel
> and [4] for IGT. There were some rebase conflicts in i915 and I'm
> not entirely sure I've resolved all of them correctly, but the branch
> compiles and shows my thoughts for the new DRM concepts to support
> Intel's pipeline.
>
> [1] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-color-pipeline-v5
> [2] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-color-pipeline-v5
> [3] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-intel-color-pipeline-v5
> [4] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-intel-color-pipeline-v5
>
>
> v5:
>  - amdgpu 3D LUT
>  - Don't require BYPASS
>  - update RFC docs and add to TOC tree
>  - add drm_colorop and COLOR_PIPELINE kernel docs (non-RFC)
>  - add amdgpu color pipeline doc
>  - define SIZE property similar to drm_crtc's GAMMA_SIZE
>  - various minor fixes and cleanups
>
> v4:
>  - Add amdgpu color pipeline (WIP)
>  - Don't block setting of deprecated properties, instead pass client cap
>    to atomic check so drivers can ignore these props
>  - Drop IOCTL definitions (Pekka)
>  - Use enum property for colorop TYPE (Pekka)
>  - A few cleanups to the docs (Pekka)
>  - Rework the TYPE enum to name relation to avoid code duplication (Pekka)
>  - Add missing function declarations (Chaitanya Kumar Borah)
>  - Allow setting of NEXT property to NULL in _set_ function (Chaitanya Kumar Borah)
>  - Add helper for creation of pipeline drm_plane property (Pekka)
>  - Always create Bypass pipeline (Pekka)
>  - A bunch of changes to VKMS kunit tests (Pekka)
>  - Fix index in CTM doc (Pekka)
>
> v3:
>  - Abandon IOCTLs and discover colorops as clients iterate the pipeline
>  - Remove need for libdrm
>  - Add color_pipeline client cap and make mutually exclusive with
>    COLOR_RANGE and COLOR_ENCODING properties
>  - add CTM colorop to VKMS
>  - Use include way for kunit testing static functions (Arthur)
>  - Make TYPE a range property
>  - Move enum drm_colorop_type to uapi header
>  - and a bunch of smaller bits that are highlighted in the relevant commit
>    description
>
> v2:
>  - Rebased on drm-misc-next
>  - Introduce a VKMS Kunit so we can test LUT functionality in vkms_composer
>  - Incorporate feedback in color_pipeline.rst doc
>  - Add support for sRGB inverse EOTF
>  - Add 2nd enumerated TF colorop to VKMS
>  - Fix LUTs and some issues with applying LUTs in VKMS
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> Cc: Simon Ser <contact@emersion.fr>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Melissa Wen <mwen@igalia.com>
> Cc: Jonas Ådahl <jadahl@redhat.com>
> Cc: Sebastian Wick <sebastian.wick@redhat.com>
> Cc: Shashank Sharma <shashank.sharma@amd.com>
> Cc: Alexander Goins <agoins@nvidia.com>
> Cc: Joshua Ashton <joshua@froggi.es>
> Cc: Michel Dänzer <mdaenzer@redhat.com>
> Cc: Aleix Pol <aleixpol@kde.org>
> Cc: Xaver Hugl <xaver.hugl@gmail.com>
> Cc: Victoria Brekenfeld <victoria@system76.com>
> Cc: Sima <daniel@ffwll.ch>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Naseer Ahmed <quic_naseer@quicinc.com>
> Cc: Christopher Braga <quic_cbraga@quicinc.com>
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Arthur Grillo <arthurgrillo@riseup.net>
> Cc: Hector Martin <marcan@marcan.st>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: Sasha McIntosh <sashamcintosh@google.com>
> Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>
> Alex Hung (13):
>   drm/colorop: define a new macro for_each_new_colorop_in_state
>   drm/amd/display: Skip color pipeline initialization for cursor plane
>   drm/amd/display: Add support for sRGB EOTF in DEGAM block
>   drm/amd/display: Add support for sRGB Inverse EOTF in SHAPER block
>   drm/amd/display: Add support for sRGB EOTF in BLND block
>   drm/colorop: Add 1D Curve Custom LUT type
>   drm/amd/display: add shaper and blend colorops for 1D Curve Custom LUT
>   drm/amd/display: add 3x4 matrix colorop
>   drm/colorop: Add mutliplier type
>   drm/amd/display: add multiplier colorop
>   drm/amd/display: Swap matrix and multiplier
>   drm/colorop: Add 3D LUT supports to color pipeline
>   drm/amd/display: add 3D LUT colorop
>
> Harry Wentland (31):
>   drm: Add helper for conversion from signed-magnitude
>   drm/vkms: Round fixp2int conversion in lerp_u16
>   drm/vkms: Add kunit tests for VKMS LUT handling
>   drm/doc/rfc: Describe why prescriptive color pipeline is needed
>   drm/colorop: Introduce new drm_colorop mode object
>   drm/colorop: Add TYPE property
>   drm/colorop: Add 1D Curve subtype
>   Documentation/gpu: document drm_colorop
>   drm/colorop: Add BYPASS property
>   drm/colorop: Add NEXT property
>   drm/colorop: Add atomic state print for drm_colorop
>   drm/plane: Add COLOR PIPELINE property
>   drm/colorop: Add NEXT to colorop state print
>   drm/vkms: Add enumerated 1D curve colorop
>   drm/vkms: Add kunit tests for linear and sRGB LUTs
>   drm/colorop: Introduce DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
>   drm/colorop: Add 3x4 CTM type
>   drm/vkms: Use s32 for internal color pipeline precision
>   drm/vkms: add 3x4 matrix in color pipeline
>   drm/tests: Add a few tests around drm_fixed.h
>   drm/vkms: Add tests for CTM handling
>   drm/colorop: pass plane_color_pipeline client cap to atomic check
>   drm/amd/display: Ignore deprecated props when plane_color_pipeline set
>   drm/amd/display: Add bypass COLOR PIPELINE
>   drm/colorop: Add PQ 125 EOTF and its inverse
>   drm/amd/display: Enable support for PQ 125 EOTF and Inverse
>   drm/colorop: add BT2020/BT709 OETF and Inverse OETF
>   drm/amd/display: Add support for BT.709 and BT.2020 TFs
>   drm/colorop: Define LUT_1D interpolation
>   drm/colorop: allow non-bypass colorops
>   drm/amd/display: Add AMD color pipeline doc
>
>  Documentation/gpu/drm-kms.rst                 |  15 +
>  Documentation/gpu/rfc/color_pipeline.rst      | 376 ++++++++
>  Documentation/gpu/rfc/index.rst               |   3 +
>  drivers/gpu/drm/Makefile                      |   1 +
>  .../gpu/drm/amd/display/amdgpu_dm/Makefile    |   3 +-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   4 +
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 581 ++++++++++++-
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 196 +++++
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |  36 +
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  32 +
>  drivers/gpu/drm/drm_atomic.c                  | 184 +++-
>  drivers/gpu/drm/drm_atomic_helper.c           |  12 +
>  drivers/gpu/drm/drm_atomic_state_helper.c     |   5 +
>  drivers/gpu/drm/drm_atomic_uapi.c             | 175 ++++
>  drivers/gpu/drm/drm_colorop.c                 | 562 ++++++++++++
>  drivers/gpu/drm/drm_ioctl.c                   |   7 +
>  drivers/gpu/drm/drm_mode_config.c             |   7 +
>  drivers/gpu/drm/drm_plane.c                   |  52 ++
>  drivers/gpu/drm/tests/Makefile                |   3 +-
>  drivers/gpu/drm/tests/drm_fixp_test.c         |  69 ++
>  drivers/gpu/drm/vkms/Kconfig                  |   5 +
>  drivers/gpu/drm/vkms/Makefile                 |   4 +-
>  drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
>  drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 449 ++++++++++
>  drivers/gpu/drm/vkms/vkms_colorop.c           | 102 +++
>  drivers/gpu/drm/vkms/vkms_composer.c          | 115 ++-
>  drivers/gpu/drm/vkms/vkms_drv.h               |   8 +
>  drivers/gpu/drm/vkms/vkms_luts.c              | 802 ++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_luts.h              |  12 +
>  drivers/gpu/drm/vkms/vkms_plane.c             |   2 +
>  include/drm/drm_atomic.h                      | 130 +++
>  include/drm/drm_atomic_uapi.h                 |   3 +
>  include/drm/drm_colorop.h                     | 427 ++++++++++
>  include/drm/drm_file.h                        |   7 +
>  include/drm/drm_fixed.h                       |  18 +
>  include/drm/drm_mode_config.h                 |  18 +
>  include/drm/drm_plane.h                       |  19 +
>  include/uapi/drm/amdgpu_drm.h                 |   9 -
>  include/uapi/drm/drm.h                        |  16 +
>  include/uapi/drm/drm_mode.h                   | 156 ++++
>  40 files changed, 4595 insertions(+), 34 deletions(-)
>  create mode 100644 Documentation/gpu/rfc/color_pipeline.rst
>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
>  create mode 100644 drivers/gpu/drm/drm_colorop.c
>  create mode 100644 drivers/gpu/drm/tests/drm_fixp_test.c
>  create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
>  create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.h
>  create mode 100644 include/drm/drm_colorop.h
>
> --
> 2.46.0
>
Harry Wentland Sept. 9, 2024, 6:40 p.m. UTC | #3
On 2024-08-29 10:55, Xaver Hugl wrote:
> Hi,
> 
> I have a WIP implementation of this for KWin at
> https://invent.kde.org/plasma/kwin/-/commits/work/zamundaaa/drm-colorop.
> It maps KWin's color pipeline to the drm one to get (primary plane
> only, for now) direct scanout of HDR content on SDR displays while
> doing tone mapping (or SDR on HDR, but that's been possible before).
> It currently uses the two 1D custom luts and the 3D lut for this; once
> I add support for named 1D curves, it should also make use of that and
> the matrix. I haven't tested the Intel version yet, but if it can
> match the pipeline, it should work the same there.
> 
> In testing your amd-color-pipeline-v5 branch I get two crashes in the
> kernel though: https://invent.kde.org/-/snippets/3217
> The null pointer dereference only happens sometimes, seemingly
> randomly, but if I make KWin generate new blobs each frame, I can 100%
> reliably reproduce the list_add corruption problem by just opening
> glxgears in fullscreen while the color profile of the display is set
> to "built in" (which makes it use the 3D LUT for gamut mapping).
> Here's a branch that generates the color pipeline each frame and
> triggers this: https://invent.kde.org/plasma/kwin/-/commits/work/zamundaaa/drm-colorop-list-add-corruption
> 

Thanks for the test branch. Got a repro and will investigate.

Harry

> 
> 
> Am Mo., 19. Aug. 2024 um 22:57 Uhr schrieb Harry Wentland
> <harry.wentland@amd.com>:
>>
>> This is an RFC set for a color pipeline API, along with implementations
>> in VKMS and amdgpu. It is tested with a set of IGT tests that can be
>> found at [1]. The IGT tests run a pixel-by-pixel comparison with an
>> allowable delta variation as the goal for these transformations is
>> perceptual correctness, not complete pixel accuracy.
>>
>> v5 of this patchset fleshed out documentation for colorops and the
>> various defines that are being introduced.
>>
>> VKMS supports two named transfer function colorops and two matrix
>> colorops.
>>
>> Amdgpu advertises the following pipeline for GPUs with DCN 3 or newer:
>>
>> 1. 1D Curve EOTF
>> 2. 3x4 CTM
>> 3. Multiplier
>> 4. 1D Curve Inverse EOTF
>> 5. 1D LUT
>> 6. 3D LUT
>> 7. 1D Curve EOTF
>> 8. 1D LUT
>>
>> The supported curves for the 1D Curve type are:
>> - sRGB EOTF and its inverse
>> - PQ EOTF, scaled to [0.0, 125.0] and its inverse
>> - BT.2020/BT.709 OETF and its inverse
>>
>> Note that the 1st and 5th colorops take the EOTF or Inverse
>> OETF while the 3rd colorop takes the Inverse EOTF or OETF.
>>
>> The 3D LUT is a 17^3 tetrahedrally interpolated LUT but the mechanism
>> exists for other drivers to describe their own 3D LUT capability.
>>
>> This mirrors the color pipeline used by gamescope and presented by
>> Melissa Wen, with the exception of the DEGAM LUT, which is not currently
>> used. See [1]
>> https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf
>>
>> At this point we're hoping to see gamescope and kwin implementations
>> take shape. The existing pipeline should be enough to satisfy the
>> gamescope use-cases on the drm_plane.
>>
>> In order to support YUV we'll need to add COLOR_ENCODING and COLOR_RANGE
>> support to the color pipeline. I have sketched these out already but
>> don't have it all hooked up yet. This should not hinder adoption of this
>> API for gaming use-cases.
>>
>> We'll also want to advertise IN_FORMATS on a color pipeline as some
>> color pipelines won't be able to work for all IN_FORMATS on a plane.
>> Again, I have a sketch but no full implementation yet. This is not
>> currently required by the AMD color pipeline and could be added after
>> the merge of this set.
>>
>> VKMS patches could still be improved in a few ways, though the
>> payoff might be limited and I would rather focus on other work
>> at the moment. The most obvious thing to improve would be to
>> eliminate the hard-coded LUTs for identity, and sRGB, and replace
>> them with fixed-point math instead.
>>
>> There are plenty of things that I would like to see, but they could
>> be added after the merge of this patchset:
>>  - COLOR_ENCODING and COLOR_RANGE
>>  - IN_FORMATS for a color pipeline
>>  - Is it possible to support HW which can't bypass entire pipeline?
>>  - Can we do a LOAD / COMMIT model for LUTs (and other properties)?
>>  - read-only scaling colorop which defines scaling taps and position
>>  - named matrices, for things like converting YUV to RGB
>>  - Add custom LUT colorops to VKMS
>>
>> IGT tests can be found at [1] or on the igt-dev mailing list.
>>
>> A kernel branch can be found at [2].
>>
>> I've also rebased Uma and Chaitanya's patches for the Intel color
>> pipeline on top of this to show how I envision them to mesh with
>> my changes. The relevant branches can be found at [3] for the kernel
>> and [4] for IGT. There were some rebase conflicts in i915 and I'm
>> not entirely sure I've resolved all of them correctly, but the branch
>> compiles and shows my thoughts for the new DRM concepts to support
>> Intel's pipeline.
>>
>> [1] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-color-pipeline-v5
>> [2] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-color-pipeline-v5
>> [3] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-intel-color-pipeline-v5
>> [4] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-intel-color-pipeline-v5
>>
>>
>> v5:
>>  - amdgpu 3D LUT
>>  - Don't require BYPASS
>>  - update RFC docs and add to TOC tree
>>  - add drm_colorop and COLOR_PIPELINE kernel docs (non-RFC)
>>  - add amdgpu color pipeline doc
>>  - define SIZE property similar to drm_crtc's GAMMA_SIZE
>>  - various minor fixes and cleanups
>>
>> v4:
>>  - Add amdgpu color pipeline (WIP)
>>  - Don't block setting of deprecated properties, instead pass client cap
>>    to atomic check so drivers can ignore these props
>>  - Drop IOCTL definitions (Pekka)
>>  - Use enum property for colorop TYPE (Pekka)
>>  - A few cleanups to the docs (Pekka)
>>  - Rework the TYPE enum to name relation to avoid code duplication (Pekka)
>>  - Add missing function declarations (Chaitanya Kumar Borah)
>>  - Allow setting of NEXT property to NULL in _set_ function (Chaitanya Kumar Borah)
>>  - Add helper for creation of pipeline drm_plane property (Pekka)
>>  - Always create Bypass pipeline (Pekka)
>>  - A bunch of changes to VKMS kunit tests (Pekka)
>>  - Fix index in CTM doc (Pekka)
>>
>> v3:
>>  - Abandon IOCTLs and discover colorops as clients iterate the pipeline
>>  - Remove need for libdrm
>>  - Add color_pipeline client cap and make mutually exclusive with
>>    COLOR_RANGE and COLOR_ENCODING properties
>>  - add CTM colorop to VKMS
>>  - Use include way for kunit testing static functions (Arthur)
>>  - Make TYPE a range property
>>  - Move enum drm_colorop_type to uapi header
>>  - and a bunch of smaller bits that are highlighted in the relevant commit
>>    description
>>
>> v2:
>>  - Rebased on drm-misc-next
>>  - Introduce a VKMS Kunit so we can test LUT functionality in vkms_composer
>>  - Incorporate feedback in color_pipeline.rst doc
>>  - Add support for sRGB inverse EOTF
>>  - Add 2nd enumerated TF colorop to VKMS
>>  - Fix LUTs and some issues with applying LUTs in VKMS
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
>> Cc: Simon Ser <contact@emersion.fr>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Melissa Wen <mwen@igalia.com>
>> Cc: Jonas Ådahl <jadahl@redhat.com>
>> Cc: Sebastian Wick <sebastian.wick@redhat.com>
>> Cc: Shashank Sharma <shashank.sharma@amd.com>
>> Cc: Alexander Goins <agoins@nvidia.com>
>> Cc: Joshua Ashton <joshua@froggi.es>
>> Cc: Michel Dänzer <mdaenzer@redhat.com>
>> Cc: Aleix Pol <aleixpol@kde.org>
>> Cc: Xaver Hugl <xaver.hugl@gmail.com>
>> Cc: Victoria Brekenfeld <victoria@system76.com>
>> Cc: Sima <daniel@ffwll.ch>
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Cc: Naseer Ahmed <quic_naseer@quicinc.com>
>> Cc: Christopher Braga <quic_cbraga@quicinc.com>
>> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Cc: Arthur Grillo <arthurgrillo@riseup.net>
>> Cc: Hector Martin <marcan@marcan.st>
>> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
>> Cc: Sasha McIntosh <sashamcintosh@google.com>
>> Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>>
>> Alex Hung (13):
>>   drm/colorop: define a new macro for_each_new_colorop_in_state
>>   drm/amd/display: Skip color pipeline initialization for cursor plane
>>   drm/amd/display: Add support for sRGB EOTF in DEGAM block
>>   drm/amd/display: Add support for sRGB Inverse EOTF in SHAPER block
>>   drm/amd/display: Add support for sRGB EOTF in BLND block
>>   drm/colorop: Add 1D Curve Custom LUT type
>>   drm/amd/display: add shaper and blend colorops for 1D Curve Custom LUT
>>   drm/amd/display: add 3x4 matrix colorop
>>   drm/colorop: Add mutliplier type
>>   drm/amd/display: add multiplier colorop
>>   drm/amd/display: Swap matrix and multiplier
>>   drm/colorop: Add 3D LUT supports to color pipeline
>>   drm/amd/display: add 3D LUT colorop
>>
>> Harry Wentland (31):
>>   drm: Add helper for conversion from signed-magnitude
>>   drm/vkms: Round fixp2int conversion in lerp_u16
>>   drm/vkms: Add kunit tests for VKMS LUT handling
>>   drm/doc/rfc: Describe why prescriptive color pipeline is needed
>>   drm/colorop: Introduce new drm_colorop mode object
>>   drm/colorop: Add TYPE property
>>   drm/colorop: Add 1D Curve subtype
>>   Documentation/gpu: document drm_colorop
>>   drm/colorop: Add BYPASS property
>>   drm/colorop: Add NEXT property
>>   drm/colorop: Add atomic state print for drm_colorop
>>   drm/plane: Add COLOR PIPELINE property
>>   drm/colorop: Add NEXT to colorop state print
>>   drm/vkms: Add enumerated 1D curve colorop
>>   drm/vkms: Add kunit tests for linear and sRGB LUTs
>>   drm/colorop: Introduce DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
>>   drm/colorop: Add 3x4 CTM type
>>   drm/vkms: Use s32 for internal color pipeline precision
>>   drm/vkms: add 3x4 matrix in color pipeline
>>   drm/tests: Add a few tests around drm_fixed.h
>>   drm/vkms: Add tests for CTM handling
>>   drm/colorop: pass plane_color_pipeline client cap to atomic check
>>   drm/amd/display: Ignore deprecated props when plane_color_pipeline set
>>   drm/amd/display: Add bypass COLOR PIPELINE
>>   drm/colorop: Add PQ 125 EOTF and its inverse
>>   drm/amd/display: Enable support for PQ 125 EOTF and Inverse
>>   drm/colorop: add BT2020/BT709 OETF and Inverse OETF
>>   drm/amd/display: Add support for BT.709 and BT.2020 TFs
>>   drm/colorop: Define LUT_1D interpolation
>>   drm/colorop: allow non-bypass colorops
>>   drm/amd/display: Add AMD color pipeline doc
>>
>>  Documentation/gpu/drm-kms.rst                 |  15 +
>>  Documentation/gpu/rfc/color_pipeline.rst      | 376 ++++++++
>>  Documentation/gpu/rfc/index.rst               |   3 +
>>  drivers/gpu/drm/Makefile                      |   1 +
>>  .../gpu/drm/amd/display/amdgpu_dm/Makefile    |   3 +-
>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   4 +
>>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 581 ++++++++++++-
>>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 196 +++++
>>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |  36 +
>>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  32 +
>>  drivers/gpu/drm/drm_atomic.c                  | 184 +++-
>>  drivers/gpu/drm/drm_atomic_helper.c           |  12 +
>>  drivers/gpu/drm/drm_atomic_state_helper.c     |   5 +
>>  drivers/gpu/drm/drm_atomic_uapi.c             | 175 ++++
>>  drivers/gpu/drm/drm_colorop.c                 | 562 ++++++++++++
>>  drivers/gpu/drm/drm_ioctl.c                   |   7 +
>>  drivers/gpu/drm/drm_mode_config.c             |   7 +
>>  drivers/gpu/drm/drm_plane.c                   |  52 ++
>>  drivers/gpu/drm/tests/Makefile                |   3 +-
>>  drivers/gpu/drm/tests/drm_fixp_test.c         |  69 ++
>>  drivers/gpu/drm/vkms/Kconfig                  |   5 +
>>  drivers/gpu/drm/vkms/Makefile                 |   4 +-
>>  drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
>>  drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 449 ++++++++++
>>  drivers/gpu/drm/vkms/vkms_colorop.c           | 102 +++
>>  drivers/gpu/drm/vkms/vkms_composer.c          | 115 ++-
>>  drivers/gpu/drm/vkms/vkms_drv.h               |   8 +
>>  drivers/gpu/drm/vkms/vkms_luts.c              | 802 ++++++++++++++++++
>>  drivers/gpu/drm/vkms/vkms_luts.h              |  12 +
>>  drivers/gpu/drm/vkms/vkms_plane.c             |   2 +
>>  include/drm/drm_atomic.h                      | 130 +++
>>  include/drm/drm_atomic_uapi.h                 |   3 +
>>  include/drm/drm_colorop.h                     | 427 ++++++++++
>>  include/drm/drm_file.h                        |   7 +
>>  include/drm/drm_fixed.h                       |  18 +
>>  include/drm/drm_mode_config.h                 |  18 +
>>  include/drm/drm_plane.h                       |  19 +
>>  include/uapi/drm/amdgpu_drm.h                 |   9 -
>>  include/uapi/drm/drm.h                        |  16 +
>>  include/uapi/drm/drm_mode.h                   | 156 ++++
>>  40 files changed, 4595 insertions(+), 34 deletions(-)
>>  create mode 100644 Documentation/gpu/rfc/color_pipeline.rst
>>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
>>  create mode 100644 drivers/gpu/drm/drm_colorop.c
>>  create mode 100644 drivers/gpu/drm/tests/drm_fixp_test.c
>>  create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
>>  create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>>  create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c
>>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.c
>>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.h
>>  create mode 100644 include/drm/drm_colorop.h
>>
>> --
>> 2.46.0
>>
Alex Goins Sept. 10, 2024, 8:37 p.m. UTC | #4
Hi Harry,

Thanks for this. I just want to remind about a few things that would be required
for NVIDIA hardware, as discussed at the Display Next Hackfest -- fully
understand that they aren't currently included in this series because they
aren't required on AMD hardware. Allowing color ops to be non-bypassable is a
big improvement in this direction.

In each of these cases, it should be possible to represent NVIDIA's color
pipeline using the existing color ops, but it would rely on rejecting invalid
configurations in atomic check, with no way to communicate the limitations to
clients.

- Tag IN_FORMATS for a color pipeline (as you have mentioned).
    + Required to communicate that the first NVIDIA pre-blending LUT is
      incompatible with FP16 inputs.

- Introduce a single-channel 1DLUT color op, or add the ability to flag
  individual channels of an ordinary 1DLUT as read-only.
    + Required to handle NVIDIA's 1D tone-mapping LUT designed to operate only
      on the intensity component of ICtCp.
  
- Allow marking color ops as read-only and non-bypassable (halfway there)
  and/or allow marking a color op as requiring a modeset to update.
    + Required to handle specific LUTs that can't have their entries changed
      while in use without visible glitches/tearing, yet must be enabled to
      support tone-mapping and pre-blend scaling.
    + In lieu of the first option, it may be suitable to just use the existing
      1D curve colorop with only a single curve supported, marked as
      non-bypassable. Allowing modesets as in the second option would allow for
      the possibility of selecting different curves, however.

Thanks,
Alex Goins
NVIDIA

On Mon, 9 Sep 2024, Harry Wentland wrote:

> External email: Use caution opening links or attachments
> 
> 
> On 2024-08-29 10:55, Xaver Hugl wrote:
> > Hi,
> >
> > I have a WIP implementation of this for KWin at
> > https://invent.kde.org/plasma/kwin/-/commits/work/zamundaaa/drm-colorop.
> > It maps KWin's color pipeline to the drm one to get (primary plane
> > only, for now) direct scanout of HDR content on SDR displays while
> > doing tone mapping (or SDR on HDR, but that's been possible before).
> > It currently uses the two 1D custom luts and the 3D lut for this; once
> > I add support for named 1D curves, it should also make use of that and
> > the matrix. I haven't tested the Intel version yet, but if it can
> > match the pipeline, it should work the same there.
> >
> > In testing your amd-color-pipeline-v5 branch I get two crashes in the
> > kernel though: https://invent.kde.org/-/snippets/3217
> > The null pointer dereference only happens sometimes, seemingly
> > randomly, but if I make KWin generate new blobs each frame, I can 100%
> > reliably reproduce the list_add corruption problem by just opening
> > glxgears in fullscreen while the color profile of the display is set
> > to "built in" (which makes it use the 3D LUT for gamut mapping).
> > Here's a branch that generates the color pipeline each frame and
> > triggers this: https://invent.kde.org/plasma/kwin/-/commits/work/zamundaaa/drm-colorop-list-add-corruption
> >
> 
> Thanks for the test branch. Got a repro and will investigate.
> 
> Harry
> 
> >
> >
> > Am Mo., 19. Aug. 2024 um 22:57 Uhr schrieb Harry Wentland
> > <harry.wentland@amd.com>:
> >>
> >> This is an RFC set for a color pipeline API, along with implementations
> >> in VKMS and amdgpu. It is tested with a set of IGT tests that can be
> >> found at [1]. The IGT tests run a pixel-by-pixel comparison with an
> >> allowable delta variation as the goal for these transformations is
> >> perceptual correctness, not complete pixel accuracy.
> >>
> >> v5 of this patchset fleshed out documentation for colorops and the
> >> various defines that are being introduced.
> >>
> >> VKMS supports two named transfer function colorops and two matrix
> >> colorops.
> >>
> >> Amdgpu advertises the following pipeline for GPUs with DCN 3 or newer:
> >>
> >> 1. 1D Curve EOTF
> >> 2. 3x4 CTM
> >> 3. Multiplier
> >> 4. 1D Curve Inverse EOTF
> >> 5. 1D LUT
> >> 6. 3D LUT
> >> 7. 1D Curve EOTF
> >> 8. 1D LUT
> >>
> >> The supported curves for the 1D Curve type are:
> >> - sRGB EOTF and its inverse
> >> - PQ EOTF, scaled to [0.0, 125.0] and its inverse
> >> - BT.2020/BT.709 OETF and its inverse
> >>
> >> Note that the 1st and 5th colorops take the EOTF or Inverse
> >> OETF while the 3rd colorop takes the Inverse EOTF or OETF.
> >>
> >> The 3D LUT is a 17^3 tetrahedrally interpolated LUT but the mechanism
> >> exists for other drivers to describe their own 3D LUT capability.
> >>
> >> This mirrors the color pipeline used by gamescope and presented by
> >> Melissa Wen, with the exception of the DEGAM LUT, which is not currently
> >> used. See [1]
> >> https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf
> >>
> >> At this point we're hoping to see gamescope and kwin implementations
> >> take shape. The existing pipeline should be enough to satisfy the
> >> gamescope use-cases on the drm_plane.
> >>
> >> In order to support YUV we'll need to add COLOR_ENCODING and COLOR_RANGE
> >> support to the color pipeline. I have sketched these out already but
> >> don't have it all hooked up yet. This should not hinder adoption of this
> >> API for gaming use-cases.
> >>
> >> We'll also want to advertise IN_FORMATS on a color pipeline as some
> >> color pipelines won't be able to work for all IN_FORMATS on a plane.
> >> Again, I have a sketch but no full implementation yet. This is not
> >> currently required by the AMD color pipeline and could be added after
> >> the merge of this set.
> >>
> >> VKMS patches could still be improved in a few ways, though the
> >> payoff might be limited and I would rather focus on other work
> >> at the moment. The most obvious thing to improve would be to
> >> eliminate the hard-coded LUTs for identity, and sRGB, and replace
> >> them with fixed-point math instead.
> >>
> >> There are plenty of things that I would like to see, but they could
> >> be added after the merge of this patchset:
> >>  - COLOR_ENCODING and COLOR_RANGE
> >>  - IN_FORMATS for a color pipeline
> >>  - Is it possible to support HW which can't bypass entire pipeline?
> >>  - Can we do a LOAD / COMMIT model for LUTs (and other properties)?
> >>  - read-only scaling colorop which defines scaling taps and position
> >>  - named matrices, for things like converting YUV to RGB
> >>  - Add custom LUT colorops to VKMS
> >>
> >> IGT tests can be found at [1] or on the igt-dev mailing list.
> >>
> >> A kernel branch can be found at [2].
> >>
> >> I've also rebased Uma and Chaitanya's patches for the Intel color
> >> pipeline on top of this to show how I envision them to mesh with
> >> my changes. The relevant branches can be found at [3] for the kernel
> >> and [4] for IGT. There were some rebase conflicts in i915 and I'm
> >> not entirely sure I've resolved all of them correctly, but the branch
> >> compiles and shows my thoughts for the new DRM concepts to support
> >> Intel's pipeline.
> >>
> >> [1] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-color-pipeline-v5
> >> [2] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-color-pipeline-v5
> >> [3] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-intel-color-pipeline-v5
> >> [4] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-intel-color-pipeline-v5
> >>
> >>
> >> v5:
> >>  - amdgpu 3D LUT
> >>  - Don't require BYPASS
> >>  - update RFC docs and add to TOC tree
> >>  - add drm_colorop and COLOR_PIPELINE kernel docs (non-RFC)
> >>  - add amdgpu color pipeline doc
> >>  - define SIZE property similar to drm_crtc's GAMMA_SIZE
> >>  - various minor fixes and cleanups
> >>
> >> v4:
> >>  - Add amdgpu color pipeline (WIP)
> >>  - Don't block setting of deprecated properties, instead pass client cap
> >>    to atomic check so drivers can ignore these props
> >>  - Drop IOCTL definitions (Pekka)
> >>  - Use enum property for colorop TYPE (Pekka)
> >>  - A few cleanups to the docs (Pekka)
> >>  - Rework the TYPE enum to name relation to avoid code duplication (Pekka)
> >>  - Add missing function declarations (Chaitanya Kumar Borah)
> >>  - Allow setting of NEXT property to NULL in _set_ function (Chaitanya Kumar Borah)
> >>  - Add helper for creation of pipeline drm_plane property (Pekka)
> >>  - Always create Bypass pipeline (Pekka)
> >>  - A bunch of changes to VKMS kunit tests (Pekka)
> >>  - Fix index in CTM doc (Pekka)
> >>
> >> v3:
> >>  - Abandon IOCTLs and discover colorops as clients iterate the pipeline
> >>  - Remove need for libdrm
> >>  - Add color_pipeline client cap and make mutually exclusive with
> >>    COLOR_RANGE and COLOR_ENCODING properties
> >>  - add CTM colorop to VKMS
> >>  - Use include way for kunit testing static functions (Arthur)
> >>  - Make TYPE a range property
> >>  - Move enum drm_colorop_type to uapi header
> >>  - and a bunch of smaller bits that are highlighted in the relevant commit
> >>    description
> >>
> >> v2:
> >>  - Rebased on drm-misc-next
> >>  - Introduce a VKMS Kunit so we can test LUT functionality in vkms_composer
> >>  - Incorporate feedback in color_pipeline.rst doc
> >>  - Add support for sRGB inverse EOTF
> >>  - Add 2nd enumerated TF colorop to VKMS
> >>  - Fix LUTs and some issues with applying LUTs in VKMS
> >>
> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> >> Cc: Simon Ser <contact@emersion.fr>
> >> Cc: Harry Wentland <harry.wentland@amd.com>
> >> Cc: Melissa Wen <mwen@igalia.com>
> >> Cc: Jonas Ådahl <jadahl@redhat.com>
> >> Cc: Sebastian Wick <sebastian.wick@redhat.com>
> >> Cc: Shashank Sharma <shashank.sharma@amd.com>
> >> Cc: Alexander Goins <agoins@nvidia.com>
> >> Cc: Joshua Ashton <joshua@froggi.es>
> >> Cc: Michel Dänzer <mdaenzer@redhat.com>
> >> Cc: Aleix Pol <aleixpol@kde.org>
> >> Cc: Xaver Hugl <xaver.hugl@gmail.com>
> >> Cc: Victoria Brekenfeld <victoria@system76.com>
> >> Cc: Sima <daniel@ffwll.ch>
> >> Cc: Uma Shankar <uma.shankar@intel.com>
> >> Cc: Naseer Ahmed <quic_naseer@quicinc.com>
> >> Cc: Christopher Braga <quic_cbraga@quicinc.com>
> >> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> >> Cc: Arthur Grillo <arthurgrillo@riseup.net>
> >> Cc: Hector Martin <marcan@marcan.st>
> >> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> >> Cc: Sasha McIntosh <sashamcintosh@google.com>
> >> Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> >>
> >> Alex Hung (13):
> >>   drm/colorop: define a new macro for_each_new_colorop_in_state
> >>   drm/amd/display: Skip color pipeline initialization for cursor plane
> >>   drm/amd/display: Add support for sRGB EOTF in DEGAM block
> >>   drm/amd/display: Add support for sRGB Inverse EOTF in SHAPER block
> >>   drm/amd/display: Add support for sRGB EOTF in BLND block
> >>   drm/colorop: Add 1D Curve Custom LUT type
> >>   drm/amd/display: add shaper and blend colorops for 1D Curve Custom LUT
> >>   drm/amd/display: add 3x4 matrix colorop
> >>   drm/colorop: Add mutliplier type
> >>   drm/amd/display: add multiplier colorop
> >>   drm/amd/display: Swap matrix and multiplier
> >>   drm/colorop: Add 3D LUT supports to color pipeline
> >>   drm/amd/display: add 3D LUT colorop
> >>
> >> Harry Wentland (31):
> >>   drm: Add helper for conversion from signed-magnitude
> >>   drm/vkms: Round fixp2int conversion in lerp_u16
> >>   drm/vkms: Add kunit tests for VKMS LUT handling
> >>   drm/doc/rfc: Describe why prescriptive color pipeline is needed
> >>   drm/colorop: Introduce new drm_colorop mode object
> >>   drm/colorop: Add TYPE property
> >>   drm/colorop: Add 1D Curve subtype
> >>   Documentation/gpu: document drm_colorop
> >>   drm/colorop: Add BYPASS property
> >>   drm/colorop: Add NEXT property
> >>   drm/colorop: Add atomic state print for drm_colorop
> >>   drm/plane: Add COLOR PIPELINE property
> >>   drm/colorop: Add NEXT to colorop state print
> >>   drm/vkms: Add enumerated 1D curve colorop
> >>   drm/vkms: Add kunit tests for linear and sRGB LUTs
> >>   drm/colorop: Introduce DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
> >>   drm/colorop: Add 3x4 CTM type
> >>   drm/vkms: Use s32 for internal color pipeline precision
> >>   drm/vkms: add 3x4 matrix in color pipeline
> >>   drm/tests: Add a few tests around drm_fixed.h
> >>   drm/vkms: Add tests for CTM handling
> >>   drm/colorop: pass plane_color_pipeline client cap to atomic check
> >>   drm/amd/display: Ignore deprecated props when plane_color_pipeline set
> >>   drm/amd/display: Add bypass COLOR PIPELINE
> >>   drm/colorop: Add PQ 125 EOTF and its inverse
> >>   drm/amd/display: Enable support for PQ 125 EOTF and Inverse
> >>   drm/colorop: add BT2020/BT709 OETF and Inverse OETF
> >>   drm/amd/display: Add support for BT.709 and BT.2020 TFs
> >>   drm/colorop: Define LUT_1D interpolation
> >>   drm/colorop: allow non-bypass colorops
> >>   drm/amd/display: Add AMD color pipeline doc
> >>
> >>  Documentation/gpu/drm-kms.rst                 |  15 +
> >>  Documentation/gpu/rfc/color_pipeline.rst      | 376 ++++++++
> >>  Documentation/gpu/rfc/index.rst               |   3 +
> >>  drivers/gpu/drm/Makefile                      |   1 +
> >>  .../gpu/drm/amd/display/amdgpu_dm/Makefile    |   3 +-
> >>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   4 +
> >>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 581 ++++++++++++-
> >>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 196 +++++
> >>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |  36 +
> >>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  32 +
> >>  drivers/gpu/drm/drm_atomic.c                  | 184 +++-
> >>  drivers/gpu/drm/drm_atomic_helper.c           |  12 +
> >>  drivers/gpu/drm/drm_atomic_state_helper.c     |   5 +
> >>  drivers/gpu/drm/drm_atomic_uapi.c             | 175 ++++
> >>  drivers/gpu/drm/drm_colorop.c                 | 562 ++++++++++++
> >>  drivers/gpu/drm/drm_ioctl.c                   |   7 +
> >>  drivers/gpu/drm/drm_mode_config.c             |   7 +
> >>  drivers/gpu/drm/drm_plane.c                   |  52 ++
> >>  drivers/gpu/drm/tests/Makefile                |   3 +-
> >>  drivers/gpu/drm/tests/drm_fixp_test.c         |  69 ++
> >>  drivers/gpu/drm/vkms/Kconfig                  |   5 +
> >>  drivers/gpu/drm/vkms/Makefile                 |   4 +-
> >>  drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
> >>  drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 449 ++++++++++
> >>  drivers/gpu/drm/vkms/vkms_colorop.c           | 102 +++
> >>  drivers/gpu/drm/vkms/vkms_composer.c          | 115 ++-
> >>  drivers/gpu/drm/vkms/vkms_drv.h               |   8 +
> >>  drivers/gpu/drm/vkms/vkms_luts.c              | 802 ++++++++++++++++++
> >>  drivers/gpu/drm/vkms/vkms_luts.h              |  12 +
> >>  drivers/gpu/drm/vkms/vkms_plane.c             |   2 +
> >>  include/drm/drm_atomic.h                      | 130 +++
> >>  include/drm/drm_atomic_uapi.h                 |   3 +
> >>  include/drm/drm_colorop.h                     | 427 ++++++++++
> >>  include/drm/drm_file.h                        |   7 +
> >>  include/drm/drm_fixed.h                       |  18 +
> >>  include/drm/drm_mode_config.h                 |  18 +
> >>  include/drm/drm_plane.h                       |  19 +
> >>  include/uapi/drm/amdgpu_drm.h                 |   9 -
> >>  include/uapi/drm/drm.h                        |  16 +
> >>  include/uapi/drm/drm_mode.h                   | 156 ++++
> >>  40 files changed, 4595 insertions(+), 34 deletions(-)
> >>  create mode 100644 Documentation/gpu/rfc/color_pipeline.rst
> >>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> >>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
> >>  create mode 100644 drivers/gpu/drm/drm_colorop.c
> >>  create mode 100644 drivers/gpu/drm/tests/drm_fixp_test.c
> >>  create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
> >>  create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
> >>  create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c
> >>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.c
> >>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.h
> >>  create mode 100644 include/drm/drm_colorop.h
> >>
> >> --
> >> 2.46.0
> >>
> 
>
Harry Wentland Oct. 3, 2024, 8 p.m. UTC | #5
On 2024-09-10 16:37, Alex Goins wrote:
> Hi Harry,
>
> Thanks for this. I just want to remind about a few things that would be required
> for NVIDIA hardware, as discussed at the Display Next Hackfest -- fully
> understand that they aren't currently included in this series because they
> aren't required on AMD hardware. Allowing color ops to be non-bypassable is a
> big improvement in this direction.
>

Thanks for your feedback.

> In each of these cases, it should be possible to represent NVIDIA's color
> pipeline using the existing color ops, but it would rely on rejecting invalid
> configurations in atomic check, with no way to communicate the limitations to
> clients.
>
> - Tag IN_FORMATS for a color pipeline (as you have mentioned).
>     + Required to communicate that the first NVIDIA pre-blending LUT is
>       incompatible with FP16 inputs.
>

I have a sketch in a branch [1] but the work was a bit larger than
I anticipated so I parked it for now.

> - Introduce a single-channel 1DLUT color op, or add the ability to flag
>   individual channels of an ordinary 1DLUT as read-only.
>     + Required to handle NVIDIA's 1D tone-mapping LUT designed to operate only
>       on the intensity component of ICtCp.
>

I'm reluctant to add a new single-channel colorop type without having the
ability to test it. Though I guess VKMS could serve as a stand-in but that
would mean an implementation for it in VKMS.

> - Allow marking color ops as read-only and non-bypassable (halfway there)
>   and/or allow marking a color op as requiring a modeset to update.
>     + Required to handle specific LUTs that can't have their entries changed
>       while in use without visible glitches/tearing, yet must be enabled to
>       support tone-mapping and pre-blend scaling.
>     + In lieu of the first option, it may be suitable to just use the existing
>       1D curve colorop with only a single curve supported, marked as
>       non-bypassable. Allowing modesets as in the second option would allow for
>       the possibility of selecting different curves, however.
>

Flags should be easy to add to a colorop because a colorop is just a DRM
object, so we can extend it with new properties. I envision this to be a
FLAGS bitmask property, so the implementation can probably look similar
to the rotation property [2]. And the flags can be REQUIRES_MODESET and
SINGLE_CHANNEL, or something like that.

[1] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-color-pipeline-v6-in-formats
[2] https://drmdb.emersion.fr/properties/3435973836/rotation

Harry

> Thanks,
> Alex Goins
> NVIDIA
Harry Wentland Oct. 3, 2024, 8:09 p.m. UTC | #6
On 2024-08-27 13:49, Louis Chauvet wrote:
> Le 19/08/24 - 16:56, Harry Wentland a écrit :
>> This is an RFC set for a color pipeline API, along with implementations
>> in VKMS and amdgpu. It is tested with a set of IGT tests that can be
>> found at [1]. The IGT tests run a pixel-by-pixel comparison with an
>> allowable delta variation as the goal for these transformations is
>> perceptual correctness, not complete pixel accuracy.
>>
>> v5 of this patchset fleshed out documentation for colorops and the
>> various defines that are being introduced.
>>
>> VKMS supports two named transfer function colorops and two matrix
>> colorops.
>>
>> Amdgpu advertises the following pipeline for GPUs with DCN 3 or newer:
>>
>> 1. 1D Curve EOTF
>> 2. 3x4 CTM
>> 3. Multiplier
>> 4. 1D Curve Inverse EOTF
>> 5. 1D LUT
>> 6. 3D LUT
>> 7. 1D Curve EOTF
>> 8. 1D LUT
>>
>> The supported curves for the 1D Curve type are:
>> - sRGB EOTF and its inverse
>> - PQ EOTF, scaled to [0.0, 125.0] and its inverse
>> - BT.2020/BT.709 OETF and its inverse
>>
>> Note that the 1st and 5th colorops take the EOTF or Inverse
>> OETF while the 3rd colorop takes the Inverse EOTF or OETF.
>>
>> The 3D LUT is a 17^3 tetrahedrally interpolated LUT but the mechanism
>> exists for other drivers to describe their own 3D LUT capability.
>>
>> This mirrors the color pipeline used by gamescope and presented by
>> Melissa Wen, with the exception of the DEGAM LUT, which is not currently
>> used. See [1]
>> https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf
>>
>> At this point we're hoping to see gamescope and kwin implementations
>> take shape. The existing pipeline should be enough to satisfy the
>> gamescope use-cases on the drm_plane.
>>
>> In order to support YUV we'll need to add COLOR_ENCODING and COLOR_RANGE
>> support to the color pipeline. I have sketched these out already but
>> don't have it all hooked up yet. This should not hinder adoption of this
>> API for gaming use-cases.
>>
>> We'll also want to advertise IN_FORMATS on a color pipeline as some
>> color pipelines won't be able to work for all IN_FORMATS on a plane.
>> Again, I have a sketch but no full implementation yet. This is not
>> currently required by the AMD color pipeline and could be added after
>> the merge of this set.
>>
>> VKMS patches could still be improved in a few ways, though the
>> payoff might be limited and I would rather focus on other work
>> at the moment. The most obvious thing to improve would be to
>> eliminate the hard-coded LUTs for identity, and sRGB, and replace
>> them with fixed-point math instead.
> 
> Hi!
> 
> I reviewed your VKMS patches and added a few comments in your series. This 
> series looks very good.
> 

Thanks so much for your review. v6 just went out.

Harry

> Thanks for this work,
> Louis Chauvet
>  
>> There are plenty of things that I would like to see, but they could
>> be added after the merge of this patchset:
>>  - COLOR_ENCODING and COLOR_RANGE
>>  - IN_FORMATS for a color pipeline
>>  - Is it possible to support HW which can't bypass entire pipeline?
>>  - Can we do a LOAD / COMMIT model for LUTs (and other properties)?
>>  - read-only scaling colorop which defines scaling taps and position
>>  - named matrices, for things like converting YUV to RGB
>>  - Add custom LUT colorops to VKMS
>>
>> IGT tests can be found at [1] or on the igt-dev mailing list.
>>
>> A kernel branch can be found at [2].
>>
>> I've also rebased Uma and Chaitanya's patches for the Intel color
>> pipeline on top of this to show how I envision them to mesh with
>> my changes. The relevant branches can be found at [3] for the kernel
>> and [4] for IGT. There were some rebase conflicts in i915 and I'm
>> not entirely sure I've resolved all of them correctly, but the branch
>> compiles and shows my thoughts for the new DRM concepts to support
>> Intel's pipeline.
>>
>> [1] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-color-pipeline-v5
>> [2] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-color-pipeline-v5
>> [3] https://gitlab.freedesktop.org/hwentland/linux/-/tree/amd-intel-color-pipeline-v5
>> [4] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/amd-intel-color-pipeline-v5
>>
>>
>> v5:
>>  - amdgpu 3D LUT
>>  - Don't require BYPASS
>>  - update RFC docs and add to TOC tree
>>  - add drm_colorop and COLOR_PIPELINE kernel docs (non-RFC)
>>  - add amdgpu color pipeline doc
>>  - define SIZE property similar to drm_crtc's GAMMA_SIZE
>>  - various minor fixes and cleanups
>>
>> v4:
>>  - Add amdgpu color pipeline (WIP)
>>  - Don't block setting of deprecated properties, instead pass client cap
>>    to atomic check so drivers can ignore these props
>>  - Drop IOCTL definitions (Pekka)
>>  - Use enum property for colorop TYPE (Pekka)
>>  - A few cleanups to the docs (Pekka)
>>  - Rework the TYPE enum to name relation to avoid code duplication (Pekka)
>>  - Add missing function declarations (Chaitanya Kumar Borah)
>>  - Allow setting of NEXT property to NULL in _set_ function (Chaitanya Kumar Borah)
>>  - Add helper for creation of pipeline drm_plane property (Pekka)
>>  - Always create Bypass pipeline (Pekka)
>>  - A bunch of changes to VKMS kunit tests (Pekka)
>>  - Fix index in CTM doc (Pekka)
>>
>> v3:
>>  - Abandon IOCTLs and discover colorops as clients iterate the pipeline
>>  - Remove need for libdrm
>>  - Add color_pipeline client cap and make mutually exclusive with
>>    COLOR_RANGE and COLOR_ENCODING properties
>>  - add CTM colorop to VKMS
>>  - Use include way for kunit testing static functions (Arthur)
>>  - Make TYPE a range property
>>  - Move enum drm_colorop_type to uapi header
>>  - and a bunch of smaller bits that are highlighted in the relevant commit
>>    description
>>
>> v2:
>>  - Rebased on drm-misc-next
>>  - Introduce a VKMS Kunit so we can test LUT functionality in vkms_composer
>>  - Incorporate feedback in color_pipeline.rst doc
>>  - Add support for sRGB inverse EOTF
>>  - Add 2nd enumerated TF colorop to VKMS
>>  - Fix LUTs and some issues with applying LUTs in VKMS
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
>> Cc: Simon Ser <contact@emersion.fr>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Melissa Wen <mwen@igalia.com>
>> Cc: Jonas Ådahl <jadahl@redhat.com>
>> Cc: Sebastian Wick <sebastian.wick@redhat.com>
>> Cc: Shashank Sharma <shashank.sharma@amd.com>
>> Cc: Alexander Goins <agoins@nvidia.com>
>> Cc: Joshua Ashton <joshua@froggi.es>
>> Cc: Michel Dänzer <mdaenzer@redhat.com>
>> Cc: Aleix Pol <aleixpol@kde.org>
>> Cc: Xaver Hugl <xaver.hugl@gmail.com>
>> Cc: Victoria Brekenfeld <victoria@system76.com>
>> Cc: Sima <daniel@ffwll.ch>
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Cc: Naseer Ahmed <quic_naseer@quicinc.com>
>> Cc: Christopher Braga <quic_cbraga@quicinc.com>
>> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Cc: Arthur Grillo <arthurgrillo@riseup.net>
>> Cc: Hector Martin <marcan@marcan.st>
>> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
>> Cc: Sasha McIntosh <sashamcintosh@google.com>
>> Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>>
>> Alex Hung (13):
>>   drm/colorop: define a new macro for_each_new_colorop_in_state
>>   drm/amd/display: Skip color pipeline initialization for cursor plane
>>   drm/amd/display: Add support for sRGB EOTF in DEGAM block
>>   drm/amd/display: Add support for sRGB Inverse EOTF in SHAPER block
>>   drm/amd/display: Add support for sRGB EOTF in BLND block
>>   drm/colorop: Add 1D Curve Custom LUT type
>>   drm/amd/display: add shaper and blend colorops for 1D Curve Custom LUT
>>   drm/amd/display: add 3x4 matrix colorop
>>   drm/colorop: Add mutliplier type
>>   drm/amd/display: add multiplier colorop
>>   drm/amd/display: Swap matrix and multiplier
>>   drm/colorop: Add 3D LUT supports to color pipeline
>>   drm/amd/display: add 3D LUT colorop
>>
>> Harry Wentland (31):
>>   drm: Add helper for conversion from signed-magnitude
>>   drm/vkms: Round fixp2int conversion in lerp_u16
>>   drm/vkms: Add kunit tests for VKMS LUT handling
>>   drm/doc/rfc: Describe why prescriptive color pipeline is needed
>>   drm/colorop: Introduce new drm_colorop mode object
>>   drm/colorop: Add TYPE property
>>   drm/colorop: Add 1D Curve subtype
>>   Documentation/gpu: document drm_colorop
>>   drm/colorop: Add BYPASS property
>>   drm/colorop: Add NEXT property
>>   drm/colorop: Add atomic state print for drm_colorop
>>   drm/plane: Add COLOR PIPELINE property
>>   drm/colorop: Add NEXT to colorop state print
>>   drm/vkms: Add enumerated 1D curve colorop
>>   drm/vkms: Add kunit tests for linear and sRGB LUTs
>>   drm/colorop: Introduce DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
>>   drm/colorop: Add 3x4 CTM type
>>   drm/vkms: Use s32 for internal color pipeline precision
>>   drm/vkms: add 3x4 matrix in color pipeline
>>   drm/tests: Add a few tests around drm_fixed.h
>>   drm/vkms: Add tests for CTM handling
>>   drm/colorop: pass plane_color_pipeline client cap to atomic check
>>   drm/amd/display: Ignore deprecated props when plane_color_pipeline set
>>   drm/amd/display: Add bypass COLOR PIPELINE
>>   drm/colorop: Add PQ 125 EOTF and its inverse
>>   drm/amd/display: Enable support for PQ 125 EOTF and Inverse
>>   drm/colorop: add BT2020/BT709 OETF and Inverse OETF
>>   drm/amd/display: Add support for BT.709 and BT.2020 TFs
>>   drm/colorop: Define LUT_1D interpolation
>>   drm/colorop: allow non-bypass colorops
>>   drm/amd/display: Add AMD color pipeline doc
>>
>>  Documentation/gpu/drm-kms.rst                 |  15 +
>>  Documentation/gpu/rfc/color_pipeline.rst      | 376 ++++++++
>>  Documentation/gpu/rfc/index.rst               |   3 +
>>  drivers/gpu/drm/Makefile                      |   1 +
>>  .../gpu/drm/amd/display/amdgpu_dm/Makefile    |   3 +-
>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   4 +
>>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 581 ++++++++++++-
>>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 196 +++++
>>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.h |  36 +
>>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  32 +
>>  drivers/gpu/drm/drm_atomic.c                  | 184 +++-
>>  drivers/gpu/drm/drm_atomic_helper.c           |  12 +
>>  drivers/gpu/drm/drm_atomic_state_helper.c     |   5 +
>>  drivers/gpu/drm/drm_atomic_uapi.c             | 175 ++++
>>  drivers/gpu/drm/drm_colorop.c                 | 562 ++++++++++++
>>  drivers/gpu/drm/drm_ioctl.c                   |   7 +
>>  drivers/gpu/drm/drm_mode_config.c             |   7 +
>>  drivers/gpu/drm/drm_plane.c                   |  52 ++
>>  drivers/gpu/drm/tests/Makefile                |   3 +-
>>  drivers/gpu/drm/tests/drm_fixp_test.c         |  69 ++
>>  drivers/gpu/drm/vkms/Kconfig                  |   5 +
>>  drivers/gpu/drm/vkms/Makefile                 |   4 +-
>>  drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
>>  drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 449 ++++++++++
>>  drivers/gpu/drm/vkms/vkms_colorop.c           | 102 +++
>>  drivers/gpu/drm/vkms/vkms_composer.c          | 115 ++-
>>  drivers/gpu/drm/vkms/vkms_drv.h               |   8 +
>>  drivers/gpu/drm/vkms/vkms_luts.c              | 802 ++++++++++++++++++
>>  drivers/gpu/drm/vkms/vkms_luts.h              |  12 +
>>  drivers/gpu/drm/vkms/vkms_plane.c             |   2 +
>>  include/drm/drm_atomic.h                      | 130 +++
>>  include/drm/drm_atomic_uapi.h                 |   3 +
>>  include/drm/drm_colorop.h                     | 427 ++++++++++
>>  include/drm/drm_file.h                        |   7 +
>>  include/drm/drm_fixed.h                       |  18 +
>>  include/drm/drm_mode_config.h                 |  18 +
>>  include/drm/drm_plane.h                       |  19 +
>>  include/uapi/drm/amdgpu_drm.h                 |   9 -
>>  include/uapi/drm/drm.h                        |  16 +
>>  include/uapi/drm/drm_mode.h                   | 156 ++++
>>  40 files changed, 4595 insertions(+), 34 deletions(-)
>>  create mode 100644 Documentation/gpu/rfc/color_pipeline.rst
>>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
>>  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.h
>>  create mode 100644 drivers/gpu/drm/drm_colorop.c
>>  create mode 100644 drivers/gpu/drm/tests/drm_fixp_test.c
>>  create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
>>  create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>>  create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c
>>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.c
>>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.h
>>  create mode 100644 include/drm/drm_colorop.h
>>
>> --
>> 2.46.0
>>