On Thu, 10 Nov 2022, Ville Syrjala <[email protected]> wrote:
> From: Ville Syrjälä <[email protected]>
>
> Use consistent bit definitions for the legacy gamma LUT. We just
> define these alongside the pre-ilk register definitions and point
> to those from the ilk+ defines.
>
> Also use the these appropriately in the LUT entry pack/unpack
> functions.
>
> Signed-off-by: Ville Syrjälä <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 24 +++++++++++-----------
>  drivers/gpu/drm/i915/i915_reg.h            | 11 +++++-----
>  2 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 93509cf7bbcc..ff4a5167df57 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -424,32 +424,32 @@ static u32 intel_color_lut_pack(u32 val, int 
> bit_precision)
>  
>  static u32 i9xx_lut_8(const struct drm_color_lut *color)
>  {
> -     return drm_color_lut_extract(color->red, 8) << 16 |
> -             drm_color_lut_extract(color->green, 8) << 8 |
> -             drm_color_lut_extract(color->blue, 8);
> +     return REG_FIELD_PREP(PALETTE_RED_MASK, 
> drm_color_lut_extract(color->red, 8)) |
> +             REG_FIELD_PREP(PALETTE_GREEN_MASK, 
> drm_color_lut_extract(color->green, 8)) |
> +             REG_FIELD_PREP(PALETTE_BLUE_MASK, 
> drm_color_lut_extract(color->blue, 8));
>  }
>  
>  static void i9xx_lut_8_pack(struct drm_color_lut *entry, u32 val)
>  {
> -     entry->red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, 
> val), 8);
> -     entry->green = 
> intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);
> -     entry->blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, 
> val), 8);
> +     entry->red = intel_color_lut_pack(REG_FIELD_GET(PALETTE_RED_MASK, val), 
> 8);
> +     entry->green = intel_color_lut_pack(REG_FIELD_GET(PALETTE_GREEN_MASK, 
> val), 8);
> +     entry->blue = intel_color_lut_pack(REG_FIELD_GET(PALETTE_BLUE_MASK, 
> val), 8);
>  }
>  
>  /* i965+ "10.6" bit interpolated format "even DW" (low 8 bits) */
>  static u32 i965_lut_10p6_ldw(const struct drm_color_lut *color)
>  {
> -     return (color->red & 0xff) << 16 |
> -             (color->green & 0xff) << 8 |
> -             (color->blue & 0xff);
> +     return REG_FIELD_PREP(PALETTE_RED_MASK, color->red & 0xff) |
> +             REG_FIELD_PREP(PALETTE_GREEN_MASK, color->green & 0xff) |
> +             REG_FIELD_PREP(PALETTE_BLUE_MASK, color->blue & 0xff);

The & 0xff masking is redundant with REG_FIELD_PREP(), but I understand
if you want to leave them in for consistency with the next function.

Reviewed-by: Jani Nikula <[email protected]>

>  }
>  
>  /* i965+ "10.6" interpolated format "odd DW" (high 8 bits) */
>  static u32 i965_lut_10p6_udw(const struct drm_color_lut *color)
>  {
> -     return (color->red >> 8) << 16 |
> -             (color->green >> 8) << 8 |
> -             (color->blue >> 8);
> +     return REG_FIELD_PREP(PALETTE_RED_MASK, color->red >> 8) |
> +             REG_FIELD_PREP(PALETTE_GREEN_MASK, color->green >> 8) |
> +             REG_FIELD_PREP(PALETTE_BLUE_MASK, color->blue >> 8);
>  }
>  
>  static void i965_lut_10p6_pack(struct drm_color_lut *entry, u32 ldw, u32 udw)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a37ed0c61f20..91ee00c347e4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1782,9 +1782,10 @@
>  #define _PALETTE_A           0xa000
>  #define _PALETTE_B           0xa800
>  #define _CHV_PALETTE_C               0xc000
> -#define PALETTE_RED_MASK        REG_GENMASK(23, 16)
> -#define PALETTE_GREEN_MASK      REG_GENMASK(15, 8)
> -#define PALETTE_BLUE_MASK       REG_GENMASK(7, 0)
> +/* 8bit mode / i965+ 10.6 interpolated mode ldw/udw */
> +#define   PALETTE_RED_MASK           REG_GENMASK(23, 16)
> +#define   PALETTE_GREEN_MASK         REG_GENMASK(15, 8)
> +#define   PALETTE_BLUE_MASK          REG_GENMASK(7, 0)
>  #define PALETTE(pipe, i)     _MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
>                                     _PICK((pipe), _PALETTE_A,         \
>                                           _PALETTE_B, _CHV_PALETTE_C) + \
> @@ -5380,9 +5381,7 @@
>  /* legacy palette */
>  #define _LGC_PALETTE_A           0x4a000
>  #define _LGC_PALETTE_B           0x4a800
> -#define LGC_PALETTE_RED_MASK     REG_GENMASK(23, 16)
> -#define LGC_PALETTE_GREEN_MASK   REG_GENMASK(15, 8)
> -#define LGC_PALETTE_BLUE_MASK    REG_GENMASK(7, 0)
> +/* see PALETTE_* for the bits */
>  #define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, 
> _LGC_PALETTE_B) + (i) * 4)
>  
>  /* ilk/snb precision palette */

-- 
Jani Nikula, Intel Open Source Graphics Center

Reply via email to