Re: [v6 15/16] drm/i915/color: Add 3D LUT to color pipeline

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 11/18/2025 2:20 PM, Kandpal, Suraj wrote:
>> Subject: [v6 15/16] drm/i915/color: Add 3D LUT to color pipeline
>>
>> From: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>>
>> Add helpers to program the 3D LUT registers and arm them.
>>
>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_color.c    | 57 +++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_color.h    |  2 +
>>   .../drm/i915/display/intel_color_pipeline.c   | 35 +++++++++++-
>>   .../drm/i915/display/intel_color_pipeline.h   |  3 +-
>>   .../drm/i915/display/intel_display_limits.h   |  1 +
>>   .../drm/i915/display/intel_display_types.h    |  2 +-
>>   drivers/gpu/drm/i915/display/intel_plane.c    |  2 +
>>   7 files changed, 97 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>> b/drivers/gpu/drm/i915/display/intel_color.c
>> index f290432f3c31..ab9e889ce19f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -4061,6 +4061,44 @@ xelpd_plane_load_luts(struct intel_dsb *dsb,
>> const struct intel_plane_state *pla
>>   		xelpd_program_plane_post_csc_lut(dsb, plane_state);  }
>>
>> +static u32 glk_3dlut_10(const struct drm_color_lut32 *color) {
>> +	return REG_FIELD_PREP(LUT_3D_DATA_RED_MASK,
>> drm_color_lut32_extract(color->red, 10)) |
>> +		REG_FIELD_PREP(LUT_3D_DATA_GREEN_MASK,
>> drm_color_lut32_extract(color->green, 10)) |
>> +		REG_FIELD_PREP(LUT_3D_DATA_BLUE_MASK,
>> +drm_color_lut32_extract(color->blue, 10)); }
>> +
>> +static void glk_load_lut_3d(struct intel_dsb *dsb,
>> +			    struct intel_crtc *crtc,
>> +			    const struct drm_property_blob *blob) {
>> +	struct intel_display *display = to_intel_display(crtc->base.dev);
>> +	const struct drm_color_lut32 *lut = blob->data;
>> +	int i, lut_size = drm_color_lut32_size(blob);
>> +	enum pipe pipe = crtc->pipe;
>> +
>> +	WARN_ON(intel_de_read(display, LUT_3D_CTL(pipe)) &
>> LUT_3D_READY);
>> +	intel_de_write_dsb(display, dsb, LUT_3D_INDEX(pipe),
>> LUT_3D_AUTO_INCREMENT);
>> +	for (i = 0; i < lut_size; i++)
>> +		intel_de_write_dsb(display, dsb, LUT_3D_DATA(pipe),
>> glk_3dlut_10(&lut[i]));
>> +	intel_de_write_dsb(display, dsb, LUT_3D_INDEX(pipe), 0); }
>> +
>> +static void glk_lut_3d_commit(struct intel_dsb *dsb, struct intel_crtc
>> +*crtc, bool enable) {
>> +	struct intel_display *display = to_intel_display(crtc);
>> +	enum pipe pipe = crtc->pipe;
>> +	u32 val;
>> +
>> +	WARN_ON(intel_de_read(display, LUT_3D_CTL(pipe)) &
>> LUT_3D_READY);
>> +
>> +	if (enable)
>> +		val = LUT_3D_ENABLE | LUT_3D_READY |
>> LUT_3D_BIND_PLANE_1;
>> +	else
>> +		val = 0;
>> +	intel_de_write_dsb(display, dsb, LUT_3D_CTL(pipe), val); }
>> +
>>   static const struct intel_color_funcs chv_color_funcs = {
>>   	.color_check = chv_color_check,
>>   	.color_commit_arm = i9xx_color_commit_arm, @@ -4190,6 +4228,14
>> @@ static const struct intel_color_funcs ilk_color_funcs = {
>>   	.get_config = ilk_get_config,
>>   };
>>
>> +void intel_color_plane_commit_arm(struct intel_dsb *dsb,
>> +				  const struct intel_plane_state *plane_state)
>> {
>> +	struct intel_crtc *crtc = to_intel_crtc(plane_state->uapi.crtc);
>> +
>> +	glk_lut_3d_commit(dsb, crtc, !!plane_state->hw.lut_3d); }
>> +
>>   static void
>>   intel_color_load_plane_csc_matrix(struct intel_dsb *dsb,
>>   				  const struct intel_plane_state *plane_state)
>> @@ -4210,6 +4256,15 @@ intel_color_load_plane_luts(struct intel_dsb *dsb,
>>   		display->funcs.color->load_plane_luts(dsb, plane_state);  }
>>
>> +static void
>> +intel_color_load_3dlut(struct intel_dsb *dsb,
>> +		       const struct intel_plane_state *plane_state) {
>> +	struct intel_crtc *crtc = to_intel_crtc(plane_state->uapi.crtc);
>> +
>> +	glk_load_lut_3d(dsb, crtc, plane_state->hw.lut_3d); }
>> +
>>   void intel_color_plane_program_pipeline(struct intel_dsb *dsb,
>>   					const struct intel_plane_state
>> *plane_state)  { @@ -4217,6 +4272,8 @@ void
>> intel_color_plane_program_pipeline(struct intel_dsb *dsb,
>>   		intel_color_load_plane_csc_matrix(dsb, plane_state);
>>   	if (plane_state->hw.degamma_lut || plane_state->hw.gamma_lut)
>>   		intel_color_load_plane_luts(dsb, plane_state);
>> +	if (plane_state->hw.lut_3d)
>> +		intel_color_load_3dlut(dsb, plane_state);
>>   }
>>
>>   void intel_color_crtc_init(struct intel_crtc *crtc) diff --git
>> a/drivers/gpu/drm/i915/display/intel_color.h
>> b/drivers/gpu/drm/i915/display/intel_color.h
>> index 8051c827a1d8..a70df0c2bd9a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.h
>> +++ b/drivers/gpu/drm/i915/display/intel_color.h
>> @@ -43,4 +43,6 @@ bool intel_color_lut_equal(const struct intel_crtc_state
>> *crtc_state,  void intel_color_assert_luts(const struct intel_crtc_state
>> *crtc_state);  void intel_color_plane_program_pipeline(struct intel_dsb *dsb,
>>   					const struct intel_plane_state
>> *plane_state);
>> +void intel_color_plane_commit_arm(struct intel_dsb *dsb,
>> +				  const struct intel_plane_state *plane_state);
>>   #endif /* __INTEL_COLOR_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> index b6e6ebdd0dff..fd7d63ce6d9f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
>> @@ -12,11 +12,24 @@
>>   #define PLANE_DEGAMMA_SIZE 128
>>   #define PLANE_GAMMA_SIZE 32
>>
>> +static inline bool
>> +intel_crtc_has_lut_3d(struct intel_display *display, enum pipe pipe) {
>> +	if (DISPLAY_VER(display) >= 12)
>> +		return pipe == PIPE_A || pipe == PIPE_B;
>> +	else if (DISPLAY_VER(display) >= 10 /*|| IS_GEMINILAKE(display)*/)
>> +		return pipe == PIPE_A;
> 
> Is this required specially if this function is only used for color pipeline which
> Is only being exposed for DISPLAY_VER(display) >= 12 so maybe the else if part is not needed
> 

This will be needed once we have the *PIPE* color pipeline but for now 
we can skip.

==
Chaitanya

> Regards,
> Suraj Kandpal
> 
>> +	else
>> +		return false;
>> +}
>> +
>>   static
>> -int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
>> drm_prop_enum_list *list)
>> +int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
>> drm_prop_enum_list *list,
>> +				     enum pipe pipe)
>>   {
>>   	struct intel_colorop *colorop;
>>   	struct drm_device *dev = plane->dev;
>> +	struct intel_display *display = to_intel_display(dev);
>>   	int ret;
>>   	struct drm_colorop *prev_op;
>>
>> @@ -36,6 +49,22 @@ int _intel_color_pipeline_plane_init(struct drm_plane
>> *plane, struct drm_prop_en
>>   	/* TODO: handle failures and clean up */
>>   	prev_op = &colorop->base;
>>
>> +	if (DISPLAY_VER(display) >= 35 &&
>> +	    intel_crtc_has_lut_3d(display, pipe) &&
>> +	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
>> +		colorop = intel_colorop_create(INTEL_PLANE_CB_3DLUT);
>> +
>> +		ret = drm_plane_colorop_3dlut_init(dev, &colorop->base,
>> plane, 17,
>> +
>> DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL,
>> +						   true);
>> +		if (ret)
>> +			return ret;
>> +
>> +		drm_colorop_set_next_property(prev_op, &colorop->base);
>> +
>> +		prev_op = &colorop->base;
>> +	}
>> +
>>   	colorop = intel_colorop_create(INTEL_PLANE_CB_CSC);
>>   	ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane,
>>
>> DRM_COLOROP_FLAG_ALLOW_BYPASS); @@ -58,7 +87,7 @@ int
>> _intel_color_pipeline_plane_init(struct drm_plane *plane, struct
>> drm_prop_en
>>   	return 0;
>>   }
>>
>> -int intel_color_pipeline_plane_init(struct drm_plane *plane)
>> +int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe
>> +pipe)
>>   {
>>   	struct drm_device *dev = plane->dev;
>>   	struct intel_display *display = to_intel_display(dev); @@ -77,7 +106,7
>> @@ int intel_color_pipeline_plane_init(struct drm_plane *plane)
>>   	len++;
>>
>>   	/* Add pipeline consisting of transfer functions */
>> -	ret = _intel_color_pipeline_plane_init(plane, &pipelines[len]);
>> +	ret = _intel_color_pipeline_plane_init(plane, &pipelines[len], pipe);
>>   	if (ret)
>>   		return ret;
>>   	len++;
>> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.h
>> b/drivers/gpu/drm/i915/display/intel_color_pipeline.h
>> index 7f1d32bc9202..a457d306da7f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.h
>> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.h
>> @@ -7,7 +7,8 @@
>>   #define __INTEL_COLOR_PIPELINE_H__
>>
>>   struct drm_plane;
>> +enum pipe;
>>
>> -int intel_color_pipeline_plane_init(struct drm_plane *plane);
>> +int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe
>> +pipe);
>>
>>   #endif /* __INTEL_COLOR_PIPELINE_H__ */ diff --git
>> a/drivers/gpu/drm/i915/display/intel_display_limits.h
>> b/drivers/gpu/drm/i915/display/intel_display_limits.h
>> index 55fd574ba313..cb3c9c665c44 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_limits.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_limits.h
>> @@ -142,6 +142,7 @@ enum intel_color_block {
>>   	INTEL_PLANE_CB_PRE_CSC_LUT,
>>   	INTEL_PLANE_CB_CSC,
>>   	INTEL_PLANE_CB_POST_CSC_LUT,
>> +	INTEL_PLANE_CB_3DLUT,
>>
>>   	INTEL_CB_MAX
>>   };
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>> b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index d8fe80a55601..50a14f75b727 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -646,7 +646,7 @@ struct intel_plane_state {
>>   		enum drm_color_encoding color_encoding;
>>   		enum drm_color_range color_range;
>>   		enum drm_scaling_filter scaling_filter;
>> -		struct drm_property_blob *ctm, *degamma_lut,
>> *gamma_lut;
>> +		struct drm_property_blob *ctm, *degamma_lut, *gamma_lut,
>> *lut_3d;
>>   	} hw;
>>
>>   	struct i915_vma *ggtt_vma;
>> diff --git a/drivers/gpu/drm/i915/display/intel_plane.c
>> b/drivers/gpu/drm/i915/display/intel_plane.c
>> index 298f8e186fee..7c230a2832c2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_plane.c
>> +++ b/drivers/gpu/drm/i915/display/intel_plane.c
>> @@ -348,6 +348,8 @@ intel_plane_colorop_replace_blob(struct
>> intel_plane_state *plane_state,
>>   		return	drm_property_replace_blob(&plane_state-
>>> hw.degamma_lut, blob);
>>   	else if (intel_colorop->id == INTEL_PLANE_CB_POST_CSC_LUT)
>>   		return drm_property_replace_blob(&plane_state-
>>> hw.gamma_lut, blob);
>> +	else if (intel_colorop->id == INTEL_PLANE_CB_3DLUT)
>> +		return	drm_property_replace_blob(&plane_state-
>>> hw.lut_3d, blob);
>>
>>   	return false;
>>   }
>> --
>> 2.50.1
>