RE: RFC: hwmod, iclks, auto-idle and autodeps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Paul Walmsley [mailto:paul@xxxxxxxxx]
> Sent: Tuesday, November 23, 2010 9:31 AM
> To: Kevin Hilman
> Cc: Cousson, Benoit; Basak, Partha; Varadarajan, Charulatha; linux-
> omap@xxxxxxxxxxxxxxx
> Subject: Re: RFC: hwmod, iclks, auto-idle and autodeps
> 
> Hi
> 
> Kevin and I discussed this privately, this is just to summarize for
> everyone else.
> 
> On Wed, 17 Nov 2010, Kevin Hilman wrote:
> 
> > There have been a few discussions over the few months about how to
> > properly use omap_hwmod to manage certain IPs that have the interface
> > clock as the functional clock (e.g. OMAP3 GPIOs.)  The goal of this
> RFC
> > is to come to a conclusion about what should be done, and how to go
> > about implementing it.
> >
> > In the initial conversion of the GPIO core to omap_hwmod, main_clk
> was
> > left NULL so that hwmod was not managing the clock on every hwmod
> > enable/disable.
> 
> Since GPIO has a register target, main_clk cannot be null.  There's an
> erroneous comment in plat/omap_hwmod.h about this; I'll add it to my
> list
> of things to fix.
> 
> > This behavior matched current mainline GPIO code, which does not
> > dynamically disable/enable GPIO iclks after init time. Instead, it
> > relies on the module-level auto idle feature in HW.
> >
> > However, without dynamically managing the clock in hwmod, this meant
> > that there were no autodeps tracked for GPIO and thus the PER
> > powerdomain could transition independently of MPU/CORE.
> 
> The current GPIO driver works only because it exploits some incidental
> properties of the OMAP core code.  It implicitly relies on CM_AUTOIDLE
> = 1
> on its iclk for power management to work, since the driver never
> disables
> its iclk.  The driver also relies on the addition of MPU/IVA2 autodeps
> to
> avoid losing logic context once all devices in PER go idle.  And by
> never
> explicitly disabling its iclk, the driver avoids dealing with the
> various
> GPIO wakeup/interrupt modes, causing its context save/restore to be
> implemented as a weird hack in pm34xx.c.
> 
> That said, there definitely is one real limitation/bug in the OMAP PM
> core
> code that the GPIO driver has to work around.  The current core code
> doesn't try to keep a powerdomain powered when an IP block inside the
> powerdomain is still considered to be in use but all its system-sourced
> clocks are cut.  This can result in unexpected context loss and
> malfunction in some GPIO and McBSP cases, possibly some other modules
> also
> that can be externally clocked and contain FIFOs/buffers, or that can
> generate asynchronous wakeups.  There's a patch in the works here that
> will require a powerdomain to stay on as long as a hwmod in that
> powerdomain is enabled.  Once omap_hwmod_idle() is called for that
> hwmod,
> the lower-bound on the power state of the powerdomain is removed.
> 
> So in the context of the PM runtime conversion of the GPIO driver, the
> thing to do is to only do a pm_runtime_put*() once the GPIO module is
> really ready to be powered down.  After that call, the GPIO block may
> lose
> its logic context, and it may not be able to generate interrupts or
> wakeups.  We may enforce the latter in the hwmod code for debugging
> purposes by forcing the module into idle and instructing the PRCM to
> ignore wakeups from the module in omap_hwmod_idle().  IO ring/chain
> wakeups may still occur, but these are independent of the GPIO block.
> 
> The rest of the time, if the GPIO driver wants to use idle-mode wakeups
> (presumably higher wakeup latency, but lower power consumption), it
> should
> just clk_disable() its clocks, but not call pm_runtime_put*().  After

This would require that clk_disable() be called directly by the driver
outside of pm_runtime_putsync(). 
Isn't this not in line with runtime PM paradigm?

> the
> previously-mentioned improvement to the powerdomain/hwmod code is
> completed and applied, that should result in the powerdomain staying
> powered, but all clocks being disabled.  Otherwise, if the GPIO driver
> wants to use active-mode interrupts (presumably lower wakeup latency,
> but
> higher power consumption), then the driver should just leave its clocks
> enabled and never call pm_runtime_put*().

So, if I understood correctly, it is up to the driver to take the call.
It is perfectly OK if the driver decides to do a pm_runtime_get_sync()
when a GPIO module is requested and not call pm_runtime_put_sync()
until it is released. Now, in OMAP, because of the incidental availability
of AutoIdle feature, this will not prevent the clocks getting auto-gated.
> 
> Both of these latter modes will block some low-power states.  At some
> point, the selection of the interrupt/wakeup mode -- GPIO active, GPIO
> idle, IO ring/chain -- should be made based on the required wakeup
> latency
> of the GPIO user.  Multiple modes may need to used simultaneously,
> since
> individual modes are restricted to certain power states (e.g. IO ring
> is
> only available in CORE off, IO chain is only available in CORE/PER
> retention)
> 
> > The initial solution to this was to set the iclk to be the main_clk
> of
> > the hwmod, such that autodeps were managed dynamically as well.  This
> > led to a change in functionality in current code, since the iclk was
> now
> > being manually enabled/disabled at runtime instead of being left for
> HW
> > to manage by module autoidle.  It also led to problems in correctly
> > handling asynchronous GPIO wakeups.
> 
> If idle-mode wakeup is used, then the PRCM ISR code that notes the GPIO
> wakeup event either needs to enable the GPIO main clock before calling
> its
> ISR, or the GPIO ISR needs to enable its main clock first thing.  If
> active-mode interrupts are used, then the GPIO ISR doesn't need to
> enable
> its main clock since it is already running at that point.

Enabling main clock via pm_runtime_getsync() in interrupt context will
be a problem. Can we use pm_runtime_get() instead? This will lead to some
delay in the interrupts getting processed.
> 
> > Alternatively, would it make sense to do something different with
> > autodeps, such that modules like this that don't have a separate
> > functional clock still have autodeps handled, possibly by using an
> > optional clock?
> >
> > Does extending autodeps make sense since, IIUC, we won't really need
> > them after all devices are using hwmod?
> 
> The goal is definitely to get rid of the autodeps.  They are an old CDP
> artifact that shouldn't be necessary once the device power control is
> cleaned up.  The autodeps are almost certainly wasting power - they are
> added for every clockdomain for both the MPU and IVA2 processor
> modules.
> Hopefully they should be removable after the hwmod conversion is
> complete
> and targeted dependencies have been added for chip limitations/bugs
> (e.g.
> the PER/CORE dependency issues)
> 
> 
> - Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux