Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state

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

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

On Mon, 2012-06-25 at 17:57 +0530, Jassi Brar wrote:
> On 25 June 2012 15:00, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> > The driver needs to enable the HW and the call to pm_runtime_get() is
> > skipped. Won't this lead to crash as the DSS registers are accessed
> > without the HW in enabled state?
> >
> Hmm...  how does the extant code in hdmi driver ensures DSS is up and running ?
> While it does sound important even to my limited knowledge of OMAPDSS,
> I see rpm of HDMI, VENC and RFBI only dependent on DISPC, not DSS.

DSS device is parent to all the DSS subdevices. So when a subdevice is
enabled, DSS device is enabled first.

But anyway, I wasn't referring to the DSS part of OMAPDSS, but to
omapdss generally. If we do this:

/* this is skipped, if runtime PM is disabled */
dispc_runtime_get();

/* this accesses a register, but the HW is disabled? */
dispc_read_reg(FOO);

So again, I don't understand how the underlying HW gets enabled. Or does
hwmod/omap_device code make sure that it's enabled while the board is
being resumed? If so, what would happen if we continue the above
scenario as follows:

/* this is skipped, if runtime PM is disabled */
dispc_runtime_get();

/* this accesses a register, the HW is kept enabled by hwmod */
dispc_read_reg(FOO);

/* at some time later the resume procedure ends, and hwmod doesn't keep
the HW enabled any more */

/* this accesses a register, the HW is disabled */
dispc_read_reg(FOO);

> And for DISPC these drivers already hold a reference in their display
> enable/resume and keep it until disable/suspend. So we don't lose
> DISPC anytime it is really required.

If all the displays are disabled, nobody keeps a reference to dispc.

> > And what happens if the pm_runtime_get() call is skipped, but pm_runtime_put() is not?
> >
> Not sure in what newly introduced scenario by this patch, because
> get/put both check for pm_enabled before proceeding. Am I overlooking
> something?

Currently (for example) dispc_runtime_get/put call
pm_runtime_get/put_sync. When somebody uses dispc_runtime_get, the same
somebody knows it needs to call dispc_runtime_put later.

Now, what happens if dispc_runtime_get is called when runtime PM is
disabled (i.e. pm_runtime_get_sync is skipped), but runtime PM is
enabled later when that somebody calls dispc_runtime_put (i.e.
pm_runtime_put_sync is _not_ skipped)?

 Tomi


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