Re: Qcom clock performance votes on mainline

[Stephan Gerhold <stephan@gerhold.net>]

On Thu, Sep 10, 2020 at 02:28:01PM -0700, Stephen Boyd wrote:
> Quoting Stephan Gerhold (2020-09-10 09:26:10)
> > 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?
> 
> Yes.
> 
> > 
> > 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?
> 
> Sounds like the right approach.
> 

Thanks for the quick reply!

> > 
> > 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/