Thread (15 messages) 15 messages, 3 authors, 2021-01-19

Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback optional

From: Ulf Hansson <hidden>
Date: 2021-01-19 10:26:05
Also in: linux-tegra, lkml

On Tue, 19 Jan 2021 at 04:44, Viresh Kumar [off-list ref] wrote:
On 18-01-21, 13:46, Ulf Hansson wrote:
quoted
You seem to be worried about latency/overhead while doing the
propagation upwards in the hierarchy. That seems like a reasonable
concern to me, especially as the genpd lock is taken at each level.
I am not sure how many levels of domains we have normally, but unless the number
is big it won't be a very big problem.
It depends on the SoC topology, so obviously it differs. But more than
a couple is unusual, I would say.

However, I think it may also depend on how many devices that are
hooked up to the domains and how actively these are being used.
quoted
However, to mitigate this can be rather messy. In principle, we would
need to walk the hierarchy upwards, each time a new subdomain is added
in genpd_add_subdomain(). While doing this, we would also need to keep
track on what level we set to continue the propagation of the
performance states for. Even if this can be done in non-latency
sensitive paths, I don't think it's worth it because of complexity (I
haven't even thought of what happens when removing a subdomain).
What about a new field in the domain structure like 'can-handle-pstates', and
then whenever sub-domain gets added it just needs to check its parent's field
and set his own.
That would work if the topology is built from top to bottom, but I
don't think we can rely on that.

For example, when a domain A is added as a child to domain B, domain B
doesn't have a parent yet (and the "can-handle-pstates" don't get set
for neither domain A or domain B). Next, domain B is added as child
domain to domain C. Domain C has the "can-handle-pstates" set, which
means domain B gets the "can-handle-pstates" set as well. This means
domain A, will not have "can-handle-pstates" set, while it probably
should have.
quoted
So, maybe we should simply just stick to the existing code, forcing
the parent to have a ->set_performance() callback assigned if
propagation should continue?
I think it would be better to fix the issue even if we aren't fully optimized
and making the change to make sure we keep propagating is rather important.
Alright, let's continue with Dmitry's patches and discuss this further
when v4 is out, as he seems to have it almost ready.

Kind regards
Uffe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help