[Intel-gfx] [PATCH v3 02/15] drm/i915/rkl: Program BW_BUDDY0 registers instead of BW_BUDDY1/2

[Matt Roper]

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 at intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper at 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
> > 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795