Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jernej =?utf-8?B?xaBrcmFiZWM=?= <jernej.skrabec@gmail.com>
To:     samuel@sholland.org, Roman Stratiienko <r.stratiienko@gmail.com>
Cc:     mturquette@baylibre.com, sboyd@kernel.org, mripard@kernel.org,
        wens@csie.org, linux-clk@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev,
        linux-kernel@vger.kernel.org,
        Roman Stratiienko <r.stratiienko@gmail.com>
Subject: Re: [RFC] ccu-sun50i-h6: Bump-up DDR0 PLL to 1800MHz
Date:   Sun, 10 Jul 2022 20:47:55 +0200
Message-ID: <1917526.jZfb76A358@kista>
In-Reply-To: <20220710175214.544748-1-r.stratiienko@gmail.com>
References: <20220710175214.544748-1-r.stratiienko@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

Hi Roman,

Dne nedelja, 10. julij 2022 ob 19:52:14 CEST je Roman Stratiienko napisal(a):
> While debugging complex animated composition cases  I noticed a glitch
> which as it turned-out eventually was caused by lack of memory bandwidth.
> 
> I can't find a DRAMC manual to check what input frequency must be
> supplied to avoid exceeding the 800MHz LPDDR3 which is installed on
> my orangepi3 board. But the system is running stable so far.

this is hard topic. Generally, max. supported DRAM frequency is board specific 
and is set by SPL (part of U-Boot), so we can't assume every board supports 
max. frequency for its memory type. In fact, this assumption was proven wrong. 
Some boards were unstable until DRAM frequency was lowered. So in short, Linux 
clock driver certainly shouldn't set DRAM frequency. There is, however, Linux 
MBUS driver for H5 and A64, which can throttle DRAM frequency when system is 
idle, but max. frequency is still that one which was programmed by SPL.

There is no definitive DRAM documentation, because Allwinner usually tries to 
hide all details. DRAM drivers are, except in rare cases, all reverse 
engineered by community. Register values (mostly) match those produced by 
vendor driver. Fortunately, at least H6 DRAM controller matches pretty good 
with one other memory controller (one in Zynq UltraScale+ MPSoC), which is 
fully documented (see comments in U-Boot driver). MBUS is also pretty standard 
and it's not a mystery.

Note that it's hard to convince maintainers to either lower or increase DRAM 
frequency as it was common issue in the past. You have to do a lot of memory 
intensive tests, both CPU and GPU, running at the same time for extended 
period of time. Initial board bring ups usually just use same frequency as in 
BSP based images. If I understand procedure correctly, boards are sent to 
Allwinner, so their engineers determine max. DRAM frequency and timings, so 
these values are pretty trustwothy.

There is other possibility. Along with DRAM controller and PHY, there is MBUS 
core, which is memory arbiter and responsible for bandwidth allocation. You 
can change DE priority and bandwith there (again, in U-Boot DRAM driver).

I hope this explains why things are the way they are.

Best regards,
Jernej

> 
> Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>
> ---
>  drivers/clk/sunxi-ng/ccu-sun50i-h6.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c index 750368a86b8b6..abdde80307993
> 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> @@ -1203,6 +1203,17 @@ static int sun50i_h6_ccu_probe(struct platform_device
> *pdev) val &= ~GENMASK(3, 0);
>  	writel(val, reg + gpu_clk.common.reg);
> 
> +	/*
> +	 * Increase DDR0 PLL from 1488(default) to 1800MHz.
> +	 * (DE3.0 require higher memory bandwidth while displaying
> +	 * complex composition at 1920x1080@60FPS)
> +	 */
> +
> +	val = readl(reg + SUN50I_H6_PLL_DDR0_REG);
> +	val &= ~GENMASK(15, 0);
> +	val |= 74 << 8;
> +	writel(val, reg + SUN50I_H6_PLL_DDR0_REG);
> +
>  	/* Enable the lock bits on all PLLs */
>  	for (i = 0; i < ARRAY_SIZE(pll_regs); i++) {
>  		val = readl(reg + pll_regs[i]);
> --
> 2.34.1