[PATCH v1 0/4] EMC fixes for Tegra30+

[PATCH v1 0/4] EMC fixes for Tegra30+

[Dmitry Osipenko]

Hello,

This patch series fixes couple bugs in the memory self-refresh code.
The EMC / MC state is properly restored after patches being applied,
please review.

Dmitry Osipenko (4):
  ARM: tegra: Fix missed EMC registers latching on resume from LP1 on
    Tegra30+
  ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on
    Tegra30
  ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+
  ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+

 arch/arm/mach-tegra/iomap.h         |  9 ++++++++
 arch/arm/mach-tegra/sleep-tegra30.S | 32 +++++++++++++++++++++++++++--
 2 files changed, 39 insertions(+), 2 deletions(-)

-- 
2.18.0

[PATCH v1 1/4] ARM: tegra: Fix missed EMC registers latching on resume from LP1 on Tegra30+

[Dmitry Osipenko]

The memory interface configuration and re-calibration interval are left
unassigned on resume from LP1 because these registers are shadowed and
require latching after being adjusted.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/sleep-tegra30.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index 127fc78365fe..801fe58978ae 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -521,6 +521,8 @@ zcal_done:
 	ldr	r1, [r5, #0x0]		@ restore EMC_CFG
 	str	r1, [r0, #EMC_CFG]
 
+	emc_timing_update r1, r0
+
 	/* Tegra114 had dual EMC channel, now config the other one */
 	cmp	r10, #TEGRA114
 	bne	__no_dual_emc_chanl
-- 
2.18.0

[PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30

[Dmitry Osipenko]

The DRAM refresh-interval is getting erroneously set to "1" on exiting
from memory self-refreshing mode. The clobbered interval causes the
"refresh request overflow timeout" error raised by the External Memory
Controller on exiting from LP1 on Tegra30.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/sleep-tegra30.S | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index 801fe58978ae..99ac9c6dcf7c 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -29,7 +29,6 @@
 #define EMC_CFG				0xc
 #define EMC_ADR_CFG			0x10
 #define EMC_TIMING_CONTROL		0x28
-#define EMC_REFRESH			0x70
 #define EMC_NOP				0xdc
 #define EMC_SELF_REF			0xe0
 #define EMC_MRW				0xe8
@@ -459,7 +458,6 @@ emc_wait_auto_cal_onetime:
 	cmp	r10, #TEGRA30
 	streq	r1, [r0, #EMC_NOP]
 	streq	r1, [r0, #EMC_NOP]
-	streq	r1, [r0, #EMC_REFRESH]
 
 	emc_device_mask r1, r0
 
-- 
2.18.0

[PATCH v1 3/4] ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+

[Dmitry Osipenko]

The external memory arbitration configuration is getting reset after
memory entering into self-refresh mode, it shall be restored on the
exit. Note that MC_EMEM_ARB_CFG register is shadowed and latching
happens on the EMC timing update. This fixes 2x GPU performance
degradation after resuming from LP1 on Tegra30.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/iomap.h         |  9 +++++++++
 arch/arm/mach-tegra/sleep-tegra30.S | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/arch/arm/mach-tegra/iomap.h b/arch/arm/mach-tegra/iomap.h
index 9e5b2f869fc8..9bc291e76887 100644
--- a/arch/arm/mach-tegra/iomap.h
+++ b/arch/arm/mach-tegra/iomap.h
@@ -79,15 +79,24 @@
 #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
 
diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index 99ac9c6dcf7c..828f6c37afde 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -44,6 +44,8 @@
 #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 */
 
@@ -418,6 +420,22 @@ _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]
@@ -546,6 +564,7 @@ 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:
@@ -562,6 +581,7 @@ 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:
@@ -573,6 +593,7 @@ 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:
-- 
2.18.0

[PATCH v1 4/4] ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+

[Dmitry Osipenko]

Two interrupts are raised on resume from LP1 on Tegra30+: first is the
clock change completed interrupt which is set after updating timing
configuration, second is DLL alarm interrupt which is set when DLL
starts re-calibration after being reset. Clear these two interrupts
in the end of exiting from the self-refresh mode for consistency, that
will also allow to not receive spurious interrupts in the EMC driver
after resume from suspend.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/sleep-tegra30.S | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index 828f6c37afde..78c6e9fb56e7 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -26,6 +26,7 @@
 #include "irammap.h"
 #include "sleep.h"
 
+#define EMC_INTSTATUS			0x0
 #define EMC_CFG				0xc
 #define EMC_ADR_CFG			0x10
 #define EMC_TIMING_CONTROL		0x28
@@ -44,6 +45,9 @@
 #define EMC_XM2VTTGENPADCTRL		0x310
 #define EMC_XM2VTTGENPADCTRL2		0x314
 
+#define EMC_CLKCHANGE_COMPLETE_INT	(1 << 4)
+#define EMC_DLL_ALARM_INT		(1 << 7)
+
 #define MC_EMEM_ARB_CFG			0x90
 
 #define PMC_CTRL			0x0
@@ -539,6 +543,9 @@ zcal_done:
 
 	emc_timing_update r1, r0
 
+	mov	r1, #(EMC_CLKCHANGE_COMPLETE_INT | EMC_DLL_ALARM_INT)
+	str	r1, [r0, #EMC_INTSTATUS]	@ clear interrupts
+
 	/* Tegra114 had dual EMC channel, now config the other one */
 	cmp	r10, #TEGRA114
 	bne	__no_dual_emc_chanl
-- 
2.18.0

Re: [PATCH v1 0/4] EMC fixes for Tegra30+

[Dmitry Osipenko]

On 8/30/18 9:54 PM, Dmitry Osipenko wrote:
> Hello,
> 
> This patch series fixes couple bugs in the memory self-refresh code.
> The EMC / MC state is properly restored after patches being applied,
> please review.
> 
> Dmitry Osipenko (4):
>   ARM: tegra: Fix missed EMC registers latching on resume from LP1 on
>     Tegra30+
>   ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on
>     Tegra30
>   ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+
>   ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+
> 
>  arch/arm/mach-tegra/iomap.h         |  9 ++++++++
>  arch/arm/mach-tegra/sleep-tegra30.S | 32 +++++++++++++++++++++++++++--
>  2 files changed, 39 insertions(+), 2 deletions(-)
> 

ping

Re: [PATCH v1 0/4] EMC fixes for Tegra30+

[Dmitry Osipenko]

On 30.08.2018 21:54, Dmitry Osipenko wrote:
> Hello,
> 
> This patch series fixes couple bugs in the memory self-refresh code.
> The EMC / MC state is properly restored after patches being applied,
> please review.
> 
> Dmitry Osipenko (4):
>   ARM: tegra: Fix missed EMC registers latching on resume from LP1 on
>     Tegra30+
>   ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on
>     Tegra30
>   ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+
>   ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+
> 
>  arch/arm/mach-tegra/iomap.h         |  9 ++++++++
>  arch/arm/mach-tegra/sleep-tegra30.S | 32 +++++++++++++++++++++++++++--
>  2 files changed, 39 insertions(+), 2 deletions(-)
> 

Hello Jon,

Could you please test these patches using your testing setup? Will be awesome if you could give t-b/r-b to the series, then there should be a better chance that Thierry could apply it eventually ;)

Re: [PATCH v1 0/4] EMC fixes for Tegra30+

[Jon Hunter]

On 18/11/2018 22:06, Dmitry Osipenko wrote:
> On 30.08.2018 21:54, Dmitry Osipenko wrote:
>> Hello,
>>
>> This patch series fixes couple bugs in the memory self-refresh code.
>> The EMC / MC state is properly restored after patches being applied,
>> please review.
>>
>> Dmitry Osipenko (4):
>>   ARM: tegra: Fix missed EMC registers latching on resume from LP1 on
>>     Tegra30+
>>   ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on
>>     Tegra30
>>   ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+
>>   ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+
>>
>>  arch/arm/mach-tegra/iomap.h         |  9 ++++++++
>>  arch/arm/mach-tegra/sleep-tegra30.S | 32 +++++++++++++++++++++++++++--
>>  2 files changed, 39 insertions(+), 2 deletions(-)
>>
> 
> Hello Jon,
> 
> Could you please test these patches using your testing setup? Will be awesome if you could give t-b/r-b to the series, then there should be a better chance that Thierry could apply it eventually ;)

Yes I will give them a test and let you know.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 0/4] EMC fixes for Tegra30+

[Dmitry Osipenko]

On 19.11.2018 18:42, Jon Hunter wrote:
> 
> On 18/11/2018 22:06, Dmitry Osipenko wrote:
>> On 30.08.2018 21:54, Dmitry Osipenko wrote:
>>> Hello,
>>>
>>> This patch series fixes couple bugs in the memory self-refresh code.
>>> The EMC / MC state is properly restored after patches being applied,
>>> please review.
>>>
>>> Dmitry Osipenko (4):
>>>   ARM: tegra: Fix missed EMC registers latching on resume from LP1 on
>>>     Tegra30+
>>>   ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on
>>>     Tegra30
>>>   ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+
>>>   ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+
>>>
>>>  arch/arm/mach-tegra/iomap.h         |  9 ++++++++
>>>  arch/arm/mach-tegra/sleep-tegra30.S | 32 +++++++++++++++++++++++++++--
>>>  2 files changed, 39 insertions(+), 2 deletions(-)
>>>
>>
>> Hello Jon,
>>
>> Could you please test these patches using your testing setup? Will be awesome if you could give t-b/r-b to the series, then there should be a better chance that Thierry could apply it eventually ;)
> 
> Yes I will give them a test and let you know.

Thank you!

Re: [PATCH v1 0/4] EMC fixes for Tegra30+

[Jon Hunter]

On 19/11/2018 17:05, Dmitry Osipenko wrote:
> On 19.11.2018 18:42, Jon Hunter wrote:
>>
>> On 18/11/2018 22:06, Dmitry Osipenko wrote:
>>> On 30.08.2018 21:54, Dmitry Osipenko wrote:
>>>> Hello,
>>>>
>>>> This patch series fixes couple bugs in the memory self-refresh code.
>>>> The EMC / MC state is properly restored after patches being applied,
>>>> please review.
>>>>
>>>> Dmitry Osipenko (4):
>>>>   ARM: tegra: Fix missed EMC registers latching on resume from LP1 on
>>>>     Tegra30+
>>>>   ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on
>>>>     Tegra30
>>>>   ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+
>>>>   ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+
>>>>
>>>>  arch/arm/mach-tegra/iomap.h         |  9 ++++++++
>>>>  arch/arm/mach-tegra/sleep-tegra30.S | 32 +++++++++++++++++++++++++++--
>>>>  2 files changed, 39 insertions(+), 2 deletions(-)
>>>>
>>>
>>> Hello Jon,
>>>
>>> Could you please test these patches using your testing setup? Will be awesome if you could give t-b/r-b to the series, then there should be a better chance that Thierry could apply it eventually ;)
>>
>> Yes I will give them a test and let you know.
> 
> Thank you!

No problem. The tests all passed and so I don't see any issues. I have a
couple comments.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 1/4] ARM: tegra: Fix missed EMC registers latching on resume from LP1 on Tegra30+

