Messages in this thread Patch in this message | | | Date | Tue, 3 Oct 2023 13:53:22 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps |
| |
On 25-09-23, 15:33, Ulf Hansson wrote: > On Mon, 25 Sept 2023 at 15:18, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > At this point the level (performance state) for an OPP is currently limited > > to be requested for a device that is attached to a PM domain. Moreover, > > the device needs to have the so called required-opps assigned to it, which > > are based upon OPP tables being described in DT. > > > > To extend the support beyond required-opps and DT, let's enable the level > > to be set for all OPPs. More precisely, if the requested OPP has a valid > > level let's try to request it through the device's optional PM domain, via > > calling dev_pm_domain_set_performance_state(). > > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > --- > > drivers/opp/core.c | 29 +++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > > index 60dca60ac4af..afb73978cdcb 100644 > > --- a/drivers/opp/core.c > > +++ b/drivers/opp/core.c > > @@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table) > > opp_table->set_required_opps = _opp_set_required_opps_generic; > > } > > > > +static int _set_opp_level(struct device *dev, struct opp_table *opp_table, > > + struct dev_pm_opp *opp) > > +{ > > + int ret = 0; > > + > > + /* Request a new performance state through the device's PM domain. */ > > + if (opp && opp->level) { > > + ret = dev_pm_domain_set_performance_state(dev, opp->level); > > + if (ret) > > + dev_err(dev, "Failed to set performance state %u (%d)\n", > > + opp->level, ret); > > + } > > Okay, so reviewing my own code found a problem here. We need an "else" > here, that should request the performance state to be set to 0. > > I am not sending a new version at this point, but awaiting more feedback first.
I am looking to add below to this patch, is that okay with you ?
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 7b505316bb1c..a113e9caaa5a 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1135,16 +1135,22 @@ void _update_set_required_opps(struct opp_table *opp_table) static int _set_opp_level(struct device *dev, struct opp_table *opp_table, struct dev_pm_opp *opp) { + unsigned int level = 0; int ret = 0;
- /* Request a new performance state through the device's PM domain. */ - if (opp && opp->level) { - ret = dev_pm_domain_set_performance_state(dev, opp->level); - if (ret) - dev_err(dev, "Failed to set performance state %u (%d)\n", - opp->level, ret); + if (opp) { + if (!opp->level) + return 0; + + level = opp->level; }
+ /* Request a new performance state through the device's PM domain. */ + ret = dev_pm_genpd_set_performance_state(dev, level); + if (ret) + dev_err(dev, "Failed to set performance state %u (%d)\n", level, + ret); + return ret; }
I am switching back to dev_pm_genpd_set_performance_state() as I won't be applying the power domain changes. I will then push out a branch and you can rebase your patches on top of it ? And then probably Sudeep or someone else can apply everything ? -- viresh
| |