Re: [PATCH V11 06/47] drm/colorop: Add 1D Curve subtype

[Shengyu Qu]

在 2025/9/26 2:22, Harry Wentland 写道:

On 2025-09-25 04:11, Pekka Paalanen wrote:

On Tue, 23 Sep 2025 11:41:24 -0600
Alex Hung <alex.h...@amd.com> wrote:

On 9/23/25 10:16, Alex Hung wrote:

On 9/23/25 01:59, Pekka Paalanen wrote:

On Mon, 22 Sep 2025 21:16:45 -0600
Alex Hung <alex.h...@amd.com> wrote:

On 9/18/25 02:40, Pekka Paalanen wrote:

...

The problem is that "H.273 TransferCharacteristics code point 13" a.k.a

the sRGB curve means different things for different people (two-piece
vs. power-2.2).

The difference is minor but visible, and therefore I would not make
two-piece and power-2.2 equivalent nor have one approximated by the
other.

They both need their own entries in the enum. Let's leave any decision
about whether substituting one for the other is ok to the userspace.

            */
           DRM_COLOROP_1D_CURVE_SRGB_EOTF,


It is also possible to add GAMMA 2.2 in addition to sRGB piece-wise

not be used at all, right?

EOTF. But if I understand correctly, DRM_COLOROP_1D_CURVE_SRGB_EOTF may

If hardware implements the two-piece curve, then there is reason to
expose it, especially when it does not implement power-2.2. Userspace
can choose to use it as an approximation when that is appropriate.


Thanks,
pq

Does the following diff make sense?

Yes.

1. Change "sRGB EOTF" -> "Piece-wise EOTF"

I think simply naming it "Piece-wise EOTF" is problematic
as it doesn't say anything about the curve. It's one of
the curves that are used for sRGB (the other being the
gamma/power 2.2 curve). I'd suggest we keep sRGB in the
name.

Otherwise I agree with advertising both the piece-wise
sRGB curve, as well as the Gamma (or Power) 2.2 curve.

A mathematical description of the curves would be good
but not a blocker to merge this, IMO.

Harry

Shengyu

I also agree that we need to keep sRGB in the name, since BT.709 has a slightly different piece-wise curve.

2. Add "Gamma 2.2"


index e1b2b446faf2..823e39b8f3fe 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -71,12 +71,13 @@ static const struct drm_prop_enum_list
drm_colorop_type_enum_list[] = {
   };

   static const char * const colorop_curve_1d_type_names[] = {
-    [DRM_COLOROP_1D_CURVE_SRGB_EOTF] = "sRGB EOTF",
+    [DRM_COLOROP_1D_CURVE_SRGB_EOTF] = "Piece-wise EOTF",
       [DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF] = "sRGB Inverse EOTF",
       [DRM_COLOROP_1D_CURVE_PQ_125_EOTF] = "PQ 125 EOTF",
       [DRM_COLOROP_1D_CURVE_PQ_125_INV_EOTF] = "PQ 125 Inverse EOTF",
       [DRM_COLOROP_1D_CURVE_BT2020_INV_OETF] = "BT.2020 Inverse OETF",
       [DRM_COLOROP_1D_CURVE_BT2020_OETF] = "BT.2020 OETF",
+    [DRM_COLOROP_1D_CURVE_GAMMA22] = "Gamma 2.2",

diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/ drm_colorop.c

If I wanted to really nitpick, I'd propose "Power 2.2" instead of
"Gamma 2.2", but I suppose it's clear anyway. And Wayland already went
with GAMMA22.

   };

   static const struct drm_prop_enum_list
drm_colorop_lut1d_interpolation_list[] = {
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 3e70f66940e0..3428a27cd9ad 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.hsRGB EOTF
@@ -43,12 +43,9 @@ enum drm_colorop_curve_1d_type {
       /**
        * @DRM_COLOROP_1D_CURVE_SRGB_EOTF:
        *
-     * enum string "sRGB EOTF"
+     * enum string "Piece-wise EOTF"
        *
-     * sRGB piece-wise electro-optical transfer function. Transfer
-     * characteristics as defined by IEC 61966-2-1 sRGB. Equivalent
-     * to H.273 TransferCharacteristics code point 13 with
-     * MatrixCoefficients set to 0.
+     * sRGB piece-wise electro-optical transfer function.
        */
       DRM_COLOROP_1D_CURVE_SRGB_EOTF,

@@ -108,6 +105,16 @@ enum drm_colorop_curve_1d_type {
        */
       DRM_COLOROP_1D_CURVE_BT2020_OETF,

+    /**
+     * @DRM_COLOROP_1D_CURVE_GAMMA22:
+     *
+     * enum string "Gamma 2.2"
+     *
+     * A gamma 2.2 power function. This applies a power curve with
+     * gamma value of 2.2 to the input values.

I'd prefer "exponent" rather than "gamma value" to be more explicit.
Just in case. There is quite some confusion around the term "gamma".
I've used to call this function a "pure power-low with exponent 2.2" to
explain that there are no offsets or multipliers or anything else.

For documentation it would best to write down the mathematical formula
rather than describe it in words. I understand that may be problematic
in kerneldoc, and we didn't do it in Wayland XML either but in a
proposed Gitlab-Markdown appendix.

+     */
+    DRM_COLOROP_1D_CURVE_GAMMA22,
+
       /**
        * @DRM_COLOROP_1D_CURVE_COUNT:
        *

Both DRM_COLOROP_1D_CURVE_SRGB_EOTF and DRM_COLOROP_1D_CURVE_GAMMA22 are
defined and it should be clear that sRGB EOTF are piece-wise TF and
Gamma 2.2 is for power 2.2. Is it still a concern of using "sRGB" for as
the original patch?

Not enough of a concern for me to continue nagging about it. :-)

More precisely, adding DRM_COLOROP_1D_CURVE_GAMMA22 with "Gamma 2.2"
string without touching "sRGB EOTF" should be sufficient. If a userspace
need to choose one or another it can precisely do so.

As long as everyone reads the documentation, and the documentation is
clear. I failed at clarity in Wayland, so I'm perhaps a bit paranoid
here.


Thanks,
pq

OpenPGP_0xE3520CC91929C8E7.asc
Description: OpenPGP public key

OpenPGP_signature.asc
Description: OpenPGP digital signature