On Thu, Dec 05, 2019 at 09:19:43AM -0800, Lucas De Marchi wrote:
> On Wed, Dec 04, 2019 at 12:51:59PM -0800, Matt Roper wrote:
> >Gen12 can improve bandwidth efficiency by pairing up memory requests
> >with similar addresses. We need to program the BW_BUDDY1 and BW_BUDDY2
> >registers according to the memory configuration during display
> >initialization to take advantage of this capability.
> >
> >The magic numbers we program here feel like something that could
> >definitely change on future platforms, so let's use a table-based
> >programming scheme to make this easy to extend in the future.
> >
> >Bspec: 49189
> >Bspec: 49218
> >Cc: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> >Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> >Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> >Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> >---
> > .../drm/i915/display/intel_display_power.c | 40 +++++++++++++++++++
> > drivers/gpu/drm/i915/i915_reg.h | 8 ++++
> > 2 files changed, 48 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> >index ce1b64f4dd44..1d26f741b5b4 100644
> >--- a/drivers/gpu/drm/i915/display/intel_display_power.c
> >+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> >@@ -4781,6 +4781,42 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
> > intel_combo_phy_uninit(dev_priv);
> > }
> >
> >+struct buddy_page_mask {
> >+ u8 num_channels;
>
> I'd make this a word rather than introducing a hole here just to match
> the type.
I'd rather just shrink page_mask and reorder a bit to get a smaller
struct. If only we could also make enums take less that 4 bytes.
u8/s8 would be more than enough for most enums.
>
> >+ enum intel_dram_type type;
> >+ u32 page_mask;
> >+};
> >+
> >+const struct buddy_page_mask tgl_buddy_page_masks[] = {
> >+ { 1, INTEL_DRAM_LPDDR4, 0xE },
> >+ { 1, INTEL_DRAM_DDR4, 0xF },
> >+ { 2, INTEL_DRAM_LPDDR4, 0x1C },
> >+ { 2, INTEL_DRAM_DDR4, 0x1F },
> >+ {}
> >+};
> >+
> >+static void tgl_arbiter_init(struct drm_i915_private *dev_priv)
>
> can we call this something related to bw_buddy? Like
> tgl_bw_buddy_init() or tgl_bw_buddy_arbiter_init()
>
> >+{
> >+ 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 = tgl_buddy_page_masks;
> >+ int i;
> >+
> >+ for (i = 0; table[i].page_mask != 0; i++)
> >+ if (table[i].num_channels == num_channels &&
> >+ table[i].type == type)
> >+ break;
> >+
> >+ if (table[i].page_mask == 0) {
> >+ DRM_DEBUG_DRIVER("Unknown memory configuration; disabling address buddy logic.\n");
> >+ I915_WRITE(BW_BUDDY1_CTL, BW_BUDDY_DISABLE);
> >+ I915_WRITE(BW_BUDDY2_CTL, BW_BUDDY_DISABLE);
> >+ } else {
> >+ I915_WRITE(BW_BUDDY1_PAGE_MASK, table[i].page_mask);
> >+ I915_WRITE(BW_BUDDY2_PAGE_MASK, table[i].page_mask);
> >+ }
> >+}
> >+
> > static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > bool resume)
> > {
> >@@ -4813,6 +4849,10 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
> > /* 6. Setup MBUS. */
> > icl_mbus_init(dev_priv);
> >
> >+ /* 7. Program arbiter BW_BUDDY registers */
> >+ if (INTEL_GEN(dev_priv) >= 12)
> >+ tgl_arbiter_init(dev_priv);
>
> looks sane, but watch out for the WA here.
>
> thanks
> Lucas De Marchi
>
> >+
> > if (resume && dev_priv->csr.dmc_payload)
> > intel_csr_load_program(dev_priv);
> > }
> >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >index 1a6376a97d48..082190c2dc48 100644
> >--- a/drivers/gpu/drm/i915/i915_reg.h
> >+++ b/drivers/gpu/drm/i915/i915_reg.h
> >@@ -7765,6 +7765,14 @@ enum {
> > #define GEN7_MSG_CTL _MMIO(0x45010)
> > #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_BUDDY_DISABLE REG_BIT(31)
> >+
> >+#define BW_BUDDY1_PAGE_MASK _MMIO(0x45144)
> >+#define BW_BUDDY2_PAGE_MASK _MMIO(0x45154)
> >+
> > #define HSW_NDE_RSTWRN_OPT _MMIO(0x46408)
> > #define RESET_PCH_HANDSHAKE_ENABLE (1 << 4)
> >
> >--
> >2.23.0
> >
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
