Message-ID: <84744442-73e4-49c0-a54f-1530093fb27b@linaro.org>
Date: Mon, 2 Dec 2024 14:29:00 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Bjorn Andersson <andersson@...nel.org>
Cc: Michael Turquette <mturquette@...libre.com>,
 Stephen Boyd <sboyd@...nel.org>,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
 linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 3/3] clk: qcom: Support attaching GDSCs to multiple
 parents

On 02/12/2024 03:58, Bjorn Andersson wrote:
> On Fri, Nov 29, 2024 at 01:06:49PM +0000, Bryan O'Donoghue wrote:
>> When a clock-controller has multiple power-domains we need to attach
>> parent GDSCs in that clock-controller as subdomains of each of the
>> power-domains.
> 
> I envision a fair number of current and future readers will wonder why
> this is... Why do we _need_ to perform this attachment?
> 
>>
>> Testing on the x1e80100 reference shows that both power-domains need to be
>> switched on for the GDSCs in the clock-controller to work.
> 
> You're making a completely generic change, but are referring here to
> some specific (probably camera) case.
> 
>> Some open
>> questions remain.
>>
>> 1. Should there be a hirearchy of power-domains in the clock-controller.
> 
> Your TITAN_TOP patches is already an example of such hierarchy, so I
> don't think that's an open question.
> 
>> 2. If there should be no hirearchy should the parent GDSC inside the
>>     clock-controller attach to each power-domain in the clock-controller.
> 
> I suspect that the TITAN_TOP gives you this impression that there is a
> "parent GDSC"; that's generally not the case - but the mechanism
> introduced by the patch is still needed.
> 
>> 3. If there are multiple parent GDSCs in a clock-controller do we attach
>>     those top-level GDSCs to each controller power-domain.
>> 4. Finally should performance-states be applied equally across those
>>     power-domains.
>>
>> It may be if we see more clock-controllers with multiple power-domains that
>> some mixture of these questions will need to be implemented for specific
>> hardware.
> 
> GPUCC has always been an example of this and there are several other
> examples in multimedia, which we've just ignored. And even for GCC you
> have a mix of voltage requirements cast across CX and MX.
> 
>> Right now the approach taken here is to attach the
>> clock-controller GDSC parent to each clock-controller power-domain.
>>
> 
> What is "the clock-controller GDSC parent"? Perhaps I'm just parsing
> this incorrectly?

> Perhaps we can use the naming from the API and say "each GDSC is put in
> the subdomain of all power-domains of the clock-controller"?

OK

> 
> I'm not convinced that the propagation of set_performance_state has been
> adequately been understood.
> 
> But, in general, I'm not against the idea of starting off by voting on
> all rails, mentioning that it's likely that in some cases more effective
> propagation of votes can be made and then leave this as a future
> exercise.
> 
> I would like to see 1-2 use cases beyond camcc being exposed to this
> though.

I'm not sure we have one on x1e - GPUCC maybe, I think you mentioned that.

                 gpucc: clock-controller@...0000 {
                         compatible = "qcom,x1e80100-gpucc";
                         reg = <0 0x03d90000 0 0xa000>;
                         clocks = <&bi_tcxo_div2>,
                                  <&gcc GCC_GPU_GPLL0_CPH_CLK_SRC>,
                                  <&gcc GCC_GPU_GPLL0_DIV_CPH_CLK_SRC>;
                         #clock-cells = <1>;
                         #reset-cells = <1>;
                         #power-domain-cells = <1>;
                 };

I can look around for a list of power-domains = <>; for this block.

I suspect though they are already on as they aren't list.

Do you want examples in the commit log or dtsi changes to accompany ?

---
bod

Powered by blists - more mailing lists