diff mbox series

[02/21] drm: Add Plane Degamma Mode property

Message ID 20210601105218.29185-3-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series Add Support for Plane Color Lut and CSC features | expand

Commit Message

Shankar, Uma June 1, 2021, 10:51 a.m. UTC
Add Plane Degamma Mode as an enum property. Create a helper
function for all plane color management features.

This is an enum property with values as blob_id's and exposes
the various gamma modes supported and the lut ranges. Getting
the blob id in userspace, user can get the mode supported and
also the range of gamma mode supported with number of lut
coefficients. It can then set one of the modes using this
enum property.

Lut values will be sent through separate GAMMA_LUT blob property.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
 drivers/gpu/drm/drm_atomic.c              |  1 +
 drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
 drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
 drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
 include/drm/drm_mode_object.h             |  2 +-
 include/drm/drm_plane.h                   | 23 ++++++
 7 files changed, 212 insertions(+), 3 deletions(-)

Comments

Harry Wentland June 4, 2021, 6:24 p.m. UTC | #1
On 2021-06-01 6:51 a.m., Uma Shankar wrote:
> Add Plane Degamma Mode as an enum property. Create a helper
> function for all plane color management features.
> 
> This is an enum property with values as blob_id's and exposes
> the various gamma modes supported and the lut ranges. Getting
> the blob id in userspace, user can get the mode supported and
> also the range of gamma mode supported with number of lut
> coefficients. It can then set one of the modes using this
> enum property.
> 
> Lut values will be sent through separate GAMMA_LUT blob property.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic.c              |  1 +
>  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
>  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
>  drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
>  include/drm/drm_mode_object.h             |  2 +-
>  include/drm/drm_plane.h                   | 23 ++++++
>  7 files changed, 212 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 87e5023e3f55..752be545e7d7 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -514,9 +514,99 @@ Damage Tracking Properties
>  Color Management Properties
>  ---------------------------
>  
> +Below is how a typical hardware pipeline for color
> +will look like:
> +
> +.. kernel-render:: DOT
> +   :alt: Display Color Pipeline
> +   :caption: Display Color Pipeline Overview
> +
> +   digraph "KMS" {
> +      node [shape=box]
> +
> +      subgraph cluster_static {
> +          style=dashed
> +          label="Display Color Hardware Blocks"
> +
> +          node [bgcolor=grey style=filled]
> +          "Plane Degamma A" -> "Plane CSC/CTM A"
> +          "Plane CSC/CTM A" -> "Plane Gamma A"
> +          "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];
> +          "Plane Gamma A" -> "Pipe Blender"
> +	  "Pipe Blender" -> "Pipe DeGamma"
> +          "Pipe DeGamma" -> "Pipe CSC/CTM"
> +          "Pipe CSC/CTM" -> "Pipe Gamma"
> +          "Pipe Gamma" -> "Pipe Output"
> +      }
> +

It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline,
between the FB and Plane degamma, i.e.
  YCbCr coefficients > plane degamma > csc > ...

One problem with this view is that not all HW will support all (or any) of these
CM blocks on all planes. For example, on AMD HW cursors are very different from
other planes and don't really have full CM support.

> +      subgraph cluster_static {
> +          style=dashed
> +
> +          node [shape=box]
> +          "Plane Degamma B" -> "Plane CSC/CTM B"
> +          "Plane CSC/CTM B" -> "Plane Gamma B"
> +          "Plane Gamma B" -> "Pipe Blender"
> +      }
> +
> +      subgraph cluster_static {
> +          style=dashed
> +
> +          node [shape=box]
> +          "Plane Degamma C" -> "Plane CSC/CTM C"
> +          "Plane CSC/CTM C" -> "Plane Gamma C"
> +          "Plane Gamma C" -> "Pipe Blender"
> +      }
> +
> +      subgraph cluster_fb {
> +          style=dashed
> +          label="RAM"
> +
> +          node [shape=box width=1.7 height=0.2]
> +
> +          "FB 1" -> "Plane Degamma A"
> +          "FB 2" -> "Plane Degamma B"
> +          "FB 3" -> "Plane Degamma C"
> +      }
> +   }
> +
> +In real world usecases,
> +
> +1. Plane Degamma can be used to linearize a non linear gamma
> +encoded framebuffer. This is needed to do any linear math like
> +color space conversion. For ex, linearize frames encoded in SRGB
> +or by HDR curve.
> +
> +2. Later Plane CTM block can convert the content to some different
> +colorspace. For ex, SRGB to BT2020 etc.
> +
> +3. Plane Gamma block can be used later to re-apply the non-linear
> +curve. This can also be used to apply Tone Mapping for HDR usecases.
> +

This would mean you're blending in gamma space which is likely not what
most compositors expect. There are numerous articles that describe why
blending in gamma space is problematic, such as [1]

