2021-08-12 15:15:42

by Taniya Das

[permalink] [raw]
Subject: [PATCH v2] cpufreq: qcom-hw: Set dvfs_possible_from_any_cpu cpufreq driver flag

As remote cpufreq updates are supported on QCOM platforms, set
dvfs_possible_from_any_cpu cpufreq driver flag.

Signed-off-by: Taniya Das <[email protected]>
---

[v2]
* update the dvfs_possible_from_any_cpu always.

drivers/cpufreq/qcom-cpufreq-hw.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index f86859b..53d3898 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -223,6 +223,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,

table[i].frequency = CPUFREQ_TABLE_END;
policy->freq_table = table;
+ policy->dvfs_possible_from_any_cpu = true;
+
dev_pm_opp_set_sharing_cpus(cpu_dev, policy->cpus);

return 0;
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


2021-08-12 18:09:59

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2] cpufreq: qcom-hw: Set dvfs_possible_from_any_cpu cpufreq driver flag

Quoting Taniya Das (2021-08-12 07:19:15)
> As remote cpufreq updates are supported on QCOM platforms, set
> dvfs_possible_from_any_cpu cpufreq driver flag.
>
> Signed-off-by: Taniya Das <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2021-08-12 20:49:52

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v2] cpufreq: qcom-hw: Set dvfs_possible_from_any_cpu cpufreq driver flag

Hi,

On Thu, Aug 12, 2021 at 7:19 AM Taniya Das <[email protected]> wrote:
>
> As remote cpufreq updates are supported on QCOM platforms, set
> dvfs_possible_from_any_cpu cpufreq driver flag.
>
> Signed-off-by: Taniya Das <[email protected]>
> ---
>
> [v2]
> * update the dvfs_possible_from_any_cpu always.
>
> drivers/cpufreq/qcom-cpufreq-hw.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index f86859b..53d3898 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -223,6 +223,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
>
> table[i].frequency = CPUFREQ_TABLE_END;
> policy->freq_table = table;
> + policy->dvfs_possible_from_any_cpu = true;
> +

Why is this in the qcom_cpufreq_hw_read_lut() function? Shouldn't it
be straight in qcom_cpufreq_hw_cpu_init()?

-Doug

2021-08-18 10:09:31

by Taniya Das

[permalink] [raw]
Subject: Re: [PATCH v2] cpufreq: qcom-hw: Set dvfs_possible_from_any_cpu cpufreq driver flag

Hi Doug,

Thanks for your review.

On 8/13/2021 2:03 AM, Doug Anderson wrote:
> Hi,
>
> On Thu, Aug 12, 2021 at 7:19 AM Taniya Das <[email protected]> wrote:
>>
>> As remote cpufreq updates are supported on QCOM platforms, set
>> dvfs_possible_from_any_cpu cpufreq driver flag.
>>
>> Signed-off-by: Taniya Das <[email protected]>
>> ---
>>
>> [v2]
>> * update the dvfs_possible_from_any_cpu always.
>>
>> drivers/cpufreq/qcom-cpufreq-hw.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
>> index f86859b..53d3898 100644
>> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
>> @@ -223,6 +223,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
>>
>> table[i].frequency = CPUFREQ_TABLE_END;
>> policy->freq_table = table;
>> + policy->dvfs_possible_from_any_cpu = true;
>> +
>
> Why is this in the qcom_cpufreq_hw_read_lut() function? Shouldn't it
> be straight in qcom_cpufreq_hw_cpu_init()?
>
All CPU policy related data is updated here, thus wanted to keep it
together.

> -Doug
>

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

2021-08-18 10:29:16

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v2] cpufreq: qcom-hw: Set dvfs_possible_from_any_cpu cpufreq driver flag

On 18-08-21, 15:33, Taniya Das wrote:
> Hi Doug,
>
> Thanks for your review.
>
> On 8/13/2021 2:03 AM, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Aug 12, 2021 at 7:19 AM Taniya Das <[email protected]> wrote:
> > >
> > > As remote cpufreq updates are supported on QCOM platforms, set
> > > dvfs_possible_from_any_cpu cpufreq driver flag.
> > >
> > > Signed-off-by: Taniya Das <[email protected]>
> > > ---
> > >
> > > [v2]
> > > * update the dvfs_possible_from_any_cpu always.
> > >
> > > drivers/cpufreq/qcom-cpufreq-hw.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> > > index f86859b..53d3898 100644
> > > --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> > > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> > > @@ -223,6 +223,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
> > >
> > > table[i].frequency = CPUFREQ_TABLE_END;
> > > policy->freq_table = table;
> > > + policy->dvfs_possible_from_any_cpu = true;
> > > +
> >
> > Why is this in the qcom_cpufreq_hw_read_lut() function? Shouldn't it
> > be straight in qcom_cpufreq_hw_cpu_init()?
> >
> All CPU policy related data is updated here, thus wanted to keep it
> together.

This function should only be updating freq table, please do it in
qcom_cpufreq_hw_cpu_init() itself, around where you set
policy->driver_data.

--
viresh

2021-08-23 10:20:29

by Taniya Das

[permalink] [raw]
Subject: Re: [PATCH v2] cpufreq: qcom-hw: Set dvfs_possible_from_any_cpu cpufreq driver flag

Hi Viresh,

Thanks for the review.

On 8/18/2021 3:56 PM, Viresh Kumar wrote:
> On 18-08-21, 15:33, Taniya Das wrote:
>> Hi Doug,
>>
>> Thanks for your review.
>>
>> On 8/13/2021 2:03 AM, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Thu, Aug 12, 2021 at 7:19 AM Taniya Das <[email protected]> wrote:
>>>>
>>>> As remote cpufreq updates are supported on QCOM platforms, set
>>>> dvfs_possible_from_any_cpu cpufreq driver flag.
>>>>
>>>> Signed-off-by: Taniya Das <[email protected]>
>>>> ---
>>>>
>>>> [v2]
>>>> * update the dvfs_possible_from_any_cpu always.
>>>>
>>>> drivers/cpufreq/qcom-cpufreq-hw.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
>>>> index f86859b..53d3898 100644
>>>> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
>>>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
>>>> @@ -223,6 +223,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
>>>>
>>>> table[i].frequency = CPUFREQ_TABLE_END;
>>>> policy->freq_table = table;
>>>> + policy->dvfs_possible_from_any_cpu = true;
>>>> +
>>>
>>> Why is this in the qcom_cpufreq_hw_read_lut() function? Shouldn't it
>>> be straight in qcom_cpufreq_hw_cpu_init()?
>>>
>> All CPU policy related data is updated here, thus wanted to keep it
>> together.
>
> This function should only be updating freq table, please do it in
> qcom_cpufreq_hw_cpu_init() itself, around where you set
> policy->driver_data.
>

Thanks Viresh, updated the same in the next patch.

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--