Re: [PATCH 04/15] clk: qcom: gcc-sm6375: Add runtime PM

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 18.07.2023 14:07, Stephan Gerhold wrote:
> On Mon, Jul 17, 2023 at 09:02:29PM -0700, Bjorn Andersson wrote:
>> On Mon, Jul 17, 2023 at 06:26:35PM +0200, Stephan Gerhold wrote:
>>> On Mon, Jul 17, 2023 at 05:19:11PM +0200, Konrad Dybcio wrote:
>>>> The GCC block on SM6375 is powered by the VDD_CX rail. We need to ensure
>>>> that it's enabled to prevent unwanted power collapse.
>>>>
>>>> Enable runtime PM to keep the power flowing only when necessary.
>>>>
>>>
>>> Are you sure this is necessary? If VDD_CX was really possible to fully
>>> "power collapse" then I would expect that you lose all register
>>> settings. This is not something we want or can even handle for GCC.
>>> You would need to restore all frequency settings, branch bits etc etc.
>>>
>>
>> This differ between platforms, some allow us to completely power down CX
>> while keeping registers state using MX, others require that CX stays in
>> retention at least.
>>
>> So, CX isn't the only rail powering GCC. For the most part though, we
>> have a relationship between frequencies votes for by clients and the
>> corner of CX, and hence I think the current description is ok...
>>
> 
> This patch is just about sending enable/disable votes for the power
> domains though, based on runtime PM which triggers when all the clocks
> are disabled.
> 
> It's unrelated to voting for CX corners required by certain clock
> frequencies (we handle those in the OPP tables of the consumers).
> And it's also unrelated to ensuring rentention of register contents
> since we actually release all votes when the clocks are idle.
> 
> So while adding runtime PM to all the clock drivers sounds nice, I'm
> a bit confused what problem we're actually solving with this patch. :)
In a very specific and unfortunate situation, there could be no other
CX votes, and trying to access (perhaps at least parts of) GCC would
result in a failure.

Konrad