[PATCH v5 0/3] GENPD API improvements

[PATCH v5 0/3] GENPD API improvements

[Dmitry Osipenko]

Hi,

This series is a prerequisite for the power domain driver of NVIDIA Tegra
SoCs [1]. It extends the GENPD core with a better support of performance
states and eases linking of child/parent PDs for the PD drivers.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=221130

Changelog:

v5: - Renamed genpd_xlate_performance_state() and its arguments like it
      was requested by Ulf Hansson and Viresh Kumar in a v4 review.

    - Added r-b from Ulf Hansson to "Make set_performance_state() callback
      optional" patch.

v4: - Updated the "Make set_performance_state() callback optional" patch.
      Now the case where one of intermediate domains doesn't implement
      the set_performance_state() callback is handled properly, thanks
      to Viresh and Ulf for catching this drawback and suggesting the fix.

    - Added more r-bs from Ulf Hansson and Viresh Kumar.

v3: - Added r-b from Ulf Hansson.

    - Added new patch "Make of_genpd_add_subdomain() to return -EPROBE_DEFER",
      which was suggested by Ulf Hansson.

    - Improved "Add "performance" column to debug summary" patch by
      correcting the formatting of debug output, which had a superfluous
      whitespace.


Dmitry Osipenko (3):
  PM: domains: Make set_performance_state() callback optional
  PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER
  PM: domains: Add "performance" column to debug summary

 drivers/base/power/domain.c | 54 +++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 20 deletions(-)

-- 
2.29.2

[PATCH v5 1/3] PM: domains: Make set_performance_state() callback optional

[Dmitry Osipenko]

Make set_performance_state() callback optional in order to remove the
need from power domain drivers to implement a dummy callback. If callback
isn't implemented by a GENPD driver, then the performance state is passed
to the parent domain.

Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
[tested on NVIDIA Tegra20/30/124 SoCs]
Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/base/power/domain.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 9a14eedacb92..5c843772f7f4 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -297,6 +297,18 @@ static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
 	return state;
 }
 
+static int genpd_xlate_performance_state(struct generic_pm_domain *genpd,
+					 struct generic_pm_domain *parent,
+					 unsigned int pstate)
+{
+	if (!parent->set_performance_state)
+		return pstate;
+
+	return dev_pm_opp_xlate_performance_state(genpd->opp_table,
+						  parent->opp_table,
+						  pstate);
+}
+
 static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 					unsigned int state, int depth)
 {
@@ -311,13 +323,8 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 	list_for_each_entry(link, &genpd->child_links, child_node) {
 		parent = link->parent;
 
-		if (!parent->set_performance_state)
-			continue;
-
 		/* Find parent's performance state */
-		ret = dev_pm_opp_xlate_performance_state(genpd->opp_table,
-							 parent->opp_table,
-							 state);
+		ret = genpd_xlate_performance_state(genpd, parent, state);
 		if (unlikely(ret < 0))
 			goto err;
 
@@ -339,9 +346,11 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 			goto err;
 	}
 
-	ret = genpd->set_performance_state(genpd, state);
-	if (ret)
-		goto err;
+	if (genpd->set_performance_state) {
+		ret = genpd->set_performance_state(genpd, state);
+		if (ret)
+			goto err;
+	}
 
 	genpd->performance_state = state;
 	return 0;
@@ -352,9 +361,6 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 					     child_node) {
 		parent = link->parent;
 
-		if (!parent->set_performance_state)
-			continue;
-
 		genpd_lock_nested(parent, depth + 1);
 
 		parent_state = link->prev_performance_state;
@@ -399,9 +405,6 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state)
 	if (!genpd)
 		return -ENODEV;
 
-	if (unlikely(!genpd->set_performance_state))
-		return -EINVAL;
-
 	if (WARN_ON(!dev->power.subsys_data ||
 		     !dev->power.subsys_data->domain_data))
 		return -EINVAL;
-- 
2.29.2

