* [PATCH v4 0/3] GENPD API improvements
@ 2021-01-20 1:50 Dmitry Osipenko
2021-01-20 1:50 ` [PATCH v4 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2021-01-20 1:50 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Ulf Hansson, Peter Geis,
Nicolas Chauvet, Rafael J. Wysocki, Kevin Hilman,
Peter De Schrijver, Viresh Kumar, Stephen Boyd,
Greg Kroah-Hartman, Matt Merhar
Cc: linux-kernel, linux-tegra, linux-pm
Hi,
This is a continuation of [1], where Ulf Hansson suggested to factor out
GENPD patches into a separate series. This series is a prerequisite for
the power domain driver of NVIDIA Tegra SoCs.
[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=221130
Changelog:
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/3] PM: domains: Make set_performance_state() callback optional
2021-01-20 1:50 [PATCH v4 0/3] GENPD API improvements Dmitry Osipenko
@ 2021-01-20 1:50 ` Dmitry Osipenko
2021-01-20 4:39 ` Viresh Kumar
2021-01-20 1:50 ` [PATCH v4 2/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER Dmitry Osipenko
2021-01-20 1:50 ` [PATCH v4 3/3] PM: domains: Add "performance" column to debug summary Dmitry Osipenko
2 siblings, 1 reply; 6+ messages in thread
From: Dmitry Osipenko @ 2021-01-20 1:50 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Ulf Hansson, Peter Geis,
Nicolas Chauvet, Rafael J. Wysocki, Kevin Hilman,
Peter De Schrijver, Viresh Kumar, Stephen Boyd,
Greg Kroah-Hartman, Matt Merhar
Cc: linux-kernel, linux-tegra, linux-pm
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>
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..0bd0cdc30393 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 *src_genpd,
+ struct generic_pm_domain *dst_genpd,
+ unsigned int pstate)
+{
+ if (!dst_genpd->set_performance_state)
+ return pstate;
+
+ return dev_pm_opp_xlate_performance_state(src_genpd->opp_table,
+ dst_genpd->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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 2/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER
2021-01-20 1:50 [PATCH v4 0/3] GENPD API improvements Dmitry Osipenko
2021-01-20 1:50 ` [PATCH v4 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko
@ 2021-01-20 1:50 ` Dmitry Osipenko
2021-01-20 1:50 ` [PATCH v4 3/3] PM: domains: Add "performance" column to debug summary Dmitry Osipenko
2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2021-01-20 1:50 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Ulf Hansson, Peter Geis,
Nicolas Chauvet, Rafael J. Wysocki, Kevin Hilman,
Peter De Schrijver, Viresh Kumar, Stephen Boyd,
Greg Kroah-Hartman, Matt Merhar
Cc: linux-kernel, linux-tegra, linux-pm
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 0bd0cdc30393..def1299ad085 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 3/3] PM: domains: Add "performance" column to debug summary
2021-01-20 1:50 [PATCH v4 0/3] GENPD API improvements Dmitry Osipenko
2021-01-20 1:50 ` [PATCH v4 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko
2021-01-20 1:50 ` [PATCH v4 2/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER Dmitry Osipenko
@ 2021-01-20 1:50 ` Dmitry Osipenko
2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2021-01-20 1:50 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Ulf Hansson, Peter Geis,
Nicolas Chauvet, Rafael J. Wysocki, Kevin Hilman,
Peter De Schrijver, Viresh Kumar, Stephen Boyd,
Greg Kroah-Hartman, Matt Merhar
Cc: linux-kernel, linux-tegra, linux-pm
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 def1299ad085..7c831b00f2ee 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/3] PM: domains: Make set_performance_state() callback optional
2021-01-20 1:50 ` [PATCH v4 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko
@ 2021-01-20 4:39 ` Viresh Kumar
2021-01-20 9:46 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Viresh Kumar @ 2021-01-20 4:39 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Thierry Reding, Jonathan Hunter, Ulf Hansson, Peter Geis,
Nicolas Chauvet, Rafael J. Wysocki, Kevin Hilman,
Peter De Schrijver, Viresh Kumar, Stephen Boyd,
Greg Kroah-Hartman, Matt Merhar, linux-kernel, linux-tegra,
linux-pm
On 20-01-21, 04:50, 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>
> 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..0bd0cdc30393 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 *src_genpd,
> + struct generic_pm_domain *dst_genpd,
> + unsigned int pstate)
I would name them as genpd and parent, that makes it more readable for it.
--
viresh
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/3] PM: domains: Make set_performance_state() callback optional
2021-01-20 4:39 ` Viresh Kumar
@ 2021-01-20 9:46 ` Ulf Hansson
0 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2021-01-20 9:46 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Viresh Kumar, Thierry Reding, Jonathan Hunter, Peter Geis,
Nicolas Chauvet, Rafael J. Wysocki, Kevin Hilman,
Peter De Schrijver, Viresh Kumar, Stephen Boyd,
Greg Kroah-Hartman, Matt Merhar, Linux Kernel Mailing List,
linux-tegra, Linux PM
On Wed, 20 Jan 2021 at 05:39, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 20-01-21, 04:50, 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>
> > 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..0bd0cdc30393 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 *src_genpd,
> > + struct generic_pm_domain *dst_genpd,
> > + unsigned int pstate)
>
> I would name them as genpd and parent, that makes it more readable for it.
... and while at it, probably also drop the "_" as the prefix of the
function name.
Other than that, please add:
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-20 11:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 1:50 [PATCH v4 0/3] GENPD API improvements Dmitry Osipenko
2021-01-20 1:50 ` [PATCH v4 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko
2021-01-20 4:39 ` Viresh Kumar
2021-01-20 9:46 ` Ulf Hansson
2021-01-20 1:50 ` [PATCH v4 2/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER Dmitry Osipenko
2021-01-20 1:50 ` [PATCH v4 3/3] PM: domains: Add "performance" column to debug summary Dmitry Osipenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.