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

On 26 June 2012 12:49, Tomi Valkeinen [off-list ref] wrote:

On Mon, 2012-06-25 at 22:36 +0530, Jassi Brar wrote:

quoted

quoted

Normally if the driver does dispc_runtime_get() and dispc_read_reg(),
the first call will enable the HW so the reg read works.

But if the pm_runtime is disabled, say, during system suspend, with your
patch dispc_runtime_get() will just return 0 without doing anything, and
the dispc_read_reg() will crash because the HW is disabled (because
nobody enabled it).

Hmm, I am not sure if new calls would/should be made to dispc.c after
the system has suspended and before resumed. That is, anything other
than from runtime_resume/suspend callbacks of DSS, DISPC, HDMI, VENC
and RFBI, which rightly don't touch any dss reg but only
enable/disable a clock.

They do touch the registers. For example, dispc's callbacks save and
restore the registers. The HW should be fully functional during the
callbacks. The point of the callbacks is to suspend/resume the HW in
question, which of course requires accessing the HW.

DISPC being held by HDMI, VENC and RFBI would be the last to suspend
and first to resume. And it won't have its registers touched between
dispc_runtime_suspend() and dispc_runtime_resume(), which seems ok to
me (?)
HDMI, VENC and RFBI directly fooling around with DISPC regs would have
been a problem, which isn't the case.

quoted

As we know, a subsystem should make sure any active work is cleared
out before suspending and set some flag so that nothing runs until it
has resumed. I don't say we can't crash the system with this patch,
but then we would be violating rules of suspend-resume.

Let's go back a bit. I feel like I'm missing some pieces of information,
as I still don't quite grasp the problem.

In the patch you said this fixes an issue with HDMI. Can you tell more
about the problem? What code path is being run? Any error messages?

I tried system suspend with omap4-sdp and panda, with 3.5-rc2, but
neither board seems to wake up from the suspend. Does it work for you?

Something non-omapdss in vanilla breaks suspend/resume.
Without this patch I see the upstream's display broken after the
suspend attempt.
    $ echo mem > /sys/power/state

I work on TILT tree, which has suspend/resume working after some more
local patches.
    http://git.linaro.org/gitweb?p=people/andygreen/kernel-tilt.git;a=shortlog;h=refs/heads/tilt-3.4

I don't have SDP so not sure, but it should simply be testable with
Panda4460 and the omap4plus_defconfig there.  Please feel free to ask
if you have any issue checking that out.

Thanks.