Re: [PATCH v1 2/2] drm/i915/xe2lpd+: disable 3D LUT if unsupported

On Tue, 28 Jul 2026, Michał Grzelak <[email protected]> wrote:
> XE2LPD_DE_CAP_3DLUT_MASK is defined but never used. Check with it if
> 3DLUT is enabled.
>
> Check DE_CAP register if 3D LUT is disabled. Store the info in a new
> field of struct intel_display_runtime_info.
>
> Add macros for checking both 3D LUT hardware and 3D LUT capability.

What's the difference?

> Initialize has_3dlut to 1 on each platform since LNL.
>
> Log value of has_3dlut in intel_display_device_info_print().
>
> Bspec: 71161
> Cc: Chaitanya Borah Kumar <[email protected]>
> Signed-off-by: Michał Grzelak <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c          | 3 +++
>  drivers/gpu/drm/i915/display/intel_display_device.c | 8 ++++++++
>  drivers/gpu/drm/i915/display/intel_display_device.h | 4 ++++
>  drivers/gpu/drm/i915/display/intel_display_regs.h   | 1 +
>  4 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 87ced9f6ff40..ffdefb51a264 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -4258,6 +4258,9 @@ intel_color_load_plane_luts(struct intel_dsb *dsb,
>  bool
>  intel_color_crtc_has_3dlut(struct intel_display *display, enum pipe pipe)
>  {
> +	if (!HAS_3DLUT(display))
> +		return false;
> +
>  	if (DISPLAY_VER(display) >= 12)
>  		return pipe == PIPE_A || pipe == PIPE_B;
>  	else
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
> index f17fc2c68472..45c791531f85 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> @@ -1335,6 +1335,7 @@ static const struct platform_desc dg2_desc = {
>  		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |				\
>  		BIT(TRANSCODER_C) | BIT(TRANSCODER_D),				\
>  	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A) | BIT(INTEL_FBC_B),	\
> +	.__runtime_defaults.has_3dlut = 1,					\
>  	.__runtime_defaults.has_dmc = 1,					\
>  	.__runtime_defaults.has_dsc = 1,					\
>  	.__runtime_defaults.has_hdcp = 1,					\
> @@ -1345,6 +1346,7 @@ static const struct platform_desc dg2_desc = {
>  
>  static const struct intel_display_device_info xe_lpdp_display = {
>  	XE_LPDP_FEATURES,
> +	.__runtime_defaults.has_3dlut = 0,

There's no need to initialize to 0.

>  };
>  
>  static const struct intel_display_device_info xe2_lpd_display = {
> @@ -1385,6 +1387,7 @@ static const struct intel_display_device_info xe2_hpd_display = {
>  	XE_LPDP_FEATURES,
>  	.__runtime_defaults.port_mask = BIT(PORT_A) |
>  		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4),
> +	.__runtime_defaults.has_3dlut = 0,

Ditto.

>  };
>  
>  static const u16 mtl_u_ids[] = {
> @@ -1935,6 +1938,10 @@ static void __intel_display_device_info_runtime_init(struct intel_display *displ
>  	if (DISPLAY_VER(display) >= 20) {
>  		u32 cap = intel_de_read(display, XE2LPD_DE_CAP);
>  
> +		if (REG_FIELD_GET(XE2LPD_DE_CAP_3DLUT_MASK, cap) ==
> +		    XE2LPD_DE_CAP_3DLUT_REMOVED)
> +			display_runtime->has_3dlut = 0;
> +
>  		if (REG_FIELD_GET(XE2LPD_DE_CAP_DSC_MASK, cap) ==
>  		    XE2LPD_DE_CAP_DSC_REMOVED)
>  			display_runtime->has_dsc = 0;
> @@ -1996,6 +2003,7 @@ void intel_display_device_info_print(const struct intel_display_device_info *inf
>  	DEV_INFO_DISPLAY_FOR_EACH_FLAG(PRINT_FLAG);
>  #undef PRINT_FLAG
>  
> +	drm_printf(p, "has_3dlut: %s\n", str_yes_no(runtime->has_3dlut));
>  	drm_printf(p, "has_hdcp: %s\n", str_yes_no(runtime->has_hdcp));
>  	drm_printf(p, "has_dmc: %s\n", str_yes_no(runtime->has_dmc));
>  	drm_printf(p, "has_dsc: %s\n", str_yes_no(runtime->has_dsc));
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 7121e7cd9512..34ee7993be33 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -147,6 +147,9 @@ struct intel_display_platforms {
>  
>  #define HAS_128B_Y_TILING(__display)	(!(__display)->platform.i915g && !(__display)->platform.i915gm)
>  #define HAS_2PPC(__display)		(DISPLAY_VER(__display) >= 10)
> +#define HAS_3DLUT_HW(__display)		(DISPLAY_VER(__display) >= 10)
> +#define HAS_3DLUT_CAP(__display)	(IS_DISPLAY_VER(__display, 10, 20) || DISPLAY_RUNTIME_INFO(__display)->has_3dlut)

I don't understand the difference between "HW" and "CAP" or the need for
both.

It's generally better to either use the runtime info flag or the display
version check, not both.

> +#define HAS_3DLUT(__display)		(HAS_3DLUT_HW(__display) && HAS_3DLUT_CAP(__display))
>  #define HAS_4TILE(__display)		((__display)->platform.dg2 || DISPLAY_VER(__display) >= 14)
>  #define HAS_ASYNC_FLIPS(__display)	(DISPLAY_VER(__display) >= 5)
>  #define HAS_AS_SDP(__display)		(DISPLAY_VER(__display) >= 13)
> @@ -304,6 +307,7 @@ struct intel_display_runtime_info {
>  
>  	u8 fbc_mask;
>  
> +	bool has_3dlut;
>  	bool has_hdcp;
>  	bool has_dmc;
>  	bool has_dsc;
> diff --git a/drivers/gpu/drm/i915/display/intel_display_regs.h b/drivers/gpu/drm/i915/display/intel_display_regs.h
> index 0677ca2dde00..7b2332e2fae2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_regs.h
> @@ -1773,6 +1773,7 @@
>  
>  #define XE2LPD_DE_CAP			_MMIO(0x41100)
>  #define   XE2LPD_DE_CAP_3DLUT_MASK	REG_GENMASK(31, 30)
> +#define   XE2LPD_DE_CAP_3DLUT_REMOVED	1
>  #define   XE2LPD_DE_CAP_DSC_MASK	REG_GENMASK(29, 28)
>  #define   XE2LPD_DE_CAP_DSC_REMOVED	1
>  #define   XE2LPD_DE_CAP_SCALER_MASK	REG_GENMASK(27, 26)

-- 
Jani Nikula, Intel