Re: [Intel-gfx] [PATCH v2 03/18] drm/i915: Clean up 12.4bit precision palette defines

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

On Thu, 10 Nov 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Use consistent bit definitions for the 12.4bit precision palette bits.
> We just define these alongside the ilk/snb register definitions and
> point to those from the icl+ superfine segment defines (and we also
> already pointed to them from the ivb+ precision palette defines).
>
> Also use the these appropriately in the LUT entry pack/unpack
> functions.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 22 ++++++++++++----------
>  drivers/gpu/drm/i915/i915_reg.h            | 15 +++++++++------
>  2 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 6486a0890583..758869971e45 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -485,25 +485,27 @@ static void ilk_lut_10_pack(struct drm_color_lut *entry, u32 val)
>  /* ilk+ "12.4" interpolated format (high 10 bits) */
>  static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
>  {
> -	return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
> -		(color->blue >> 6);
> +	return REG_FIELD_PREP(PREC_PALETTE_12P4_RED_UDW_MASK, color->red >> 6) |
> +		REG_FIELD_PREP(PREC_PALETTE_12P4_GREEN_UDW_MASK, color->green >> 6) |
> +		REG_FIELD_PREP(PREC_PALETTE_12P4_BLUE_UDW_MASK, color->blue >> 6);
>  }
>  
>  /* ilk+ "12.4" interpolated format (low 6 bits) */
>  static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
>  {
> -	return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
> -		(color->blue & 0x3f) << 4;
> +	return REG_FIELD_PREP(PREC_PALETTE_12P4_RED_LDW_MASK, color->red & 0x3f) |
> +		REG_FIELD_PREP(PREC_PALETTE_12P4_GREEN_LDW_MASK, color->green & 0x3f) |
> +		REG_FIELD_PREP(PREC_PALETTE_12P4_BLUE_LDW_MASK, color->blue & 0x3f);

Same thing with masking as in an earlier patch.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>  }
>  
>  static void ilk_lut_12p4_pack(struct drm_color_lut *entry, u32 ldw, u32 udw)
>  {
> -	entry->red = REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_UDW_MASK, udw) << 6 |
> -		REG_FIELD_GET(PAL_PREC_MULTI_SEG_RED_LDW_MASK, ldw);
> -	entry->green = REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_UDW_MASK, udw) << 6 |
> -		REG_FIELD_GET(PAL_PREC_MULTI_SEG_GREEN_LDW_MASK, ldw);
> -	entry->blue = REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_UDW_MASK, udw) << 6 |
> -		REG_FIELD_GET(PAL_PREC_MULTI_SEG_BLUE_LDW_MASK, ldw);
> +	entry->red = REG_FIELD_GET(PREC_PALETTE_12P4_RED_UDW_MASK, udw) << 6 |
> +		REG_FIELD_GET(PREC_PALETTE_12P4_RED_LDW_MASK, ldw);
> +	entry->green = REG_FIELD_GET(PREC_PALETTE_12P4_GREEN_UDW_MASK, udw) << 6 |
> +		REG_FIELD_GET(PREC_PALETTE_12P4_GREEN_LDW_MASK, ldw);
> +	entry->blue = REG_FIELD_GET(PREC_PALETTE_12P4_BLUE_UDW_MASK, udw) << 6 |
> +		REG_FIELD_GET(PREC_PALETTE_12P4_BLUE_LDW_MASK, ldw);
>  }
>  
>  static void icl_color_commit_noarm(const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3aa3db2b56f5..ecb34f133980 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5391,6 +5391,14 @@
>  #define   PREC_PALETTE_10_RED_MASK		REG_GENMASK(29, 20)
>  #define   PREC_PALETTE_10_GREEN_MASK		REG_GENMASK(19, 10)
>  #define   PREC_PALETTE_10_BLUE_MASK		REG_GENMASK(9, 0)
> +/* 12.4 interpolated mode ldw */
> +#define   PREC_PALETTE_12P4_RED_LDW_MASK	REG_GENMASK(29, 24)
> +#define   PREC_PALETTE_12P4_GREEN_LDW_MASK	REG_GENMASK(19, 14)
> +#define   PREC_PALETTE_12P4_BLUE_LDW_MASK	REG_GENMASK(9, 4)
> +/* 12.4 interpolated mode udw */
> +#define   PREC_PALETTE_12P4_RED_UDW_MASK	REG_GENMASK(29, 20)
> +#define   PREC_PALETTE_12P4_GREEN_UDW_MASK	REG_GENMASK(19, 10)
> +#define   PREC_PALETTE_12P4_BLUE_UDW_MASK	REG_GENMASK(9, 0)
>  #define PREC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _PREC_PALETTE_A, _PREC_PALETTE_B) + (i) * 4)
>  
>  #define  _PREC_PIPEAGCMAX              0x4d000
> @@ -7656,12 +7664,7 @@ enum skl_power_gate {
>  
>  #define _PAL_PREC_MULTI_SEG_DATA_A	0x4A40C
>  #define _PAL_PREC_MULTI_SEG_DATA_B	0x4AC0C
> -#define  PAL_PREC_MULTI_SEG_RED_LDW_MASK   REG_GENMASK(29, 24)
> -#define  PAL_PREC_MULTI_SEG_RED_UDW_MASK   REG_GENMASK(29, 20)
> -#define  PAL_PREC_MULTI_SEG_GREEN_LDW_MASK REG_GENMASK(19, 14)
> -#define  PAL_PREC_MULTI_SEG_GREEN_UDW_MASK REG_GENMASK(19, 10)
> -#define  PAL_PREC_MULTI_SEG_BLUE_LDW_MASK  REG_GENMASK(9, 4)
> -#define  PAL_PREC_MULTI_SEG_BLUE_UDW_MASK  REG_GENMASK(9, 0)
> +/* see PREC_PALETTE_12P4_* for the bits */
>  
>  #define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
>  					_PAL_PREC_MULTI_SEG_INDEX_A, \

-- 
Jani Nikula, Intel Open Source Graphics Center