diff mbox series

[RFC,v5,2/8] drm: Add Plane Degamma properties

Message ID 1537085731-6355-3-git-send-email-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series Add Plane Color Properties | expand

Commit Message

Shankar, Uma Sept. 16, 2018, 8:15 a.m. UTC
Add Plane Degamma as a blob property and plane degamma size as
a range property.

v2: Rebase

v3: Fixed Sean, Paul's review comments. Moved the property from
mode_config to drm_plane. Created a helper function to instantiate
these properties and removed from drm_mode_create_standard_properties
Added property documentation as suggested by Daniel, Vetter.

v4: Rebase

v5: Added "Display Color Hardware Pipeline" flow to kernel
documentation as suggested by "Ville Syrjala" and "Brian Starkey".
Moved the property creation to drm_color_mgmt.c file to consolidate
all color operations at one place.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
---
 Documentation/gpu/drm-kms.rst       | 90 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_atomic.c        | 13 ++++++
 drivers/gpu/drm/drm_atomic_helper.c |  6 +++
 drivers/gpu/drm/drm_color_mgmt.c    | 44 ++++++++++++++++--
 include/drm/drm_plane.h             | 24 ++++++++++
 5 files changed, 174 insertions(+), 3 deletions(-)

Comments

Alexandru-Cosmin Gheorghe Sept. 18, 2018, 4:08 p.m. UTC | #1
Hi,

