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

On 28 June 2012 12:11, Tomi Valkeinen [off-list ref] wrote:

On Wed, 2012-06-27 at 20:23 +0530, Jassi Brar wrote:

quoted

On 27 June 2012 13:43, Tomi Valkeinen [off-list ref] wrote:

quoted

I don't like it at all that omapdss disables and enables the panels in
omapdss's suspend/resume hooks. But I'm not sure how this should work...
Should panel drivers each have their own suspend/resume hooks, and
handle it themselves? Or should the call to suspend/resume come from
upper layers, like omapfb or omapdrm.

I made a prototype patch a few weeks ago to move the suspend to omapfb,
and it feels better than the current one, but I'm still not sure...

IIUC, I have similar opinion.
Each panel having its own suspend/resume sounds like inter-dependency trouble.

What do you mean with that? If we just consider omapdss and the panel
drivers, I see no dependency trouble. Panels are independent of each
other, and omapdss is supposed to handle any locking & refcounting
related to multiple panels already, as from omapdss's point of view
panel suspend is the same as panel disable.

And if we take omapfb/omapdrm into equation, well, in any case it
couldn't be any worse than the current one where suspend is handled by
omapdss.

I just anticipate it not trivial keeping  omap_dss_device.state in
sync with omap_dss_driver.suspend/resume
when the latter is made system suspend/resume and former still
affected by hdmi_panel_disable/enable.
Perhaps .disable and .suspend would need merging?

But as I said, it 'sounds' like.  It all may be straight forward - you
would know better.

quoted

I too would prefer suspend/resume propagating from omap-fb/drm, which
imho fits better with the notion of a linux device(omapdss is only
backend). Though I don't have strong feelings about how core then take
various panels up/down optimally.

One one hand, I see the combination of omapdss (or the "output" side of
omapdss) and a panel as a whole entity. I mean, if you just load omapdss
and a panel driver, but no omapfb/omapdrm, you already have a working
panel. You don't _need_ omapfb/omapdrm there. And in that sense it'd
make sense if the panels did handle their own suspend/resume.

Well, I would think if omapdss+panels (backend) serve none other
omapfb/drm, they should simply lie dormant hogging no resources if the
omapfb/drm driver isn't loaded (if that isn't already the case).
Also one usually has tighter control over a private subsystem by
having only one "point of intervention by system" as compared to two
or more.

But then, in real life it may be just simpler if omapfb/omapdrm handles
the suspend.

Yeah, being a simpler option is always there.