Message ID | 20181211094938.25086-1-rnayak@codeaurora.org |
---|---|
Headers | show |
Series | Add power domain driver for corners on msm8996/sdm845 | expand |
On Tue, 11 Dec 2018 at 10:50, Rajendra Nayak <rnayak@codeaurora.org> wrote: > > Specify the active + sleep and active-only MX power domains as > the parents of the corresponding CX power domains. This will ensure that > performance state requests on CX automatically generate equivalent requests > on MX power domains. > > This is used to enforce a requirement that exists for various > hardware blocks on SDM845 that MX performance state >= CX performance > state for a given operating frequency. I assume that also means the MX power domain must not be power off as long as the CX power domain is powered on? Just to make sure there are no conflicting hierarchical constraints between idle management and performance state management!? Kind regards Uffe > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > This patch is dependent on the series from > Viresh [1] which adds support to propogate performance states across the > power domain hierarchy which is still being reviewed. > > [1] https://lkml.org/lkml/2018/11/26/333 > > drivers/soc/qcom/rpmhpd.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c > index 1ce86f0cc9fa..af7b07d49e4f 100644 > --- a/drivers/soc/qcom/rpmhpd.c > +++ b/drivers/soc/qcom/rpmhpd.c > @@ -102,12 +102,14 @@ static struct rpmhpd sdm845_cx_ao; > static struct rpmhpd sdm845_cx = { > .pd = { .name = "cx", }, > .peer = &sdm845_cx_ao, > + .parent = &sdm845_mx.pd, > .res_name = "cx.lvl", > }; > > static struct rpmhpd sdm845_cx_ao = { > .pd = { .name = "cx_ao", }, > .peer = &sdm845_cx, > + .parent = &sdm845_mx_ao.pd, > .res_name = "cx.lvl", > }; > > @@ -389,6 +391,15 @@ static int rpmhpd_probe(struct platform_device *pdev) > data->domains[i] = &rpmhpds[i]->pd; > } > > + /* Add subdomains */ > + for (i = 0; i < num_pds; i++) { > + if (!rpmhpds[i]) > + continue; > + if (rpmhpds[i]->parent) > + pm_genpd_add_subdomain(rpmhpds[i]->parent, > + &rpmhpds[i]->pd); > + } > + > return of_genpd_add_provider_onecell(pdev->dev.of_node, data); > } > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation >
On 12/11/2018 3:52 PM, Ulf Hansson wrote: > On Tue, 11 Dec 2018 at 10:50, Rajendra Nayak <rnayak@codeaurora.org> wrote: >> >> Specify the active + sleep and active-only MX power domains as >> the parents of the corresponding CX power domains. This will ensure that >> performance state requests on CX automatically generate equivalent requests >> on MX power domains. >> >> This is used to enforce a requirement that exists for various >> hardware blocks on SDM845 that MX performance state >= CX performance >> state for a given operating frequency. > > I assume that also means the MX power domain must not be power off as > long as the CX power domain is powered on? So with rpmh, there's really no separate on/off control, we just put it in the lowest perf state at off. > > Just to make sure there are no conflicting hierarchical constraints > between idle management and performance state management!? > > Kind regards > Uffe > >> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> Acked-by: Viresh Kumar <viresh.kumar@linaro.org> >> --- >> This patch is dependent on the series from >> Viresh [1] which adds support to propogate performance states across the >> power domain hierarchy which is still being reviewed. >> >> [1] https://lkml.org/lkml/2018/11/26/333 >> >> drivers/soc/qcom/rpmhpd.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c >> index 1ce86f0cc9fa..af7b07d49e4f 100644 >> --- a/drivers/soc/qcom/rpmhpd.c >> +++ b/drivers/soc/qcom/rpmhpd.c >> @@ -102,12 +102,14 @@ static struct rpmhpd sdm845_cx_ao; >> static struct rpmhpd sdm845_cx = { >> .pd = { .name = "cx", }, >> .peer = &sdm845_cx_ao, >> + .parent = &sdm845_mx.pd, >> .res_name = "cx.lvl", >> }; >> >> static struct rpmhpd sdm845_cx_ao = { >> .pd = { .name = "cx_ao", }, >> .peer = &sdm845_cx, >> + .parent = &sdm845_mx_ao.pd, >> .res_name = "cx.lvl", >> }; >> >> @@ -389,6 +391,15 @@ static int rpmhpd_probe(struct platform_device *pdev) >> data->domains[i] = &rpmhpds[i]->pd; >> } >> >> + /* Add subdomains */ >> + for (i = 0; i < num_pds; i++) { >> + if (!rpmhpds[i]) >> + continue; >> + if (rpmhpds[i]->parent) >> + pm_genpd_add_subdomain(rpmhpds[i]->parent, >> + &rpmhpds[i]->pd); >> + } >> + >> return of_genpd_add_provider_onecell(pdev->dev.of_node, data); >> } >> >> -- >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member >> of Code Aurora Forum, hosted by The Linux Foundation >>
Quoting Rajendra Nayak (2018-12-11 01:49:36) > Now that we have atleast 2 genpd providers, both using a simple > routine to read a performance state value from device tree and > return it, in order to implement the .opp_to_performance_state > callback, add a simple_opp_to_performance_state() helper to do > it, so it can be resued across all such genpd providers which > just need to read the value from DT. > > Suggested-by: Stephen Boyd <sboyd@kernel.org> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Rajendra Nayak (2018-12-11 01:49:37) > Get rid of the duplicate code across rpmpd and rpmhpd to read the > performance state value from Device tree and use the > simple_opp_to_performance_state() helper instead. > > Suggested-by: Stephen Boyd <sboyd@kernel.org> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Rajendra Nayak (2018-12-11 02:33:23) > > > On 12/11/2018 3:52 PM, Ulf Hansson wrote: > > On Tue, 11 Dec 2018 at 10:50, Rajendra Nayak <rnayak@codeaurora.org> wrote: > >> > >> Specify the active + sleep and active-only MX power domains as > >> the parents of the corresponding CX power domains. This will ensure that > >> performance state requests on CX automatically generate equivalent requests > >> on MX power domains. > >> > >> This is used to enforce a requirement that exists for various > >> hardware blocks on SDM845 that MX performance state >= CX performance > >> state for a given operating frequency. > > > > I assume that also means the MX power domain must not be power off as > > long as the CX power domain is powered on? > > So with rpmh, there's really no separate on/off control, we just put > it in the lowest perf state at off. I think in theory the answer is MX can't be off if CX is on, but in reality, MX and CX are never turned off, just set to something really low and even then the constraint applies for MX >= CX. Is that right? > > > > > Just to make sure there are no conflicting hierarchical constraints > > between idle management and performance state management!? > > I'm not sure what idle states mean to the CX and MX domains. Would it be some sort of idle state governor attached at genpd creation time that would adjust the main SoC power rails when all devices attached are idle? Maybe I don't understand how idle states are different from performance states. My understanding is that devices using these domains would almost always expect their clk frequency and clk on/off state to decide what the performance state is, unless they need to ignore clk state because they aren't managing clks and bump up the voltage directly when the device is active. Either way, devices are actively managing the voltage they need these voltage domains to operate at by using the genpd performance states APIs.
Quoting Rajendra Nayak (2018-12-11 01:49:28) > Changes in v6: > * OPP binding updates for qcom,level reviewed by Rob > * DT bindings for rpmpd and rpmhpd updated to specify the > OPP tables as child nodes of the power-controller itself > * Removed some module specific remains from the drivers, > now that they can only be built-in > * Added a simple_opp_to_performance_state() helper Is the idea that it can be squashed into the patches that use it by reordering the series? But otherwise left at the end of the series to make things simpler to merge without it?
Quoting Rajendra Nayak (2018-12-11 01:49:31) > The Power domains for corners just pass the performance state set by the > consumers to the RPM (Remote Power manager) which then takes care > of setting the appropriate voltage on the corresponding rails to > meet the performance needs. > > We add all power domain data needed on msm8996 here. This driver can easily > be extended by adding data for other qualcomm SoCs as well. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Acked-by: Rob Herring <robh@kernel.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Rajendra Nayak (2018-12-11 01:49:33) > Add rpmpd device node and its OPP table > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Rajendra Nayak (2018-12-11 01:49:34) > The RPMh power domain driver aggregates the corner votes from various > consumers for the ARC resources and communicates it to RPMh. > > With RPMh we use 2 different numbering space for corners, one used > by the clients to express their performance needs, and another used > to communicate to RPMh hardware. > > The clients express their performance requirements using a sparse > numbering space which are mapped to meaningful levels like RET, SVS, > NOMINAL, TURBO etc which then get mapped to another number space > between 0 and 15 which is communicated to RPMh. The sparse number space, > also referred to as vlvl is mapped to the continuous number space of 0 > to 15, also referred to as hlvl, using command DB. > > Some power domain clients could request a performance state only while > the CPU is active, while some others could request for a certain > performance state all the time regardless of the state of the CPU. > We handle this by internally aggregating the votes from both type of > clients and then send the aggregated votes to RPMh. > > There are also 3 different types of Votes that are comunicated to RPMh Why capitalize vote? > for every resource. > 1. ACTIVE_ONLY: This specifies the requirement for the resource when the > CPU is active > 2. SLEEP: This specifies the requirement for the resource when the CPU > is going to sleep > 3. WAKE_ONLY: This specifies the requirement for the resource when the > CPU is coming out of sleep to active state Can you tab these in? > > We add data for all power domains on sdm845 SoC as part of the patch. > The driver can be extended to support other SoCs which support RPMh > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Just minor nitpicks ahead and a warning. > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile > index f1b25fdcf2ad..dd6ca92985ee 100644 > --- a/drivers/soc/qcom/Makefile > +++ b/drivers/soc/qcom/Makefile > @@ -22,3 +22,4 @@ obj-$(CONFIG_QCOM_APR) += apr.o > obj-$(CONFIG_QCOM_LLCC) += llcc-slice.o > obj-$(CONFIG_QCOM_SDM845_LLCC) += llcc-sdm845.o > obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o > +obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o Put this before RPMPD? At least it would be semi-sorted then. > diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c > new file mode 100644 > index 000000000000..f993a86be48c > --- /dev/null > +++ b/drivers/soc/qcom/rpmhpd.c > @@ -0,0 +1,417 @@ [..] > + > +static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd) > +{ > + u8 buf[RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE]; > + int i, j, len, ret; > + > + len = cmd_db_read_aux_data_len(rpmhpd->res_name); > + if (len <= 0) > + return len; > + else if (len > RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE) > + return -EINVAL; > + > + ret = cmd_db_read_aux_data(rpmhpd->res_name, buf, len); > + if (ret < 0) > + return ret; I've changed cmd_db_read_aux_data() and that change is winding through the arm-soc tree. This will break in the compilation phase in linux-next. > + > + rpmhpd->level_count = len / RPMH_ARC_LEVEL_SIZE; > + > + for (i = 0; i < rpmhpd->level_count; i++) { > + rpmhpd->level[i] = 0; > + for (j = 0; j < RPMH_ARC_LEVEL_SIZE; j++) > + rpmhpd->level[i] |= > + buf[i * RPMH_ARC_LEVEL_SIZE + j] << (8 * j); > + > + /* > + * The AUX data may be zero padded. These 0 valued entries at > + * the end of the map must be ignored. > + */ > + if (i > 0 && rpmhpd->level[i] == 0) { > + rpmhpd->level_count = i; > + break; > + } > + pr_debug("%s: ARC hlvl=%2d --> vlvl=%4u\n", rpmhpd->res_name, i, > + rpmhpd->level[i]); > + } > + > + return 0; > +} > + > +static int rpmhpd_probe(struct platform_device *pdev) > +{ > + int i, ret; > + size_t num_pds; > + struct device *dev = &pdev->dev; > + struct genpd_onecell_data *data; > + struct rpmhpd **rpmhpds; > + const struct rpmhpd_desc *desc; > + > + desc = of_device_get_match_data(dev); > + if (!desc) > + return -EINVAL; > + > + rpmhpds = desc->rpmhpds; > + num_pds = desc->num_pds; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->domains = devm_kcalloc(dev, num_pds, sizeof(*data->domains), > + GFP_KERNEL); > + if (!data->domains) > + return -ENOMEM; > + > + data->num_domains = num_pds; > + > + ret = cmd_db_ready(); Does this matter? I thought we forced cmd_db_ready() in the rpmh driver probe, and that was the parent of all rpmh devices so we should be fine to not need to check it again here? > + if (ret) { > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Command DB unavailable, ret=%d\n", ret); > + return ret; > + } > +
Quoting Rajendra Nayak (2018-12-11 01:49:35) > Add the DT node for the rpmhpd powercontroller. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
[]... >>>> This is used to enforce a requirement that exists for various >>>> hardware blocks on SDM845 that MX performance state >= CX performance >>>> state for a given operating frequency. >>> >>> I assume that also means the MX power domain must not be power off as >>> long as the CX power domain is powered on? >> >> So with rpmh, there's really no separate on/off control, we just put >> it in the lowest perf state at off. > > I think in theory the answer is MX can't be off if CX is on, but in > reality, MX and CX are never turned off, just set to something really > low and even then the constraint applies for MX >= CX. Is that right? to some extent, let me clarify this a bit more. rpmh is the central entity controlling these rails and takes a final call on when it can or cannot be 'really' off and when its turned on, at what level it should be set to. CPU is just one of the entities 'voting' on them. Now legacy platforms which had rpm (like msm8996) did have separate control to send a on/off vote and a performance level vote. With rpmh, its just that there is no separate message that you send to rpmh for OFF, its just mapped to the lowest perf level. So think of it as the lowest perf level is mapped to a '0' which is same as OFF. So when you say 'MX can't be off if CX is on', yes MX can't be at the lowest level '0', while CX is at something higher. >>> Just to make sure there are no conflicting hierarchical constraints >>> between idle management and performance state management!? >>> > > I'm not sure what idle states mean to the CX and MX domains. Would it be > some sort of idle state governor attached at genpd creation time that > would adjust the main SoC power rails when all devices attached are > idle? Maybe I don't understand how idle states are different from > performance states. > My understanding is that devices using these domains would almost always > expect their clk frequency and clk on/off state to decide what the > performance state is, unless they need to ignore clk state because they > aren't managing clks and bump up the voltage directly when the device is > active. Either way, devices are actively managing the voltage they need > these voltage domains to operate at by using the genpd performance > states APIs. I am not quite sure whats the point that you are trying to make here, but this is what I would expect the users of these genpds to do, regardless of if they have a clk dependency or not. When the device is active, vote for a performance state they need then request for the genpd to be on. When they are idle, request for the genpd to be turned off.
On 12/12/2018 3:21 AM, Stephen Boyd wrote: > Quoting Rajendra Nayak (2018-12-11 01:49:28) >> Changes in v6: >> * OPP binding updates for qcom,level reviewed by Rob >> * DT bindings for rpmpd and rpmhpd updated to specify the >> OPP tables as child nodes of the power-controller itself >> * Removed some module specific remains from the drivers, >> now that they can only be built-in >> * Added a simple_opp_to_performance_state() helper > > Is the idea that it can be squashed into the patches that use it by > reordering the series? But otherwise left at the end of the series to > make things simpler to merge without it? Yes, I left them at the end so its easier to merge. The PM domains change would need to go via Ulf/Viresh while the SoC drivers using them would go via Andy. So now its only 09/10 that's dependent on 08/10 and not the entire series.
On 12/12/2018 3:31 AM, Stephen Boyd wrote: > Quoting Rajendra Nayak (2018-12-11 01:49:34) >> The RPMh power domain driver aggregates the corner votes from various >> consumers for the ARC resources and communicates it to RPMh. >> >> With RPMh we use 2 different numbering space for corners, one used >> by the clients to express their performance needs, and another used >> to communicate to RPMh hardware. >> >> The clients express their performance requirements using a sparse >> numbering space which are mapped to meaningful levels like RET, SVS, >> NOMINAL, TURBO etc which then get mapped to another number space >> between 0 and 15 which is communicated to RPMh. The sparse number space, >> also referred to as vlvl is mapped to the continuous number space of 0 >> to 15, also referred to as hlvl, using command DB. >> >> Some power domain clients could request a performance state only while >> the CPU is active, while some others could request for a certain >> performance state all the time regardless of the state of the CPU. >> We handle this by internally aggregating the votes from both type of >> clients and then send the aggregated votes to RPMh. >> >> There are also 3 different types of Votes that are comunicated to RPMh > > Why capitalize vote? will fix, > >> for every resource. >> 1. ACTIVE_ONLY: This specifies the requirement for the resource when the >> CPU is active >> 2. SLEEP: This specifies the requirement for the resource when the CPU >> is going to sleep >> 3. WAKE_ONLY: This specifies the requirement for the resource when the >> CPU is coming out of sleep to active state > > Can you tab these in? sure, will do > >> >> We add data for all power domains on sdm845 SoC as part of the patch. >> The driver can be extended to support other SoCs which support RPMh >> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > > Just minor nitpicks ahead and a warning. > >> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile >> index f1b25fdcf2ad..dd6ca92985ee 100644 >> --- a/drivers/soc/qcom/Makefile >> +++ b/drivers/soc/qcom/Makefile >> @@ -22,3 +22,4 @@ obj-$(CONFIG_QCOM_APR) += apr.o >> obj-$(CONFIG_QCOM_LLCC) += llcc-slice.o >> obj-$(CONFIG_QCOM_SDM845_LLCC) += llcc-sdm845.o >> obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o >> +obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o > > Put this before RPMPD? At least it would be semi-sorted then. okay, will do > >> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c >> new file mode 100644 >> index 000000000000..f993a86be48c >> --- /dev/null >> +++ b/drivers/soc/qcom/rpmhpd.c >> @@ -0,0 +1,417 @@ > [..] >> + >> +static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd) >> +{ >> + u8 buf[RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE]; >> + int i, j, len, ret; >> + >> + len = cmd_db_read_aux_data_len(rpmhpd->res_name); >> + if (len <= 0) >> + return len; >> + else if (len > RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE) >> + return -EINVAL; >> + >> + ret = cmd_db_read_aux_data(rpmhpd->res_name, buf, len); >> + if (ret < 0) >> + return ret; > > I've changed cmd_db_read_aux_data() and that change is winding through > the arm-soc tree. This will break in the compilation phase in > linux-next. will resend these based on Andy's for-next > >> + >> + rpmhpd->level_count = len / RPMH_ARC_LEVEL_SIZE; >> + >> + for (i = 0; i < rpmhpd->level_count; i++) { >> + rpmhpd->level[i] = 0; >> + for (j = 0; j < RPMH_ARC_LEVEL_SIZE; j++) >> + rpmhpd->level[i] |= >> + buf[i * RPMH_ARC_LEVEL_SIZE + j] << (8 * j); >> + >> + /* >> + * The AUX data may be zero padded. These 0 valued entries at >> + * the end of the map must be ignored. >> + */ >> + if (i > 0 && rpmhpd->level[i] == 0) { >> + rpmhpd->level_count = i; >> + break; >> + } >> + pr_debug("%s: ARC hlvl=%2d --> vlvl=%4u\n", rpmhpd->res_name, i, >> + rpmhpd->level[i]); >> + } >> + >> + return 0; >> +} >> + >> +static int rpmhpd_probe(struct platform_device *pdev) >> +{ >> + int i, ret; >> + size_t num_pds; >> + struct device *dev = &pdev->dev; >> + struct genpd_onecell_data *data; >> + struct rpmhpd **rpmhpds; >> + const struct rpmhpd_desc *desc; >> + >> + desc = of_device_get_match_data(dev); >> + if (!desc) >> + return -EINVAL; >> + >> + rpmhpds = desc->rpmhpds; >> + num_pds = desc->num_pds; >> + >> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->domains = devm_kcalloc(dev, num_pds, sizeof(*data->domains), >> + GFP_KERNEL); >> + if (!data->domains) >> + return -ENOMEM; >> + >> + data->num_domains = num_pds; >> + >> + ret = cmd_db_ready(); > > Does this matter? I thought we forced cmd_db_ready() in the rpmh driver > probe, and that was the parent of all rpmh devices so we should be fine > to not need to check it again here? sure, will remove. thanks for the review. > >> + if (ret) { >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "Command DB unavailable, ret=%d\n", ret); >> + return ret; >> + } >> +
Quoting Rajendra Nayak (2018-12-11 20:13:13) > > >>> Just to make sure there are no conflicting hierarchical constraints > >>> between idle management and performance state management!? > >>> > > > > I'm not sure what idle states mean to the CX and MX domains. Would it be > > some sort of idle state governor attached at genpd creation time that > > would adjust the main SoC power rails when all devices attached are > > idle? Maybe I don't understand how idle states are different from > > performance states. > > My understanding is that devices using these domains would almost always > > expect their clk frequency and clk on/off state to decide what the > > performance state is, unless they need to ignore clk state because they > > aren't managing clks and bump up the voltage directly when the device is > > active. Either way, devices are actively managing the voltage they need > > these voltage domains to operate at by using the genpd performance > > states APIs. > > I am not quite sure whats the point that you are trying to make here, > but this is what I would expect the users of these genpds to do, > regardless of if they have a clk dependency or not. > When the device is active, vote for a performance state they need > then request for the genpd to be on. When they are idle, request for the > genpd to be turned off. > I believe Ulf is asking because he's proposing to make genpd idle states and genpd performance states orthogonal to each other. And to also make performance states unaffected by the on/off state of the genpd.
On Wed, 12 Dec 2018 at 19:32, Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Rajendra Nayak (2018-12-11 20:13:13) > > > > >>> Just to make sure there are no conflicting hierarchical constraints > > >>> between idle management and performance state management!? > > >>> > > > > > > I'm not sure what idle states mean to the CX and MX domains. Would it be > > > some sort of idle state governor attached at genpd creation time that > > > would adjust the main SoC power rails when all devices attached are > > > idle? Maybe I don't understand how idle states are different from > > > performance states. > > > My understanding is that devices using these domains would almost always > > > expect their clk frequency and clk on/off state to decide what the > > > performance state is, unless they need to ignore clk state because they > > > aren't managing clks and bump up the voltage directly when the device is > > > active. Either way, devices are actively managing the voltage they need > > > these voltage domains to operate at by using the genpd performance > > > states APIs. > > > > I am not quite sure whats the point that you are trying to make here, > > but this is what I would expect the users of these genpds to do, > > regardless of if they have a clk dependency or not. > > When the device is active, vote for a performance state they need > > then request for the genpd to be on. When they are idle, request for the > > genpd to be turned off. > > > > I believe Ulf is asking because he's proposing to make genpd idle states > and genpd performance states orthogonal to each other. And to also make > performance states unaffected by the on/off state of the genpd. Yes, that's one of the reasons. Anyway, I appreciate both of yours descriptive feedback, no further worries from my side! Kind regards Uffe