Messages in this thread | | | Date | Tue, 15 Mar 2022 11:29:19 +0800 | From | Leo Yan <> | Subject | Re: [PATCH v1 0/3] arch_topology: Correct CPU capacity scaling |
| |
Hi Ionela,
On Mon, Mar 14, 2022 at 06:10:58PM +0000, Ionela Voinescu wrote:
[...]
> > Patch 03 is to handle the case for absenting "capacity-dmips-mhz" > > property in CPU nodes, the patch proceeds to do CPU capacity scaling based > > on CPU maximum capacity. Thus it can reflect the correct CPU capacity for > > Arm platforms with "fast" and "slow" clusters (CPUs in two clusters have > > the same raw capacity but with different maximum frequencies). > > > > In my opinion it's difficult to handle absent "capacity-dmips-mhz" > properties, as they can be a result of 3 scenarios: potential.. > 1. bug in DT > 2. unwillingness to fill this information in DT > 3. suggestion that we're dealing with CPUs with same u-arch > (same capacity-dmips-mhz)
For absent "capacity-dmips-mhz" properties, I think we could divide into two sub classes:
For all CPU nodes are absent "capacity-dmips-mhz" properties, it's likely all CPUs have the same micro architecture, thus developers are not necessarily to explictly set the property.
For partial CPUs absent "capacity-dmips-mhz" properties, this is an usage issue in DT and kernel should handle this as an error and report it.
> I'm not sure it's up to us to interpret suggestions in the code so I > believe treating missing information as error is the right choice, which > is how we're handling this now.
Yes, current kernel means to treat missing info as error, whatever if all CPUs or partial CPUs are absent "capacity-dmips-mhz" properties.
> For 3. (and patch 03), isn't it easier to populate capacity-dmips-mhz to > the same value (say 1024) in DT? That is a clear message that we're > dealing with CPUs with the same u-arch.
"capacity-dmips-mhz" is defined as a _optional_ property in the DT document (see devicetree/bindings/arm/cpu-capacity.txt).
Current kernel rolls back every CPU raw capacity to 1024 if DT doesn't bind "capacity-dmips-mhz" properties, given many SoCs with same CPU u-arch this is right thing to do; here I think kernel should proceed to scale CPU capacity with its maximum frequency.
When I worked on a platform with a fast and a slow clusters (two clusters have different max frequencies and with the same CPU u-arch), it's a bit puzzle when I saw all CPU's capacities are always 1024. In this case, since a platform have no CPU capacity modeling, and "capacity-dmips-mhz" property is not needed to populate in DT, but at the end the kernel should can reflect the scaled CPU capacity correctly.
Thanks a lot for review,
Leo
| |