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

[Melissa Wen <mwen@igalia.com>]

On 17/06/2025 00:17, 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>
> ---
> 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 b53aecd1bebc..c6d4a9365c00 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
> @@ -1825,6 +1825,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 */
> @@ -1857,32 +1858,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) {

No I think it will not work as expected for movable 3D LUT, since 
movable 3D LUT is a MPC capability.
I have actually sent a patch in the past to clarify this on DCN401. So, 
this check doesn't cover this driver anymore, for example.
- https://lore.kernel.org/amd-gfx/20250425205236.318520-1-mwen@igalia.com/

But I also don't know how to make a more generic check in this movable 
3D LUT case.
Any better idea?

Melissa

> +		/* 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 680b4e783959..fdb653548c9a 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]) {