[Jon Hunter]

On 30/08/2018 19:54, Dmitry Osipenko wrote:
> The memory interface configuration and re-calibration interval are left
> unassigned on resume from LP1 because these registers are shadowed and
> require latching after being adjusted.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/sleep-tegra30.S | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
> index 127fc78365fe..801fe58978ae 100644
> --- a/arch/arm/mach-tegra/sleep-tegra30.S
> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
> @@ -521,6 +521,8 @@ zcal_done:
>  	ldr	r1, [r5, #0x0]		@ restore EMC_CFG
>  	str	r1, [r0, #EMC_CFG]
>  
> +	emc_timing_update r1, r0
> +
>  	/* Tegra114 had dual EMC channel, now config the other one */
>  	cmp	r10, #TEGRA114
>  	bne	__no_dual_emc_chanl
> 

This is stated in the TRM as what needs to be done. So ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30

[Jon Hunter]

On 30/08/2018 19:54, Dmitry Osipenko wrote:
> The DRAM refresh-interval is getting erroneously set to "1" on exiting
> from memory self-refreshing mode. The clobbered interval causes the
> "refresh request overflow timeout" error raised by the External Memory
> Controller on exiting from LP1 on Tegra30.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/sleep-tegra30.S | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
> index 801fe58978ae..99ac9c6dcf7c 100644
> --- a/arch/arm/mach-tegra/sleep-tegra30.S
> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
> @@ -29,7 +29,6 @@
>  #define EMC_CFG				0xc
>  #define EMC_ADR_CFG			0x10
>  #define EMC_TIMING_CONTROL		0x28
> -#define EMC_REFRESH			0x70
>  #define EMC_NOP				0xdc
>  #define EMC_SELF_REF			0xe0
>  #define EMC_MRW				0xe8
> @@ -459,7 +458,6 @@ emc_wait_auto_cal_onetime:
>  	cmp	r10, #TEGRA30
>  	streq	r1, [r0, #EMC_NOP]
>  	streq	r1, [r0, #EMC_NOP]
> -	streq	r1, [r0, #EMC_REFRESH]
>  
>  	emc_device_mask r1, r0

This does look incorrect and it appears Tegra20 has the same bug.
However, looking at the EMC_REFRESH register it appears that bits 5:0
are the REFRESH_LO and bits 15:6 are the refresh interval. So this seems
to imply the interval is set to 0 and not 1. So maybe the commit message
needs to be fixed up.

The other question I have, should we be restoring the refresh value here
somewhere?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 3/4] ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+

[Jon Hunter]

On 30/08/2018 19:54, Dmitry Osipenko wrote:
> The external memory arbitration configuration is getting reset after
> memory entering into self-refresh mode, it shall be restored on the
> exit. Note that MC_EMEM_ARB_CFG register is shadowed and latching
> happens on the EMC timing update. This fixes 2x GPU performance
> degradation after resuming from LP1 on Tegra30.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/iomap.h         |  9 +++++++++
>  arch/arm/mach-tegra/sleep-tegra30.S | 21 +++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/arch/arm/mach-tegra/iomap.h b/arch/arm/mach-tegra/iomap.h
> index 9e5b2f869fc8..9bc291e76887 100644
> --- a/arch/arm/mach-tegra/iomap.h
> +++ b/arch/arm/mach-tegra/iomap.h
> @@ -79,15 +79,24 @@
>  #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
>  
> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
> index 99ac9c6dcf7c..828f6c37afde 100644
> --- a/arch/arm/mach-tegra/sleep-tegra30.S
> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
> @@ -44,6 +44,8 @@
>  #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 */
>  
> @@ -418,6 +420,22 @@ _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]
> @@ -546,6 +564,7 @@ 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:
> @@ -562,6 +581,7 @@ 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:
> @@ -573,6 +593,7 @@ 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:
> 

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>


-- 
nvpublic

Re: [PATCH v1 1/4] ARM: tegra: Fix missed EMC registers latching on resume from LP1 on Tegra30+

[Jon Hunter]

On 19/11/2018 21:27, Jon Hunter wrote:
> 
> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>> The memory interface configuration and re-calibration interval are left
>> unassigned on resume from LP1 because these registers are shadowed and
>> require latching after being adjusted.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  arch/arm/mach-tegra/sleep-tegra30.S | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>> index 127fc78365fe..801fe58978ae 100644
>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>> @@ -521,6 +521,8 @@ zcal_done:
>>  	ldr	r1, [r5, #0x0]		@ restore EMC_CFG
>>  	str	r1, [r0, #EMC_CFG]
>>  
>> +	emc_timing_update r1, r0
>> +
>>  	/* Tegra114 had dual EMC channel, now config the other one */
>>  	cmp	r10, #TEGRA114
>>  	bne	__no_dual_emc_chanl
>>
> 
> This is stated in the TRM as what needs to be done. So ...
> 
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Also ...

Tested-by: Jon Hunter <jonathanh@nvidia.com>

Jon

-- 
nvpublic

Re: [PATCH v1 4/4] ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+

[Jon Hunter]

On 30/08/2018 19:54, Dmitry Osipenko wrote:
> Two interrupts are raised on resume from LP1 on Tegra30+: first is the
> clock change completed interrupt which is set after updating timing
> configuration, second is DLL alarm interrupt which is set when DLL
> starts re-calibration after being reset. Clear these two interrupts
> in the end of exiting from the self-refresh mode for consistency, that
> will also allow to not receive spurious interrupts in the EMC driver
> after resume from suspend.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/sleep-tegra30.S | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
> index 828f6c37afde..78c6e9fb56e7 100644
> --- a/arch/arm/mach-tegra/sleep-tegra30.S
> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
> @@ -26,6 +26,7 @@
>  #include "irammap.h"
>  #include "sleep.h"
>  
> +#define EMC_INTSTATUS			0x0
>  #define EMC_CFG				0xc
>  #define EMC_ADR_CFG			0x10
>  #define EMC_TIMING_CONTROL		0x28
> @@ -44,6 +45,9 @@
>  #define EMC_XM2VTTGENPADCTRL		0x310
>  #define EMC_XM2VTTGENPADCTRL2		0x314
>  
> +#define EMC_CLKCHANGE_COMPLETE_INT	(1 << 4)
> +#define EMC_DLL_ALARM_INT		(1 << 7)
> +
>  #define MC_EMEM_ARB_CFG			0x90
>  
>  #define PMC_CTRL			0x0
> @@ -539,6 +543,9 @@ zcal_done:
>  
>  	emc_timing_update r1, r0
>  
> +	mov	r1, #(EMC_CLKCHANGE_COMPLETE_INT | EMC_DLL_ALARM_INT)
> +	str	r1, [r0, #EMC_INTSTATUS]	@ clear interrupts
> +
>  	/* Tegra114 had dual EMC channel, now config the other one */
>  	cmp	r10, #TEGRA114
>  	bne	__no_dual_emc_chanl
> 

Where are these interrupts enabled? I did not see where they are
enabled. I see that the Tegra24 EMC driver does poll these, but it did
not look like they were enabled. If they are enabled, I wondering if
they should be masked on entering self-refresh?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30

[Dmitry Osipenko]

On 20.11.2018 0:34, Jon Hunter wrote:
> 
> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>> The DRAM refresh-interval is getting erroneously set to "1" on exiting
>> from memory self-refreshing mode. The clobbered interval causes the
>> "refresh request overflow timeout" error raised by the External Memory
>> Controller on exiting from LP1 on Tegra30.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  arch/arm/mach-tegra/sleep-tegra30.S | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>> index 801fe58978ae..99ac9c6dcf7c 100644
>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>> @@ -29,7 +29,6 @@
>>  #define EMC_CFG				0xc
>>  #define EMC_ADR_CFG			0x10
>>  #define EMC_TIMING_CONTROL		0x28
>> -#define EMC_REFRESH			0x70
>>  #define EMC_NOP				0xdc
>>  #define EMC_SELF_REF			0xe0
>>  #define EMC_MRW				0xe8
>> @@ -459,7 +458,6 @@ emc_wait_auto_cal_onetime:
>>  	cmp	r10, #TEGRA30
>>  	streq	r1, [r0, #EMC_NOP]
>>  	streq	r1, [r0, #EMC_NOP]
>> -	streq	r1, [r0, #EMC_REFRESH]
>>  
>>  	emc_device_mask r1, r0
> 
> This does look incorrect and it appears Tegra20 has the same bug.

Indeed.. somehow this doesn't cause any problems on T20. Maybe this affects only specific timing configurations and it's just a luck that "refresh overflow" isn't getting raised.

> However, looking at the EMC_REFRESH register it appears that bits 5:0
> are the REFRESH_LO and bits 15:6 are the refresh interval. So this seems
> to imply the interval is set to 0 and not 1. So maybe the commit message
> needs to be fixed up.

Do you mean that EMC_REFRESH is a fractional value?

> The other question I have, should we be restoring the refresh value here
> somewhere?

The EMC_REFRESH value isn't altered on enter/exit self-refresh, at least on Tegra30.. very likely it should be the same for other gens.

Re: [PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30

[Dmitry Osipenko]

On 20.11.2018 1:09, Dmitry Osipenko wrote:
> On 20.11.2018 0:34, Jon Hunter wrote:
>>
>> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>>> The DRAM refresh-interval is getting erroneously set to "1" on exiting
>>> from memory self-refreshing mode. The clobbered interval causes the
>>> "refresh request overflow timeout" error raised by the External Memory
>>> Controller on exiting from LP1 on Tegra30.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  arch/arm/mach-tegra/sleep-tegra30.S | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>>> index 801fe58978ae..99ac9c6dcf7c 100644
>>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>>> @@ -29,7 +29,6 @@
>>>  #define EMC_CFG				0xc
>>>  #define EMC_ADR_CFG			0x10
>>>  #define EMC_TIMING_CONTROL		0x28
>>> -#define EMC_REFRESH			0x70
>>>  #define EMC_NOP				0xdc
>>>  #define EMC_SELF_REF			0xe0
>>>  #define EMC_MRW				0xe8
>>> @@ -459,7 +458,6 @@ emc_wait_auto_cal_onetime:
>>>  	cmp	r10, #TEGRA30
>>>  	streq	r1, [r0, #EMC_NOP]
>>>  	streq	r1, [r0, #EMC_NOP]
>>> -	streq	r1, [r0, #EMC_REFRESH]
>>>  
>>>  	emc_device_mask r1, r0
>>
>> This does look incorrect and it appears Tegra20 has the same bug.
> 
> Indeed.. somehow this doesn't cause any problems on T20. Maybe this affects only specific timing configurations and it's just a luck that "refresh overflow" isn't getting raised.

Ah, T20 exit_selfrefresh_loop doesn't latch registers.. that's probably why it stayed unnoticed.

Re: [PATCH v1 4/4] ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+

[Dmitry Osipenko]

On 20.11.2018 1:00, Jon Hunter wrote:
> 
> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>> Two interrupts are raised on resume from LP1 on Tegra30+: first is the
>> clock change completed interrupt which is set after updating timing
>> configuration, second is DLL alarm interrupt which is set when DLL
>> starts re-calibration after being reset. Clear these two interrupts
>> in the end of exiting from the self-refresh mode for consistency, that
>> will also allow to not receive spurious interrupts in the EMC driver
>> after resume from suspend.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  arch/arm/mach-tegra/sleep-tegra30.S | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>> index 828f6c37afde..78c6e9fb56e7 100644
>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>> @@ -26,6 +26,7 @@
>>  #include "irammap.h"
>>  #include "sleep.h"
>>  
>> +#define EMC_INTSTATUS			0x0
>>  #define EMC_CFG				0xc
>>  #define EMC_ADR_CFG			0x10
>>  #define EMC_TIMING_CONTROL		0x28
>> @@ -44,6 +45,9 @@
>>  #define EMC_XM2VTTGENPADCTRL		0x310
>>  #define EMC_XM2VTTGENPADCTRL2		0x314
>>  
>> +#define EMC_CLKCHANGE_COMPLETE_INT	(1 << 4)
>> +#define EMC_DLL_ALARM_INT		(1 << 7)
>> +
>>  #define MC_EMEM_ARB_CFG			0x90
>>  
>>  #define PMC_CTRL			0x0
>> @@ -539,6 +543,9 @@ zcal_done:
>>  
>>  	emc_timing_update r1, r0
>>  
>> +	mov	r1, #(EMC_CLKCHANGE_COMPLETE_INT | EMC_DLL_ALARM_INT)
>> +	str	r1, [r0, #EMC_INTSTATUS]	@ clear interrupts
>> +
>>  	/* Tegra114 had dual EMC channel, now config the other one */
>>  	cmp	r10, #TEGRA114
>>  	bne	__no_dual_emc_chanl
>>
> 
> Where are these interrupts enabled? I did not see where they are
> enabled. I see that the Tegra24 EMC driver does poll these, but it did
> not look like they were enabled. If they are enabled, I wondering if
> they should be masked on entering self-refresh?

EMC interrupt is not enabled on Tegra124. IIRC, it doesn't use the interrupt at all in the driver. Indeed, T124 EMC driver polls the interrupt status, but it clears the status before starting to poll. Probably I was just thinking about to write T30 EMC driver at that time and to utilize the interrupt.. 

Seems it should be okay to drop this patch for now, but maybe then we will have to return to it sometime later. Up to you to decide.

Re: [PATCH v1 0/4] EMC fixes for Tegra30+

[Dmitry Osipenko]

On 20.11.2018 0:26, Jon Hunter wrote:
> 
> On 19/11/2018 17:05, Dmitry Osipenko wrote:
>> On 19.11.2018 18:42, Jon Hunter wrote:
>>>
>>> On 18/11/2018 22:06, Dmitry Osipenko wrote:
>>>> On 30.08.2018 21:54, Dmitry Osipenko wrote:
>>>>> Hello,
>>>>>
>>>>> This patch series fixes couple bugs in the memory self-refresh code.
>>>>> The EMC / MC state is properly restored after patches being applied,
>>>>> please review.
>>>>>
>>>>> Dmitry Osipenko (4):
>>>>>   ARM: tegra: Fix missed EMC registers latching on resume from LP1 on
>>>>>     Tegra30+
>>>>>   ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on
>>>>>     Tegra30
>>>>>   ARM: tegra: Restore memory arbitration on resume from LP1 on Tegra30+
>>>>>   ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+
>>>>>
>>>>>  arch/arm/mach-tegra/iomap.h         |  9 ++++++++
>>>>>  arch/arm/mach-tegra/sleep-tegra30.S | 32 +++++++++++++++++++++++++++--
>>>>>  2 files changed, 39 insertions(+), 2 deletions(-)
>>>>>
>>>>
>>>> Hello Jon,
>>>>
>>>> Could you please test these patches using your testing setup? Will be awesome if you could give t-b/r-b to the series, then there should be a better chance that Thierry could apply it eventually ;)
>>>
>>> Yes I will give them a test and let you know.
>>
>> Thank you!
> 
> No problem. The tests all passed and so I don't see any issues. I have a
> couple comments.

Very nice, thank you very much!

Re: [PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30

[Jon Hunter]

On 19/11/2018 22:09, Dmitry Osipenko wrote:
> On 20.11.2018 0:34, Jon Hunter wrote:
>>
>> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>>> The DRAM refresh-interval is getting erroneously set to "1" on exiting
>>> from memory self-refreshing mode. The clobbered interval causes the
>>> "refresh request overflow timeout" error raised by the External Memory
>>> Controller on exiting from LP1 on Tegra30.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  arch/arm/mach-tegra/sleep-tegra30.S | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>>> index 801fe58978ae..99ac9c6dcf7c 100644
>>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>>> @@ -29,7 +29,6 @@
>>>  #define EMC_CFG				0xc
>>>  #define EMC_ADR_CFG			0x10
>>>  #define EMC_TIMING_CONTROL		0x28
>>> -#define EMC_REFRESH			0x70
>>>  #define EMC_NOP				0xdc
>>>  #define EMC_SELF_REF			0xe0
>>>  #define EMC_MRW				0xe8
>>> @@ -459,7 +458,6 @@ emc_wait_auto_cal_onetime:
>>>  	cmp	r10, #TEGRA30
>>>  	streq	r1, [r0, #EMC_NOP]
>>>  	streq	r1, [r0, #EMC_NOP]
>>> -	streq	r1, [r0, #EMC_REFRESH]
>>>  
>>>  	emc_device_mask r1, r0
>>
>> This does look incorrect and it appears Tegra20 has the same bug.
> 
> Indeed.. somehow this doesn't cause any problems on T20. Maybe this affects only specific timing configurations and it's just a luck that "refresh overflow" isn't getting raised.
> 
>> However, looking at the EMC_REFRESH register it appears that bits 5:0
>> are the REFRESH_LO and bits 15:6 are the refresh interval. So this seems
>> to imply the interval is set to 0 and not 1. So maybe the commit message
>> needs to be fixed up.
> 
> Do you mean that EMC_REFRESH is a fractional value?

No the more I look at this, I just think it is a badly describe register
in the TRM. I think that your description is correct afterall.

Cheers
Jon


-- 
nvpublic

Re: [PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30

[Jon Hunter]

On 19/11/2018 22:32, Dmitry Osipenko wrote:
> On 20.11.2018 1:09, Dmitry Osipenko wrote:
>> On 20.11.2018 0:34, Jon Hunter wrote:
>>>
>>> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>>>> The DRAM refresh-interval is getting erroneously set to "1" on exiting
>>>> from memory self-refreshing mode. The clobbered interval causes the
>>>> "refresh request overflow timeout" error raised by the External Memory
>>>> Controller on exiting from LP1 on Tegra30.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  arch/arm/mach-tegra/sleep-tegra30.S | 2 --
>>>>  1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>>>> index 801fe58978ae..99ac9c6dcf7c 100644
>>>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>>>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>>>> @@ -29,7 +29,6 @@
>>>>  #define EMC_CFG				0xc
>>>>  #define EMC_ADR_CFG			0x10
>>>>  #define EMC_TIMING_CONTROL		0x28
>>>> -#define EMC_REFRESH			0x70
>>>>  #define EMC_NOP				0xdc
>>>>  #define EMC_SELF_REF			0xe0
>>>>  #define EMC_MRW				0xe8
>>>> @@ -459,7 +458,6 @@ emc_wait_auto_cal_onetime:
>>>>  	cmp	r10, #TEGRA30
>>>>  	streq	r1, [r0, #EMC_NOP]
>>>>  	streq	r1, [r0, #EMC_NOP]
>>>> -	streq	r1, [r0, #EMC_REFRESH]
>>>>  
>>>>  	emc_device_mask r1, r0
>>>
>>> This does look incorrect and it appears Tegra20 has the same bug.
>>
>> Indeed.. somehow this doesn't cause any problems on T20. Maybe this affects only specific timing configurations and it's just a luck that "refresh overflow" isn't getting raised.
> 
> Ah, T20 exit_selfrefresh_loop doesn't latch registers.. that's probably why it stayed unnoticed.

Good to know.

Cheers Jon

-- 
nvpublic

Re: [PATCH v1 4/4] ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+

[Jon Hunter]

On 19/11/2018 22:35, Dmitry Osipenko wrote:
> On 20.11.2018 1:00, Jon Hunter wrote:
>>
>> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>>> Two interrupts are raised on resume from LP1 on Tegra30+: first is the
>>> clock change completed interrupt which is set after updating timing
>>> configuration, second is DLL alarm interrupt which is set when DLL
>>> starts re-calibration after being reset. Clear these two interrupts
>>> in the end of exiting from the self-refresh mode for consistency, that
>>> will also allow to not receive spurious interrupts in the EMC driver
>>> after resume from suspend.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  arch/arm/mach-tegra/sleep-tegra30.S | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>>> index 828f6c37afde..78c6e9fb56e7 100644
>>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>>> @@ -26,6 +26,7 @@
>>>  #include "irammap.h"
>>>  #include "sleep.h"
>>>  
>>> +#define EMC_INTSTATUS			0x0
>>>  #define EMC_CFG				0xc
>>>  #define EMC_ADR_CFG			0x10
>>>  #define EMC_TIMING_CONTROL		0x28
>>> @@ -44,6 +45,9 @@
>>>  #define EMC_XM2VTTGENPADCTRL		0x310
>>>  #define EMC_XM2VTTGENPADCTRL2		0x314
>>>  
>>> +#define EMC_CLKCHANGE_COMPLETE_INT	(1 << 4)
>>> +#define EMC_DLL_ALARM_INT		(1 << 7)
>>> +
>>>  #define MC_EMEM_ARB_CFG			0x90
>>>  
>>>  #define PMC_CTRL			0x0
>>> @@ -539,6 +543,9 @@ zcal_done:
>>>  
>>>  	emc_timing_update r1, r0
>>>  
>>> +	mov	r1, #(EMC_CLKCHANGE_COMPLETE_INT | EMC_DLL_ALARM_INT)
>>> +	str	r1, [r0, #EMC_INTSTATUS]	@ clear interrupts
>>> +
>>>  	/* Tegra114 had dual EMC channel, now config the other one */
>>>  	cmp	r10, #TEGRA114
>>>  	bne	__no_dual_emc_chanl
>>>
>>
>> Where are these interrupts enabled? I did not see where they are
>> enabled. I see that the Tegra24 EMC driver does poll these, but it did
>> not look like they were enabled. If they are enabled, I wondering if
>> they should be masked on entering self-refresh?
> 
> EMC interrupt is not enabled on Tegra124. IIRC, it doesn't use the interrupt at all in the driver. Indeed, T124 EMC driver polls the interrupt status, but it clears the status before starting to poll. Probably I was just thinking about to write T30 EMC driver at that time and to utilize the interrupt.. 
> 
> Seems it should be okay to drop this patch for now, but maybe then we will have to return to it sometime later. Up to you to decide.

I would be tempted to drop for now. If you have such a driver maybe the
driver should mask any interrupts it uses on suspending.

Cheers
Jon
-- 
nvpublic

Re: [PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30

[Jon Hunter]

On 30/08/2018 19:54, Dmitry Osipenko wrote:
> The DRAM refresh-interval is getting erroneously set to "1" on exiting
> from memory self-refreshing mode. The clobbered interval causes the
> "refresh request overflow timeout" error raised by the External Memory
> Controller on exiting from LP1 on Tegra30.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/sleep-tegra30.S | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
> index 801fe58978ae..99ac9c6dcf7c 100644
> --- a/arch/arm/mach-tegra/sleep-tegra30.S
> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
> @@ -29,7 +29,6 @@
>  #define EMC_CFG				0xc
>  #define EMC_ADR_CFG			0x10
>  #define EMC_TIMING_CONTROL		0x28
> -#define EMC_REFRESH			0x70
>  #define EMC_NOP				0xdc
>  #define EMC_SELF_REF			0xe0
>  #define EMC_MRW				0xe8
> @@ -459,7 +458,6 @@ emc_wait_auto_cal_onetime:
>  	cmp	r10, #TEGRA30
>  	streq	r1, [r0, #EMC_NOP]
>  	streq	r1, [r0, #EMC_NOP]
> -	streq	r1, [r0, #EMC_REFRESH]
>  
>  	emc_device_mask r1, r0

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30

[Dmitry Osipenko]

On 20.11.2018 13:26, Jon Hunter wrote:
> 
> On 19/11/2018 22:32, Dmitry Osipenko wrote:
>> On 20.11.2018 1:09, Dmitry Osipenko wrote:
>>> On 20.11.2018 0:34, Jon Hunter wrote:
>>>>
>>>> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>>>>> The DRAM refresh-interval is getting erroneously set to "1" on exiting
>>>>> from memory self-refreshing mode. The clobbered interval causes the
>>>>> "refresh request overflow timeout" error raised by the External Memory
>>>>> Controller on exiting from LP1 on Tegra30.
>>>>>
>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>> ---
>>>>>  arch/arm/mach-tegra/sleep-tegra30.S | 2 --
>>>>>  1 file changed, 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>>>>> index 801fe58978ae..99ac9c6dcf7c 100644
>>>>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>>>>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>>>>> @@ -29,7 +29,6 @@
>>>>>  #define EMC_CFG				0xc
>>>>>  #define EMC_ADR_CFG			0x10
>>>>>  #define EMC_TIMING_CONTROL		0x28
>>>>> -#define EMC_REFRESH			0x70
>>>>>  #define EMC_NOP				0xdc
>>>>>  #define EMC_SELF_REF			0xe0
>>>>>  #define EMC_MRW				0xe8
>>>>> @@ -459,7 +458,6 @@ emc_wait_auto_cal_onetime:
>>>>>  	cmp	r10, #TEGRA30
>>>>>  	streq	r1, [r0, #EMC_NOP]
>>>>>  	streq	r1, [r0, #EMC_NOP]
>>>>> -	streq	r1, [r0, #EMC_REFRESH]
>>>>>  
>>>>>  	emc_device_mask r1, r0
>>>>
>>>> This does look incorrect and it appears Tegra20 has the same bug.
>>>
>>> Indeed.. somehow this doesn't cause any problems on T20. Maybe this affects only specific timing configurations and it's just a luck that "refresh overflow" isn't getting raised.
>>
>> Ah, T20 exit_selfrefresh_loop doesn't latch registers.. that's probably why it stayed unnoticed.
> 
> Good to know.

Thank you for pointing at it, will be fixed in v2.

Re: [PATCH v1 4/4] ARM: tegra: Clear EMC interrupts on resume from LP1 on Tegra30+

[Dmitry Osipenko]

On 20.11.2018 13:27, Jon Hunter wrote:
> 
> On 19/11/2018 22:35, Dmitry Osipenko wrote:
>> On 20.11.2018 1:00, Jon Hunter wrote:
>>>
>>> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>>>> Two interrupts are raised on resume from LP1 on Tegra30+: first is the
>>>> clock change completed interrupt which is set after updating timing
>>>> configuration, second is DLL alarm interrupt which is set when DLL
>>>> starts re-calibration after being reset. Clear these two interrupts
>>>> in the end of exiting from the self-refresh mode for consistency, that
>>>> will also allow to not receive spurious interrupts in the EMC driver
>>>> after resume from suspend.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  arch/arm/mach-tegra/sleep-tegra30.S | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>>>> index 828f6c37afde..78c6e9fb56e7 100644
>>>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>>>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>>>> @@ -26,6 +26,7 @@
>>>>  #include "irammap.h"
>>>>  #include "sleep.h"
>>>>  
>>>> +#define EMC_INTSTATUS			0x0
>>>>  #define EMC_CFG				0xc
>>>>  #define EMC_ADR_CFG			0x10
>>>>  #define EMC_TIMING_CONTROL		0x28
>>>> @@ -44,6 +45,9 @@
>>>>  #define EMC_XM2VTTGENPADCTRL		0x310
>>>>  #define EMC_XM2VTTGENPADCTRL2		0x314
>>>>  
>>>> +#define EMC_CLKCHANGE_COMPLETE_INT	(1 << 4)
>>>> +#define EMC_DLL_ALARM_INT		(1 << 7)
>>>> +
>>>>  #define MC_EMEM_ARB_CFG			0x90
>>>>  
>>>>  #define PMC_CTRL			0x0
>>>> @@ -539,6 +543,9 @@ zcal_done:
>>>>  
>>>>  	emc_timing_update r1, r0
>>>>  
>>>> +	mov	r1, #(EMC_CLKCHANGE_COMPLETE_INT | EMC_DLL_ALARM_INT)
>>>> +	str	r1, [r0, #EMC_INTSTATUS]	@ clear interrupts
>>>> +
>>>>  	/* Tegra114 had dual EMC channel, now config the other one */
>>>>  	cmp	r10, #TEGRA114
>>>>  	bne	__no_dual_emc_chanl
>>>>
>>>
>>> Where are these interrupts enabled? I did not see where they are
>>> enabled. I see that the Tegra24 EMC driver does poll these, but it did
>>> not look like they were enabled. If they are enabled, I wondering if
>>> they should be masked on entering self-refresh?
>>
>> EMC interrupt is not enabled on Tegra124. IIRC, it doesn't use the interrupt at all in the driver. Indeed, T124 EMC driver polls the interrupt status, but it clears the status before starting to poll. Probably I was just thinking about to write T30 EMC driver at that time and to utilize the interrupt.. 
>>
>> Seems it should be okay to drop this patch for now, but maybe then we will have to return to it sometime later. Up to you to decide.
> 
> I would be tempted to drop for now. If you have such a driver maybe the
> driver should mask any interrupts it uses on suspending.

I don't have such driver yet. Masking interrupts on suspend and clearing status on resume within the driver should result in possibility to miss some trouble that happened between "CPU wake"->"EMC driver resuming", but probably that's not too important and unlikely to happen.