Re: [PATCH RFC 07/16] clk: qcom: Add clock-owned GenPD DVFS aggregation
From: Konrad Dybcio
Date: Wed Oct 14 2026 - 11:27:48 EST
On 10/14/26 4:36 PM, Dmitry Baryshkov wrote:
> On Wed, Oct 14, 2026 at 02:19:07PM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
>>
>> Clocks on Qualcomm SoCs need a minimum power rail performance state to
>> operate at a given frequency. We currently leave this to clock
>> consumers, so a driver has to know which power domains supply both the
>> clock and its parents. This ties knowledge of the clock tree to
>> individual consumers and doesn't protect other users that prepare or
>> change the same clock without making those power requests.
>
> Why would they prepare or change the same clock without making power
> requests? If it's a legitimate user of the clock, it should have an OPP
> table. If it's not, it should have a separate API to set the clock via a
> legitimate user / aggregator.
That's the entire problem that motivated this series. The consumer
doesn't know what it needs on Qualcomm platforms. The power
requirements are that of the clock controller's, not of that device's.
> I see one usecase where this would help though. If the clocks use
> different power domains and the corners on one of the domains don't
> match voltage on the other domain. E.g. it's not infrequent that the PLL
> (powered by MX) requires performance vote change at a different
> frequency than the rest of the clock tree.
Correct
[...]
>> Move these requirements into the clock provider and aggregate the votes
>> per power domain. Keep the highest level needed by prepared clocks and
>> in-progress rate or parent changes so that one clock can't withdraw
>> another clock's requirement. Use separate virtual GenPD devices for
>> these votes and controller register access so their power requirements
>> can be managed independently.
>
> The biggest question at this point would be the locking and lock
> nesting. What is the expected order of operations? Currently it's
> limited to pmdomains, but for some of the (older) platforms clock
> supplies are expressd as the normal regulators rather than pmdomains.
The Qualcomm case is easy - RPMH is *always* reachable, so the
pmdomain provider needs no clocks itself. For the other cases,
"patches welcome".
[...]
>> +static unsigned int
>> +qcom_clk_dvfs_aggregate(struct qcom_clk_dvfs_rail_voter *rail_voter)
>> +{
>> + struct qcom_clk_dvfs_vote *vote;
>> + unsigned int level = 0;
>> +
>> + list_for_each_entry(vote, &rail_voter->votes, node)
>> + level = max(level, vote->level);
>> +
>> + return max(level, rail_voter->safety_floor_level);
>
> I guess you want to avoid having too many voters, but would it be
> possible to have one voter per 'signficant' clock and let GenPD itself
> aggregate all the votes?
What does "significant" mean here?
Konrad