Re: [PATCH 1/11] ARM: tegra: add function to control the GPU rail clamp

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2942 bytes --]

On Thu, Jan 08, 2015 at 11:32:06AM +0200, Peter De Schrijver wrote:
> > > And specify the dependencies between domains in DT?
> > 
> > I think the dependencies could be in the driver. Of course the power
> > domains are per-SoC data, so really shouldn't be in the DTS either (the
> > data is all implied by the compatible value) but there's no good way to
> > get at the clocks and resets without DT, so I think that's a reasonable
> > trade-off.
> > 
> > It seems to me like there are only two dependencies:
> > 
> > 	DIS and DISB depend on SOR
> > 	VE depends on DIS
> > 
> > That's according to 5.6.6 "Programming Guide for Power Gating and
> > Ungating" of the Tegra K1 TRM. It also seems like a bunch of modules are
> > part of seemingly unrelated domains. Especially SOR seems to cover a
> > large range of modules (MIPI-CAL, DPAUX, SOR, HDMI, DSI, DSIB and
> > HDA2HDMI).
> > 
> > Given that we may want to more fine-grainedly control clocks to save
> > power, don't we need to control clocks and resets within the drivers? I
> > think the runtime PM framework makes sure to call this in the right
> > order, so for suspend, the sequence would be:
> > 
> > 	1) device->suspend
> > 	2) domain->suspend
> > 
> > and for resume:
> > 
> > 	1) domain->resume
> > 	2) device->resume
> > 
> > But then we're back to square one, namely that the MC flush doesn't work
> > properly, since it needs to be implemented in domain->suspend. Does that
> > mean we can't clock-gate modules? In order to ensure a proper powergate
> > sequence, the domain code would need to clk_enable() the module clock to
> > make sure it stays on during the reset sequence. But if the domain code
> > has a reference to the clock, then the driver can't clock-gate the
> > module anymore by calling clk_disable().
> > 
> > Am I missing something?
> > 
> 
> There's a difference between having a reference and actually enabling the
> clock.

My point was that as long as anyone was keeping a reference the clock
couldn't in fact be turned off.

> the domain powergate method will only be called when the clocks of
> all modules in the domain are off.

No, the power domain will be disabled when all devices in the domain are
idle.

> So the powergate method can then turn them on again, do the module
> resets and client flushes and then disable them again. Same for
> ungate. So I don't see a problem here?

I think that could work, but we'd need to make sure that drivers that
use runtime PM and are connected to a power domain enable clocks only
after taking a runtime PM reference and disable the clocks before they
release that reference.

So to simplify things, maybe all clock handling for drivers should be
moved into runtime PM operations, and whenever the driver needs the
clock enabled it takes a runtime PM reference. That would ensure that
clocks aren't accidentally left on.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]