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 <[email protected]>
> > > Signed-off-by: Matt Roper <[email protected]>
> > > ---
> > >  .../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
> > > [email protected]
> > > 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

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to