[Intel-xe] [Intel-gfx] [PATCH v3 13/29] drm/i915/display: Fix style and conventions for DP AUX regs
Lucas De Marchi
lucas.demarchi at intel.com
Tue Sep 12 16:00:19 UTC 2023
On Tue, Sep 12, 2023 at 08:32:54AM -0700, Matt Roper wrote:
>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 at intel.com>
>
>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
I think I prefer to keep as is. I don't think we have any convention to
align on a certain column. It's rather about aligning to surrounding
definitions and make a best effort between making the line too long or
with too many line breaks.
>consistency or not. Either way,
>
>Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
thanks
Lucas De Marchi
>
>> ---
>> .../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
More information about the Intel-xe
mailing list