Bug 74200

I presume it doesn't actually capture an error-state?

My prime suspect here actually landed in 3.12, so is there any chance you could bisect?

Sure, could you give me a hint where to start? 3.12 contained quite a few commits... ;)

Created attachment 93045 [details]
i915_error_state

The i915_error_state looks the same before as after the attempted suspend (no changes at all).

Similar story as bug 73261 - between us initialising the ring upon resume and writing the first few commands, something else (BIOS!) overwrites our instructions.

(In reply to comment #4)
> Similar story as bug 73261 - between us initialising the ring upon resume
> and writing the first few commands, something else (BIOS!) overwrites our
> instructions.

Not sure if I understand your comment: The problem from my point of view is not the resume but that the system does not suspend at all. Do I misunderstand something here?

My fault, so this is before suspend. Forget everything I said - this should be self-inflicted by i915.ko.

To narrow down the bisect, you can do git bisect start -- drivers/gpu/drm/i915

If it's your first time bisecting, you can follow:

http://landley.net/writing/git-bisect-howto.html

You'll have to bisect between a known to be good and known to be bad versions. If suspend is working in 3.12 for you, then between 3.12.0 and 3.13.0.

Are you comfortable with building kernels? if not I can drop a few pointers here as well.

Thanks for asking, but I know the basics about bisecting and kernel building.
I have found out already that 3.12.9 seems to work fine. I will post here as soon as I have identified the culprit.

And we have a winner:
de45eaf7b9530b6137d3ce370b12b199fae01419 is the first bad commit
commit de45eaf7b9530b6137d3ce370b12b199fae01419
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date:   Fri Oct 18 18:48:24 2013 -0300

    drm/i915: fix open-coded DIV_ROUND_UP
    
    Use the nice Kernel macro, it makes the code much more readable.
    
    Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
    Reviewed-by: Jani Nikula <jani.nikula@intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

:040000 040000 79d8a19a29e2c5bff059ba59625463e17b6a7aa9 bff465921f1f29e2c63bee605759e544e64052c8 M      drivers

If you revert that patch on top of 3.13.0, does that indeed make suspend work again?

git revert de45eaf7b9530b6137d3ce370b12b199fae01419

All I can think of is that the macros get confused:

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ab34163..abe91b8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -251,7 +251,8 @@ i915_gem_dumb_create(struct drm_file *file,
                     struct drm_mode_create_dumb *args)
 {
        /* have to work out size/pitch and return them */
-       args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64);
+       args->pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
+       args->pitch = ALIGN(args->pitch, 64);
        args->size = args->pitch * args->height;
        return i915_gem_create(file, dev, args->size, &args->handle);
 }
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index d6a8a71..f0ef01a 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -74,8 +74,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
        mode_cmd.width = sizes->surface_width;
        mode_cmd.height = sizes->surface_height;
 
-       mode_cmd.pitches[0] = ALIGN(mode_cmd.width *
-                                   DIV_ROUND_UP(sizes->surface_bpp, 8), 64);
+       mode_cmd.pitches[0] = mode_cmd.with * DIV_ROUND_UP(sizes->surface_bpp, 8);
+       mode_cmd.pitches[0] = ALIGN(mode_cmd.pitches[0], 64);
        mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
                                                          sizes->surface_depth);

Unfortunately reverting on top of 3.13.1 does not solve the problem. Somehow bisecting led the wrong way???

It's quite easy to take a wrong turn when bisecting. Next try,

git checkout de45eaf7b9530b6137d3ce370b12b199fae01419 # should fail
git checkout de45eaf7b9530b6137d3ce370b12b199fae01419^ # should pass

If either of those does not perform as expected, start again. However, you can start your bisect with a narrower range to speed up the process (by a couple of steps).

Weird enough manual tests confirm the bisection result...
With dc39fff7229c01550cad1ee8fa0309dfafdcd2e7 (the commit before the one from Paul) it works, with the bisection result it does not.

(In reply to comment #15)
> Weird enough manual tests confirm the bisection result...
> With dc39fff7229c01550cad1ee8fa0309dfafdcd2e7 (the commit before the one
> from Paul) it works, with the bisection result it does not.

How many times did you try both? Once is not enough.

(In reply to comment #16)
> How many times did you try both? Once is not enough.
Each at least 3 times, it is reproducible.

Ok, so based on comment #17 we confirmed that commit de45eaf7b9530b6137d3ce370b12b199fae01419 introduced the problem, but, based on comment #13, if we do a "git revert" on it, the problem does not go away? I'm confused.

This seems to imply that between this commit and 3.13.1 another issue has been introduced which causes a similar issue.

I can't seem to reproduce this on my SNB. Which tree/branch are you using exactly for the bisect? Does this bug still happen for you on drm-intel-nightly branch of our tree linux-3.13.y branch of linux-stable?

Just a shot in the dark: can you please try reverting 828c79087cec61eaf4c76bb32c222fbe35ac3930 (drm/i915: Disable GGTT PTEs on GEN6+ suspend) and/or b35b380ed46bb01726bec1795e6443e625306757 (drm/i915: Make PTE valid encoding optional)? They're an important patch for suspend that happened near the DIV_ROUND_UP patch.

Just recompiled 3.13.1 once more and the issue is gone. Must have done something very weired. Sorry for the noise.
I will try again with 3.13.2 and come back if it happens again.