On Mon, Sep 11, 2023 at 09:48:21PM -0700, Lucas De Marchi wrote: > Fix some whitespace issues for register definitions and keep the defines > for DP_AUX_CH_CTL and DP_AUX_CH_DATA in the right place: together with > the bit definition. > > While at it add a TODO entry that those defines shouldn't be using an > implicit dev_priv. > > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> In most of our other register headers we try to indent the values to column 49. It looks like a bunch of the definitions here are only indented to column 41. Not sure if you want to change that for consistency or not. Either way, Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > --- > .../gpu/drm/i915/display/intel_dp_aux_regs.h | 72 +++++++++---------- > 1 file changed, 35 insertions(+), 37 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h b/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h > index 5185345277c7..4503d94115d7 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h > @@ -13,48 +13,28 @@ > * packet size supported is 20 bytes in each direction, hence the 5 fixed data > * registers > */ > -#define _DPA_AUX_CH_CTL (DISPLAY_MMIO_BASE(dev_priv) + 0x64010) > -#define _DPA_AUX_CH_DATA1 (DISPLAY_MMIO_BASE(dev_priv) + 0x64014) > - > -#define _DPB_AUX_CH_CTL (DISPLAY_MMIO_BASE(dev_priv) + 0x64110) > -#define _DPB_AUX_CH_DATA1 (DISPLAY_MMIO_BASE(dev_priv) + 0x64114) > - > -#define DP_AUX_CH_CTL(aux_ch) _MMIO_PORT(aux_ch, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL) > -#define DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PORT(aux_ch, _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */ > - > -#define _XELPDP_USBC1_AUX_CH_CTL 0x16F210 > -#define _XELPDP_USBC2_AUX_CH_CTL 0x16F410 > -#define _XELPDP_USBC3_AUX_CH_CTL 0x16F610 > -#define _XELPDP_USBC4_AUX_CH_CTL 0x16F810 > - > -#define XELPDP_DP_AUX_CH_CTL(aux_ch) _MMIO(_PICK(aux_ch, \ > - _DPA_AUX_CH_CTL, \ > - _DPB_AUX_CH_CTL, \ > - 0, /* port/aux_ch C is non-existent */ \ > - _XELPDP_USBC1_AUX_CH_CTL, \ > - _XELPDP_USBC2_AUX_CH_CTL, \ > - _XELPDP_USBC3_AUX_CH_CTL, \ > - _XELPDP_USBC4_AUX_CH_CTL)) > - > -#define _XELPDP_USBC1_AUX_CH_DATA1 0x16F214 > -#define _XELPDP_USBC2_AUX_CH_DATA1 0x16F414 > -#define _XELPDP_USBC3_AUX_CH_DATA1 0x16F614 > -#define _XELPDP_USBC4_AUX_CH_DATA1 0x16F814 > - > -#define XELPDP_DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PICK(aux_ch, \ > - _DPA_AUX_CH_DATA1, \ > - _DPB_AUX_CH_DATA1, \ > - 0, /* port/aux_ch C is non-existent */ \ > - _XELPDP_USBC1_AUX_CH_DATA1, \ > - _XELPDP_USBC2_AUX_CH_DATA1, \ > - _XELPDP_USBC3_AUX_CH_DATA1, \ > - _XELPDP_USBC4_AUX_CH_DATA1) + (i) * 4) > > +/* TODO: Remove implicit dev_priv */ > +#define _DPA_AUX_CH_CTL (DISPLAY_MMIO_BASE(dev_priv) + 0x64010) > +#define _DPB_AUX_CH_CTL (DISPLAY_MMIO_BASE(dev_priv) + 0x64110) > +#define _XELPDP_USBC1_AUX_CH_CTL 0x16f210 > +#define _XELPDP_USBC2_AUX_CH_CTL 0x16f410 > +#define _XELPDP_USBC3_AUX_CH_CTL 0x16f610 > +#define _XELPDP_USBC4_AUX_CH_CTL 0x16f810 > +#define DP_AUX_CH_CTL(aux_ch) _MMIO_PORT(aux_ch, _DPA_AUX_CH_CTL, \ > + _DPB_AUX_CH_CTL) > +#define XELPDP_DP_AUX_CH_CTL(aux_ch) _MMIO(_PICK(aux_ch, \ > + _DPA_AUX_CH_CTL, \ > + _DPB_AUX_CH_CTL, \ > + 0, /* port/aux_ch C is non-existent */ \ > + _XELPDP_USBC1_AUX_CH_CTL, \ > + _XELPDP_USBC2_AUX_CH_CTL, \ > + _XELPDP_USBC3_AUX_CH_CTL, \ > + _XELPDP_USBC4_AUX_CH_CTL)) > #define DP_AUX_CH_CTL_SEND_BUSY REG_BIT(31) > #define DP_AUX_CH_CTL_DONE REG_BIT(30) > #define DP_AUX_CH_CTL_INTERRUPT REG_BIT(29) > #define DP_AUX_CH_CTL_TIME_OUT_ERROR REG_BIT(28) > - > #define DP_AUX_CH_CTL_TIME_OUT_MASK REG_GENMASK(27, 26) > #define DP_AUX_CH_CTL_TIME_OUT_400us REG_FIELD_PREP(DP_AUX_CH_CTL_TIME_OUT_MASK, 0) > #define DP_AUX_CH_CTL_TIME_OUT_600us REG_FIELD_PREP(DP_AUX_CH_CTL_TIME_OUT_MASK, 1) > @@ -83,4 +63,22 @@ > #define DP_AUX_CH_CTL_SYNC_PULSE_SKL_MASK REG_GENMASK(4, 0) /* skl+ */ > #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) REG_FIELD_PREP(DP_AUX_CH_CTL_SYNC_PULSE_SKL_MASK, (c) - 1) > > +/* TODO: Remove implicit dev_priv */ > +#define _DPA_AUX_CH_DATA1 (DISPLAY_MMIO_BASE(dev_priv) + 0x64014) > +#define _DPB_AUX_CH_DATA1 (DISPLAY_MMIO_BASE(dev_priv) + 0x64114) > +#define _XELPDP_USBC1_AUX_CH_DATA1 0x16f214 > +#define _XELPDP_USBC2_AUX_CH_DATA1 0x16f414 > +#define _XELPDP_USBC3_AUX_CH_DATA1 0x16f614 > +#define _XELPDP_USBC4_AUX_CH_DATA1 0x16f814 > +#define DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PORT(aux_ch, _DPA_AUX_CH_DATA1, \ > + _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */ > +#define XELPDP_DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PICK(aux_ch, \ > + _DPA_AUX_CH_DATA1, \ > + _DPB_AUX_CH_DATA1, \ > + 0, /* port/aux_ch C is non-existent */ \ > + _XELPDP_USBC1_AUX_CH_DATA1, \ > + _XELPDP_USBC2_AUX_CH_DATA1, \ > + _XELPDP_USBC3_AUX_CH_DATA1, \ > + _XELPDP_USBC4_AUX_CH_DATA1) + (i) * 4) > + > #endif /* __INTEL_DP_AUX_REGS_H__ */ > -- > 2.40.1 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation