Skip to contentSkip to conversation

memory: tegra: Fix memory arbitration programming

PATCH Applied Dmitry Osipenko 2 participants 6 messages Apr 11, 2019 – Apr 18, 2019

Subsystems (4)
Arm (32-bit) Arm sub-architectures Tegra architecture support Memory controller drivers
lore
Share & tools
lore

Dmitry Osipenko

author
#0 lore raw

cover memory: tegra: Fix memory arbitration programming

Hello, this series properly addresses the bugged memory arbitration programming and in result the previous suspend-resume workaround change is reverted as it was not entirely correct.

Dmitry Osipenko (4):
memory: tegra: Fix missed registers values latching
memory: tegra: Fix integer overflow on tick value calculation
memory: tegra: Replace readl-writel with mc_readl-mc_writel Revert "ARM: tegra: Restore memory arbitration on resume from LP1 on

    Tegra30+"
 arch/arm/mach-tegra/iomap.h         |  9 ---------
 arch/arm/mach-tegra/sleep-tegra30.S | 21 ---------------------
 drivers/memory/tegra/mc.c           | 16 +++++++++++-----
 3 files changed, 11 insertions(+), 35 deletions(-)
Signature · 1 line
2.21.0
author
#1 lore raw

PATCH 1/4 memory: tegra: Fix missed registers values latching

Some of Memory Controller registers are shadowed and require latching in order to copy assembly state into the active, MC_EMEM_ARB_CFG is one of these registers.

Signed-off-by Dmitry Osipenko <[email protected]>
---
 drivers/memory/tegra/mc.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/memory/tegra/mc.c +6 -0
index 0a53598d982f..60474597180b 100644
@@ -51,6 +51,9 @@
 #define MC_EMEM_ADR_CFG 0x54
 #define MC_EMEM_ADR_CFG_EMEM_NUMDEV BIT(0)
 
+#define MC_TIMING_CONTROL		0xfc
+#define MC_TIMING_UPDATE		BIT(0)
+
 static const struct of_device_id tegra_mc_of_match[] = {
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 	{ .compatible = "nvidia,tegra20-mc-gart", .data = &tegra20_mc_soc },

@@ -301,6 +304,9 @@ static int tegra_mc_setup_latency_allowance(struct tegra_mc *mc)
 		writel(value, mc->regs + la->reg);
 	}
 
+	/* latch new values */
+	writel(MC_TIMING_UPDATE, mc->regs + MC_TIMING_CONTROL);
+
 	return 0;
 }
Signature · 1 line
2.21.0
author
#2 lore raw

PATCH 2/4 memory: tegra: Fix integer overflow on tick value calculation

In reply to Dmitry Osipenko

Multiplying the Memory Controller clock rate by the tick count results in an integer overflow and in result the truncated tick value is being programmed into hardware, such that the GR3D memory client performance is reduced by two times.

Cc stable <[email protected]>
Signed-off-by Dmitry Osipenko <[email protected]>
---
 drivers/memory/tegra/mc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/memory/tegra/mc.c +1 -1
index 60474597180b..89e463952b8a 100644
@@ -285,7 +285,7 @@ static int tegra_mc_setup_latency_allowance(struct tegra_mc *mc)
 	u32 value;
 
 	/* compute the number of MC clock cycles per tick */
-	tick = mc->tick * clk_get_rate(mc->clk);
+	tick = (unsigned long long)mc->tick * clk_get_rate(mc->clk);
 	do_div(tick, NSEC_PER_SEC);
 
 	value = readl(mc->regs + MC_EMEM_ARB_CFG);
Signature · 1 line
2.21.0
author
#3 lore raw

PATCH 3/4 memory: tegra: Replace readl-writel with mc_readl-mc_writel

In reply to Dmitry Osipenko

There is no need for a memory barriers on reading/writing of register values as we only care about the read/write order, hence let's use the common helpers.

Signed-off-by Dmitry Osipenko <[email protected]>
---
 drivers/memory/tegra/mc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/memory/tegra/mc.c +5 -5
