2021-05-26 04:10:30

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor

Hi,
On 5/26/21 11:22 AM, andrew-sh.cheng wrote:
> On Thu, 2021-04-08 at 11:47 +0900, Chanwoo Choi wrote:
>> On 4/1/21 9:16 AM, Chanwoo Choi wrote:
>>> On 3/31/21 10:03 PM, andrew-sh.cheng wrote:
>>>> On Wed, 2021-03-31 at 17:35 +0900, Chanwoo Choi wrote:
>>>>> On 3/31/21 5:27 PM, Chanwoo Choi wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 3/31/21 5:03 PM, andrew-sh.cheng wrote:
>>>>>>> On Thu, 2021-03-25 at 17:14 +0900, Chanwoo Choi wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> You are missing to add these patches to linux-pm mailing list.
>>>>>>>> Need to send them to linu-pm ML.
>>>>>>>>
>>>>>>>> Also, before received this series, I tried to clean-up these patches
>>>>>>>> on testing branch[1]. So that I add my comment with my clean-up case.
>>>>>>>> [1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgAPh9XRRs$
>>>>>>>>
>>>>>>>> And 'Saravana Kannan <[email protected]>' is wrong email address.
>>>>>>>> Please update the email or drop this email.
>>>>>>>
>>>>>>> Hi Chanwoo,
>>>>>>>
>>>>>>> Thank you for the advices.
>>>>>>> I will resend patch v9 (add to linux-pm ML), remove this patch, and note
>>>>>>> that my patch set base on
>>>>>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJ8WFhg_g$
>>>>>>
>>>>>> I has not yet test this patch[1] on devfreq-testing-passive-gov branch.
>>>>>> So that if possible, I'd like you to test your patches with this patch[1]
>>>>>> and then if there is no problem, could you send the next patches with patch[1]?
>>>>>>
>>>>>> [1]https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=39c80d11a8f42dd63ecea1e0df595a0ceb83b454__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJR2cQqZs$
>>>>>
>>>>>
>>>>> Sorry for the confusion. I make the devfreq-testing-passive-gov[1]
>>>>> branch based on latest devfreq-next branch.
>>>>> [1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJ8WFhg_g$
>>>>>
>>>>> First of all, if possible, I want to test them[1] with your patches in this series.
>>>>> And then if there are no any problem, please let me know. After confirmed from you,
>>>>> I'll send the patches of devfreq-testing-passive-gov[1] branch.
>>>>> How about that?
>>>>>
>>>> Hi Chanwoo~
>>>>
>>>> We will use this on Google Chrome project.
>>>> Google Hsin-Yi has test your patch + my patch set v8 [2~8]
>>>>
>>>> make sure cci devfreqs runs with cpufreq.
>>>> suspend resume
>>>> speedometer2 benchmark
>>>> It is okay.
>>>>
>>>> Please send the patches of devfreq-testing-passive-gov[1] branch.
>>>>
>>>> I will send patch v9 base on yours latter.
>>>
>>> Thanks for your test. I'll send the patches today.
>>
>> I'm sorry for delay because when I tested the patches
>> for devfreq parent type on Odroid-xu3, there are some problem
>> related to lazy linking of OPP. So I'm trying to analyze them.
>> Unfortunately, we need to postpone these patches to next linux
>> version.
>>
> Hi Chanwoo Choi~
>
> It is said that you are busy on another task recently.
> May I know your plan on this patch?
> Thank you.

Sorry for late work. I have a question.
When I tested exynos-bus.c with adding the 'required-opp' property
on odroid-xu3 board. I got some fail about

When calling _set_required_opps(), always _set_required_opp() returns
-EBUSY error because of following lazy linking case[1].

[1] https://elixir.bootlin.com/linux/v5.13-rc3/source/drivers/opp/core.c#L896

/* required-opps not fully initialized yet */
if (lazy_linking_pending(opp_table))
return -EBUSY;


For calling dev_pm_opp_of_add_table(), lazy_link_required_opp_table() function
will be called. But, there is constraint[2]. If is_genpd of opp_table is false,
driver/opp/of.c cannot resolve the lazy linking issue.

[2] https://elixir.bootlin.com/linux/v5.13-rc3/source/drivers/opp/of.c#L386

/* Link required OPPs for all OPPs of the newly added OPP table */
static void lazy_link_required_opp_table(struct opp_table *new_table)
{
struct opp_table *opp_table, *temp, **required_opp_tables;
struct device_node *required_np, *opp_np, *required_table_np;
struct dev_pm_opp *opp;
int i, ret;

/*
* We only support genpd's OPPs in the "required-opps" for now,
* as we don't know much about other cases.
*/
if (!new_table->is_genpd)
return;

Even if this case, there are no problem on your test case?

--
Best Regards,
Chanwoo Choi
Samsung Electronics