Re: [PATCH v11 1/6] soc: qcom: ice: Add OPP-based clock scaling support for ICE
From: Konrad Dybcio
Date: Mon Jun 29 2026 - 07:13:05 EST
On 6/22/26 9:34 AM, Abhinaba Rakshit wrote:
> On Thu, Jun 18, 2026 at 03:01:54PM +0200, Konrad Dybcio wrote:
>> On 6/8/26 11:47 PM, Abhinaba Rakshit wrote:
>>> Register optional operation-points-v2 table for ICE device
>>> during device probe. Attach the OPP-table with only the ICE
>>> core clock. Since, dtbinding is on a transition phase to include
>>> iface clock and clock-names, attaching the opp-table to core clock
>>> remains optional such that it does not cause probe failures.
>>>
>>> Introduce clock scaling API qcom_ice_scale_clk which scale ICE
>>> core clock based on the target frequency provided and if a valid
>>> OPP-table is registered. Use round_ceil passed to decide on the
>>> rounding of the clock freq against OPP-table. Clock scaling is
>>> disabled when a valid OPP-table is not registered.
>>>
>>> This ensures when an ICE-device specific OPP table is available,
>>> use the PM OPP framework to manage frequency scaling and maintain
>>> proper power-domain constraints.
>>>
>>> Also, ensure to drop the votes in suspend to prevent power/thermal
>>> retention. Subsequently restore the frequency in resume from
>>> core_clk_freq which stores the last ICE core clock operating frequency.
>>>
>>> Reviewed-by: Harshal Dev <harshal.dev@xxxxxxxxxxxxxxxx>
>>> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@xxxxxxxxxxxxxxxx>
>>> ---
>>
>> [...]
>>
>>> @@ -335,6 +342,11 @@ int qcom_ice_suspend(struct qcom_ice *ice)
>>> {
>>> clk_disable_unprepare(ice->iface_clk);
>>> clk_disable_unprepare(ice->core_clk);
>>> +
>>> + /* Drop the clock votes while suspend */
>>> + if (ice->has_opp)
>>> + dev_pm_opp_set_rate(ice->dev, 0);
>>
>> The PM core will quiesce the vote as the device suspends, this is
>> unnecessary. Similarly, the rate restore logic will become unnecessary.
>> Especially since dev_pm_opp_set_rate(0) does not actually do any rate
>> setting.
>
> This section was earlier discussed in the patchset v4:
> https://lore.kernel.org/all/7b219a50-6971-4a0c-a465-418f8abd5556@xxxxxxxxxxxxxxxx/
> The intention here was to drop the RPMh votes once the device goes to suspend same
> as the storage drivers such as mmc drivers does:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/mmc/host/sdhci-msm.c#n2946
> This was done to leave the hanging votes *on* for unused clocks.
>
> However, I get your point, due to mean to say that once device goes to suspend
> and GDSC power-domain will be turned OFF, it will automatically quiesce the
> performance votes?
When the device's runtime state goes to 'suspended', all votes are
dropped (which then propagates up the power domain tree, effectively
lowering the vote which passes through the GDSC to the parent CX domain)
i.e. "yes"
Konrad