index 89e463952b8a..31b47459c84d 100644
@@ -288,24 +288,24 @@ static int tegra_mc_setup_latency_allowance(struct tegra_mc *mc)
 	tick = (unsigned long long)mc->tick * clk_get_rate(mc->clk);
 	do_div(tick, NSEC_PER_SEC);
 
-	value = readl(mc->regs + MC_EMEM_ARB_CFG);
+	value = mc_readl(mc, MC_EMEM_ARB_CFG);
 	value &= ~MC_EMEM_ARB_CFG_CYCLES_PER_UPDATE_MASK;
 	value |= MC_EMEM_ARB_CFG_CYCLES_PER_UPDATE(tick);
-	writel(value, mc->regs + MC_EMEM_ARB_CFG);
+	mc_writel(mc, value, MC_EMEM_ARB_CFG);
 
 	/* write latency allowance defaults */
 	for (i = 0; i < mc->soc->num_clients; i++) {
 		const struct tegra_mc_la *la = &mc->soc->clients[i].la;
 		u32 value;
 
-		value = readl(mc->regs + la->reg);
+		value = mc_readl(mc, la->reg);
 		value &= ~(la->mask << la->shift);
 		value |= (la->def & la->mask) << la->shift;
-		writel(value, mc->regs + la->reg);
+		mc_writel(mc, value, la->reg);
 	}
 
 	/* latch new values */
-	writel(MC_TIMING_UPDATE, mc->regs + MC_TIMING_CONTROL);
+	mc_writel(mc, MC_TIMING_UPDATE, MC_TIMING_CONTROL);
 
 	return 0;
 }
Signature · 1 line
2.21.0
author
#4 lore raw

PATCH 4/4 Revert "ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+"

In reply to Dmitry Osipenko

Turned out that the actual bug was in the Memory Controller driver that programmed shadowed registers without latching the new values and then there was a bug on EMEM arbitration configuration calculation that results in a wrong value being latched on resume from suspend. The Memory Controller has been fixed properly now, hence the workaround patch could be reverted safely.

Signed-off-by Dmitry Osipenko <[email protected]>
---
 arch/arm/mach-tegra/iomap.h         |  9 ---------
 arch/arm/mach-tegra/sleep-tegra30.S | 21 ---------------------
 2 files changed, 30 deletions(-)
arch/arm/mach-tegra/iomap.h +0 -9
index 4af9e92a216f..ba61db7fe533 100644
@@ -79,24 +79,15 @@
 #define TEGRA_PMC_BASE			0x7000E400
 #define TEGRA_PMC_SIZE			SZ_256
 
-#define TEGRA_MC_BASE			0x7000F000
-#define TEGRA_MC_SIZE			SZ_1K
-
 #define TEGRA_EMC_BASE			0x7000F400
 #define TEGRA_EMC_SIZE			SZ_1K
 
-#define TEGRA114_MC_BASE		0x70019000
-#define TEGRA114_MC_SIZE		SZ_4K
-
 #define TEGRA_EMC0_BASE			0x7001A000
 #define TEGRA_EMC0_SIZE			SZ_2K
 
 #define TEGRA_EMC1_BASE			0x7001A800
 #define TEGRA_EMC1_SIZE			SZ_2K
 
-#define TEGRA124_MC_BASE		0x70019000
-#define TEGRA124_MC_SIZE		SZ_4K
-
 #define TEGRA124_EMC_BASE		0x7001B000
 #define TEGRA124_EMC_SIZE		SZ_2K
arch/arm/mach-tegra/sleep-tegra30.S +0 -21
index d0b4c486ddbf..7727e005c30e 100644
@@ -44,8 +44,6 @@
 #define EMC_XM2VTTGENPADCTRL		0x310
 #define EMC_XM2VTTGENPADCTRL2		0x314
 
-#define MC_EMEM_ARB_CFG			0x90
-
 #define PMC_CTRL			0x0
 #define PMC_CTRL_SIDE_EFFECT_LP0 (1 << 14) /* enter LP0 when CPU pwr gated */
 

