diff mbox series

[RFC,4/5] drm/drm_color_mgmt: add 3D LUT to color mgmt properties

Message ID 20220619223104.667413-5-mwen@igalia.com (mailing list archive)
State New, archived
Headers show
Series DRM CRTC 3D LUT interface for AMD DCN | expand

Commit Message

Melissa Wen June 19, 2022, 10:31 p.m. UTC
Add 3D LUT for gammar correction using a 3D lookup table.  The position
in the color correction pipeline where 3D LUT is applied depends on hw
design, being after CTM or gamma. If just after CTM, a shaper lut must
be set to shape the content for a non-linear space. That details should
be handled by the driver according to its color capabilities.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 drivers/gpu/drm/drm_atomic_state_helper.c |  3 ++
 drivers/gpu/drm/drm_atomic_uapi.c         | 14 +++++-
 drivers/gpu/drm/drm_color_mgmt.c          | 58 +++++++++++++++++++++++
 drivers/gpu/drm/drm_fb_helper.c           |  2 +
 drivers/gpu/drm/drm_mode_config.c         | 14 ++++++
 include/drm/drm_color_mgmt.h              |  4 ++
 include/drm/drm_crtc.h                    | 12 ++++-
 include/drm/drm_mode_config.h             | 13 +++++
 8 files changed, 117 insertions(+), 3 deletions(-)

Comments

Daniel Vetter June 24, 2022, 9:40 p.m. UTC | #1
On Sun, Jun 19, 2022 at 09:31:03PM -0100, Melissa Wen wrote:
> Add 3D LUT for gammar correction using a 3D lookup table.  The position
> in the color correction pipeline where 3D LUT is applied depends on hw
> design, being after CTM or gamma. If just after CTM, a shaper lut must
> be set to shape the content for a non-linear space. That details should
> be handled by the driver according to its color capabilities.
> 
> Signed-off-by: Melissa Wen <mwen@igalia.com>

