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 <[email protected]>

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 <[email protected]>

> ---
>  .../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

Reply via email to