| Message ID | 20190715124417.4787-28-l.luba@partner.samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1,01/50] clk: samsung: add new IDs for Exynos5420 clocks | expand |
On Mon, 15 Jul 2019 at 14:44, Lukasz Luba <l.luba@partner.samsung.com> wrote: > > This is the most important bus in the Exynos5x SoC. The whole communication > inside SoC does through that bus (apart from direct requests from CCI to > DRAM controller). It is also modeled as a master bus in devfreq framework. > It is also the only one OPP table throughout other buses which has voltage > values. The devfreq software controls the speed of that bus and other > buses. The other buses follows the rate of the master. There is only one > regulator. The old lowest OPP had pair 925mV, 84MHz which is enough for s/lowest/slowest/ > this frequency. However, due to the fact that the other buses follows the > WCORE bus by taking the OPP from their table with the same id, e.g. opp02, > the children frequency should be stable with the set voltage. > It could cause random faults very hard to debug. > Thus, the patch removes the lowest OPP to make other buses' lowest OPPs s/lowest/slowest/ > working. The new lowest OPP has voltage high enough for buses working up > to 333MHz. It also changes the frequencies of the OPPs to align them to > PLL value such that it is possible to set them using only a divider without > reprogramming OPP. Reprogramming OPP? What is it? > Reprogramming the PLL was not set, so the real frequency I understood from the previous that reprogramming the OPP (PLL?) was happening... Please rephrase entire sentence. BR, Krzysztof > values were not the one from the OPP table, which could confuse the > governor algorithms which relay on OPP speed values making the system to > behave weird. > > Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com> > --- > arch/arm/boot/dts/exynos5420.dtsi | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi > index f8c36ff0d4c3..a355c76af5a5 100644 > --- a/arch/arm/boot/dts/exynos5420.dtsi > +++ b/arch/arm/boot/dts/exynos5420.dtsi > @@ -1107,22 +1107,18 @@ > compatible = "operating-points-v2"; > > opp00 { > - opp-hz = /bits/ 64 <84000000>; > - opp-microvolt = <925000>; > + opp-hz = /bits/ 64 <150000000>; > + opp-microvolt = <950000>; > }; > opp01 { > - opp-hz = /bits/ 64 <111000000>; > + opp-hz = /bits/ 64 <200000000>; > opp-microvolt = <950000>; > }; > opp02 { > - opp-hz = /bits/ 64 <222000000>; > + opp-hz = /bits/ 64 <300000000>; > opp-microvolt = <950000>; > }; > opp03 { > - opp-hz = /bits/ 64 <333000000>; > - opp-microvolt = <950000>; > - }; > - opp04 { > opp-hz = /bits/ 64 <400000000>; > opp-microvolt = <987500>; > }; > -- > 2.17.1 >
On 7/17/19 12:15 PM, Krzysztof Kozlowski wrote: > On Mon, 15 Jul 2019 at 14:44, Lukasz Luba <l.luba@partner.samsung.com> wrote: >> >> This is the most important bus in the Exynos5x SoC. The whole communication >> inside SoC does through that bus (apart from direct requests from CCI to >> DRAM controller). It is also modeled as a master bus in devfreq framework. >> It is also the only one OPP table throughout other buses which has voltage >> values. The devfreq software controls the speed of that bus and other >> buses. The other buses follows the rate of the master. There is only one >> regulator. The old lowest OPP had pair 925mV, 84MHz which is enough for > > s/lowest/slowest/ OK > >> this frequency. However, due to the fact that the other buses follows the >> WCORE bus by taking the OPP from their table with the same id, e.g. opp02, >> the children frequency should be stable with the set voltage. >> It could cause random faults very hard to debug. >> Thus, the patch removes the lowest OPP to make other buses' lowest OPPs > > s/lowest/slowest/ OK > >> working. The new lowest OPP has voltage high enough for buses working up >> to 333MHz. It also changes the frequencies of the OPPs to align them to >> PLL value such that it is possible to set them using only a divider without >> reprogramming OPP. > > Reprogramming OPP? What is it? Mistake, should be PLL. Thanks for that. > >> Reprogramming the PLL was not set, so the real frequency > > I understood from the previous that reprogramming the OPP (PLL?) was > happening... Please rephrase entire sentence. Yes, I will rewrite it when I will combine these patches into one. Regards, Lukasz > > BR, > Krzysztof > >> values were not the one from the OPP table, which could confuse the >> governor algorithms which relay on OPP speed values making the system to >> behave weird. >> >> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com> >> --- >> arch/arm/boot/dts/exynos5420.dtsi | 12 ++++-------- >> 1 file changed, 4 insertions(+), 8 deletions(-) >> >> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi >> index f8c36ff0d4c3..a355c76af5a5 100644 >> --- a/arch/arm/boot/dts/exynos5420.dtsi >> +++ b/arch/arm/boot/dts/exynos5420.dtsi >> @@ -1107,22 +1107,18 @@ >> compatible = "operating-points-v2"; >> >> opp00 { >> - opp-hz = /bits/ 64 <84000000>; >> - opp-microvolt = <925000>; >> + opp-hz = /bits/ 64 <150000000>; >> + opp-microvolt = <950000>; >> }; >> opp01 { >> - opp-hz = /bits/ 64 <111000000>; >> + opp-hz = /bits/ 64 <200000000>; >> opp-microvolt = <950000>; >> }; >> opp02 { >> - opp-hz = /bits/ 64 <222000000>; >> + opp-hz = /bits/ 64 <300000000>; >> opp-microvolt = <950000>; >> }; >> opp03 { >> - opp-hz = /bits/ 64 <333000000>; >> - opp-microvolt = <950000>; >> - }; >> - opp04 { >> opp-hz = /bits/ 64 <400000000>; >> opp-microvolt = <987500>; >> }; >> -- >> 2.17.1 >> > >
Hi Krzysztof, On 7/17/19 12:15 PM, Krzysztof Kozlowski wrote: > On Mon, 15 Jul 2019 at 14:44, Lukasz Luba <l.luba@partner.samsung.com> wrote: >> >> This is the most important bus in the Exynos5x SoC. The whole communication >> inside SoC does through that bus (apart from direct requests from CCI to >> DRAM controller). It is also modeled as a master bus in devfreq framework. >> It is also the only one OPP table throughout other buses which has voltage >> values. The devfreq software controls the speed of that bus and other >> buses. The other buses follows the rate of the master. There is only one >> regulator. The old lowest OPP had pair 925mV, 84MHz which is enough for > > s/lowest/slowest/ please see below > >> this frequency. However, due to the fact that the other buses follows the >> WCORE bus by taking the OPP from their table with the same id, e.g. opp02, >> the children frequency should be stable with the set voltage. >> It could cause random faults very hard to debug. >> Thus, the patch removes the lowest OPP to make other buses' lowest OPPs > > s/lowest/slowest/ Actually, I have double checked that, because we always used this terminology: low OPP, high OPP, lower OPPs, higher OPPs. I can change it here for you, but I think this is not something that people are used to. Please check EAS pdf documentation or this file: https://www.kernel.org/doc/Documentation/scheduler/sched-energy.txt i.e. "running at a lower OPP" or "high OPPs", "lowest OPPs". Regards, Lukasz > >> working. The new lowest OPP has voltage high enough for buses working up >> to 333MHz. It also changes the frequencies of the OPPs to align them to >> PLL value such that it is possible to set them using only a divider without >> reprogramming OPP. > > Reprogramming OPP? What is it? > >> Reprogramming the PLL was not set, so the real frequency > > I understood from the previous that reprogramming the OPP (PLL?) was > happening... Please rephrase entire sentence. > > BR, > Krzysztof > >> values were not the one from the OPP table, which could confuse the >> governor algorithms which relay on OPP speed values making the system to >> behave weird. >> >> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com> >> --- >> arch/arm/boot/dts/exynos5420.dtsi | 12 ++++-------- >> 1 file changed, 4 insertions(+), 8 deletions(-) >> >> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi >> index f8c36ff0d4c3..a355c76af5a5 100644 >> --- a/arch/arm/boot/dts/exynos5420.dtsi >> +++ b/arch/arm/boot/dts/exynos5420.dtsi >> @@ -1107,22 +1107,18 @@ >> compatible = "operating-points-v2"; >> >> opp00 { >> - opp-hz = /bits/ 64 <84000000>; >> - opp-microvolt = <925000>; >> + opp-hz = /bits/ 64 <150000000>; >> + opp-microvolt = <950000>; >> }; >> opp01 { >> - opp-hz = /bits/ 64 <111000000>; >> + opp-hz = /bits/ 64 <200000000>; >> opp-microvolt = <950000>; >> }; >> opp02 { >> - opp-hz = /bits/ 64 <222000000>; >> + opp-hz = /bits/ 64 <300000000>; >> opp-microvolt = <950000>; >> }; >> opp03 { >> - opp-hz = /bits/ 64 <333000000>; >> - opp-microvolt = <950000>; >> - }; >> - opp04 { >> opp-hz = /bits/ 64 <400000000>; >> opp-microvolt = <987500>; >> }; >> -- >> 2.17.1 >> > >
On Wed, 17 Jul 2019 at 18:58, Lukasz Luba <l.luba@partner.samsung.com> wrote: > > Hi Krzysztof, > > On 7/17/19 12:15 PM, Krzysztof Kozlowski wrote: > > On Mon, 15 Jul 2019 at 14:44, Lukasz Luba <l.luba@partner.samsung.com> wrote: > >> > >> This is the most important bus in the Exynos5x SoC. The whole communication > >> inside SoC does through that bus (apart from direct requests from CCI to > >> DRAM controller). It is also modeled as a master bus in devfreq framework. > >> It is also the only one OPP table throughout other buses which has voltage > >> values. The devfreq software controls the speed of that bus and other > >> buses. The other buses follows the rate of the master. There is only one > >> regulator. The old lowest OPP had pair 925mV, 84MHz which is enough for > > > > s/lowest/slowest/ > please see below > > > >> this frequency. However, due to the fact that the other buses follows the > >> WCORE bus by taking the OPP from their table with the same id, e.g. opp02, > >> the children frequency should be stable with the set voltage. > >> It could cause random faults very hard to debug. > >> Thus, the patch removes the lowest OPP to make other buses' lowest OPPs > > > > s/lowest/slowest/ > Actually, I have double checked that, because we always used this > terminology: low OPP, high OPP, lower OPPs, higher OPPs. I can change > it here for you, but I think this is not something that people are used > to. Please check EAS pdf documentation or this file: > https://www.kernel.org/doc/Documentation/scheduler/sched-energy.txt > i.e. "running at a lower OPP" or "high OPPs", "lowest OPPs". Hmm, indeed, you're right. Don't change it then. Best regards, Krzysztof
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi index f8c36ff0d4c3..a355c76af5a5 100644 --- a/arch/arm/boot/dts/exynos5420.dtsi +++ b/arch/arm/boot/dts/exynos5420.dtsi @@ -1107,22 +1107,18 @@ compatible = "operating-points-v2"; opp00 { - opp-hz = /bits/ 64 <84000000>; - opp-microvolt = <925000>; + opp-hz = /bits/ 64 <150000000>; + opp-microvolt = <950000>; }; opp01 { - opp-hz = /bits/ 64 <111000000>; + opp-hz = /bits/ 64 <200000000>; opp-microvolt = <950000>; }; opp02 { - opp-hz = /bits/ 64 <222000000>; + opp-hz = /bits/ 64 <300000000>; opp-microvolt = <950000>; }; opp03 { - opp-hz = /bits/ 64 <333000000>; - opp-microvolt = <950000>; - }; - opp04 { opp-hz = /bits/ 64 <400000000>; opp-microvolt = <987500>; };
This is the most important bus in the Exynos5x SoC. The whole communication inside SoC does through that bus (apart from direct requests from CCI to DRAM controller). It is also modeled as a master bus in devfreq framework. It is also the only one OPP table throughout other buses which has voltage values. The devfreq software controls the speed of that bus and other buses. The other buses follows the rate of the master. There is only one regulator. The old lowest OPP had pair 925mV, 84MHz which is enough for this frequency. However, due to the fact that the other buses follows the WCORE bus by taking the OPP from their table with the same id, e.g. opp02, the children frequency should be stable with the set voltage. It could cause random faults very hard to debug. Thus, the patch removes the lowest OPP to make other buses' lowest OPPs working. The new lowest OPP has voltage high enough for buses working up to 333MHz. It also changes the frequencies of the OPPs to align them to PLL value such that it is possible to set them using only a divider without reprogramming OPP. Reprogramming the PLL was not set, so the real frequency values were not the one from the OPP table, which could confuse the governor algorithms which relay on OPP speed values making the system to behave weird. Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com> --- arch/arm/boot/dts/exynos5420.dtsi | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)