cpufreq: mediatek: Raise proc and sram max voltage for MT7622/7623

  During the addition of SRAM voltage tracking for CCI scaling, this
driver got some voltage limits set for the vtrack algorithm: these
were moved to platform data first, then enforced in a later commit
6a17b3876bc8 ("cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()")
using these as max values for the regulator_set_voltage() calls.

In this case, the vsram/vproc constraints for MT7622 and MT7623
were supposed to be the same as MT2701 (and a number of other SoCs),
but that turned out to be a mistake because the aforementioned two
SoCs' maximum voltage for both VPROC and VPROC_SRAM is 1.36V.

Fix that by adding new platform data for MT7622/7623 declaring the
right {proc,sram}_max_volt parameter.

Fixes: ead858bd128d ("cpufreq: mediatek: Move voltage limits to platform data")
Fixes: 6a17b3876bc8 ("cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
  

Il 02/12/22 10:52, AngeloGioacchino Del Regno ha scritto:
> During the addition of SRAM voltage tracking for CCI scaling, this
> driver got some voltage limits set for the vtrack algorithm: these
> were moved to platform data first, then enforced in a later commit
> 6a17b3876bc8 ("cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()")
> using these as max values for the regulator_set_voltage() calls.
> 
> In this case, the vsram/vproc constraints for MT7622 and MT7623
> were supposed to be the same as MT2701 (and a number of other SoCs),
> but that turned out to be a mistake because the aforementioned two
> SoCs' maximum voltage for both VPROC and VPROC_SRAM is 1.36V.
> 
> Fix that by adding new platform data for MT7622/7623 declaring the
> right {proc,sram}_max_volt parameter.
> 
> Fixes: ead858bd128d ("cpufreq: mediatek: Move voltage limits to platform data")
> Fixes: 6a17b3876bc8 ("cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Hello Vincent,
in regard to the regression that you've (not so) recently reported, can you please
give this commit a try and check if this helps resolving your issue?

Many thanks,
Angelo

> ---
>   drivers/cpufreq/mediatek-cpufreq.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> index 7f2680bc9a0f..f9a9f08c75c4 100644
> --- a/drivers/cpufreq/mediatek-cpufreq.c
> +++ b/drivers/cpufreq/mediatek-cpufreq.c
> @@ -695,6 +695,15 @@ static const struct mtk_cpufreq_platform_data mt2701_platform_data = {
>   	.ccifreq_supported = false,
>   };
>   
> +static const struct mtk_cpufreq_platform_data mt7622_platform_data = {
> +	.min_volt_shift = 100000,
> +	.max_volt_shift = 200000,
> +	.proc_max_volt = 1360000,
> +	.sram_min_volt = 0,
> +	.sram_max_volt = 1360000,
> +	.ccifreq_supported = false,
> +};
> +
>   static const struct mtk_cpufreq_platform_data mt8183_platform_data = {
>   	.min_volt_shift = 100000,
>   	.max_volt_shift = 200000,
> @@ -717,8 +726,8 @@ static const struct mtk_cpufreq_platform_data mt8186_platform_data = {
>   static const struct of_device_id mtk_cpufreq_machines[] __initconst = {
>   	{ .compatible = "mediatek,mt2701", .data = &mt2701_platform_data },
>   	{ .compatible = "mediatek,mt2712", .data = &mt2701_platform_data },
> -	{ .compatible = "mediatek,mt7622", .data = &mt2701_platform_data },
> -	{ .compatible = "mediatek,mt7623", .data = &mt2701_platform_data },
> +	{ .compatible = "mediatek,mt7622", .data = &mt7622_platform_data },
> +	{ .compatible = "mediatek,mt7623", .data = &mt7622_platform_data },
>   	{ .compatible = "mediatek,mt8167", .data = &mt2701_platform_data },
>   	{ .compatible = "mediatek,mt817x", .data = &mt2701_platform_data },
>   	{ .compatible = "mediatek,mt8173", .data = &mt2701_platform_data },
  

It now starts, however, with a lot of those messages (I applied the 
patch to linux/master and not to linux-next, because next is currently 
not compiling anymore for me):

> [   10.777041] cpufreq: __target_index: Failed to change cpu 
> frequency: -22
> [   10.791577] cpu cpu0: cpu0: failed to scale up voltage!
The complete log:
https://gist.githubusercontent.com/PolynomialDivision/267c83c7a21a359cbb4e8d99d0303201/raw/28d3568a26634bebef2d91ebe37fc5f76ae58add/mt7622-patch-cpufreq.log

Bests
Nick

On 12/2/22 10:52, AngeloGioacchino Del Regno wrote:
> During the addition of SRAM voltage tracking for CCI scaling, this
> driver got some voltage limits set for the vtrack algorithm: these
> were moved to platform data first, then enforced in a later commit
> 6a17b3876bc8 ("cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()")
> using these as max values for the regulator_set_voltage() calls.
>
> In this case, the vsram/vproc constraints for MT7622 and MT7623
> were supposed to be the same as MT2701 (and a number of other SoCs),
> but that turned out to be a mistake because the aforementioned two
> SoCs' maximum voltage for both VPROC and VPROC_SRAM is 1.36V.
>
> Fix that by adding new platform data for MT7622/7623 declaring the
> right {proc,sram}_max_volt parameter.
>
> Fixes: ead858bd128d ("cpufreq: mediatek: Move voltage limits to platform data")
> Fixes: 6a17b3876bc8 ("cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>   drivers/cpufreq/mediatek-cpufreq.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> index 7f2680bc9a0f..f9a9f08c75c4 100644
> --- a/drivers/cpufreq/mediatek-cpufreq.c
> +++ b/drivers/cpufreq/mediatek-cpufreq.c
> @@ -695,6 +695,15 @@ static const struct mtk_cpufreq_platform_data mt2701_platform_data = {
>   	.ccifreq_supported = false,
>   };
>   
> +static const struct mtk_cpufreq_platform_data mt7622_platform_data = {
> +	.min_volt_shift = 100000,
> +	.max_volt_shift = 200000,
> +	.proc_max_volt = 1360000,
> +	.sram_min_volt = 0,
> +	.sram_max_volt = 1360000,
> +	.ccifreq_supported = false,
> +};
> +
>   static const struct mtk_cpufreq_platform_data mt8183_platform_data = {
>   	.min_volt_shift = 100000,
>   	.max_volt_shift = 200000,
> @@ -717,8 +726,8 @@ static const struct mtk_cpufreq_platform_data mt8186_platform_data = {
>   static const struct of_device_id mtk_cpufreq_machines[] __initconst = {
>   	{ .compatible = "mediatek,mt2701", .data = &mt2701_platform_data },
>   	{ .compatible = "mediatek,mt2712", .data = &mt2701_platform_data },
> -	{ .compatible = "mediatek,mt7622", .data = &mt2701_platform_data },
> -	{ .compatible = "mediatek,mt7623", .data = &mt2701_platform_data },
> +	{ .compatible = "mediatek,mt7622", .data = &mt7622_platform_data },
> +	{ .compatible = "mediatek,mt7623", .data = &mt7622_platform_data },
>   	{ .compatible = "mediatek,mt8167", .data = &mt2701_platform_data },
>   	{ .compatible = "mediatek,mt817x", .data = &mt2701_platform_data },
>   	{ .compatible = "mediatek,mt8173", .data = &mt2701_platform_data },
  

Mail loop Sean.Wang for MT7622 cpufreq.


On Fri, 2022-12-02 at 13:37 +0100, Nick wrote:
> It now starts, however, with a lot of those messages (I applied the 
> patch to linux/master and not to linux-next, because next is
> currently 
> not compiling anymore for me):
> 
> > [   10.777041] cpufreq: __target_index: Failed to change cpu 
> > frequency: -22

Hi Nick,

The errno occurs when cpufreq platform driver attempts to do
mtk_cpufreq_voltage_tracking() but it is not able to accomplish voltage
scaling within a certain of times, vtrack_max.

It will needs further debug message to figure it out.

> > [   10.791577] cpu cpu0: cpu0: failed to scale up voltage!
> 
> The complete log:
> 
https://urldefense.com/v3/__https://gist.githubusercontent.com/PolynomialDivision/267c83c7a21a359cbb4e8d99d0303201/raw/28d3568a26634bebef2d91ebe37fc5f76ae58add/mt7622-patch-cpufreq.log__;!!CTRNKA9wMg0ARbw!msWJync96Qc5bvMB-5gCpepj-5zYWizO_98rjuaLWc7fSBNSSpPMPvZqq1mprBgRq11GAqYvf4hUzwZkX_IhM6k0$ 
> 

May I know if this failure log can be 100 % reproduced on your
platform?

Thanks.

>  
> 
> Bests
> Nick
> 
> On 12/2/22 10:52, AngeloGioacchino Del Regno wrote:
> > During the addition of SRAM voltage tracking for CCI scaling, this
> > driver got some voltage limits set for the vtrack algorithm: these
> > were moved to platform data first, then enforced in a later commit
> > 6a17b3876bc8 ("cpufreq: mediatek: Refine
> > mtk_cpufreq_voltage_tracking()")
> > using these as max values for the regulator_set_voltage() calls.
> > 
> > In this case, the vsram/vproc constraints for MT7622 and MT7623
> > were supposed to be the same as MT2701 (and a number of other
> > SoCs),
> > but that turned out to be a mistake because the aforementioned two
> > SoCs' maximum voltage for both VPROC and VPROC_SRAM is 1.36V.
> > 
> > Fix that by adding new platform data for MT7622/7623 declaring the
> > right {proc,sram}_max_volt parameter.
> > 
> > Fixes: ead858bd128d ("cpufreq: mediatek: Move voltage limits to
> > platform data")
> > Fixes: 6a17b3876bc8 ("cpufreq: mediatek: Refine
> > mtk_cpufreq_voltage_tracking()")
> > Signed-off-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > ---
> >   drivers/cpufreq/mediatek-cpufreq.c | 13 +++++++++++--
> >   1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/mediatek-cpufreq.c
> > b/drivers/cpufreq/mediatek-cpufreq.c
> > index 7f2680bc9a0f..f9a9f08c75c4 100644
> > --- a/drivers/cpufreq/mediatek-cpufreq.c
> > +++ b/drivers/cpufreq/mediatek-cpufreq.c
> > @@ -695,6 +695,15 @@ static const struct mtk_cpufreq_platform_data
> > mt2701_platform_data = {
> >   	.ccifreq_supported = false,
> >   };
> >   
> > +static const struct mtk_cpufreq_platform_data mt7622_platform_data
> > = {
> > +	.min_volt_shift = 100000,
> > +	.max_volt_shift = 200000,
> > +	.proc_max_volt = 1360000,
> > +	.sram_min_volt = 0,
> > +	.sram_max_volt = 1360000,
> > +	.ccifreq_supported = false,
> > +};
> > +
> >   static const struct mtk_cpufreq_platform_data
> > mt8183_platform_data = {
> >   	.min_volt_shift = 100000,
> >   	.max_volt_shift = 200000,
> > @@ -717,8 +726,8 @@ static const struct mtk_cpufreq_platform_data
> > mt8186_platform_data = {
> >   static const struct of_device_id mtk_cpufreq_machines[]
> > __initconst = {
> >   	{ .compatible = "mediatek,mt2701", .data =
> > &mt2701_platform_data },
> >   	{ .compatible = "mediatek,mt2712", .data =
> > &mt2701_platform_data },
> > -	{ .compatible = "mediatek,mt7622", .data =
> > &mt2701_platform_data },
> > -	{ .compatible = "mediatek,mt7623", .data =
> > &mt2701_platform_data },
> > +	{ .compatible = "mediatek,mt7622", .data =
> > &mt7622_platform_data },
> > +	{ .compatible = "mediatek,mt7623", .data =
> > &mt7622_platform_data },
> >   	{ .compatible = "mediatek,mt8167", .data =
> > &mt2701_platform_data },
> >   	{ .compatible = "mediatek,mt817x", .data =
> > &mt2701_platform_data },
> >   	{ .compatible = "mediatek,mt8173", .data =
> > &mt2701_platform_data },

Hi Angelo,

Thanks for your help.

I also found the similar problem that platforms using incorrect
vsram/vproc constraint platform data.  They are MT8167 and MT8516 which
refer to mt8516.dtsi and the max voltage value of vproc is 1300 mV.

Can you help to fix the correct platform data for MT8167 and MT8516 as
well?

Thanks.
  

Il 02/12/22 13:37, Nick ha scritto:
> It now starts, however, with a lot of those messages (I applied the patch to 
> linux/master and not to linux-next, because next is currently not compiling anymore 
> for me):
> 
>> [   10.777041] cpufreq: __target_index: Failed to change cpu frequency: -22
>> [   10.791577] cpu cpu0: cpu0: failed to scale up voltage!
> The complete log:
> https://gist.githubusercontent.com/PolynomialDivision/267c83c7a21a359cbb4e8d99d0303201/raw/28d3568a26634bebef2d91ebe37fc5f76ae58add/mt7622-patch-cpufreq.log
> 

Thanks for the feedback!

I am totally sure that the platform_data is correct as that's based on datasheets.

Checking mt6380-regulator.c and mt6380.dtsi also confirms that the min/max volt
shift and proc/sram max voltage that I've specified is currently supported.

That "failed to scale up voltage!" is a bit strange, as this means that
the regulator_set_voltage() call returns -EINVAL for .. reasons, and that's a bit
strange, since the constraints look good in the code, unless there's anything that
I'm missing here.

Can you please try a vanilla kernel?

Also, since I don't have any MT7622/7623 board to test with, if the issue persists,
it would be helpful if you could place some debugging prints in mediatek-cpufreq.c
to specifically check which regulator_set_voltage() call fails, as to try to make
me have a better overview on the problem that you're facing.

Alternatively, we can eventually setup a debugging session on IRC to make things
a bit faster.

Cheers,
Angelo

> Bests
> Nick
> 
> On 12/2/22 10:52, AngeloGioacchino Del Regno wrote:
>> During the addition of SRAM voltage tracking for CCI scaling, this
>> driver got some voltage limits set for the vtrack algorithm: these
>> were moved to platform data first, then enforced in a later commit
>> 6a17b3876bc8 ("cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()")
>> using these as max values for the regulator_set_voltage() calls.
>>
>> In this case, the vsram/vproc constraints for MT7622 and MT7623
>> were supposed to be the same as MT2701 (and a number of other SoCs),
>> but that turned out to be a mistake because the aforementioned two
>> SoCs' maximum voltage for both VPROC and VPROC_SRAM is 1.36V.
>>
>> Fix that by adding new platform data for MT7622/7623 declaring the
>> right {proc,sram}_max_volt parameter.
>>
>> Fixes: ead858bd128d ("cpufreq: mediatek: Move voltage limits to platform data")
>> Fixes: 6a17b3876bc8 ("cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()")
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/cpufreq/mediatek-cpufreq.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
>> index 7f2680bc9a0f..f9a9f08c75c4 100644
>> --- a/drivers/cpufreq/mediatek-cpufreq.c
>> +++ b/drivers/cpufreq/mediatek-cpufreq.c
>> @@ -695,6 +695,15 @@ static const struct mtk_cpufreq_platform_data 
>> mt2701_platform_data = {
>>       .ccifreq_supported = false,
>>   };
>> +static const struct mtk_cpufreq_platform_data mt7622_platform_data = {
>> +    .min_volt_shift = 100000,
>> +    .max_volt_shift = 200000,
>> +    .proc_max_volt = 1360000,
>> +    .sram_min_volt = 0,
>> +    .sram_max_volt = 1360000,
>> +    .ccifreq_supported = false,
>> +};
>> +
>>   static const struct mtk_cpufreq_platform_data mt8183_platform_data = {
>>       .min_volt_shift = 100000,
>>       .max_volt_shift = 200000,
>> @@ -717,8 +726,8 @@ static const struct mtk_cpufreq_platform_data 
>> mt8186_platform_data = {
>>   static const struct of_device_id mtk_cpufreq_machines[] __initconst = {
>>       { .compatible = "mediatek,mt2701", .data = &mt2701_platform_data },
>>       { .compatible = "mediatek,mt2712", .data = &mt2701_platform_data },
>> -    { .compatible = "mediatek,mt7622", .data = &mt2701_platform_data },
>> -    { .compatible = "mediatek,mt7623", .data = &mt2701_platform_data },
>> +    { .compatible = "mediatek,mt7622", .data = &mt7622_platform_data },
>> +    { .compatible = "mediatek,mt7623", .data = &mt7622_platform_data },
>>       { .compatible = "mediatek,mt8167", .data = &mt2701_platform_data },
>>       { .compatible = "mediatek,mt817x", .data = &mt2701_platform_data },
>>       { .compatible = "mediatek,mt8173", .data = &mt2701_platform_data },