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@3d90000 {
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
