[PATCH v4 09/32] drm/amd/display: add plane 3D LUT driver-specific properties

Joshua Ashton joshua at froggi.es
Thu Nov 2 03:48:41 UTC 2023



On 10/5/23 18:15, Melissa Wen wrote:
> Add 3D LUT property for plane color transformations using a 3D lookup
> table. 3D LUT allows for highly accurate and complex color
> transformations and is suitable to adjust the balance between color
> channels. It's also more complex to manage and require more
> computational resources.
> 
> Since a 3D LUT has a limited number of entries in each dimension we want
> to use them in an optimal fashion. This means using the 3D LUT in a
> colorspace that is optimized for human vision, such as sRGB, PQ, or
> another non-linear space. Therefore, userpace may need one 1D LUT
> (shaper) before it to delinearize content and another 1D LUT after 3D
> LUT (blend) to linearize content again for blending. The next patches
> add these 1D LUTs to the plane color mgmt pipeline.
> 
> v3:
> - improve commit message about 3D LUT
> - describe the 3D LUT entries and size (Harry)
> 
> v4:
> - advertise 3D LUT max size as the size of a single-dimension
> 
> Signed-off-by: Melissa Wen <mwen at igalia.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h      | 18 +++++++++++++++
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  9 ++++++++
>   .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 14 +++++++++++
>   .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 23 +++++++++++++++++++
>   4 files changed, 64 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 62044d41da75..f7adaa52c23f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -363,6 +363,24 @@ struct amdgpu_mode_info {
>   	 * @plane_hdr_mult_property:
>   	 */
>   	struct drm_property *plane_hdr_mult_property;
> +	/**
> +	 * @plane_lut3d_property: Plane property for color transformation using
> +	 * a 3D LUT (pre-blending), a three-dimensional array where each
> +	 * element is an RGB triplet. Each dimension has a size of the cubed
> +	 * root of lut3d_size. The array contains samples from the approximated
> +	 * function. On AMD, values between samples are estimated by
> +	 * tetrahedral interpolation. The array is accessed with three indices,
> +	 * one for each input dimension (color channel), blue being the
> +	 * outermost dimension, red the innermost.
> +	 */
> +	struct drm_property *plane_lut3d_property;
> +	/**
> +	 * @plane_degamma_lut_size_property: Plane property to define the max
> +	 * size of 3D LUT as supported by the driver (read-only). The max size
> +	 * is the max size of one dimension and, therefore, the max number of
> +	 * entries for 3D LUT array is the 3D LUT size cubed;
> +	 */
> +	struct drm_property *plane_lut3d_size_property;
>   };
>   
>   #define AMDGPU_MAX_BL_LEVEL 0xFF
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index bb2ce843369d..7a2350c62cf1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -784,6 +784,11 @@ struct dm_plane_state {
>   	 * TF is needed for any subsequent linear-to-non-linear transforms.
>   	 */
>   	__u64 hdr_mult;
> +	/**
> +	 * @lut3d: 3D lookup table blob. The blob (if not NULL) is an array of
> +	 * &struct drm_color_lut.
> +	 */
> +	struct drm_property_blob *lut3d;
>   };
>   
>   struct dm_crtc_state {
> @@ -869,6 +874,10 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>   
>   void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
>   
> +/* 3D LUT max size is 17x17x17 (4913 entries) */
> +#define MAX_COLOR_3DLUT_SIZE 17
> +#define MAX_COLOR_3DLUT_BITDEPTH 12
> +/* 1D LUT size */
>   #define MAX_COLOR_LUT_ENTRIES 4096
>   /* Legacy gamm LUT users such as X doesn't like large LUT sizes */
>   #define MAX_COLOR_LEGACY_LUT_ENTRIES 256
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index caf49a044ab4..011f2f9ec890 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -230,6 +230,20 @@ amdgpu_dm_create_color_properties(struct amdgpu_device *adev)
>   		return -ENOMEM;
>   	adev->mode_info.plane_hdr_mult_property = prop;
>   
> +	prop = drm_property_create(adev_to_drm(adev),
> +				   DRM_MODE_PROP_BLOB,
> +				   "AMD_PLANE_LUT3D", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	adev->mode_info.plane_lut3d_property = prop;
> +
> +	prop = drm_property_create_range(adev_to_drm(adev),
> +					 DRM_MODE_PROP_IMMUTABLE,
> +					 "AMD_PLANE_LUT3D_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	adev->mode_info.plane_lut3d_size_property = prop;
> +
>   	return 0;
>   }
>   #endif
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index ae64d4b73360..068798ffdd56 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1359,6 +1359,8 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
>   
>   	if (dm_plane_state->degamma_lut)
>   		drm_property_blob_get(dm_plane_state->degamma_lut);
> +	if (dm_plane_state->lut3d)
> +		drm_property_blob_get(dm_plane_state->lut3d);

This is wrong, and was breaking state restoration on suspend.

dm_plane_state->lut3d is always NULL as it's the new zero-ed state, you 
need to check the old_dm_plane_state's values and then set it on the new 
state after _get.

It should be like this: (based on end of the patchset).

	if (old_dm_plane_state->degamma_lut) {
		drm_property_blob_get(old_dm_plane_state->degamma_lut);
		dm_plane_state->degamma_lut = old_dm_plane_state->degamma_lut;
	}
	if (old_dm_plane_state->ctm) {
		drm_property_blob_get(old_dm_plane_state->ctm);
		dm_plane_state->ctm = old_dm_plane_state->ctm;
	}
	if (old_dm_plane_state->shaper_lut) {
		drm_property_blob_get(old_dm_plane_state->shaper_lut);
		dm_plane_state->shaper_lut = old_dm_plane_state->shaper_lut;
	}
	if (old_dm_plane_state->lut3d) {
		drm_property_blob_get(old_dm_plane_state->lut3d);
		dm_plane_state->lut3d = old_dm_plane_state->lut3d;
	}
	if (old_dm_plane_state->blend_lut) {
		drm_property_blob_get(old_dm_plane_state->blend_lut);
		dm_plane_state->blend_lut = old_dm_plane_state->blend_lut;
	}

because it doesn't do the memcpy like the base drm atomic stuff.

I also had to add a patch to ensure color mgmt was updated on unsuspend 
always too, but that's not related to this patchset and also affected 
stuff beforehand.

I'll send that patch in a bit.

Thanks!
- Joshie 🐸✨

>   
>   	dm_plane_state->degamma_tf = old_dm_plane_state->degamma_tf;
>   	dm_plane_state->hdr_mult = old_dm_plane_state->hdr_mult;
> @@ -1432,6 +1434,8 @@ static void dm_drm_plane_destroy_state(struct drm_plane *plane,
>   
>   	if (dm_plane_state->degamma_lut)
>   		drm_property_blob_put(dm_plane_state->degamma_lut);
> +	if (dm_plane_state->lut3d)
> +		drm_property_blob_put(dm_plane_state->lut3d);
>   
>   	if (dm_plane_state->dc_state)
>   		dc_plane_state_release(dm_plane_state->dc_state);
> @@ -1462,6 +1466,14 @@ dm_atomic_plane_attach_color_mgmt_properties(struct amdgpu_display_manager *dm,
>   	drm_object_attach_property(&plane->base,
>   				   dm->adev->mode_info.plane_hdr_mult_property,
>   				   AMDGPU_HDR_MULT_DEFAULT);
> +
> +	if (dpp_color_caps.hw_3d_lut) {
> +		drm_object_attach_property(&plane->base,
> +					   mode_info.plane_lut3d_property, 0);
> +		drm_object_attach_property(&plane->base,
> +					   mode_info.plane_lut3d_size_property,
> +					   MAX_COLOR_3DLUT_SIZE);
> +	}
>   }
>   
>   static int
> @@ -1493,6 +1505,14 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
>   			dm_plane_state->hdr_mult = val;
>   			dm_plane_state->base.color_mgmt_changed = 1;
>   		}
> +	} else if (property == adev->mode_info.plane_lut3d_property) {
> +		ret = drm_property_replace_blob_from_id(plane->dev,
> +							&dm_plane_state->lut3d,
> +							val, -1,
> +							sizeof(struct drm_color_lut),
> +							&replaced);
> +		dm_plane_state->base.color_mgmt_changed |= replaced;
> +		return ret;
>   	} else {
>   		drm_dbg_atomic(plane->dev,
>   			       "[PLANE:%d:%s] unknown property [PROP:%d:%s]]\n",
> @@ -1520,6 +1540,9 @@ dm_atomic_plane_get_property(struct drm_plane *plane,
>   		*val = dm_plane_state->degamma_tf;
>   	} else if (property == adev->mode_info.plane_hdr_mult_property) {
>   		*val = dm_plane_state->hdr_mult;
> +	} else 	if (property == adev->mode_info.plane_lut3d_property) {
> +		*val = (dm_plane_state->lut3d) ?
> +			dm_plane_state->lut3d->base.id : 0;
>   	} else {
>   		return -EINVAL;
>   	}


More information about the dri-devel mailing list