I think our color correction pipeline is at a complexity where a DOT graph
that shows how it all works together is justified. So including color
conversion from planes (like yuv->rgb), plane blending and then the color
pipeline here. Maybe in the plane composition section, with the color
conversion pipeline docs linking to that?
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c |  3 ++
>  drivers/gpu/drm/drm_atomic_uapi.c         | 14 +++++-
>  drivers/gpu/drm/drm_color_mgmt.c          | 58 +++++++++++++++++++++++
>  drivers/gpu/drm/drm_fb_helper.c           |  2 +
>  drivers/gpu/drm/drm_mode_config.c         | 14 ++++++
>  include/drm/drm_color_mgmt.h              |  4 ++
>  include/drm/drm_crtc.h                    | 12 ++++-
>  include/drm/drm_mode_config.h             | 13 +++++
>  8 files changed, 117 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index cf0545bb6e00..64800bc41365 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -141,6 +141,8 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  		drm_property_blob_get(state->ctm);
>  	if (state->shaper_lut)
>  		drm_property_blob_get(state->shaper_lut);
> +	if (state->lut3d)
> +		drm_property_blob_get(state->lut3d);
>  	if (state->gamma_lut)
>  		drm_property_blob_get(state->gamma_lut);
>  
> @@ -216,6 +218,7 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
>  	drm_property_blob_put(state->degamma_lut);
>  	drm_property_blob_put(state->ctm);
>  	drm_property_blob_put(state->shaper_lut);
> +	drm_property_blob_put(state->lut3d);
>  	drm_property_blob_put(state->gamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 6468f2a080bc..1896c0422f73 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -472,6 +472,14 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  					&replaced);
>  		state->color_mgmt_changed |= replaced;
>  		return ret;
> +	} else if (property == config->lut3d_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->lut3d,
> +					val,
> +					-1, sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (property == config->gamma_lut_property) {
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->gamma_lut,
> @@ -523,10 +531,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>  		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
>  	else if (property == config->ctm_property)
>  		*val = (state->ctm) ? state->ctm->base.id : 0;
> -	else if (property == config->gamma_lut_property)
> -		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (property == config->shaper_lut_property)
>  		*val = (state->shaper_lut) ? state->shaper_lut->base.id : 0;
> +	else if (property == config->lut3d_property)
> +		*val = (state->lut3d) ? state->lut3d->base.id : 0;
> +	else if (property == config->gamma_lut_property)
> +		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (property == config->prop_out_fence_ptr)
>  		*val = 0;
>  	else if (property == crtc->scaling_filter_property)
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 4f57dc60fe03..696fe1e37801 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -87,6 +87,25 @@
>   *	publish the largest size, and sub-sample smaller sized LUTs
>   *	appropriately.
>   *
> + * “LUT3D”:
> + *	Blob property to set the 3D LUT mapping pixel data after the color
> + *	transformation matrix and before gamma 1D lut correction. The
> + *	data is interpreted as an array of &struct drm_color_lut elements.
> + *	Hardware might choose not to use the full precision of the LUT
> + *	elements.
> + *
> + *	Setting this to NULL (blob property value set to 0) means a the output
> + *	color is identical to the input color. This is generally the driver
> + *	boot-up state too. Drivers can access this blob through
> + *	&drm_crtc_state.gamma_lut.
> + *
> + * “LUT3D_SIZE”:
> + *	Unsigned range property to give the size of the 3D lookup table to be
> + *	set on the LUT3D property (the size depends on the underlying
> + *	hardware). If drivers support multiple LUT sizes then they should
> + *	publish the largest size, and sub-sample smaller sized LUTs
> + *	appropriately.
> + *
>   * “GAMMA_LUT”:
>   *	Blob property to set the gamma lookup table (LUT) mapping pixel data
>   *	after the transformation matrix to data sent to the connector. The
> @@ -204,6 +223,45 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  }
>  EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
>  
> +/**
> + * drm_crtc_enable_lut3d - enable 3D LUT properties
> + * @crtc: DRM CRTC
> + * @shaper_lut_size: the size of shaper lut
> + * @lut3d_size: the size of 3D lut
> + *
> + * This function lets the driver enable the 3D LUT color correction property
> + * on a CRTC. This includes 3D LUT and also a shaper LUT, if set. The shaper
> + * LUT property is only attached if its size is not 0 and 3D LUT is set, being
> + * therefore optional.
> + */
> +void drm_crtc_enable_lut3d(struct drm_crtc *crtc,
> +			   uint shaper_lut_size,
> +			   uint lut3d_size)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (!lut3d_size)
> +		return;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->lut3d_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->lut3d_size_property,
> +				   lut3d_size);
> +	if (!shaper_lut_size)
> +		return;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->shaper_lut_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->shaper_lut_size_property,
> +				   lut3d_size);
> +
> +}
> +EXPORT_SYMBOL(drm_crtc_enable_lut3d);
> +
> +
>  /**
>   * drm_mode_crtc_set_gamma_size - set the gamma table size
>   * @crtc: CRTC to set the gamma table size for
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index bdd33817d968..358c528c7c80 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1069,6 +1069,8 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
>  		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
>  		replaced |= drm_property_replace_blob(&crtc_state->shaper_lut,
>  						      NULL);
> +		replaced |= drm_property_replace_blob(&crtc_state->lut3d,
> +						      NULL);
>  		replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
>  						      gamma_lut);
>  
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 4ba2a95b88e8..5458a7dfbe63 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -364,6 +364,20 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.shaper_lut_size_property = prop;
>  
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"LUT3D", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.lut3d_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"LUT3D_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.lut3d_size_property = prop;
> +
>  	prop = drm_property_create(dev,
>  			DRM_MODE_PROP_BLOB,
>  			"GAMMA_LUT", 0);
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 81c298488b0c..a4f054e0108f 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -59,6 +59,10 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				bool has_ctm,
>  				uint gamma_lut_size);
>  
> +void drm_crtc_enable_lut3d(struct drm_crtc *crtc,
> +			   uint shaper_lut_size,
> +			   uint lut3d_size);
> +
>  int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
>  				 int gamma_size);
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a318d5feb44b..c22ffcc4d7aa 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -165,7 +165,7 @@ struct drm_crtc_state {
>  	bool zpos_changed : 1;
>  	/**
>  	 * @color_mgmt_changed: Color management properties have changed
> -	 * (@shaper_lut, @gamma_lut, @degamma_lut or @ctm). Used by
> +	 * (@shaper_lut, @lut3d, @gamma_lut, @degamma_lut or @ctm). Used by
>  	 * the atomic helpers and drivers to steer the atomic commit control
>  	 * flow.
>  	 */
> @@ -298,6 +298,16 @@ struct drm_crtc_state {
>  	 */
>  	struct drm_property_blob *shaper_lut;
>  
> +	/**
> +	 * @lut3d:
> +	 *
> +	 * 3D Lookup table for converting pixel data. Position where it takes
> +	 * place depends on hw design, after @ctm or @gamma_lut. See
> +	 * drm_crtc_enable_color_mgmt(). The blob (if not NULL) is an array of
> +	 * &struct drm_color_lut.
> +	 */
> +	struct drm_property_blob *lut3d;
> +
>  	/**
>  	 * @target_vblank:
>  	 *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 2df7e171add9..87280694e70d 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -812,6 +812,19 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *shaper_lut_size_property;
>  
> +	/**
> +	 * @lut3d_property: Optional CRTC property to set the 3D LUT used to
> +	 * convert colors; before or after gamma conversion depends on hw
> +	 * design. A shaper LUT can be used to delinearize content before apply
> +	 * 3D LUT correction.
> +	 */
> +	struct drm_property *lut3d_property;
> +	/**
> +	 * @lut3d_size_property: Optional CRTC property for the size of the
> +	 * 3D LUT as supported by the driver (read-only).
> +	 */
> +	struct drm_property *lut3d_size_property;
> +
>  	/**
>  	 * @gamma_lut_property: Optional CRTC property to set the LUT used to
>  	 * convert the colors, after the CTM matrix, to the gamma space of the
> -- 
> 2.35.1
>
Ville Syrjälä June 27, 2022, 12:18 p.m. UTC | #2
On Sun, Jun 19, 2022 at 09:31:03PM -0100, Melissa Wen wrote:
> Add 3D LUT for gammar correction using a 3D lookup table.  The position
> in the color correction pipeline where 3D LUT is applied depends on hw
> design, being after CTM or gamma. If just after CTM, a shaper lut must
> be set to shape the content for a non-linear space. That details should
> be handled by the driver according to its color capabilities.

I also cooked up a WIP 3D LUT support some time ago for Intel hw:
https://github.com/vsyrjala/linux/commits/3dlut
But that dried up due to having no userspace for it.

I also cooked up some basic igts for it:
https://patchwork.freedesktop.org/series/90165/

<snip>
> + * “LUT3D”:
> + *	Blob property to set the 3D LUT mapping pixel data after the color
> + *	transformation matrix and before gamma 1D lut correction.

On Intel hw the 3DLUT is after the gamma LUT in the pipeline, which is
where I placed it in my branch.

There is now some discussion happening about exposing some
kind of color pipeline description/configuration properties:
https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11
Harry Wentland June 28, 2022, 7:41 p.m. UTC | #3
On 2022-06-27 08:18, Ville Syrjälä wrote:
> On Sun, Jun 19, 2022 at 09:31:03PM -0100, Melissa Wen wrote:
>> Add 3D LUT for gammar correction using a 3D lookup table.  The position
>> in the color correction pipeline where 3D LUT is applied depends on hw
>> design, being after CTM or gamma. If just after CTM, a shaper lut must
>> be set to shape the content for a non-linear space. That details should
>> be handled by the driver according to its color capabilities.
> 
> I also cooked up a WIP 3D LUT support some time ago for Intel hw:
> https://github.com/vsyrjala/linux/commits/3dlut>> But that dried up due to having no userspace for it.
> 
> I also cooked up some basic igts for it:
> https://patchwork.freedesktop.org/series/90165/>> 
> <snip>
>> + * “LUT3D”:
>> + *	Blob property to set the 3D LUT mapping pixel data after the color
>> + *	transformation matrix and before gamma 1D lut correction.
> 
> On Intel hw the 3DLUT is after the gamma LUT in the pipeline, which is
> where I placed it in my branch.
> 
> There is now some discussion happening about exposing some
> kind of color pipeline description/configuration properties:
> https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11>> 

After all the discussions about properties to support color management for
HDR and other features it's becoming clear to me that we'll need some color
pipeline description going forward, i.e. something like the one Sebastian
proposed. It's complex but if we're not defining this now we'll be in a pickle
when the next driver implementer goes and finds that their HW looks different
yet again and doesn't match any of the orders we've defined so far.

Harry
Harry Wentland June 28, 2022, 9:34 p.m. UTC | #4
On 6/19/22 18:31, Melissa Wen wrote:
> Add 3D LUT for gammar correction using a 3D lookup table.  The position
> in the color correction pipeline where 3D LUT is applied depends on hw
> design, being after CTM or gamma. If just after CTM, a shaper lut must
> be set to shape the content for a non-linear space. That details should
> be handled by the driver according to its color capabilities.
> 
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c |  3 ++
>  drivers/gpu/drm/drm_atomic_uapi.c         | 14 +++++-
>  drivers/gpu/drm/drm_color_mgmt.c          | 58 +++++++++++++++++++++++
>  drivers/gpu/drm/drm_fb_helper.c           |  2 +
>  drivers/gpu/drm/drm_mode_config.c         | 14 ++++++
>  include/drm/drm_color_mgmt.h              |  4 ++
>  include/drm/drm_crtc.h                    | 12 ++++-
>  include/drm/drm_mode_config.h             | 13 +++++
>  8 files changed, 117 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index cf0545bb6e00..64800bc41365 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -141,6 +141,8 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  		drm_property_blob_get(state->ctm);
>  	if (state->shaper_lut)
>  		drm_property_blob_get(state->shaper_lut);
> +	if (state->lut3d)
> +		drm_property_blob_get(state->lut3d);
>  	if (state->gamma_lut)
>  		drm_property_blob_get(state->gamma_lut);
>  
> @@ -216,6 +218,7 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
>  	drm_property_blob_put(state->degamma_lut);
>  	drm_property_blob_put(state->ctm);
>  	drm_property_blob_put(state->shaper_lut);
> +	drm_property_blob_put(state->lut3d);
>  	drm_property_blob_put(state->gamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 6468f2a080bc..1896c0422f73 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -472,6 +472,14 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  					&replaced);
>  		state->color_mgmt_changed |= replaced;
>  		return ret;
> +	} else if (property == config->lut3d_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +					&state->lut3d,
> +					val,
> +					-1, sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed |= replaced;
> +		return ret;
>  	} else if (property == config->gamma_lut_property) {
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->gamma_lut,
> @@ -523,10 +531,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>  		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
>  	else if (property == config->ctm_property)
>  		*val = (state->ctm) ? state->ctm->base.id : 0;
> -	else if (property == config->gamma_lut_property)
> -		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (property == config->shaper_lut_property)
>  		*val = (state->shaper_lut) ? state->shaper_lut->base.id : 0;
> +	else if (property == config->lut3d_property)
> +		*val = (state->lut3d) ? state->lut3d->base.id : 0;
> +	else if (property == config->gamma_lut_property)
> +		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (property == config->prop_out_fence_ptr)
>  		*val = 0;
>  	else if (property == crtc->scaling_filter_property)
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 4f57dc60fe03..696fe1e37801 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -87,6 +87,25 @@
>   *	publish the largest size, and sub-sample smaller sized LUTs
>   *	appropriately.
>   *
> + * “LUT3D”:
> + *	Blob property to set the 3D LUT mapping pixel data after the color
> + *	transformation matrix and before gamma 1D lut correction. The
> + *	data is interpreted as an array of &struct drm_color_lut elements.
> + *	Hardware might choose not to use the full precision of the LUT
> + *	elements.
> + *
> + *	Setting this to NULL (blob property value set to 0) means a the output
> + *	color is identical to the input color. This is generally the driver
> + *	boot-up state too. Drivers can access this blob through
> + *	&drm_crtc_state.gamma_lut.
> + *
> + * “LUT3D_SIZE”:
> + *	Unsigned range property to give the size of the 3D lookup table to be
> + *	set on the LUT3D property (the size depends on the underlying
> + *	hardware). If drivers support multiple LUT sizes then they should
> + *	publish the largest size, and sub-sample smaller sized LUTs
> + *	appropriately.
> + *
>   * “GAMMA_LUT”:
>   *	Blob property to set the gamma lookup table (LUT) mapping pixel data
>   *	after the transformation matrix to data sent to the connector. The
> @@ -204,6 +223,45 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  }
>  EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
>  
> +/**
> + * drm_crtc_enable_lut3d - enable 3D LUT properties
> + * @crtc: DRM CRTC
> + * @shaper_lut_size: the size of shaper lut
> + * @lut3d_size: the size of 3D lut
> + *
> + * This function lets the driver enable the 3D LUT color correction property
> + * on a CRTC. This includes 3D LUT and also a shaper LUT, if set. The shaper
> + * LUT property is only attached if its size is not 0 and 3D LUT is set, being
> + * therefore optional.
> + */
> +void drm_crtc_enable_lut3d(struct drm_crtc *crtc,
> +			   uint shaper_lut_size,
> +			   uint lut3d_size)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (!lut3d_size)
> +		return;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->lut3d_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->lut3d_size_property,
> +				   lut3d_size);
> +	if (!shaper_lut_size)
> +		return;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->shaper_lut_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->shaper_lut_size_property,
> +				   lut3d_size);
> +
> +}
> +EXPORT_SYMBOL(drm_crtc_enable_lut3d);
> +
> +
>  /**
>   * drm_mode_crtc_set_gamma_size - set the gamma table size
>   * @crtc: CRTC to set the gamma table size for
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index bdd33817d968..358c528c7c80 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1069,6 +1069,8 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
>  		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
>  		replaced |= drm_property_replace_blob(&crtc_state->shaper_lut,
>  						      NULL);
> +		replaced |= drm_property_replace_blob(&crtc_state->lut3d,
> +						      NULL);
>  		replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
>  						      gamma_lut);
>  
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 4ba2a95b88e8..5458a7dfbe63 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -364,6 +364,20 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.shaper_lut_size_property = prop;
>  
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"LUT3D", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.lut3d_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"LUT3D_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.lut3d_size_property = prop;
> +
>  	prop = drm_property_create(dev,
>  			DRM_MODE_PROP_BLOB,
>  			"GAMMA_LUT", 0);
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 81c298488b0c..a4f054e0108f 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -59,6 +59,10 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				bool has_ctm,
>  				uint gamma_lut_size);
>  
> +void drm_crtc_enable_lut3d(struct drm_crtc *crtc,
> +			   uint shaper_lut_size,
> +			   uint lut3d_size);
> +
>  int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
>  				 int gamma_size);
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a318d5feb44b..c22ffcc4d7aa 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -165,7 +165,7 @@ struct drm_crtc_state {
>  	bool zpos_changed : 1;
>  	/**
>  	 * @color_mgmt_changed: Color management properties have changed
> -	 * (@shaper_lut, @gamma_lut, @degamma_lut or @ctm). Used by
> +	 * (@shaper_lut, @lut3d, @gamma_lut, @degamma_lut or @ctm). Used by
>  	 * the atomic helpers and drivers to steer the atomic commit control
>  	 * flow.
>  	 */
> @@ -298,6 +298,16 @@ struct drm_crtc_state {
>  	 */
>  	struct drm_property_blob *shaper_lut;
>  
> +	/**
> +	 * @lut3d:
> +	 *
> +	 * 3D Lookup table for converting pixel data. Position where it takes
> +	 * place depends on hw design, after @ctm or @gamma_lut. See
> +	 * drm_crtc_enable_color_mgmt(). The blob (if not NULL) is an array of
> +	 * &struct drm_color_lut.
> +	 */
> +	struct drm_property_blob *lut3d;
> +
>  	/**
>  	 * @target_vblank:
>  	 *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 2df7e171add9..87280694e70d 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -812,6 +812,19 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *shaper_lut_size_property;
>  
> +	/**
> +	 * @lut3d_property: Optional CRTC property to set the 3D LUT used to
> +	 * convert colors; before or after gamma conversion depends on hw
> +	 * design. A shaper LUT can be used to delinearize content before apply
> +	 * 3D LUT correction.
> +	 */
> +	struct drm_property *lut3d_property;
> +	/**
> +	 * @lut3d_size_property: Optional CRTC property for the size of the
> +	 * 3D LUT as supported by the driver (read-only).
> +	 */
> +	struct drm_property *lut3d_size_property;

I wonder if we need caps to describe more than the size of the 3DLUT,
i.e. something like what vaapi does:

https://intel.github.io/libva/structVAProcFilterCap3DLUT.html

Harry

> +
>  	/**
>  	 * @gamma_lut_property: Optional CRTC property to set the LUT used to
>  	 * convert the colors, after the CTM matrix, to the gamma space of the
Melissa Wen July 12, 2022, 2:53 p.m. UTC | #5
On 06/27, Ville Syrjälä wrote:
> On Sun, Jun 19, 2022 at 09:31:03PM -0100, Melissa Wen wrote:
> > Add 3D LUT for gammar correction using a 3D lookup table.  The position
> > in the color correction pipeline where 3D LUT is applied depends on hw
> > design, being after CTM or gamma. If just after CTM, a shaper lut must
> > be set to shape the content for a non-linear space. That details should
> > be handled by the driver according to its color capabilities.
> 
> I also cooked up a WIP 3D LUT support some time ago for Intel hw:
> https://github.com/vsyrjala/linux/commits/3dlut
> But that dried up due to having no userspace for it.
> 
> I also cooked up some basic igts for it:
> https://patchwork.freedesktop.org/series/90165/

Yes, I found your work on it, so I based part of my proposal on your
previous proposal and also the cubic-LUT by Laurent for rcar-du [1].
They helped me to find a path to expose 3D LUT caps, thanks.

> 
> <snip>
> > + * “LUT3D”:
> > + *	Blob property to set the 3D LUT mapping pixel data after the color
> > + *	transformation matrix and before gamma 1D lut correction.
> 
> On Intel hw the 3DLUT is after the gamma LUT in the pipeline, which is
> where I placed it in my branch.
> 
> There is now some discussion happening about exposing some
> kind of color pipeline description/configuration properties:
> https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11

So, initially I thought we would just map the properties according to hw
pipeline, but in fact it isn't a good path to follow and would make a
big mess, so Sebastian's proposal for a pipeline description makes a lot
of sense to me. I'll join the discussion.

Anyway, thanks for all inputs,

Melissa

[1] https://lore.kernel.org/all/20201221015730.28333-1-laurent.pinchart+renesas@ideasonboard.com/
> 
> -- 
> Ville Syrjälä
> Intel
Melissa Wen July 12, 2022, 3:01 p.m. UTC | #6
On 06/28, Harry Wentland wrote:
> 
> 
> On 6/19/22 18:31, Melissa Wen wrote:
> > Add 3D LUT for gammar correction using a 3D lookup table.  The position
> > in the color correction pipeline where 3D LUT is applied depends on hw
> > design, being after CTM or gamma. If just after CTM, a shaper lut must
> > be set to shape the content for a non-linear space. That details should
> > be handled by the driver according to its color capabilities.
> > 
> > Signed-off-by: Melissa Wen <mwen@igalia.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_state_helper.c |  3 ++
> >  drivers/gpu/drm/drm_atomic_uapi.c         | 14 +++++-
> >  drivers/gpu/drm/drm_color_mgmt.c          | 58 +++++++++++++++++++++++
> >  drivers/gpu/drm/drm_fb_helper.c           |  2 +
> >  drivers/gpu/drm/drm_mode_config.c         | 14 ++++++
> >  include/drm/drm_color_mgmt.h              |  4 ++
> >  include/drm/drm_crtc.h                    | 12 ++++-
> >  include/drm/drm_mode_config.h             | 13 +++++
> >  8 files changed, 117 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index cf0545bb6e00..64800bc41365 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -141,6 +141,8 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
> >  		drm_property_blob_get(state->ctm);
> >  	if (state->shaper_lut)
> >  		drm_property_blob_get(state->shaper_lut);
> > +	if (state->lut3d)
> > +		drm_property_blob_get(state->lut3d);
> >  	if (state->gamma_lut)
> >  		drm_property_blob_get(state->gamma_lut);
> >  
> > @@ -216,6 +218,7 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
> >  	drm_property_blob_put(state->degamma_lut);
> >  	drm_property_blob_put(state->ctm);
> >  	drm_property_blob_put(state->shaper_lut);
> > +	drm_property_blob_put(state->lut3d);
> >  	drm_property_blob_put(state->gamma_lut);
> >  }
> >  EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 6468f2a080bc..1896c0422f73 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -472,6 +472,14 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >  					&replaced);
> >  		state->color_mgmt_changed |= replaced;
> >  		return ret;
> > +	} else if (property == config->lut3d_property) {
> > +		ret = drm_atomic_replace_property_blob_from_id(dev,
> > +					&state->lut3d,
> > +					val,
> > +					-1, sizeof(struct drm_color_lut),
> > +					&replaced);
> > +		state->color_mgmt_changed |= replaced;
> > +		return ret;
> >  	} else if (property == config->gamma_lut_property) {
> >  		ret = drm_atomic_replace_property_blob_from_id(dev,
> >  					&state->gamma_lut,
> > @@ -523,10 +531,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >  		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
> >  	else if (property == config->ctm_property)
> >  		*val = (state->ctm) ? state->ctm->base.id : 0;
> > -	else if (property == config->gamma_lut_property)
> > -		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >  	else if (property == config->shaper_lut_property)
> >  		*val = (state->shaper_lut) ? state->shaper_lut->base.id : 0;
> > +	else if (property == config->lut3d_property)
> > +		*val = (state->lut3d) ? state->lut3d->base.id : 0;
> > +	else if (property == config->gamma_lut_property)
> > +		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >  	else if (property == config->prop_out_fence_ptr)
> >  		*val = 0;
> >  	else if (property == crtc->scaling_filter_property)
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index 4f57dc60fe03..696fe1e37801 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -87,6 +87,25 @@
> >   *	publish the largest size, and sub-sample smaller sized LUTs
> >   *	appropriately.
> >   *
> > + * “LUT3D”:
> > + *	Blob property to set the 3D LUT mapping pixel data after the color
> > + *	transformation matrix and before gamma 1D lut correction. The
> > + *	data is interpreted as an array of &struct drm_color_lut elements.
> > + *	Hardware might choose not to use the full precision of the LUT
> > + *	elements.
> > + *
> > + *	Setting this to NULL (blob property value set to 0) means a the output
> > + *	color is identical to the input color. This is generally the driver
> > + *	boot-up state too. Drivers can access this blob through
> > + *	&drm_crtc_state.gamma_lut.
> > + *
> > + * “LUT3D_SIZE”:
> > + *	Unsigned range property to give the size of the 3D lookup table to be
> > + *	set on the LUT3D property (the size depends on the underlying
> > + *	hardware). If drivers support multiple LUT sizes then they should
> > + *	publish the largest size, and sub-sample smaller sized LUTs
> > + *	appropriately.
> > + *
> >   * “GAMMA_LUT”:
> >   *	Blob property to set the gamma lookup table (LUT) mapping pixel data
> >   *	after the transformation matrix to data sent to the connector. The
> > @@ -204,6 +223,45 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >  }
> >  EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
> >  
> > +/**
> > + * drm_crtc_enable_lut3d - enable 3D LUT properties
> > + * @crtc: DRM CRTC
> > + * @shaper_lut_size: the size of shaper lut
> > + * @lut3d_size: the size of 3D lut
> > + *
> > + * This function lets the driver enable the 3D LUT color correction property
> > + * on a CRTC. This includes 3D LUT and also a shaper LUT, if set. The shaper
> > + * LUT property is only attached if its size is not 0 and 3D LUT is set, being
> > + * therefore optional.
> > + */
> > +void drm_crtc_enable_lut3d(struct drm_crtc *crtc,
> > +			   uint shaper_lut_size,
> > +			   uint lut3d_size)
> > +{
> > +	struct drm_device *dev = crtc->dev;
> > +	struct drm_mode_config *config = &dev->mode_config;
> > +
> > +	if (!lut3d_size)
> > +		return;
> > +
> > +	drm_object_attach_property(&crtc->base,
> > +				   config->lut3d_property, 0);
> > +	drm_object_attach_property(&crtc->base,
> > +				   config->lut3d_size_property,
> > +				   lut3d_size);
> > +	if (!shaper_lut_size)
> > +		return;
> > +
> > +	drm_object_attach_property(&crtc->base,
> > +				   config->shaper_lut_property, 0);
> > +	drm_object_attach_property(&crtc->base,
> > +				   config->shaper_lut_size_property,
> > +				   lut3d_size);
> > +
> > +}
> > +EXPORT_SYMBOL(drm_crtc_enable_lut3d);
> > +
> > +
> >  /**
> >   * drm_mode_crtc_set_gamma_size - set the gamma table size
> >   * @crtc: CRTC to set the gamma table size for
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index bdd33817d968..358c528c7c80 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -1069,6 +1069,8 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
> >  		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
> >  		replaced |= drm_property_replace_blob(&crtc_state->shaper_lut,
> >  						      NULL);
> > +		replaced |= drm_property_replace_blob(&crtc_state->lut3d,
> > +						      NULL);
> >  		replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
> >  						      gamma_lut);
> >  
> > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > index 4ba2a95b88e8..5458a7dfbe63 100644
> > --- a/drivers/gpu/drm/drm_mode_config.c
> > +++ b/drivers/gpu/drm/drm_mode_config.c
> > @@ -364,6 +364,20 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> >  		return -ENOMEM;
> >  	dev->mode_config.shaper_lut_size_property = prop;
> >  
> > +	prop = drm_property_create(dev,
> > +			DRM_MODE_PROP_BLOB,
> > +			"LUT3D", 0);
> > +	if (!prop)
> > +		return -ENOMEM;
> > +	dev->mode_config.lut3d_property = prop;
> > +
> > +	prop = drm_property_create_range(dev,
> > +			DRM_MODE_PROP_IMMUTABLE,
> > +			"LUT3D_SIZE", 0, UINT_MAX);
> > +	if (!prop)
> > +		return -ENOMEM;
> > +	dev->mode_config.lut3d_size_property = prop;
> > +
> >  	prop = drm_property_create(dev,
> >  			DRM_MODE_PROP_BLOB,
> >  			"GAMMA_LUT", 0);
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index 81c298488b0c..a4f054e0108f 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -59,6 +59,10 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >  				bool has_ctm,
> >  				uint gamma_lut_size);
> >  
> > +void drm_crtc_enable_lut3d(struct drm_crtc *crtc,
> > +			   uint shaper_lut_size,
> > +			   uint lut3d_size);
> > +
> >  int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
> >  				 int gamma_size);
> >  
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index a318d5feb44b..c22ffcc4d7aa 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -165,7 +165,7 @@ struct drm_crtc_state {
> >  	bool zpos_changed : 1;
> >  	/**
> >  	 * @color_mgmt_changed: Color management properties have changed
> > -	 * (@shaper_lut, @gamma_lut, @degamma_lut or @ctm). Used by
> > +	 * (@shaper_lut, @lut3d, @gamma_lut, @degamma_lut or @ctm). Used by
> >  	 * the atomic helpers and drivers to steer the atomic commit control
> >  	 * flow.
> >  	 */
> > @@ -298,6 +298,16 @@ struct drm_crtc_state {
> >  	 */
> >  	struct drm_property_blob *shaper_lut;
> >  
> > +	/**
> > +	 * @lut3d:
> > +	 *
> > +	 * 3D Lookup table for converting pixel data. Position where it takes
> > +	 * place depends on hw design, after @ctm or @gamma_lut. See
> > +	 * drm_crtc_enable_color_mgmt(). The blob (if not NULL) is an array of
> > +	 * &struct drm_color_lut.
> > +	 */
> > +	struct drm_property_blob *lut3d;
> > +
> >  	/**
> >  	 * @target_vblank:
> >  	 *
> > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > index 2df7e171add9..87280694e70d 100644
> > --- a/include/drm/drm_mode_config.h
> > +++ b/include/drm/drm_mode_config.h
> > @@ -812,6 +812,19 @@ struct drm_mode_config {
> >  	 */
> >  	struct drm_property *shaper_lut_size_property;
> >  
> > +	/**
> > +	 * @lut3d_property: Optional CRTC property to set the 3D LUT used to
> > +	 * convert colors; before or after gamma conversion depends on hw
> > +	 * design. A shaper LUT can be used to delinearize content before apply
> > +	 * 3D LUT correction.
> > +	 */
> > +	struct drm_property *lut3d_property;
> > +	/**
> > +	 * @lut3d_size_property: Optional CRTC property for the size of the
> > +	 * 3D LUT as supported by the driver (read-only).
> > +	 */
> > +	struct drm_property *lut3d_size_property;
> 
> I wonder if we need caps to describe more than the size of the 3DLUT,
> i.e. something like what vaapi does:
> 
> https://intel.github.io/libva/structVAProcFilterCap3DLUT.html

Makes sense. I'll keep it in mind when working on a next version.

Thanks,

Melissa
> 
> Harry
> 
> > +
> >  	/**
> >  	 * @gamma_lut_property: Optional CRTC property to set the LUT used to
> >  	 * convert the colors, after the CTM matrix, to the gamma space of the
Joshua Ashton Jan. 9, 2023, 12:50 p.m. UTC | #7
On 6/27/22 13:18, Ville Syrjälä wrote:
> On Sun, Jun 19, 2022 at 09:31:03PM -0100, Melissa Wen wrote:
>> Add 3D LUT for gammar correction using a 3D lookup table.  The position
>> in the color correction pipeline where 3D LUT is applied depends on hw
>> design, being after CTM or gamma. If just after CTM, a shaper lut must
>> be set to shape the content for a non-linear space. That details should
>> be handled by the driver according to its color capabilities.
> 
> I also cooked up a WIP 3D LUT support some time ago for Intel hw:
> https://github.com/vsyrjala/linux/commits/3dlut
> But that dried up due to having no userspace for it.
> 
> I also cooked up some basic igts for it:
> https://patchwork.freedesktop.org/series/90165/
> 
> <snip>
>> + * “LUT3D”:
>> + *	Blob property to set the 3D LUT mapping pixel data after the color
>> + *	transformation matrix and before gamma 1D lut correction.
> 
> On Intel hw the 3DLUT is after the gamma LUT in the pipeline, which is
> where I placed it in my branch.
> 

If the problem here in getting stuff moving for 3D LUT support in DRM is 
lack of a userspace that wants to use it, I would like to just make 
people aware that we are planning on shipping support for this in 
Gamescope/SteamOS.
(It is hooked up right now in the current Gamescope main branch).

We have pulled the patches for AMDGPU by Melissa into our tree and 
hooked it up (with a prefix VALVE1_ before the properties for now as 
stuff is not upstream in the kernel yet) and it seems to be working well 
right now.

I know that the work here not final, and we will definitely change it 
and update our kernel and userspace impl to accomodate that and are more 
than happy to provide testing for this work and other color work.

I understand there is a lot moving right now, with the new color API 
being proposed, etc; but I don't think this should necessarily require 
us blocking any 3D LUT, shaper LUT or other color work on the 
"legacy"(?) path while stuff there is being planned out.

I think it's really important that we keep moving with color support on 
the legacy path while the new one is being planned out to ensure we 
don't accidentally miss something later or end up with something 
suboptimal for a specific vendor.

- Joshie 
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index cf0545bb6e00..64800bc41365 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -141,6 +141,8 @@  void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 		drm_property_blob_get(state->ctm);
 	if (state->shaper_lut)
 		drm_property_blob_get(state->shaper_lut);
+	if (state->lut3d)
+		drm_property_blob_get(state->lut3d);
 	if (state->gamma_lut)
 		drm_property_blob_get(state->gamma_lut);
 
@@ -216,6 +218,7 @@  void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
 	drm_property_blob_put(state->degamma_lut);
 	drm_property_blob_put(state->ctm);
 	drm_property_blob_put(state->shaper_lut);
+	drm_property_blob_put(state->lut3d);
 	drm_property_blob_put(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 6468f2a080bc..1896c0422f73 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -472,6 +472,14 @@  static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
+	} else if (property == config->lut3d_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+					&state->lut3d,
+					val,
+					-1, sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed |= replaced;
+		return ret;
 	} else if (property == config->gamma_lut_property) {
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->gamma_lut,
@@ -523,10 +531,12 @@  drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
 	else if (property == config->ctm_property)
 		*val = (state->ctm) ? state->ctm->base.id : 0;
-	else if (property == config->gamma_lut_property)
-		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	else if (property == config->shaper_lut_property)
 		*val = (state->shaper_lut) ? state->shaper_lut->base.id : 0;
+	else if (property == config->lut3d_property)
+		*val = (state->lut3d) ? state->lut3d->base.id : 0;
+	else if (property == config->gamma_lut_property)
+		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	else if (property == config->prop_out_fence_ptr)
 		*val = 0;
 	else if (property == crtc->scaling_filter_property)
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 4f57dc60fe03..696fe1e37801 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -87,6 +87,25 @@ 
  *	publish the largest size, and sub-sample smaller sized LUTs
  *	appropriately.
  *
+ * “LUT3D”:
+ *	Blob property to set the 3D LUT mapping pixel data after the color
+ *	transformation matrix and before gamma 1D lut correction. The
+ *	data is interpreted as an array of &struct drm_color_lut elements.
+ *	Hardware might choose not to use the full precision of the LUT
+ *	elements.
+ *
+ *	Setting this to NULL (blob property value set to 0) means a the output
+ *	color is identical to the input color. This is generally the driver
+ *	boot-up state too. Drivers can access this blob through
+ *	&drm_crtc_state.gamma_lut.
+ *
+ * “LUT3D_SIZE”:
+ *	Unsigned range property to give the size of the 3D lookup table to be
+ *	set on the LUT3D property (the size depends on the underlying
+ *	hardware). If drivers support multiple LUT sizes then they should
+ *	publish the largest size, and sub-sample smaller sized LUTs
+ *	appropriately.
+ *
  * “GAMMA_LUT”:
  *	Blob property to set the gamma lookup table (LUT) mapping pixel data
  *	after the transformation matrix to data sent to the connector. The
@@ -204,6 +223,45 @@  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 }
 EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
 
+/**
+ * drm_crtc_enable_lut3d - enable 3D LUT properties
+ * @crtc: DRM CRTC
+ * @shaper_lut_size: the size of shaper lut
+ * @lut3d_size: the size of 3D lut
+ *
+ * This function lets the driver enable the 3D LUT color correction property
+ * on a CRTC. This includes 3D LUT and also a shaper LUT, if set. The shaper
+ * LUT property is only attached if its size is not 0 and 3D LUT is set, being
+ * therefore optional.
+ */
+void drm_crtc_enable_lut3d(struct drm_crtc *crtc,
+			   uint shaper_lut_size,
+			   uint lut3d_size)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (!lut3d_size)
+		return;
+
+	drm_object_attach_property(&crtc->base,
+				   config->lut3d_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->lut3d_size_property,
+				   lut3d_size);
+	if (!shaper_lut_size)
+		return;
+
+	drm_object_attach_property(&crtc->base,
+				   config->shaper_lut_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->shaper_lut_size_property,
+				   lut3d_size);
+
+}
+EXPORT_SYMBOL(drm_crtc_enable_lut3d);
+
+
 /**
  * drm_mode_crtc_set_gamma_size - set the gamma table size
  * @crtc: CRTC to set the gamma table size for
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index bdd33817d968..358c528c7c80 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1069,6 +1069,8 @@  static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
 		replaced |= drm_property_replace_blob(&crtc_state->shaper_lut,
 						      NULL);
+		replaced |= drm_property_replace_blob(&crtc_state->lut3d,
+						      NULL);
 		replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
 						      gamma_lut);
 
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 4ba2a95b88e8..5458a7dfbe63 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -364,6 +364,20 @@  static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.shaper_lut_size_property = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"LUT3D", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.lut3d_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"LUT3D_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.lut3d_size_property = prop;
+
 	prop = drm_property_create(dev,
 			DRM_MODE_PROP_BLOB,
 			"GAMMA_LUT", 0);
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 81c298488b0c..a4f054e0108f 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -59,6 +59,10 @@  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				bool has_ctm,
 				uint gamma_lut_size);
 
+void drm_crtc_enable_lut3d(struct drm_crtc *crtc,
+			   uint shaper_lut_size,
+			   uint lut3d_size);
+
 int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
 				 int gamma_size);
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index a318d5feb44b..c22ffcc4d7aa 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -165,7 +165,7 @@  struct drm_crtc_state {
 	bool zpos_changed : 1;
 	/**
 	 * @color_mgmt_changed: Color management properties have changed
-	 * (@shaper_lut, @gamma_lut, @degamma_lut or @ctm). Used by
+	 * (@shaper_lut, @lut3d, @gamma_lut, @degamma_lut or @ctm). Used by
 	 * the atomic helpers and drivers to steer the atomic commit control
 	 * flow.
 	 */
@@ -298,6 +298,16 @@  struct drm_crtc_state {
 	 */
 	struct drm_property_blob *shaper_lut;
 
+	/**
+	 * @lut3d:
+	 *
+	 * 3D Lookup table for converting pixel data. Position where it takes
+	 * place depends on hw design, after @ctm or @gamma_lut. See
+	 * drm_crtc_enable_color_mgmt(). The blob (if not NULL) is an array of
+	 * &struct drm_color_lut.
+	 */
+	struct drm_property_blob *lut3d;
+
 	/**
 	 * @target_vblank:
 	 *
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 2df7e171add9..87280694e70d 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -812,6 +812,19 @@  struct drm_mode_config {
 	 */
 	struct drm_property *shaper_lut_size_property;
 
+	/**
+	 * @lut3d_property: Optional CRTC property to set the 3D LUT used to
+	 * convert colors; before or after gamma conversion depends on hw
+	 * design. A shaper LUT can be used to delinearize content before apply
+	 * 3D LUT correction.
+	 */
+	struct drm_property *lut3d_property;
+	/**
+	 * @lut3d_size_property: Optional CRTC property for the size of the
+	 * 3D LUT as supported by the driver (read-only).
+	 */
+	struct drm_property *lut3d_size_property;
+
 	/**
 	 * @gamma_lut_property: Optional CRTC property to set the LUT used to
 	 * convert the colors, after the CTM matrix, to the gamma space of the