Laurent Pinchart [off-list ref] writes:
Hi Grygorii and Grant,
On Monday 28 July 2014 23:52:34 Grant Likely wrote:
quoted
On Mon, Jul 28, 2014 at 11:47 AM, Grygorii Strashko wrote:
quoted
On 07/28/2014 05:05 PM, Grant Likely wrote:
quoted
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko wrote:
[...]
quoted
quoted
- Where and when to call of_clk_register_runtime_pm_clocks()?
Bus notifier/ platform core/ device drivers
I would say in device drivers.
I tend to agree with that.
It will help here to take a step back and remember what the problem we're
trying to solve is.
[jumping in late, after Grygorii ping'd me about looking at this]
Laurent, thanks for summarizing the problem so well. It helped me
catchup on the discussion.
At the root is clock management. Our system comprise many clocks, and they
need to be handled. The Common Clock Framework nicely models the clocks, and
offers an API for drivers to retrieve device clocks and control them. Drivers
can thus implement clock management manually without much pain.
A clock can be managed in roughly three different ways :
- it can be enabled at probe time and disabled at remove time ;
- it can be enabled right before the device leaves its idle state and disabled
when the device goes back to idle ; or
- it can be enabled and disabled in a more fine-grained, device-specific
manner.
The selected clock management granularity depends on constraints specific to
the device and on how aggressive power saving needs to be. Enabling the clocks
at probe time and disabling them at remove time is enough for most devices,
but leads to a high power consumption. For that reason the second clock
management scheme is often desired.
Managing clocks manually in the driver is a valid option. However, when adding
runtime PM to the equation, and realizing that the clocks need to be enabled
in the runtime PM resume handler and disabled in the suspend handler, the
clock management code starts looking very similar in most drivers. We're thus
tempted to factorize it away from the drivers into a shared location.
It's important to note at this point that the goal here is only to simplify
drivers. Moving clock management code out of the drivers doesn't (unless I'm
missing something) open the door to new possibilities, it just serves as a
simplification.
I disagree. Actually, it opens up the door to lots of new possibilities
that are crucial for fine-grained PM with QoS. It is not just
simplification. There are many good reasons that some SoCs have moved
all the management of PM-related clocks *out* of device drivers. More
on that below...
Now, as Grygorii mentioned, differences between how a given IP core is
integrated in various SoCs can make clock management SoC-dependent. In the
vast majority of cases (which is really what we need to target, given that our
target is simplifying drivers) SoC integration can be described as a list of
clocks that must be managed. That list can be common to all devices in a given
SoC, or can be device-dependent as well.
If we care about fine-grained PM, this is a way-too oversimplified
version of what SoC integragion means.
There are lots of pieces which fall under "SoC integration", for
example: clock domains, power domains, voltage domains, SoC-specific
wakeup capabilities, etc. etc. Then, for fun throw in QoS constraints,
and things get really exciting.
IOW, if you care about fine-grained PM and QoS, you simply can't reduce
SoC integration down to "a list of clocks to be managed."
QoS makes this interesting as well because a device driver's decision to
gate its own clocks may have serious repercussions on the wakeup latency
of *other* devices in the same power domain. For example, the clock
gating of the last active device in a powerdomain may cause the
enclosing power-domain to be power gated, having a major impact on the
wakup latency of *all* devices in that power domain.
So if we're going to manage the list of PM-related clocks in the device
driver, we'll also keep track of all the other devices in the same power
domain, whether or not they're active, whether or not they have QoS
constraints, etc. etc. Hopefully you can see that we're quickly way
outside the scope of the IP block that the device driver is intended to
manage.
All of this is "SoC integration" knowledge, and IMO doen't belong in the
device drivers. It belongs at the SoC integration level, and in todays
kernel frameworks that means pm_domain/genpd.
Few locations can be used to express a per-device list of per-SoC clocks. We
can have clocks lists in a per-SoC and per-device location, per-device clocks
lists in an SoC-specific location, or per-SoC clocks lists in a device-
specific location.
The first option would require listing clocks to be managed by runtime PM in
DT nodes, as proposed by this patch set. I don't think this is the best
option, as that information is a mix of hardware description and software
policy, with the hardware description part being already present in DT in the
clocks property.
I'm not seeing which part you think is software policy here? Which
clocks are driving which IP blocks is a hardware description.
Which clocks are actually gated, and if/when is software policy and
should be decided by the SoC-specific runtime PM and genpd
implementations, but describing which clocks are wired to which IP
blocks is certainly hardware description IMO.
The second option calls for storing the lists in SoC code under arch/. As
we're trying to minimize the amount of SoC code there (and even remove SoC
code completely when possible) I don't think that's a good option.
The third option would require storing the clocks lists in device drivers. I
believe this is our best option, as a trade-off between simplicity and
versatility. Drivers that use runtime PM already need to enable it explicitly
when probing devices. Passing a list of clock names to runtime PM at that
point wouldn't complicate drivers much. When the clocks list isn't SoC-
dependent it could be stored as static information. Otherwise it could be
derived from DT (or any other source of hardware description) using C code,
offering all the versatility we need.
As is probably clear from above, I don't like this approach at all.
The only drawback of this solution I can think of right now is that the
runtime PM core couldn't manage device clocks before probing the device.
Specifically device clocks couldn't be managed if no driver is loaded for that
device. I somehow recall that someone raised this as being a problem, but I
can't remember why.
The bigger drawback of this approach is that the device-drivers become a
repository for SoC integration details that IMO don't belong in a device
driver for a specific IP block.
Over the last few years, we've created abstractions for this kind of SoC
integration information (pm_domains) as well as frameworks for handling
much of the common parts (genpd) and in doing so, have been able to
remove PM-related clock management from device drivers altogether.
I think managing this stuff back in device drivers would be a major step
backwards.
Kevin
Hi Kevin,
On Monday 08 September 2014 13:13:25 Kevin Hilman wrote:
Laurent Pinchart [off-list ref] writes:
quoted
On Monday 28 July 2014 23:52:34 Grant Likely wrote:
quoted
On Mon, Jul 28, 2014 at 11:47 AM, Grygorii Strashko wrote:
quoted
On 07/28/2014 05:05 PM, Grant Likely wrote:
quoted
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko wrote:
[...]
quoted
quoted
quoted
- Where and when to call of_clk_register_runtime_pm_clocks()?
Bus notifier/ platform core/ device drivers
I would say in device drivers.
I tend to agree with that.
It will help here to take a step back and remember what the problem we're
trying to solve is.
[jumping in late, after Grygorii ping'd me about looking at this]
Laurent, thanks for summarizing the problem so well. It helped me
catchup on the discussion.
You're welcome. Sorry for the very late reply.
quoted
At the root is clock management. Our system comprise many clocks, and they
need to be handled. The Common Clock Framework nicely models the clocks,
and offers an API for drivers to retrieve device clocks and control them.
Drivers can thus implement clock management manually without much pain.
A clock can be managed in roughly three different ways :
- it can be enabled at probe time and disabled at remove time ;
- it can be enabled right before the device leaves its idle state and
disabled when the device goes back to idle ; or
- it can be enabled and disabled in a more fine-grained, device-specific
manner.
The selected clock management granularity depends on constraints specific
to the device and on how aggressive power saving needs to be. Enabling
the clocks at probe time and disabling them at remove time is enough for
most devices, but leads to a high power consumption. For that reason the
second clock management scheme is often desired.
Managing clocks manually in the driver is a valid option. However, when
adding runtime PM to the equation, and realizing that the clocks need to
be enabled in the runtime PM resume handler and disabled in the suspend
handler, the clock management code starts looking very similar in most
drivers. We're thus tempted to factorize it away from the drivers into a
shared location.
It's important to note at this point that the goal here is only to
simplify drivers. Moving clock management code out of the drivers doesn't
(unless I'm missing something) open the door to new possibilities, it just
serves as a simplification.
I disagree. Actually, it opens up the door to lots of new possibilities
that are crucial for fine-grained PM with QoS. It is not just
simplification. There are many good reasons that some SoCs have moved all
the management of PM-related clocks *out* of device drivers. More on that
below...
quoted
Now, as Grygorii mentioned, differences between how a given IP core is
integrated in various SoCs can make clock management SoC-dependent. In the
vast majority of cases (which is really what we need to target, given that
our target is simplifying drivers) SoC integration can be described as a
list of clocks that must be managed. That list can be common to all
devices in a given SoC, or can be device-dependent as well.
If we care about fine-grained PM, this is a way-too oversimplified
version of what SoC integragion means.
There are lots of pieces which fall under "SoC integration", for
example: clock domains, power domains, voltage domains, SoC-specific
wakeup capabilities, etc. etc. Then, for fun throw in QoS constraints,
and things get really exciting.
IOW, if you care about fine-grained PM and QoS, you simply can't reduce
SoC integration down to "a list of clocks to be managed."
Of course. I was talking about SoC integration for clocks, not about SoC
integration in general.
QoS makes this interesting as well because a device driver's decision to
gate its own clocks may have serious repercussions on the wakeup latency
of *other* devices in the same power domain. For example, the clock gating
of the last active device in a powerdomain may cause the enclosing power-
domain to be power gated, having a major impact on the wakup latency of
*all* devices in that power domain.
So if we're going to manage the list of PM-related clocks in the device
driver, we'll also keep track of all the other devices in the same power
domain, whether or not they're active, whether or not they have QoS
constraints, etc. etc. Hopefully you can see that we're quickly way outside
the scope of the IP block that the device driver is intended to manage.
All of this is "SoC integration" knowledge, and IMO doen't belong in the
device drivers. It belongs at the SoC integration level, and in todays
kernel frameworks that means pm_domain/genpd.
Ok, there's more to it than I initially thought. Let's see how we can make
this happen then :-)
quoted
Few locations can be used to express a per-device list of per-SoC clocks.
We can have clocks lists in a per-SoC and per-device location, per-device
clocks lists in an SoC-specific location, or per-SoC clocks lists in a
device- specific location.
The first option would require listing clocks to be managed by runtime PM
in DT nodes, as proposed by this patch set. I don't think this is the
best option, as that information is a mix of hardware description and
software policy, with the hardware description part being already present
in DT in the clocks property.
I'm not seeing which part you think is software policy here? Which clocks
are driving which IP blocks is a hardware description.
Which clocks are actually gated, and if/when is software policy and should
be decided by the SoC-specific runtime PM and genpd implementations, but
describing which clocks are wired to which IP blocks is certainly hardware
description IMO.
The clocks property is a hardware description, but a new property that would
hold a list of clocks to be managed by the runtime PM core is less so.
quoted
The second option calls for storing the lists in SoC code under arch/. As
we're trying to minimize the amount of SoC code there (and even remove SoC
code completely when possible) I don't think that's a good option.
The third option would require storing the clocks lists in device drivers.
I believe this is our best option, as a trade-off between simplicity and
versatility. Drivers that use runtime PM already need to enable it
explicitly when probing devices. Passing a list of clock names to runtime
PM at that point wouldn't complicate drivers much. When the clocks list
isn't SoC- dependent it could be stored as static information. Otherwise
it could be derived from DT (or any other source of hardware description)
using C code, offering all the versatility we need.
As is probably clear from above, I don't like this approach at all.
Do we at least agree that option 2 (storing the lists in arch/) is a bad idea
and should be ruled out ?
quoted
The only drawback of this solution I can think of right now is that the
runtime PM core couldn't manage device clocks before probing the device.
Specifically device clocks couldn't be managed if no driver is loaded for
that device. I somehow recall that someone raised this as being a
problem, but I can't remember why.
The bigger drawback of this approach is that the device-drivers become a
repository for SoC integration details that IMO don't belong in a device
driver for a specific IP block.
I agree that SoC integration details don't belong in device drivers, but I
don't see this as SoC integration details. The driver knows that the IP core
it manages has two functional clock inputs, and knows how those clock inputs
are named. The name is a property of the IP core, not of the SoC. Only how
(and whether) those clocks are connected in the SoC is integration
information.
Now, if the IP core itself can be synthesized with different clock option,
those synthesis options belong to DT, either explicitly in the clock-names
property or implicitly in the compatible property.
Over the last few years, we've created abstractions for this kind of SoC
integration information (pm_domains) as well as frameworks for handling
much of the common parts (genpd) and in doing so, have been able to
remove PM-related clock management from device drivers altogether.
I think managing this stuff back in device drivers would be a major step
backwards.
--
Regards,
Laurent Pinchart