Re: [PATCH] suniv: fix dramc autofresh freq calculation issue

[Andre Przywara <andre.przywara@arm.com>]

On Mon, 20 Mar 2023 08:59:59 +0800
John Sanpe <sanpeqf@gmail.com> wrote:

Hi John,

thanks for sending this, and sorry for the delay.

> External use mhz to express frequency, autofresh use hz,
> no unit conversion is performed when calling, cause dram
> instability at low frequency.

What low frequency? Are you running it with less than 156 MHz?

Anyway I had a deeper look at this DRAM driver a while ago, and made
some significant cleanups. One of them affects
dram_set_autofresh_cycle(), which is ... weird ... to say at least. Not
only does it support specifying the frequency both in MHz or Hz, it also
uses some quite obfuscated arithmetic to avoid divisions.
This is what I made of this function:
https://github.com/apritzel/u-boot/commit/729d3c61b62

This is part of the whole cleanup series, which is not quite ready yet,
but I put a WIP branch up anyway:
https://github.com/apritzel/u-boot/commits/suniv-dram-WIP

As a bonus this allows to run the DRAM at 204 MHz, with JEDEC timings,
which was not only quite stable for me (ran memtester over the
weekend), but also provided quite some performance advantage.

It would be nice if you could give this series a go, and see if it
works for you. Feel free to play around with the frequency, it should
now program optimised timings for each rate.

Regarding your patch: I am afraid this is somewhat obsoleted by my
series, also it seems to just paper over the actual problems, which
probably are in the crude arithmetic.

Cheers,
Andre

> Incorporated xboot repair patch for this driver.
> 
> Signed-off-by: John Sanpe <sanpeqf@gmail.com>
> ---
>  arch/arm/mach-sunxi/dram_suniv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-sunxi/dram_suniv.c b/arch/arm/mach-sunxi/dram_suniv.c
> index 3aa3ce7627..830fa7895d 100644
> --- a/arch/arm/mach-sunxi/dram_suniv.c
> +++ b/arch/arm/mach-sunxi/dram_suniv.c
> @@ -310,7 +310,7 @@ static u32 dram_get_dram_size(struct dram_para *para)
>  		para->size = 64;
>  	else
>  		para->size = 32;
> -	dram_set_autofresh_cycle(para->clk);
> +	dram_set_autofresh_cycle(para->clk * 1000000);
>  	para->access_mode = 0;
>  	dram_para_setup(para);
>