Re: [RFC PATCH 00/16] Centralize the ownership of power rail votes for Qualcomm clocks

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 10/14/26 4:37 PM, Dmitry Baryshkov wrote:
> On Wed, Oct 14, 2026 at 02:19:00PM +0200, Konrad Dybcio wrote:
>> On Qualcomm SoCs, both the PLLs and other types of clocks alike require
>> specific voltage rail levels to operate in-spec, but through the
>> separation and decentralization of clock and voltage votes, these
>> requirements are often not met and are never explicitly stated.
>> This results in clocks being undervolted, or at the very least a state
>> we're unable to reason about.

[...]

>> To illustrate the nuances involved, here's a couple examples of hidden
>> dependencies, as found on SM8650:
>> * video_cc_mvs0_clk requires LOW_SVS to run at 900 MHz, but the
>>   PLL it sources the signal from requires SVS (higher corner) to
>>   produce that frequency. Similarly, to produce 1140 MHz, the clock
>>   requires SVS, but the PLL then requires SVS_L1.
>>   Analogous story for mvs1_clk.
> 
> Is PLL a part of the the same domain or a different domain?

The same.

>> * gpu_cc_gmu_clk requires MX to be at SVS to output 625 MHz, the PLL
>>   requires SVS_L1 to produce that frequency.
>> * A number of camera clocks follow the same pattern.
>> * Additionally, some clocks like cam_cc_cci_{0,1,2}_clk require
>>   MMCX to run at LOW_SVS, but the PLL has the additional requirement
>>   of keeping MXC (a wholly different rail) at SVS_L1.
>>
>> There's a broad agreement that a consumer should only have to care
>> about the clock it consumes, and that configuring its parents is the
>> responsibility of the CCF. Requiring the consumers to care about the
>> above cases (as is the expectation today) is a clear violation of
>> that contract.
> 
> I think that the conclusion was that the consumers should care about the
> full set of supplies they need, which includes all transient and leaf
> supplies.

Right, and my argument is that it's impractical and effectively
impossible to enforce and audit.

>> A clock controller may have on the order of hundreds of clocks, each
>> one with its own frequency-power map, and potentially all of them
>> could be similarly affected.
> 
> You have described several cases, where PLLs stand out, while the rest
> of the clocks are typically more or less in line. Would it make sense
> instead to solve the same issue in a slightly different way:

[...]

> The clock controller driver would then create a per-PLL device, bind it
> with the OPP table, etc. The consumers would still vote on the resources
> necessary to run the leaf clock, while the clock controller would care
> about the PLLs.
> 
> You can guess, I don't like the idea of having voting tables in the
> clock driver...

I don't think the PLLs are any special here. It just happened that
the special cases involved them. If we had OPP tables for PLLs, we'd
probably desire to keep *all* of the data in the DT (so also for all
the other clocks), and i don't want 50% of the DT to become clock data.

As you can guess, I don't like boardfiles in general too, but I think
containing that in the driver is the right choice here.

>> Recently, some drivers (Iris, CAMSS) have attempted to circumvent these
>> problems by holding a direct reference to the power rails and managing
>> the votes manually, through placing direct votes on each rail via an
>> OPP table. That however has a number of drawbacks:
>> * Because more than just one clock needs to be scaled at runtime, the
>>   OPP table either has to contain NUM_CLOCKS * NUM_RATES to handle
>>   that (which blows up pretty quickly), assume the worst case (and
>>   waste power), or ignore that problem (and undervolt the clock
>>   controller), instead of just being able to call set_rate() on each
>>   clock as needed
> 
> If the clocks are independent, it's a different question. I don't think
> we have a way to describe that we need OPP table a for clock CLK_A and
> table b for CLK_B.

Yes, they are! The AHB/AXI clocks for example (at the very least for
camera).

> If we need to scale several clocks in a lock, the problem is manageable.
> We need 2x entries (at least for the typical cases), because we need to
> have one entry for each MMCX level and for each MX level.

That's already solved today, and as you said that's just a different
requirement.

>> * The data ends up being dispersed across various consumers, even
>>   though it logically belongs to the clock controller, making it
>>   absolutely unauditable
> 
> Yes... See the proposal above.
> 
>> * It's horrendously error-prone because of the number of clocks
>>   involved, the depth of the clock tree and the fact that at the end
>>   of the day, one still has to consult the clock controller
>>   documentation
> 
> One needs to consult it anyway, more or less.

Yes, but provided you have the data, with this patchset, you can look
at the known single source and relay its contents to the known single
place (the clock driver), letting the computer do the path traversal
and max() along the way.

[...]

>>  +------------------------------------------------------------------+
>>  |                                                                    |
>>  |    GPLL9 ------> SDCC2 RCG -------> SDCC2 apps clock               |
>>  |      |               |                                             |
>>  |      |               |                         Other GCC clocks    |
>>  |      v               v                              |              |
>>  |  +------------+  +------------+                     v              |
>>  |  | PLL's own  |  | RCG's own  |          Per-clock requirements    |
>>  |  | rate table |  | rate table |                         |          |
>>  |  +-----+------+  +-----+------+                         |          |
>>  |        |               |                                |          |
>>  |        | CX            | CX + MX                        |          |
> 
> RCG doesn't (typically) care about MX.

That's unfortunately not a universal truth.

> It is not split between multiple
> domains, is it?

I think the most common case is "RCG needs CX and its direct child
branch needs MX at the same level" and the generator ends up treating
that as "RCG needs CX and MX" (which is totally fair). But again, RCGs
sourced from MX is also a valid config.

> 
>>  |        v               v                                v          |
>>  |    +--------------------------------------------------------+      |
>>  |    |                  Clock-owned DVFS                        |    |
>>  |    |                                                          |    |
>>  |    | Prepared votes + temporary transition votes              |    |
> 
> This can quickly become a lock dependency (and lockdep) hell. Consider
> two consumers setting the same clock concurrently (heh, nice design, I
> know). For each of the consumers we must make sure that there is no
> AB-BA voting update, which means that the whole sequence of
> pre-set-rate vote, clk_set_rate() and post-set-rate vote should happen
> under a single lock. However under the lock we vote for genpds, which
> might trigger other transitions or, in the worst case, wake up the
> domain.

Lockdep didn't complain on SM8650 and Glymur. I'm assuming the only case
where it actually matters is that if the power-domain provider is
suspended and needs a clock to turn on before accessing it. But that'
not a problem on Qualcomm platforms and that case would have bigger
issues anyway..

[...]

>> Straightening out all of the above lets us tackle another problem
>> with the upstream description of the power grid - the dependency
>> between various rails. Most notably:
>> * CX does NOT need to follow the performance state of MMCX
> 
> I think this depends on the RPMh translating votes or not. Is it really
> fine to run MMCX at turbo if CX is at low_svs?

Yes, that's actually the reason MMCX exists - so that the multimedia
power is wholly separate from the main rail.

Konrad