[v3,02/15] drm/i915/rkl: Program BW_BUDDY0 registers instead of BW_BUDDY1/2
diff mbox series

Message ID 20200603211529.3005059-3-matthew.d.roper@intel.com
State New
Headers show
Series
  • Remaining RKL patches
Related show

Commit Message

Matt Roper June 3, 2020, 9:15 p.m. UTC
RKL uses the same BW_BUDDY programming table as TGL, but programs the
values into a single set BUDDY0 set of registers rather than the
BUDDY1/BUDDY2 sets used by TGL.

Bspec: 49218
Cc: Aditya Swarup <aditya.swarup@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 .../drm/i915/display/intel_display_power.c    | 44 +++++++++++--------
 drivers/gpu/drm/i915/i915_reg.h               | 14 ++++--
 2 files changed, 35 insertions(+), 23 deletions(-)

Comments

Aditya Swarup June 3, 2020, 10:34 p.m. UTC | #1
On Wed, Jun 03, 2020 at 02:15:16PM -0700, Matt Roper wrote:
> RKL uses the same BW_BUDDY programming table as TGL, but programs the
> values into a single set BUDDY0 set of registers rather than the
> BUDDY1/BUDDY2 sets used by TGL.
> 
> Bspec: 49218
> Cc: Aditya Swarup <aditya.swarup@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    | 44 +++++++++++--------
>  drivers/gpu/drm/i915/i915_reg.h               | 14 ++++--
>  2 files changed, 35 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 72312b67b57a..2c1ce50b572b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -5254,7 +5254,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
>  	enum intel_dram_type type = dev_priv->dram_info.type;
>  	u8 num_channels = dev_priv->dram_info.num_channels;
>  	const struct buddy_page_mask *table;
> -	int i;
> +	int config, min_buddy, max_buddy, i;
>  
>  	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
>  		/* Wa_1409767108: tgl */
> @@ -5262,29 +5262,35 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
>  	else
>  		table = tgl_buddy_page_masks;
>  
> -	for (i = 0; table[i].page_mask != 0; i++)
> -		if (table[i].num_channels == num_channels &&
> -		    table[i].type == type)
> +	if (IS_ROCKETLAKE(dev_priv)) {
> +		min_buddy = max_buddy = 0;
> +	} else {
> +		min_buddy = 1;
> +		max_buddy = 2;
> +	}
> +
> +	for (config = 0; table[config].page_mask != 0; config++)
> +		if (table[config].num_channels == num_channels &&
> +		    table[config].type == type)
>  			break;
>  
> -	if (table[i].page_mask == 0) {
> +	if (table[config].page_mask == 0) {
>  		drm_dbg(&dev_priv->drm,
>  			"Unknown memory configuration; disabling address buddy logic.\n");
> -		intel_de_write(dev_priv, BW_BUDDY1_CTL, BW_BUDDY_DISABLE);
> -		intel_de_write(dev_priv, BW_BUDDY2_CTL, BW_BUDDY_DISABLE);
> +		for (i = min_buddy; i <= max_buddy; i++)
> +			intel_de_write(dev_priv, BW_BUDDY_CTL(i),
> +				       BW_BUDDY_DISABLE);
>  	} else {
> -		intel_de_write(dev_priv, BW_BUDDY1_PAGE_MASK,
> -			       table[i].page_mask);
> -		intel_de_write(dev_priv, BW_BUDDY2_PAGE_MASK,
> -			       table[i].page_mask);
> -
> -		/* Wa_22010178259:tgl */
> -		intel_de_rmw(dev_priv, BW_BUDDY1_CTL,
> -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> -		intel_de_rmw(dev_priv, BW_BUDDY2_CTL,
> -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> +		for (i = min_buddy; i <= max_buddy; i++) {
> +			intel_de_write(dev_priv, BW_BUDDY_PAGE_MASK(i),
> +				       table[config].page_mask);
> +
> +			/* Wa_22010178259:tgl,rkl */
> +			intel_de_rmw(dev_priv, BW_BUDDY_CTL(i),
> +				     BW_BUDDY_TLB_REQ_TIMER_MASK,
> +				     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK,
> +						    0x8));
We should be using REG_FIELD_PREP() in i915_reg.h to declare
TLB_REQ_TIMER value and then use the value here.
> +		}
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 578cfe11cbb9..3e79cefc510a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7837,13 +7837,19 @@ enum {
>  #define  WAIT_FOR_PCH_RESET_ACK		(1 << 1)
>  #define  WAIT_FOR_PCH_FLR_ACK		(1 << 0)
>  
> -#define BW_BUDDY1_CTL			_MMIO(0x45140)
> -#define BW_BUDDY2_CTL			_MMIO(0x45150)
> +#define _BW_BUDDY0_CTL			0x45130
> +#define _BW_BUDDY1_CTL			0x45140
> +#define BW_BUDDY_CTL(x)			_MMIO(_PICK_EVEN(x, \
> +							 _BW_BUDDY0_CTL, \
> +							 _BW_BUDDY1_CTL))
>  #define   BW_BUDDY_DISABLE		REG_BIT(31)
>  #define   BW_BUDDY_TLB_REQ_TIMER_MASK	REG_GENMASK(21, 16)
>  
> -#define BW_BUDDY1_PAGE_MASK		_MMIO(0x45144)
> -#define BW_BUDDY2_PAGE_MASK		_MMIO(0x45154)
> +#define _BW_BUDDY0_PAGE_MASK		0x45134
> +#define _BW_BUDDY1_PAGE_MASK		0x45144
> +#define BW_BUDDY_PAGE_MASK(x)		_MMIO(_PICK_EVEN(x, \
> +							 _BW_BUDDY0_PAGE_MASK, \
> +							 _BW_BUDDY1_PAGE_MASK))
>  
>  #define HSW_NDE_RSTWRN_OPT	_MMIO(0x46408)
>  #define  RESET_PCH_HANDSHAKE_ENABLE	(1 << 4)
> -- 
> 2.24.1
>
Matt Roper June 3, 2020, 11:12 p.m. UTC | #2
On Wed, Jun 03, 2020 at 03:34:32PM -0700, Aditya Swarup wrote:
> On Wed, Jun 03, 2020 at 02:15:16PM -0700, Matt Roper wrote:
> > RKL uses the same BW_BUDDY programming table as TGL, but programs the
> > values into a single set BUDDY0 set of registers rather than the
> > BUDDY1/BUDDY2 sets used by TGL.
> > 
> > Bspec: 49218
> > Cc: Aditya Swarup <aditya.swarup@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_power.c    | 44 +++++++++++--------
> >  drivers/gpu/drm/i915/i915_reg.h               | 14 ++++--
> >  2 files changed, 35 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 72312b67b57a..2c1ce50b572b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -5254,7 +5254,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> >  	enum intel_dram_type type = dev_priv->dram_info.type;
> >  	u8 num_channels = dev_priv->dram_info.num_channels;
> >  	const struct buddy_page_mask *table;
> > -	int i;
> > +	int config, min_buddy, max_buddy, i;
> >  
> >  	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
> >  		/* Wa_1409767108: tgl */
> > @@ -5262,29 +5262,35 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> >  	else
> >  		table = tgl_buddy_page_masks;
> >  
> > -	for (i = 0; table[i].page_mask != 0; i++)
> > -		if (table[i].num_channels == num_channels &&
> > -		    table[i].type == type)
> > +	if (IS_ROCKETLAKE(dev_priv)) {
> > +		min_buddy = max_buddy = 0;
> > +	} else {
> > +		min_buddy = 1;
> > +		max_buddy = 2;
> > +	}
> > +
> > +	for (config = 0; table[config].page_mask != 0; config++)
> > +		if (table[config].num_channels == num_channels &&
> > +		    table[config].type == type)
> >  			break;
> >  
> > -	if (table[i].page_mask == 0) {
> > +	if (table[config].page_mask == 0) {
> >  		drm_dbg(&dev_priv->drm,
> >  			"Unknown memory configuration; disabling address buddy logic.\n");
> > -		intel_de_write(dev_priv, BW_BUDDY1_CTL, BW_BUDDY_DISABLE);
> > -		intel_de_write(dev_priv, BW_BUDDY2_CTL, BW_BUDDY_DISABLE);
> > +		for (i = min_buddy; i <= max_buddy; i++)
> > +			intel_de_write(dev_priv, BW_BUDDY_CTL(i),
> > +				       BW_BUDDY_DISABLE);
> >  	} else {
> > -		intel_de_write(dev_priv, BW_BUDDY1_PAGE_MASK,
> > -			       table[i].page_mask);
> > -		intel_de_write(dev_priv, BW_BUDDY2_PAGE_MASK,
> > -			       table[i].page_mask);
> > -
> > -		/* Wa_22010178259:tgl */
> > -		intel_de_rmw(dev_priv, BW_BUDDY1_CTL,
> > -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> > -		intel_de_rmw(dev_priv, BW_BUDDY2_CTL,
> > -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> > +		for (i = min_buddy; i <= max_buddy; i++) {
> > +			intel_de_write(dev_priv, BW_BUDDY_PAGE_MASK(i),
> > +				       table[config].page_mask);
> > +
> > +			/* Wa_22010178259:tgl,rkl */
> > +			intel_de_rmw(dev_priv, BW_BUDDY_CTL(i),
> > +				     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > +				     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK,
> > +						    0x8));
> We should be using REG_FIELD_PREP() in i915_reg.h to declare
> TLB_REQ_TIMER value and then use the value here.

Any specific reason why?  The value "8" doesn't have any specific
hardware meaning that would be meaningful to define in the general
register definitions.  It's just a value that this specific hardware
workaround asked for in this case.  I'm not sure if we want to spread
the definition of the workaround into the register file if the value
isn't going to be meaningful to other driver programming or workarounds.


Matt

> > +		}
> >  	}
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 578cfe11cbb9..3e79cefc510a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7837,13 +7837,19 @@ enum {
> >  #define  WAIT_FOR_PCH_RESET_ACK		(1 << 1)
> >  #define  WAIT_FOR_PCH_FLR_ACK		(1 << 0)
> >  
> > -#define BW_BUDDY1_CTL			_MMIO(0x45140)
> > -#define BW_BUDDY2_CTL			_MMIO(0x45150)
> > +#define _BW_BUDDY0_CTL			0x45130
> > +#define _BW_BUDDY1_CTL			0x45140
> > +#define BW_BUDDY_CTL(x)			_MMIO(_PICK_EVEN(x, \
> > +							 _BW_BUDDY0_CTL, \
> > +							 _BW_BUDDY1_CTL))
> >  #define   BW_BUDDY_DISABLE		REG_BIT(31)
> >  #define   BW_BUDDY_TLB_REQ_TIMER_MASK	REG_GENMASK(21, 16)
> >  
> > -#define BW_BUDDY1_PAGE_MASK		_MMIO(0x45144)
> > -#define BW_BUDDY2_PAGE_MASK		_MMIO(0x45154)
> > +#define _BW_BUDDY0_PAGE_MASK		0x45134
> > +#define _BW_BUDDY1_PAGE_MASK		0x45144
> > +#define BW_BUDDY_PAGE_MASK(x)		_MMIO(_PICK_EVEN(x, \
> > +							 _BW_BUDDY0_PAGE_MASK, \
> > +							 _BW_BUDDY1_PAGE_MASK))
> >  
> >  #define HSW_NDE_RSTWRN_OPT	_MMIO(0x46408)
> >  #define  RESET_PCH_HANDSHAKE_ENABLE	(1 << 4)
> > -- 
> > 2.24.1
> >
Aditya Swarup June 4, 2020, 1:18 a.m. UTC | #3
On Wed, Jun 03, 2020 at 04:12:59PM -0700, Matt Roper wrote:
> On Wed, Jun 03, 2020 at 03:34:32PM -0700, Aditya Swarup wrote:
> > On Wed, Jun 03, 2020 at 02:15:16PM -0700, Matt Roper wrote:
> > > RKL uses the same BW_BUDDY programming table as TGL, but programs the
> > > values into a single set BUDDY0 set of registers rather than the
> > > BUDDY1/BUDDY2 sets used by TGL.
> > > 
> > > Bspec: 49218
> > > Cc: Aditya Swarup <aditya.swarup@intel.com>
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > >  .../drm/i915/display/intel_display_power.c    | 44 +++++++++++--------
> > >  drivers/gpu/drm/i915/i915_reg.h               | 14 ++++--
> > >  2 files changed, 35 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > index 72312b67b57a..2c1ce50b572b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > @@ -5254,7 +5254,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> > >  	enum intel_dram_type type = dev_priv->dram_info.type;
> > >  	u8 num_channels = dev_priv->dram_info.num_channels;
> > >  	const struct buddy_page_mask *table;
> > > -	int i;
> > > +	int config, min_buddy, max_buddy, i;
> > >  
> > >  	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
> > >  		/* Wa_1409767108: tgl */
> > > @@ -5262,29 +5262,35 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> > >  	else
> > >  		table = tgl_buddy_page_masks;
> > >  
> > > -	for (i = 0; table[i].page_mask != 0; i++)
> > > -		if (table[i].num_channels == num_channels &&
> > > -		    table[i].type == type)
> > > +	if (IS_ROCKETLAKE(dev_priv)) {
> > > +		min_buddy = max_buddy = 0;
> > > +	} else {
> > > +		min_buddy = 1;
> > > +		max_buddy = 2;
> > > +	}
> > > +
> > > +	for (config = 0; table[config].page_mask != 0; config++)
> > > +		if (table[config].num_channels == num_channels &&
> > > +		    table[config].type == type)
> > >  			break;
> > >  
> > > -	if (table[i].page_mask == 0) {
> > > +	if (table[config].page_mask == 0) {
> > >  		drm_dbg(&dev_priv->drm,
> > >  			"Unknown memory configuration; disabling address buddy logic.\n");
> > > -		intel_de_write(dev_priv, BW_BUDDY1_CTL, BW_BUDDY_DISABLE);
> > > -		intel_de_write(dev_priv, BW_BUDDY2_CTL, BW_BUDDY_DISABLE);
> > > +		for (i = min_buddy; i <= max_buddy; i++)
> > > +			intel_de_write(dev_priv, BW_BUDDY_CTL(i),
> > > +				       BW_BUDDY_DISABLE);
> > >  	} else {
> > > -		intel_de_write(dev_priv, BW_BUDDY1_PAGE_MASK,
> > > -			       table[i].page_mask);
> > > -		intel_de_write(dev_priv, BW_BUDDY2_PAGE_MASK,
> > > -			       table[i].page_mask);
> > > -
> > > -		/* Wa_22010178259:tgl */
> > > -		intel_de_rmw(dev_priv, BW_BUDDY1_CTL,
> > > -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > > -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> > > -		intel_de_rmw(dev_priv, BW_BUDDY2_CTL,
> > > -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > > -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> > > +		for (i = min_buddy; i <= max_buddy; i++) {
> > > +			intel_de_write(dev_priv, BW_BUDDY_PAGE_MASK(i),
> > > +				       table[config].page_mask);
> > > +
> > > +			/* Wa_22010178259:tgl,rkl */
> > > +			intel_de_rmw(dev_priv, BW_BUDDY_CTL(i),
> > > +				     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > > +				     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK,
> > > +						    0x8));
> > We should be using REG_FIELD_PREP() in i915_reg.h to declare
> > TLB_REQ_TIMER value and then use the value here.
> 
> Any specific reason why?  The value "8" doesn't have any specific
> hardware meaning that would be meaningful to define in the general
> register definitions.  It's just a value that this specific hardware
> workaround asked for in this case.  I'm not sure if we want to spread
> the definition of the workaround into the register file if the value
> isn't going to be meaningful to other driver programming or workarounds.
> 
> 
> Matt
The value 8 is constant and as such should be defined in a header file
for that bitmask. If there was variable used to prepare the value on the
fly, I would have understood this usage.

If you are concerned about 8 being a random value and not spreading to
i915_reg.h, I would prefer a macro in i915_reg.h like:
#define TLB_REQ_TIMER(x)    REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK,(x))       

Then the value doesn't filter to i915_reg.h and according to me looks
cleaner.

Most of the usage of REG_FIELD_PREP in *.c files is based on creating
bitfields using variable. Here we are using a constant which can easily
be moved to i915_reg.h. It shouldn't matter if it is a WA or not. 

Adi
> 
> > > +		}
> > >  	}
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 578cfe11cbb9..3e79cefc510a 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7837,13 +7837,19 @@ enum {
> > >  #define  WAIT_FOR_PCH_RESET_ACK		(1 << 1)
> > >  #define  WAIT_FOR_PCH_FLR_ACK		(1 << 0)
> > >  
> > > -#define BW_BUDDY1_CTL			_MMIO(0x45140)
> > > -#define BW_BUDDY2_CTL			_MMIO(0x45150)
> > > +#define _BW_BUDDY0_CTL			0x45130
> > > +#define _BW_BUDDY1_CTL			0x45140
> > > +#define BW_BUDDY_CTL(x)			_MMIO(_PICK_EVEN(x, \
> > > +							 _BW_BUDDY0_CTL, \
> > > +							 _BW_BUDDY1_CTL))
> > >  #define   BW_BUDDY_DISABLE		REG_BIT(31)
> > >  #define   BW_BUDDY_TLB_REQ_TIMER_MASK	REG_GENMASK(21, 16)
> > >  
> > > -#define BW_BUDDY1_PAGE_MASK		_MMIO(0x45144)
> > > -#define BW_BUDDY2_PAGE_MASK		_MMIO(0x45154)
> > > +#define _BW_BUDDY0_PAGE_MASK		0x45134
> > > +#define _BW_BUDDY1_PAGE_MASK		0x45144
> > > +#define BW_BUDDY_PAGE_MASK(x)		_MMIO(_PICK_EVEN(x, \
> > > +							 _BW_BUDDY0_PAGE_MASK, \
> > > +							 _BW_BUDDY1_PAGE_MASK))
> > >  
> > >  #define HSW_NDE_RSTWRN_OPT	_MMIO(0x46408)
> > >  #define  RESET_PCH_HANDSHAKE_ENABLE	(1 << 4)
> > > -- 
> > > 2.24.1
> > > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
Ville Syrjälä June 4, 2020, 5:01 p.m. UTC | #4
On Wed, Jun 03, 2020 at 02:15:16PM -0700, Matt Roper wrote:
> RKL uses the same BW_BUDDY programming table as TGL, but programs the
> values into a single set BUDDY0 set of registers rather than the
> BUDDY1/BUDDY2 sets used by TGL.

