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.swa...@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.ro...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
