Re: DSS2/PM on 3.2 broken?

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

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

On Fri, 2012-01-13 at 11:30 -0800, Kevin Hilman wrote:
> Tomi Valkeinen <tomi.valkeinen@ti.com> writes:

> > pm_runtime_put() is called inside omapdss driver's .suspend callback. So
> > I guess that means the system suspend has started. However, the logs
> > show that the runtime_suspend callback _is_ being called before the
> > system suspend is finished, so the workqueue can't be frozen...
> 
> ...or there are other ways that the runtime_suspend callback is called.
> 
> In order to ensure that devices are properly idled for system-wide
> suspend, The OMAP PM domain layer will call the drivers
> ->runtime_suspend callback during "late" suspend (using the PM domain's
> _noirq callbacks.)
> 
> This is there so that even when runtime PM has been disabled (via
> userspace, or pm_runtime_forbid() calls) the driver can still be
> properly idled before suspend.
> 
> In your case, I suspect you're seeing the driver's ->runtime_suspend
> callback called during late suspend, not by the PM workqueue.

Ok, that explains the invocation of the callbacks.

> I don't understand DSS enough to make sense of the logs you sent, so not
> sure how to be of more help.

Well, I think I can summarize the situation:

Normally, when the user wants to turn a display off, the panel drivers
call DSS functions to disable the corresponding output, leading to
pm_runtime_put() calls.

In system suspend case, the suspend callback initiates the turning off
of the displays, which again leads to pm_runtime_put() calls.

Now, you already said using pm_runtime_put_sync version is the correct
way when suspending. But to use that I need to either always use
pm_runtime_put_sync, or add an extra boolean which marks that we're
suspending, and pass that around, or make it a DSS global variable.

Both of those options sound a bit ugly to me. So my two questions are,

1) Is there something funny with the DSS architecture to have a problem
like this? I tried to grep around, but I didn't see any other driver
doing something like that (but I could've missed it).

2) why can't the PM framework handle this, by directing the
pm_runtime_put calls to the _sync version when suspending. Somehow it
feels strange to me that the driver needs to care about that.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]