Re: Commit 500f7147cf5bafd139056d521536b10c2bc2e154 breaks _resume_

["Indan Zupancic" <indan@nul.nu>]

On Wed, February 9, 2011 02:05, Jeff Chua wrote:
> On Wed, Feb 9, 2011 at 8:55 AM, Jeff Chua <jeff.chua.linux@gmail.com> wrote:
>
>> And the console hangs even without starting X.
>
> I went back to retry suspending without starting X and realized that
> it's only the "screen" that's hang .. and that's without drm and i915
> loaded.

According to the dmesg you sent, you do have drm (and probably i915 too) loaded.
It seems the hang is the first bit, and then you rebooted into X to capture the
log.

Perhaps relevant message (probably not):

"No ACPI video bus found"

> On the console, I could still reboot the machine normally, but
> not when in X (everything hangs including keybard).
>
> Here's the kernel log without X.
>
> Thanks.
> Jeff
>

Looking at the commit, all it does is changing the timing.

It used to set active to true when intel_crtc_init() was called, but now
it does it always when the drm reset() callback is called.

intel_crtc->active = true; /* force the pipe off on setup_init_config */

I can't find a setup_init_config anywhere, but looking at the other code
it assumes that *_crtc_disable() will be called just after the forced true.

All in all it seems quite wrong, no matter if it happens to work, because
it depends on the calling order done by the drm layer. If *_crtc_enable()
is called instead it won't do anything because of that active = true thing.
This seems to be happening in your case.

So I'd get rid of that dodgy active = true assignment altogether. Isn't
the introduction of the reset() callback done to avoid exactly this kind
of subtle state fiddling? And removing it might solve the original problem
that the move tried to fix as well.

I can't check the rest of the code as I'm still on patched 37 (won't move
till the fix for bug 23472 is upstream), but my gut feeling is that removing
that weird active = true will solve most problems.

Greetings,

Indan