* [PATCH v3 0/3] GENPD API improvements @ 2021-01-18 1:13 Dmitry Osipenko 2021-01-18 1:13 ` [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Dmitry Osipenko @ 2021-01-18 1:13 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 of the power domain driver for NVIDIA Tegra SoCs. [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=221130 Changelog: 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: Add "performance" column to debug summary PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER drivers/base/power/domain.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) -- 2.29.2 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional 2021-01-18 1:13 [PATCH v3 0/3] GENPD API improvements Dmitry Osipenko @ 2021-01-18 1:13 ` Dmitry Osipenko 2021-01-18 7:28 ` Viresh Kumar 2021-01-18 1:13 ` [PATCH v3 2/3] PM: domains: Add "performance" column to debug summary Dmitry Osipenko 2021-01-18 1:13 ` [PATCH v3 3/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER Dmitry Osipenko 2 siblings, 1 reply; 15+ messages in thread From: Dmitry Osipenko @ 2021-01-18 1:13 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> 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 | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 9a14eedacb92..a3e1bfc233d4 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -339,9 +339,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; @@ -399,9 +401,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] 15+ messages in thread
* Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional 2021-01-18 1:13 ` [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko @ 2021-01-18 7:28 ` Viresh Kumar 2021-01-18 10:59 ` Ulf Hansson 0 siblings, 1 reply; 15+ messages in thread From: Viresh Kumar @ 2021-01-18 7:28 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 18-01-21, 04:13, 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> > 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 | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 9a14eedacb92..a3e1bfc233d4 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -339,9 +339,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; > + } Earlier in this routine we also have this: if (!parent->set_performance_state) continue; Should we change that too ? > > genpd->performance_state = state; > return 0; > @@ -399,9 +401,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; Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional 2021-01-18 7:28 ` Viresh Kumar @ 2021-01-18 10:59 ` Ulf Hansson 2021-01-18 11:05 ` Viresh Kumar 2021-01-18 18:44 ` Dmitry Osipenko 0 siblings, 2 replies; 15+ messages in thread From: Ulf Hansson @ 2021-01-18 10:59 UTC (permalink / raw) To: Viresh Kumar, Dmitry Osipenko Cc: 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 Mon, 18 Jan 2021 at 08:28, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 18-01-21, 04:13, 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> > > 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 | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index 9a14eedacb92..a3e1bfc233d4 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -339,9 +339,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; > > + } > > Earlier in this routine we also have this: > > if (!parent->set_performance_state) > continue; > > Should we change that too ? Good point! I certainly overlooked that when reviewing. We need to reevaluate the new state when propagating to the parent(s). To me, it looks like when doing the propagation we must check if the parent has the ->set_performance_state() callback assigned. If so, we should call dev_pm_opp_xlate_performance_state(), but otherwise just use the value of "state", when doing the reevaluation. Does it make sense? > > > > > genpd->performance_state = state; > > return 0; > > @@ -399,9 +401,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; > > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> > > -- > viresh Kind regards Uffe ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional 2021-01-18 10:59 ` Ulf Hansson @ 2021-01-18 11:05 ` Viresh Kumar 2021-01-18 12:46 ` Ulf Hansson 2021-01-18 18:44 ` Dmitry Osipenko 1 sibling, 1 reply; 15+ messages in thread From: Viresh Kumar @ 2021-01-18 11:05 UTC (permalink / raw) To: Ulf Hansson Cc: Dmitry Osipenko, 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 18-01-21, 11:59, Ulf Hansson wrote: > Good point! I certainly overlooked that when reviewing. We need to > reevaluate the new state when propagating to the parent(s). > > To me, it looks like when doing the propagation we must check if the > parent has the ->set_performance_state() callback assigned. If so, we > should call dev_pm_opp_xlate_performance_state(), but otherwise just > use the value of "state", when doing the reevaluation. > > Does it make sense? That will work but I am wondering if there is a way to avoid the unnecessary propagation if we can somehow find out if someone above in hierarchy supports pstates or not ? -- viresh ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional 2021-01-18 11:05 ` Viresh Kumar @ 2021-01-18 12:46 ` Ulf Hansson 2021-01-19 3:44 ` Viresh Kumar 0 siblings, 1 reply; 15+ messages in thread From: Ulf Hansson @ 2021-01-18 12:46 UTC (permalink / raw) To: Viresh Kumar Cc: Dmitry Osipenko, 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 Mon, 18 Jan 2021 at 12:05, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 18-01-21, 11:59, Ulf Hansson wrote: > > Good point! I certainly overlooked that when reviewing. We need to > > reevaluate the new state when propagating to the parent(s). > > > > To me, it looks like when doing the propagation we must check if the > > parent has the ->set_performance_state() callback assigned. If so, we > > should call dev_pm_opp_xlate_performance_state(), but otherwise just > > use the value of "state", when doing the reevaluation. > > > > Does it make sense? > > That will work but I am wondering if there is a way to avoid the > unnecessary propagation if we can somehow find out if someone above in > hierarchy supports pstates or not ? You seem to be worried about latency/overhead while doing the propagation upwards in the hierarchy. That seems like a reasonable concern to me, especially as the genpd lock is taken at each level. However, to mitigate this can be rather messy. In principle, we would need to walk the hierarchy upwards, each time a new subdomain is added in genpd_add_subdomain(). While doing this, we would also need to keep track on what level we set to continue the propagation of the performance states for. Even if this can be done in non-latency sensitive paths, I don't think it's worth it because of complexity (I haven't even thought of what happens when removing a subdomain). So, maybe we should simply just stick to the existing code, forcing the parent to have a ->set_performance() callback assigned if propagation should continue? Kind regards Uffe ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional 2021-01-18 12:46 ` Ulf Hansson @ 2021-01-19 3:44 ` Viresh Kumar 2021-01-19 9:52 ` Ulf Hansson 0 siblings, 1 reply; 15+ messages in thread From: Viresh Kumar @ 2021-01-19 3:44 UTC (permalink / raw) To: Ulf Hansson Cc: Dmitry Osipenko, 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 18-01-21, 13:46, Ulf Hansson wrote: > You seem to be worried about latency/overhead while doing the > propagation upwards in the hierarchy. That seems like a reasonable > concern to me, especially as the genpd lock is taken at each level. I am not sure how many levels of domains we have normally, but unless the number is big it won't be a very big problem. > However, to mitigate this can be rather messy. In principle, we would > need to walk the hierarchy upwards, each time a new subdomain is added > in genpd_add_subdomain(). While doing this, we would also need to keep > track on what level we set to continue the propagation of the > performance states for. Even if this can be done in non-latency > sensitive paths, I don't think it's worth it because of complexity (I > haven't even thought of what happens when removing a subdomain). What about a new field in the domain structure like 'can-handle-pstates', and then whenever sub-domain gets added it just needs to check its parent's field and set his own. > So, maybe we should simply just stick to the existing code, forcing > the parent to have a ->set_performance() callback assigned if > propagation should continue? I think it would be better to fix the issue even if we aren't fully optimized and making the change to make sure we keep propagating is rather important. -- viresh ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional 2021-01-19 3:44 ` Viresh Kumar @ 2021-01-19 9:52 ` Ulf Hansson 2021-01-19 9:55 ` Viresh Kumar 0 siblings, 1 reply; 15+ messages in thread From: Ulf Hansson @ 2021-01-19 9:52 UTC (permalink / raw) To: Viresh Kumar Cc: Dmitry Osipenko, 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 Tue, 19 Jan 2021 at 04:44, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 18-01-21, 13:46, Ulf Hansson wrote: > > You seem to be worried about latency/overhead while doing the > > propagation upwards in the hierarchy. That seems like a reasonable > > concern to me, especially as the genpd lock is taken at each level. > > I am not sure how many levels of domains we have normally, but unless the number > is big it won't be a very big problem. It depends on the SoC topology, so obviously it differs. But more than a couple is unusual, I would say. However, I think it may also depend on how many devices that are hooked up to the domains and how actively these are being used. > > > However, to mitigate this can be rather messy. In principle, we would > > need to walk the hierarchy upwards, each time a new subdomain is added > > in genpd_add_subdomain(). While doing this, we would also need to keep > > track on what level we set to continue the propagation of the > > performance states for. Even if this can be done in non-latency > > sensitive paths, I don't think it's worth it because of complexity (I > > haven't even thought of what happens when removing a subdomain). > > What about a new field in the domain structure like 'can-handle-pstates', and > then whenever sub-domain gets added it just needs to check its parent's field > and set his own. That would work if the topology is built from top to bottom, but I don't think we can rely on that. For example, when a domain A is added as a child to domain B, domain B doesn't have a parent yet (and the "can-handle-pstates" don't get set for neither domain A or domain B). Next, domain B is added as child domain to domain C. Domain C has the "can-handle-pstates" set, which means domain B gets the "can-handle-pstates" set as well. This means domain A, will not have "can-handle-pstates" set, while it probably should have. > > > So, maybe we should simply just stick to the existing code, forcing > > the parent to have a ->set_performance() callback assigned if > > propagation should continue? > > I think it would be better to fix the issue even if we aren't fully optimized > and making the change to make sure we keep propagating is rather important. Alright, let's continue with Dmitry's patches and discuss this further when v4 is out, as he seems to have it almost ready. Kind regards Uffe ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional 2021-01-19 9:52 ` Ulf Hansson @ 2021-01-19 9:55 ` Viresh Kumar 0 siblings, 0 replies; 15+ messages in thread From: Viresh Kumar @ 2021-01-19 9:55 UTC (permalink / raw) To: Ulf Hansson Cc: Dmitry Osipenko, 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 19-01-21, 10:52, Ulf Hansson wrote: > That would work if the topology is built from top to bottom, but I > don't think we can rely on that. > > For example, when a domain A is added as a child to domain B, domain B > doesn't have a parent yet (and the "can-handle-pstates" don't get set > for neither domain A or domain B). Next, domain B is added as child > domain to domain C. Domain C has the "can-handle-pstates" set, which > means domain B gets the "can-handle-pstates" set as well. This means > domain A, will not have "can-handle-pstates" set, while it probably > should have. Okay, I missed that part. > > > > > So, maybe we should simply just stick to the existing code, forcing > > > the parent to have a ->set_performance() callback assigned if > > > propagation should continue? > > > > I think it would be better to fix the issue even if we aren't fully optimized > > and making the change to make sure we keep propagating is rather important. > > Alright, let's continue with Dmitry's patches and discuss this further > when v4 is out, as he seems to have it almost ready. Right. -- viresh ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional 2021-01-18 10:59 ` Ulf Hansson 2021-01-18 11:05 ` Viresh Kumar @ 2021-01-18 18:44 ` Dmitry Osipenko 1 sibling, 0 replies; 15+ messages in thread From: Dmitry Osipenko @ 2021-01-18 18:44 UTC (permalink / raw) To: Ulf Hansson, Viresh Kumar Cc: 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 18.01.2021 13:59, Ulf Hansson пишет: > On Mon, 18 Jan 2021 at 08:28, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> >> On 18-01-21, 04:13, 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> >>> 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 | 11 +++++------ >>> 1 file changed, 5 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >>> index 9a14eedacb92..a3e1bfc233d4 100644 >>> --- a/drivers/base/power/domain.c >>> +++ b/drivers/base/power/domain.c >>> @@ -339,9 +339,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; >>> + } >> >> Earlier in this routine we also have this: >> >> if (!parent->set_performance_state) >> continue; >> >> Should we change that too ? > > Good point! I certainly overlooked that when reviewing. We need to > reevaluate the new state when propagating to the parent(s). > > To me, it looks like when doing the propagation we must check if the > parent has the ->set_performance_state() callback assigned. If so, we > should call dev_pm_opp_xlate_performance_state(), but otherwise just > use the value of "state", when doing the reevaluation. > > Does it make sense? I played a tad with the power domains by creating a couple dummy domains and putting them into a parent->child chain. Yours suggestion works well for the case where intermediate domain doesn't implement the set_performance_state() callback, i.e. the performance state is propagated through the whole chain instead of stopping on the domain that doesn't implement the callback. I'll make a v4, thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 2/3] PM: domains: Add "performance" column to debug summary 2021-01-18 1:13 [PATCH v3 0/3] GENPD API improvements Dmitry Osipenko 2021-01-18 1:13 ` [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko @ 2021-01-18 1:13 ` Dmitry Osipenko 2021-01-18 7:31 ` Viresh Kumar 2021-01-18 1:13 ` [PATCH v3 3/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER Dmitry Osipenko 2 siblings, 1 reply; 15+ messages in thread From: Dmitry Osipenko @ 2021-01-18 1:13 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> Reviewed-by: Ulf Hansson <ulf.hansson@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 a3e1bfc233d4..2ab77aa516eb 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2951,7 +2951,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, @@ -2979,7 +2987,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 @@ -2987,6 +2995,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, ", "); @@ -3001,6 +3011,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); } @@ -3016,9 +3027,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] 15+ messages in thread
* Re: [PATCH v3 2/3] PM: domains: Add "performance" column to debug summary 2021-01-18 1:13 ` [PATCH v3 2/3] PM: domains: Add "performance" column to debug summary Dmitry Osipenko @ 2021-01-18 7:31 ` Viresh Kumar 0 siblings, 0 replies; 15+ messages in thread From: Viresh Kumar @ 2021-01-18 7:31 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 18-01-21, 04:13, Dmitry Osipenko wrote: > 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> > Reviewed-by: Ulf Hansson <ulf.hansson@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 a3e1bfc233d4..2ab77aa516eb 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -2951,7 +2951,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, > @@ -2979,7 +2987,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 > @@ -2987,6 +2995,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, ", "); > @@ -3001,6 +3011,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); > } > > @@ -3016,9 +3027,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) Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 3/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER 2021-01-18 1:13 [PATCH v3 0/3] GENPD API improvements Dmitry Osipenko 2021-01-18 1:13 ` [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko 2021-01-18 1:13 ` [PATCH v3 2/3] PM: domains: Add "performance" column to debug summary Dmitry Osipenko @ 2021-01-18 1:13 ` Dmitry Osipenko 2021-01-18 7:32 ` Viresh Kumar 2021-01-18 10:32 ` Ulf Hansson 2 siblings, 2 replies; 15+ messages in thread From: Dmitry Osipenko @ 2021-01-18 1:13 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. Suggested-by: Ulf Hansson <ulf.hansson@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 2ab77aa516eb..7ef6764ee30f 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2462,7 +2462,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] 15+ messages in thread
* Re: [PATCH v3 3/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER 2021-01-18 1:13 ` [PATCH v3 3/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER Dmitry Osipenko @ 2021-01-18 7:32 ` Viresh Kumar 2021-01-18 10:32 ` Ulf Hansson 1 sibling, 0 replies; 15+ messages in thread From: Viresh Kumar @ 2021-01-18 7:32 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 18-01-21, 04:13, Dmitry Osipenko wrote: > 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. > > Suggested-by: Ulf Hansson <ulf.hansson@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 2ab77aa516eb..7ef6764ee30f 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -2462,7 +2462,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); > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER 2021-01-18 1:13 ` [PATCH v3 3/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER Dmitry Osipenko 2021-01-18 7:32 ` Viresh Kumar @ 2021-01-18 10:32 ` Ulf Hansson 1 sibling, 0 replies; 15+ messages in thread From: Ulf Hansson @ 2021-01-18 10:32 UTC (permalink / raw) To: Dmitry Osipenko Cc: 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 Mon, 18 Jan 2021 at 02:13, Dmitry Osipenko <digetx@gmail.com> wrote: > > 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. > > Suggested-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe > --- > 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 2ab77aa516eb..7ef6764ee30f 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -2462,7 +2462,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 [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-01-19 12:07 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-18 1:13 [PATCH v3 0/3] GENPD API improvements Dmitry Osipenko 2021-01-18 1:13 ` [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional Dmitry Osipenko 2021-01-18 7:28 ` Viresh Kumar 2021-01-18 10:59 ` Ulf Hansson 2021-01-18 11:05 ` Viresh Kumar 2021-01-18 12:46 ` Ulf Hansson 2021-01-19 3:44 ` Viresh Kumar 2021-01-19 9:52 ` Ulf Hansson 2021-01-19 9:55 ` Viresh Kumar 2021-01-18 18:44 ` Dmitry Osipenko 2021-01-18 1:13 ` [PATCH v3 2/3] PM: domains: Add "performance" column to debug summary Dmitry Osipenko 2021-01-18 7:31 ` Viresh Kumar 2021-01-18 1:13 ` [PATCH v3 3/3] PM: domains: Make of_genpd_add_subdomain() to return -EPROBE_DEFER Dmitry Osipenko 2021-01-18 7:32 ` Viresh Kumar 2021-01-18 10:32 ` Ulf Hansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).