This patch set has below two changes:
1) get consistent cpuinfo_cur_freq value from freq_table.
2) Fix unlisted boot freq warning by setting closest ndiv value
from freq_table if the boot frequency gets filtered while
creating freq_table in kernel.
Sumit Gupta (2):
cpufreq: tegra194: get consistent cpuinfo_cur_freq
cpufreq: tegra194: Fix unlisted boot freq warning
drivers/cpufreq/tegra194-cpufreq.c | 182 ++++++++++++++++++++++++++++++++++---
1 file changed, 167 insertions(+), 15 deletions(-)
--
2.7.4
Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed
and keeps changing slightly. This change returns a consistent value
from freq_table. If the reconstructed frequency has acceptable delta
from the last written value, then return the frequency corresponding
to the last written ndiv value from freq_table. Otherwise, print a
warning and return the reconstructed freq.
Signed-off-by: Sumit Gupta <[email protected]>
---
drivers/cpufreq/tegra194-cpufreq.c | 66 ++++++++++++++++++++++++++++++++------
1 file changed, 57 insertions(+), 9 deletions(-)
diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c
index e1d931c..d5b608d 100644
--- a/drivers/cpufreq/tegra194-cpufreq.c
+++ b/drivers/cpufreq/tegra194-cpufreq.c
@@ -180,9 +180,65 @@ static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay)
return (rate_mhz * KHZ); /* in KHz */
}
+static void get_cpu_ndiv(void *ndiv)
+{
+ u64 ndiv_val;
+
+ asm volatile("mrs %0, s3_0_c15_c0_4" : "=r" (ndiv_val) : );
+
+ *(u64 *)ndiv = ndiv_val;
+}
+
+static void set_cpu_ndiv(void *data)
+{
+ struct cpufreq_frequency_table *tbl = data;
+ u64 ndiv_val = (u64)tbl->driver_data;
+
+ asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val));
+}
+
static unsigned int tegra194_get_speed(u32 cpu)
{
- return tegra194_get_speed_common(cpu, US_DELAY);
+ struct cpufreq_frequency_table *table, *pos;
+ struct cpufreq_policy policy;
+ unsigned int rate;
+ u64 ndiv;
+ int err;
+
+ cpufreq_get_policy(&policy, cpu);
+ table = policy.freq_table;
+
+ /* reconstruct actual cpu freq using counters*/
+ rate = tegra194_get_speed_common(cpu, US_DELAY);
+
+ /* get last written ndiv value*/
+ err = smp_call_function_single(cpu, get_cpu_ndiv, &ndiv, true);
+ if (err) {
+ pr_err("cpufreq: Failed to get ndiv for CPU%d, ret:%d\n",
+ cpu, err);
+ return rate;
+ }
+
+ /* if the reconstructed frequency has acceptable delta from
+ * the last written value, then return freq corresponding
+ * to the last written ndiv value from freq_table. This is
+ * done to return consistent value.
+ */
+ cpufreq_for_each_valid_entry(pos, table) {
+ if (pos->driver_data != ndiv)
+ continue;
+
+ if (abs(pos->frequency - rate) > 115200) {
+ pr_warn("cpufreq: high delta (%d) on CPU%d\n",
+ abs(pos->frequency - rate), cpu);
+ pr_warn("cpufreq: cur:%u, set:%u, set ndiv:%llu\n",
+ rate, pos->frequency, ndiv);
+ } else {
+ rate = pos->frequency;
+ }
+ break;
+ }
+ return rate;
}
static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
@@ -209,14 +265,6 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
return 0;
}
-static void set_cpu_ndiv(void *data)
-{
- struct cpufreq_frequency_table *tbl = data;
- u64 ndiv_val = (u64)tbl->driver_data;
-
- asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val));
-}
-
static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy,
unsigned int index)
{
--
2.7.4
On 16/09/2020 18:11, Sumit Gupta wrote:
> Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed
> and keeps changing slightly. This change returns a consistent value
> from freq_table. If the reconstructed frequency has acceptable delta
> from the last written value, then return the frequency corresponding
> to the last written ndiv value from freq_table. Otherwise, print a
> warning and return the reconstructed freq.
We should include the Fixes tag here ...
Fixes: df320f89359c ("cpufreq: Add Tegra194 cpufreq driver")
>
> Signed-off-by: Sumit Gupta <[email protected]>
Otherwise ...
Reviewed-by: Jon Hunter <[email protected]>
Tested-by: Jon Hunter <[email protected]>
Viresh, ideally we need to include this fix for v5.9. Do you need Sumit
to resend with the Fixes tag or are you happy to add?
Thanks
Jon
--
nvpublic
Adding Sudeep ...
On 17/09/2020 09:36, Jon Hunter wrote:
>
>
> On 16/09/2020 18:11, Sumit Gupta wrote:
>> Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed
>> and keeps changing slightly. This change returns a consistent value
>> from freq_table. If the reconstructed frequency has acceptable delta
>> from the last written value, then return the frequency corresponding
>> to the last written ndiv value from freq_table. Otherwise, print a
>> warning and return the reconstructed freq.
>
> We should include the Fixes tag here ...
>
> Fixes: df320f89359c ("cpufreq: Add Tegra194 cpufreq driver")
>
>>
>> Signed-off-by: Sumit Gupta <[email protected]>
>
> Otherwise ...
>
> Reviewed-by: Jon Hunter <[email protected]>
> Tested-by: Jon Hunter <[email protected]>
>
> Viresh, ideally we need to include this fix for v5.9. Do you need Sumit
> to resend with the Fixes tag or are you happy to add?
Sudeep, Rafael, looks like Viresh is out of office until next month.
Please let me know if we can pick up both this patch and following patch
for v5.9.
Thanks!
Jon
--
nvpublic
Hi Rafael, Sudeep,
On 17/09/2020 09:44, Jon Hunter wrote:
> Adding Sudeep ...
>
> On 17/09/2020 09:36, Jon Hunter wrote:
>>
>>
>> On 16/09/2020 18:11, Sumit Gupta wrote:
>>> Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed
>>> and keeps changing slightly. This change returns a consistent value
>>> from freq_table. If the reconstructed frequency has acceptable delta
>>> from the last written value, then return the frequency corresponding
>>> to the last written ndiv value from freq_table. Otherwise, print a
>>> warning and return the reconstructed freq.
>>
>> We should include the Fixes tag here ...
>>
>> Fixes: df320f89359c ("cpufreq: Add Tegra194 cpufreq driver")
>>
>>>
>>> Signed-off-by: Sumit Gupta <[email protected]>
>>
>> Otherwise ...
>>
>> Reviewed-by: Jon Hunter <[email protected]>
>> Tested-by: Jon Hunter <[email protected]>
>>
>> Viresh, ideally we need to include this fix for v5.9. Do you need Sumit
>> to resend with the Fixes tag or are you happy to add?
>
>
> Sudeep, Rafael, looks like Viresh is out of office until next month.
> Please let me know if we can pick up both this patch and following patch
> for v5.9.
Any chance we can get these patches into v5.9?
Thanks!
Jon
--
nvpublic
On Wed, Sep 16, 2020 at 10:41:16PM +0530, Sumit Gupta wrote:
> Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed
> and keeps changing slightly. This change returns a consistent value
> from freq_table. If the reconstructed frequency has acceptable delta
> from the last written value, then return the frequency corresponding
> to the last written ndiv value from freq_table. Otherwise, print a
> warning and return the reconstructed freq.
>
> Signed-off-by: Sumit Gupta <[email protected]>
> ---
> drivers/cpufreq/tegra194-cpufreq.c | 66 ++++++++++++++++++++++++++++++++------
> 1 file changed, 57 insertions(+), 9 deletions(-)
Acked-by: Thierry Reding <[email protected]>
On 17-09-20, 09:36, Jon Hunter wrote:
> Viresh, ideally we need to include this fix for v5.9. Do you need Sumit
> to resend with the Fixes tag or are you happy to add?
I understand that this fixes a patch which got merged recently, but I am not
sure if anything is broken badly right now, i.e. will make the hardware work
incorrectly.
Do we really need to get these in 5.9 ? As these are significant changes and may
cause more bugs. Won't getting them in 5.9-stable and 5.10-rc1 be enough ?
--
viresh
On 16-09-20, 22:41, Sumit Gupta wrote:
> Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed
> and keeps changing slightly. This change returns a consistent value
> from freq_table. If the reconstructed frequency has acceptable delta
> from the last written value, then return the frequency corresponding
> to the last written ndiv value from freq_table. Otherwise, print a
> warning and return the reconstructed freq.
>
> Signed-off-by: Sumit Gupta <[email protected]>
> ---
> drivers/cpufreq/tegra194-cpufreq.c | 66 ++++++++++++++++++++++++++++++++------
> 1 file changed, 57 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c
> index e1d931c..d5b608d 100644
> --- a/drivers/cpufreq/tegra194-cpufreq.c
> +++ b/drivers/cpufreq/tegra194-cpufreq.c
> @@ -180,9 +180,65 @@ static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay)
> return (rate_mhz * KHZ); /* in KHz */
> }
>
> +static void get_cpu_ndiv(void *ndiv)
> +{
> + u64 ndiv_val;
> +
> + asm volatile("mrs %0, s3_0_c15_c0_4" : "=r" (ndiv_val) : );
> +
> + *(u64 *)ndiv = ndiv_val;
> +}
> +
> +static void set_cpu_ndiv(void *data)
> +{
> + struct cpufreq_frequency_table *tbl = data;
> + u64 ndiv_val = (u64)tbl->driver_data;
> +
> + asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val));
> +}
> +
> static unsigned int tegra194_get_speed(u32 cpu)
> {
> - return tegra194_get_speed_common(cpu, US_DELAY);
> + struct cpufreq_frequency_table *table, *pos;
> + struct cpufreq_policy policy;
> + unsigned int rate;
> + u64 ndiv;
> + int err;
> +
> + cpufreq_get_policy(&policy, cpu);
> + table = policy.freq_table;
Maybe getting freq-table from cluster specific data would be better/faster.
> +
> + /* reconstruct actual cpu freq using counters*/
> + rate = tegra194_get_speed_common(cpu, US_DELAY);
> +
> + /* get last written ndiv value*/
> + err = smp_call_function_single(cpu, get_cpu_ndiv, &ndiv, true);
> + if (err) {
> + pr_err("cpufreq: Failed to get ndiv for CPU%d, ret:%d\n",
> + cpu, err);
> + return rate;
> + }
> +
> + /* if the reconstructed frequency has acceptable delta from
Both have got the multi-line comments wrong. Format is wrong and every sentence
needs to start with a capital letter.
> + * the last written value, then return freq corresponding
> + * to the last written ndiv value from freq_table. This is
> + * done to return consistent value.
> + */
> + cpufreq_for_each_valid_entry(pos, table) {
> + if (pos->driver_data != ndiv)
> + continue;
> +
> + if (abs(pos->frequency - rate) > 115200) {
> + pr_warn("cpufreq: high delta (%d) on CPU%d\n",
> + abs(pos->frequency - rate), cpu);
> + pr_warn("cpufreq: cur:%u, set:%u, set ndiv:%llu\n",
> + rate, pos->frequency, ndiv);
Both of these can be merged in a single line I think. There is no need to print
delta as you already print both the frequencies.
> + } else {
> + rate = pos->frequency;
> + }
> + break;
> + }
> + return rate;
> }
>
> static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
> @@ -209,14 +265,6 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
> return 0;
> }
>
> -static void set_cpu_ndiv(void *data)
> -{
> - struct cpufreq_frequency_table *tbl = data;
> - u64 ndiv_val = (u64)tbl->driver_data;
> -
> - asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val));
> -}
> -
> static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy,
> unsigned int index)
> {
> --
> 2.7.4
--
viresh
On 05/10/2020 05:34, Viresh Kumar wrote:
> On 17-09-20, 09:36, Jon Hunter wrote:
>> Viresh, ideally we need to include this fix for v5.9. Do you need Sumit
>> to resend with the Fixes tag or are you happy to add?
>
> I understand that this fixes a patch which got merged recently, but I am not
> sure if anything is broken badly right now, i.e. will make the hardware work
> incorrectly.
>
> Do we really need to get these in 5.9 ? As these are significant changes and may
> cause more bugs. Won't getting them in 5.9-stable and 5.10-rc1 be enough ?
Yes we want these in v5.9 ideally but yes we could merge to 5.9-stable
once accepted into the mainline.
Jon
--
nvpublic
>> Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed
>> and keeps changing slightly. This change returns a consistent value
>> from freq_table. If the reconstructed frequency has acceptable delta
>> from the last written value, then return the frequency corresponding
>> to the last written ndiv value from freq_table. Otherwise, print a
>> warning and return the reconstructed freq.
>>
>> Signed-off-by: Sumit Gupta <[email protected]>
>> ---
>> drivers/cpufreq/tegra194-cpufreq.c | 66 ++++++++++++++++++++++++++++++++------
>> 1 file changed, 57 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c
>> index e1d931c..d5b608d 100644
>> --- a/drivers/cpufreq/tegra194-cpufreq.c
>> +++ b/drivers/cpufreq/tegra194-cpufreq.c
>> @@ -180,9 +180,65 @@ static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay)
>> return (rate_mhz * KHZ); /* in KHz */
>> }
>>
>> +static void get_cpu_ndiv(void *ndiv)
>> +{
>> + u64 ndiv_val;
>> +
>> + asm volatile("mrs %0, s3_0_c15_c0_4" : "=r" (ndiv_val) : );
>> +
>> + *(u64 *)ndiv = ndiv_val;
>> +}
>> +
>> +static void set_cpu_ndiv(void *data)
>> +{
>> + struct cpufreq_frequency_table *tbl = data;
>> + u64 ndiv_val = (u64)tbl->driver_data;
>> +
>> + asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val));
>> +}
>> +
>> static unsigned int tegra194_get_speed(u32 cpu)
>> {
>> - return tegra194_get_speed_common(cpu, US_DELAY);
>> + struct cpufreq_frequency_table *table, *pos;
>> + struct cpufreq_policy policy;
>> + unsigned int rate;
>> + u64 ndiv;
>> + int err;
>> +
>> + cpufreq_get_policy(&policy, cpu);
>> + table = policy.freq_table;
>
> Maybe getting freq-table from cluster specific data would be better/faster.
>
will do the change in next patch version.
>> +
>> + /* reconstruct actual cpu freq using counters*/
>> + rate = tegra194_get_speed_common(cpu, US_DELAY);
>> +
>> + /* get last written ndiv value*/
>> + err = smp_call_function_single(cpu, get_cpu_ndiv, &ndiv, true);
>> + if (err) {
>> + pr_err("cpufreq: Failed to get ndiv for CPU%d, ret:%d\n",
>> + cpu, err);
>> + return rate;
>> + }
>> +
>> + /* if the reconstructed frequency has acceptable delta from
>
> Both have got the multi-line comments wrong. Format is wrong and every sentence
> needs to start with a capital letter.
>
will correct.
>> + * the last written value, then return freq corresponding
>> + * to the last written ndiv value from freq_table. This is
>> + * done to return consistent value.
>> + */
>> + cpufreq_for_each_valid_entry(pos, table) {
>> + if (pos->driver_data != ndiv)
>> + continue;
>> +
>> + if (abs(pos->frequency - rate) > 115200) {
>> + pr_warn("cpufreq: high delta (%d) on CPU%d\n",
>> + abs(pos->frequency - rate), cpu);
>> + pr_warn("cpufreq: cur:%u, set:%u, set ndiv:%llu\n",
>> + rate, pos->frequency, ndiv);
>
> Both of these can be merged in a single line I think. There is no need to print
> delta as you already print both the frequencies.
>
Will do this also in next patch version.
>> + } else {
>> + rate = pos->frequency;
>> + }
>> + break;
>> + }
>> + return rate;
>> }
>>
>> static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
>> @@ -209,14 +265,6 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
>> return 0;
>> }
>>
>> -static void set_cpu_ndiv(void *data)
>> -{
>> - struct cpufreq_frequency_table *tbl = data;
>> - u64 ndiv_val = (u64)tbl->driver_data;
>> -
>> - asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val));
>> -}
>> -
>> static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy,
>> unsigned int index)
>> {
>> --
>> 2.7.4
>
> --
> viresh
>