Messages in this thread Patch in this message | | | Date | Wed, 25 Oct 2023 13:06:34 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH 2/2] OPP: Call dev_pm_opp_set_opp() for required OPPs |
| |
Thanks a lot for taking this up, really appreciate it Stephan.
On 24-10-23, 13:18, Stephan Gerhold wrote: > Unfortunately this patch breaks scaling the performance state of the > virtual genpd devices completely. As far as I can tell it just keeps > setting level = 0 for every OPP switch (this is on MSM8909 with [1], > a single "perf" power domain attached to the CPU): > > [ 457.252255] cpu cpu0: _set_opp: switching OPP: Freq 998400000 -> 799999804 Hz, Level 0 -> 0, Bw 3200000 -> 1600000 > [ 457.253739] genpd genpd:1:cpu0: _set_opp: switching OPP: Freq 998400000 -> 799999804 Hz, Level 0 -> 0, Bw 3200000 -> 1600000
The thing I was more worried about worked fine it seems (recursively calling dev_pm_opp_set_opp() i.e.).
> [ 457.304581] cpu cpu0: _set_opp: switching OPP: Freq 799999804 -> 533333202 Hz, Level 0 -> 0, Bw 1600000 -> 1600000 > [ 457.306091] genpd genpd:1:cpu0: _set_opp: switching OPP: Freq 799999804 -> 533333202 Hz, Level 0 -> 0, Bw 1600000 -> 1600000 > [ 457.323489] cpu cpu0: _set_opp: switching OPP: Freq 533333202 -> 399999902 Hz, Level 0 -> 0, Bw 1600000 -> 800000 > [ 457.352792] genpd genpd:1:cpu0: _set_opp: switching OPP: Freq 533333202 -> 399999902 Hz, Level 0 -> 0, Bw 1600000 -> 800000 > [ 457.353056] cpu cpu0: _set_opp: switching OPP: Freq 399999902 -> 199999951 Hz, Level 0 -> 0, Bw 800000 -> 800000 > [ 457.355285] genpd genpd:1:cpu0: _set_opp: switching OPP: Freq 399999902 -> 199999951 Hz, Level 0 -> 0, Bw 800000 -> 800000 > > Where do you handle setting the pstate specified in the "required-opps" > of the OPP table with this patch? I've looked at your changes for some > time but must admit I haven't really understood how it is supposed to > work. :-)
Thanks for the debug print, they helped me find the issue.
> [ 457.304581] cpu cpu0: _set_opp: switching OPP: Freq 799999804 -> 533333202 Hz, Level 0 -> 0, Bw 1600000 -> 1600000 > [ 457.306091] genpd genpd:1:cpu0: _set_opp: switching OPP: Freq 799999804 -> 533333202 Hz, Level 0 -> 0, Bw 1600000 -> 1600000
The second line shouldn't have had freq/bw/etc, but just level. Hopefully this will fix it. Pushed to my branch too. Thanks. Please try again.
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 056b51abc501..cb2b353129c6 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1070,7 +1070,7 @@ static int _set_required_opps(struct device *dev, struct opp_table *opp_table,
while (index != target) { if (devs[index]) { - ret = dev_pm_opp_set_opp(devs[index], opp); + ret = dev_pm_opp_set_opp(devs[index], opp->required_opps[index]); if (ret) return ret; } -- viresh
| |