From: Viresh Kumar <viresh.kumar@linaro.org> To: Ulf Hansson <ulf.hansson@linaro.org> Cc: Sudeep Holla <sudeep.holla@arm.com>, Cristian Marussi <cristian.marussi@arm.com>, Viresh Kumar <vireshk@kernel.org>, "Rafael J . Wysocki" <rafael@kernel.org>, Nishanth Menon <nm@ti.com>, Stephen Boyd <sboyd@kernel.org>, Nikunj Kela <nkela@quicinc.com>, Prasad Sodagudi <psodagud@quicinc.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps Date: Tue, 3 Oct 2023 13:53:22 +0530 [thread overview] Message-ID: <20231003082322.cmok766jxzftsgrg@vireshk-i7> (raw) In-Reply-To: <CAPDyKFo+XCEtTSNqtA1SeajWo4tuRroA4GrRegta5TsqSGd4eQ@mail.gmail.com> On 25-09-23, 15:33, Ulf Hansson wrote: > On Mon, 25 Sept 2023 at 15:18, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > At this point the level (performance state) for an OPP is currently limited > > to be requested for a device that is attached to a PM domain. Moreover, > > the device needs to have the so called required-opps assigned to it, which > > are based upon OPP tables being described in DT. > > > > To extend the support beyond required-opps and DT, let's enable the level > > to be set for all OPPs. More precisely, if the requested OPP has a valid > > level let's try to request it through the device's optional PM domain, via > > calling dev_pm_domain_set_performance_state(). > > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > --- > > drivers/opp/core.c | 29 +++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > > index 60dca60ac4af..afb73978cdcb 100644 > > --- a/drivers/opp/core.c > > +++ b/drivers/opp/core.c > > @@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table) > > opp_table->set_required_opps = _opp_set_required_opps_generic; > > } > > > > +static int _set_opp_level(struct device *dev, struct opp_table *opp_table, > > + struct dev_pm_opp *opp) > > +{ > > + int ret = 0; > > + > > + /* Request a new performance state through the device's PM domain. */ > > + if (opp && opp->level) { > > + ret = dev_pm_domain_set_performance_state(dev, opp->level); > > + if (ret) > > + dev_err(dev, "Failed to set performance state %u (%d)\n", > > + opp->level, ret); > > + } > > Okay, so reviewing my own code found a problem here. We need an "else" > here, that should request the performance state to be set to 0. > > I am not sending a new version at this point, but awaiting more feedback first. I am looking to add below to this patch, is that okay with you ? diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 7b505316bb1c..a113e9caaa5a 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1135,16 +1135,22 @@ void _update_set_required_opps(struct opp_table *opp_table) static int _set_opp_level(struct device *dev, struct opp_table *opp_table, struct dev_pm_opp *opp) { + unsigned int level = 0; int ret = 0; - /* Request a new performance state through the device's PM domain. */ - if (opp && opp->level) { - ret = dev_pm_domain_set_performance_state(dev, opp->level); - if (ret) - dev_err(dev, "Failed to set performance state %u (%d)\n", - opp->level, ret); + if (opp) { + if (!opp->level) + return 0; + + level = opp->level; } + /* Request a new performance state through the device's PM domain. */ + ret = dev_pm_genpd_set_performance_state(dev, level); + if (ret) + dev_err(dev, "Failed to set performance state %u (%d)\n", level, + ret); + return ret; } I am switching back to dev_pm_genpd_set_performance_state() as I won't be applying the power domain changes. I will then push out a branch and you can rebase your patches on top of it ? And then probably Sudeep or someone else can apply everything ? -- viresh
WARNING: multiple messages have this Message-ID (diff)
From: Viresh Kumar <viresh.kumar@linaro.org> To: Ulf Hansson <ulf.hansson@linaro.org> Cc: Sudeep Holla <sudeep.holla@arm.com>, Cristian Marussi <cristian.marussi@arm.com>, Viresh Kumar <vireshk@kernel.org>, "Rafael J . Wysocki" <rafael@kernel.org>, Nishanth Menon <nm@ti.com>, Stephen Boyd <sboyd@kernel.org>, Nikunj Kela <nkela@quicinc.com>, Prasad Sodagudi <psodagud@quicinc.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps Date: Tue, 3 Oct 2023 13:53:22 +0530 [thread overview] Message-ID: <20231003082322.cmok766jxzftsgrg@vireshk-i7> (raw) In-Reply-To: <CAPDyKFo+XCEtTSNqtA1SeajWo4tuRroA4GrRegta5TsqSGd4eQ@mail.gmail.com> On 25-09-23, 15:33, Ulf Hansson wrote: > On Mon, 25 Sept 2023 at 15:18, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > At this point the level (performance state) for an OPP is currently limited > > to be requested for a device that is attached to a PM domain. Moreover, > > the device needs to have the so called required-opps assigned to it, which > > are based upon OPP tables being described in DT. > > > > To extend the support beyond required-opps and DT, let's enable the level > > to be set for all OPPs. More precisely, if the requested OPP has a valid > > level let's try to request it through the device's optional PM domain, via > > calling dev_pm_domain_set_performance_state(). > > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > --- > > drivers/opp/core.c | 29 +++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > > index 60dca60ac4af..afb73978cdcb 100644 > > --- a/drivers/opp/core.c > > +++ b/drivers/opp/core.c > > @@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table) > > opp_table->set_required_opps = _opp_set_required_opps_generic; > > } > > > > +static int _set_opp_level(struct device *dev, struct opp_table *opp_table, > > + struct dev_pm_opp *opp) > > +{ > > + int ret = 0; > > + > > + /* Request a new performance state through the device's PM domain. */ > > + if (opp && opp->level) { > > + ret = dev_pm_domain_set_performance_state(dev, opp->level); > > + if (ret) > > + dev_err(dev, "Failed to set performance state %u (%d)\n", > > + opp->level, ret); > > + } > > Okay, so reviewing my own code found a problem here. We need an "else" > here, that should request the performance state to be set to 0. > > I am not sending a new version at this point, but awaiting more feedback first. I am looking to add below to this patch, is that okay with you ? diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 7b505316bb1c..a113e9caaa5a 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1135,16 +1135,22 @@ void _update_set_required_opps(struct opp_table *opp_table) static int _set_opp_level(struct device *dev, struct opp_table *opp_table, struct dev_pm_opp *opp) { + unsigned int level = 0; int ret = 0; - /* Request a new performance state through the device's PM domain. */ - if (opp && opp->level) { - ret = dev_pm_domain_set_performance_state(dev, opp->level); - if (ret) - dev_err(dev, "Failed to set performance state %u (%d)\n", - opp->level, ret); + if (opp) { + if (!opp->level) + return 0; + + level = opp->level; } + /* Request a new performance state through the device's PM domain. */ + ret = dev_pm_genpd_set_performance_state(dev, level); + if (ret) + dev_err(dev, "Failed to set performance state %u (%d)\n", level, + ret); + return ret; } I am switching back to dev_pm_genpd_set_performance_state() as I won't be applying the power domain changes. I will then push out a branch and you can rebase your patches on top of it ? And then probably Sudeep or someone else can apply everything ? -- viresh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-10-03 8:24 UTC|newest] Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-09-25 13:17 [PATCH 0/9] PM: domains/opp/arm_scmi: Extend performance scaling support Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-25 13:17 ` [PATCH 1/9] PM: domains: Introduce dev_pm_domain_set_performance_state() Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-25 13:17 ` [PATCH 2/9] PM: domains: Implement the ->set_performance_state() callback for genpd Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-25 13:17 ` [PATCH 3/9] OPP: Add dev_pm_opp_add_dynamic() to allow more flexibility Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-25 13:17 ` [PATCH 4/9] OPP: Extend dev_pm_opp_data with a level Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-25 13:17 ` [PATCH 5/9] OPP: Switch to use dev_pm_domain_set_performance_state() Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-25 13:17 ` [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-25 13:33 ` Ulf Hansson 2023-09-25 13:33 ` Ulf Hansson 2023-10-03 8:23 ` Viresh Kumar [this message] 2023-10-03 8:23 ` Viresh Kumar 2023-10-03 12:36 ` Ulf Hansson 2023-10-03 12:36 ` Ulf Hansson 2023-10-04 5:04 ` Viresh Kumar 2023-10-04 5:04 ` Viresh Kumar 2023-10-04 9:06 ` Ulf Hansson 2023-10-04 9:06 ` Ulf Hansson 2023-09-25 13:17 ` [PATCH 7/9] firmware: arm_scmi: Simplify error path in scmi_dvfs_device_opps_add() Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-25 13:17 ` [PATCH 8/9] firmware: arm_scmi: Specify the performance level when adding an OPP Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-25 13:17 ` [PATCH 9/9] firmware: arm_scmi: Add generic OPP support to the SCMI performance domain Ulf Hansson 2023-09-25 13:17 ` Ulf Hansson 2023-09-29 16:25 ` Sudeep Holla 2023-09-29 16:25 ` Sudeep Holla 2023-10-03 8:21 ` Viresh Kumar 2023-10-03 8:21 ` Viresh Kumar 2023-10-03 11:26 ` Sudeep Holla 2023-10-03 11:26 ` Sudeep Holla 2023-10-04 5:08 ` Viresh Kumar 2023-10-04 5:08 ` Viresh Kumar 2023-10-05 15:06 ` Sudeep Holla 2023-10-05 15:06 ` Sudeep Holla 2023-10-06 7:09 ` Viresh Kumar 2023-10-06 7:09 ` Viresh Kumar 2023-10-06 9:43 ` Sudeep Holla 2023-10-06 9:43 ` Sudeep Holla 2023-09-26 16:54 ` [PATCH 0/9] PM: domains/opp/arm_scmi: Extend performance scaling support Rafael J. Wysocki 2023-09-26 16:54 ` Rafael J. Wysocki 2023-10-06 14:30 ` (subset) " Sudeep Holla 2023-10-06 14:30 ` Sudeep Holla
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20231003082322.cmok766jxzftsgrg@vireshk-i7 \ --to=viresh.kumar@linaro.org \ --cc=alexandre.torgue@foss.st.com \ --cc=cristian.marussi@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=nkela@quicinc.com \ --cc=nm@ti.com \ --cc=psodagud@quicinc.com \ --cc=rafael@kernel.org \ --cc=sboyd@kernel.org \ --cc=sudeep.holla@arm.com \ --cc=ulf.hansson@linaro.org \ --cc=vireshk@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.