diff mbox

ARM: dts: Add missing CPU frequencies for Exynos5422/5800

Message ID 5220084.l31t5oJbsy@amdc3058 (mailing list archive)
State Superseded
Headers show

Commit Message

Bartlomiej Zolnierkiewicz Dec. 13, 2016, 4:52 p.m. UTC
Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
(for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
cooling maps to account for new OPPs.

Since new OPPs are not available on all Exynos5422/5800 boards modify
dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.

Tested on Odroid-XU3 and XU3 Lite.

Cc: Doug Anderson <dianders@chromium.org>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Andreas Faerber <afaerber@suse.de>
Cc: Thomas Abraham <thomas.ab@samsung.com>
Cc: Ben Gamari <ben@smart-cactus.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
 arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
 arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
 arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
 4 files changed, 43 insertions(+), 7 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Javier Martinez Canillas Dec. 13, 2016, 7:18 p.m. UTC | #1
Hello Bartlomiej,

On 12/13/2016 01:52 PM, Bartlomiej Zolnierkiewicz wrote:
> Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
> (for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
> cooling maps to account for new OPPs.
> 
> Since new OPPs are not available on all Exynos5422/5800 boards modify
> dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
> Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
> 
> Tested on Odroid-XU3 and XU3 Lite.
> 
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Andreas Faerber <afaerber@suse.de>
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> Cc: Ben Gamari <ben@smart-cactus.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
>  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
>  arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
>  arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
>  4 files changed, 43 insertions(+), 7 deletions(-)
> 
> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.779763261 +0100
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.775763261 +0100
> @@ -118,7 +118,7 @@
>  				/*
>  				 * When reaching cpu_alert3, reduce CPU
>  				 * by 2 steps. On Exynos5422/5800 that would
> -				 * be: 1600 MHz and 1100 MHz.
> +				 * (usually) be: 1800 MHz and 1200 MHz.
>  				 */
>  				map3 {
>  					trip = <&cpu_alert3>;
> @@ -131,16 +131,16 @@
>  
>  				/*
>  				 * When reaching cpu_alert4, reduce CPU
> -				 * further, down to 600 MHz (11 steps for big,
> -				 * 7 steps for LITTLE).
> +				 * further, down to 600 MHz (13 steps for big,
> +				 * 8 steps for LITTLE).
>  				 */
> -				map5 {
> +				cooling_map5: map5 {
>  					trip = <&cpu_alert4>;
> -					cooling-device = <&cpu0 3 7>;
> +					cooling-device = <&cpu0 3 8>;
>  				};
> -				map6 {
> +				cooling_map6: map6 {
>  					trip = <&cpu_alert4>;
> -					cooling-device = <&cpu4 3 11>;
> +					cooling-device = <&cpu4 3 13>;
>  				};
>  			};
>  		};
> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.779763261 +0100
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.775763261 +0100
> @@ -21,6 +21,23 @@
>  	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
>  };
>  
> +&cluster_a15_opp_table {
> +	/delete-node/opp@2000000000;
> +	/delete-node/opp@1900000000;
> +};
> +
> +&cluster_a7_opp_table {
> +	/delete-node/opp@1400000000;
> +};
> +

I think that a comment in the DTS why these operating points aren't available
in this board will make more clear why the nodes are being deleted.

> +&cooling_map5 {
> +	cooling-device = <&cpu0 3 7>;
> +};
> +
> +&cooling_map6 {
> +	cooling-device = <&cpu4 3 11>;
> +};
> +
>  &pwm {
>  	/*
>  	 * PWM 0 -- fan
> Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> @@ -146,6 +146,10 @@
>  	vdd-supply = <&ldo9_reg>;
>  };
>  
> +&cluster_a7_opp_table {
> +	/delete-property/opp@1400000000;
> +};
> +
>  &cpu0 {
>  	cpu-supply = <&buck2_reg>;
>  };
> Index: b/arch/arm/boot/dts/exynos5800.dtsi
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> +++ b/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> @@ -24,6 +24,16 @@
>  };
>  
>  &cluster_a15_opp_table {
> +	opp@2000000000 {
> +		opp-hz = /bits/ 64 <2000000000>;
> +		opp-microvolt = <1250000>;
> +		clock-latency-ns = <140000>;
> +	};
> +	opp@1900000000 {
> +		opp-hz = /bits/ 64 <1900000000>;
> +		opp-microvolt = <1250000>;
> +		clock-latency-ns = <140000>;
> +	};
>  	opp@1700000000 {
>  		opp-microvolt = <1250000>;
>  	};
> @@ -85,6 +95,11 @@
>  };
>

AFAIK Thomas restricted the maximum OPP, because for A15 freqs > 1.8GHz the
INT rail would need to be scaled up as well since there's a maximum voltage
difference between the ARM and INT rails before the system becomes unstable:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/276766.html
https://lkml.org/lkml/2014/5/2/419

The ChromiumOS vendor tree uses a virtual regulator driver that makes sure
the maximum voltage skew is between a limit. But that never made to mainline:

https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/boot/dts/exynos5420-peach-pit.dtsi#90
https://lkml.org/lkml/2014/4/29/28

Did that change and there's infrastructure in mainline now to cope with that?
If that's the case, I think it would be good to mention in the commit message.

Best regards,
Bartlomiej Zolnierkiewicz Dec. 14, 2016, 1:28 p.m. UTC | #2
On Tuesday, December 13, 2016 04:18:05 PM Javier Martinez Canillas wrote:
> Hello Bartlomiej,

Hi,

> On 12/13/2016 01:52 PM, Bartlomiej Zolnierkiewicz wrote:
> > Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
> > (for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
> > cooling maps to account for new OPPs.
> > 
> > Since new OPPs are not available on all Exynos5422/5800 boards modify
> > dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
> > Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
> > 
> > Tested on Odroid-XU3 and XU3 Lite.
> > 
> > Cc: Doug Anderson <dianders@chromium.org>
> > Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> > Cc: Andreas Faerber <afaerber@suse.de>
> > Cc: Thomas Abraham <thomas.ab@samsung.com>
> > Cc: Ben Gamari <ben@smart-cactus.org>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
> >  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
> >  arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
> >  arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
> >  4 files changed, 43 insertions(+), 7 deletions(-)
> > 
> > Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.779763261 +0100
> > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.775763261 +0100
> > @@ -118,7 +118,7 @@
> >  				/*
> >  				 * When reaching cpu_alert3, reduce CPU
> >  				 * by 2 steps. On Exynos5422/5800 that would
> > -				 * be: 1600 MHz and 1100 MHz.
> > +				 * (usually) be: 1800 MHz and 1200 MHz.
> >  				 */
> >  				map3 {
> >  					trip = <&cpu_alert3>;
> > @@ -131,16 +131,16 @@
> >  
> >  				/*
> >  				 * When reaching cpu_alert4, reduce CPU
> > -				 * further, down to 600 MHz (11 steps for big,
> > -				 * 7 steps for LITTLE).
> > +				 * further, down to 600 MHz (13 steps for big,
> > +				 * 8 steps for LITTLE).
> >  				 */
> > -				map5 {
> > +				cooling_map5: map5 {
> >  					trip = <&cpu_alert4>;
> > -					cooling-device = <&cpu0 3 7>;
> > +					cooling-device = <&cpu0 3 8>;
> >  				};
> > -				map6 {
> > +				cooling_map6: map6 {
> >  					trip = <&cpu_alert4>;
> > -					cooling-device = <&cpu4 3 11>;
> > +					cooling-device = <&cpu4 3 13>;
> >  				};
> >  			};
> >  		};
> > Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.779763261 +0100
> > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.775763261 +0100
> > @@ -21,6 +21,23 @@
> >  	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
> >  };
> >  
> > +&cluster_a15_opp_table {
> > +	/delete-node/opp@2000000000;
> > +	/delete-node/opp@1900000000;
> > +};
> > +
> > +&cluster_a7_opp_table {
> > +	/delete-node/opp@1400000000;
> > +};
> > +
> 
> I think that a comment in the DTS why these operating points aren't available
> in this board will make more clear why the nodes are being deleted.

Ok, I will add these comments in the next patch revision.

> > +&cooling_map5 {
> > +	cooling-device = <&cpu0 3 7>;
> > +};
> > +
> > +&cooling_map6 {
> > +	cooling-device = <&cpu4 3 11>;
> > +};
> > +
> >  &pwm {
> >  	/*
> >  	 * PWM 0 -- fan
> > Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> > +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> > @@ -146,6 +146,10 @@
> >  	vdd-supply = <&ldo9_reg>;
> >  };
> >  
> > +&cluster_a7_opp_table {
> > +	/delete-property/opp@1400000000;
> > +};
> > +
> >  &cpu0 {
> >  	cpu-supply = <&buck2_reg>;
> >  };
> > Index: b/arch/arm/boot/dts/exynos5800.dtsi
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> > +++ b/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> > @@ -24,6 +24,16 @@
> >  };
> >  
> >  &cluster_a15_opp_table {
> > +	opp@2000000000 {
> > +		opp-hz = /bits/ 64 <2000000000>;
> > +		opp-microvolt = <1250000>;
> > +		clock-latency-ns = <140000>;
> > +	};
> > +	opp@1900000000 {
> > +		opp-hz = /bits/ 64 <1900000000>;
> > +		opp-microvolt = <1250000>;
> > +		clock-latency-ns = <140000>;
> > +	};
> >  	opp@1700000000 {
> >  		opp-microvolt = <1250000>;
> >  	};
> > @@ -85,6 +95,11 @@
> >  };
> >
> 
> AFAIK Thomas restricted the maximum OPP, because for A15 freqs > 1.8GHz the
> INT rail would need to be scaled up as well since there's a maximum voltage
> difference between the ARM and INT rails before the system becomes unstable:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/276766.html
> https://lkml.org/lkml/2014/5/2/419
> 
> The ChromiumOS vendor tree uses a virtual regulator driver that makes sure
> the maximum voltage skew is between a limit. But that never made to mainline:
> 
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/boot/dts/exynos5420-peach-pit.dtsi#90
> https://lkml.org/lkml/2014/4/29/28
> 
> Did that change and there's infrastructure in mainline now to cope with that?
> If that's the case, I think it would be good to mention in the commit message.

I was not aware of this limitation and AFAIK mainline has currently
no code to handle it.  I also cannot find any code to handle this in
Hardkernel's vendor kernel for Odroid-XU3 board.

Do you know whether this problem exists also on Exynos5422/5800
SoCs or only on Exynos5420 one?  I see that ChromiumOS uses virtual
regulator code also on Exynos5800 SoC based Peach Pi board but was
the problem actually present on this board?

[ I added Arjun to Cc:, maybe he can help in explaining this issue
  (unfortunately Inderpal's email is no longer working). ]

Please also note that on Exynos5422/5800 SoCs the same ARM rail
voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
IOW if the problem exists it is already present in the mainline
kernel.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Dec. 14, 2016, 2:06 p.m. UTC | #3
Hello Bartlomiej,

On 12/14/2016 10:28 AM, Bartlomiej Zolnierkiewicz wrote:
> 
> On Tuesday, December 13, 2016 04:18:05 PM Javier Martinez Canillas wrote:
>> Hello Bartlomiej,
> 
> Hi,
> 
>> On 12/13/2016 01:52 PM, Bartlomiej Zolnierkiewicz wrote:
>>> Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
>>> (for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
>>> cooling maps to account for new OPPs.
>>>
>>> Since new OPPs are not available on all Exynos5422/5800 boards modify
>>> dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
>>> Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
>>>
>>> Tested on Odroid-XU3 and XU3 Lite.
>>>
>>> Cc: Doug Anderson <dianders@chromium.org>
>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>>> Cc: Andreas Faerber <afaerber@suse.de>
>>> Cc: Thomas Abraham <thomas.ab@samsung.com>
>>> Cc: Ben Gamari <ben@smart-cactus.org>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>> ---
>>>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
>>>  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
>>>  arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
>>>  arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
>>>  4 files changed, 43 insertions(+), 7 deletions(-)
>>>
>>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>>> ===================================================================
>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.779763261 +0100
>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.775763261 +0100
>>> @@ -118,7 +118,7 @@
>>>  				/*
>>>  				 * When reaching cpu_alert3, reduce CPU
>>>  				 * by 2 steps. On Exynos5422/5800 that would
>>> -				 * be: 1600 MHz and 1100 MHz.
>>> +				 * (usually) be: 1800 MHz and 1200 MHz.
>>>  				 */
>>>  				map3 {
>>>  					trip = <&cpu_alert3>;
>>> @@ -131,16 +131,16 @@
>>>  
>>>  				/*
>>>  				 * When reaching cpu_alert4, reduce CPU
>>> -				 * further, down to 600 MHz (11 steps for big,
>>> -				 * 7 steps for LITTLE).
>>> +				 * further, down to 600 MHz (13 steps for big,
>>> +				 * 8 steps for LITTLE).
>>>  				 */
>>> -				map5 {
>>> +				cooling_map5: map5 {
>>>  					trip = <&cpu_alert4>;
>>> -					cooling-device = <&cpu0 3 7>;
>>> +					cooling-device = <&cpu0 3 8>;
>>>  				};
>>> -				map6 {
>>> +				cooling_map6: map6 {
>>>  					trip = <&cpu_alert4>;
>>> -					cooling-device = <&cpu4 3 11>;
>>> +					cooling-device = <&cpu4 3 13>;
>>>  				};
>>>  			};
>>>  		};
>>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
>>> ===================================================================
>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.779763261 +0100
>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.775763261 +0100
>>> @@ -21,6 +21,23 @@
>>>  	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
>>>  };
>>>  
>>> +&cluster_a15_opp_table {
>>> +	/delete-node/opp@2000000000;
>>> +	/delete-node/opp@1900000000;
>>> +};
>>> +
>>> +&cluster_a7_opp_table {
>>> +	/delete-node/opp@1400000000;
>>> +};
>>> +
>>
>> I think that a comment in the DTS why these operating points aren't available
>> in this board will make more clear why the nodes are being deleted.
> 
> Ok, I will add these comments in the next patch revision.
> 
>>> +&cooling_map5 {
>>> +	cooling-device = <&cpu0 3 7>;
>>> +};
>>> +
>>> +&cooling_map6 {
>>> +	cooling-device = <&cpu4 3 11>;
>>> +};
>>> +
>>>  &pwm {
>>>  	/*
>>>  	 * PWM 0 -- fan
>>> Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>>> ===================================================================
>>> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
>>> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
>>> @@ -146,6 +146,10 @@
>>>  	vdd-supply = <&ldo9_reg>;
>>>  };
>>>  
>>> +&cluster_a7_opp_table {
>>> +	/delete-property/opp@1400000000;
>>> +};
>>> +
>>>  &cpu0 {
>>>  	cpu-supply = <&buck2_reg>;
>>>  };
>>> Index: b/arch/arm/boot/dts/exynos5800.dtsi
>>> ===================================================================
>>> --- a/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
>>> +++ b/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
>>> @@ -24,6 +24,16 @@
>>>  };
>>>  
>>>  &cluster_a15_opp_table {
>>> +	opp@2000000000 {
>>> +		opp-hz = /bits/ 64 <2000000000>;
>>> +		opp-microvolt = <1250000>;
>>> +		clock-latency-ns = <140000>;
>>> +	};
>>> +	opp@1900000000 {
>>> +		opp-hz = /bits/ 64 <1900000000>;
>>> +		opp-microvolt = <1250000>;
>>> +		clock-latency-ns = <140000>;
>>> +	};
>>>  	opp@1700000000 {
>>>  		opp-microvolt = <1250000>;
>>>  	};
>>> @@ -85,6 +95,11 @@
>>>  };
>>>
>>
>> AFAIK Thomas restricted the maximum OPP, because for A15 freqs > 1.8GHz the
>> INT rail would need to be scaled up as well since there's a maximum voltage
>> difference between the ARM and INT rails before the system becomes unstable:
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/276766.html
>> https://lkml.org/lkml/2014/5/2/419
>>
>> The ChromiumOS vendor tree uses a virtual regulator driver that makes sure
>> the maximum voltage skew is between a limit. But that never made to mainline:
>>
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/boot/dts/exynos5420-peach-pit.dtsi#90
>> https://lkml.org/lkml/2014/4/29/28
>>
>> Did that change and there's infrastructure in mainline now to cope with that?
>> If that's the case, I think it would be good to mention in the commit message.
> 
> I was not aware of this limitation and AFAIK mainline has currently
> no code to handle it.  I also cannot find any code to handle this in

Yes, that's what I thought as well but maybe I was missing something.

> Hardkernel's vendor kernel for Odroid-XU3 board.
> 
> Do you know whether this problem exists also on Exynos5422/5800
> SoCs or only on Exynos5420 one?  I see that ChromiumOS uses virtual
> regulator code also on Exynos5800 SoC based Peach Pi board but was
> the problem actually present on this board?
>

My understanding is that this problem is present in 5420/5422/5800
but I have no way to confirm this. I'm just guessing from the info
in the ChromiumOS vendor tree.

If you look at the commit that added the regulator-locker driver,
the test says to be done in a Peach Pi so it seems the problem is
not restricted to only Exynos5420:

https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ba63620feace4eaed5572ac2e77d8d61754af704
 
> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>   (unfortunately Inderpal's email is no longer working). ]
>

Added Abhilash to cc list as well since he was involved in the Exynos ChromeOS tree.

> Please also note that on Exynos5422/5800 SoCs the same ARM rail
> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
> IOW if the problem exists it is already present in the mainline
> kernel.
>

Ah, I only looked at your patch and I didn't compare the voltage levels
in your 1.9 GHz and 2.0 GHz OPPs with the current 1.8 GHz in mainline.

I wonder if the voltage levels in your patch are correct then, since the
ChromiumOS tree uses a higher voltages for the 1.9 GHz and 2.0 GHz OPPs:

/*
 * Default ASV table
 */
static const unsigned int asv_voltage_5420[CPUFREQ_NUM_LEVELS] = {
	1362500,	/* L0  2100 */
	1312500,	/* L1  2000 */
	1275000,	/* L2  1900 */
	1225000,	/* L3  1800 */
	1200000,	/* L4  1700 */

This is from https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#175

> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 

Best regards,
Bartlomiej Zolnierkiewicz Dec. 14, 2016, 2:25 p.m. UTC | #4
Hi,

On Wednesday, December 14, 2016 11:06:45 AM Javier Martinez Canillas wrote:
> 
> Hello Bartlomiej,
> 
> On 12/14/2016 10:28 AM, Bartlomiej Zolnierkiewicz wrote:
> > 
> > On Tuesday, December 13, 2016 04:18:05 PM Javier Martinez Canillas wrote:
> >> Hello Bartlomiej,
> > 
> > Hi,
> > 
> >> On 12/13/2016 01:52 PM, Bartlomiej Zolnierkiewicz wrote:
> >>> Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
> >>> (for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
> >>> cooling maps to account for new OPPs.
> >>>
> >>> Since new OPPs are not available on all Exynos5422/5800 boards modify
> >>> dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
> >>> Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
> >>>
> >>> Tested on Odroid-XU3 and XU3 Lite.
> >>>
> >>> Cc: Doug Anderson <dianders@chromium.org>
> >>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> >>> Cc: Andreas Faerber <afaerber@suse.de>
> >>> Cc: Thomas Abraham <thomas.ab@samsung.com>
> >>> Cc: Ben Gamari <ben@smart-cactus.org>
> >>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> >>> ---
> >>>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
> >>>  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
> >>>  arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
> >>>  arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
> >>>  4 files changed, 43 insertions(+), 7 deletions(-)
> >>>
> >>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> >>> ===================================================================
> >>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.779763261 +0100
> >>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.775763261 +0100
> >>> @@ -118,7 +118,7 @@
> >>>  				/*
> >>>  				 * When reaching cpu_alert3, reduce CPU
> >>>  				 * by 2 steps. On Exynos5422/5800 that would
> >>> -				 * be: 1600 MHz and 1100 MHz.
> >>> +				 * (usually) be: 1800 MHz and 1200 MHz.
> >>>  				 */
> >>>  				map3 {
> >>>  					trip = <&cpu_alert3>;
> >>> @@ -131,16 +131,16 @@
> >>>  
> >>>  				/*
> >>>  				 * When reaching cpu_alert4, reduce CPU
> >>> -				 * further, down to 600 MHz (11 steps for big,
> >>> -				 * 7 steps for LITTLE).
> >>> +				 * further, down to 600 MHz (13 steps for big,
> >>> +				 * 8 steps for LITTLE).
> >>>  				 */
> >>> -				map5 {
> >>> +				cooling_map5: map5 {
> >>>  					trip = <&cpu_alert4>;
> >>> -					cooling-device = <&cpu0 3 7>;
> >>> +					cooling-device = <&cpu0 3 8>;
> >>>  				};
> >>> -				map6 {
> >>> +				cooling_map6: map6 {
> >>>  					trip = <&cpu_alert4>;
> >>> -					cooling-device = <&cpu4 3 11>;
> >>> +					cooling-device = <&cpu4 3 13>;
> >>>  				};
> >>>  			};
> >>>  		};
> >>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
> >>> ===================================================================
> >>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.779763261 +0100
> >>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.775763261 +0100
> >>> @@ -21,6 +21,23 @@
> >>>  	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
> >>>  };
> >>>  
> >>> +&cluster_a15_opp_table {
> >>> +	/delete-node/opp@2000000000;
> >>> +	/delete-node/opp@1900000000;
> >>> +};
> >>> +
> >>> +&cluster_a7_opp_table {
> >>> +	/delete-node/opp@1400000000;
> >>> +};
> >>> +
> >>
> >> I think that a comment in the DTS why these operating points aren't available
> >> in this board will make more clear why the nodes are being deleted.
> > 
> > Ok, I will add these comments in the next patch revision.
> > 
> >>> +&cooling_map5 {
> >>> +	cooling-device = <&cpu0 3 7>;
> >>> +};
> >>> +
> >>> +&cooling_map6 {
> >>> +	cooling-device = <&cpu4 3 11>;
> >>> +};
> >>> +
> >>>  &pwm {
> >>>  	/*
> >>>  	 * PWM 0 -- fan
> >>> Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> >>> ===================================================================
> >>> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> >>> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> >>> @@ -146,6 +146,10 @@
> >>>  	vdd-supply = <&ldo9_reg>;
> >>>  };
> >>>  
> >>> +&cluster_a7_opp_table {
> >>> +	/delete-property/opp@1400000000;
> >>> +};
> >>> +
> >>>  &cpu0 {
> >>>  	cpu-supply = <&buck2_reg>;
> >>>  };
> >>> Index: b/arch/arm/boot/dts/exynos5800.dtsi
> >>> ===================================================================
> >>> --- a/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> >>> +++ b/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> >>> @@ -24,6 +24,16 @@
> >>>  };
> >>>  
> >>>  &cluster_a15_opp_table {
> >>> +	opp@2000000000 {
> >>> +		opp-hz = /bits/ 64 <2000000000>;
> >>> +		opp-microvolt = <1250000>;
> >>> +		clock-latency-ns = <140000>;
> >>> +	};
> >>> +	opp@1900000000 {
> >>> +		opp-hz = /bits/ 64 <1900000000>;
> >>> +		opp-microvolt = <1250000>;
> >>> +		clock-latency-ns = <140000>;
> >>> +	};
> >>>  	opp@1700000000 {
> >>>  		opp-microvolt = <1250000>;
> >>>  	};
> >>> @@ -85,6 +95,11 @@
> >>>  };
> >>>
> >>
> >> AFAIK Thomas restricted the maximum OPP, because for A15 freqs > 1.8GHz the
> >> INT rail would need to be scaled up as well since there's a maximum voltage
> >> difference between the ARM and INT rails before the system becomes unstable:
> >>
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/276766.html
> >> https://lkml.org/lkml/2014/5/2/419
> >>
> >> The ChromiumOS vendor tree uses a virtual regulator driver that makes sure
> >> the maximum voltage skew is between a limit. But that never made to mainline:
> >>
> >> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/boot/dts/exynos5420-peach-pit.dtsi#90
> >> https://lkml.org/lkml/2014/4/29/28
> >>
> >> Did that change and there's infrastructure in mainline now to cope with that?
> >> If that's the case, I think it would be good to mention in the commit message.
> > 
> > I was not aware of this limitation and AFAIK mainline has currently
> > no code to handle it.  I also cannot find any code to handle this in
> 
> Yes, that's what I thought as well but maybe I was missing something.
> 
> > Hardkernel's vendor kernel for Odroid-XU3 board.
> > 
> > Do you know whether this problem exists also on Exynos5422/5800
> > SoCs or only on Exynos5420 one?  I see that ChromiumOS uses virtual
> > regulator code also on Exynos5800 SoC based Peach Pi board but was
> > the problem actually present on this board?
> >
> 
> My understanding is that this problem is present in 5420/5422/5800
> but I have no way to confirm this. I'm just guessing from the info
> in the ChromiumOS vendor tree.
> 
> If you look at the commit that added the regulator-locker driver,
> the test says to be done in a Peach Pi so it seems the problem is
> not restricted to only Exynos5420:
> 
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ba63620feace4eaed5572ac2e77d8d61754af704
>  
> > [ I added Arjun to Cc:, maybe he can help in explaining this issue
> >   (unfortunately Inderpal's email is no longer working). ]
> >
> 
> Added Abhilash to cc list as well since he was involved in the Exynos ChromeOS tree.

Abhilash's email is also no longer valid.. :(

> > Please also note that on Exynos5422/5800 SoCs the same ARM rail
> > voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
> > IOW if the problem exists it is already present in the mainline
> > kernel.
> >
> 
> Ah, I only looked at your patch and I didn't compare the voltage levels
> in your 1.9 GHz and 2.0 GHz OPPs with the current 1.8 GHz in mainline.
> 
> I wonder if the voltage levels in your patch are correct then, since the
> ChromiumOS tree uses a higher voltages for the 1.9 GHz and 2.0 GHz OPPs:
> 
> /*
>  * Default ASV table
>  */
> static const unsigned int asv_voltage_5420[CPUFREQ_NUM_LEVELS] = {
> 	1362500,	/* L0  2100 */
> 	1312500,	/* L1  2000 */
> 	1275000,	/* L2  1900 */
> 	1225000,	/* L3  1800 */
> 	1200000,	/* L4  1700 */
> 
> This is from https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#175

I think that the above voltages are original ones for Exynos5420
(not for Exynos5422/5800).

The voltages in my patch were taken from Hardkernel's kernel for
Odroid-XU3:

/*
 * ASV group voltage table
 */
static const unsigned int asv_voltage_5422_CA15[CPUFREQ_LEVEL_END_CA15] = {
...
	1250000,    /* L4  2000 */
	1250000,    /* L5  1900 */
	1250000,    /* L6  1800 */
...

https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y-android/drivers/cpufreq/exynos5422-eagle-cpufreq.c#L314

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Dec. 14, 2016, 2:40 p.m. UTC | #5
Hello Bartlomiej,

On 12/14/2016 11:25 AM, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Wednesday, December 14, 2016 11:06:45 AM Javier Martinez Canillas wrote:
>>
>> Hello Bartlomiej,
>>
>> On 12/14/2016 10:28 AM, Bartlomiej Zolnierkiewicz wrote:
>>>
>>> On Tuesday, December 13, 2016 04:18:05 PM Javier Martinez Canillas wrote:
>>>> Hello Bartlomiej,
>>>
>>> Hi,
>>>
>>>> On 12/13/2016 01:52 PM, Bartlomiej Zolnierkiewicz wrote:
>>>>> Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
>>>>> (for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
>>>>> cooling maps to account for new OPPs.
>>>>>
>>>>> Since new OPPs are not available on all Exynos5422/5800 boards modify
>>>>> dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
>>>>> Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
>>>>>
>>>>> Tested on Odroid-XU3 and XU3 Lite.
>>>>>
>>>>> Cc: Doug Anderson <dianders@chromium.org>
>>>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>>>>> Cc: Andreas Faerber <afaerber@suse.de>
>>>>> Cc: Thomas Abraham <thomas.ab@samsung.com>
>>>>> Cc: Ben Gamari <ben@smart-cactus.org>
>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>>>> ---
>>>>>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
>>>>>  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
>>>>>  arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
>>>>>  arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
>>>>>  4 files changed, 43 insertions(+), 7 deletions(-)
>>>>>
>>>>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>>>>> ===================================================================
>>>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.779763261 +0100
>>>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.775763261 +0100
>>>>> @@ -118,7 +118,7 @@
>>>>>  				/*
>>>>>  				 * When reaching cpu_alert3, reduce CPU
>>>>>  				 * by 2 steps. On Exynos5422/5800 that would
>>>>> -				 * be: 1600 MHz and 1100 MHz.
>>>>> +				 * (usually) be: 1800 MHz and 1200 MHz.
>>>>>  				 */
>>>>>  				map3 {
>>>>>  					trip = <&cpu_alert3>;
>>>>> @@ -131,16 +131,16 @@
>>>>>  
>>>>>  				/*
>>>>>  				 * When reaching cpu_alert4, reduce CPU
>>>>> -				 * further, down to 600 MHz (11 steps for big,
>>>>> -				 * 7 steps for LITTLE).
>>>>> +				 * further, down to 600 MHz (13 steps for big,
>>>>> +				 * 8 steps for LITTLE).
>>>>>  				 */
>>>>> -				map5 {
>>>>> +				cooling_map5: map5 {
>>>>>  					trip = <&cpu_alert4>;
>>>>> -					cooling-device = <&cpu0 3 7>;
>>>>> +					cooling-device = <&cpu0 3 8>;
>>>>>  				};
>>>>> -				map6 {
>>>>> +				cooling_map6: map6 {
>>>>>  					trip = <&cpu_alert4>;
>>>>> -					cooling-device = <&cpu4 3 11>;
>>>>> +					cooling-device = <&cpu4 3 13>;
>>>>>  				};
>>>>>  			};
>>>>>  		};
>>>>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
>>>>> ===================================================================
>>>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.779763261 +0100
>>>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.775763261 +0100
>>>>> @@ -21,6 +21,23 @@
>>>>>  	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
>>>>>  };
>>>>>  
>>>>> +&cluster_a15_opp_table {
>>>>> +	/delete-node/opp@2000000000;
>>>>> +	/delete-node/opp@1900000000;
>>>>> +};
>>>>> +
>>>>> +&cluster_a7_opp_table {
>>>>> +	/delete-node/opp@1400000000;
>>>>> +};
>>>>> +
>>>>
>>>> I think that a comment in the DTS why these operating points aren't available
>>>> in this board will make more clear why the nodes are being deleted.
>>>
>>> Ok, I will add these comments in the next patch revision.
>>>
>>>>> +&cooling_map5 {
>>>>> +	cooling-device = <&cpu0 3 7>;
>>>>> +};
>>>>> +
>>>>> +&cooling_map6 {
>>>>> +	cooling-device = <&cpu4 3 11>;
>>>>> +};
>>>>> +
>>>>>  &pwm {
>>>>>  	/*
>>>>>  	 * PWM 0 -- fan
>>>>> Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>>>>> ===================================================================
>>>>> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
>>>>> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
>>>>> @@ -146,6 +146,10 @@
>>>>>  	vdd-supply = <&ldo9_reg>;
>>>>>  };
>>>>>  
>>>>> +&cluster_a7_opp_table {
>>>>> +	/delete-property/opp@1400000000;
>>>>> +};
>>>>> +
>>>>>  &cpu0 {
>>>>>  	cpu-supply = <&buck2_reg>;
>>>>>  };
>>>>> Index: b/arch/arm/boot/dts/exynos5800.dtsi
>>>>> ===================================================================
>>>>> --- a/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
>>>>> +++ b/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
>>>>> @@ -24,6 +24,16 @@
>>>>>  };
>>>>>  
>>>>>  &cluster_a15_opp_table {
>>>>> +	opp@2000000000 {
>>>>> +		opp-hz = /bits/ 64 <2000000000>;
>>>>> +		opp-microvolt = <1250000>;
>>>>> +		clock-latency-ns = <140000>;
>>>>> +	};
>>>>> +	opp@1900000000 {
>>>>> +		opp-hz = /bits/ 64 <1900000000>;
>>>>> +		opp-microvolt = <1250000>;
>>>>> +		clock-latency-ns = <140000>;
>>>>> +	};
>>>>>  	opp@1700000000 {
>>>>>  		opp-microvolt = <1250000>;
>>>>>  	};
>>>>> @@ -85,6 +95,11 @@
>>>>>  };
>>>>>
>>>>
>>>> AFAIK Thomas restricted the maximum OPP, because for A15 freqs > 1.8GHz the
>>>> INT rail would need to be scaled up as well since there's a maximum voltage
>>>> difference between the ARM and INT rails before the system becomes unstable:
>>>>
>>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/276766.html
>>>> https://lkml.org/lkml/2014/5/2/419
>>>>
>>>> The ChromiumOS vendor tree uses a virtual regulator driver that makes sure
>>>> the maximum voltage skew is between a limit. But that never made to mainline:
>>>>
>>>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/boot/dts/exynos5420-peach-pit.dtsi#90
>>>> https://lkml.org/lkml/2014/4/29/28
>>>>
>>>> Did that change and there's infrastructure in mainline now to cope with that?
>>>> If that's the case, I think it would be good to mention in the commit message.
>>>
>>> I was not aware of this limitation and AFAIK mainline has currently
>>> no code to handle it.  I also cannot find any code to handle this in
>>
>> Yes, that's what I thought as well but maybe I was missing something.
>>
>>> Hardkernel's vendor kernel for Odroid-XU3 board.
>>>
>>> Do you know whether this problem exists also on Exynos5422/5800
>>> SoCs or only on Exynos5420 one?  I see that ChromiumOS uses virtual
>>> regulator code also on Exynos5800 SoC based Peach Pi board but was
>>> the problem actually present on this board?
>>>
>>
>> My understanding is that this problem is present in 5420/5422/5800
>> but I have no way to confirm this. I'm just guessing from the info
>> in the ChromiumOS vendor tree.
>>
>> If you look at the commit that added the regulator-locker driver,
>> the test says to be done in a Peach Pi so it seems the problem is
>> not restricted to only Exynos5420:
>>
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ba63620feace4eaed5572ac2e77d8d61754af704
>>  
>>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>>>   (unfortunately Inderpal's email is no longer working). ]
>>>
>>
>> Added Abhilash to cc list as well since he was involved in the Exynos ChromeOS tree.
> 
> Abhilash's email is also no longer valid.. :(
> 

Yes, I noticed that as well :(

>>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
>>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
>>> IOW if the problem exists it is already present in the mainline
>>> kernel.
>>>
>>
>> Ah, I only looked at your patch and I didn't compare the voltage levels
>> in your 1.9 GHz and 2.0 GHz OPPs with the current 1.8 GHz in mainline.
>>
>> I wonder if the voltage levels in your patch are correct then, since the
>> ChromiumOS tree uses a higher voltages for the 1.9 GHz and 2.0 GHz OPPs:
>>
>> /*
>>  * Default ASV table
>>  */
>> static const unsigned int asv_voltage_5420[CPUFREQ_NUM_LEVELS] = {
>> 	1362500,	/* L0  2100 */
>> 	1312500,	/* L1  2000 */
>> 	1275000,	/* L2  1900 */
>> 	1225000,	/* L3  1800 */
>> 	1200000,	/* L4  1700 */
>>
>> This is from https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#175
> 
> I think that the above voltages are original ones for Exynos5420
> (not for Exynos5422/5800).
>

In the ChromiumOS tree, the same CPUFreq driver is used for both the Peach
Pit (Exynos5420) and Peach Pi (Exynos5800) Chromebooks.

> The voltages in my patch were taken from Hardkernel's kernel for
> Odroid-XU3:
> 
> /*
>  * ASV group voltage table
>  */
> static const unsigned int asv_voltage_5422_CA15[CPUFREQ_LEVEL_END_CA15] = {
> ...
> 	1250000,    /* L4  2000 */
> 	1250000,    /* L5  1900 */
> 	1250000,    /* L6  1800 */
> ...
> 
> https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y-android/drivers/cpufreq/exynos5422-eagle-cpufreq.c#L314
>

In general I prefer to use the ChromiumOS tree as a reference instead of the
HardKernel one, since the former is in a much better shape IMHO.

I think is worth to check in a kernel tree in http://opensource.samsung.com/
for a product that's Exynos5422/5800 based.

> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 

Best regards,
Bartlomiej Zolnierkiewicz Dec. 14, 2016, 4:10 p.m. UTC | #6
Hi,

On Wednesday, December 14, 2016 11:40:08 AM Javier Martinez Canillas wrote:
> Hello Bartlomiej,
> 
> On 12/14/2016 11:25 AM, Bartlomiej Zolnierkiewicz wrote:
> > 
> > Hi,
> > 
> > On Wednesday, December 14, 2016 11:06:45 AM Javier Martinez Canillas wrote:
> >>
> >> Hello Bartlomiej,
> >>
> >> On 12/14/2016 10:28 AM, Bartlomiej Zolnierkiewicz wrote:
> >>>
> >>> On Tuesday, December 13, 2016 04:18:05 PM Javier Martinez Canillas wrote:
> >>>> Hello Bartlomiej,
> >>>
> >>> Hi,
> >>>
> >>>> On 12/13/2016 01:52 PM, Bartlomiej Zolnierkiewicz wrote:
> >>>>> Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
> >>>>> (for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
> >>>>> cooling maps to account for new OPPs.
> >>>>>
> >>>>> Since new OPPs are not available on all Exynos5422/5800 boards modify
> >>>>> dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
> >>>>> Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
> >>>>>
> >>>>> Tested on Odroid-XU3 and XU3 Lite.
> >>>>>
> >>>>> Cc: Doug Anderson <dianders@chromium.org>
> >>>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> >>>>> Cc: Andreas Faerber <afaerber@suse.de>
> >>>>> Cc: Thomas Abraham <thomas.ab@samsung.com>
> >>>>> Cc: Ben Gamari <ben@smart-cactus.org>
> >>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> >>>>> ---
> >>>>>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
> >>>>>  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
> >>>>>  arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
> >>>>>  arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
> >>>>>  4 files changed, 43 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> >>>>> ===================================================================
> >>>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.779763261 +0100
> >>>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.775763261 +0100
> >>>>> @@ -118,7 +118,7 @@
> >>>>>  				/*
> >>>>>  				 * When reaching cpu_alert3, reduce CPU
> >>>>>  				 * by 2 steps. On Exynos5422/5800 that would
> >>>>> -				 * be: 1600 MHz and 1100 MHz.
> >>>>> +				 * (usually) be: 1800 MHz and 1200 MHz.
> >>>>>  				 */
> >>>>>  				map3 {
> >>>>>  					trip = <&cpu_alert3>;
> >>>>> @@ -131,16 +131,16 @@
> >>>>>  
> >>>>>  				/*
> >>>>>  				 * When reaching cpu_alert4, reduce CPU
> >>>>> -				 * further, down to 600 MHz (11 steps for big,
> >>>>> -				 * 7 steps for LITTLE).
> >>>>> +				 * further, down to 600 MHz (13 steps for big,
> >>>>> +				 * 8 steps for LITTLE).
> >>>>>  				 */
> >>>>> -				map5 {
> >>>>> +				cooling_map5: map5 {
> >>>>>  					trip = <&cpu_alert4>;
> >>>>> -					cooling-device = <&cpu0 3 7>;
> >>>>> +					cooling-device = <&cpu0 3 8>;
> >>>>>  				};
> >>>>> -				map6 {
> >>>>> +				cooling_map6: map6 {
> >>>>>  					trip = <&cpu_alert4>;
> >>>>> -					cooling-device = <&cpu4 3 11>;
> >>>>> +					cooling-device = <&cpu4 3 13>;
> >>>>>  				};
> >>>>>  			};
> >>>>>  		};
> >>>>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
> >>>>> ===================================================================
> >>>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.779763261 +0100
> >>>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.775763261 +0100
> >>>>> @@ -21,6 +21,23 @@
> >>>>>  	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
> >>>>>  };
> >>>>>  
> >>>>> +&cluster_a15_opp_table {
> >>>>> +	/delete-node/opp@2000000000;
> >>>>> +	/delete-node/opp@1900000000;
> >>>>> +};
> >>>>> +
> >>>>> +&cluster_a7_opp_table {
> >>>>> +	/delete-node/opp@1400000000;
> >>>>> +};
> >>>>> +
> >>>>
> >>>> I think that a comment in the DTS why these operating points aren't available
> >>>> in this board will make more clear why the nodes are being deleted.
> >>>
> >>> Ok, I will add these comments in the next patch revision.
> >>>
> >>>>> +&cooling_map5 {
> >>>>> +	cooling-device = <&cpu0 3 7>;
> >>>>> +};
> >>>>> +
> >>>>> +&cooling_map6 {
> >>>>> +	cooling-device = <&cpu4 3 11>;
> >>>>> +};
> >>>>> +
> >>>>>  &pwm {
> >>>>>  	/*
> >>>>>  	 * PWM 0 -- fan
> >>>>> Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> >>>>> ===================================================================
> >>>>> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> >>>>> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> >>>>> @@ -146,6 +146,10 @@
> >>>>>  	vdd-supply = <&ldo9_reg>;
> >>>>>  };
> >>>>>  
> >>>>> +&cluster_a7_opp_table {
> >>>>> +	/delete-property/opp@1400000000;
> >>>>> +};
> >>>>> +
> >>>>>  &cpu0 {
> >>>>>  	cpu-supply = <&buck2_reg>;
> >>>>>  };
> >>>>> Index: b/arch/arm/boot/dts/exynos5800.dtsi
> >>>>> ===================================================================
> >>>>> --- a/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> >>>>> +++ b/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> >>>>> @@ -24,6 +24,16 @@
> >>>>>  };
> >>>>>  
> >>>>>  &cluster_a15_opp_table {
> >>>>> +	opp@2000000000 {
> >>>>> +		opp-hz = /bits/ 64 <2000000000>;
> >>>>> +		opp-microvolt = <1250000>;
> >>>>> +		clock-latency-ns = <140000>;
> >>>>> +	};
> >>>>> +	opp@1900000000 {
> >>>>> +		opp-hz = /bits/ 64 <1900000000>;
> >>>>> +		opp-microvolt = <1250000>;
> >>>>> +		clock-latency-ns = <140000>;
> >>>>> +	};
> >>>>>  	opp@1700000000 {
> >>>>>  		opp-microvolt = <1250000>;
> >>>>>  	};
> >>>>> @@ -85,6 +95,11 @@
> >>>>>  };
> >>>>>
> >>>>
> >>>> AFAIK Thomas restricted the maximum OPP, because for A15 freqs > 1.8GHz the
> >>>> INT rail would need to be scaled up as well since there's a maximum voltage
> >>>> difference between the ARM and INT rails before the system becomes unstable:
> >>>>
> >>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/276766.html
> >>>> https://lkml.org/lkml/2014/5/2/419
> >>>>
> >>>> The ChromiumOS vendor tree uses a virtual regulator driver that makes sure
> >>>> the maximum voltage skew is between a limit. But that never made to mainline:
> >>>>
> >>>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/boot/dts/exynos5420-peach-pit.dtsi#90
> >>>> https://lkml.org/lkml/2014/4/29/28
> >>>>
> >>>> Did that change and there's infrastructure in mainline now to cope with that?
> >>>> If that's the case, I think it would be good to mention in the commit message.
> >>>
> >>> I was not aware of this limitation and AFAIK mainline has currently
> >>> no code to handle it.  I also cannot find any code to handle this in
> >>
> >> Yes, that's what I thought as well but maybe I was missing something.
> >>
> >>> Hardkernel's vendor kernel for Odroid-XU3 board.
> >>>
> >>> Do you know whether this problem exists also on Exynos5422/5800
> >>> SoCs or only on Exynos5420 one?  I see that ChromiumOS uses virtual
> >>> regulator code also on Exynos5800 SoC based Peach Pi board but was
> >>> the problem actually present on this board?
> >>>
> >>
> >> My understanding is that this problem is present in 5420/5422/5800
> >> but I have no way to confirm this. I'm just guessing from the info
> >> in the ChromiumOS vendor tree.
> >>
> >> If you look at the commit that added the regulator-locker driver,
> >> the test says to be done in a Peach Pi so it seems the problem is
> >> not restricted to only Exynos5420:
> >>
> >> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ba63620feace4eaed5572ac2e77d8d61754af704
> >>  
> >>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
> >>>   (unfortunately Inderpal's email is no longer working). ]
> >>>
> >>
> >> Added Abhilash to cc list as well since he was involved in the Exynos ChromeOS tree.
> > 
> > Abhilash's email is also no longer valid.. :(
> > 
> 
> Yes, I noticed that as well :(
> 
> >>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
> >>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
> >>> IOW if the problem exists it is already present in the mainline
> >>> kernel.
> >>>
> >>
> >> Ah, I only looked at your patch and I didn't compare the voltage levels
> >> in your 1.9 GHz and 2.0 GHz OPPs with the current 1.8 GHz in mainline.
> >>
> >> I wonder if the voltage levels in your patch are correct then, since the
> >> ChromiumOS tree uses a higher voltages for the 1.9 GHz and 2.0 GHz OPPs:
> >>
> >> /*
> >>  * Default ASV table
> >>  */
> >> static const unsigned int asv_voltage_5420[CPUFREQ_NUM_LEVELS] = {
> >> 	1362500,	/* L0  2100 */
> >> 	1312500,	/* L1  2000 */
> >> 	1275000,	/* L2  1900 */
> >> 	1225000,	/* L3  1800 */
> >> 	1200000,	/* L4  1700 */
> >>
> >> This is from https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#175
> > 
> > I think that the above voltages are original ones for Exynos5420
> > (not for Exynos5422/5800).
> >
> 
> In the ChromiumOS tree, the same CPUFreq driver is used for both the Peach
> Pit (Exynos5420) and Peach Pi (Exynos5800) Chromebooks.

This seems suboptimal (or maybe even incorrect as Exynos5422/5800
SoCs also use different clock divider values for clocks related to
CPU clock than Exynos5420 SoC).

Anyway, I can drop Peach Pi support from my patch for now if you want.

> > The voltages in my patch were taken from Hardkernel's kernel for
> > Odroid-XU3:
> > 
> > /*
> >  * ASV group voltage table
> >  */
> > static const unsigned int asv_voltage_5422_CA15[CPUFREQ_LEVEL_END_CA15] = {
> > ...
> > 	1250000,    /* L4  2000 */
> > 	1250000,    /* L5  1900 */
> > 	1250000,    /* L6  1800 */
> > ...
> > 
> > https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y-android/drivers/cpufreq/exynos5422-eagle-cpufreq.c#L314
> >
> 
> In general I prefer to use the ChromiumOS tree as a reference instead of the
> HardKernel one, since the former is in a much better shape IMHO.

I generally agree, OTOH Hardkernel's tree is based on Samsung's
vendor trees and additionally it contains all low-level hardware
details for Odroid-XU3 board (not present in ChromiumOS tree).

> I think is worth to check in a kernel tree in http://opensource.samsung.com/
> for a product that's Exynos5422/5800 based.

I've just checked kernel from SM-G900H_LL_Opensource.zip (for Galaxy
S5 which is Exynos5422 based) and it uses 50mV lower voltages for
1.6 GHz - 2.0 GHz OPPs, for the lower frequencies OPPs voltages are
identical to the ones used by HardKernel's tree,

drivers/cpufreq/exynos5422-eagle-cpufreq.c:

/*
 * ASV group voltage table
 */
static const unsigned int asv_voltage_5422_CA15[CPUFREQ_LEVEL_END_CA15] = {
...
        1200000,    /* L4  2000 */
        1200000,    /* L5  1900 */
        1200000,    /* L6  1800 */
        1200000,    /* L7  1700 */
        1200000,    /* L8  1600 */
        1100000,    /* L9  1500 */
        1100000,    /* L10 1400 */
        1100000,    /* L11 1300 */
        1000000,    /* L12 1200 */
        1000000,    /* L13 1100 */
        1000000,    /* L14 1000 */
        1000000,    /* L15  900 */
         900000,    /* L16  800 */
         900000,    /* L17  700 */
         900000,    /* L18  600 */
         900000,    /* L19  500 */
         900000,    /* L20  400 */
         900000,    /* L22  300 */
         900000,    /* L22  200 */
};

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Dec. 14, 2016, 5:19 p.m. UTC | #7
Hello Bartlomiej,

On 12/14/2016 01:10 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Wednesday, December 14, 2016 11:40:08 AM Javier Martinez Canillas wrote:
>> Hello Bartlomiej,
>>
>> On 12/14/2016 11:25 AM, Bartlomiej Zolnierkiewicz wrote:
>>>
>>> Hi,
>>>
>>> On Wednesday, December 14, 2016 11:06:45 AM Javier Martinez Canillas wrote:
>>>>
>>>> Hello Bartlomiej,
>>>>
>>>> On 12/14/2016 10:28 AM, Bartlomiej Zolnierkiewicz wrote:
>>>>>
>>>>> On Tuesday, December 13, 2016 04:18:05 PM Javier Martinez Canillas wrote:
>>>>>> Hello Bartlomiej,
>>>>>
>>>>> Hi,
>>>>>
>>>>>> On 12/13/2016 01:52 PM, Bartlomiej Zolnierkiewicz wrote:
>>>>>>> Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
>>>>>>> (for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
>>>>>>> cooling maps to account for new OPPs.
>>>>>>>
>>>>>>> Since new OPPs are not available on all Exynos5422/5800 boards modify
>>>>>>> dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
>>>>>>> Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
>>>>>>>
>>>>>>> Tested on Odroid-XU3 and XU3 Lite.
>>>>>>>
>>>>>>> Cc: Doug Anderson <dianders@chromium.org>
>>>>>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>>>>>>> Cc: Andreas Faerber <afaerber@suse.de>
>>>>>>> Cc: Thomas Abraham <thomas.ab@samsung.com>
>>>>>>> Cc: Ben Gamari <ben@smart-cactus.org>
>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>>>>>> ---
>>>>>>>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
>>>>>>>  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
>>>>>>>  arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
>>>>>>>  arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
>>>>>>>  4 files changed, 43 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>>>>>>> ===================================================================
>>>>>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.779763261 +0100
>>>>>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.775763261 +0100
>>>>>>> @@ -118,7 +118,7 @@
>>>>>>>  				/*
>>>>>>>  				 * When reaching cpu_alert3, reduce CPU
>>>>>>>  				 * by 2 steps. On Exynos5422/5800 that would
>>>>>>> -				 * be: 1600 MHz and 1100 MHz.
>>>>>>> +				 * (usually) be: 1800 MHz and 1200 MHz.
>>>>>>>  				 */
>>>>>>>  				map3 {
>>>>>>>  					trip = <&cpu_alert3>;
>>>>>>> @@ -131,16 +131,16 @@
>>>>>>>  
>>>>>>>  				/*
>>>>>>>  				 * When reaching cpu_alert4, reduce CPU
>>>>>>> -				 * further, down to 600 MHz (11 steps for big,
>>>>>>> -				 * 7 steps for LITTLE).
>>>>>>> +				 * further, down to 600 MHz (13 steps for big,
>>>>>>> +				 * 8 steps for LITTLE).
>>>>>>>  				 */
>>>>>>> -				map5 {
>>>>>>> +				cooling_map5: map5 {
>>>>>>>  					trip = <&cpu_alert4>;
>>>>>>> -					cooling-device = <&cpu0 3 7>;
>>>>>>> +					cooling-device = <&cpu0 3 8>;
>>>>>>>  				};
>>>>>>> -				map6 {
>>>>>>> +				cooling_map6: map6 {
>>>>>>>  					trip = <&cpu_alert4>;
>>>>>>> -					cooling-device = <&cpu4 3 11>;
>>>>>>> +					cooling-device = <&cpu4 3 13>;
>>>>>>>  				};
>>>>>>>  			};
>>>>>>>  		};
>>>>>>> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
>>>>>>> ===================================================================
>>>>>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.779763261 +0100
>>>>>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.775763261 +0100
>>>>>>> @@ -21,6 +21,23 @@
>>>>>>>  	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
>>>>>>>  };
>>>>>>>  
>>>>>>> +&cluster_a15_opp_table {
>>>>>>> +	/delete-node/opp@2000000000;
>>>>>>> +	/delete-node/opp@1900000000;
>>>>>>> +};
>>>>>>> +
>>>>>>> +&cluster_a7_opp_table {
>>>>>>> +	/delete-node/opp@1400000000;
>>>>>>> +};
>>>>>>> +
>>>>>>
>>>>>> I think that a comment in the DTS why these operating points aren't available
>>>>>> in this board will make more clear why the nodes are being deleted.
>>>>>
>>>>> Ok, I will add these comments in the next patch revision.
>>>>>
>>>>>>> +&cooling_map5 {
>>>>>>> +	cooling-device = <&cpu0 3 7>;
>>>>>>> +};
>>>>>>> +
>>>>>>> +&cooling_map6 {
>>>>>>> +	cooling-device = <&cpu4 3 11>;
>>>>>>> +};
>>>>>>> +
>>>>>>>  &pwm {
>>>>>>>  	/*
>>>>>>>  	 * PWM 0 -- fan
>>>>>>> Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>>>>>>> ===================================================================
>>>>>>> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
>>>>>>> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
>>>>>>> @@ -146,6 +146,10 @@
>>>>>>>  	vdd-supply = <&ldo9_reg>;
>>>>>>>  };
>>>>>>>  
>>>>>>> +&cluster_a7_opp_table {
>>>>>>> +	/delete-property/opp@1400000000;
>>>>>>> +};
>>>>>>> +
>>>>>>>  &cpu0 {
>>>>>>>  	cpu-supply = <&buck2_reg>;
>>>>>>>  };
>>>>>>> Index: b/arch/arm/boot/dts/exynos5800.dtsi
>>>>>>> ===================================================================
>>>>>>> --- a/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
>>>>>>> +++ b/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
>>>>>>> @@ -24,6 +24,16 @@
>>>>>>>  };
>>>>>>>  
>>>>>>>  &cluster_a15_opp_table {
>>>>>>> +	opp@2000000000 {
>>>>>>> +		opp-hz = /bits/ 64 <2000000000>;
>>>>>>> +		opp-microvolt = <1250000>;
>>>>>>> +		clock-latency-ns = <140000>;
>>>>>>> +	};
>>>>>>> +	opp@1900000000 {
>>>>>>> +		opp-hz = /bits/ 64 <1900000000>;
>>>>>>> +		opp-microvolt = <1250000>;
>>>>>>> +		clock-latency-ns = <140000>;
>>>>>>> +	};
>>>>>>>  	opp@1700000000 {
>>>>>>>  		opp-microvolt = <1250000>;
>>>>>>>  	};
>>>>>>> @@ -85,6 +95,11 @@
>>>>>>>  };
>>>>>>>
>>>>>>
>>>>>> AFAIK Thomas restricted the maximum OPP, because for A15 freqs > 1.8GHz the
>>>>>> INT rail would need to be scaled up as well since there's a maximum voltage
>>>>>> difference between the ARM and INT rails before the system becomes unstable:
>>>>>>
>>>>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/276766.html
>>>>>> https://lkml.org/lkml/2014/5/2/419
>>>>>>
>>>>>> The ChromiumOS vendor tree uses a virtual regulator driver that makes sure
>>>>>> the maximum voltage skew is between a limit. But that never made to mainline:
>>>>>>
>>>>>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/boot/dts/exynos5420-peach-pit.dtsi#90
>>>>>> https://lkml.org/lkml/2014/4/29/28
>>>>>>
>>>>>> Did that change and there's infrastructure in mainline now to cope with that?
>>>>>> If that's the case, I think it would be good to mention in the commit message.
>>>>>
>>>>> I was not aware of this limitation and AFAIK mainline has currently
>>>>> no code to handle it.  I also cannot find any code to handle this in
>>>>
>>>> Yes, that's what I thought as well but maybe I was missing something.
>>>>
>>>>> Hardkernel's vendor kernel for Odroid-XU3 board.
>>>>>
>>>>> Do you know whether this problem exists also on Exynos5422/5800
>>>>> SoCs or only on Exynos5420 one?  I see that ChromiumOS uses virtual
>>>>> regulator code also on Exynos5800 SoC based Peach Pi board but was
>>>>> the problem actually present on this board?
>>>>>
>>>>
>>>> My understanding is that this problem is present in 5420/5422/5800
>>>> but I have no way to confirm this. I'm just guessing from the info
>>>> in the ChromiumOS vendor tree.
>>>>
>>>> If you look at the commit that added the regulator-locker driver,
>>>> the test says to be done in a Peach Pi so it seems the problem is
>>>> not restricted to only Exynos5420:
>>>>
>>>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ba63620feace4eaed5572ac2e77d8d61754af704
>>>>  
>>>>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>>>>>   (unfortunately Inderpal's email is no longer working). ]
>>>>>
>>>>
>>>> Added Abhilash to cc list as well since he was involved in the Exynos ChromeOS tree.
>>>
>>> Abhilash's email is also no longer valid.. :(
>>>
>>
>> Yes, I noticed that as well :(
>>
>>>>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
>>>>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
>>>>> IOW if the problem exists it is already present in the mainline
>>>>> kernel.
>>>>>
>>>>
>>>> Ah, I only looked at your patch and I didn't compare the voltage levels
>>>> in your 1.9 GHz and 2.0 GHz OPPs with the current 1.8 GHz in mainline.
>>>>
>>>> I wonder if the voltage levels in your patch are correct then, since the
>>>> ChromiumOS tree uses a higher voltages for the 1.9 GHz and 2.0 GHz OPPs:
>>>>
>>>> /*
>>>>  * Default ASV table
>>>>  */
>>>> static const unsigned int asv_voltage_5420[CPUFREQ_NUM_LEVELS] = {
>>>> 	1362500,	/* L0  2100 */
>>>> 	1312500,	/* L1  2000 */
>>>> 	1275000,	/* L2  1900 */
>>>> 	1225000,	/* L3  1800 */
>>>> 	1200000,	/* L4  1700 */
>>>>
>>>> This is from https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#175
>>>
>>> I think that the above voltages are original ones for Exynos5420
>>> (not for Exynos5422/5800).
>>>
>>
>> In the ChromiumOS tree, the same CPUFreq driver is used for both the Peach
>> Pit (Exynos5420) and Peach Pi (Exynos5800) Chromebooks.
> 
> This seems suboptimal (or maybe even incorrect as Exynos5422/5800
> SoCs also use different clock divider values for clocks related to
> CPU clock than Exynos5420 SoC).
>

Yes, I know. There's lot of if (soc_is_exynos5420()) and if (soc_is_exynos5422())
checks in the driver though so I guess those differences are taken into account.

I haven't reviewed that driver in detail though so maybe something is missing.

> Anyway, I can drop Peach Pi support from my patch for now if you want.
>

I'm not against adding all the missing operating points btw, I'm just trying to
make sure that's safe to do so in mainline.

>>> The voltages in my patch were taken from Hardkernel's kernel for
>>> Odroid-XU3:
>>>
>>> /*
>>>  * ASV group voltage table
>>>  */
>>> static const unsigned int asv_voltage_5422_CA15[CPUFREQ_LEVEL_END_CA15] = {
>>> ...
>>> 	1250000,    /* L4  2000 */
>>> 	1250000,    /* L5  1900 */
>>> 	1250000,    /* L6  1800 */
>>> ...
>>>
>>> https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y-android/drivers/cpufreq/exynos5422-eagle-cpufreq.c#L314
>>>
>>
>> In general I prefer to use the ChromiumOS tree as a reference instead of the
>> HardKernel one, since the former is in a much better shape IMHO.
> 
> I generally agree, OTOH Hardkernel's tree is based on Samsung's
> vendor trees and additionally it contains all low-level hardware
> details for Odroid-XU3 board (not present in ChromiumOS tree).
>

Fair enough.

>> I think is worth to check in a kernel tree in http://opensource.samsung.com/
>> for a product that's Exynos5422/5800 based.
> 
> I've just checked kernel from SM-G900H_LL_Opensource.zip (for Galaxy
> S5 which is Exynos5422 based) and it uses 50mV lower voltages for
> 1.6 GHz - 2.0 GHz OPPs, for the lower frequencies OPPs voltages are
> identical to the ones used by HardKernel's tree,
>

Interesting, thanks a lot for checking. So if the voltage levels for 1.9 GHz
and 2.0 GHz are the same than 1.8 GHz, then I agree with you that either is
safe to add these OPPs in mainline or the problem is already present there.

Best regards,
Krzysztof Kozlowski Dec. 14, 2016, 6:20 p.m. UTC | #8
On Tue, Dec 13, 2016 at 04:18:05PM -0300, Javier Martinez Canillas wrote:
> Hello Bartlomiej,
> 
> On 12/13/2016 01:52 PM, Bartlomiej Zolnierkiewicz wrote:
> > Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
> > (for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
> > cooling maps to account for new OPPs.
> > 
> > Since new OPPs are not available on all Exynos5422/5800 boards modify
> > dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
> > Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
> > 
> > Tested on Odroid-XU3 and XU3 Lite.
> > 
> > Cc: Doug Anderson <dianders@chromium.org>
> > Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> > Cc: Andreas Faerber <afaerber@suse.de>
> > Cc: Thomas Abraham <thomas.ab@samsung.com>
> > Cc: Ben Gamari <ben@smart-cactus.org>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
> >  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
> >  arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
> >  arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
> >  4 files changed, 43 insertions(+), 7 deletions(-)
> > 
> > Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.779763261 +0100
> > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.775763261 +0100
> > @@ -118,7 +118,7 @@
> >  				/*
> >  				 * When reaching cpu_alert3, reduce CPU
> >  				 * by 2 steps. On Exynos5422/5800 that would
> > -				 * be: 1600 MHz and 1100 MHz.
> > +				 * (usually) be: 1800 MHz and 1200 MHz.
> >  				 */
> >  				map3 {
> >  					trip = <&cpu_alert3>;
> > @@ -131,16 +131,16 @@
> >  
> >  				/*
> >  				 * When reaching cpu_alert4, reduce CPU
> > -				 * further, down to 600 MHz (11 steps for big,
> > -				 * 7 steps for LITTLE).
> > +				 * further, down to 600 MHz (13 steps for big,
> > +				 * 8 steps for LITTLE).
> >  				 */
> > -				map5 {
> > +				cooling_map5: map5 {
> >  					trip = <&cpu_alert4>;
> > -					cooling-device = <&cpu0 3 7>;
> > +					cooling-device = <&cpu0 3 8>;
> >  				};
> > -				map6 {
> > +				cooling_map6: map6 {
> >  					trip = <&cpu_alert4>;
> > -					cooling-device = <&cpu4 3 11>;
> > +					cooling-device = <&cpu4 3 13>;
> >  				};
> >  			};
> >  		};
> > Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.779763261 +0100
> > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.775763261 +0100
> > @@ -21,6 +21,23 @@
> >  	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
> >  };
> >  
> > +&cluster_a15_opp_table {
> > +	/delete-node/opp@2000000000;
> > +	/delete-node/opp@1900000000;
> > +};
> > +
> > +&cluster_a7_opp_table {
> > +	/delete-node/opp@1400000000;
> > +};
> > +
> 
> I think that a comment in the DTS why these operating points aren't available
> in this board will make more clear why the nodes are being deleted.
> 
> > +&cooling_map5 {
> > +	cooling-device = <&cpu0 3 7>;
> > +};
> > +
> > +&cooling_map6 {
> > +	cooling-device = <&cpu4 3 11>;
> > +};
> > +
> >  &pwm {
> >  	/*
> >  	 * PWM 0 -- fan
> > Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> > +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
> > @@ -146,6 +146,10 @@
> >  	vdd-supply = <&ldo9_reg>;
> >  };
> >  
> > +&cluster_a7_opp_table {
> > +	/delete-property/opp@1400000000;
> > +};
> > +
> >  &cpu0 {
> >  	cpu-supply = <&buck2_reg>;
> >  };
> > Index: b/arch/arm/boot/dts/exynos5800.dtsi
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> > +++ b/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
> > @@ -24,6 +24,16 @@
> >  };
> >  
> >  &cluster_a15_opp_table {
> > +	opp@2000000000 {
> > +		opp-hz = /bits/ 64 <2000000000>;
> > +		opp-microvolt = <1250000>;
> > +		clock-latency-ns = <140000>;
> > +	};
> > +	opp@1900000000 {
> > +		opp-hz = /bits/ 64 <1900000000>;
> > +		opp-microvolt = <1250000>;
> > +		clock-latency-ns = <140000>;
> > +	};
> >  	opp@1700000000 {
> >  		opp-microvolt = <1250000>;
> >  	};
> > @@ -85,6 +95,11 @@
> >  };
> >
> 
> AFAIK Thomas restricted the maximum OPP, because for A15 freqs > 1.8GHz the
> INT rail would need to be scaled up as well since there's a maximum voltage
> difference between the ARM and INT rails before the system becomes unstable:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/276766.html
> https://lkml.org/lkml/2014/5/2/419

The choice of skipping > 1.8 GHz could be also made because of missing
ASV which is important to detect the BIN2 of Exynos5422. The BIN2 is
capped to 1.8/1.3.

Anyway this shouldn't be limiting us now because we have all data
statically coded in DTS - in mainline boards, only Odroid XU3-lite
uses BIN2.

Beside comments from Javier, I would be happy to see high frequencies
Tested-by on Peach Pi/Pit. AFAIR, we did not have these in SRPOL. :)

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Anderson Dec. 16, 2016, 12:52 a.m. UTC | #9
Hi,

On Wed, Dec 14, 2016 at 5:28 AM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
> On Tuesday, December 13, 2016 04:18:05 PM Javier Martinez Canillas wrote:
>> Hello Bartlomiej,
>
> Hi,
>
>> On 12/13/2016 01:52 PM, Bartlomiej Zolnierkiewicz wrote:
>> > Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
>> > (for A7 cores).  Also update common Odroid-XU3 Lite/XU3/XU4 thermal
>> > cooling maps to account for new OPPs.
>> >
>> > Since new OPPs are not available on all Exynos5422/5800 boards modify
>> > dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
>> > Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
>> >
>> > Tested on Odroid-XU3 and XU3 Lite.
>> >
>> > Cc: Doug Anderson <dianders@chromium.org>
>> > Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>> > Cc: Andreas Faerber <afaerber@suse.de>
>> > Cc: Thomas Abraham <thomas.ab@samsung.com>
>> > Cc: Ben Gamari <ben@smart-cactus.org>
>> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> > ---
>> >  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |   14 +++++++-------
>> >  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |   17 +++++++++++++++++
>> >  arch/arm/boot/dts/exynos5800-peach-pi.dts          |    4 ++++
>> >  arch/arm/boot/dts/exynos5800.dtsi                  |   15 +++++++++++++++
>> >  4 files changed, 43 insertions(+), 7 deletions(-)
>> >
>> > Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>> > ===================================================================
>> > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi    2016-12-13 15:59:33.779763261 +0100
>> > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi    2016-12-13 15:59:33.775763261 +0100
>> > @@ -118,7 +118,7 @@
>> >                             /*
>> >                              * When reaching cpu_alert3, reduce CPU
>> >                              * by 2 steps. On Exynos5422/5800 that would
>> > -                            * be: 1600 MHz and 1100 MHz.
>> > +                            * (usually) be: 1800 MHz and 1200 MHz.
>> >                              */
>> >                             map3 {
>> >                                     trip = <&cpu_alert3>;
>> > @@ -131,16 +131,16 @@
>> >
>> >                             /*
>> >                              * When reaching cpu_alert4, reduce CPU
>> > -                            * further, down to 600 MHz (11 steps for big,
>> > -                            * 7 steps for LITTLE).
>> > +                            * further, down to 600 MHz (13 steps for big,
>> > +                            * 8 steps for LITTLE).
>> >                              */
>> > -                           map5 {
>> > +                           cooling_map5: map5 {
>> >                                     trip = <&cpu_alert4>;
>> > -                                   cooling-device = <&cpu0 3 7>;
>> > +                                   cooling-device = <&cpu0 3 8>;
>> >                             };
>> > -                           map6 {
>> > +                           cooling_map6: map6 {
>> >                                     trip = <&cpu_alert4>;
>> > -                                   cooling-device = <&cpu4 3 11>;
>> > +                                   cooling-device = <&cpu4 3 13>;
>> >                             };
>> >                     };
>> >             };
>> > Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
>> > ===================================================================
>> > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts       2016-12-13 15:59:33.779763261 +0100
>> > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts       2016-12-13 15:59:33.775763261 +0100
>> > @@ -21,6 +21,23 @@
>> >     compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
>> >  };
>> >
>> > +&cluster_a15_opp_table {
>> > +   /delete-node/opp@2000000000;
>> > +   /delete-node/opp@1900000000;
>> > +};
>> > +
>> > +&cluster_a7_opp_table {
>> > +   /delete-node/opp@1400000000;
>> > +};
>> > +
>>
>> I think that a comment in the DTS why these operating points aren't available
>> in this board will make more clear why the nodes are being deleted.
>
> Ok, I will add these comments in the next patch revision.
>
>> > +&cooling_map5 {
>> > +   cooling-device = <&cpu0 3 7>;
>> > +};
>> > +
>> > +&cooling_map6 {
>> > +   cooling-device = <&cpu4 3 11>;
>> > +};
>> > +
>> >  &pwm {
>> >     /*
>> >      * PWM 0 -- fan
>> > Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
>> > ===================================================================
>> > --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts     2016-12-13 15:59:33.779763261 +0100
>> > +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts     2016-12-13 15:59:33.779763261 +0100
>> > @@ -146,6 +146,10 @@
>> >     vdd-supply = <&ldo9_reg>;
>> >  };
>> >
>> > +&cluster_a7_opp_table {
>> > +   /delete-property/opp@1400000000;
>> > +};
>> > +
>> >  &cpu0 {
>> >     cpu-supply = <&buck2_reg>;
>> >  };
>> > Index: b/arch/arm/boot/dts/exynos5800.dtsi
>> > ===================================================================
>> > --- a/arch/arm/boot/dts/exynos5800.dtsi     2016-12-13 15:59:33.779763261 +0100
>> > +++ b/arch/arm/boot/dts/exynos5800.dtsi     2016-12-13 15:59:33.779763261 +0100
>> > @@ -24,6 +24,16 @@
>> >  };
>> >
>> >  &cluster_a15_opp_table {
>> > +   opp@2000000000 {
>> > +           opp-hz = /bits/ 64 <2000000000>;
>> > +           opp-microvolt = <1250000>;
>> > +           clock-latency-ns = <140000>;
>> > +   };
>> > +   opp@1900000000 {
>> > +           opp-hz = /bits/ 64 <1900000000>;
>> > +           opp-microvolt = <1250000>;
>> > +           clock-latency-ns = <140000>;
>> > +   };
>> >     opp@1700000000 {
>> >             opp-microvolt = <1250000>;
>> >     };
>> > @@ -85,6 +95,11 @@
>> >  };
>> >
>>
>> AFAIK Thomas restricted the maximum OPP, because for A15 freqs > 1.8GHz the
>> INT rail would need to be scaled up as well since there's a maximum voltage
>> difference between the ARM and INT rails before the system becomes unstable:
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/276766.html
>> https://lkml.org/lkml/2014/5/2/419
>>
>> The ChromiumOS vendor tree uses a virtual regulator driver that makes sure
>> the maximum voltage skew is between a limit. But that never made to mainline:
>>
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/boot/dts/exynos5420-peach-pit.dtsi#90
>> https://lkml.org/lkml/2014/4/29/28
>>
>> Did that change and there's infrastructure in mainline now to cope with that?
>> If that's the case, I think it would be good to mention in the commit message.
>
> I was not aware of this limitation and AFAIK mainline has currently
> no code to handle it.  I also cannot find any code to handle this in
> Hardkernel's vendor kernel for Odroid-XU3 board.
>
> Do you know whether this problem exists also on Exynos5422/5800
> SoCs or only on Exynos5420 one?  I see that ChromiumOS uses virtual
> regulator code also on Exynos5800 SoC based Peach Pi board but was
> the problem actually present on this board?

This was a long time ago and my memory is quite fuzzy.  ...and I think
others have answered some of this already...

...but from my memory:

* This problem was said to exist on all Exynos 5420/5422/5800 SoCs.

* Samsung's original proposal included using the QoS subsystem to
enforce these constraints.  I don't know where the Hardkernel source
is offhand, but you could check if that solution was used there?  I
see one comment that links to these CLs:

https://chromium-review.googlesource.com/#/c/187420/
https://chromium-review.googlesource.com/#/c/187231/2
https://chromium-review.googlesource.com/#/c/184439/7
https://chromium-review.googlesource.com/#/c/184460/10
https://chromium-review.googlesource.com/#/c/186804/4
https://chromium-review.googlesource.com/#/c/186805/4
https://chromium-review.googlesource.com/#/c/186806/3
https://chromium-review.googlesource.com/#/c/186353/6

* Before using the voltage locker, we used an interrim solution of
bumping the INT frequency up to 500 MHz.  See
<https://chromium-review.googlesource.com/#/c/187992/> and
<https://chromium-review.googlesource.com/#/c/187888/>.  Perhaps this
is something that's happening upstream?


> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>   (unfortunately Inderpal's email is no longer working). ]
>
> Please also note that on Exynos5422/5800 SoCs the same ARM rail
> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
> IOW if the problem exists it is already present in the mainline
> kernel.

Interesting.  In the ChromeOS tree I see significantly higher voltages
needed...  Note that one might naively look at
<https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#178>.

1362500, /* L0  2100 */
1312500, /* L1  2000 */

..but, amazingly enough those voltages aren't used at all.  Surprise!

I believe that the above numbers are actually not used and the ASV
numbers are used instead.  See
<https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/mach-exynos/include/mach/asv-exynos542x.h#452>

{ 2100000,
1350000, 1350000, 1350000, 1350000, 1350000,
1337500, 1325000, 1312500, 1300000, 1287500,
1275000, 1262500, 1250000, 1237500 },

I believe that interpretation there is: some bins of the CPU can run
at 2.1 GHz just fine at 1.25 V but others need up to 1.35V.


...so if you're running at 2.1 GHz at 1.25V then perhaps you're just
running on a CPU from a nice bin?

---

Anyway, I'm not setup at the moment to do a whole lot on exynos boards
(I'd have to go and dig some out and set them up again), so not sure
I'll be terribly useful in this discussion.  ...but I can try to dig
up history, anyway.


-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Dec. 16, 2016, 7:37 a.m. UTC | #10
On Thu, Dec 15, 2016 at 04:52:58PM -0800, Doug Anderson wrote:
> > [ I added Arjun to Cc:, maybe he can help in explaining this issue
> >   (unfortunately Inderpal's email is no longer working). ]
> >
> > Please also note that on Exynos5422/5800 SoCs the same ARM rail
> > voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
> > IOW if the problem exists it is already present in the mainline
> > kernel.
> 
> Interesting.  In the ChromeOS tree I see significantly higher voltages
> needed...  Note that one might naively look at
> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#178>.
> 
> 1362500, /* L0  2100 */
> 1312500, /* L1  2000 */
> 
> ..but, amazingly enough those voltages aren't used at all.  Surprise!
> 
> I believe that the above numbers are actually not used and the ASV
> numbers are used instead.  See
> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/mach-exynos/include/mach/asv-exynos542x.h#452>
> 
> { 2100000,
> 1350000, 1350000, 1350000, 1350000, 1350000,
> 1337500, 1325000, 1312500, 1300000, 1287500,
> 1275000, 1262500, 1250000, 1237500 },
> 
> I believe that interpretation there is: some bins of the CPU can run
> at 2.1 GHz just fine at 1.25 V but others need up to 1.35V.

That is definitely the case. One could just look at vendors ASV table
(for 1.9 GHz):
{ 1900000, 1300000, 1287500, 1262500, 1237500, 1225000, 1212500,
                    1200000, 1187500, 1175000, 1162500, 1150000,
		             1137500, 1125000, 1112500, 1112500},

The theoretical difference is up to 1.875V! From my experiments I saw
BIN1 chips which should be the same... but some working on 1.2V, some on
1.225V (@1.9 GHz). I didn't see any requiring higher voltages but that
does not mean that there aren't such...

> ...so if you're running at 2.1 GHz at 1.25V then perhaps you're just
> running on a CPU from a nice bin?

Would be nice to see a dump of PKG_ID and AUX_INFO chipid registers
along with name of tested board. Because the "Tested on XU3" is not
sufficient.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Markus Reichl Dec. 16, 2016, 9:08 a.m. UTC | #11
Am 16.12.2016 um 08:37 schrieb Krzysztof Kozlowski:
> On Thu, Dec 15, 2016 at 04:52:58PM -0800, Doug Anderson wrote:
>>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>>>   (unfortunately Inderpal's email is no longer working). ]
>>>
>>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
>>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
>>> IOW if the problem exists it is already present in the mainline
>>> kernel.
>>
>> Interesting.  In the ChromeOS tree I see significantly higher voltages
>> needed...  Note that one might naively look at
>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#178>.
>>
>> 1362500, /* L0  2100 */
>> 1312500, /* L1  2000 */
>>
>> ..but, amazingly enough those voltages aren't used at all.  Surprise!
>>
>> I believe that the above numbers are actually not used and the ASV
>> numbers are used instead.  See
>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/mach-exynos/include/mach/asv-exynos542x.h#452>
>>
>> { 2100000,
>> 1350000, 1350000, 1350000, 1350000, 1350000,
>> 1337500, 1325000, 1312500, 1300000, 1287500,
>> 1275000, 1262500, 1250000, 1237500 },
>>
>> I believe that interpretation there is: some bins of the CPU can run
>> at 2.1 GHz just fine at 1.25 V but others need up to 1.35V.
> 
> That is definitely the case. One could just look at vendors ASV table
> (for 1.9 GHz):
> { 1900000, 1300000, 1287500, 1262500, 1237500, 1225000, 1212500,
>                     1200000, 1187500, 1175000, 1162500, 1150000,
> 		             1137500, 1125000, 1112500, 1112500},
> 
> The theoretical difference is up to 1.875V! From my experiments I saw
> BIN1 chips which should be the same... but some working on 1.2V, some on
> 1.225V (@1.9 GHz). I didn't see any requiring higher voltages but that
> does not mean that there aren't such...
> 
>> ...so if you're running at 2.1 GHz at 1.25V then perhaps you're just
>> running on a CPU from a nice bin?

I've been running the proposed frequency/voltage combinations without any
stability problems on my XU4, XU3 and even XU3-lite ( I did not delete the
nodes on XU3-lite dts) with make -j8 kernel and ssvb-cpuburn.
The chips are poorly cooled, especially the XU4 and quickly step down.

> 
> Would be nice to see a dump of PKG_ID and AUX_INFO chipid registers
> along with name of tested board. Because the "Tested on XU3" is not
> sufficient.

If you point me to how to read these values out, I will publish them.

> 
> Best regards,
> Krzysztof
--
Markus Reichl
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anand Moon Dec. 17, 2016, 7:31 a.m. UTC | #12
Hi Markus,

On 16 December 2016 at 14:38, Markus Reichl <m.reichl@fivetechno.de> wrote:
> Am 16.12.2016 um 08:37 schrieb Krzysztof Kozlowski:
>> On Thu, Dec 15, 2016 at 04:52:58PM -0800, Doug Anderson wrote:
>>>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>>>>   (unfortunately Inderpal's email is no longer working). ]
>>>>
>>>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
>>>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
>>>> IOW if the problem exists it is already present in the mainline
>>>> kernel.
>>>
>>> Interesting.  In the ChromeOS tree I see significantly higher voltages
>>> needed...  Note that one might naively look at
>>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#178>.
>>>
>>> 1362500, /* L0  2100 */
>>> 1312500, /* L1  2000 */
>>>
>>> ..but, amazingly enough those voltages aren't used at all.  Surprise!
>>>
>>> I believe that the above numbers are actually not used and the ASV
>>> numbers are used instead.  See
>>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/mach-exynos/include/mach/asv-exynos542x.h#452>
>>>
>>> { 2100000,
>>> 1350000, 1350000, 1350000, 1350000, 1350000,
>>> 1337500, 1325000, 1312500, 1300000, 1287500,
>>> 1275000, 1262500, 1250000, 1237500 },
>>>
>>> I believe that interpretation there is: some bins of the CPU can run
>>> at 2.1 GHz just fine at 1.25 V but others need up to 1.35V.
>>
>> That is definitely the case. One could just look at vendors ASV table
>> (for 1.9 GHz):
>> { 1900000, 1300000, 1287500, 1262500, 1237500, 1225000, 1212500,
>>                     1200000, 1187500, 1175000, 1162500, 1150000,
>>                            1137500, 1125000, 1112500, 1112500},
>>
>> The theoretical difference is up to 1.875V! From my experiments I saw
>> BIN1 chips which should be the same... but some working on 1.2V, some on
>> 1.225V (@1.9 GHz). I didn't see any requiring higher voltages but that
>> does not mean that there aren't such...
>>
>>> ...so if you're running at 2.1 GHz at 1.25V then perhaps you're just
>>> running on a CPU from a nice bin?
>
> I've been running the proposed frequency/voltage combinations without any
> stability problems on my XU4, XU3 and even XU3-lite ( I did not delete the
> nodes on XU3-lite dts) with make -j8 kernel and ssvb-cpuburn.
> The chips are poorly cooled, especially the XU4 and quickly step down.

[snip]

As per my knowlegde Odroid XU3/4 can throttle at much high temperature.

https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y/drivers/thermal/exynos_thermal.c#L1629

The device tree binding for thermal-zone is kept bit low alert
temperature values
in-order to avoid reaches critical temperature and board shutdown
when compiling the source code. We need t fix this thermal-zone

Their could be some race in thermal or the step wise governor for
exynos is not working correctly.

Better option is to print the cpufreq for cpu0 and cpu4 and respective temp
and plot a graph along timeline. It will give us clear idea on how much
time is spend on high frequency on stress testing.

#!/bin/bash
t=0
while true :
do
 a=`cat /sys/devices/virtual/thermal/thermal_zone0/temp`
 b=`cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq`
 c=`cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq`
 d=`cat /sys/devices/system/cpu/cpu4/cpufreq/scaling_cur_freq`
 e=`cat /sys/devices/system/cpu/cpu4/cpufreq/cpuinfo_cur_freq`
 (( t += 5 ))
 echo $t,$a,$b,$d,$e
 sleep 1
done

Best Regards
-Anand Moon
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alim Akhtar Dec. 19, 2016, 1:35 p.m. UTC | #13
Hi,

On 12/16/2016 01:07 PM, Krzysztof Kozlowski wrote:
> On Thu, Dec 15, 2016 at 04:52:58PM -0800, Doug Anderson wrote:
>>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>>>    (unfortunately Inderpal's email is no longer working). ]
>>>
>>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
>>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
>>> IOW if the problem exists it is already present in the mainline
>>> kernel.
>>
>> Interesting.  In the ChromeOS tree I see significantly higher voltages
>> needed...  Note that one might naively look at
>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#178>.
>>
>> 1362500, /* L0  2100 */
>> 1312500, /* L1  2000 */
>>
>> ..but, amazingly enough those voltages aren't used at all.  Surprise!
>>
>> I believe that the above numbers are actually not used and the ASV
>> numbers are used instead.  See
>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/mach-exynos/include/mach/asv-exynos542x.h#452>
>>
>> { 2100000,
>> 1350000, 1350000, 1350000, 1350000, 1350000,
>> 1337500, 1325000, 1312500, 1300000, 1287500,
>> 1275000, 1262500, 1250000, 1237500 },
>>
>> I believe that interpretation there is: some bins of the CPU can run
>> at 2.1 GHz just fine at 1.25 V but others need up to 1.35V.
>
> That is definitely the case. One could just look at vendors ASV table
> (for 1.9 GHz):
> { 1900000, 1300000, 1287500, 1262500, 1237500, 1225000, 1212500,
>                      1200000, 1187500, 1175000, 1162500, 1150000,
> 		             1137500, 1125000, 1112500, 1112500},
>
> The theoretical difference is up to 1.875V! From my experiments I saw
> BIN1 chips which should be the same... but some working on 1.2V, some on
> 1.225V (@1.9 GHz). I didn't see any requiring higher voltages but that
> does not mean that there aren't such...
>
>> ...so if you're running at 2.1 GHz at 1.25V then perhaps you're just
>> running on a CPU from a nice bin?
>
> Would be nice to see a dump of PKG_ID and AUX_INFO chipid registers
> along with name of tested board. Because the "Tested on XU3" is not
> sufficient.
>
I agree, we should be dumping PKG_ID and other chip info to know on 
which BIN sample this patch is tested on...
As far as Peach-{pit/pi} boards are concerns, this is what I can remember:
1> 5420 (PIT) -> max recommended target frequency is 1800 MHz for A15
2> 5800 (PI)-> max recommended target frequency can go upto 2000 MHz, 
with INT rail locking.
INT rail locking schemes never made to mainline, so to be safer side 
instead of bumping the clock and voltages better to keep it at safer 
range for pit and pi, probably thats why it was kept at 1800MHz.
I am not sure if the same limitation applies to Odroid-XU3 samples.


> Best regards,
> Krzysztof
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
===================================================================
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.779763261 +0100
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi	2016-12-13 15:59:33.775763261 +0100
@@ -118,7 +118,7 @@ 
 				/*
 				 * When reaching cpu_alert3, reduce CPU
 				 * by 2 steps. On Exynos5422/5800 that would
-				 * be: 1600 MHz and 1100 MHz.
+				 * (usually) be: 1800 MHz and 1200 MHz.
 				 */
 				map3 {
 					trip = <&cpu_alert3>;
@@ -131,16 +131,16 @@ 
 
 				/*
 				 * When reaching cpu_alert4, reduce CPU
-				 * further, down to 600 MHz (11 steps for big,
-				 * 7 steps for LITTLE).
+				 * further, down to 600 MHz (13 steps for big,
+				 * 8 steps for LITTLE).
 				 */
-				map5 {
+				cooling_map5: map5 {
 					trip = <&cpu_alert4>;
-					cooling-device = <&cpu0 3 7>;
+					cooling-device = <&cpu0 3 8>;
 				};
-				map6 {
+				cooling_map6: map6 {
 					trip = <&cpu_alert4>;
-					cooling-device = <&cpu4 3 11>;
+					cooling-device = <&cpu4 3 13>;
 				};
 			};
 		};
Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
===================================================================
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.779763261 +0100
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts	2016-12-13 15:59:33.775763261 +0100
@@ -21,6 +21,23 @@ 
 	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
 };
 
+&cluster_a15_opp_table {
+	/delete-node/opp@2000000000;
+	/delete-node/opp@1900000000;
+};
+
+&cluster_a7_opp_table {
+	/delete-node/opp@1400000000;
+};
+
+&cooling_map5 {
+	cooling-device = <&cpu0 3 7>;
+};
+
+&cooling_map6 {
+	cooling-device = <&cpu4 3 11>;
+};
+
 &pwm {
 	/*
 	 * PWM 0 -- fan
Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
===================================================================
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts	2016-12-13 15:59:33.779763261 +0100
@@ -146,6 +146,10 @@ 
 	vdd-supply = <&ldo9_reg>;
 };
 
+&cluster_a7_opp_table {
+	/delete-property/opp@1400000000;
+};
+
 &cpu0 {
 	cpu-supply = <&buck2_reg>;
 };
Index: b/arch/arm/boot/dts/exynos5800.dtsi
===================================================================
--- a/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
+++ b/arch/arm/boot/dts/exynos5800.dtsi	2016-12-13 15:59:33.779763261 +0100
@@ -24,6 +24,16 @@ 
 };
 
 &cluster_a15_opp_table {
+	opp@2000000000 {
+		opp-hz = /bits/ 64 <2000000000>;
+		opp-microvolt = <1250000>;
+		clock-latency-ns = <140000>;
+	};
+	opp@1900000000 {
+		opp-hz = /bits/ 64 <1900000000>;
+		opp-microvolt = <1250000>;
+		clock-latency-ns = <140000>;
+	};
 	opp@1700000000 {
 		opp-microvolt = <1250000>;
 	};
@@ -85,6 +95,11 @@ 
 };
 
 &cluster_a7_opp_table {
+	opp_a7_14: opp@1400000000 {
+		opp-hz = /bits/ 64 <1400000000>;
+		opp-microvolt = <1250000>;
+		clock-latency-ns = <140000>;
+	};
 	opp@1300000000 {
 		opp-microvolt = <1250000>;
 	};