On 2019-10-16 07:54, Leonard Crestez wrote:
On 16.10.2019 17:09, Angus Ainslie wrote:
On 2019-10-15 07:05, Leonard Crestez wrote:
On 10.10.2019 17:43, Angus Ainslie wrote:
I've integrated your u-boot and ATF on our board and I have a couple
of questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
It looks like this line limits the training frequencies to 800 MHz
and
166 MHz
Yes! This is due to a hardware errata which was solved in B1: DRAM
pll
can't be disabled. This means that instead of 25/100/800 freqs are
166/800, and this requires code changes.
Does 100 MHz and 25 MHz not work on B0 ?
No, lower rates require dram clk from a composite slice
(dram_alt_root)
I added the ddrc_and noc opp as well as the 166MHz opp
I also added the interconnects ( do we need them on imx8mq ? )
The interconnect stuff is not required to switch dram frequency, it's
for device to make minimum bandwidth requests. It an additional
feature
on top.
As a hack I configured FEC to do so but a saner example would be to
request bandwidth based on display resolution and color depth.
I had to add a hack as the PM QoS was limiting the bus speed to
399MHz,
if you have any ideas why that would be appreciated.
You probably need to set ethernet down (which is awkward) or better
just drop the interconnect properties and test using the devfreq
userspace
governor.
The driver is probing
[ 12.136537] bus: 'platform': driver_probe_device: matched device
3d400000.dram-controller with driver imx-ddrc-devfrq
[ 12.147259] bus: 'platform': really_probe: probing driver
imx-ddrc-devfreq with device 3d400000.dram-controller
[ 12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
handle
[ 12.164197] arm_smcc rate 0 800000000
[ 12.167880] arm_smcc rate 1 166750000
[ 12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
uvmin:0 uvmax:0 latency:0
[ 12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
uvmin:0 uvmax:0 latency:0
[ 12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
uvmin:0 uvmax:0 latency:0
[ 12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
uvmin:0 uvmax:0 latency:0
[ 12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events
from
pmu imx8_ddr0
[ 12.212403] Added freq 0 25000000
[ 12.215742] Added freq 1 100000000
[ 12.219177] Added freq 2 166750000
[ 12.222648] Added freq 3 800000000
[ 12.226105] device: 'devfreq0': device_add
[ 12.230287] PM: Adding info for No Bus:devfreq0
[ 12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to
device
'3d400000.dram-controller'
[ 12.243699] bus: 'platform': really_probe: bound device
3d400000.dram-controller to driver imx-ddrc-devfreq
Add seems to run correctly until it tries to adjust the clock to
166MHz
[ 19.555482] ddrc checking rate 800000000 166750000
[ 19.555489] ddrc checking rate 166750000 166750000
[ 19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0
to
driver option1
[ 19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about
to
change freq 800000000 to 166750000
And the board hangs there. Any ideas on how to get past this ?
Please try this ATF patch:
Ok applied this to the tree we're using
I tested switching on imx8mq-evk with B0 SoC but a few additional
changes are required in kernel to support switching between rates
which
are both backed by PLL:
* Mark the PLL CLK_GET_RATE_NOCACHE
Is this what you meant ?
diff --git a/drivers/clk/imx/clk-imx8mq.c
b/drivers/clk/imx/clk-imx8mq.c
index 2813884f69c1..e5f50cf8a264 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -345,7 +345,7 @@ static int imx8mq_clocks_probe(struct
platform_device *pdev)
clks[IMX8MQ_SYS1_PLL_OUT] = imx_clk_sccg_pll("sys1_pll_out",
sys1_pll_out_sels, ARRAY_SIZE(sys1_pll_out_sels), 0, 0, 0, base +
0x30,
CLK_IS_CRITICAL);
clks[IMX8MQ_SYS2_PLL_OUT] = imx_clk_sccg_pll("sys2_pll_out",
sys2_pll_out_sels, ARRAY_SIZE(sys2_pll_out_sels), 0, 0, 1, base +
0x3c,
CLK_IS_CRITICAL);
clks[IMX8MQ_SYS3_PLL_OUT] = imx_clk_sccg_pll("sys3_pll_out",
sys3_pll_out_sels, ARRAY_SIZE(sys3_pll_out_sels), 0, 0, 1, base +
0x48,
CLK_IS_CRITICAL);
- clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base +
0x60,
CLK_IS_CRITICAL);
+ clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base +
0x60,
CLK_IS_CRITICAL|CLK_GET_RATE_NOCACHE);
Yes.
* Set the rate to 166935483 exactly (to match clk_get_rate)
Okay I hacked that in
diff --git a/drivers/devfreq/imx-ddrc.c b/drivers/devfreq/imx-ddrc.c
index 4eed6f50bb8d..a832768a865f 100644
--- a/drivers/devfreq/imx-ddrc.c
+++ b/drivers/devfreq/imx-ddrc.c
@@ -436,6 +436,10 @@ static int imx_ddrc_init_freq_info(struct device
*dev)
return -ENODEV;
}
+ /* B0 hack */
+ if ( freq->rate == 166750000 )
+ freq->rate = 166935483;
+inserting
pr_err( "arm_smcc rate %d %lu\n", index, freq->rate
);
}
A nicer solution would be to keep imx_ddrc_freq.rate in MT/s as
reported
by firmware and divide by 25000 in imx_ddrc_find_freq instead.
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -211,7 +211,7 @@
opp-hz = /bits/ 64 <100000000>;
};
opp-166M {
- opp-hz = /bits/ 64 <166750000>;
+ opp-hz = /bits/ 64 <166935483>;
};
opp-800M {
opp-hz = /bits/ 64 <800000000>;
Yes, this is the precise clock rate in Hz.
* Make the rounding in imx-ddrc more generous.
Sorry I don't understand what you mean by this
I meant to make imx_ddrc_find_freq find 667 MT/s for an OPP of
166935483:
/*
* Firmware reports values in MT/s, so we round-down from Hz
* Rounding is extra generous to ensure a match.
*/
rate = DIV_ROUND_CLOSEST(rate, 250000);
for (i = 0; i < priv->freq_count; ++i) {
struct imx_ddrc_freq *freq = &priv->freq_table[i];
if (freq->rate == rate ||
freq->rate + 1 == rate ||
freq->rate - 1 == rate)
return freq;
}
But your B0 hack above should also work.
Thanks with those additions the driver is working on imx8mq B0.
There is a small adjustment to the code above because you're scaling the
rate you also need to scale the matching rate.
--- a/drivers/devfreq/imx-ddrc.c
+++ b/drivers/devfreq/imx-ddrc.c
@@ -106,9 +106,17 @@ static struct imx_ddrc_freq
*imx_ddrc_find_freq(struct imx_ddrc *priv,
{
int i;
+ /*
+ * Firmware reports values in MT/s, so we round-down from Hz
+ * Rounding is extra generous to ensure a match.
+ */
+ rate = DIV_ROUND_CLOSEST(rate, 250000);
for (i = 0; i < priv->freq_count; ++i) {
- if (priv->freq_table[i].rate == rate)
+ struct imx_ddrc_freq *freq = &priv->freq_table[i];
+ unsigned long match_rate =
DIV_ROUND_CLOSEST(freq->rate,250000);
+ if (match_rate + 1 >= rate &&
+ match_rate - 1 <= rate)
return &priv->freq_table[i];
}
Cheers
Angus
Adding the other changes the board no longer hangs when trying to
change
frequencies but it also doesn't seem to actually change the frequency.
[ 3.076426] ddrc checking rate 800000000 166935483
[ 3.081290] ddrc checking rate 166935483 166935483
[ 3.086225] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about
to
change freq 800000000 to 166935483
[ 3.086891] imx-ddrc-devfreq 3d400000.dram-controller: ddrc changed
freq 800000000 to 166935483
root@pureos:~# cat /sys/class/devfreq/devfreq0/cur_freq
800000000
root@pureos:~# cat /sys/class/devfreq/devfreq0/target_freq
166935483
The target_freq value is from clk_get_rate(dram_core) but it is
dram_core's parent which gets updated. It seems that a clk mux ignores
CLK_GET_RATE_NOCACHE on the parent.
An update can be forced by adding `clk_get_rate(new_dram_core_parent);`
at the end of imx_ddrc_set_freq.
You should also be able to check by looking at clk_summary or
/sys/kernel/debug/clk/dram_core_clk/clk_rate
/sys/kernel/debug/clk/dram_pll_out/clk_rate
--
Regards,
Leonard
