Re: [PATCH] video: omap2: dss: RET on idle, enable/disable dss clocks only when needed.

[Kevin Hilman <khilman@deeprootsystems.com>]

Tomi Valkeinen <tomi.valkeinen@nokia.com> writes:

> On Wed, 2009-09-30 at 20:31 +0200, ext Kevin Hilman wrote:
>> On Fri, Sep 25, 2009 at 2:19 AM, Tomi Valkeinen
>> <tomi.valkeinen@nokia.com> wrote:
>> > On Thu, 2009-09-24 at 17:52 +0200, ext Kevin Hilman wrote:
>> >> Tomi Valkeinen wrote:
>> >
>> >> > If it is not like that, and the driver initialization is included, how
>> >> > does the PM layer know how long it takes for the DSS driver to
>> >> > reconfigure the DSS hardware from OFF mode?
>> >>
>> >> Currently it doesn't, but if you were measure it, we can use those
>> >> numbers in the decision making process.
>> >
>> > Ok, now I see. However, I'm not sure if that will work. The problem is
>> > that the wakeup latency depends on many things. When using DPI/RFBI the
>> > wakeup is very fast. With SDI it's probably a bit slower and with DSI
>> > even slower.
>> 
>> The varying latencies are not an issues.  When in the different modes,
>> just register a different latency requirement.
>
> How does that help? The problem is not that there are different max
> latency requirements in different modes, but that the actual wake-up
> latency varies.
>
> I found some latency values in resource34xx.h. If they are what I
> presume they are, they define that waking DSS from OFF takes 70us. Let's
> presume it's correct for DPI, so reconfiguring DSS for DPI use takes
> ~70us.
>
> But reconfiguring DSS for DSI use may take, say, 5000us. So if I set max
> latency req to 100us, OFF mode will be "enabled", and it will work fine
> for DPI. But for DSI we will get lantencies around 5000us.
>
> So is there an API to change that value in resource34xx.h dynamically,
> depending on what DSS block is in use? Or am I still missing something
> here? =)

You're right, we currently do not have a way to dynamically update
this table and we should for completeness.

>> 
>> > And at least with DSI PLL, the wakeup time depends on the frequencies
>> > used (according to TRM), and in some cases it can be optimized, in some
>> > cases not. So I don't think there's one single value that fits all.
>> 
>> A single value isn't necessary.
>> 
>> > Also, I still think it would be better if the driver was also able to
>> > prevent OFF mode explicitely. Defining the max-wakeup-lat with a magic
>> > number sounds a bit prone to breaking up.
>> 
>> I disagree.  What is important is that the driver communicates *why*
>> it needs to prevent OFF mode (can't handle the latency etc.)  and
>> decision making up to the PM core.  The drivers should not embed
>> policy in them.
>> 
>> > But perhaps, as you said, when drivers work properly they don't have to
>> > care about OFF mode as such, but only about the wakeup latency, and thus
>> > the max-wakeup-lat is enough. I'm just not quite sure about that, as OFF
>> > mode may have side effects as the module is totally powered off, while
>> > with RET the side effects should be minimal.
>> >
>> > I don't have any concrete example about the side effects, but one
>> > particular thing I'm thinking about is DSI PLL. If DSS is in RET, I
>> > believe DSI PLL works normally. But if the DSS is reset via OFF mode, I
>> > believe DSI PLL is also reset. But I'm not sure if DSI PLL is ever
>> > needed while DSS would be off, so this may be theoretical =).
>> >
>> >  Tomi
>> 
>> This problem is not unique to DSS, and the other drivers are handling this.
>
> So, how can that DSI PLL example be done? An example use case for the
> above could be a DSI peripheral that requires continuous DSI clock
> (generated by DSI PLL). OFF mode would kill that clock, while RET would
> not. Here we are not interested in latencies, but only in that the DSS
> block is not powered off.

Excuse my ignorance of the DSS/DSI/etc., but if a DSI periperal is use
that requires a continual DSI clock then shouldn't the driver always
keep the DSI clock enabled (iow, it should never call clk_disable()).
If a clock is left enabled, even if OFF-mode is targetted for that
powerdomain, it will not reach OFF because the clockdomain is active.

> Well, I have to say that this example may be a bit far fetched, I'm
> not 100% sure how the HW works. It may be that it's enough to keep
> the DSI power on to keep the DSI clock going. Or not. But the point
> was that perhaps there are situations where OFF mode has side
> effects, while RET doesn't. And in these cases the driver wants to
> disable OFF mode, but doesn't care about the latencies.

I think those specific cases should be explored rather than just
trying to disable OFF mode at a high level.  As I said before, for
good PM, we really want to have a good understanding of all the
reasons for preventing off-mode, with well documented constraints.

In the special cases or special modes that might have OFF-mode side
effects where latency is not the cocern, then those should be handled
by the driver by leaving the (sub)modules active/on or by leaving
clocks enabled for those modes.

So, getting back to the problem of how to prevent DSS OFF because of
latency, the right API to use is

   omap_pm_set_max_dev_wakeup_lat()

The latency passed in is compared to the device-specific latencies in
the table you found and that device's powerdomain state is set
accordingly.

BUT, this currently doesn't work. :(

The problem is that while the omap_device infrastructure is now in
mainline, this API will not work until an omap_device is implemented
for the DSS module.

After I finish pushing a new PM branch based on 2.6.32, I can have a
look at the current DSS driver and make a proposal/patch for how to
add an omap_device.  

In the DSS git, the master branch seems to be based at 2.6.31-rc5.  Do
you have an updated version against 2.6.32-rc1 or omap/master?

Kevin