Re: [PATCH 2/2] drm/i915/color: Attach the 3D LUT block to required DE Plane.

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 06 Feb 2026, Austin Hu <austin.hu@intel.com> wrote:
> Or attach to Pipe directly for the unsupported Plane(s).

Insufficient commit message. Please don't add a period at the end of the
subject.

>
> Signed-off-by: Austin Hu <austin.hu@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c    | 29 ++++++++++++++++---
>  drivers/gpu/drm/i915/display/intel_color.h    | 11 ++-----
>  drivers/gpu/drm/i915/display/intel_plane.c    |  4 +--
>  .../drm/i915/display/skl_universal_plane.c    |  2 +-
>  4 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index e79506554..dff33c9c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -4090,7 +4090,10 @@ static void glk_load_lut_3d(struct intel_dsb *dsb,
>  	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)
> +static void glk_lut_3d_commit(struct intel_dsb *dsb,
> +		struct intel_crtc *crtc,
> +		struct intel_plane *plane,
> +		bool enable)

Please fix the indentation, that's not a style used anywhere.

>  {
>  	struct intel_display *display = to_intel_display(crtc);
>  	enum pipe pipe = crtc->pipe;
> @@ -4102,8 +4105,25 @@ static void glk_lut_3d_commit(struct intel_dsb *dsb, struct intel_crtc *crtc, bo
>  		return;
>  	}
>  
> -	if (enable)
> -		val = LUT_3D_ENABLE | LUT_3D_READY | LUT_3D_BIND_PLANE_1;
> +	if (enable) {
> +		val = LUT_3D_ENABLE | LUT_3D_READY;
> +
> +		switch (plane->id) {
> +		case PLANE_1:
> +			val |= LUT_3D_BIND_PLANE_1;
> +			break;
> +		case PLANE_2:
> +			val |= LUT_3D_BIND_PLANE_2;
> +			break;
> +		case PLANE_3:
> +			val |= LUT_3D_BIND_PLANE_3;
> +			break;
> +		default:
> +			/* Attached the 3D LUT block to Pipe. */

Attached? Or Attach? There's no need to capitalize pipe.

> +			val |= LUT_3D_BIND_PIPE;
> +			break;
> +		}
> +	}
>  
>  	intel_de_write_dsb(display, dsb, LUT_3D_CTL(pipe), val);
>  }
> @@ -4238,13 +4258,14 @@ static const struct intel_color_funcs ilk_color_funcs = {
>  };
>  
>  void intel_color_plane_commit_arm(struct intel_dsb *dsb,
> +				  struct intel_plane *plane,
>  				  const struct intel_plane_state *plane_state)
>  {
>  	struct intel_display *display = to_intel_display(plane_state);
>  	struct intel_crtc *crtc = to_intel_crtc(plane_state->uapi.crtc);
>  
>  	if (crtc && intel_color_crtc_has_3dlut(display, crtc->pipe))
> -		glk_lut_3d_commit(dsb, crtc, !!plane_state->hw.lut_3d);
> +		glk_lut_3d_commit(dsb, crtc, plane, !!plane_state->hw.lut_3d);

Just pass plane_state, and figure the rest out from that.

	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);

>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
> index c21b9bdf7..bc8192d75 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.h
> +++ b/drivers/gpu/drm/i915/display/intel_color.h
> @@ -7,15 +7,7 @@
>  #define __INTEL_COLOR_H__
>  
>  #include <linux/types.h>
> -
> -struct intel_atomic_state;
> -struct intel_crtc_state;
> -struct intel_crtc;
> -struct intel_display;
> -struct intel_dsb;
> -struct intel_plane_state;
> -struct drm_property_blob;
> -enum pipe;
> +#include "intel_display_types.h"

Absolutely not.

Please never include header from headers unless you absolutely have
to. Use forward declarations instead.

We've put a lot of effort into reducing header interdependencies, which
improves incremental build times quite nicely.

Here, it would be sufficient to add

struct intel_plane;

forward declaration, *except* you can get the plane from the plane state
with no changes in the API:

	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);

>  void intel_color_init_hooks(struct intel_display *display);
>  int intel_color_init(struct intel_display *display);
> @@ -45,6 +37,7 @@ 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,
> +				  struct intel_plane *plane,
>  				  const struct intel_plane_state *plane_state);
>  bool intel_color_crtc_has_3dlut(struct intel_display *display, enum pipe pipe);
>  #endif /* __INTEL_COLOR_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
> index ab6a58530..305e8e60f 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane.c
> @@ -345,11 +345,11 @@ intel_plane_colorop_replace_blob(struct intel_plane_state *plane_state,
>  	if (intel_colorop->id == INTEL_PLANE_CB_CSC)
>  		return drm_property_replace_blob(&plane_state->hw.ctm, blob);
>  	else if (intel_colorop->id == INTEL_PLANE_CB_PRE_CSC_LUT)
> -		return	drm_property_replace_blob(&plane_state->hw.degamma_lut, blob);
> +		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 drm_property_replace_blob(&plane_state->hw.lut_3d, blob);

Unrelated changes that don't belong in this patch.

>  
>  	return false;
>  }
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index ee8e24497..b68e222c3 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1673,7 +1673,7 @@ icl_plane_update_arm(struct intel_dsb *dsb,
>  
>  	icl_plane_update_sel_fetch_arm(dsb, plane, crtc_state, plane_state);
>  
> -	intel_color_plane_commit_arm(dsb, plane_state);
> +	intel_color_plane_commit_arm(dsb, plane, plane_state);
>  
>  	/*
>  	 * In order to have FBC for fp16 formats pixel normalizer block must be

-- 
Jani Nikula, Intel