Re: [PATCH 2/2] drm/i915: Move common mmio base out of private macros

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 31 Jan 2023, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote:
> Instead of using the common DISPLAY_MMIO_BASE(dev_priv) in all single
> macros, only use them in the macros that are to be used outside the
> header. This reduces the use of the implicit dev_priv, making it easier
> to remove it later.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 73 ++++++++++++++++++---------------
>  1 file changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 943db8ec63f8..1cde3bcb9c88 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1182,9 +1182,9 @@
>  #define PM_VEBOX_CS_ERROR_INTERRUPT		(1 << 12) /* hsw+ */
>  #define PM_VEBOX_USER_INTERRUPT			(1 << 10) /* hsw+ */
>  
> -#define GT_PARITY_ERROR(dev_priv) \
> +#define GT_PARITY_ERROR(__i915) \
>  	(GT_RENDER_L3_PARITY_ERROR_INTERRUPT | \
> -	 (IS_HASWELL(dev_priv) ? GT_RENDER_L3_PARITY_ERROR_INTERRUPT_S1 : 0))
> +	 (IS_HASWELL(__i915) ? GT_RENDER_L3_PARITY_ERROR_INTERRUPT_S1 : 0))

Unrelated change.

>  
>  /* These are all the "old" interrupts */
>  #define ILK_BSD_USER_INTERRUPT				(1 << 5)
> @@ -1403,10 +1403,11 @@
>  /*
>   * Clock control & power management
>   */
> -#define _DPLL_A (DISPLAY_MMIO_BASE(dev_priv) + 0x6014)
> -#define _DPLL_B (DISPLAY_MMIO_BASE(dev_priv) + 0x6018)
> -#define _CHV_DPLL_C (DISPLAY_MMIO_BASE(dev_priv) + 0x6030)
> -#define DPLL(pipe) _MMIO_PIPE3((pipe), _DPLL_A, _DPLL_B, _CHV_DPLL_C)
> +#define _DPLL_A		0x6014
> +#define _DPLL_B		0x6018
> +#define _CHV_DPLL_C	0x6030
> +#define DPLL(pipe) _MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
> +			 _PIPE3((pipe), _DPLL_A, _DPLL_B, _CHV_DPLL_C))
>  
>  #define VGA0	_MMIO(0x6000)
>  #define VGA1	_MMIO(0x6004)
> @@ -1502,10 +1503,11 @@
>  #define   SDVO_MULTIPLIER_SHIFT_HIRES		4
>  #define   SDVO_MULTIPLIER_SHIFT_VGA		0
>  
> -#define _DPLL_A_MD (DISPLAY_MMIO_BASE(dev_priv) + 0x601c)
> -#define _DPLL_B_MD (DISPLAY_MMIO_BASE(dev_priv) + 0x6020)
> -#define _CHV_DPLL_C_MD (DISPLAY_MMIO_BASE(dev_priv) + 0x603c)
> -#define DPLL_MD(pipe) _MMIO_PIPE3((pipe), _DPLL_A_MD, _DPLL_B_MD, _CHV_DPLL_C_MD)
> +#define _DPLL_A_MD				0x601c
> +#define _DPLL_B_MD				0x6020
> +#define _CHV_DPLL_C_MD				0x603c
> +#define DPLL_MD(pipe) _MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
> +			    _PIPE3((pipe), _DPLL_A_MD, _DPLL_B_MD, _CHV_DPLL_C_MD))
>  
>  /*
>   * UDI pixel divider, controlling how many pixels are stuffed into a packet.
> @@ -3323,42 +3325,45 @@
>   * 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 _DPA_AUX_CH_CTL		0x64010
> +#define _DPA_AUX_CH_DATA1	0x64014
> +#define _DPB_AUX_CH_CTL		0x64110
> +#define _DPB_AUX_CH_DATA1	0x64114
> +#define DP_AUX_CH_CTL(aux_ch)	_MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
> +				      _PORT(aux_ch, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL))
> +#define DP_AUX_CH_DATA(aux_ch, i)		\
> +	_MMIO(DISPLAY_MMIO_BASE(dev_priv) +	\
> +	      _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_DP_AUX_CH_CTL(aux_ch)		_MMIO(DISPLAY_MMIO_BASE(dev_priv) + \

Note that only VLV and CHV have DISPLAY_MMIO_BASE() != 0.

This is an XELPDP specific macro. Just drop the DISPLAY_MMIO_BASE() part
altogether, and you've removed an implicit dev_priv. Yay.

This also makes me think we should probably add VLV/CHV specific
DP_AUX_CH_CTL and DP_AUX_CH_DATA macros that just add VLV_DISPLAY_BASE
directly, and use that to ditch the implicit dev_priv there too. This
approach doesn't work for everything, but the aux channel stuff is both
fairly limited use and already has if ladders to pick the
registers. Handling VLV/CHV separately is not a big deal.

(DPLL on the other hand seems much harder to deal that way.)


BR,
Jani.


> +						      _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)
> +#define XELPDP_DP_AUX_CH_DATA(aux_ch, i)	_MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
> +						      _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)
>  
>  #define   DP_AUX_CH_CTL_SEND_BUSY	    (1 << 31)
>  #define   DP_AUX_CH_CTL_DONE		    (1 << 30)

-- 
Jani Nikula, Intel Open Source Graphics Center



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux