On Thu, 10 Nov 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > 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ä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > 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 <jani.nikula@xxxxxxxxx> > } > > /* 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