Qcom clock performance votes on mainline
Subsystems (2)
Hi Stephen, Hi Rajendra,
while working on some MSM8916 things I've been staring at the downstream clock-gcc-8916.c [1] driver a bit. One thing that confuses me are the voltage/performance state votes that are made for certain clocks within the driver. Specifically lines like
VDD_DIG_FMAX_MAP2(LOW, 32000000, NOMINAL, 64000000),
on certain clocks like UART, I2C or SPI. There does not seem to be anything equivalent in the mainline clock driver at the moment.
As far as I understand from related discussions on mailing lists [2], these performance votes are not supposed to be added to the clock driver(s), but rather as required-opps within OPP tables of all the consumers. Is that correct?
As a second question, I'm wondering about one particular case:
I've been trying to get CPR / all the CPU frequencies working on MSM8916. For that, I already added performance state votes for VDDMX and CPR as required-opps to the CPU OPP table.
After a recent discussion [3] with Viresh about where to enable power domains managed by the OPP core, I've been looking at all the performance state votes made in the downstream kernel again.
Actually, the A53 PLL used for the higher CPU frequencies also has such voltage/performance state votes. The downstream driver declares the clock like [4]:
.vdd_class = &vdd_sr2_pll,
.fmax = (unsigned long [VDD_SR2_PLL_NUM]) {
[VDD_SR2_PLL_SVS] = 1000000000,
[VDD_SR2_PLL_NOM] = 1900000000,
},
.num_fmax = VDD_SR2_PLL_NUM,which ends up as votes for the VDDCX power domain.
Now I'm wondering: Where should I make these votes on mainline? Should I add it as yet another required-opps to the CPU OPP table?
It would be a bit of a special case because these votes are only done for the A53 PLL (which is only used for the higher CPU frequencies, not the lower ones)...
Thanks in advance!
Stephan
[1]: https://source.codeaurora.org/quic/la/kernel/msm-3.10/tree/drivers/clk/qcom/clock-gcc-8916.c?h=LA.BR.1.2.9.1-02310-8x16.0
[2]: https://lore.kernel.org/linux-arm-msm/[email protected]/
[3]: https://lore.kernel.org/linux-pm/[email protected]/
[4]: https://source.codeaurora.org/quic/la/kernel/msm-3.10/tree/drivers/clk/qcom/clock-gcc-8916.c?h=LA.BR.1.2.9.1-02310-8x16.0#n354
Stephen Boyd
maintainerQuoting Stephan Gerhold (2020-09-10 09:26:10)
Quoted 9 lines
Hi Stephen, Hi Rajendra,
while working on some MSM8916 things I've been staring at the downstream
clock-gcc-8916.c[1] driver a bit. One thing that confuses me are the voltage/performance state votes that are made for certain clocks within the driver. Specifically lines likeVDD_DIG_FMAX_MAP2(LOW, 32000000, NOMINAL, 64000000),
Quoted 7 lines
on certain clocks like UART, I2C or SPI. There does not seem to be anything equivalent in the mainline clock driver at the moment.
As far as I understand from related discussions on mailing lists [2], these performance votes are not supposed to be added to the clock driver(s), but rather as required-opps within OPP tables of all the consumers. Is that correct?
Yes.
Quoted 21 lines
As a second question, I'm wondering about one particular case:
I've been trying to get CPR / all the CPU frequencies working on MSM8916. For that, I already added performance state votes for VDDMX and CPR as required-opps to the CPU OPP table.After a recent discussion [3] with Viresh about where to enable power domains managed by the OPP core, I've been looking at all the performance state votes made in the downstream kernel again.
Actually, the A53 PLL used for the higher CPU frequencies also has such voltage/performance state votes. The downstream driver declares the clock like [4]:
.vdd_class = &vdd_sr2_pll, .fmax = (unsigned long [VDD_SR2_PLL_NUM]) { [VDD_SR2_PLL_SVS] = 1000000000, [VDD_SR2_PLL_NOM] = 1900000000, }, .num_fmax = VDD_SR2_PLL_NUM,
Quoted 4 lines
which ends up as votes for the VDDCX power domain.
Now I'm wondering: Where should I make these votes on mainline? Should I add it as yet another required-opps to the CPU OPP table?
Sounds like the right approach.
Quoted 4 lines
It would be a bit of a special case because these votes are only done for the A53 PLL (which is only used for the higher CPU frequencies, not the lower ones)...
Can that be put into the OPP table somehow for only the high frequencies? The OPP tables for CPUs sometimes cover the CPU PLL voltage requirements too so it doesn't seem like a totally bad idea.
Quoted 41 lines · Stephen Boyd
Quoting Stephan Gerhold (2020-09-10 09:26:10)
Quoted 9 lines
Hi Stephen, Hi Rajendra,
while working on some MSM8916 things I've been staring at the downstream
clock-gcc-8916.c[1] driver a bit. One thing that confuses me are the voltage/performance state votes that are made for certain clocks within the driver. Specifically lines likeVDD_DIG_FMAX_MAP2(LOW, 32000000, NOMINAL, 64000000),Quoted 7 lines
on certain clocks like UART, I2C or SPI. There does not seem to be anything equivalent in the mainline clock driver at the moment.
As far as I understand from related discussions on mailing lists [2], these performance votes are not supposed to be added to the clock driver(s), but rather as required-opps within OPP tables of all the consumers. Is that correct?
Yes.
Quoted 21 lines
As a second question, I'm wondering about one particular case:
I've been trying to get CPR / all the CPU frequencies working on MSM8916. For that, I already added performance state votes for VDDMX and CPR as required-opps to the CPU OPP table.After a recent discussion [3] with Viresh about where to enable power domains managed by the OPP core, I've been looking at all the performance state votes made in the downstream kernel again.
Actually, the A53 PLL used for the higher CPU frequencies also has such voltage/performance state votes. The downstream driver declares the clock like [4]:
.vdd_class = &vdd_sr2_pll, .fmax = (unsigned long [VDD_SR2_PLL_NUM]) { [VDD_SR2_PLL_SVS] = 1000000000, [VDD_SR2_PLL_NOM] = 1900000000, }, .num_fmax = VDD_SR2_PLL_NUM,
Quoted 7 lines
Quoted 4 lines
which ends up as votes for the VDDCX power domain.
Now I'm wondering: Where should I make these votes on mainline? Should I add it as yet another required-opps to the CPU OPP table?
Sounds like the right approach.
Thanks for the quick reply!
Quoted 8 lines
Quoted 4 lines
It would be a bit of a special case because these votes are only done for the A53 PLL (which is only used for the higher CPU frequencies, not the lower ones)...
Can that be put into the OPP table somehow for only the high frequencies? The OPP tables for CPUs sometimes cover the CPU PLL voltage requirements too so it doesn't seem like a totally bad idea.
I don't think it's possible at the moment, but actually Viresh mentioned that use case (scaling a power domain only for some of the OPPs) when we discussed where to enable the power domains listed in the OPP table [1].
Problem back then was that we didn't have a real example where this would be needed. It seems like such an example exists now, so I will discuss ways to implement that with Viresh.
I just wanted to be sure that adding the additional power domain to the CPU OPP table is the right approach.
Thanks again!
Stephan
[1]: https://lore.kernel.org/linux-pm/20200828063511.y47ofywtu5qo57bq@vireshk-i7/
Quoted 44 lines · Stephan Gerhold
Quoted 43 lines · Stephen Boyd
Quoting Stephan Gerhold (2020-09-10 09:26:10)
Quoted 9 lines
Hi Stephen, Hi Rajendra,
while working on some MSM8916 things I've been staring at the downstream
clock-gcc-8916.c[1] driver a bit. One thing that confuses me are the voltage/performance state votes that are made for certain clocks within the driver. Specifically lines likeVDD_DIG_FMAX_MAP2(LOW, 32000000, NOMINAL, 64000000),Quoted 7 lines
on certain clocks like UART, I2C or SPI. There does not seem to be anything equivalent in the mainline clock driver at the moment.
As far as I understand from related discussions on mailing lists [2], these performance votes are not supposed to be added to the clock driver(s), but rather as required-opps within OPP tables of all the consumers. Is that correct?
Yes.
Quoted 23 lines
As a second question, I'm wondering about one particular case:
I've been trying to get CPR / all the CPU frequencies working on MSM8916. For that, I already added performance state votes for VDDMX and CPR as required-opps to the CPU OPP table.After a recent discussion [3] with Viresh about where to enable power domains managed by the OPP core, I've been looking at all the performance state votes made in the downstream kernel again.
Actually, the A53 PLL used for the higher CPU frequencies also has such voltage/performance state votes. The downstream driver declares the clock like [4]:
.vdd_class = &vdd_sr2_pll, .fmax = (unsigned long [VDD_SR2_PLL_NUM]) { [VDD_SR2_PLL_SVS] = 1000000000, [VDD_SR2_PLL_NOM] = 1900000000, }, .num_fmax = VDD_SR2_PLL_NUM,which ends up as votes for the VDDCX power domain.
Quoted 16 lines
Quoted 5 lines
Quoted 2 lines
Now I'm wondering: Where should I make these votes on mainline? Should I add it as yet another required-opps to the CPU OPP table?
Sounds like the right approach.
Thanks for the quick reply!
Quoted 8 lines
Quoted 4 lines
It would be a bit of a special case because these votes are only done for the A53 PLL (which is only used for the higher CPU frequencies, not the lower ones)...
Can that be put into the OPP table somehow for only the high frequencies? The OPP tables for CPUs sometimes cover the CPU PLL voltage requirements too so it doesn't seem like a totally bad idea.
Maybe we can allow the vote value to be 0 somehow ?
Quoted 15 lines
I don't think it's possible at the moment, but actually Viresh mentioned that use case (scaling a power domain only for some of the OPPs) when we discussed where to enable the power domains listed in the OPP table [1].
Problem back then was that we didn't have a real example where this would be needed. It seems like such an example exists now, so I will discuss ways to implement that with Viresh.
I just wanted to be sure that adding the additional power domain to the CPU OPP table is the right approach.
Thanks again!
Stephan[1]: https://lore.kernel.org/linux-pm/20200828063511.y47ofywtu5qo57bq@vireshk-i7/
Signature · 1 line
viresh
Quoted 58 lines · Viresh Kumar
Quoted 57 lines · Stephan Gerhold
Quoted 41 lines · Stephen Boyd
Quoting Stephan Gerhold (2020-09-10 09:26:10)
Quoted 9 lines
Hi Stephen, Hi Rajendra,
while working on some MSM8916 things I've been staring at the downstream
clock-gcc-8916.c[1] driver a bit. One thing that confuses me are the voltage/performance state votes that are made for certain clocks within the driver. Specifically lines likeVDD_DIG_FMAX_MAP2(LOW, 32000000, NOMINAL, 64000000),Quoted 7 lines
on certain clocks like UART, I2C or SPI. There does not seem to be anything equivalent in the mainline clock driver at the moment.
As far as I understand from related discussions on mailing lists [2], these performance votes are not supposed to be added to the clock driver(s), but rather as required-opps within OPP tables of all the consumers. Is that correct?
Yes.
Quoted 21 lines
As a second question, I'm wondering about one particular case:
I've been trying to get CPR / all the CPU frequencies working on MSM8916. For that, I already added performance state votes for VDDMX and CPR as required-opps to the CPU OPP table.After a recent discussion [3] with Viresh about where to enable power domains managed by the OPP core, I've been looking at all the performance state votes made in the downstream kernel again.
Actually, the A53 PLL used for the higher CPU frequencies also has such voltage/performance state votes. The downstream driver declares the clock like [4]:
.vdd_class = &vdd_sr2_pll, .fmax = (unsigned long [VDD_SR2_PLL_NUM]) { [VDD_SR2_PLL_SVS] = 1000000000, [VDD_SR2_PLL_NOM] = 1900000000, }, .num_fmax = VDD_SR2_PLL_NUM,Quoted 7 lines
Quoted 4 lines
which ends up as votes for the VDDCX power domain.
Now I'm wondering: Where should I make these votes on mainline? Should I add it as yet another required-opps to the CPU OPP table?
Sounds like the right approach.
Thanks for the quick reply!
Quoted 5 lines
Quoted 4 lines
It would be a bit of a special case because these votes are only done for the A53 PLL (which is only used for the higher CPU frequencies, not the lower ones)...
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Can that be put into the OPP table somehow for only the high frequencies? The OPP tables for CPUs sometimes cover the CPU PLL voltage requirements too so it doesn't seem like a totally bad idea.
Maybe we can allow the vote value to be 0 somehow ?
Yep, I tried using an extra opp-level = <0> OPP for that in [1]. It does not work at the moment but maybe we can (somehow) add support for that. I have some more thoughts about where/how to enable the power domains from the OPP core in this case. The discussion probably fits better to the other mail thread so I will reply there later.
Thanks!
Stephan