On Sat, 3 Aug 2024 16:17:26 +0300 Mikhail Kalashnikov iuncuim@gmail.com wrote:
Hi Mikhail,
On 02.08.2024 01:55, Chris Morgan wrote:
From: Jernej Skrabec jernej.skrabec@gmail.com
Adjust H616 LPDDR3 DRAM settings to be in line with vendor driver.
Signed-off-by: Jernej Skrabec jernej.skrabec@gmail.com Tested-by: Chris Morgan macromorgan@hotmail.com
arch/arm/mach-sunxi/dram_sun50i_h616.c | 2 +- arch/arm/mach-sunxi/dram_timings/h616_lpddr3.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c index 37c139e0ee..a20264d8b4 100644 --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c @@ -945,7 +945,7 @@ static bool mctl_phy_init(const struct dram_para *para, val = para->tpr6 & 0xff; break; case SUNXI_DRAM_TYPE_LPDDR3:
val = para->tpr6 >> 8 & 0xff;
val = para->tpr6 >> 16 & 0xff;This is the correct change to match the factory tpr6 parameters.
I think, we also need to change the default value in arch/arm/mach-sunxi/Kconfig:
from: config DRAM_SUN50I_H616_TPR6 hex "H616 DRAM TPR6 parameter" default 0x3300c080 to default 0x33c00080
Can you please confirm what the vendor code does here? Does it use a value of 0xc0 for "val" above, so we need to change *both* the shift and the default value? Or does the vendor code write 0 (the original bits[23:16]) into val? I am a bit puzzled because I think we copied the TPR values verbatim from the vendor firmware, so changing DRAM_SUN50I_H616_TPR6 looks a bit odd.
break;case SUNXI_DRAM_TYPE_LPDDR4: val = para->tpr6 >> 24 & 0xff; diff --git a/arch/arm/mach-sunxi/dram_timings/h616_lpddr3.c b/arch/arm/mach-sunxi/dram_timings/h616_lpddr3.c index ce2ffa7a02..82b86084a6 100644 --- a/arch/arm/mach-sunxi/dram_timings/h616_lpddr3.c +++ b/arch/arm/mach-sunxi/dram_timings/h616_lpddr3.c @@ -24,8 +24,8 @@ void mctl_set_timing_params(const struct dram_para *para) u8 trrd = max(ns_to_t(6), 4); u8 trcd = ns_to_t(24); u8 trc = ns_to_t(70);
- u8 txp = max(ns_to_t(8), 3); u8 trtp = max(ns_to_t(8), 2);
- u8 txp = trtp;
I think Jernejchanged this value using RE. I checked the 047fb104 register (dramtmg[1])
on my t98-h2b-lp3 tvbox, it has not changed and is the same as the factory value.
So my educated guess (if that change originates from REing of binary driver code): - tXP and tRTP do not seem related, judging by JEDEC documentation. If the txp value is assigned the value of trtp, that's probably some compiler optimisation, because both variables were assigned to the same expression, in the original vendor code. - The LPDDR3 spec describes tXP as "max(7.5ns,3nCK)", that would be the original setting. Down to 500 MHz DRAM clock ns_to_t(8) results in a value of 3 or higher, so this change would only be relevant for DRAM clock speeds below 500 MHz. I doubt we have a board with LPDDR3 DRAM that slow?
So I am happy to change that code, if that's closer to what the vendor driver does, but I doubt that this will impact any actual hardware. Also I would like to change the assignment to copy the *formula* that trtp is using, instead of copying the value of it. So this effectively just lowers the minimum clock cycles to 2 (instead of 3).
Does that make sense?
Cheers, Andre
=> md.l 0x47fb100 047fb100: 10141811 0004041c 04070d0d 0050500c .............PP.