On Fri, 8 Sep 2023 11:02:26 -0400 Harry Wentland <harry.wentl...@amd.com> wrote:
> Signed-off-by: Harry Wentland <harry.wentl...@amd.com>
> Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> Cc: Pekka Paalanen <pekka.paala...@collabora.com>
> Cc: Simon Ser <cont...@emersion.fr>
> Cc: Harry Wentland <harry.wentl...@amd.com>
> Cc: Melissa Wen <m...@igalia.com>
> Cc: Jonas Ådahl <jad...@redhat.com>
> Cc: Sebastian Wick <sebastian.w...@redhat.com>
> Cc: Shashank Sharma <shashank.sha...@amd.com>
> Cc: Alexander Goins <ago...@nvidia.com>
> Cc: Joshua Ashton <jos...@froggi.es>
> Cc: Michel Dänzer <mdaen...@redhat.com>
> Cc: Aleix Pol <aleix...@kde.org>
> Cc: Xaver Hugl <xaver.h...@gmail.com>
> Cc: Victoria Brekenfeld <victo...@system76.com>
> Cc: Daniel Vetter <dan...@ffwll.ch>
> Cc: Uma Shankar <uma.shan...@intel.com>
> Cc: Naseer Ahmed <quic_nas...@quicinc.com>
> Cc: Christopher Braga <quic_cbr...@quicinc.com>
> ---
> Documentation/gpu/rfc/color_pipeline.rst | 278 +++++++++++++++++++++++
> 1 file changed, 278 insertions(+)
> create mode 100644 Documentation/gpu/rfc/color_pipeline.rst
Hi Harry,
it's really nice to see this!
Sebastian started on the backward/forward compatibility, so I'll
comment on everything else here, and leave the compatibility for that
thread.
> diff --git a/Documentation/gpu/rfc/color_pipeline.rst
> b/Documentation/gpu/rfc/color_pipeline.rst
> new file mode 100644
> index 000000000000..bfa4a8f12087
> --- /dev/null
> +++ b/Documentation/gpu/rfc/color_pipeline.rst
> @@ -0,0 +1,278 @@
> +========================
> +Linux Color Pipeline API
> +========================
> +
> +What problem are we solving?
> +============================
> +
> +We would like to support pre-, and post-blending complex color
> transformations
+in display controller hardware
> +in order to allow for HW-supported HDR use-cases, as well as to provide
> support
> +to color-managed applications, such as video or image editors.
> +
> +While it is possible to support an HDR output on HW supporting the Colorspace
> +and HDR Metadata drm_connector properties that requires the compositor or
> +application to render and compose the content into one final buffer intended
> for
> +display. Doing so is costly.
I think a tiny re-wording would make it easier to read:
+~While i~*I*t is possible to support an HDR output on HW supporting the
Colorspace
+and HDR Metadata drm_connector properties*, but* that requires the compositor
or
+application to render and compose the content into one final buffer intended
for
+display. Doing so is costly.
deletion ~~
addition **
> +
> +Most modern display HW offers various 1D LUTs, 3D LUTs, matrices, and other
> +operations to support color transformations. These operations are often
> +implemented in fixed-function HW and therefore much more power efficient than
> +performing similar operations via shaders or CPU.
> +
> +We would like to make use of this HW functionality to support complex color
> +transformations with no, or minimal CPU or shader load.
> +
> +
> +How are other OSes solving this problem?
> +========================================
> +
> +The most widely supported use-cases regard HDR content, whether video or
> +gaming.
> +
> +Most OSes will specify the source content format (color gamut, encoding
> transfer
> +function, and other metadata, such as max and average light levels) to a
> driver.
> +Drivers will then program their fixed-function HW accordingly to map from a
> +source content buffer's space to a display's space.
> +
> +When fixed-function HW is not available the compositor will assemble a
> shader to
> +ask the GPU to perform the transformation from the source content format to
> the
> +display's format.
> +
> +A compositor's mapping function and a driver's mapping function are usually
> +entirely separate concepts. On OSes where a HW vendor has no insight into
> +closed-source compositor code such a vendor will tune their color management
> +code to visually match the compositor's. On other OSes, where both mapping
> +functions are open to an implementer they will ensure both mappings match.
> +
I'd add, assuming it's true:
This results in mapping algorithm lock-in, meaning that no-one alone can
experiment with or introduce new mapping algorithms and achieve
consistent results regardless of which implementation path is taken.
> +
> +Why is Linux different?
> +=======================
> +
> +Unlike other OSes, where there is one compositor for one or more drivers, on
> +Linux we have a many-to-many relationship. Many compositors; many drivers.
> +In addition each compositor vendor or community has their own view of how
> +color management should be done. This is what makes Linux so beautiful.
> +
> +This means that a HW vendor can now no longer tune their driver to one
> +compositor, as tuning it to one will almost inevitably make it look very
> +different from another compositor's color mapping.
This is easy to misunderstand as "all Linux desktops will get your
colors different so you cannot have a consistent look in an app". That
might trigger a few flamewars, even though it is a true goal in essence.
Maybe "almost inevitably" should be worded much more uncertain. Maybe
just "could make it look fairly different".
Much of color management is about user preferences. Different desktops
may have different sets of tunables, like different monitors and TV
have different tunables and color modes. It is still also an active
research area, and new image formats and new ways of driving displays
will surely emerge.
Different use cases have different fundamental goals with color
management, which warrants the "different colors". Linux should be
applicable to a wide range of use cases.
If these thoughts give you ideas how to rewrite this section, go for
it, but it could also be enough to change the couple words I suggested.
> +
> +We need a better solution.
> +
> +
> +Descriptive API
> +===============
> +
> +An API that describes the source and destination colorspaces is a descriptive
> +API. It describes the input and output color spaces but does not describe
> +how precisely they should be mapped. Such a mapping includes many minute
> +design decision that can greatly affect the look of the final result.
> +
> +It is not feasible to describe such mapping with enough detail to ensure the
> +same result from each implementation. In fact, these mappings are a very
> active
> +research area.
> +
> +
> +Prescriptive API
> +================
> +
> +A prescriptive API describes not the source and destination colorspaces. It
> +instead prescribes a recipe for how to manipulate pixel values to arrive at
> the
> +desired outcome.
> +
> +This recipe is generally an order straight-forward operations, with clear
Is this line missing some words?
> +mathematical definitions, such as 1D LUTs, 3D LUTs, matrices, or other
> +operations that can be described in a precise manner.
> +
> +
> +The Color Pipeline API
> +======================
> +
> +HW color management pipelines can significantly differ between HW
> +vendors in terms of availability, ordering, and capabilities of HW
> +blocks. This makes a common definition of color management blocks and
> +their ordering nigh impossible. Instead we are defining an API that
> +allows user space to discover the HW capabilities.
+in a generic manner, agnostic of specific drivers and hardware.
> +
> +
> +drm_colorop Object & IOCTLs
> +===========================
> +
> +To support the definition of color pipelines we introduce a new DRM core
"we define the DRM core object type drm_colorop."
I think that reads better after being merged upstream.
> +object, a drm_colorop. Individual drm_colorop objects will be chained
> +via the NEXT property of a drm_colorop to constitute a color pipeline.
> +Each drm_colorop object is unique, i.e., even if multiple color
> +pipelines have the same operation they won't share the same drm_colorop
> +object to describe that operation.
Maybe add some words here how drivers are not expected to map a
drm_colorop object statically to a specific HW block?
If someone was to assume the contrary, they would be wondering how they
can ever expose multiple pipelines.
The mapping between drm_colorop objects and HW blocks is completely a
driver internal detail, and can be as dynamic or static as the driver
needs it to be.
Speaking of drivers, I remember recently writing an email about what
would be a good way to expose HW functionality as drm_colorops.
Something about that would be good to document for driver writers,
maybe in a section of its own:
- Try to expose pipelines that use already defined colorops, even if
your hardware pipeline is split differently. This allows existing
userspace to immediately take advantage of the hardware.
- Additionally, try to expose your actual hardware blocks as colorops.
Define new colorop types where you believe it can offer significant
benefits if userspace learns to program them.
- Avoid defining new colorops for compound operations with very narrow
scope. If you have a hardware block for a special operation that
cannot be split further, you can expose that as a new colorop type.
However, try to not define colorops for "use cases", especially if
they require you to combine multiple hardware blocks.
- Design new colorops as prescriptive, not descriptive; by the
mathematical formula, not by the assumed input and output.
A defined colorop type must be deterministic. Its operation can depend
only on its properties (and input?) and nothing else, allowed error
tolerance notwithstanding.
(By input I'm thinking of a block that maintains some state from
previous frame color statistics, and adjusts its behaviour.)
> +
> +Just like other DRM objects the drm_colorop objects are discovered via
> +IOCTLs:
> +
> +DRM_IOCTL_MODE_GETCOLOROPRESOURCES: This IOCTL is used to retrieve the
> +number of all drm_colorop objects.
What is this useful for? Isn't the COLOR_PIPELINE plane property enough
to discover everything?
> +DRM_IOCTL_MODE_GETCOLOROP: This IOCTL is used to read one drm_colorop.
> +It includes the ID for the colorop object, as well as the plane_id of
> +the associated plane. All other values should be registered as
> +properties.
Why should plane_id not be a property? Could we remove GETCOLOROP
completely?
> +
> +Each drm_colorop has three core properties:
> +
> +TYPE: The type of transformation, such as
> +* enumerated curve
> +* custom (uniform) 1D LUT
> +* 3x3 matrix
> +* 3x4 matrix
> +* 3D LUT
> +* etc.
> +
> +Depending on the type of transformation other properties will describe
> +more details.
> +
> +BYPASS: A boolean property that can be used to easily put a block into
> +bypass mode. While setting other properties might fail atomic check,
> +setting the BYPASS property to true should never fail. This allows DRM
> +clients to fallback to other methods of color management if an atomic
> +check for KMS color operations fails.
> +
> +NEXT: The ID of the next drm_colorop in a color pipeline, or 0 if this
> +drm_colorop is the last in the chain.
> +
> +An example of a drm_colorop object might look like one of these::
> +
> + Color operation 42
> + ├─ "type": enum {Bypass, 1D curve} = 1D curve
> + ├─ "1d_curve_type": enum {LUT, sRGB, PQ, BT.709, HLG, …} = LUT
> + ├─ "lut_size": immutable range = 4096
> + ├─ "lut_data": blob
> + └─ "next": immutable color operation ID = 43
> +
> + Color operation 42
> + ├─ "type": enum {Bypass, 3D LUT} = 3D LUT
> + ├─ "lut_size": immutable range = 33
> + ├─ "lut_data": blob
> + └─ "next": immutable color operation ID = 43
> +
> + Color operation 42
> + ├─ "type": enum {Bypass, Matrix} = Matrix
> + ├─ "matrix_data": blob
> + └─ "next": immutable color operation ID = 43
> +
> +
> +COLOR_PIPELINE Plane Property
> +=============================
> +
> +Because we don't have existing KMS color properties in the pre-blending
> +portion of display pipelines (i.e. on drm_planes) we are introducing
> +color pipelines here first. Eventually we'll want to use the same
> +concept for the post-blending portion, i.e. drm_crtcs.
This paragraph might fit better in a cover letter.
> +
> +Color Pipelines are created by a driver and advertised via a new
> +COLOR_PIPELINE enum property on each plane. Values of the property
> +always include '0', which is the default and means all color processing
> +is disabled. Additional values will be the object IDs of the first
> +drm_colorop in a pipeline. A driver can create and advertise none, one,
> +or more possible color pipelines. A DRM client will select a color
> +pipeline by setting the COLOR PIPELINE to the respective value.
> +
> +In the case where drivers have custom support for pre-blending color
> +processing those drivers shall reject atomic commits that are trying to
> +set both the custom color properties, as well as the COLOR_PIPELINE
s/set/use/ because one of them could be carried-over state from
previous commits while not literally set in this one.
> +property.
> +
> +An example of a COLOR_PIPELINE property on a plane might look like this::
> +
> + Plane 10
> + ├─ "type": immutable enum {Overlay, Primary, Cursor} = Primary
> + ├─ …
> + └─ "color_pipeline": enum {0, 42, 52} = 0
Enum values are string names. I presume the intention here is that the
strings will never need to be parsed, and the uint64_t is always equal
to the string representation, right?
That needs a statement here. It differs from all previous uses of
enums, and e.g. requires a little bit of new API in libweston's
DRM-backend to handle since it has its own enums referring to the
string names that get mapped to the uint64_t per owning KMS object.
> +
> +
> +Color Pipeline Discovery
> +========================
> +
> +A DRM client wanting color management on a drm_plane will:
> +
> +1. Read all drm_colorop objects
What does this do?
> +2. Get the COLOR_PIPELINE property of the plane
> +3. iterate all COLOR_PIPELINE enum values
> +4. for each enum value walk the color pipeline (via the NEXT pointers)
> + and see if the available color operations are suitable for the
> + desired color management operations
> +
> +An example of chained properties to define an AMD pre-blending color
> +pipeline might look like this::
> +
> + Plane 10
> + ├─ "type": immutable enum {Overlay, Primary, Cursor} = Primary
> + └─ "color_pipeline": enum {0, 42} = 0
> + Color operation 42 (input CSC)
I presume the string "(input CSC)" does not come from KMS, and is
actually just a comment added here by hand?
Thanks,
pq
> + ├─ "type": enum {Bypass, Matrix} = Matrix
> + ├─ "matrix_data": blob
> + └─ "next": immutable color operation ID = 43
> + Color operation 43
> + ├─ "type": enum {Scaling} = Scaling
> + └─ "next": immutable color operation ID = 44
> + Color operation 44 (DeGamma)
> + ├─ "type": enum {Bypass, 1D curve} = 1D curve
> + ├─ "1d_curve_type": enum {sRGB, PQ, …} = sRGB
> + └─ "next": immutable color operation ID = 45
> + Color operation 45 (gamut remap)
> + ├─ "type": enum {Bypass, Matrix} = Matrix
> + ├─ "matrix_data": blob
> + └─ "next": immutable color operation ID = 46
> + Color operation 46 (shaper LUT RAM)
> + ├─ "type": enum {Bypass, 1D curve} = 1D curve
> + ├─ "1d_curve_type": enum {LUT} = LUT
> + ├─ "lut_size": immutable range = 4096
> + ├─ "lut_data": blob
> + └─ "next": immutable color operation ID = 47
> + Color operation 47 (3D LUT RAM)
> + ├─ "type": enum {Bypass, 3D LUT} = 3D LUT
> + ├─ "lut_size": immutable range = 17
> + ├─ "lut_data": blob
> + └─ "next": immutable color operation ID = 48
> + Color operation 48 (blend gamma)
> + ├─ "type": enum {Bypass, 1D curve} = 1D curve
> + ├─ "1d_curve_type": enum {LUT, sRGB, PQ, …} = LUT
> + ├─ "lut_size": immutable range = 4096
> + ├─ "lut_data": blob
> + └─ "next": immutable color operation ID = 0
> +
> +
> +Color Pipeline Programming
> +==========================
> +
> +Once a DRM client has found a suitable pipeline it will:
> +
> +1. Set the COLOR_PIPELINE enum value to the one pointing at the first
> + drm_colorop object of the desired pipeline
> +2. Set the properties for all drm_colorop objects in the pipeline to the
> + desired values, setting BYPASS to true for unused drm_colorop blocks,
> + and false for enabled drm_colorop blocks
> +3. Perform atomic_check/commit as desired
> +
> +To configure the pipeline for an HDR10 PQ plane and blending in linear
> +space, a compositor might perform an atomic commit with the following
> +property values::
> +
> + Plane 10
> + └─ "color_pipeline" = 42
> + Color operation 42 (input CSC)
> + └─ "bypass" = true
> + Color operation 44 (DeGamma)
> + └─ "bypass" = true
> + Color operation 45 (gamut remap)
> + └─ "bypasse" = true
> + Color operation 46 (shaper LUT RAM)
> + └─ "bypass" = true
> + Color operation 47 (3D LUT RAM)
> + └─ "lut_data" = Gamut mapping + tone mapping + night mode
> + Color operation 48 (blend gamma)
> + └─ "1d_curve_type" = PQ inverse EOTF
> +
> +
> +References
> +==========
> +
> +1.
> https://lore.kernel.org/dri-devel/QMers3awXvNCQlyhWdTtsPwkp5ie9bze_hD5nAccFW7a_RXlWjYB7MoUW_8CKLT2bSQwIXVi5H6VULYIxCdgvryZoAoJnC5lZgyK1QWn488=@emersion.fr/
> \ No newline at end of file
pgpmp6POwkHyR.pgp
Description: OpenPGP digital signature
