Re: [PATCH v2] opp: Power on (virtual) power domains managed by the OPP core

Viresh Kumar
2020-08-31T12:15:06Z

On 26-08-20, 11:33, Stephan Gerhold wrote:
> dev_pm_opp_attach_genpd() allows attaching an arbitrary number of
> power domains to an OPP table. In that case, the genpd core will
> create a virtual device for each of the power domains.
>
> At the moment, the OPP core only calls
> dev_pm_genpd_set_performance_state() on these virtual devices.
> It does not attempt to power on the power domains. Therefore
> the required power domain might never get turned on.
>
> So far, dev_pm_opp_attach_genpd() is only used in qcom-cpufreq-nvmem.c
> to attach the CPR power domain to the CPU OPP table. The CPR driver
> does not check if it was actually powered on so this did not cause
> any problems. However, other drivers (e.g. rpmpd) might ignore the
> performance state until the power domain is actually powered on.
>
> Since these virtual devices are managed exclusively by the OPP core,
> I would say that it should also be responsible to ensure they are
> enabled.
>
> This commit implements this similar to the non-virtual power domains;
> we create device links for each of attached power domains so that they
> are turned on whenever the consumer device is active.
>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---

Applied with some changes, hope that is fine:

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[ Viresh: Rearranged the code to remove extra loop and minor cleanup ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/opp/core.c | 37 ++++++++++++++++++++++++++++++++++++-
drivers/opp/opp.h | 1 +
2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index e65174725a4d..b608b0253079 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -17,6 +17,7 @@
#include <linux/device.h>
#include <linux/export.h>
#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>

#include "opp.h"
@@ -1967,10 +1968,15 @@ static void _opp_detach_genpd(struct opp_table *opp_table)
if (!opp_table->genpd_virt_devs[index])
continue;

+ if (opp_table->genpd_virt_links[index])
+ device_link_del(opp_table->genpd_virt_links[index]);
+
dev_pm_domain_detach(opp_table->genpd_virt_devs[index], false);
opp_table->genpd_virt_devs[index] = NULL;
}

+ kfree(opp_table->genpd_virt_links);
+ opp_table->genpd_virt_links = NULL;
kfree(opp_table->genpd_virt_devs);
opp_table->genpd_virt_devs = NULL;
}
@@ -2002,8 +2008,10 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
{
struct opp_table *opp_table;
struct device *virt_dev;
+ struct device_link *dev_link;
int index = 0, ret = -EINVAL;
const char **name = names;
+ u32 flags;

opp_table = dev_pm_opp_get_opp_table(dev);
if (IS_ERR(opp_table))
@@ -2030,6 +2038,21 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
if (!opp_table->genpd_virt_devs)
goto unlock;

+ opp_table->genpd_virt_links = kcalloc(opp_table->required_opp_count,
+ sizeof(*opp_table->genpd_virt_links),
+ GFP_KERNEL);
+ if (!opp_table->genpd_virt_links) {
+ kfree(opp_table->genpd_virt_devs);
+ opp_table->genpd_virt_devs = NULL;
+ goto unlock;
+ }
+
+ /* Turn on power domain initially if consumer is active */
+ pm_runtime_get_noresume(dev);
+ flags = DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS;
+ if (pm_runtime_active(dev))
+ flags |= DL_FLAG_RPM_ACTIVE;
+
while (*name) {
if (index >= opp_table->required_opp_count) {
dev_err(dev, "Index can't be greater than required-opp-count - 1, %s (%d : %d)\n",
@@ -2043,12 +2066,23 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
dev_err(dev, "Couldn't attach to pm_domain: %d\n", ret);
goto err;
}
-
opp_table->genpd_virt_devs[index] = virt_dev;
+
+ /* Create device links to manage runtime PM */
+ dev_link = device_link_add(dev, opp_table->genpd_virt_devs[index],
+ flags);
+ if (!dev_link) {
+ dev_err(dev, "Failed to create device link\n");
+ goto err;
+ }
+ opp_table->genpd_virt_links[index] = dev_link;
+
index++;
name++;
}

+ pm_runtime_put(dev);
+
if (virt_devs)
*virt_devs = opp_table->genpd_virt_devs;
mutex_unlock(&opp_table->genpd_virt_dev_lock);
@@ -2056,6 +2090,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
return opp_table;

err:
+ pm_runtime_put(dev);
_opp_detach_genpd(opp_table);
unlock:
mutex_unlock(&opp_table->genpd_virt_dev_lock);
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 78e876ec803e..be5526cdbdba 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -186,6 +186,7 @@ struct opp_table {

struct mutex genpd_virt_dev_lock;
struct device **genpd_virt_devs;
+ struct device_link **genpd_virt_links;
struct opp_table **required_opp_tables;
unsigned int required_opp_count;