Maybe we just want some kind of HAS_ABOX() so we could use the same
thing here and in the ABOX_CTL programming?

> 
> Bspec: 49218
> Cc: Aditya Swarup <aditya.swarup@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    | 44 +++++++++++--------
>  drivers/gpu/drm/i915/i915_reg.h               | 14 ++++--
>  2 files changed, 35 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 72312b67b57a..2c1ce50b572b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -5254,7 +5254,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
>  	enum intel_dram_type type = dev_priv->dram_info.type;
>  	u8 num_channels = dev_priv->dram_info.num_channels;
>  	const struct buddy_page_mask *table;
> -	int i;
> +	int config, min_buddy, max_buddy, i;
>  
>  	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
>  		/* Wa_1409767108: tgl */
> @@ -5262,29 +5262,35 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
>  	else
>  		table = tgl_buddy_page_masks;
>  
> -	for (i = 0; table[i].page_mask != 0; i++)
> -		if (table[i].num_channels == num_channels &&
> -		    table[i].type == type)
> +	if (IS_ROCKETLAKE(dev_priv)) {
> +		min_buddy = max_buddy = 0;
> +	} else {
> +		min_buddy = 1;
> +		max_buddy = 2;
> +	}
> +
> +	for (config = 0; table[config].page_mask != 0; config++)
> +		if (table[config].num_channels == num_channels &&
> +		    table[config].type == type)
>  			break;
>  
> -	if (table[i].page_mask == 0) {
> +	if (table[config].page_mask == 0) {
>  		drm_dbg(&dev_priv->drm,
>  			"Unknown memory configuration; disabling address buddy logic.\n");
> -		intel_de_write(dev_priv, BW_BUDDY1_CTL, BW_BUDDY_DISABLE);
> -		intel_de_write(dev_priv, BW_BUDDY2_CTL, BW_BUDDY_DISABLE);
> +		for (i = min_buddy; i <= max_buddy; i++)
> +			intel_de_write(dev_priv, BW_BUDDY_CTL(i),
> +				       BW_BUDDY_DISABLE);
>  	} else {
> -		intel_de_write(dev_priv, BW_BUDDY1_PAGE_MASK,
> -			       table[i].page_mask);
> -		intel_de_write(dev_priv, BW_BUDDY2_PAGE_MASK,
> -			       table[i].page_mask);
> -
> -		/* Wa_22010178259:tgl */
> -		intel_de_rmw(dev_priv, BW_BUDDY1_CTL,
> -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> -		intel_de_rmw(dev_priv, BW_BUDDY2_CTL,
> -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> +		for (i = min_buddy; i <= max_buddy; i++) {
> +			intel_de_write(dev_priv, BW_BUDDY_PAGE_MASK(i),
> +				       table[config].page_mask);
> +
> +			/* Wa_22010178259:tgl,rkl */
> +			intel_de_rmw(dev_priv, BW_BUDDY_CTL(i),
> +				     BW_BUDDY_TLB_REQ_TIMER_MASK,
> +				     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK,
> +						    0x8));
> +		}
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 578cfe11cbb9..3e79cefc510a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7837,13 +7837,19 @@ enum {
>  #define  WAIT_FOR_PCH_RESET_ACK		(1 << 1)
>  #define  WAIT_FOR_PCH_FLR_ACK		(1 << 0)
>  
> -#define BW_BUDDY1_CTL			_MMIO(0x45140)
> -#define BW_BUDDY2_CTL			_MMIO(0x45150)
> +#define _BW_BUDDY0_CTL			0x45130
> +#define _BW_BUDDY1_CTL			0x45140
> +#define BW_BUDDY_CTL(x)			_MMIO(_PICK_EVEN(x, \
> +							 _BW_BUDDY0_CTL, \
> +							 _BW_BUDDY1_CTL))
>  #define   BW_BUDDY_DISABLE		REG_BIT(31)
>  #define   BW_BUDDY_TLB_REQ_TIMER_MASK	REG_GENMASK(21, 16)
>  
> -#define BW_BUDDY1_PAGE_MASK		_MMIO(0x45144)
> -#define BW_BUDDY2_PAGE_MASK		_MMIO(0x45154)
> +#define _BW_BUDDY0_PAGE_MASK		0x45134
> +#define _BW_BUDDY1_PAGE_MASK		0x45144
> +#define BW_BUDDY_PAGE_MASK(x)		_MMIO(_PICK_EVEN(x, \
> +							 _BW_BUDDY0_PAGE_MASK, \
> +							 _BW_BUDDY1_PAGE_MASK))
>  
>  #define HSW_NDE_RSTWRN_OPT	_MMIO(0x46408)
>  #define  RESET_PCH_HANDSHAKE_ENABLE	(1 << 4)
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Matt Roper June 4, 2020, 10:12 p.m. UTC | #5
On Thu, Jun 04, 2020 at 08:01:57PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 03, 2020 at 02:15:16PM -0700, Matt Roper wrote:
> > RKL uses the same BW_BUDDY programming table as TGL, but programs the
> > values into a single set BUDDY0 set of registers rather than the
> > BUDDY1/BUDDY2 sets used by TGL.
> 
> Maybe we just want some kind of HAS_ABOX() so we could use the same
> thing here and in the ABOX_CTL programming?

