Re: [PATCH V12 46/49] drm/amd/display: Ensure 3D LUT for color pipeline

[Harry Wentland <harry.wentland@amd.com>]

On 2025-10-29 23:42, Alex Hung wrote:
> Check dpp.hw_3d_lut before creating shaper tf/lut and 3dlut colorops in
> colorpipeline and handling these colorops.
> 
> Signed-off-by: Alex Hung <alex.hung@amd.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
> v10:
>  - Check dpp.hw_3d_lut before creating shaper tf/lut and 3dlut colorops
>    (Melissa Wen)
> 
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   |  47 ++++----
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 103 +++++++++---------
>  2 files changed, 78 insertions(+), 72 deletions(-)
> 
> 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 e6da42e7fcb3..c3ddf136dbc8 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
> @@ -1868,6 +1868,7 @@ amdgpu_dm_plane_set_colorop_properties(struct drm_plane_state *plane_state,
>  {
>  	struct drm_colorop *colorop = plane_state->color_pipeline;
>  	struct drm_device *dev = plane_state->plane->dev;
> +	struct amdgpu_device *adev = drm_to_adev(dev);
>  	int ret;
>  
>  	/* 1D Curve - DEGAM TF */
> @@ -1900,32 +1901,34 @@ amdgpu_dm_plane_set_colorop_properties(struct drm_plane_state *plane_state,
>  	if (ret)
>  		return ret;
>  
> -	/* 1D Curve & LUT - SHAPER TF & LUT */
> -	colorop = colorop->next;
> -	if (!colorop) {
> -		drm_dbg(dev, "no Shaper TF colorop found\n");
> -		return -EINVAL;
> -	}
> +	if (adev->dm.dc->caps.color.dpp.hw_3d_lut) {
> +		/* 1D Curve & LUT - SHAPER TF & LUT */
> +		colorop = colorop->next;
> +		if (!colorop) {
> +			drm_dbg(dev, "no Shaper TF colorop found\n");
> +			return -EINVAL;
> +		}
>  
> -	ret = __set_dm_plane_colorop_shaper(plane_state, dc_plane_state, colorop);
> -	if (ret)
> -		return ret;
> +		ret = __set_dm_plane_colorop_shaper(plane_state, dc_plane_state, colorop);
> +		if (ret)
> +			return ret;
>  
> -	/* Shaper LUT colorop is already handled, just skip here */
> -	colorop = colorop->next;
> -	if (!colorop)
> -		return -EINVAL;
> +		/* Shaper LUT colorop is already handled, just skip here */
> +		colorop = colorop->next;
> +		if (!colorop)
> +			return -EINVAL;
>  
> -	/* 3D LUT */
> -	colorop = colorop->next;
> -	if (!colorop) {
> -		drm_dbg(dev, "no 3D LUT colorop found\n");
> -		return -EINVAL;
> -	}
> +		/* 3D LUT */
> +		colorop = colorop->next;
> +		if (!colorop) {
> +			drm_dbg(dev, "no 3D LUT colorop found\n");
> +			return -EINVAL;
> +		}
>  
> -	ret = __set_dm_plane_colorop_3dlut(plane_state, dc_plane_state, colorop);
> -	if (ret)
> -		return ret;
> +		ret = __set_dm_plane_colorop_3dlut(plane_state, dc_plane_state, colorop);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	/* 1D Curve & LUT - BLND TF & LUT */
>  	colorop = colorop->next;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index f89a887c6b14..f205f3c31f6a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> @@ -31,6 +31,7 @@
>  
>  #include "amdgpu.h"
>  #include "amdgpu_dm_colorop.h"
> +#include "dc.h"
>  
>  const u64 amdgpu_dm_supported_degam_tfs =
>  	BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
> @@ -55,6 +56,7 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>  {
>  	struct drm_colorop *ops[MAX_COLOR_PIPELINE_OPS];
>  	struct drm_device *dev = plane->dev;
> +	struct amdgpu_device *adev = drm_to_adev(dev);
>  	int ret;
>  	int i = 0;
>  
> @@ -108,57 +110,58 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane *plane, struct drm_pr
>  
>  	i++;
>  
> -	/* 1D curve - SHAPER TF */
> -	ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
> -	if (!ops[i]) {
> -		ret = -ENOMEM;
> -		goto cleanup;
> -	}
> -
> -	ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> -					      amdgpu_dm_supported_shaper_tfs,
> -					      DRM_COLOROP_FLAG_ALLOW_BYPASS);
> -	if (ret)
> -		goto cleanup;
> -
> -	drm_colorop_set_next_property(ops[i-1], ops[i]);
> -
> -	i++;
> -
> -	/* 1D LUT - SHAPER LUT */
> -	ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
> -	if (!ops[i]) {
> -		ret = -ENOMEM;
> -		goto cleanup;
> +	if (adev->dm.dc->caps.color.dpp.hw_3d_lut) {
> +		/* 1D curve - SHAPER TF */
> +		ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
> +		if (!ops[i]) {
> +			ret = -ENOMEM;
> +			goto cleanup;
> +		}
> +
> +		ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> +						amdgpu_dm_supported_shaper_tfs,
> +						DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +		if (ret)
> +			goto cleanup;
> +
> +		drm_colorop_set_next_property(ops[i-1], ops[i]);
> +
> +		i++;
> +
> +		/* 1D LUT - SHAPER LUT */
> +		ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
> +		if (!ops[i]) {
> +			ret = -ENOMEM;
> +			goto cleanup;
> +		}
> +
> +		ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, MAX_COLOR_LUT_ENTRIES,
> +							DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> +							DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +		if (ret)
> +			goto cleanup;
> +
> +		drm_colorop_set_next_property(ops[i-1], ops[i]);
> +
> +		i++;
> +
> +		/* 3D LUT */
> +		ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
> +		if (!ops[i]) {
> +			ret = -ENOMEM;
> +			goto cleanup;
> +		}
> +
> +		ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, LUT3D_SIZE,
> +					DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
> +					DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +		if (ret)
> +			goto cleanup;
> +
> +		drm_colorop_set_next_property(ops[i-1], ops[i]);
> +
> +		i++;
>  	}
> -
> -	ret = drm_plane_colorop_curve_1d_lut_init(dev, ops[i], plane, MAX_COLOR_LUT_ENTRIES,
> -						  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> -						  DRM_COLOROP_FLAG_ALLOW_BYPASS);
> -	if (ret)
> -		goto cleanup;
> -
> -	drm_colorop_set_next_property(ops[i-1], ops[i]);
> -
> -	i++;
> -
> -	/* 3D LUT */
> -	ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
> -	if (!ops[i]) {
> -		ret = -ENOMEM;
> -		goto cleanup;
> -	}
> -
> -	ret = drm_plane_colorop_3dlut_init(dev, ops[i], plane, LUT3D_SIZE,
> -				     DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
> -				     DRM_COLOROP_FLAG_ALLOW_BYPASS);
> -	if (ret)
> -		goto cleanup;
> -
> -	drm_colorop_set_next_property(ops[i-1], ops[i]);
> -
> -	i++;
> -
>  	/* 1D curve - BLND TF */
>  	ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
>  	if (!ops[i]) {