| Message ID | 1628074696-7979-2-git-send-email-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PM / Domains: Add support for 'required-opps' to set default perf state | expand |
On Wed, 4 Aug 2021 at 12:58, Rajendra Nayak <rnayak@codeaurora.org> wrote: > > Some devices within power domains with performance states do not > support DVFS, but still need to vote on a default/static state > while they are active. They can express this using the 'required-opps' > property in device tree, which points to the phandle of the OPP > supported by the corresponding power-domains. > > Add support to parse this information from DT and then set the > specified performance state during attach and drop it on detach. > runtime suspend/resume callbacks already have logic to drop/set > the vote as needed and should take care of dropping the default > perf state vote on runtime suspend and restore it back on runtime > resume. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > drivers/base/power/domain.c | 28 ++++++++++++++++++++++++++-- > include/linux/pm_domain.h | 1 + > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index a934c67..b9b5a9b 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -2598,6 +2598,12 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off) > > dev_dbg(dev, "removing from PM domain %s\n", pd->name); > > + /* Drop the default performance state */ > + if (dev_gpd_data(dev)->default_pstate) { > + dev_pm_genpd_set_performance_state(dev, 0); > + dev_gpd_data(dev)->default_pstate = 0; > + } > + > for (i = 1; i < GENPD_RETRY_MAX_MS; i <<= 1) { > ret = genpd_remove_device(pd, dev); > if (ret != -EAGAIN) > @@ -2637,6 +2643,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, > { > struct of_phandle_args pd_args; > struct generic_pm_domain *pd; > + struct device_node *np; > + int pstate; > int ret; > > ret = of_parse_phandle_with_args(dev->of_node, "power-domains", > @@ -2675,10 +2683,26 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, > genpd_unlock(pd); > } > > - if (ret) > + if (ret) { > genpd_remove_device(pd, dev); > + return -EPROBE_DEFER; > + } > + > + /* Set the default performance state */ > + np = dev->of_node; > + if (of_parse_phandle(np, "required-opps", index)) { Looks like Viresh thinks it's a good idea to drop the error print in of_get_required_opp_performance_state() when there is no "required-opps" specifier. Would you mind folding in a patch for that in the series, so this code can be simplified according to our earlier discussions? > + pstate = of_get_required_opp_performance_state(np, index); > + if (pstate < 0) { > + ret = pstate; > + dev_err(dev, "failed to set required performance state for power-domain %s: %d\n", > + pd->name, ret); > + } else { > + dev_pm_genpd_set_performance_state(dev, pstate); > + dev_gpd_data(dev)->default_pstate = pstate; > + } > + } > > - return ret ? -EPROBE_DEFER : 1; > + return ret ? ret : 1; > } > > /** > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index 21a0577..67017c9 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -198,6 +198,7 @@ struct generic_pm_domain_data { > struct notifier_block *power_nb; > int cpu; > unsigned int performance_state; > + unsigned int default_pstate; > unsigned int rpm_pstate; > ktime_t next_wakeup; > void *data; Other than the above, this looks good to me! Kind regards Uffe
On 8/6/2021 3:02 PM, Ulf Hansson wrote: > On Wed, 4 Aug 2021 at 12:58, Rajendra Nayak <rnayak@codeaurora.org> wrote: >> >> Some devices within power domains with performance states do not >> support DVFS, but still need to vote on a default/static state >> while they are active. They can express this using the 'required-opps' >> property in device tree, which points to the phandle of the OPP >> supported by the corresponding power-domains. >> >> Add support to parse this information from DT and then set the >> specified performance state during attach and drop it on detach. >> runtime suspend/resume callbacks already have logic to drop/set >> the vote as needed and should take care of dropping the default >> perf state vote on runtime suspend and restore it back on runtime >> resume. >> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> --- >> drivers/base/power/domain.c | 28 ++++++++++++++++++++++++++-- >> include/linux/pm_domain.h | 1 + >> 2 files changed, 27 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >> index a934c67..b9b5a9b 100644 >> --- a/drivers/base/power/domain.c >> +++ b/drivers/base/power/domain.c >> @@ -2598,6 +2598,12 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off) >> >> dev_dbg(dev, "removing from PM domain %s\n", pd->name); >> >> + /* Drop the default performance state */ >> + if (dev_gpd_data(dev)->default_pstate) { >> + dev_pm_genpd_set_performance_state(dev, 0); >> + dev_gpd_data(dev)->default_pstate = 0; >> + } >> + >> for (i = 1; i < GENPD_RETRY_MAX_MS; i <<= 1) { >> ret = genpd_remove_device(pd, dev); >> if (ret != -EAGAIN) >> @@ -2637,6 +2643,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >> { >> struct of_phandle_args pd_args; >> struct generic_pm_domain *pd; >> + struct device_node *np; >> + int pstate; >> int ret; >> >> ret = of_parse_phandle_with_args(dev->of_node, "power-domains", >> @@ -2675,10 +2683,26 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >> genpd_unlock(pd); >> } >> >> - if (ret) >> + if (ret) { >> genpd_remove_device(pd, dev); >> + return -EPROBE_DEFER; >> + } >> + >> + /* Set the default performance state */ >> + np = dev->of_node; >> + if (of_parse_phandle(np, "required-opps", index)) { > > Looks like Viresh thinks it's a good idea to drop the error print in > of_get_required_opp_performance_state() when there is no > "required-opps" specifier. > > Would you mind folding in a patch for that in the series, so this code > can be simplified according to our earlier discussions? Sure, I can do that, apart from the error print, the function currently also returns a -EINVAL in case of the missing 'required-opps', are we suggesting we change that to not return an error also? Since this is completely optional in the device node, we would want the function to ideally not return error and only do so in case 'required-opps' exists and the translation to performance state fails. I am not sure that's the behavior we expect in case of 'required-opps' in the OPP tables also, Viresh? > >> + pstate = of_get_required_opp_performance_state(np, index); >> + if (pstate < 0) { >> + ret = pstate; >> + dev_err(dev, "failed to set required performance state for power-domain %s: %d\n", >> + pd->name, ret); >> + } else { >> + dev_pm_genpd_set_performance_state(dev, pstate); >> + dev_gpd_data(dev)->default_pstate = pstate; >> + } >> + } >> >> - return ret ? -EPROBE_DEFER : 1; >> + return ret ? ret : 1; >> } >> >> /** >> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h >> index 21a0577..67017c9 100644 >> --- a/include/linux/pm_domain.h >> +++ b/include/linux/pm_domain.h >> @@ -198,6 +198,7 @@ struct generic_pm_domain_data { >> struct notifier_block *power_nb; >> int cpu; >> unsigned int performance_state; >> + unsigned int default_pstate; >> unsigned int rpm_pstate; >> ktime_t next_wakeup; >> void *data; > > Other than the above, this looks good to me! > > Kind regards > Uffe >
04.08.2021 13:58, Rajendra Nayak пишет: > @@ -2637,6 +2643,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, > { > struct of_phandle_args pd_args; > struct generic_pm_domain *pd; > + struct device_node *np; > + int pstate; > int ret; > > ret = of_parse_phandle_with_args(dev->of_node, "power-domains", > @@ -2675,10 +2683,26 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, > genpd_unlock(pd); > } > > - if (ret) > + if (ret) { > genpd_remove_device(pd, dev); > + return -EPROBE_DEFER; > + } > + > + /* Set the default performance state */ > + np = dev->of_node; > + if (of_parse_phandle(np, "required-opps", index)) { > + pstate = of_get_required_opp_performance_state(np, index); > + if (pstate < 0) { > + ret = pstate; > + dev_err(dev, "failed to set required performance state for power-domain %s: %d\n", > + pd->name, ret); > + } else { > + dev_pm_genpd_set_performance_state(dev, pstate); > + dev_gpd_data(dev)->default_pstate = pstate; > + } > + } Why performance state is set after genpd_power_on()?
10.08.2021 01:26, Dmitry Osipenko пишет: > 04.08.2021 13:58, Rajendra Nayak пишет: >> @@ -2637,6 +2643,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >> { >> struct of_phandle_args pd_args; >> struct generic_pm_domain *pd; >> + struct device_node *np; >> + int pstate; >> int ret; >> >> ret = of_parse_phandle_with_args(dev->of_node, "power-domains", >> @@ -2675,10 +2683,26 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >> genpd_unlock(pd); >> } >> >> - if (ret) >> + if (ret) { >> genpd_remove_device(pd, dev); >> + return -EPROBE_DEFER; >> + } >> + >> + /* Set the default performance state */ >> + np = dev->of_node; >> + if (of_parse_phandle(np, "required-opps", index)) { >> + pstate = of_get_required_opp_performance_state(np, index); >> + if (pstate < 0) { >> + ret = pstate; >> + dev_err(dev, "failed to set required performance state for power-domain %s: %d\n", >> + pd->name, ret); >> + } else { >> + dev_pm_genpd_set_performance_state(dev, pstate); Where is error handling? >> + dev_gpd_data(dev)->default_pstate = pstate; >> + } >> + } > > Why performance state is set after genpd_power_on()? >
10.08.2021 01:44, Dmitry Osipenko пишет: > 10.08.2021 01:26, Dmitry Osipenko пишет: >> 04.08.2021 13:58, Rajendra Nayak пишет: >>> @@ -2637,6 +2643,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >>> { >>> struct of_phandle_args pd_args; >>> struct generic_pm_domain *pd; >>> + struct device_node *np; >>> + int pstate; >>> int ret; >>> >>> ret = of_parse_phandle_with_args(dev->of_node, "power-domains", >>> @@ -2675,10 +2683,26 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >>> genpd_unlock(pd); >>> } >>> >>> - if (ret) >>> + if (ret) { >>> genpd_remove_device(pd, dev); >>> + return -EPROBE_DEFER; >>> + } >>> + >>> + /* Set the default performance state */ >>> + np = dev->of_node; >>> + if (of_parse_phandle(np, "required-opps", index)) { The OF node returned by of_parse_phandle() must be put. >>> + pstate = of_get_required_opp_performance_state(np, index); If you have more than one power domain, then this will override the pstate which was set for a previous domain. This code doesn't feel solid to me, at least a clarifying comment is needed about how it's supposed to work. >>> + if (pstate < 0) { >>> + ret = pstate; >>> + dev_err(dev, "failed to set required performance state for power-domain %s: %d\n", >>> + pd->name, ret); >>> + } else { >>> + dev_pm_genpd_set_performance_state(dev, pstate); > > Where is error handling? > >>> + dev_gpd_data(dev)->default_pstate = pstate; >>> + } >>> + } >> >> Why performance state is set after genpd_power_on()? Should set_performance_state() be invoked when power_on=false? I assume it should be: if (power_on) { dev_pm_genpd_set_performance_state(dev, pstate); dev_gpd_data(dev)->default_pstate = pstate; } else { dev_gpd_data(dev)->rpm_pstate = pstate; }
10.08.2021 02:46, Dmitry Osipenko пишет: > 10.08.2021 01:44, Dmitry Osipenko пишет: >> 10.08.2021 01:26, Dmitry Osipenko пишет: >>> 04.08.2021 13:58, Rajendra Nayak пишет: >>>> @@ -2637,6 +2643,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >>>> { >>>> struct of_phandle_args pd_args; >>>> struct generic_pm_domain *pd; >>>> + struct device_node *np; >>>> + int pstate; >>>> int ret; >>>> >>>> ret = of_parse_phandle_with_args(dev->of_node, "power-domains", >>>> @@ -2675,10 +2683,26 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >>>> genpd_unlock(pd); >>>> } >>>> >>>> - if (ret) >>>> + if (ret) { >>>> genpd_remove_device(pd, dev); >>>> + return -EPROBE_DEFER; >>>> + } >>>> + >>>> + /* Set the default performance state */ >>>> + np = dev->of_node; >>>> + if (of_parse_phandle(np, "required-opps", index)) { > > The OF node returned by of_parse_phandle() must be put. > >>>> + pstate = of_get_required_opp_performance_state(np, index); > > If you have more than one power domain, then this will override the > pstate which was set for a previous domain. This code doesn't feel solid > to me, at least a clarifying comment is needed about how it's supposed > to work. > >>>> + if (pstate < 0) { >>>> + ret = pstate; >>>> + dev_err(dev, "failed to set required performance state for power-domain %s: %d\n", >>>> + pd->name, ret); >>>> + } else { >>>> + dev_pm_genpd_set_performance_state(dev, pstate); >> >> Where is error handling? >> >>>> + dev_gpd_data(dev)->default_pstate = pstate; >>>> + } >>>> + } >>> >>> Why performance state is set after genpd_power_on()? > > Should set_performance_state() be invoked when power_on=false? > > I assume it should be: > > if (power_on) { > dev_pm_genpd_set_performance_state(dev, pstate); > dev_gpd_data(dev)->default_pstate = pstate; > } else { > dev_gpd_data(dev)->rpm_pstate = pstate; > } > Although, thinking a bit more about this, it should be better to skip setting perf state in a case of power_on=false and let drivers to handle it, IMO. Too much trouble with it for the core.
On 09-08-21, 16:38, Rajendra Nayak wrote: > Sure, I can do that, apart from the error print, the function currently also > returns a -EINVAL in case of the missing 'required-opps', are we suggesting > we change that to not return an error also? No. > Since this is completely optional in the device node, we would want the function to > ideally not return error and only do so in case 'required-opps' exists and the > translation to performance state fails. Not really. The function should return failure if the property isn't there, but it shouldn't be EINVAL but ENODEV.
On 8/10/2021 5:16 AM, Dmitry Osipenko wrote: > 10.08.2021 01:44, Dmitry Osipenko пишет: >> 10.08.2021 01:26, Dmitry Osipenko пишет: >>> 04.08.2021 13:58, Rajendra Nayak пишет: >>>> @@ -2637,6 +2643,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >>>> { >>>> struct of_phandle_args pd_args; >>>> struct generic_pm_domain *pd; >>>> + struct device_node *np; >>>> + int pstate; >>>> int ret; >>>> >>>> ret = of_parse_phandle_with_args(dev->of_node, "power-domains", >>>> @@ -2675,10 +2683,26 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >>>> genpd_unlock(pd); >>>> } >>>> >>>> - if (ret) >>>> + if (ret) { >>>> genpd_remove_device(pd, dev); >>>> + return -EPROBE_DEFER; >>>> + } >>>> + >>>> + /* Set the default performance state */ >>>> + np = dev->of_node; >>>> + if (of_parse_phandle(np, "required-opps", index)) { > > The OF node returned by of_parse_phandle() must be put. Thanks for spotting that, I will fix it, though I might just drop the use of of_parse_phandle() and call of_get_required_opp_performance_state() unconditionally as suggested by Ulf. > >>>> + pstate = of_get_required_opp_performance_state(np, index); > > If you have more than one power domain, then this will override the > pstate which was set for a previous domain. This code doesn't feel solid > to me, at least a clarifying comment is needed about how it's supposed > to work. I don't quite understand the concern here, this should work with devices having multiple power-domains as well. __genpd_dev_pm_attach gets called once per power-domain, and we use the index above to identify the power-domain. >>>> + if (pstate < 0) { >>>> + ret = pstate; >>>> + dev_err(dev, "failed to set required performance state for power-domain %s: %d\n", >>>> + pd->name, ret); >>>> + } else { >>>> + dev_pm_genpd_set_performance_state(dev, pstate); >> >> Where is error handling? Sure, I'll fix that. >> >>>> + dev_gpd_data(dev)->default_pstate = pstate; >>>> + } >>>> + } >>> >>> Why performance state is set after genpd_power_on()? The requirement is that the domain is operating at a given performance state before the device becomes operational. The driver should ideally wait for the attach to complete before expecting the device to be functional. So the order here should really not matter, or rather the small amount of time while the power-domain is on but not at the right performance state should not matter if that's the concern. > > Should set_performance_state() be invoked when power_on=false? > > I assume it should be: > > if (power_on) { > dev_pm_genpd_set_performance_state(dev, pstate); > dev_gpd_data(dev)->default_pstate = pstate; > } else { > dev_gpd_data(dev)->rpm_pstate = pstate; > } >
On 8/10/2021 8:13 AM, Viresh Kumar wrote: > On 09-08-21, 16:38, Rajendra Nayak wrote: >> Sure, I can do that, apart from the error print, the function currently also >> returns a -EINVAL in case of the missing 'required-opps', are we suggesting >> we change that to not return an error also? > > No. > >> Since this is completely optional in the device node, we would want the function to >> ideally not return error and only do so in case 'required-opps' exists and the >> translation to performance state fails. > > Not really. The function should return failure if the property isn't > there, but it shouldn't be EINVAL but ENODEV. In my case I don't want to error out if the property is missing, I want to error out only when the property exists but can't be translated into a performance state. So currently I check if the property exists and *only then* try to translate it, Ulf asked me to skip the check. If I do that and I call of_get_required_opp_performance_state() unconditionally, and if it errors out I will need to put in additional logic (check for return value of ENODEV) to distinguish between the property-does-not-exist vs property-exists-but-cannot-be-translated case. It just seems more straight-forward to call this only when the property exists, Ulf?
On 11-08-21, 15:30, Rajendra Nayak wrote: > In my case I don't want to error out if the property is missing, I want to error out > only when the property exists but can't be translated into a performance state. > > So currently I check if the property exists and *only then* try to translate it, Ulf asked > me to skip the check. If I do that and I call of_get_required_opp_performance_state() > unconditionally, and if it errors out I will need to put in additional logic (check for > return value of ENODEV) to distinguish between the property-does-not-exist vs > property-exists-but-cannot-be-translated case. > It just seems more straight-forward to call this only when the property exists, Ulf? The same check will be done by OPP core as well, so it is better to optimize for the success case here. I will say, don't error out on ENODEV, rest you know well.
On Wed, 11 Aug 2021 at 12:07, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 11-08-21, 15:30, Rajendra Nayak wrote: > > In my case I don't want to error out if the property is missing, I want to error out > > only when the property exists but can't be translated into a performance state. > > > > So currently I check if the property exists and *only then* try to translate it, Ulf asked > > me to skip the check. If I do that and I call of_get_required_opp_performance_state() > > unconditionally, and if it errors out I will need to put in additional logic (check for > > return value of ENODEV) to distinguish between the property-does-not-exist vs > > property-exists-but-cannot-be-translated case. > > It just seems more straight-forward to call this only when the property exists, Ulf? > > The same check will be done by OPP core as well, so it is better to > optimize for the success case here. I will say, don't error out on > ENODEV, rest you know well. This should work, while I generally favor not having to parse for specific return codes. Another option is to add a new OPP OF helperfunction that just informs the caller whether the required-opps property exists (instead of open-coding that part), and if so, the caller can continue with of_get_required_opp_performance_state() and then expect it to succeed. I have no strong opinion though! Whatever works for me. Kind regards Uffe
On 11-08-21, 12:52, Ulf Hansson wrote: > On Wed, 11 Aug 2021 at 12:07, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 11-08-21, 15:30, Rajendra Nayak wrote: > > > In my case I don't want to error out if the property is missing, I want to error out > > > only when the property exists but can't be translated into a performance state. > > > > > > So currently I check if the property exists and *only then* try to translate it, Ulf asked > > > me to skip the check. If I do that and I call of_get_required_opp_performance_state() > > > unconditionally, and if it errors out I will need to put in additional logic (check for > > > return value of ENODEV) to distinguish between the property-does-not-exist vs > > > property-exists-but-cannot-be-translated case. > > > It just seems more straight-forward to call this only when the property exists, Ulf? > > > > The same check will be done by OPP core as well, so it is better to > > optimize for the success case here. I will say, don't error out on > > ENODEV, rest you know well. > > This should work, while I generally favor not having to parse for > specific return codes. > > Another option is to add a new OPP OF helperfunction that just informs > the caller whether the required-opps property exists (instead of > open-coding that part), and if so, the caller can continue with > of_get_required_opp_performance_state() and then expect it to succeed. > > I have no strong opinion though! Whatever works for me. I would like to work with the existing set of APIs, as the OPP core is going to do that check anyways, again.
11.08.2021 12:45, Rajendra Nayak пишет: >> >> If you have more than one power domain, then this will override the >> pstate which was set for a previous domain. This code doesn't feel solid >> to me, at least a clarifying comment is needed about how it's supposed >> to work. > > I don't quite understand the concern here, this should work with devices > having multiple power-domains as well. __genpd_dev_pm_attach gets called > once per power-domain, and we use the index above to identify the > power-domain. The domain core code supports only one domain per device, see what genpd_set_performance_state() does. This means that the second domain will set the state of the *first* domain, which doesn't make sense. The genpd_set_performance_state() will actually fail with -ENODEV for all domains if you will try to do that.
11.08.2021 22:48, Dmitry Osipenko пишет: > 11.08.2021 12:45, Rajendra Nayak пишет: >>> >>> If you have more than one power domain, then this will override the >>> pstate which was set for a previous domain. This code doesn't feel solid >>> to me, at least a clarifying comment is needed about how it's supposed >>> to work. >> >> I don't quite understand the concern here, this should work with devices >> having multiple power-domains as well. __genpd_dev_pm_attach gets called >> once per power-domain, and we use the index above to identify the >> power-domain. > > The domain core code supports only one domain per device, see what > genpd_set_performance_state() does. This means that the second domain > will set the state of the *first* domain, which doesn't make sense. The > genpd_set_performance_state() will actually fail with -ENODEV for all > domains if you will try to do that. > I confused the base device with the virtual device there, looks like it should be okay then.
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index a934c67..b9b5a9b 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2598,6 +2598,12 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off) dev_dbg(dev, "removing from PM domain %s\n", pd->name); + /* Drop the default performance state */ + if (dev_gpd_data(dev)->default_pstate) { + dev_pm_genpd_set_performance_state(dev, 0); + dev_gpd_data(dev)->default_pstate = 0; + } + for (i = 1; i < GENPD_RETRY_MAX_MS; i <<= 1) { ret = genpd_remove_device(pd, dev); if (ret != -EAGAIN) @@ -2637,6 +2643,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, { struct of_phandle_args pd_args; struct generic_pm_domain *pd; + struct device_node *np; + int pstate; int ret; ret = of_parse_phandle_with_args(dev->of_node, "power-domains", @@ -2675,10 +2683,26 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, genpd_unlock(pd); } - if (ret) + if (ret) { genpd_remove_device(pd, dev); + return -EPROBE_DEFER; + } + + /* Set the default performance state */ + np = dev->of_node; + if (of_parse_phandle(np, "required-opps", index)) { + pstate = of_get_required_opp_performance_state(np, index); + if (pstate < 0) { + ret = pstate; + dev_err(dev, "failed to set required performance state for power-domain %s: %d\n", + pd->name, ret); + } else { + dev_pm_genpd_set_performance_state(dev, pstate); + dev_gpd_data(dev)->default_pstate = pstate; + } + } - return ret ? -EPROBE_DEFER : 1; + return ret ? ret : 1; } /** diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 21a0577..67017c9 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -198,6 +198,7 @@ struct generic_pm_domain_data { struct notifier_block *power_nb; int cpu; unsigned int performance_state; + unsigned int default_pstate; unsigned int rpm_pstate; ktime_t next_wakeup; void *data;
Some devices within power domains with performance states do not support DVFS, but still need to vote on a default/static state while they are active. They can express this using the 'required-opps' property in device tree, which points to the phandle of the OPP supported by the corresponding power-domains. Add support to parse this information from DT and then set the specified performance state during attach and drop it on detach. runtime suspend/resume callbacks already have logic to drop/set the vote as needed and should take care of dropping the default perf state vote on runtime suspend and restore it back on runtime resume. Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- drivers/base/power/domain.c | 28 ++++++++++++++++++++++++++-- include/linux/pm_domain.h | 1 + 2 files changed, 27 insertions(+), 2 deletions(-)