[Intel-gfx] [PATCH] drm/i915/tgl: Program BW_BUDDY registers during display init

[Ville Syrjälä]

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 at intel.com>
> >Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> >Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> >Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> >---
> > .../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