On Thu, Dec 05, 2019 at 07:31:15PM +0200, Ville Syrjälä wrote:
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.
we can, unfortunately that would need to be definition rather than
declaration. This works for me:
enum __packed bla {
...
};
But this doesn't if the enum wasn't defined as packed:
enum __packed bla bla;
Lucas De Marchi
>+ 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
