[Intel-gfx] [PATCH 2/2] drm/i915: Move common mmio base out of private macros
Lucas De Marchi
lucas.demarchi at intel.com
Wed Feb 1 17:26:29 UTC 2023
On Wed, Feb 01, 2023 at 02:09:54PM +0200, Ville Syrjälä wrote:
>On Wed, Feb 01, 2023 at 11:59:19AM +0200, Jani Nikula wrote:
>> On Tue, 31 Jan 2023, Lucas De Marchi <lucas.demarchi at intel.com> 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 at intel.com>
>> > ---
>> > 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.)
>
>Most of the DPLL code should be have vlv/chv vs. rest specific
>codepaths. A quick scan says readout is perhaps the only exception.
and then remove display.mmio_offset since vlv/chv are the only ones
using it?
Lucas De Marchi
More information about the Intel-gfx
mailing list