On Sun, Sep 16, 2018 at 01:45:25PM +0530, Uma Shankar wrote:
> Add Plane Degamma as a blob property and plane degamma size as
> a range property.
> 
> v2: Rebase
> 
> v3: Fixed Sean, Paul's review comments. Moved the property from
> mode_config to drm_plane. Created a helper function to instantiate
> these properties and removed from drm_mode_create_standard_properties
> Added property documentation as suggested by Daniel, Vetter.
> 
> v4: Rebase
> 
> v5: Added "Display Color Hardware Pipeline" flow to kernel
> documentation as suggested by "Ville Syrjala" and "Brian Starkey".
> Moved the property creation to drm_color_mgmt.c file to consolidate
> all color operations at one place.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> ---
>  Documentation/gpu/drm-kms.rst       | 90 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic.c        | 13 ++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  6 +++
>  drivers/gpu/drm/drm_color_mgmt.c    | 44 ++++++++++++++++--
>  include/drm/drm_plane.h             | 24 ++++++++++
>  5 files changed, 174 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index f8f5bf1..253d546 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -551,12 +551,102 @@ Plane Composition 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
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>     :export:
>  
> +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 d0478ab..e716614 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
> +	bool replaced = false;
> +	int ret;
>  
>  	if (property == config->prop_fb_id) {
>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
> @@ -910,6 +912,13 @@ 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_lut_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->degamma_lut,
> +					val, -1, sizeof(struct drm_color_lut),

Sorry I missed this thing the first time, but this should be
sizeof drm_color_lut_ext.

Also I think we have the same problem as for crtc properties when
doing suspend/resume.
I think color_mgmt_changed should just be set in
drm_atomic_helper_check_planes
https://lists.freedesktop.org/archives/dri-devel/2018-September/189265.html

> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -980,6 +989,9 @@ static int drm_atomic_plane_set_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_lut_property) {
> +		*val = (state->degamma_lut) ?
> +			state->degamma_lut->base.id : 0;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> @@ -1120,6 +1132,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_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 2c23a48..203137e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3614,6 +3614,10 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  
>  	state->fence = NULL;
>  	state->commit = NULL;
> +
> +	if (state->degamma_lut)
> +		drm_property_blob_get(state->degamma_lut);
> +	state->color_mgmt_changed = false;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>  
> @@ -3658,6 +3662,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>  
>  	if (state->commit)
>  		drm_crtc_commit_put(state->commit);
> +
> +	drm_property_blob_put(state->degamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 1bdcc1a..d8a9e8b 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -29,11 +29,11 @@
>  /**
>   * 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”:
> + * "DEGAMMA_LUT
>   *	Blob property to set the degamma lookup table (LUT) mapping pixel data
>   *	from the framebuffer before it is given to the transformation matrix.
>   *	The data is interpreted as an array of &struct drm_color_lut elements.
> @@ -491,3 +491,41 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
>  	return 0;
>  }
>  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_lut_property:
> + *     Blob property which allows a userspace to provide LUT values
> + *     to apply degamma curve using the h/w plane degamma processing
> + *     engine, thereby making the content as linear for further color
> + *     processing.
> + *
> + * degamma_lut_size_property:
> + *     Range Property to indicate size of the plane degamma LUT.
> + */
> +int drm_plane_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"PLANE_DEGAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->degamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->degamma_lut_size_property = prop;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_color_create_prop);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 16f5b666..7f0d961 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -182,6 +182,14 @@ struct drm_plane_state {
>  	 */
>  	bool visible;
>  
> +	/* @degamma_lut:
> +	 *
> +	 * Lookup table for converting framebuffer pixel data before apply the
> +	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The
> +	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
> +	 */
> +	struct drm_property_blob *degamma_lut;
> +
>  	/**
>  	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
>  	 * and for async plane updates.
> @@ -192,6 +200,8 @@ struct drm_plane_state {
>  
>  	/** @state: backpointer to global drm_atomic_state */
>  	struct drm_atomic_state *state;
> +
> +	u8 color_mgmt_changed : 1;
>  };
>  
>  static inline struct drm_rect
> @@ -692,6 +702,18 @@ struct drm_plane {
>  	 * See drm_plane_create_color_properties().
>  	 */
>  	struct drm_property *color_range_property;
> +
> +	/**
> +	 * @degamma_lut_property: Optional Plane property to set the LUT
> +	 * used to convert the framebuffer's colors to linear gamma.
> +	 */
> +	struct drm_property *degamma_lut_property;
> +
> +	/**
> +	 * @degamma_lut_size_property: Optional Plane property for the
> +	 * size of the degamma LUT as supported by the driver (read-only).
> +	 */
> +	struct drm_property *degamma_lut_size_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> @@ -741,6 +763,8 @@ static inline u32 drm_plane_mask(const 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_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane);
>  
>  /**
>   * drm_plane_find - find a &drm_plane
> -- 
> 1.9.1
Matt Roper Nov. 14, 2018, 1:08 a.m. UTC | #2
On Sun, Sep 16, 2018 at 01:45:25PM +0530, Uma Shankar wrote:
> Add Plane Degamma as a blob property and plane degamma size as
> a range property.
> 
> v2: Rebase
> 
> v3: Fixed Sean, Paul's review comments. Moved the property from
> mode_config to drm_plane. Created a helper function to instantiate
> these properties and removed from drm_mode_create_standard_properties
> Added property documentation as suggested by Daniel, Vetter.
> 
> v4: Rebase
> 
> v5: Added "Display Color Hardware Pipeline" flow to kernel
> documentation as suggested by "Ville Syrjala" and "Brian Starkey".
> Moved the property creation to drm_color_mgmt.c file to consolidate
> all color operations at one place.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> ---
>  Documentation/gpu/drm-kms.rst       | 90 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic.c        | 13 ++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  6 +++
>  drivers/gpu/drm/drm_color_mgmt.c    | 44 ++++++++++++++++--
>  include/drm/drm_plane.h             | 24 ++++++++++
>  5 files changed, 174 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index f8f5bf1..253d546 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -551,12 +551,102 @@ Plane Composition 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
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>     :export:
>  
> +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 d0478ab..e716614 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
> +	bool replaced = false;
> +	int ret;
>  
>  	if (property == config->prop_fb_id) {
>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
> @@ -910,6 +912,13 @@ 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_lut_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->degamma_lut,
> +					val, -1, sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -980,6 +989,9 @@ static int drm_atomic_plane_set_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_lut_property) {
> +		*val = (state->degamma_lut) ?
> +			state->degamma_lut->base.id : 0;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> @@ -1120,6 +1132,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_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 2c23a48..203137e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3614,6 +3614,10 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>  
>  	state->fence = NULL;
>  	state->commit = NULL;
> +
> +	if (state->degamma_lut)
> +		drm_property_blob_get(state->degamma_lut);
> +	state->color_mgmt_changed = false;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>  
> @@ -3658,6 +3662,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>  
>  	if (state->commit)
>  		drm_crtc_commit_put(state->commit);
> +
> +	drm_property_blob_put(state->degamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 1bdcc1a..d8a9e8b 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -29,11 +29,11 @@
>  /**
>   * 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”:
> + * "DEGAMMA_LUT

This change seems unrelated/unwanted.

>   *	Blob property to set the degamma lookup table (LUT) mapping pixel data
>   *	from the framebuffer before it is given to the transformation matrix.
>   *	The data is interpreted as an array of &struct drm_color_lut elements.
> @@ -491,3 +491,41 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
>  	return 0;
>  }
>  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_lut_property:
> + *     Blob property which allows a userspace to provide LUT values
> + *     to apply degamma curve using the h/w plane degamma processing
> + *     engine, thereby making the content as linear for further color
> + *     processing.
> + *
> + * degamma_lut_size_property:
> + *     Range Property to indicate size of the plane degamma LUT.

Elsewhere we're documenting the string name for the property, which is
part of the ABI (e.g., "DEGAMMA_LUT") but here you're documenting the
property variable name in the code.

I think there was also discussion at one point of having a more complex
LUT interface that deals with things like non-linear ranges and such.
If that's still planned, would it be layered on top of these properties,
or would it be a completely separate, replacement ABI down the road?

> + */
> +int drm_plane_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"PLANE_DEGAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	plane->degamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;

If this fails, should we try to clean up the earlier property?  It looks
the i915 callsite for this function (in patch #6 of the series) doesn't
check the return value, so even if we return -ENOMEM, the driver will
continue with some properties created and some not.

Actually, aside from using names "PLANE_FOO" instead of "FOO," is there
a reason why we need to create new properties for planes?  I think we
could still attach the same properties we're already using for CRTC's
(dev->mode_config.degamma_lut_property and such) since afaik there isn't
anything that prevents us from attaching the same property to different
kinds of drm_object's.


Matt

> +	plane->degamma_lut_size_property = prop;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_color_create_prop);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 16f5b666..7f0d961 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -182,6 +182,14 @@ struct drm_plane_state {
>  	 */
>  	bool visible;
>  
> +	/* @degamma_lut:
> +	 *
> +	 * Lookup table for converting framebuffer pixel data before apply the
> +	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The
> +	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
> +	 */
> +	struct drm_property_blob *degamma_lut;
> +
>  	/**
>  	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
>  	 * and for async plane updates.
> @@ -192,6 +200,8 @@ struct drm_plane_state {
>  
>  	/** @state: backpointer to global drm_atomic_state */
>  	struct drm_atomic_state *state;
> +
> +	u8 color_mgmt_changed : 1;
>  };
>  
>  static inline struct drm_rect
> @@ -692,6 +702,18 @@ struct drm_plane {
>  	 * See drm_plane_create_color_properties().
>  	 */
>  	struct drm_property *color_range_property;
> +
> +	/**
> +	 * @degamma_lut_property: Optional Plane property to set the LUT
> +	 * used to convert the framebuffer's colors to linear gamma.
> +	 */
> +	struct drm_property *degamma_lut_property;
> +
> +	/**
> +	 * @degamma_lut_size_property: Optional Plane property for the
> +	 * size of the degamma LUT as supported by the driver (read-only).
> +	 */
> +	struct drm_property *degamma_lut_size_property;
>  };
>  
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> @@ -741,6 +763,8 @@ static inline u32 drm_plane_mask(const 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_color_create_prop(struct drm_device *dev,
> +				struct drm_plane *plane);
>  
>  /**
>   * drm_plane_find - find a &drm_plane
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Shankar, Uma Nov. 14, 2018, 1:59 p.m. UTC | #3
>-----Original Message-----
>From: Roper, Matthew D
>Sent: Wednesday, November 14, 2018 6:39 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>dcastagna@chromium.org; alexandru-cosmin.gheorghe@arm.com;
>seanpaul@chromium.org; Syrjala, Ville <ville.syrjala@intel.com>;
>harry.wentland@amd.com; Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [RFC v5 2/8] drm: Add Plane Degamma properties
>
>On Sun, Sep 16, 2018 at 01:45:25PM +0530, Uma Shankar wrote:
>> Add Plane Degamma as a blob property and plane degamma size as a range
>> property.
>>
>> v2: Rebase
>>
>> v3: Fixed Sean, Paul's review comments. Moved the property from
>> mode_config to drm_plane. Created a helper function to instantiate
>> these properties and removed from drm_mode_create_standard_properties
>> Added property documentation as suggested by Daniel, Vetter.
>>
>> v4: Rebase
>>
>> v5: Added "Display Color Hardware Pipeline" flow to kernel
>> documentation as suggested by "Ville Syrjala" and "Brian Starkey".
>> Moved the property creation to drm_color_mgmt.c file to consolidate
>> all color operations at one place.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
>> ---
>>  Documentation/gpu/drm-kms.rst       | 90
>+++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/drm_atomic.c        | 13 ++++++
>>  drivers/gpu/drm/drm_atomic_helper.c |  6 +++
>>  drivers/gpu/drm/drm_color_mgmt.c    | 44 ++++++++++++++++--
>>  include/drm/drm_plane.h             | 24 ++++++++++
>>  5 files changed, 174 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/gpu/drm-kms.rst
>> b/Documentation/gpu/drm-kms.rst index f8f5bf1..253d546 100644
>> --- a/Documentation/gpu/drm-kms.rst
>> +++ b/Documentation/gpu/drm-kms.rst
>> @@ -551,12 +551,102 @@ Plane Composition 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
>>
>>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>>     :export:
>>
>> +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 d0478ab..e716614 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct
>> drm_plane *plane,  {
>>  	struct drm_device *dev = plane->dev;
>>  	struct drm_mode_config *config = &dev->mode_config;
>> +	bool replaced = false;
>> +	int ret;
>>
>>  	if (property == config->prop_fb_id) {
>>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev,
>NULL,
>> val); @@ -910,6 +912,13 @@ 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_lut_property) {
>> +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> +					&state->degamma_lut,
>> +					val, -1, sizeof(struct drm_color_lut),
>> +					&replaced);
>> +		state->color_mgmt_changed |= replaced;
>> +		return ret;
>>  	} else if (plane->funcs->atomic_set_property) {
>>  		return plane->funcs->atomic_set_property(plane, state,
>>  				property, val);
>> @@ -980,6 +989,9 @@ static int drm_atomic_plane_set_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_lut_property) {
>> +		*val = (state->degamma_lut) ?
>> +			state->degamma_lut->base.id : 0;
>>  	} else if (plane->funcs->atomic_get_property) {
>>  		return plane->funcs->atomic_get_property(plane, state,
>property, val);
>>  	} else {
>> @@ -1120,6 +1132,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_helper.c
>> b/drivers/gpu/drm/drm_atomic_helper.c
>> index 2c23a48..203137e 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -3614,6 +3614,10 @@ void
>> __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>>
>>  	state->fence = NULL;
>>  	state->commit = NULL;
>> +
>> +	if (state->degamma_lut)
>> +		drm_property_blob_get(state->degamma_lut);
>> +	state->color_mgmt_changed = false;
>>  }
>>  EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>>
>> @@ -3658,6 +3662,8 @@ void
>> __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>>
>>  	if (state->commit)
>>  		drm_crtc_commit_put(state->commit);
>> +
>> +	drm_property_blob_put(state->degamma_lut);
>>  }
>>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>>
>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
>> b/drivers/gpu/drm/drm_color_mgmt.c
>> index 1bdcc1a..d8a9e8b 100644
>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>> @@ -29,11 +29,11 @@
>>  /**
>>   * 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”:
>> + * "DEGAMMA_LUT
>
>This change seems unrelated/unwanted.

Updated Color Management to Pipe Color Management here. The last part,
DEGAMMA_LUT change can be dropped and kept as is. Will update this.

>>   *	Blob property to set the degamma lookup table (LUT) mapping pixel data
>>   *	from the framebuffer before it is given to the transformation matrix.
>>   *	The data is interpreted as an array of &struct drm_color_lut elements.
>> @@ -491,3 +491,41 @@ int drm_plane_create_color_properties(struct
>drm_plane *plane,
>>  	return 0;
>>  }
>>  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_lut_property:
>> + *     Blob property which allows a userspace to provide LUT values
>> + *     to apply degamma curve using the h/w plane degamma processing
>> + *     engine, thereby making the content as linear for further color
>> + *     processing.
>> + *
>> + * degamma_lut_size_property:
>> + *     Range Property to indicate size of the plane degamma LUT.
>
>Elsewhere we're documenting the string name for the property, which is part of
>the ABI (e.g., "DEGAMMA_LUT") but here you're documenting the property
>variable name in the code.

Ok, will change this to be consistent with pipe color property documentation. Thanks..

>I think there was also discussion at one point of having a more complex LUT
>interface that deals with things like non-linear ranges and such.
>If that's still planned, would it be layered on top of these properties, or would it
>be a completely separate, replacement ABI down the road?

The non-linear and multi-segment part is a bit tricky.  We can develop that on top
of this property by creating a segment lut property blob which just tells the number of
segments and size of each segment. The actual lut values can be passed through the
current property. Driver can then use data of both these properties and use it as per
respective hardware to program LUT.

If the blob property is not set while plane degamma is set, driver functions in the legacy way
and program luts in a linear fashion (basically as per legacy design).
This is just my thought on that one. Better ideas are welcome.

>> + */
>> +int drm_plane_color_create_prop(struct drm_device *dev,
>> +				struct drm_plane *plane)
>> +{
>> +	struct drm_property *prop;
>> +
>> +	prop = drm_property_create(dev,
>> +			DRM_MODE_PROP_BLOB,
>> +			"PLANE_DEGAMMA_LUT", 0);
>> +	if (!prop)
>> +		return -ENOMEM;
>> +	plane->degamma_lut_property = prop;
>> +
>> +	prop = drm_property_create_range(dev,
>> +			DRM_MODE_PROP_IMMUTABLE,
>> +			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
>> +	if (!prop)
>> +		return -ENOMEM;
>
>If this fails, should we try to clean up the earlier property?  It looks the i915
>callsite for this function (in patch #6 of the series) doesn't check the return value,
>so even if we return -ENOMEM, the driver will continue with some properties
>created and some not.

Sure, I will fix this. Thanks..

>Actually, aside from using names "PLANE_FOO" instead of "FOO," is there a
>reason why we need to create new properties for planes?  I think we could still
>attach the same properties we're already using for CRTC's (dev-
>>mode_config.degamma_lut_property and such) since afaik there isn't anything
>that prevents us from attaching the same property to different kinds of
>drm_object's.

This point was discussed when we were planning this. Plane color is little different than
pipe color, and comes handy for making pre blending framebuffer conversion. Also, we
can extend it to handle multi segment and non-linear LUT's. Hence, we thought to keep it
separate from pipe color properties. 

Regards,
Uma Shankar
>
>Matt
>
>> +	plane->degamma_lut_size_property = prop;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(drm_plane_color_create_prop);
>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
>> 16f5b666..7f0d961 100644
>> --- a/include/drm/drm_plane.h
>> +++ b/include/drm/drm_plane.h
>> @@ -182,6 +182,14 @@ struct drm_plane_state {
>>  	 */
>>  	bool visible;
>>
>> +	/* @degamma_lut:
>> +	 *
>> +	 * Lookup table for converting framebuffer pixel data before apply the
>> +	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt().
>The
>> +	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
>> +	 */
>> +	struct drm_property_blob *degamma_lut;
>> +
>>  	/**
>>  	 * @commit: Tracks the pending commit to prevent use-after-free
>conditions,
>>  	 * and for async plane updates.
>> @@ -192,6 +200,8 @@ struct drm_plane_state {
>>
>>  	/** @state: backpointer to global drm_atomic_state */
>>  	struct drm_atomic_state *state;
>> +
>> +	u8 color_mgmt_changed : 1;
>>  };
>>
>>  static inline struct drm_rect
>> @@ -692,6 +702,18 @@ struct drm_plane {
>>  	 * See drm_plane_create_color_properties().
>>  	 */
>>  	struct drm_property *color_range_property;
>> +
>> +	/**
>> +	 * @degamma_lut_property: Optional Plane property to set the LUT
>> +	 * used to convert the framebuffer's colors to linear gamma.
>> +	 */
>> +	struct drm_property *degamma_lut_property;
>> +
>> +	/**
>> +	 * @degamma_lut_size_property: Optional Plane property for the
>> +	 * size of the degamma LUT as supported by the driver (read-only).
>> +	 */
>> +	struct drm_property *degamma_lut_size_property;
>>  };
>>
>>  #define obj_to_plane(x) container_of(x, struct drm_plane, base) @@
>> -741,6 +763,8 @@ static inline u32 drm_plane_mask(const 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_color_create_prop(struct drm_device *dev,
>> +				struct drm_plane *plane);
>>
>>  /**
>>   * drm_plane_find - find a &drm_plane
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Matt Roper
>Graphics Software Engineer
>IoTG Platform Enabling & Development
>Intel Corporation
>(916) 356-2795
diff mbox series

Patch

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index f8f5bf1..253d546 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -551,12 +551,102 @@  Plane Composition 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
 
 .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
    :export:
 
+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 d0478ab..e716614 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -857,6 +857,8 @@  static int drm_atomic_plane_set_property(struct drm_plane *plane,
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_mode_config *config = &dev->mode_config;
+	bool replaced = false;
+	int ret;
 
 	if (property == config->prop_fb_id) {
 		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
@@ -910,6 +912,13 @@  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_lut_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->degamma_lut,
+					val, -1, sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -980,6 +989,9 @@  static int drm_atomic_plane_set_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_lut_property) {
+		*val = (state->degamma_lut) ?
+			state->degamma_lut->base.id : 0;
 	} else if (plane->funcs->atomic_get_property) {
 		return plane->funcs->atomic_get_property(plane, state, property, val);
 	} else {
@@ -1120,6 +1132,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_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 2c23a48..203137e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3614,6 +3614,10 @@  void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
 
 	state->fence = NULL;
 	state->commit = NULL;
+
+	if (state->degamma_lut)
+		drm_property_blob_get(state->degamma_lut);
+	state->color_mgmt_changed = false;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
 
@@ -3658,6 +3662,8 @@  void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
 
 	if (state->commit)
 		drm_crtc_commit_put(state->commit);
+
+	drm_property_blob_put(state->degamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 1bdcc1a..d8a9e8b 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -29,11 +29,11 @@ 
 /**
  * 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”:
+ * "DEGAMMA_LUT
  *	Blob property to set the degamma lookup table (LUT) mapping pixel data
  *	from the framebuffer before it is given to the transformation matrix.
  *	The data is interpreted as an array of &struct drm_color_lut elements.
@@ -491,3 +491,41 @@  int drm_plane_create_color_properties(struct drm_plane *plane,
 	return 0;
 }
 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_lut_property:
+ *     Blob property which allows a userspace to provide LUT values
+ *     to apply degamma curve using the h/w plane degamma processing
+ *     engine, thereby making the content as linear for further color
+ *     processing.
+ *
+ * degamma_lut_size_property:
+ *     Range Property to indicate size of the plane degamma LUT.
+ */
+int drm_plane_color_create_prop(struct drm_device *dev,
+				struct drm_plane *plane)
+{
+	struct drm_property *prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"PLANE_DEGAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	plane->degamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"PLANE_DEGAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	plane->degamma_lut_size_property = prop;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_color_create_prop);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 16f5b666..7f0d961 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -182,6 +182,14 @@  struct drm_plane_state {
 	 */
 	bool visible;
 
+	/* @degamma_lut:
+	 *
+	 * Lookup table for converting framebuffer pixel data before apply the
+	 * color conversion matrix @ctm. See drm_plane_enable_color_mgmt(). The
+	 * blob (if not NULL) is an array of &struct drm_color_lut_ext.
+	 */
+	struct drm_property_blob *degamma_lut;
+
 	/**
 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
 	 * and for async plane updates.
@@ -192,6 +200,8 @@  struct drm_plane_state {
 
 	/** @state: backpointer to global drm_atomic_state */
 	struct drm_atomic_state *state;
+
+	u8 color_mgmt_changed : 1;
 };
 
 static inline struct drm_rect
@@ -692,6 +702,18 @@  struct drm_plane {
 	 * See drm_plane_create_color_properties().
 	 */
 	struct drm_property *color_range_property;
+
+	/**
+	 * @degamma_lut_property: Optional Plane property to set the LUT
+	 * used to convert the framebuffer's colors to linear gamma.
+	 */
+	struct drm_property *degamma_lut_property;
+
+	/**
+	 * @degamma_lut_size_property: Optional Plane property for the
+	 * size of the degamma LUT as supported by the driver (read-only).
+	 */
+	struct drm_property *degamma_lut_size_property;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
@@ -741,6 +763,8 @@  static inline u32 drm_plane_mask(const 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_color_create_prop(struct drm_device *dev,
+				struct drm_plane *plane);
 
 /**
  * drm_plane_find - find a &drm_plane