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

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

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

On Wed, 2012-06-27 at 10:12 +0530, Jassi Brar wrote:

> > True. But the HW could also be in disabled state. And that would lead to
> > a crash when accessing the registers.
> >
> > It is not a fatal error if pm_runtime_get returns -EACCES, but we sure
> > shouldn't ignore it (or avoid it with pm_runtime_enabled()), but handle
> > it. In some rare cases it could be ok to get -EACCES, but that's a
> > special case, not standard.
> >
> You are mixing up generic concepts with what we have in omapdss.
> Believe me, I do understand it's bad to proceed without caring for
> returned _errors_.
> The way omapdss is organized -EACCESS is _not_ an  error, it just
> denotes PM is disabled on the device and that DISPC is in RPM_ACTIVE
> is backed by the fact that HDMI always hold a reference between
> resume-suspend and DISPC goes to suspend last and resume first.

I'm not arguing that your solution would not work with the omapdss code
we have now, and presuming the underlying frameworks work fine. But I
want omapdss to have code that works also in the future, when other
parts of omapdss change.

It doesn't matter how omapdss is organized, -EACCES _is_ an error. It
tells us that something unexpected happened, and we should react to it
somehow.

When we call, for example, dispc_runtime_get(), we normally expect that
runtime PM is enabled, and it 1) "gets" it, increasing the use count,
and 2) makes sure the HW is enabled so it can be used. Your patch breaks
both of these if runtime PM is disabled.

Sure, in the current omapdss neither is a breaking problem, because 1)
the matching dispc_runtime_put() is called also with runtime PM
disabled, and thus we don't decrease the use count, and 2) the HW
happens to be already enabled. But that's just by "luck", and tomorrow
omapdss could be different. 

If, for some reason, we need to call dispc_runtime_get/put during
suspend, the caller should either 1) realize that we're suspending,
runtime PM is disabled, but the HW is anyway enabled, and thus skip
calling both get and put, or 2) call both get and put, but handle the
-EACCES, understanding what it means and knowing the HW is anyway
enabled.

 Tomi


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