[PATCH v5 2/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER

[Dmitry Osipenko]

Driver of a power domain provider may not be ready at the time of
of_genpd_add_subdomain() invocation. Make this function to return
-EPROBE_DEFER instead of -ENOENT in order to remove a need from
power domain drivers to handle the error code specially.

Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
[tested on NVIDIA Tegra20/30/124 SoCs]
Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 5c843772f7f4..25b346bb2d97 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2466,7 +2466,7 @@ int of_genpd_add_subdomain(struct of_phandle_args *parent_spec,
 out:
 	mutex_unlock(&gpd_list_lock);
 
-	return ret;
+	return ret == -ENOENT ? -EPROBE_DEFER : ret;
 }
 EXPORT_SYMBOL_GPL(of_genpd_add_subdomain);
 
-- 
2.29.2

[PATCH v5 3/3] PM: domains: Add "performance" column to debug summary

[Dmitry Osipenko]

Add "performance" column to debug summary which shows performance state
of all power domains and theirs devices.

Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
[tested on NVIDIA Tegra20/30/124 SoCs]
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/base/power/domain.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 25b346bb2d97..40b72b1d8e8f 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2955,7 +2955,15 @@ static void rtpm_status_str(struct seq_file *s, struct device *dev)
 	else
 		WARN_ON(1);
 
-	seq_puts(s, p);
+	seq_printf(s, "%-25s  ", p);
+}
+
+static void perf_status_str(struct seq_file *s, struct device *dev)
+{
+	struct generic_pm_domain_data *gpd_data;
+
+	gpd_data = to_gpd_data(dev->power.subsys_data->domain_data);
+	seq_put_decimal_ull(s, "", gpd_data->performance_state);
 }
 
 static int genpd_summary_one(struct seq_file *s,
@@ -2983,7 +2991,7 @@ static int genpd_summary_one(struct seq_file *s,
 	else
 		snprintf(state, sizeof(state), "%s",
 			 status_lookup[genpd->status]);
-	seq_printf(s, "%-30s  %-15s ", genpd->name, state);
+	seq_printf(s, "%-30s  %-50s %u", genpd->name, state, genpd->performance_state);
 
 	/*
 	 * Modifications on the list require holding locks on both
@@ -2991,6 +2999,8 @@ static int genpd_summary_one(struct seq_file *s,
 	 * Also genpd->name is immutable.
 	 */
 	list_for_each_entry(link, &genpd->parent_links, parent_node) {
+		if (list_is_first(&link->parent_node, &genpd->parent_links))
+			seq_printf(s, "\n%48s", " ");
 		seq_printf(s, "%s", link->child->name);
 		if (!list_is_last(&link->parent_node, &genpd->parent_links))
 			seq_puts(s, ", ");
@@ -3005,6 +3015,7 @@ static int genpd_summary_one(struct seq_file *s,
 
 		seq_printf(s, "\n    %-50s  ", kobj_path);
 		rtpm_status_str(s, pm_data->dev);
+		perf_status_str(s, pm_data->dev);
 		kfree(kobj_path);
 	}
 
@@ -3020,9 +3031,9 @@ static int summary_show(struct seq_file *s, void *data)
 	struct generic_pm_domain *genpd;
 	int ret = 0;
 
-	seq_puts(s, "domain                          status          children\n");
+	seq_puts(s, "domain                          status          children                           performance\n");
 	seq_puts(s, "    /device                                             runtime status\n");
-	seq_puts(s, "----------------------------------------------------------------------\n");
+	seq_puts(s, "----------------------------------------------------------------------------------------------\n");
 
 	ret = mutex_lock_interruptible(&gpd_list_lock);
 	if (ret)
-- 
2.29.2

Re: [PATCH v5 1/3] PM: domains: Make set_performance_state() callback optional

[Viresh Kumar]

On 21-01-21, 00:12, Dmitry Osipenko wrote:
> Make set_performance_state() callback optional in order to remove the
> need from power domain drivers to implement a dummy callback. If callback
> isn't implemented by a GENPD driver, then the performance state is passed
> to the parent domain.
> 
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Nicolas Chauvet <kwizart@gmail.com>
> Tested-by: Matt Merhar <mattmerhar@protonmail.com>
> [tested on NVIDIA Tegra20/30/124 SoCs]
> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/base/power/domain.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v5 0/3] GENPD API improvements

[Rafael J. Wysocki]

On Thu, Jan 21, 2021 at 12:45 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> Hi,
>
> This series is a prerequisite for the power domain driver of NVIDIA Tegra
> SoCs [1]. It extends the GENPD core with a better support of performance
> states and eases linking of child/parent PDs for the PD drivers.
>
> [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=221130
>
> Changelog:
>
> v5: - Renamed genpd_xlate_performance_state() and its arguments like it
>       was requested by Ulf Hansson and Viresh Kumar in a v4 review.
>
>     - Added r-b from Ulf Hansson to "Make set_performance_state() callback
>       optional" patch.
>
> v4: - Updated the "Make set_performance_state() callback optional" patch.
>       Now the case where one of intermediate domains doesn't implement
>       the set_performance_state() callback is handled properly, thanks
>       to Viresh and Ulf for catching this drawback and suggesting the fix.
>
>     - Added more r-bs from Ulf Hansson and Viresh Kumar.
>
> v3: - Added r-b from Ulf Hansson.
>
>     - Added new patch "Make of_genpd_add_subdomain() to return -EPROBE_DEFER",
>       which was suggested by Ulf Hansson.
>
>     - Improved "Add "performance" column to debug summary" patch by
>       correcting the formatting of debug output, which had a superfluous
>       whitespace.
>
>
> Dmitry Osipenko (3):
>   PM: domains: Make set_performance_state() callback optional
>   PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER
>   PM: domains: Add "performance" column to debug summary
>
>  drivers/base/power/domain.c | 54 +++++++++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 20 deletions(-)
>
> --

All patches applied as 5.12 material, thanks!