Although these are both related to how the display controller accesses
memory, I don't think they're quite a 1:1 mapping.  TGL has
MBUX_ABOX_CTL{0,1,2} (and we're directed to program all three), but only
has BW_BUDDY_CTL{1,2} and no 0 instance.

For now I'll just add separate bw_buddy and abox masks to our platform
device info structure.


Matt

> 
> > 
> > Bspec: 49218
> > Cc: Aditya Swarup <aditya.swarup@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_power.c    | 44 +++++++++++--------
> >  drivers/gpu/drm/i915/i915_reg.h               | 14 ++++--
> >  2 files changed, 35 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 72312b67b57a..2c1ce50b572b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -5254,7 +5254,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> >  	enum intel_dram_type type = dev_priv->dram_info.type;
> >  	u8 num_channels = dev_priv->dram_info.num_channels;
> >  	const struct buddy_page_mask *table;
> > -	int i;
> > +	int config, min_buddy, max_buddy, i;
> >  
> >  	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
> >  		/* Wa_1409767108: tgl */
> > @@ -5262,29 +5262,35 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> >  	else
> >  		table = tgl_buddy_page_masks;
> >  
> > -	for (i = 0; table[i].page_mask != 0; i++)
> > -		if (table[i].num_channels == num_channels &&
> > -		    table[i].type == type)
> > +	if (IS_ROCKETLAKE(dev_priv)) {
> > +		min_buddy = max_buddy = 0;
> > +	} else {
> > +		min_buddy = 1;
> > +		max_buddy = 2;
> > +	}
> > +
> > +	for (config = 0; table[config].page_mask != 0; config++)
> > +		if (table[config].num_channels == num_channels &&
> > +		    table[config].type == type)
> >  			break;
> >  
> > -	if (table[i].page_mask == 0) {
> > +	if (table[config].page_mask == 0) {
> >  		drm_dbg(&dev_priv->drm,
> >  			"Unknown memory configuration; disabling address buddy logic.\n");
> > -		intel_de_write(dev_priv, BW_BUDDY1_CTL, BW_BUDDY_DISABLE);
> > -		intel_de_write(dev_priv, BW_BUDDY2_CTL, BW_BUDDY_DISABLE);
> > +		for (i = min_buddy; i <= max_buddy; i++)
> > +			intel_de_write(dev_priv, BW_BUDDY_CTL(i),
> > +				       BW_BUDDY_DISABLE);
> >  	} else {
> > -		intel_de_write(dev_priv, BW_BUDDY1_PAGE_MASK,
> > -			       table[i].page_mask);
> > -		intel_de_write(dev_priv, BW_BUDDY2_PAGE_MASK,
> > -			       table[i].page_mask);
> > -
> > -		/* Wa_22010178259:tgl */
> > -		intel_de_rmw(dev_priv, BW_BUDDY1_CTL,
> > -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> > -		intel_de_rmw(dev_priv, BW_BUDDY2_CTL,
> > -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> > +		for (i = min_buddy; i <= max_buddy; i++) {
> > +			intel_de_write(dev_priv, BW_BUDDY_PAGE_MASK(i),
> > +				       table[config].page_mask);
> > +
> > +			/* Wa_22010178259:tgl,rkl */
> > +			intel_de_rmw(dev_priv, BW_BUDDY_CTL(i),
> > +				     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > +				     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK,
> > +						    0x8));
> > +		}
> >  	}
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 578cfe11cbb9..3e79cefc510a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7837,13 +7837,19 @@ enum {
> >  #define  WAIT_FOR_PCH_RESET_ACK		(1 << 1)
> >  #define  WAIT_FOR_PCH_FLR_ACK		(1 << 0)
> >  
> > -#define BW_BUDDY1_CTL			_MMIO(0x45140)
> > -#define BW_BUDDY2_CTL			_MMIO(0x45150)
> > +#define _BW_BUDDY0_CTL			0x45130
> > +#define _BW_BUDDY1_CTL			0x45140
> > +#define BW_BUDDY_CTL(x)			_MMIO(_PICK_EVEN(x, \
> > +							 _BW_BUDDY0_CTL, \
> > +							 _BW_BUDDY1_CTL))
> >  #define   BW_BUDDY_DISABLE		REG_BIT(31)
> >  #define   BW_BUDDY_TLB_REQ_TIMER_MASK	REG_GENMASK(21, 16)
> >  
> > -#define BW_BUDDY1_PAGE_MASK		_MMIO(0x45144)
> > -#define BW_BUDDY2_PAGE_MASK		_MMIO(0x45154)
> > +#define _BW_BUDDY0_PAGE_MASK		0x45134
> > +#define _BW_BUDDY1_PAGE_MASK		0x45144
> > +#define BW_BUDDY_PAGE_MASK(x)		_MMIO(_PICK_EVEN(x, \
> > +							 _BW_BUDDY0_PAGE_MASK, \
> > +							 _BW_BUDDY1_PAGE_MASK))
> >  
> >  #define HSW_NDE_RSTWRN_OPT	_MMIO(0x46408)
> >  #define  RESET_PCH_HANDSHAKE_ENABLE	(1 << 4)
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
Ville Syrjälä June 5, 2020, 11:43 a.m. UTC | #6
On Thu, Jun 04, 2020 at 03:12:40PM -0700, Matt Roper wrote:
> On Thu, Jun 04, 2020 at 08:01:57PM +0300, Ville Syrjälä wrote:
> > On Wed, Jun 03, 2020 at 02:15:16PM -0700, Matt Roper wrote:
> > > RKL uses the same BW_BUDDY programming table as TGL, but programs the
> > > values into a single set BUDDY0 set of registers rather than the
> > > BUDDY1/BUDDY2 sets used by TGL.
> > 
> > Maybe we just want some kind of HAS_ABOX() so we could use the same
> > thing here and in the ABOX_CTL programming?
> 
> Although these are both related to how the display controller accesses
> memory, I don't think they're quite a 1:1 mapping.  TGL has
> MBUX_ABOX_CTL{0,1,2} (and we're directed to program all three), but only
> has BW_BUDDY_CTL{1,2} and no 0 instance.

I see three different notes on this:

tgl style:
"Abox1 and Abox2 are used for pixel data reads. Program the BW_BUDDY1
 and BW_BUDDY2 registers."
abox0 is the legacy path, so doesn't need this stuff apparently.
abox1/2 are the dedicated path.

rkl style:
"Abox0 is used for pixel data reads. Program the BW_BUDDY0 registers."
the dedicated path is removed so I guess they had to add the buddy
registers for the legacy path abox.

some other style:
"Abox0 and Abox1 are used for pixel data reads. Program the BW_BUDDY0 and
 BW_BUDDY1 registers."
I presume this means the legacy path is getting nuked, and thus the
two aboxes for the dedicated path are just getting shifted down.
Unfortunately I can't find a hsd which tells the story. 

So yeah, there is a slight difference for the tgl style in that
we still want to program ABOX_CTL0 but not BW_BUDDY0.

> 
> For now I'll just add separate bw_buddy and abox masks to our platform
> device info structure.

Or could maybe just add an exception for tgl.

Either
if (HAS_ABOX(0) || IS_TGL)
	write(ABOX_CTL0);
...
or
if (HAS_ABOX(0) && !IS_TGL)
	write(BUDDY0);
...

would seem reasonable enough to me.

> 
> 
> Matt
> 
> > 
> > > 
> > > Bspec: 49218
> > > Cc: Aditya Swarup <aditya.swarup@intel.com>
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > >  .../drm/i915/display/intel_display_power.c    | 44 +++++++++++--------
> > >  drivers/gpu/drm/i915/i915_reg.h               | 14 ++++--
> > >  2 files changed, 35 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > index 72312b67b57a..2c1ce50b572b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > @@ -5254,7 +5254,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> > >  	enum intel_dram_type type = dev_priv->dram_info.type;
> > >  	u8 num_channels = dev_priv->dram_info.num_channels;
> > >  	const struct buddy_page_mask *table;
> > > -	int i;
> > > +	int config, min_buddy, max_buddy, i;
> > >  
> > >  	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
> > >  		/* Wa_1409767108: tgl */
> > > @@ -5262,29 +5262,35 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> > >  	else
> > >  		table = tgl_buddy_page_masks;
> > >  
> > > -	for (i = 0; table[i].page_mask != 0; i++)
> > > -		if (table[i].num_channels == num_channels &&
> > > -		    table[i].type == type)
> > > +	if (IS_ROCKETLAKE(dev_priv)) {
> > > +		min_buddy = max_buddy = 0;
> > > +	} else {
> > > +		min_buddy = 1;
> > > +		max_buddy = 2;
> > > +	}
> > > +
> > > +	for (config = 0; table[config].page_mask != 0; config++)
> > > +		if (table[config].num_channels == num_channels &&
> > > +		    table[config].type == type)
> > >  			break;
> > >  
> > > -	if (table[i].page_mask == 0) {
> > > +	if (table[config].page_mask == 0) {
> > >  		drm_dbg(&dev_priv->drm,
> > >  			"Unknown memory configuration; disabling address buddy logic.\n");
> > > -		intel_de_write(dev_priv, BW_BUDDY1_CTL, BW_BUDDY_DISABLE);
> > > -		intel_de_write(dev_priv, BW_BUDDY2_CTL, BW_BUDDY_DISABLE);
> > > +		for (i = min_buddy; i <= max_buddy; i++)
> > > +			intel_de_write(dev_priv, BW_BUDDY_CTL(i),
> > > +				       BW_BUDDY_DISABLE);
> > >  	} else {
> > > -		intel_de_write(dev_priv, BW_BUDDY1_PAGE_MASK,
> > > -			       table[i].page_mask);
> > > -		intel_de_write(dev_priv, BW_BUDDY2_PAGE_MASK,
> > > -			       table[i].page_mask);
> > > -
> > > -		/* Wa_22010178259:tgl */
> > > -		intel_de_rmw(dev_priv, BW_BUDDY1_CTL,
> > > -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > > -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> > > -		intel_de_rmw(dev_priv, BW_BUDDY2_CTL,
> > > -			     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > > -			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
> > > +		for (i = min_buddy; i <= max_buddy; i++) {
> > > +			intel_de_write(dev_priv, BW_BUDDY_PAGE_MASK(i),
> > > +				       table[config].page_mask);
> > > +
> > > +			/* Wa_22010178259:tgl,rkl */
> > > +			intel_de_rmw(dev_priv, BW_BUDDY_CTL(i),
> > > +				     BW_BUDDY_TLB_REQ_TIMER_MASK,
> > > +				     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK,
> > > +						    0x8));
> > > +		}
> > >  	}
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 578cfe11cbb9..3e79cefc510a 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7837,13 +7837,19 @@ enum {
> > >  #define  WAIT_FOR_PCH_RESET_ACK		(1 << 1)
> > >  #define  WAIT_FOR_PCH_FLR_ACK		(1 << 0)
> > >  
> > > -#define BW_BUDDY1_CTL			_MMIO(0x45140)
> > > -#define BW_BUDDY2_CTL			_MMIO(0x45150)
> > > +#define _BW_BUDDY0_CTL			0x45130
> > > +#define _BW_BUDDY1_CTL			0x45140
> > > +#define BW_BUDDY_CTL(x)			_MMIO(_PICK_EVEN(x, \
> > > +							 _BW_BUDDY0_CTL, \
> > > +							 _BW_BUDDY1_CTL))
> > >  #define   BW_BUDDY_DISABLE		REG_BIT(31)
> > >  #define   BW_BUDDY_TLB_REQ_TIMER_MASK	REG_GENMASK(21, 16)
> > >  
> > > -#define BW_BUDDY1_PAGE_MASK		_MMIO(0x45144)
> > > -#define BW_BUDDY2_PAGE_MASK		_MMIO(0x45154)
> > > +#define _BW_BUDDY0_PAGE_MASK		0x45134
> > > +#define _BW_BUDDY1_PAGE_MASK		0x45144
> > > +#define BW_BUDDY_PAGE_MASK(x)		_MMIO(_PICK_EVEN(x, \
> > > +							 _BW_BUDDY0_PAGE_MASK, \
> > > +							 _BW_BUDDY1_PAGE_MASK))
> > >  
> > >  #define HSW_NDE_RSTWRN_OPT	_MMIO(0x46408)
> > >  #define  RESET_PCH_HANDSHAKE_ENABLE	(1 << 4)
> > > -- 
> > > 2.24.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 72312b67b57a..2c1ce50b572b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -5254,7 +5254,7 @@  static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
 	enum intel_dram_type type = dev_priv->dram_info.type;
 	u8 num_channels = dev_priv->dram_info.num_channels;
 	const struct buddy_page_mask *table;