@@ -420,22 +418,6 @@ _pll_m_c_x_done:
 	movweq	r0, #:lower16:TEGRA124_EMC_BASE
 	movteq	r0, #:upper16:TEGRA124_EMC_BASE
 
-	cmp	r10, #TEGRA30
-	moveq	r2, #0x20
-	movweq	r4, #:lower16:TEGRA_MC_BASE
-	movteq	r4, #:upper16:TEGRA_MC_BASE
-	cmp	r10, #TEGRA114
-	moveq	r2, #0x34
-	movweq	r4, #:lower16:TEGRA114_MC_BASE
-	movteq	r4, #:upper16:TEGRA114_MC_BASE
-	cmp	r10, #TEGRA124
-	moveq	r2, #0x20
-	movweq	r4, #:lower16:TEGRA124_MC_BASE
-	movteq	r4, #:upper16:TEGRA124_MC_BASE
-
-	ldr	r1, [r5, r2]		@ restore MC_EMEM_ARB_CFG
-	str	r1, [r4, #MC_EMEM_ARB_CFG]
-
 exit_self_refresh:
 	ldr	r1, [r5, #0xC]		@ restore EMC_XM2VTTGENPADCTRL
 	str	r1, [r0, #EMC_XM2VTTGENPADCTRL]

@@ -564,7 +546,6 @@ tegra30_sdram_pad_address:
 	.word	TEGRA_PMC_BASE + PMC_IO_DPD_STATUS			@0x14
 	.word	TEGRA_CLK_RESET_BASE + CLK_RESET_CLK_SOURCE_MSELECT	@0x18
 	.word	TEGRA_CLK_RESET_BASE + CLK_RESET_SCLK_BURST		@0x1c
-	.word	TEGRA_MC_BASE + MC_EMEM_ARB_CFG				@0x20
 tegra30_sdram_pad_address_end:
 
 tegra114_sdram_pad_address:

@@ -581,7 +562,6 @@ tegra114_sdram_pad_address:
 	.word	TEGRA_EMC1_BASE + EMC_AUTO_CAL_INTERVAL			@0x28
 	.word	TEGRA_EMC1_BASE + EMC_XM2VTTGENPADCTRL			@0x2c
 	.word	TEGRA_EMC1_BASE + EMC_XM2VTTGENPADCTRL2			@0x30
-	.word	TEGRA114_MC_BASE + MC_EMEM_ARB_CFG			@0x34
 tegra114_sdram_pad_adress_end:
 
 tegra124_sdram_pad_address:

@@ -593,7 +573,6 @@ tegra124_sdram_pad_address:
 	.word	TEGRA_PMC_BASE + PMC_IO_DPD_STATUS			@0x14
 	.word	TEGRA_CLK_RESET_BASE + CLK_RESET_CLK_SOURCE_MSELECT	@0x18
 	.word	TEGRA_CLK_RESET_BASE + CLK_RESET_SCLK_BURST		@0x1c
-	.word	TEGRA124_MC_BASE + MC_EMEM_ARB_CFG			@0x20
 tegra124_sdram_pad_address_end:
 
 tegra30_sdram_pad_size:
Signature · 1 line
2.21.0
Applied
#5 lore raw
In reply to Dmitry Osipenko
Quoted 9 lines · Dmitry Osipenko

Hello, this series properly addresses the bugged memory arbitration programming and in result the previous suspend-resume workaround change is reverted as it was not entirely correct.

Dmitry Osipenko (4):
memory: tegra: Fix missed registers values latching
memory: tegra: Fix integer overflow on tick value calculation
memory: tegra: Replace readl-writel with mc_readl-mc_writel Revert "ARM: tegra: Restore memory arbitration on resume from LP1 on

Quoted 6 lines
    Tegra30+"
 arch/arm/mach-tegra/iomap.h         |  9 ---------
 arch/arm/mach-tegra/sleep-tegra30.S | 21 ---------------------
 drivers/memory/tegra/mc.c           | 16 +++++++++++-----
 3 files changed, 11 insertions(+), 35 deletions(-)

Applied to for-5.2/memory, thanks.

Thierry