On Mon, Nov 15, 2021 at 02:12:47PM -0500, Rodrigo Vivi wrote: > On Fri, Nov 12, 2021 at 09:38:06PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrj...@linux.intel.com> > > > > Use REG_BIT() & co. for PIPEMISC* bits, and while at it > > fill in the missing dithering bits since we already had some > > of them defined. > > > > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 18 +++++----- > > drivers/gpu/drm/i915/i915_reg.h | 35 +++++++++++--------- > > 2 files changed, 28 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 6073f94632ab..e293241450b1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -3724,18 +3724,18 @@ static void bdw_set_pipemisc(const struct > > intel_crtc_state *crtc_state) > > > > switch (crtc_state->pipe_bpp) { > > case 18: > > - val |= PIPEMISC_6_BPC; > > + val |= PIPEMISC_BPC_6; > > break; > > case 24: > > - val |= PIPEMISC_8_BPC; > > + val |= PIPEMISC_BPC_8; > > break; > > case 30: > > - val |= PIPEMISC_10_BPC; > > + val |= PIPEMISC_BPC_10; > > break; > > case 36: > > /* Port output 12BPC defined for ADLP+ */ > > if (DISPLAY_VER(dev_priv) > 12) > > - val |= PIPEMISC_12_BPC_ADLP; > > + val |= PIPEMISC_BPC_12_ADLP; > > while on it, I wonder if we could remove this "ADLP" suffix. > > First because prefix seems to be the most used case for the platform.
I don't like prefix for such things since it screws up the namespace for the bits of the same register. So suffix is better IMO. > But also because there's a clear if here and a clear comment in the > definition. Yeah, I've been removing some similar things in other places for that exact reason. The only case where we might really need platform designators is when the bit(s) move etc. Though I was even pondering removing the 'if' since this only controls dithering on older platforms and we don't enable dithering for anything but 6bpc output. Although the whole pipe_bpp concept is kinda wrong and we should perhaps split it up into pipe_bpp vs. port_bpp, or something along those lines. > (Btw, I'd prefer the ver >= 13 than > 12 to be really clear :/) Yeah, >= is what we should use. > > > break; > > default: > > MISSING_CASE(crtc_state->pipe_bpp); > > @@ -3771,7 +3771,7 @@ static void bdw_set_pipemisc(const struct > > intel_crtc_state *crtc_state) > > } > > > > intel_de_rmw(dev_priv, PIPE_MISC2(crtc->pipe), > > - PIPE_MISC2_UNDERRUN_BUBBLE_COUNTER_MASK, > > + PIPE_MISC2_BUBBLE_COUNTER_MASK, > > scaler_in_use ? > > PIPE_MISC2_BUBBLE_COUNTER_SCALER_EN : > > PIPE_MISC2_BUBBLE_COUNTER_SCALER_DIS); > > } > > @@ -3787,11 +3787,11 @@ int bdw_get_pipemisc_bpp(struct intel_crtc *crtc) > > tmp = intel_de_read(dev_priv, PIPEMISC(crtc->pipe)); > > > > switch (tmp & PIPEMISC_BPC_MASK) { > > - case PIPEMISC_6_BPC: > > + case PIPEMISC_BPC_6: > > return 18; > > - case PIPEMISC_8_BPC: > > + case PIPEMISC_BPC_8: > > return 24; > > - case PIPEMISC_10_BPC: > > + case PIPEMISC_BPC_10: > > return 30; > > /* > > * PORT OUTPUT 12 BPC defined for ADLP+. > > @@ -3803,7 +3803,7 @@ int bdw_get_pipemisc_bpp(struct intel_crtc *crtc) > > * on older platforms, need to find a workaround for 12 BPC > > * MIPI DSI HW readout. > > */ > > - case PIPEMISC_12_BPC_ADLP: > > + case PIPEMISC_BPC_12_ADLP: > > if (DISPLAY_VER(dev_priv) > 12) > > return 36; > > fallthrough; > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index f5d54ed2efc1..e300a202ce2d 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -6308,32 +6308,35 @@ enum { > > > > #define _PIPE_MISC_A 0x70030 > > #define _PIPE_MISC_B 0x71030 > > -#define PIPEMISC_YUV420_ENABLE (1 << 27) /* glk+ */ > > -#define PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26) /* glk+ */ > > -#define PIPEMISC_HDR_MODE_PRECISION (1 << 23) /* icl+ */ > > -#define PIPEMISC_OUTPUT_COLORSPACE_YUV (1 << 11) > > -#define PIPEMISC_PIXEL_ROUNDING_TRUNC REG_BIT(8) /* tgl+ */ > > +#define PIPEMISC_YUV420_ENABLE REG_BIT(27) /* glk+ */ > > +#define PIPEMISC_YUV420_MODE_FULL_BLEND REG_BIT(26) /* glk+ */ > > +#define PIPEMISC_HDR_MODE_PRECISION REG_BIT(23) /* icl+ */ > > +#define PIPEMISC_OUTPUT_COLORSPACE_YUV REG_BIT(11) > > +#define PIPEMISC_PIXEL_ROUNDING_TRUNC REG_BIT(8) /* tgl+ */ > > /* > > * For Display < 13, Bits 5-7 of PIPE MISC represent DITHER BPC with > > * valid values of: 6, 8, 10 BPC. > > * ADLP+, the bits 5-7 represent PORT OUTPUT BPC with valid values of: > > * 6, 8, 10, 12 BPC. > > */ > > -#define PIPEMISC_BPC_MASK (7 << 5) > > -#define PIPEMISC_8_BPC (0 << 5) > > -#define PIPEMISC_10_BPC (1 << 5) > > -#define PIPEMISC_6_BPC (2 << 5) > > -#define PIPEMISC_12_BPC_ADLP (4 << 5) /* adlp+ */ > > -#define PIPEMISC_DITHER_ENABLE (1 << 4) > > -#define PIPEMISC_DITHER_TYPE_MASK (3 << 2) > > -#define PIPEMISC_DITHER_TYPE_SP (0 << 2) > > +#define PIPEMISC_BPC_MASK REG_GENMASK(7, 5) > > +#define PIPEMISC_BPC_8 > > REG_FIELD_PREP(PIPEMISC_BPC_MASK, 0) > > +#define PIPEMISC_BPC_10 > > REG_FIELD_PREP(PIPEMISC_BPC_MASK, 1) > > +#define PIPEMISC_BPC_6 > > REG_FIELD_PREP(PIPEMISC_BPC_MASK, 2) > > +#define PIPEMISC_BPC_12_ADLP > > REG_FIELD_PREP(PIPEMISC_BPC_MASK, 4) /* adlp+ */ > > +#define PIPEMISC_DITHER_ENABLE REG_BIT(4) > > +#define PIPEMISC_DITHER_TYPE_MASK REG_GENMASK(3, 2) > > +#define PIPEMISC_DITHER_TYPE_SP > > REG_FIELD_PREP(PIPEMISC_DITHER_TYPE_MASK, 0) > > +#define PIPEMISC_DITHER_TYPE_ST1 > > REG_FIELD_PREP(PIPEMISC_DITHER_TYPE_MASK, 1) > > +#define PIPEMISC_DITHER_TYPE_ST2 > > REG_FIELD_PREP(PIPEMISC_DITHER_TYPE_MASK, 2) > > +#define PIPEMISC_DITHER_TYPE_TEMP > > REG_FIELD_PREP(PIPEMISC_DITHER_TYPE_MASK, 3) > > #define PIPEMISC(pipe) _MMIO_PIPE2(pipe, _PIPE_MISC_A) > > > > #define _PIPE_MISC2_A 0x7002C > > #define _PIPE_MISC2_B 0x7102C > > -#define PIPE_MISC2_BUBBLE_COUNTER_SCALER_EN (0x50 << 24) > > -#define PIPE_MISC2_BUBBLE_COUNTER_SCALER_DIS (0x14 << 24) > > -#define PIPE_MISC2_UNDERRUN_BUBBLE_COUNTER_MASK (0xff << 24) > > +#define PIPE_MISC2_BUBBLE_COUNTER_MASK REG_GENMASK(31, 24) > > +#define PIPE_MISC2_BUBBLE_COUNTER_SCALER_EN > > REG_FIELD_PREP(PIPE_MISC2_BUBBLE_COUNTER_MASK, 80) > > +#define PIPE_MISC2_BUBBLE_COUNTER_SCALER_DIS > > REG_FIELD_PREP(PIPE_MISC2_BUBBLE_COUNTER_MASK, 20) > > #define PIPE_MISC2(pipe) > > _MMIO_PIPE2(pipe, _PIPE_MISC2_A) > > > > /* Skylake+ pipe bottom (background) color */ > > -- > > 2.32.0 > > -- Ville Syrjälä Intel