-	int i;
+	int config, min_buddy, max_buddy, i;
 
 	if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
 		/* Wa_1409767108: tgl */
@@ -5262,29 +5262,35 @@  static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
 	else
 		table = tgl_buddy_page_masks;
 
-	for (i = 0; table[i].page_mask != 0; i++)
-		if (table[i].num_channels == num_channels &&
-		    table[i].type == type)
+	if (IS_ROCKETLAKE(dev_priv)) {
+		min_buddy = max_buddy = 0;
+	} else {
+		min_buddy = 1;
+		max_buddy = 2;
+	}
+
+	for (config = 0; table[config].page_mask != 0; config++)
+		if (table[config].num_channels == num_channels &&
+		    table[config].type == type)
 			break;
 
-	if (table[i].page_mask == 0) {
+	if (table[config].page_mask == 0) {
 		drm_dbg(&dev_priv->drm,
 			"Unknown memory configuration; disabling address buddy logic.\n");
-		intel_de_write(dev_priv, BW_BUDDY1_CTL, BW_BUDDY_DISABLE);
-		intel_de_write(dev_priv, BW_BUDDY2_CTL, BW_BUDDY_DISABLE);
+		for (i = min_buddy; i <= max_buddy; i++)
+			intel_de_write(dev_priv, BW_BUDDY_CTL(i),
+				       BW_BUDDY_DISABLE);
 	} else {
-		intel_de_write(dev_priv, BW_BUDDY1_PAGE_MASK,
-			       table[i].page_mask);
-		intel_de_write(dev_priv, BW_BUDDY2_PAGE_MASK,
-			       table[i].page_mask);
-
-		/* Wa_22010178259:tgl */
-		intel_de_rmw(dev_priv, BW_BUDDY1_CTL,
-			     BW_BUDDY_TLB_REQ_TIMER_MASK,
-			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
-		intel_de_rmw(dev_priv, BW_BUDDY2_CTL,
-			     BW_BUDDY_TLB_REQ_TIMER_MASK,
-			     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK, 0x8));
+		for (i = min_buddy; i <= max_buddy; i++) {
+			intel_de_write(dev_priv, BW_BUDDY_PAGE_MASK(i),
+				       table[config].page_mask);
+
+			/* Wa_22010178259:tgl,rkl */
+			intel_de_rmw(dev_priv, BW_BUDDY_CTL(i),
+				     BW_BUDDY_TLB_REQ_TIMER_MASK,
+				     REG_FIELD_PREP(BW_BUDDY_TLB_REQ_TIMER_MASK,
+						    0x8));
+		}
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 578cfe11cbb9..3e79cefc510a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7837,13 +7837,19 @@  enum {
 #define  WAIT_FOR_PCH_RESET_ACK		(1 << 1)
 #define  WAIT_FOR_PCH_FLR_ACK		(1 << 0)
 
-#define BW_BUDDY1_CTL			_MMIO(0x45140)
-#define BW_BUDDY2_CTL			_MMIO(0x45150)
+#define _BW_BUDDY0_CTL			0x45130
+#define _BW_BUDDY1_CTL			0x45140
+#define BW_BUDDY_CTL(x)			_MMIO(_PICK_EVEN(x, \
+							 _BW_BUDDY0_CTL, \
+							 _BW_BUDDY1_CTL))
 #define   BW_BUDDY_DISABLE		REG_BIT(31)
 #define   BW_BUDDY_TLB_REQ_TIMER_MASK	REG_GENMASK(21, 16)
 
-#define BW_BUDDY1_PAGE_MASK		_MMIO(0x45144)
-#define BW_BUDDY2_PAGE_MASK		_MMIO(0x45154)
+#define _BW_BUDDY0_PAGE_MASK		0x45134
+#define _BW_BUDDY1_PAGE_MASK		0x45144
+#define BW_BUDDY_PAGE_MASK(x)		_MMIO(_PICK_EVEN(x, \
+							 _BW_BUDDY0_PAGE_MASK, \
+							 _BW_BUDDY1_PAGE_MASK))
 
 #define HSW_NDE_RSTWRN_OPT	_MMIO(0x46408)
 #define  RESET_PCH_HANDSHAKE_ENABLE	(1 << 4)