[Intel-gfx] [PATCH] drm/i915/tgl: Program BW_BUDDY registers during display init
Lucas De Marchi
lucas.demarchi at intel.com
Thu Dec 5 19:26:28 UTC 2019
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 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.
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
More information about the Intel-gfx
mailing list