[1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-blend.html

To blend in linear space this should be configured to do

  Plane Degamma > Plane CTM > CRTC Gamma

I think it would also be good if we moved away from calling this gamma. It's
really only gamma for legacy SDR scenarios. For HDR cases I would never expect
these to use gamma and even though the sRGB transfer function is based on gamma
functions it's complicated [2].

[2] https://en.wikipedia.org/wiki/SRGB

A better way to describe these would be as "transfer function" and "inverse
transfer function." The space at various stages could then be described as linear
or non-linear, specifically PQ, HLG, sRGB, BT709, or using another transfer
function.

Harry

> +All the layers or framebuffers need to be converted to same color
> +space and format before blending. The plane color hardware blocks
> +can help with this. Once the Data is blended, similar color processing
> +can be done on blended output using pipe color hardware blocks.
> +
> +DRM Properties have been created to define and expose all these
> +hardware blocks to userspace. A userspace application (compositor
> +or any color app) can use these interfaces and define policies to
> +efficiently use the display hardware for such color operations.
> +
> +Pipe Color Management Properties
> +---------------------------------
> +
>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>     :doc: overview
>  
> +Plane Color Management Properties
> +---------------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> +   :doc: Plane Color Properties
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> +   :doc: export
> +
>  Tile Group Property
>  -------------------
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index a8bbb021684b..8892d03602f7 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -708,6 +708,7 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
>  		   drm_get_color_encoding_name(state->color_encoding));
>  	drm_printf(p, "\tcolor-range=%s\n",
>  		   drm_get_color_range_name(state->color_range));
> +	drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
>  
>  	if (plane->funcs->atomic_print_state)
>  		plane->funcs->atomic_print_state(p, state);
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index ddcf5c2c8e6a..f26b03853711 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -311,6 +311,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  	state->fence = NULL;
>  	state->commit = NULL;
>  	state->fb_damage_clips = NULL;
> +
> +	state->color_mgmt_changed = false;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>  
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 438e9585b225..40fa05fa33dc 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -595,6 +595,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->color_encoding = val;
>  	} else if (property == plane->color_range_property) {
>  		state->color_range = val;
> +	} else if (property == plane->degamma_mode_property) {
> +		state->degamma_mode = val;
>  	} else if (property == config->prop_fb_damage_clips) {
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->fb_damage_clips,
> @@ -661,6 +663,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  		*val = state->color_encoding;
>  	} else if (property == plane->color_range_property) {
>  		*val = state->color_range;
> +	} else if (property == plane->degamma_mode_property) {
> +		*val = state->degamma_mode;
>  	} else if (property == config->prop_fb_damage_clips) {
>  		*val = (state->fb_damage_clips) ?
>  			state->fb_damage_clips->base.id : 0;
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index bb14f488c8f6..085ed0d0db00 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -34,8 +34,8 @@
>  /**
>   * DOC: overview
>   *
> - * Color management or color space adjustments is supported through a set of 5
> - * properties on the &drm_crtc object. They are set up by calling
> + * Pipe Color management or color space adjustments is supported through a
> + * set of 5 properties on the &drm_crtc object. They are set up by calling
>   * drm_crtc_enable_color_mgmt().
>   *
>   * "DEGAMMA_LUT”:
> @@ -584,6 +584,95 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
>  }
>  EXPORT_SYMBOL(drm_plane_create_color_properties);
>  
> +/**
> + * DOC: Plane Color Properties
> + *
> + * Plane Color management or color space adjustments is supported
> + * through a set of 5 properties on the &drm_plane object.
> + *
> + * degamma_mode_property:
> + *     Blob property which advertizes the possible degamma modes and
> + *     lut ranges supported by the platform. This  allows userspace
> + *     to query and get the plane degamma color caps and choose the
> + *     appropriate degamma mode and create lut values accordingly
> + *
> + */
> +int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
> +					   struct drm_plane *plane,
> +					   int num_values)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
> +				   "PLANE_DEGAMMA_MODE", num_values);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	plane->degamma_mode_property = prop;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
> +
> +void drm_plane_attach_degamma_properties(struct drm_plane *plane)
> +{
> +	if (!plane->degamma_mode_property)
> +		return;
> +
> +	drm_object_attach_property(&plane->base,
> +				   plane->degamma_mode_property, 0);
> +}
> +EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
> +
> +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
> +						 const char *name,
> +						 const struct drm_color_lut_range *ranges,
> +						 size_t length, enum lut_type type)
> +{
> +	struct drm_property_blob *blob;
> +	struct drm_property *prop = NULL;
> +	int num_ranges = length / sizeof(ranges[0]);
> +	int i, ret, num_types_0;
> +
> +	if (type == LUT_TYPE_DEGAMMA)
> +		prop = plane->degamma_mode_property;
> +
> +	if (!prop)
> +		return -EINVAL;
> +
> +	if (length == 0 && name)
> +		return drm_property_add_enum(prop, 0, name);
> +
> +	if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
> +		return -EINVAL;
> +	num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
> +			       DRM_MODE_LUT_DEGAMMA));
> +	if (num_types_0 == 0)
> +		return -EINVAL;
> +
> +	for (i = 1; i < num_ranges; i++) {
> +		int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA |
> +					 DRM_MODE_LUT_DEGAMMA));
> +
> +		/* either all ranges have DEGAMMA|GAMMA or none have it */
> +		if (num_types_0 != num_types)
> +			return -EINVAL;
> +	}
> +
> +	blob = drm_property_create_blob(plane->dev, length, ranges);
> +	if (IS_ERR(blob))
> +		return PTR_ERR(blob);
> +
> +	ret = drm_property_add_enum(prop, blob->base.id, name);
> +	if (ret) {
> +		drm_property_blob_put(blob);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_color_add_gamma_degamma_mode_range);
> +
>  /**
>   * drm_color_lut_check - check validity of lookup table
>   * @lut: property blob containing LUT to check
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index c34a3e8030e1..d4128c7daa08 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -60,7 +60,7 @@ struct drm_mode_object {
>  	void (*free_cb)(struct kref *kref);
>  };
>  
> -#define DRM_OBJECT_MAX_PROPERTY 24
> +#define DRM_OBJECT_MAX_PROPERTY 26
>  /**
>   * struct drm_object_properties - property tracking for &drm_mode_object
>   */
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 1294610e84f4..e476a5939f8e 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -236,6 +236,15 @@ struct drm_plane_state {
>  
>  	/** @state: backpointer to global drm_atomic_state */
>  	struct drm_atomic_state *state;
> +
> +	/**
> +	 * @degamma_mode: This is a blob_id and exposes the platform capabilities
> +	 * wrt to various gamma modes and the respective lut ranges. This also
> +	 * helps user select a degamma mode amongst the supported ones.
> +	 */
> +	u32 degamma_mode;
> +
> +	u8 color_mgmt_changed : 1;
>  };
>  
>  static inline struct drm_rect
> @@ -747,6 +756,12 @@ struct drm_plane {
>  	 * scaling.
>  	 */
>  	struct drm_property *scaling_filter_property;
> +
> +	/**
> +	 * @degamma_mode_property: Optional Plane property to set the LUT
> +	 * used to convert the framebuffer's colors to linear gamma.
> +	 */
> +	struct drm_property *degamma_mode_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> @@ -838,6 +853,14 @@ void drm_plane_force_disable(struct drm_plane *plane);
>  int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
>  				       struct drm_property *property,
>  				       uint64_t value);
> +int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
> +					   struct drm_plane *plane,
> +					   int num_values);
> +void drm_plane_attach_degamma_properties(struct drm_plane *plane);
> +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
> +						 const char *name,
> +						 const struct drm_color_lut_range *ranges,
> +						 size_t length, enum lut_type type);
>  
>  /**
>   * drm_plane_find - find a &drm_plane
>
Pekka Paalanen June 7, 2021, 11 a.m. UTC | #2
On Fri, 4 Jun 2021 14:24:28 -0400
Harry Wentland <harry.wentland@amd.com> wrote:

> On 2021-06-01 6:51 a.m., Uma Shankar wrote:
> > Add Plane Degamma Mode as an enum property. Create a helper
> > function for all plane color management features.
> > 
> > This is an enum property with values as blob_id's and exposes
> > the various gamma modes supported and the lut ranges. Getting
> > the blob id in userspace, user can get the mode supported and
> > also the range of gamma mode supported with number of lut
> > coefficients. It can then set one of the modes using this
> > enum property.
> > 
> > Lut values will be sent through separate GAMMA_LUT blob property.
> > 
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
> >  drivers/gpu/drm/drm_atomic.c              |  1 +
> >  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
> >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
> >  drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
> >  include/drm/drm_mode_object.h             |  2 +-
> >  include/drm/drm_plane.h                   | 23 ++++++
> >  7 files changed, 212 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 87e5023e3f55..752be545e7d7 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -514,9 +514,99 @@ Damage Tracking Properties
> >  Color Management Properties
> >  ---------------------------
> >  
> > +Below is how a typical hardware pipeline for color
> > +will look like:
> > +
> > +.. kernel-render:: DOT
> > +   :alt: Display Color Pipeline
> > +   :caption: Display Color Pipeline Overview
> > +
> > +   digraph "KMS" {
> > +      node [shape=box]
> > +
> > +      subgraph cluster_static {
> > +          style=dashed
> > +          label="Display Color Hardware Blocks"
> > +
> > +          node [bgcolor=grey style=filled]
> > +          "Plane Degamma A" -> "Plane CSC/CTM A"
> > +          "Plane CSC/CTM A" -> "Plane Gamma A"
> > +          "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];
> > +          "Plane Gamma A" -> "Pipe Blender"
> > +	  "Pipe Blender" -> "Pipe DeGamma"
> > +          "Pipe DeGamma" -> "Pipe CSC/CTM"
> > +          "Pipe CSC/CTM" -> "Pipe Gamma"
> > +          "Pipe Gamma" -> "Pipe Output"
> > +      }
> > +  
> 
> It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline,
> between the FB and Plane degamma, i.e.
>   YCbCr coefficients > plane degamma > csc > ...
> 
> One problem with this view is that not all HW will support all (or any) of these
> CM blocks on all planes. For example, on AMD HW cursors are very different from
> other planes and don't really have full CM support.
> 
> > +      subgraph cluster_static {
> > +          style=dashed
> > +
> > +          node [shape=box]
> > +          "Plane Degamma B" -> "Plane CSC/CTM B"
> > +          "Plane CSC/CTM B" -> "Plane Gamma B"
> > +          "Plane Gamma B" -> "Pipe Blender"
> > +      }
> > +
> > +      subgraph cluster_static {
> > +          style=dashed
> > +
> > +          node [shape=box]
> > +          "Plane Degamma C" -> "Plane CSC/CTM C"
> > +          "Plane CSC/CTM C" -> "Plane Gamma C"
> > +          "Plane Gamma C" -> "Pipe Blender"
> > +      }
> > +
> > +      subgraph cluster_fb {
> > +          style=dashed
> > +          label="RAM"
> > +
> > +          node [shape=box width=1.7 height=0.2]
> > +
> > +          "FB 1" -> "Plane Degamma A"
> > +          "FB 2" -> "Plane Degamma B"
> > +          "FB 3" -> "Plane Degamma C"
> > +      }
> > +   }
> > +
> > +In real world usecases,
> > +
> > +1. Plane Degamma can be used to linearize a non linear gamma
> > +encoded framebuffer. This is needed to do any linear math like
> > +color space conversion. For ex, linearize frames encoded in SRGB
> > +or by HDR curve.
> > +
> > +2. Later Plane CTM block can convert the content to some different
> > +colorspace. For ex, SRGB to BT2020 etc.
> > +
> > +3. Plane Gamma block can be used later to re-apply the non-linear
> > +curve. This can also be used to apply Tone Mapping for HDR usecases.
> > +  
> 
> This would mean you're blending in gamma space which is likely not what
> most compositors expect. There are numerous articles that describe why
> blending in gamma space is problematic, such as [1]
> 
> [1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-blend.html
> 
> To blend in linear space this should be configured to do
> 
>   Plane Degamma > Plane CTM > CRTC Gamma
> 
> I think it would also be good if we moved away from calling this gamma. It's
> really only gamma for legacy SDR scenarios. For HDR cases I would never expect
> these to use gamma and even though the sRGB transfer function is based on gamma
> functions it's complicated [2].
> 
> [2] https://en.wikipedia.org/wiki/SRGB
> 
> A better way to describe these would be as "transfer function" and "inverse
> transfer function." The space at various stages could then be described as linear
> or non-linear, specifically PQ, HLG, sRGB, BT709, or using another transfer
> function.

Hi,

incidentally, I, Sebastian and Vitaly are discussing on what to call
these pieces in Weston:
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/582#note_947292

They might not even be the transfer functions but just some curves,
e.g. optimized to keep a 3D LUT small without sacrificing precision.


Thanks,
pq

> > +All the layers or framebuffers need to be converted to same color
> > +space and format before blending. The plane color hardware blocks
> > +can help with this. Once the Data is blended, similar color processing
> > +can be done on blended output using pipe color hardware blocks.
> > +
> > +DRM Properties have been created to define and expose all these
> > +hardware blocks to userspace. A userspace application (compositor
> > +or any color app) can use these interfaces and define policies to
> > +efficiently use the display hardware for such color operations.
> > +
> > +Pipe Color Management Properties
> > +---------------------------------
> > +
> >  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> >     :doc: overview
> >  
> > +Plane Color Management Properties
> > +---------------------------------
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> > +   :doc: Plane Color Properties
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> > +   :doc: export
> > +
> >  Tile Group Property
> >  -------------------
Shankar, Uma June 7, 2021, 5:34 p.m. UTC | #3
> -----Original Message-----
> From: Harry Wentland <harry.wentland@amd.com>
> Sent: Friday, June 4, 2021 11:54 PM
> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org
> Cc: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; Cyr, Aric
> <Aric.Cyr@amd.com>
> Subject: Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
> 
> On 2021-06-01 6:51 a.m., Uma Shankar wrote:
> > Add Plane Degamma Mode as an enum property. Create a helper function
> > for all plane color management features.
> >
> > This is an enum property with values as blob_id's and exposes the
> > various gamma modes supported and the lut ranges. Getting the blob id
> > in userspace, user can get the mode supported and also the range of
> > gamma mode supported with number of lut coefficients. It can then set
> > one of the modes using this enum property.
> >
> > Lut values will be sent through separate GAMMA_LUT blob property.
> >
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
> >  drivers/gpu/drm/drm_atomic.c              |  1 +
> >  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
> >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
> >  drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
> >  include/drm/drm_mode_object.h             |  2 +-
> >  include/drm/drm_plane.h                   | 23 ++++++
> >  7 files changed, 212 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/gpu/drm-kms.rst
> > b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7
> > 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -514,9 +514,99 @@ Damage Tracking Properties  Color Management
> > Properties
> >  ---------------------------
> >
> > +Below is how a typical hardware pipeline for color will look like:
> > +
> > +.. kernel-render:: DOT
> > +   :alt: Display Color Pipeline
> > +   :caption: Display Color Pipeline Overview
> > +
> > +   digraph "KMS" {
> > +      node [shape=box]
> > +
> > +      subgraph cluster_static {
> > +          style=dashed
> > +          label="Display Color Hardware Blocks"
> > +
> > +          node [bgcolor=grey style=filled]
> > +          "Plane Degamma A" -> "Plane CSC/CTM A"
> > +          "Plane CSC/CTM A" -> "Plane Gamma A"
> > +          "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];
> > +          "Plane Gamma A" -> "Pipe Blender"
> > +	  "Pipe Blender" -> "Pipe DeGamma"
> > +          "Pipe DeGamma" -> "Pipe CSC/CTM"
> > +          "Pipe CSC/CTM" -> "Pipe Gamma"
> > +          "Pipe Gamma" -> "Pipe Output"
> > +      }
> > +
> 
> It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline,
> between the FB and Plane degamma, i.e.
>   YCbCr coefficients > plane degamma > csc > ...
> 
> One problem with this view is that not all HW will support all (or any) of these CM
> blocks on all planes. For example, on AMD HW cursors are very different from other
> planes and don't really have full CM support.
> 
> > +      subgraph cluster_static {
> > +          style=dashed
> > +
> > +          node [shape=box]
> > +          "Plane Degamma B" -> "Plane CSC/CTM B"
> > +          "Plane CSC/CTM B" -> "Plane Gamma B"
> > +          "Plane Gamma B" -> "Pipe Blender"
> > +      }
> > +
> > +      subgraph cluster_static {
> > +          style=dashed
> > +
> > +          node [shape=box]
> > +          "Plane Degamma C" -> "Plane CSC/CTM C"
> > +          "Plane CSC/CTM C" -> "Plane Gamma C"
> > +          "Plane Gamma C" -> "Pipe Blender"
> > +      }
> > +
> > +      subgraph cluster_fb {
> > +          style=dashed
> > +          label="RAM"
> > +
> > +          node [shape=box width=1.7 height=0.2]
> > +
> > +          "FB 1" -> "Plane Degamma A"
> > +          "FB 2" -> "Plane Degamma B"
> > +          "FB 3" -> "Plane Degamma C"
> > +      }
> > +   }
> > +
> > +In real world usecases,
> > +
> > +1. Plane Degamma can be used to linearize a non linear gamma encoded
> > +framebuffer. This is needed to do any linear math like color space
> > +conversion. For ex, linearize frames encoded in SRGB or by HDR curve.
> > +
> > +2. Later Plane CTM block can convert the content to some different
> > +colorspace. For ex, SRGB to BT2020 etc.
> > +
> > +3. Plane Gamma block can be used later to re-apply the non-linear
> > +curve. This can also be used to apply Tone Mapping for HDR usecases.
> > +
> 
> This would mean you're blending in gamma space which is likely not what most
> compositors expect. There are numerous articles that describe why blending in
> gamma space is problematic, such as [1]
> 
> [1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-
> blend.html
> 
> To blend in linear space this should be configured to do
> 
>   Plane Degamma > Plane CTM > CRTC Gamma
> 
> I think it would also be good if we moved away from calling this gamma. It's really
> only gamma for legacy SDR scenarios. For HDR cases I would never expect these to
> use gamma and even though the sRGB transfer function is based on gamma
> functions it's complicated [2].
> 
> [2] https://en.wikipedia.org/wiki/SRGB
> 
> A better way to describe these would be as "transfer function" and "inverse transfer
> function." The space at various stages could then be described as linear or non-
> linear, specifically PQ, HLG, sRGB, BT709, or using another transfer function.

Yeah I just kept the naming based on what we had for crtc to avoid ambiguity. But yes, we
can rename these to have them named generically, as gamma lut actually can be used for
Tone mapping as well and its not just meant for applying a fixed non liner curve only. However
naming based on transfer function is also not optimum as Tone mapping is not related to transfer
function. May be we can name them as:
linear_lut and nl_tone_mapper_lut.

Other ideas for names are welcome.

Regards,
Uma Shankar


> Harry
> 
> > +All the layers or framebuffers need to be converted to same color
> > +space and format before blending. The plane color hardware blocks can
> > +help with this. Once the Data is blended, similar color processing
> > +can be done on blended output using pipe color hardware blocks.
> > +
> > +DRM Properties have been created to define and expose all these
> > +hardware blocks to userspace. A userspace application (compositor or
> > +any color app) can use these interfaces and define policies to
> > +efficiently use the display hardware for such color operations.
> > +
> > +Pipe Color Management Properties
> > +---------------------------------
> > +
> >  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> >     :doc: overview
> >
> > +Plane Color Management Properties
> > +---------------------------------
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> > +   :doc: Plane Color Properties
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> > +   :doc: export
> > +
> >  Tile Group Property
> >  -------------------
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c
> > b/drivers/gpu/drm/drm_atomic.c index a8bbb021684b..8892d03602f7 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -708,6 +708,7 @@ static void drm_atomic_plane_print_state(struct
> drm_printer *p,
> >  		   drm_get_color_encoding_name(state->color_encoding));
> >  	drm_printf(p, "\tcolor-range=%s\n",
> >  		   drm_get_color_range_name(state->color_range));
> > +	drm_printf(p, "\tcolor_mgmt_changed=%d\n",
> > +state->color_mgmt_changed);
> >
> >  	if (plane->funcs->atomic_print_state)
> >  		plane->funcs->atomic_print_state(p, state); diff --git
> > a/drivers/gpu/drm/drm_atomic_state_helper.c
> > b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index ddcf5c2c8e6a..f26b03853711 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -311,6 +311,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct
> drm_plane *plane,
> >  	state->fence = NULL;
> >  	state->commit = NULL;
> >  	state->fb_damage_clips = NULL;
> > +
> > +	state->color_mgmt_changed = false;
> >  }
> >  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 438e9585b225..40fa05fa33dc 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -595,6 +595,8 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> >  		state->color_encoding = val;
> >  	} else if (property == plane->color_range_property) {
> >  		state->color_range = val;
> > +	} else if (property == plane->degamma_mode_property) {
> > +		state->degamma_mode = val;
> >  	} else if (property == config->prop_fb_damage_clips) {
> >  		ret = drm_atomic_replace_property_blob_from_id(dev,
> >  					&state->fb_damage_clips,
> > @@ -661,6 +663,8 @@ drm_atomic_plane_get_property(struct drm_plane
> *plane,
> >  		*val = state->color_encoding;
> >  	} else if (property == plane->color_range_property) {
> >  		*val = state->color_range;
> > +	} else if (property == plane->degamma_mode_property) {
> > +		*val = state->degamma_mode;
> >  	} else if (property == config->prop_fb_damage_clips) {
> >  		*val = (state->fb_damage_clips) ?
> >  			state->fb_damage_clips->base.id : 0; diff --git
> > a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index bb14f488c8f6..085ed0d0db00 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -34,8 +34,8 @@
> >  /**
> >   * DOC: overview
> >   *
> > - * Color management or color space adjustments is supported through a
> > set of 5
> > - * properties on the &drm_crtc object. They are set up by calling
> > + * Pipe Color management or color space adjustments is supported
> > + through a
> > + * set of 5 properties on the &drm_crtc object. They are set up by
> > + calling
> >   * drm_crtc_enable_color_mgmt().
> >   *
> >   * "DEGAMMA_LUT”:
> > @@ -584,6 +584,95 @@ int drm_plane_create_color_properties(struct
> > drm_plane *plane,  }
> > EXPORT_SYMBOL(drm_plane_create_color_properties);
> >
> > +/**
> > + * DOC: Plane Color Properties
> > + *
> > + * Plane Color management or color space adjustments is supported
> > + * through a set of 5 properties on the &drm_plane object.
> > + *
> > + * degamma_mode_property:
> > + *     Blob property which advertizes the possible degamma modes and
> > + *     lut ranges supported by the platform. This  allows userspace
> > + *     to query and get the plane degamma color caps and choose the
> > + *     appropriate degamma mode and create lut values accordingly
> > + *
> > + */
> > +int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
> > +					   struct drm_plane *plane,
> > +					   int num_values)
> > +{
> > +	struct drm_property *prop;
> > +
> > +	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
> > +				   "PLANE_DEGAMMA_MODE", num_values);
> > +	if (!prop)
> > +		return -ENOMEM;
> > +
> > +	plane->degamma_mode_property = prop;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
> > +
> > +void drm_plane_attach_degamma_properties(struct drm_plane *plane) {
> > +	if (!plane->degamma_mode_property)
> > +		return;
> > +
> > +	drm_object_attach_property(&plane->base,
> > +				   plane->degamma_mode_property, 0); }
> > +EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
> > +
> > +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane
> *plane,
> > +						 const char *name,
> > +						 const struct drm_color_lut_range
> *ranges,
> > +						 size_t length, enum lut_type type)
> {
> > +	struct drm_property_blob *blob;
> > +	struct drm_property *prop = NULL;
> > +	int num_ranges = length / sizeof(ranges[0]);
> > +	int i, ret, num_types_0;
> > +
> > +	if (type == LUT_TYPE_DEGAMMA)
> > +		prop = plane->degamma_mode_property;
> > +
> > +	if (!prop)
> > +		return -EINVAL;
> > +
> > +	if (length == 0 && name)
> > +		return drm_property_add_enum(prop, 0, name);
> > +
> > +	if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
> > +		return -EINVAL;
> > +	num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
> > +			       DRM_MODE_LUT_DEGAMMA));
> > +	if (num_types_0 == 0)
> > +		return -EINVAL;
> > +
> > +	for (i = 1; i < num_ranges; i++) {
> > +		int num_types = hweight8(ranges[i].flags &
> (DRM_MODE_LUT_GAMMA |
> > +					 DRM_MODE_LUT_DEGAMMA));
> > +
> > +		/* either all ranges have DEGAMMA|GAMMA or none have it */
> > +		if (num_types_0 != num_types)
> > +			return -EINVAL;
> > +	}
> > +
> > +	blob = drm_property_create_blob(plane->dev, length, ranges);
> > +	if (IS_ERR(blob))
> > +		return PTR_ERR(blob);
> > +
> > +	ret = drm_property_add_enum(prop, blob->base.id, name);
> > +	if (ret) {
> > +		drm_property_blob_put(blob);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_plane_color_add_gamma_degamma_mode_range);
> > +
> >  /**
> >   * drm_color_lut_check - check validity of lookup table
> >   * @lut: property blob containing LUT to check diff --git
> > a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index
> > c34a3e8030e1..d4128c7daa08 100644
> > --- a/include/drm/drm_mode_object.h
> > +++ b/include/drm/drm_mode_object.h
> > @@ -60,7 +60,7 @@ struct drm_mode_object {
> >  	void (*free_cb)(struct kref *kref);
> >  };
> >
> > -#define DRM_OBJECT_MAX_PROPERTY 24
> > +#define DRM_OBJECT_MAX_PROPERTY 26
> >  /**
> >   * struct drm_object_properties - property tracking for &drm_mode_object
> >   */
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
> > 1294610e84f4..e476a5939f8e 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -236,6 +236,15 @@ struct drm_plane_state {
> >
> >  	/** @state: backpointer to global drm_atomic_state */
> >  	struct drm_atomic_state *state;
> > +
> > +	/**
> > +	 * @degamma_mode: This is a blob_id and exposes the platform capabilities
> > +	 * wrt to various gamma modes and the respective lut ranges. This also
> > +	 * helps user select a degamma mode amongst the supported ones.
> > +	 */
> > +	u32 degamma_mode;
> > +
> > +	u8 color_mgmt_changed : 1;
> >  };
> >
> >  static inline struct drm_rect
> > @@ -747,6 +756,12 @@ struct drm_plane {
> >  	 * scaling.
> >  	 */
> >  	struct drm_property *scaling_filter_property;
> > +
> > +	/**
> > +	 * @degamma_mode_property: Optional Plane property to set the LUT
> > +	 * used to convert the framebuffer's colors to linear gamma.
> > +	 */
> > +	struct drm_property *degamma_mode_property;
> >  };
> >
> >  #define obj_to_plane(x) container_of(x, struct drm_plane, base) @@
> > -838,6 +853,14 @@ void drm_plane_force_disable(struct drm_plane
> > *plane);  int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
> >  				       struct drm_property *property,
> >  				       uint64_t value);
> > +int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
> > +					   struct drm_plane *plane,
> > +					   int num_values);
> > +void drm_plane_attach_degamma_properties(struct drm_plane *plane);
> > +int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane
> *plane,
> > +						 const char *name,
> > +						 const struct drm_color_lut_range
> *ranges,
> > +						 size_t length, enum lut_type type);
> >
> >  /**
> >   * drm_plane_find - find a &drm_plane
> >
Pekka Paalanen June 8, 2021, 8:34 a.m. UTC | #4
On Mon, 7 Jun 2021 17:34:06 +0000
"Shankar, Uma" <uma.shankar@intel.com> wrote:

> > -----Original Message-----
> > From: Harry Wentland <harry.wentland@amd.com>
> > Sent: Friday, June 4, 2021 11:54 PM
> > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> > devel@lists.freedesktop.org
> > Cc: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; Cyr, Aric
> > <Aric.Cyr@amd.com>
> > Subject: Re: [PATCH 02/21] drm: Add Plane Degamma Mode property
> > 
> > On 2021-06-01 6:51 a.m., Uma Shankar wrote:  
> > > Add Plane Degamma Mode as an enum property. Create a helper function
> > > for all plane color management features.
> > >
> > > This is an enum property with values as blob_id's and exposes the
> > > various gamma modes supported and the lut ranges. Getting the blob id
> > > in userspace, user can get the mode supported and also the range of
> > > gamma mode supported with number of lut coefficients. It can then set
> > > one of the modes using this enum property.
> > >
> > > Lut values will be sent through separate GAMMA_LUT blob property.
> > >
> > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > ---
> > >  Documentation/gpu/drm-kms.rst             | 90 ++++++++++++++++++++++
> > >  drivers/gpu/drm/drm_atomic.c              |  1 +
> > >  drivers/gpu/drm/drm_atomic_state_helper.c |  2 +
> > >  drivers/gpu/drm/drm_atomic_uapi.c         |  4 +
> > >  drivers/gpu/drm/drm_color_mgmt.c          | 93 ++++++++++++++++++++++-
> > >  include/drm/drm_mode_object.h             |  2 +-
> > >  include/drm/drm_plane.h                   | 23 ++++++
> > >  7 files changed, 212 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/gpu/drm-kms.rst
> > > b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..752be545e7d7
> > > 100644
> > > --- a/Documentation/gpu/drm-kms.rst
> > > +++ b/Documentation/gpu/drm-kms.rst
> > > @@ -514,9 +514,99 @@ Damage Tracking Properties  Color Management
> > > Properties
> > >  ---------------------------
> > >
> > > +Below is how a typical hardware pipeline for color will look like:
> > > +
> > > +.. kernel-render:: DOT
> > > +   :alt: Display Color Pipeline
> > > +   :caption: Display Color Pipeline Overview
> > > +
> > > +   digraph "KMS" {
> > > +      node [shape=box]
> > > +
> > > +      subgraph cluster_static {
> > > +          style=dashed
> > > +          label="Display Color Hardware Blocks"
> > > +
> > > +          node [bgcolor=grey style=filled]
> > > +          "Plane Degamma A" -> "Plane CSC/CTM A"
> > > +          "Plane CSC/CTM A" -> "Plane Gamma A"
> > > +          "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];
> > > +          "Plane Gamma A" -> "Pipe Blender"
> > > +	  "Pipe Blender" -> "Pipe DeGamma"
> > > +          "Pipe DeGamma" -> "Pipe CSC/CTM"
> > > +          "Pipe CSC/CTM" -> "Pipe Gamma"
> > > +          "Pipe Gamma" -> "Pipe Output"
> > > +      }
> > > +  
> > 
> > It might be worthwhile to also highlight the YCbCr coefficient matrix in the pipeline,
> > between the FB and Plane degamma, i.e.
> >   YCbCr coefficients > plane degamma > csc > ...
> > 
> > One problem with this view is that not all HW will support all (or any) of these CM
> > blocks on all planes. For example, on AMD HW cursors are very different from other
> > planes and don't really have full CM support.
> >   
> > > +      subgraph cluster_static {
> > > +          style=dashed
> > > +
> > > +          node [shape=box]
> > > +          "Plane Degamma B" -> "Plane CSC/CTM B"
> > > +          "Plane CSC/CTM B" -> "Plane Gamma B"
> > > +          "Plane Gamma B" -> "Pipe Blender"
> > > +      }
> > > +
> > > +      subgraph cluster_static {
> > > +          style=dashed
> > > +
> > > +          node [shape=box]
> > > +          "Plane Degamma C" -> "Plane CSC/CTM C"
> > > +          "Plane CSC/CTM C" -> "Plane Gamma C"
> > > +          "Plane Gamma C" -> "Pipe Blender"
> > > +      }
> > > +
> > > +      subgraph cluster_fb {
> > > +          style=dashed
> > > +          label="RAM"
> > > +
> > > +          node [shape=box width=1.7 height=0.2]
> > > +
> > > +          "FB 1" -> "Plane Degamma A"
> > > +          "FB 2" -> "Plane Degamma B"
> > > +          "FB 3" -> "Plane Degamma C"
> > > +      }
> > > +   }
> > > +
> > > +In real world usecases,
> > > +
> > > +1. Plane Degamma can be used to linearize a non linear gamma encoded
> > > +framebuffer. This is needed to do any linear math like color space
> > > +conversion. For ex, linearize frames encoded in SRGB or by HDR curve.
> > > +
> > > +2. Later Plane CTM block can convert the content to some different
> > > +colorspace. For ex, SRGB to BT2020 etc.
> > > +
> > > +3. Plane Gamma block can be used later to re-apply the non-linear
> > > +curve. This can also be used to apply Tone Mapping for HDR usecases.
> > > +  
> > 
> > This would mean you're blending in gamma space which is likely not what most
> > compositors expect. There are numerous articles that describe why blending in
> > gamma space is problematic, such as [1]
> > 
> > [1] https://ninedegreesbelow.com/photography/linear-gamma-blur-normal-
> > blend.html
> > 
> > To blend in linear space this should be configured to do
> > 
> >   Plane Degamma > Plane CTM > CRTC Gamma
> > 
> > I think it would also be good if we moved away from calling this gamma. It's really
> > only gamma for legacy SDR scenarios. For HDR cases I would never expect these to
> > use gamma and even though the sRGB transfer function is based on gamma
> > functions it's complicated [2].
> > 
> > [2] https://en.wikipedia.org/wiki/SRGB
> > 
> > A better way to describe these would be as "transfer function" and "inverse transfer
> > function." The space at various stages could then be described as linear or non-
> > linear, specifically PQ, HLG, sRGB, BT709, or using another transfer function.  
> 
> Yeah I just kept the naming based on what we had for crtc to avoid ambiguity. But yes, we
> can rename these to have them named generically, as gamma lut actually can be used for
> Tone mapping as well and its not just meant for applying a fixed non liner curve only. However
> naming based on transfer function is also not optimum as Tone mapping is not related to transfer
> function. May be we can name them as:
> linear_lut and nl_tone_mapper_lut.
> 
> Other ideas for names are welcome.

I would suggest to avoid any kind of naming that explicitly refers to
gamma, EOTF, EOTF^-1, tone map, or anything else that has a specific
meaning. Likewise, the diagram should not claim that the pixel data at
certain stages of the pipeline is in certain form, e.g. linear,
non-linear, or in certain color space. Doing that would semantically
restrict what userspace can do. After all, kernel UAPI design is
mechanism, not policy, right?

However, to make the pipeline understandable, the documentation could
show example configurations, where you *can* say that this LUT
implements EOTF, pixels are light-linear at that point, the color space
at another point is XXX, and so on.

What exactly the LUT is, are pixel values linear or non-linear, and
what the color space is at any step are something that userspace will
define through providing the parameters for each processing step in the
pipeline.

Unless, your hardware has been designed for a certain policy and
deviating from that policy should be discouraged?

So what to name these:

That's the question we are struggling with at Weston too. You can seek
inspiration from CMM data structures (e.g. Little CMS 2) or from ICC
specifications. Just need to be careful to not pick a term that has
more meaning than kernel UAPI wants.

I personally like the terms "color curve" for 1D LUT -like things, and
"color mapping" for matrix or 3D LUT -like things. I'm using the term
"color transformation" for going from one encoding, color space, gamut,
and dynamic range to another with a specific render intent, which may
require a sequence of several color curves and color mappings to
achieve. But that's just me.

Why bother with all this indirection:

Use cases that want blending in non-linear spaces are one thing, but
probably less useful with HDR. Instead, I would imagine that doing
numerical optimization on a complete pipeline to achieve the best
precision given hardware limitations or minimizing memory consumption
or other, may result in different curves and mapping tables than simply
plugging in the EOTFs and CSCs into their intended places.

I have actually started to think that when Weston's DRM-backend is
trying to fit a window on a KMS plane, it would need to get the color
pipeline capabilities of that specific plane and ask the color manager
component to optimize the color transformation against the exact KMS
capabilities, evaluating whether a sufficient precision can be
achieved. Then the color manager gives back a description of how the
KMS plane would need to be configured, and the DRM-backend continues
with an atomic TEST_ONLY commit to see if it would work. The color
manager would quite likely delegate that optimization to a CMM, like
Little CMS 2 if possible. But this is all still so much hand-waving and
the KMS UAPI is not really there yet, that I will mostly just ignore
this idea for now and do something much simpler.

Or maybe all this optimization has already been done when designing the
new hardware, and we really do need to simply plug in the EOTFs and CSCs
without thinking?

Then again, if hardware changes every generation, that implies the
optimization is not complete, and there might be something to gain from
software optimization still. Old hardware always exists, too.


Thanks,
pq
diff mbox series

Patch

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 87e5023e3f55..752be545e7d7 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -514,9 +514,99 @@  Damage Tracking Properties
 Color Management Properties
 ---------------------------
 
+Below is how a typical hardware pipeline for color
+will look like:
+
+.. kernel-render:: DOT
+   :alt: Display Color Pipeline
+   :caption: Display Color Pipeline Overview
+
+   digraph "KMS" {
+      node [shape=box]
+
+      subgraph cluster_static {
+          style=dashed
+          label="Display Color Hardware Blocks"
+
+          node [bgcolor=grey style=filled]
+          "Plane Degamma A" -> "Plane CSC/CTM A"
+          "Plane CSC/CTM A" -> "Plane Gamma A"
+          "Pipe Blender" [color=lightblue,style=filled, width=5.25, height=0.75];
+          "Plane Gamma A" -> "Pipe Blender"
+	  "Pipe Blender" -> "Pipe DeGamma"
+          "Pipe DeGamma" -> "Pipe CSC/CTM"
+          "Pipe CSC/CTM" -> "Pipe Gamma"
+          "Pipe Gamma" -> "Pipe Output"
+      }
+
+      subgraph cluster_static {
+          style=dashed
+
+          node [shape=box]
+          "Plane Degamma B" -> "Plane CSC/CTM B"
+          "Plane CSC/CTM B" -> "Plane Gamma B"
+          "Plane Gamma B" -> "Pipe Blender"
+      }
+
+      subgraph cluster_static {
+          style=dashed
+
+          node [shape=box]
+          "Plane Degamma C" -> "Plane CSC/CTM C"
+          "Plane CSC/CTM C" -> "Plane Gamma C"
+          "Plane Gamma C" -> "Pipe Blender"
+      }
+
+      subgraph cluster_fb {
+          style=dashed
+          label="RAM"
+
+          node [shape=box width=1.7 height=0.2]
+
+          "FB 1" -> "Plane Degamma A"
+          "FB 2" -> "Plane Degamma B"
+          "FB 3" -> "Plane Degamma C"
+      }
+   }
+
+In real world usecases,
+
+1. Plane Degamma can be used to linearize a non linear gamma
+encoded framebuffer. This is needed to do any linear math like
+color space conversion. For ex, linearize frames encoded in SRGB
+or by HDR curve.
+
+2. Later Plane CTM block can convert the content to some different
+colorspace. For ex, SRGB to BT2020 etc.
+
+3. Plane Gamma block can be used later to re-apply the non-linear
+curve. This can also be used to apply Tone Mapping for HDR usecases.
+
+All the layers or framebuffers need to be converted to same color
+space and format before blending. The plane color hardware blocks
+can help with this. Once the Data is blended, similar color processing
+can be done on blended output using pipe color hardware blocks.
+
+DRM Properties have been created to define and expose all these
+hardware blocks to userspace. A userspace application (compositor
+or any color app) can use these interfaces and define policies to
+efficiently use the display hardware for such color operations.
+
+Pipe Color Management Properties
+---------------------------------
+
 .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
    :doc: overview
 
+Plane Color Management Properties
+---------------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :doc: Plane Color Properties
+
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :doc: export
+
 Tile Group Property
 -------------------
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index a8bbb021684b..8892d03602f7 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -708,6 +708,7 @@  static void drm_atomic_plane_print_state(struct drm_printer *p,
 		   drm_get_color_encoding_name(state->color_encoding));
 	drm_printf(p, "\tcolor-range=%s\n",
 		   drm_get_color_range_name(state->color_range));
+	drm_printf(p, "\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed);
 
 	if (plane->funcs->atomic_print_state)
 		plane->funcs->atomic_print_state(p, state);
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index ddcf5c2c8e6a..f26b03853711 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -311,6 +311,8 @@  void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 	state->fence = NULL;
 	state->commit = NULL;
 	state->fb_damage_clips = NULL;
+
+	state->color_mgmt_changed = false;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
 
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 438e9585b225..40fa05fa33dc 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -595,6 +595,8 @@  static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		state->color_encoding = val;
 	} else if (property == plane->color_range_property) {
 		state->color_range = val;
+	} else if (property == plane->degamma_mode_property) {
+		state->degamma_mode = val;
 	} else if (property == config->prop_fb_damage_clips) {
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->fb_damage_clips,
@@ -661,6 +663,8 @@  drm_atomic_plane_get_property(struct drm_plane *plane,
 		*val = state->color_encoding;
 	} else if (property == plane->color_range_property) {
 		*val = state->color_range;
+	} else if (property == plane->degamma_mode_property) {
+		*val = state->degamma_mode;
 	} else if (property == config->prop_fb_damage_clips) {
 		*val = (state->fb_damage_clips) ?
 			state->fb_damage_clips->base.id : 0;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index bb14f488c8f6..085ed0d0db00 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -34,8 +34,8 @@ 
 /**
  * DOC: overview
  *
- * Color management or color space adjustments is supported through a set of 5
- * properties on the &drm_crtc object. They are set up by calling
+ * Pipe Color management or color space adjustments is supported through a
+ * set of 5 properties on the &drm_crtc object. They are set up by calling
  * drm_crtc_enable_color_mgmt().
  *
  * "DEGAMMA_LUT”:
@@ -584,6 +584,95 @@  int drm_plane_create_color_properties(struct drm_plane *plane,
 }
 EXPORT_SYMBOL(drm_plane_create_color_properties);
 
+/**
+ * DOC: Plane Color Properties
+ *
+ * Plane Color management or color space adjustments is supported
+ * through a set of 5 properties on the &drm_plane object.
+ *
+ * degamma_mode_property:
+ *     Blob property which advertizes the possible degamma modes and
+ *     lut ranges supported by the platform. This  allows userspace
+ *     to query and get the plane degamma color caps and choose the
+ *     appropriate degamma mode and create lut values accordingly
+ *
+ */
+int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
+					   struct drm_plane *plane,
+					   int num_values)
+{
+	struct drm_property *prop;
+
+	prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
+				   "PLANE_DEGAMMA_MODE", num_values);
+	if (!prop)
+		return -ENOMEM;
+
+	plane->degamma_mode_property = prop;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_color_mgmt_properties);
+
+void drm_plane_attach_degamma_properties(struct drm_plane *plane)
+{
+	if (!plane->degamma_mode_property)
+		return;
+
+	drm_object_attach_property(&plane->base,
+				   plane->degamma_mode_property, 0);
+}
+EXPORT_SYMBOL(drm_plane_attach_degamma_properties);
+
+int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
+						 const char *name,
+						 const struct drm_color_lut_range *ranges,
+						 size_t length, enum lut_type type)
+{
+	struct drm_property_blob *blob;
+	struct drm_property *prop = NULL;
+	int num_ranges = length / sizeof(ranges[0]);
+	int i, ret, num_types_0;
+
+	if (type == LUT_TYPE_DEGAMMA)
+		prop = plane->degamma_mode_property;
+
+	if (!prop)
+		return -EINVAL;
+
+	if (length == 0 && name)
+		return drm_property_add_enum(prop, 0, name);
+
+	if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
+		return -EINVAL;
+	num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
+			       DRM_MODE_LUT_DEGAMMA));
+	if (num_types_0 == 0)
+		return -EINVAL;
+
+	for (i = 1; i < num_ranges; i++) {
+		int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA |
+					 DRM_MODE_LUT_DEGAMMA));
+
+		/* either all ranges have DEGAMMA|GAMMA or none have it */
+		if (num_types_0 != num_types)
+			return -EINVAL;
+	}
+
+	blob = drm_property_create_blob(plane->dev, length, ranges);
+	if (IS_ERR(blob))
+		return PTR_ERR(blob);
+
+	ret = drm_property_add_enum(prop, blob->base.id, name);
+	if (ret) {
+		drm_property_blob_put(blob);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_color_add_gamma_degamma_mode_range);
+
 /**
  * drm_color_lut_check - check validity of lookup table
  * @lut: property blob containing LUT to check
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index c34a3e8030e1..d4128c7daa08 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -60,7 +60,7 @@  struct drm_mode_object {
 	void (*free_cb)(struct kref *kref);
 };
 
-#define DRM_OBJECT_MAX_PROPERTY 24
+#define DRM_OBJECT_MAX_PROPERTY 26
 /**
  * struct drm_object_properties - property tracking for &drm_mode_object
  */
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 1294610e84f4..e476a5939f8e 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -236,6 +236,15 @@  struct drm_plane_state {
 
 	/** @state: backpointer to global drm_atomic_state */
 	struct drm_atomic_state *state;
+
+	/**
+	 * @degamma_mode: This is a blob_id and exposes the platform capabilities
+	 * wrt to various gamma modes and the respective lut ranges. This also
+	 * helps user select a degamma mode amongst the supported ones.
+	 */
+	u32 degamma_mode;
+
+	u8 color_mgmt_changed : 1;
 };
 
 static inline struct drm_rect
@@ -747,6 +756,12 @@  struct drm_plane {
 	 * scaling.
 	 */
 	struct drm_property *scaling_filter_property;
+
+	/**
+	 * @degamma_mode_property: Optional Plane property to set the LUT
+	 * used to convert the framebuffer's colors to linear gamma.
+	 */
+	struct drm_property *degamma_mode_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
@@ -838,6 +853,14 @@  void drm_plane_force_disable(struct drm_plane *plane);
 int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
 				       struct drm_property *property,
 				       uint64_t value);
+int drm_plane_create_color_mgmt_properties(struct drm_device *dev,
+					   struct drm_plane *plane,
+					   int num_values);
+void drm_plane_attach_degamma_properties(struct drm_plane *plane);
+int drm_plane_color_add_gamma_degamma_mode_range(struct drm_plane *plane,
+						 const char *name,
+						 const struct drm_color_lut_range *ranges,
+						 size_t length, enum lut_type type);
 
 /**
  * drm_plane_find - find a &drm_plane