[2.6.39 regression] hard lock when GNOME starts

[2.6.39 regression] hard lock when GNOME starts

[Andrew Lutomirski]

Hi-

Something in the range ^8aa7500 40c7f2112ce18fa5eb ^b04d0a90908c
causes by Q67 Sandy Bridge box to lock hard about one second after I
start GNOME.  It locks so hard that the reset button doesn't work and
netconsole doesn't say anything.

I'm about to have trouble finishing the bisection because I've had
trouble running revisions that are older than 2.6.38 and the next bit
of bisection has merge conflicts if I merge in 2.6.38.  I can power
through it, but maybe one of you will recognize the bug from the fact
that there aren't that many revisions in there and there can't be many
ways to make the reset button stop working.

--Andy

Re: [2.6.39 regression] hard lock when GNOME starts

[Andrew Lutomirski]

On Fri, May 13, 2011 at 1:38 AM, Andrew Lutomirski <luto@mit.edu> wrote:
> Hi-
>
> Something in the range ^8aa7500 40c7f2112ce18fa5eb ^b04d0a90908c
> causes by Q67 Sandy Bridge box to lock hard about one second after I
> start GNOME.  It locks so hard that the reset button doesn't work and
> netconsole doesn't say anything.
>
> I'm about to have trouble finishing the bisection because I've had
> trouble running revisions that are older than 2.6.38 and the next bit
> of bisection has merge conflicts if I merge in 2.6.38.  I can power
> through it, but maybe one of you will recognize the bug from the fact
> that there aren't that many revisions in there and there can't be many
> ways to make the reset button stop working.

It's semaphores.  I have:

00:02.0 VGA compatible controller: Intel Corporation Sandy Bridge
Integrated Graphics Controller (rev 09)

which is on a Q67/i7-2600 box.

The git history for semaphores is worthless.

AFAICT they were disabled around 2.6.38-rc7 by a merge that pulled in
a1656b9090f7008d2941c314f5a64724bea2ae37.

They were then re-enabled by this commit:

commit 47ae63e0c2e5fdb582d471dc906eb29be94c732f
Merge: c59a333 467cffb
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Mar 7 12:32:44 2011 +0000

    Merge branch 'drm-intel-fixes' into drm-intel-next

    Apply the trivial conflicting regression fixes, but keep GPU semaphores
    enabled.

    Conflicts:
        drivers/gpu/drm/i915/i915_drv.h
        drivers/gpu/drm/i915/i915_gem_execbuffer.c

This pulled in the disable-semaphores patch.  So the merge with
trivial regression fixes undid the patch that disabled semaphores.

This is really annoying for two reasons.  One is that the commit
message sucks.  Two is much worse: the semaphore disabling patch made
it into 2.6.38 but this merge went in for 2.6.39.  That means that the
patch that made my system unusable on 2.6.39 versions *does not exist
in the set v2.6.38..v2.6.39-rc7* except as a "trivial" merge.  Which
means that the two fscking days I spent bisecting this were completely
wasted because there was no way that the bisection could have found
the bug.

In the future, if you change something, that might cause severe
breakage, please make it an actual commit.

--Andy



>
> --Andy
>

[PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Andy Lutomirski]

My Q67 / i7-2600 box has rev09 Sandy Bridge graphics.  It hangs
instantly when GNOME loads and it hangs so hard the reset button
doesn't work.  Setting i915.semaphore=0 fixes it.

Semaphores were disabled in a1656b9090f7008d2941c314f5a64724bea2ae37
in 2.6.38 and were re-enabled by

commit 47ae63e0c2e5fdb582d471dc906eb29be94c732f
Merge: c59a333 467cffb
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Mar 7 12:32:44 2011 +0000

    Merge branch 'drm-intel-fixes' into drm-intel-next

    Apply the trivial conflicting regression fixes, but keep GPU semaphores
    enabled.

    Conflicts:
        drivers/gpu/drm/i915/i915_drv.h
        drivers/gpu/drm/i915/i915_gem_execbuffer.c

(It's worth noting that the offending change is i915_drv.c,
 which is not a conflict.)

Signed-off-by: Andy Lutomirski <luto@mit.edu>
---

This fixes a 2.6.39 regression that affects my Q67 box but not
my SNB laptop.

Send to Linus and airlied because 2.6.39 is apparently about to
be released.

 drivers/gpu/drm/i915/i915_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c34a8dd..32d1b3e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -49,7 +49,7 @@ module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600);
 unsigned int i915_powersave = 1;
 module_param_named(powersave, i915_powersave, int, 0600);
 
-unsigned int i915_semaphores = 1;
+unsigned int i915_semaphores = 0;
 module_param_named(semaphores, i915_semaphores, int, 0600);
 
 unsigned int i915_enable_rc6 = 0;
-- 
1.7.5.1

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 296 bytes --]

On Fri, 13 May 2011 12:14:54 -0400, Andy Lutomirski <luto@MIT.EDU> wrote:

> -unsigned int i915_semaphores = 1;
> +unsigned int i915_semaphores = 0;
>  module_param_named(semaphores, i915_semaphores, int, 0600);

Acked-by: Keith Packard <keithp@keithp.com>

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 581 bytes --]

On Fri, 13 May 2011 12:14:54 -0400, Andy Lutomirski <luto@MIT.EDU> wrote:

> My Q67 / i7-2600 box has rev09 Sandy Bridge graphics.  It hangs
> instantly when GNOME loads and it hangs so hard the reset button
> doesn't work.  Setting i915.semaphore=0 fixes it.

Can you describe precisely what hardware you have? The make and model of
the motherboard and the exact model of CPU? I'd like to get this issue
resolved for 2.6.40 and need to have a reliable way to reproduce the
hang; replicating your hardware may be a good way of doing that.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Andrew Lutomirski]

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

On Thu, May 19, 2011 at 3:56 PM, Keith Packard <keithp@keithp.com> wrote:
> On Fri, 13 May 2011 12:14:54 -0400, Andy Lutomirski <luto@MIT.EDU> wrote:
>
>> My Q67 / i7-2600 box has rev09 Sandy Bridge graphics.  It hangs
>> instantly when GNOME loads and it hangs so hard the reset button
>> doesn't work.  Setting i915.semaphore=0 fixes it.
>
> Can you describe precisely what hardware you have? The make and model of
> the motherboard and the exact model of CPU? I'd like to get this issue
> resolved for 2.6.40 and need to have a reliable way to reproduce the
> hang; replicating your hardware may be a good way of doing that.

Intel DQ67SW.  AMT is enabled but the port 5900 is closed (because I
can't figure out how to turn on the kvm).

The system boots from UEFI (which is buggy, both because efifb doesn't
recognize my hardware, because asking the firmware for the boot menu
causes all efivars entries to be ignored, and because unless I set
reboot=k the system crashes on reboot).

Userspace is F15 running compiz (*not* gnome-shell).

BIOS is SWQ6710H.86A.0051.2011.0413.1154.

CPU is:
processor       : 0
vendor_id       : GenuineIntel
cpu family      : 6
model           : 42
model name      : Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz
stepping        : 7

config and dmidecode output attached.

I'm happy to help test after Sunday.  I'm currently out of town and
busy trying to make my Sandy Bridge laptop work.  (It has a different
non-i915-related problem.)

--Andy

[-- Attachment #2: config.txt.xz --]
[-- Type: application/x-xz, Size: 24716 bytes --]

[-- Attachment #3: dmi.txt.xz --]
[-- Type: application/x-xz, Size: 3864 bytes --]

[-- Attachment #4: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Andrew Lutomirski]

On Thu, May 19, 2011 at 4:50 PM, Andrew Lutomirski <luto@mit.edu> wrote:
> On Thu, May 19, 2011 at 3:56 PM, Keith Packard <keithp@keithp.com> wrote:
>> On Fri, 13 May 2011 12:14:54 -0400, Andy Lutomirski <luto@MIT.EDU> wrote:
>>
>>> My Q67 / i7-2600 box has rev09 Sandy Bridge graphics.  It hangs
>>> instantly when GNOME loads and it hangs so hard the reset button
>>> doesn't work.  Setting i915.semaphore=0 fixes it.
>>
>> Can you describe precisely what hardware you have? The make and model of
>> the motherboard and the exact model of CPU? I'd like to get this issue
>> resolved for 2.6.40 and need to have a reliable way to reproduce the
>> hang; replicating your hardware may be a good way of doing that.
>

I'm getting hangs on my X220 Core i7 laptop as well with 2.6.39 and
i915.semaphores=1.  They're not as reliable -- sometimes the system
hangs on log in, sometimes after a few seconds, and sometimes it
doesn't hang.

--Andy

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 549 bytes --]

On Tue, 24 May 2011 13:10:27 -0400, Andrew Lutomirski <luto@mit.edu> wrote:

> I'm getting hangs on my X220 Core i7 laptop as well with 2.6.39 and
> i915.semaphores=1.  They're not as reliable -- sometimes the system
> hangs on log in, sometimes after a few seconds, and sometimes it
> doesn't hang.

That's consistent with our theory at least -- lockups caused by GPU/CPU
interactions, which occur far more often with semaphores disabled, but
still occur with semaphores enabled.

We're working on it...

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Ivan Bulatovic]

On Tue, 24 May 2011 13:10:27 -0400
Andrew Lutomirski <luto@mit.edu> wrote:

> On Thu, May 19, 2011 at 4:50 PM, Andrew Lutomirski <luto@mit.edu>
> wrote:
> > On Thu, May 19, 2011 at 3:56 PM, Keith Packard <keithp@keithp.com>
> > wrote:
> >> On Fri, 13 May 2011 12:14:54 -0400, Andy Lutomirski <luto@MIT.EDU>
> >> wrote:
> >>
> >>> My Q67 / i7-2600 box has rev09 Sandy Bridge graphics.  It hangs
> >>> instantly when GNOME loads and it hangs so hard the reset button
> >>> doesn't work.  Setting i915.semaphore=0 fixes it.
> >>
> >> Can you describe precisely what hardware you have? The make and
> >> model of the motherboard and the exact model of CPU? I'd like to
> >> get this issue resolved for 2.6.40 and need to have a reliable way
> >> to reproduce the hang; replicating your hardware may be a good way
> >> of doing that.
> >
> 
> I'm getting hangs on my X220 Core i7 laptop as well with 2.6.39 and
> i915.semaphores=1.  They're not as reliable -- sometimes the system
> hangs on log in, sometimes after a few seconds, and sometimes it
> doesn't hang.
> 
> --Andy

I haven't had GPU hangs since this patch:

https://bugs.freedesktop.org/show_bug.cgi?id=33921#c23

Poll the FIFO for free entries before writing the register

Gigabyte GA-H67MA-UD2H-B3 + i5 2400 here.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1850 bytes --]

On Thu, 19 May 2011 16:50:00 -0400, Andrew Lutomirski <luto@mit.edu> wrote:
> On Thu, May 19, 2011 at 3:56 PM, Keith Packard <keithp@keithp.com> wrote:
> > On Fri, 13 May 2011 12:14:54 -0400, Andy Lutomirski <luto@MIT.EDU> wrote:
> >
> >> My Q67 / i7-2600 box has rev09 Sandy Bridge graphics.  It hangs
> >> instantly when GNOME loads and it hangs so hard the reset button
> >> doesn't work.  Setting i915.semaphore=0 fixes it.
> >
> > Can you describe precisely what hardware you have? The make and model of
> > the motherboard and the exact model of CPU? I'd like to get this issue
> > resolved for 2.6.40 and need to have a reliable way to reproduce the
> > hang; replicating your hardware may be a good way of doing that.
> 
> Intel DQ67SW.  AMT is enabled but the port 5900 is closed (because I
> can't figure out how to turn on the kvm).
> 
> The system boots from UEFI (which is buggy, both because efifb doesn't
> recognize my hardware, because asking the firmware for the boot menu
> causes all efivars entries to be ignored, and because unless I set
> reboot=k the system crashes on reboot).
> 
> Userspace is F15 running compiz (*not* gnome-shell).
> 
> BIOS is SWQ6710H.86A.0051.2011.0413.1154.
> 
> CPU is:
> processor       : 0
> vendor_id       : GenuineIntel
> cpu family      : 6
> model           : 42
> model name      : Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz
> stepping        : 7

I fear this bug is just "getting more asynchronous with the GPU means we
trigger race conditions on the GPU more easily."  On that note, could
you retest with Mesa as of this commit, since I'm told this is a GT1 CPU:

commit ef59049c5242a1be7fa59a182d342191185dd62b
Author: Eric Anholt <eric@anholt.net>
Date:   Sun Jun 5 23:20:57 2011 -0700

    i965: Fix flipped GT1 vs GT2 URB VS entry count limits.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Andrew Lutomirski]

On Tue, Jun 7, 2011 at 3:12 AM, Eric Anholt <eric@anholt.net> wrote:
> On Thu, 19 May 2011 16:50:00 -0400, Andrew Lutomirski <luto@mit.edu> wrote:
>> On Thu, May 19, 2011 at 3:56 PM, Keith Packard <keithp@keithp.com> wrote:
>> > On Fri, 13 May 2011 12:14:54 -0400, Andy Lutomirski <luto@MIT.EDU> wrote:
>> >
>> >> My Q67 / i7-2600 box has rev09 Sandy Bridge graphics.  It hangs
>> >> instantly when GNOME loads and it hangs so hard the reset button
>> >> doesn't work.  Setting i915.semaphore=0 fixes it.
>> >
>> > Can you describe precisely what hardware you have? The make and model of
>> > the motherboard and the exact model of CPU? I'd like to get this issue
>> > resolved for 2.6.40 and need to have a reliable way to reproduce the
>> > hang; replicating your hardware may be a good way of doing that.
>>
>> Intel DQ67SW.  AMT is enabled but the port 5900 is closed (because I
>> can't figure out how to turn on the kvm).
>>
>> The system boots from UEFI (which is buggy, both because efifb doesn't
>> recognize my hardware, because asking the firmware for the boot menu
>> causes all efivars entries to be ignored, and because unless I set
>> reboot=k the system crashes on reboot).
>>
>> Userspace is F15 running compiz (*not* gnome-shell).
>>
>> BIOS is SWQ6710H.86A.0051.2011.0413.1154.
>>
>> CPU is:
>> processor       : 0
>> vendor_id       : GenuineIntel
>> cpu family      : 6
>> model           : 42
>> model name      : Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz
>> stepping        : 7
>
> I fear this bug is just "getting more asynchronous with the GPU means we
> trigger race conditions on the GPU more easily."  On that note, could
> you retest with Mesa as of this commit, since I'm told this is a GT1 CPU:
>
> commit ef59049c5242a1be7fa59a182d342191185dd62b
> Author: Eric Anholt <eric@anholt.net>
> Date:   Sun Jun 5 23:20:57 2011 -0700
>
>    i965: Fix flipped GT1 vs GT2 URB VS entry count limits.
>

Nope -- I still got the reset-button-not-working hang.

I'm pretty sure I installed mesa right -- I moved everything in
/usr/lib64/dri out of the way and put the files in mesa/lib/ in there.

--Andy

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Jesse Barnes]

On Fri, 10 Jun 2011 10:06:39 -0400
Andrew Lutomirski <luto@mit.edu> wrote:

> On Tue, Jun 7, 2011 at 3:12 AM, Eric Anholt <eric@anholt.net> wrote:
> > I fear this bug is just "getting more asynchronous with the GPU means we
> > trigger race conditions on the GPU more easily."  On that note, could
> > you retest with Mesa as of this commit, since I'm told this is a GT1 CPU:
> >
> > commit ef59049c5242a1be7fa59a182d342191185dd62b
> > Author: Eric Anholt <eric@anholt.net>
> > Date:   Sun Jun 5 23:20:57 2011 -0700
> >
> >    i965: Fix flipped GT1 vs GT2 URB VS entry count limits.
> >
> 
> Nope -- I still got the reset-button-not-working hang.
> 
> I'm pretty sure I installed mesa right -- I moved everything in
> /usr/lib64/dri out of the way and put the files in mesa/lib/ in there.

Andrew, what's the latest with this?  Are you still seeing problems
when semaphores are enabled?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Ben Widawsky]

On Mon, 22 Aug 2011 09:53:11 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Fri, 10 Jun 2011 10:06:39 -0400
> Andrew Lutomirski <luto@mit.edu> wrote:
> 
> > On Tue, Jun 7, 2011 at 3:12 AM, Eric Anholt <eric@anholt.net> wrote:
> > > I fear this bug is just "getting more asynchronous with the GPU
> > > means we trigger race conditions on the GPU more easily."  On
> > > that note, could you retest with Mesa as of this commit, since
> > > I'm told this is a GT1 CPU:
> > >
> > > commit ef59049c5242a1be7fa59a182d342191185dd62b
> > > Author: Eric Anholt <eric@anholt.net>
> > > Date:   Sun Jun 5 23:20:57 2011 -0700
> > >
> > >    i965: Fix flipped GT1 vs GT2 URB VS entry count limits.
> > >
> > 
> > Nope -- I still got the reset-button-not-working hang.
> > 
> > I'm pretty sure I installed mesa right -- I moved everything in
> > /usr/lib64/dri out of the way and put the files in mesa/lib/ in
> > there.
> 
> Andrew, what's the latest with this?  Are you still seeing problems
> when semaphores are enabled?
> 
> Thanks,


Ping
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Andrew Lutomirski]

[-- Attachment #1.1: Type: text/plain, Size: 1184 bytes --]

On Mon, Aug 22, 2011 at 12:53 PM, Jesse Barnes <jbarnes@virtuousgeek.org>
wrote:
> On Fri, 10 Jun 2011 10:06:39 -0400
> Andrew Lutomirski <luto@mit.edu> wrote:
>
>> On Tue, Jun 7, 2011 at 3:12 AM, Eric Anholt <eric@anholt.net> wrote:
>> > I fear this bug is just "getting more asynchronous with the GPU means
we
>> > trigger race conditions on the GPU more easily."  On that note, could
>> > you retest with Mesa as of this commit, since I'm told this is a GT1
CPU:
>> >
>> > commit ef59049c5242a1be7fa59a182d342191185dd62b
>> > Author: Eric Anholt <eric@anholt.net>
>> > Date:   Sun Jun 5 23:20:57 2011 -0700
>> >
>> >    i965: Fix flipped GT1 vs GT2 URB VS entry count limits.
>> >
>>
>> Nope -- I still got the reset-button-not-working hang.
>>
>> I'm pretty sure I installed mesa right -- I moved everything in
>> /usr/lib64/dri out of the way and put the files in mesa/lib/ in there.
>
> Andrew, what's the latest with this?  Are you still seeing problems
> when semaphores are enabled?

Yes. On the latest -rc, the system still freezes hard when mutter starts up
if I set i915.semaphores=1.

[I typed this email last week but apparently it got stuck as a draft.
Sorry.]

--Andy

[-- Attachment #1.2: Type: text/html, Size: 1668 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 339 bytes --]

On Wed, 31 Aug 2011 14:30:00 -0400, Andrew Lutomirski <luto@mit.edu> wrote:

> Yes. On the latest -rc, the system still freezes hard when mutter starts up
> if I set i915.semaphores=1.

Ok. You said that you were running compiz before and that failed, and
are now running mutter and that fails also?

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Revert i915.semaphore=1 default from 47ae63e0

[Andrew Lutomirski]

[-- Attachment #1.1: Type: text/plain, Size: 566 bytes --]

Yes.  I suspect that, as soon as any 3D happens, the machine locks hard.

How does it even get into a state that makes the hardware reset button not
work?
On Aug 31, 2011 3:08 PM, "Keith Packard" <keithp@keithp.com> wrote:
> On Wed, 31 Aug 2011 14:30:00 -0400, Andrew Lutomirski <luto@mit.edu>
wrote:
>
>> Yes. On the latest -rc, the system still freezes hard when mutter starts
up
>> if I set i915.semaphores=1.
>
> Ok. You said that you were running compiz before and that failed, and
> are now running mutter and that fails also?
>
> --
> keith.packard@intel.com

[-- Attachment #1.2: Type: text/html, Size: 850 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: kicking rings considered harmful

[Daniel Vetter]

Only do it in the hope of resurrecting the gpu. Disable when reset is
disabled because it seems to tremendously increases our changes to
actually capture an error_state before the system goes all belly-up.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
Hi Andrew,

Can you please apply this patch and boot your system with

i915.reset=0 i915.semaphores=1

and rehang your gpu? This patch to fully disable any attempts at
resurrecting a dead gpu hopefully prevents the full system hang you're
experiencing. At least it helps greatly here on my systems.

If the systems isn't completely dead with this, can you please ssh
into the machine and grabe dmesg, i915_error_state, Xorg.log and
whatever else there might be?

Thanks a lot,

Daniel

 drivers/gpu/drm/i915/i915_drv.c |    2 +-
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 drivers/gpu/drm/i915/i915_irq.c |    2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b79c6f1..ad85c13 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -91,7 +91,7 @@ MODULE_PARM_DESC(vbt_sdvo_panel_type,
 		"Override selection of SDVO panel mode in the VBT "
 		"(default: auto)");
 
-static bool i915_try_reset __read_mostly = true;
+bool i915_try_reset __read_mostly = true;
 module_param_named(reset, i915_try_reset, bool, 0600);
 MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3621336..788a801 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -995,6 +995,7 @@ extern unsigned int i915_semaphores __read_mostly;
 extern unsigned int i915_lvds_downclock __read_mostly;
 extern unsigned int i915_panel_use_ssc __read_mostly;
 extern int i915_vbt_sdvo_panel_type __read_mostly;
+extern bool i915_try_reset __read_mostly;
 extern unsigned int i915_enable_rc6 __read_mostly;
 extern unsigned int i915_enable_fbc __read_mostly;
 extern bool i915_enable_hangcheck __read_mostly;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index da5d607..09c11e4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1694,7 +1694,7 @@ void i915_hangcheck_elapsed(unsigned long data)
 		if (dev_priv->hangcheck_count++ > 1) {
 			DRM_ERROR("Hangcheck timer elapsed... GPU hung\n");
 
-			if (!IS_GEN2(dev)) {
+			if (!IS_GEN2(dev) && i915_try_reset) {
 				/* Is the chip hanging on a WAIT_FOR_EVENT?
 				 * If so we can simply poke the RB_WAIT bit
 				 * and break the hang. This should work on
-- 
1.7.6.2

Re: [PATCH] drm/i915: kicking rings considered harmful

[Andrew Lutomirski]

[-- Attachment #1.1: Type: text/plain, Size: 3131 bytes --]

On Sep 26, 2011 9:00 PM, "Daniel Vetter" <daniel.vetter@ffwll.ch> wrote:
>
> Only do it in the hope of resurrecting the gpu. Disable when reset is
> disabled because it seems to tremendously increases our changes to
> actually capture an error_state before the system goes all belly-up.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> Hi Andrew,
>
> Can you please apply this patch and boot your system with
>

Will do in a couple days.  I'm several thousand miles from that computer
right now :)

--Andy

> i915.reset=0 i915.semaphores=1
>
> and rehang your gpu? This patch to fully disable any attempts at
> resurrecting a dead gpu hopefully prevents the full system hang you're
> experiencing. At least it helps greatly here on my systems.
>
> If the systems isn't completely dead with this, can you please ssh
> into the machine and grabe dmesg, i915_error_state, Xorg.log and
> whatever else there might be?
>
> Thanks a lot,
>
> Daniel
>
>  drivers/gpu/drm/i915/i915_drv.c |    2 +-
>  drivers/gpu/drm/i915/i915_drv.h |    1 +
>  drivers/gpu/drm/i915/i915_irq.c |    2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
> index b79c6f1..ad85c13 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -91,7 +91,7 @@ MODULE_PARM_DESC(vbt_sdvo_panel_type,
>                "Override selection of SDVO panel mode in the VBT "
>                "(default: auto)");
>
> -static bool i915_try_reset __read_mostly = true;
> +bool i915_try_reset __read_mostly = true;
>  module_param_named(reset, i915_try_reset, bool, 0600);
>  MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
> index 3621336..788a801 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -995,6 +995,7 @@ extern unsigned int i915_semaphores __read_mostly;
>  extern unsigned int i915_lvds_downclock __read_mostly;
>  extern unsigned int i915_panel_use_ssc __read_mostly;
>  extern int i915_vbt_sdvo_panel_type __read_mostly;
> +extern bool i915_try_reset __read_mostly;
>  extern unsigned int i915_enable_rc6 __read_mostly;
>  extern unsigned int i915_enable_fbc __read_mostly;
>  extern bool i915_enable_hangcheck __read_mostly;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
> index da5d607..09c11e4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1694,7 +1694,7 @@ void i915_hangcheck_elapsed(unsigned long data)
>                if (dev_priv->hangcheck_count++ > 1) {
>                        DRM_ERROR("Hangcheck timer elapsed... GPU hung\n");
>
> -                       if (!IS_GEN2(dev)) {
> +                       if (!IS_GEN2(dev) && i915_try_reset) {
>                                /* Is the chip hanging on a WAIT_FOR_EVENT?
>                                 * If so we can simply poke the RB_WAIT bit
>                                 * and break the hang. This should work on
> --
> 1.7.6.2
>

[-- Attachment #1.2: Type: text/html, Size: 3911 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: kicking rings considered harmful

[Ben Widawsky]

On Mon, 26 Sep 2011 19:59:50 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Only do it in the hope of resurrecting the gpu. Disable when reset is
> disabled because it seems to tremendously increases our changes to
> actually capture an error_state before the system goes all belly-up.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> Hi Andrew,
> 
> Can you please apply this patch and boot your system with
> 
> i915.reset=0 i915.semaphores=1
> 
> and rehang your gpu? This patch to fully disable any attempts at
> resurrecting a dead gpu hopefully prevents the full system hang you're
> experiencing. At least it helps greatly here on my systems.
> 
> If the systems isn't completely dead with this, can you please ssh
> into the machine and grabe dmesg, i915_error_state, Xorg.log and
> whatever else there might be?
> 
> Thanks a lot,
> 
> Daniel
> 
>  drivers/gpu/drm/i915/i915_drv.c |    2 +-
>  drivers/gpu/drm/i915/i915_drv.h |    1 +
>  drivers/gpu/drm/i915/i915_irq.c |    2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c index b79c6f1..ad85c13 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -91,7 +91,7 @@ MODULE_PARM_DESC(vbt_sdvo_panel_type,
>  		"Override selection of SDVO panel mode in the VBT "
>  		"(default: auto)");
>  
> -static bool i915_try_reset __read_mostly = true;
> +bool i915_try_reset __read_mostly = true;
>  module_param_named(reset, i915_try_reset, bool, 0600);
>  MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 3621336..788a801 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -995,6 +995,7 @@ extern unsigned int i915_semaphores __read_mostly;
>  extern unsigned int i915_lvds_downclock __read_mostly;
>  extern unsigned int i915_panel_use_ssc __read_mostly;
>  extern int i915_vbt_sdvo_panel_type __read_mostly;
> +extern bool i915_try_reset __read_mostly;
>  extern unsigned int i915_enable_rc6 __read_mostly;
>  extern unsigned int i915_enable_fbc __read_mostly;
>  extern bool i915_enable_hangcheck __read_mostly;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c index da5d607..09c11e4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1694,7 +1694,7 @@ void i915_hangcheck_elapsed(unsigned long data)
>  		if (dev_priv->hangcheck_count++ > 1) {
>  			DRM_ERROR("Hangcheck timer elapsed... GPU
> hung\n"); 
> -			if (!IS_GEN2(dev)) {
> +			if (!IS_GEN2(dev) && i915_try_reset) {
>  				/* Is the chip hanging on a
> WAIT_FOR_EVENT?
>  				 * If so we can simply poke the
> RB_WAIT bit
>  				 * and break the hang. This should
> work on

I think you should also be able to accomplish the same thing
with enable_hangcheck param. I had the same problem with the
debugger :)

Ben

Re: [PATCH] drm/i915: kicking rings considered harmful

[Daniel Vetter]

On Mon, Sep 26, 2011 at 21:07, Andrew Lutomirski <luto@mit.edu> wrote:
> On Sep 26, 2011 9:00 PM, "Daniel Vetter" <daniel.vetter@ffwll.ch> wrote:
>>
>> Only do it in the hope of resurrecting the gpu. Disable when reset is
>> disabled because it seems to tremendously increases our changes to
>> actually capture an error_state before the system goes all belly-up.
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>> Hi Andrew,
>>
>> Can you please apply this patch and boot your system with
>>
>
> Will do in a couple days.  I'm several thousand miles from that computer
> right now :)

No problem. Something else to try: Are you by chance running with
DMAR/VT-d enabled? If so, please disable it (either in the BIOS or
with intel_iommu=off) and see whether that works around the semaphore
hangs.

Perhaps do this before trying out the patch, doesn't require a new kernel ;-)

Thanks a lot, Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: kicking rings considered harmful

[Daniel Vetter]

On Mon, Sep 26, 2011 at 10:22:01PM -0700, Ben Widawsky wrote:
> On Mon, 26 Sep 2011 19:59:50 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c index da5d607..09c11e4 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1694,7 +1694,7 @@ void i915_hangcheck_elapsed(unsigned long data)
> >  		if (dev_priv->hangcheck_count++ > 1) {
> >  			DRM_ERROR("Hangcheck timer elapsed... GPU
> > hung\n"); 
> > -			if (!IS_GEN2(dev)) {
> > +			if (!IS_GEN2(dev) && i915_try_reset) {
> >  				/* Is the chip hanging on a
> > WAIT_FOR_EVENT?
> >  				 * If so we can simply poke the
> > RB_WAIT bit
> >  				 * and break the hang. This should
> > work on
> 
> I think you should also be able to accomplish the same thing
> with enable_hangcheck param. I had the same problem with the
> debugger :)

I agree. Iirc you have some patches floating in that area to make the
hangcheck a bit more robust. Can you maybe add this to that series and
(re-)submit?

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: kicking rings considered harmful

[Ben Widawsky]

On Tue, 27 Sep 2011 12:03:22 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Sep 26, 2011 at 10:22:01PM -0700, Ben Widawsky wrote:
> > On Mon, 26 Sep 2011 19:59:50 +0200
> > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > b/drivers/gpu/drm/i915/i915_irq.c index da5d607..09c11e4 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -1694,7 +1694,7 @@ void i915_hangcheck_elapsed(unsigned long data)
> > >  		if (dev_priv->hangcheck_count++ > 1) {
> > >  			DRM_ERROR("Hangcheck timer elapsed... GPU
> > > hung\n"); 
> > > -			if (!IS_GEN2(dev)) {
> > > +			if (!IS_GEN2(dev) && i915_try_reset) {
> > >  				/* Is the chip hanging on a
> > > WAIT_FOR_EVENT?
> > >  				 * If so we can simply poke the
> > > RB_WAIT bit
> > >  				 * and break the hang. This should
> > > work on
> > 
> > I think you should also be able to accomplish the same thing
> > with enable_hangcheck param. I had the same problem with the
> > debugger :)
> 
> I agree. Iirc you have some patches floating in that area to make the
> hangcheck a bit more robust. Can you maybe add this to that series and
> (re-)submit?
> 
> Cheers, Daniel

While 9/10 times daniel > ben, I'm playing my 10% card here and
suggesting that mixing the reset variable and ring kick is not the right
way to go about this.

However I will resubmit my series based on other feedback, and if Chris
or Keith chime in and say I'm this isn't my 1/10, I'll add this too :)

Ben

Re: [PATCH] drm/i915: kicking rings considered harmful

[Chris Wilson]

On Tue, 27 Sep 2011 09:46:14 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> On Tue, 27 Sep 2011 12:03:22 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Mon, Sep 26, 2011 at 10:22:01PM -0700, Ben Widawsky wrote:
> > > On Mon, 26 Sep 2011 19:59:50 +0200
> > > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > > b/drivers/gpu/drm/i915/i915_irq.c index da5d607..09c11e4 100644
> > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > @@ -1694,7 +1694,7 @@ void i915_hangcheck_elapsed(unsigned long data)
> > > >  		if (dev_priv->hangcheck_count++ > 1) {
> > > >  			DRM_ERROR("Hangcheck timer elapsed... GPU
> > > > hung\n"); 
> > > > -			if (!IS_GEN2(dev)) {
> > > > +			if (!IS_GEN2(dev) && i915_try_reset) {
> > > >  				/* Is the chip hanging on a
> > > > WAIT_FOR_EVENT?
> > > >  				 * If so we can simply poke the
> > > > RB_WAIT bit
> > > >  				 * and break the hang. This should
> > > > work on
> > > 
> > > I think you should also be able to accomplish the same thing
> > > with enable_hangcheck param. I had the same problem with the
> > > debugger :)
> > 
> > I agree. Iirc you have some patches floating in that area to make the
> > hangcheck a bit more robust. Can you maybe add this to that series and
> > (re-)submit?
> > 
> > Cheers, Daniel
> 
> While 9/10 times daniel > ben, I'm playing my 10% card here and
> suggesting that mixing the reset variable and ring kick is not the right
> way to go about this.

One purpose of the i915.reset parameter is to disable any automatic
attempts to recover from a hang condition so that the error state is not
misleading. So preventing the kick ring does help in that regard.

A second purpose is to prevent i915_reset() from causing havoc and hanging
the machine. Daniel is implying that kicking the rings is instrumental in
making matters worse. Again using i915.reset to prevent kicking the rings
fits in with that purpose.

Since I regard kicking rings as a form of reset, I don't see it as a
conflation of terms and so a valid use of i915.reset.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: kicking rings considered harmful

[Daniel Vetter]

On Tue, Sep 27, 2011 at 06:31:59PM +0100, Chris Wilson wrote:
> On Tue, 27 Sep 2011 09:46:14 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > On Tue, 27 Sep 2011 12:03:22 +0200
> > Daniel Vetter <daniel@ffwll.ch> wrote:
> > 
> > > On Mon, Sep 26, 2011 at 10:22:01PM -0700, Ben Widawsky wrote:
> > > > On Mon, 26 Sep 2011 19:59:50 +0200
> > > > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > > > b/drivers/gpu/drm/i915/i915_irq.c index da5d607..09c11e4 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > > @@ -1694,7 +1694,7 @@ void i915_hangcheck_elapsed(unsigned long data)
> > > > >  		if (dev_priv->hangcheck_count++ > 1) {
> > > > >  			DRM_ERROR("Hangcheck timer elapsed... GPU
> > > > > hung\n"); 
> > > > > -			if (!IS_GEN2(dev)) {
> > > > > +			if (!IS_GEN2(dev) && i915_try_reset) {
> > > > >  				/* Is the chip hanging on a
> > > > > WAIT_FOR_EVENT?
> > > > >  				 * If so we can simply poke the
> > > > > RB_WAIT bit
> > > > >  				 * and break the hang. This should
> > > > > work on
> > > > 
> > > > I think you should also be able to accomplish the same thing
> > > > with enable_hangcheck param. I had the same problem with the
> > > > debugger :)
> > > 
> > > I agree. Iirc you have some patches floating in that area to make the
> > > hangcheck a bit more robust. Can you maybe add this to that series and
> > > (re-)submit?
> > > 
> > > Cheers, Daniel
> > 
> > While 9/10 times daniel > ben, I'm playing my 10% card here and
> > suggesting that mixing the reset variable and ring kick is not the right
> > way to go about this.
> 
> One purpose of the i915.reset parameter is to disable any automatic
> attempts to recover from a hang condition so that the error state is not
> misleading. So preventing the kick ring does help in that regard.
> 
> A second purpose is to prevent i915_reset() from causing havoc and hanging
> the machine. Daniel is implying that kicking the rings is instrumental in
> making matters worse. Again using i915.reset to prevent kicking the rings
> fits in with that purpose.
> 
> Since I regard kicking rings as a form of reset, I don't see it as a
> conflation of terms and so a valid use of i915.reset.

Couldn't have said it any better. The bad effects of kicking stuck rings
is mostly that when we have a sync problem there's a decent chance
somebody has written garbage into our batchbuffers. Continously trying to
execute said garbage is just tempting faith in the gpu's error resilience.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: kicking rings considered harmful

[Ben Widawsky]

On Tue, 27 Sep 2011 20:03:17 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Sep 27, 2011 at 06:31:59PM +0100, Chris Wilson wrote:
> > On Tue, 27 Sep 2011 09:46:14 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > > On Tue, 27 Sep 2011 12:03:22 +0200
> > > Daniel Vetter <daniel@ffwll.ch> wrote:
> > > 
> > > > On Mon, Sep 26, 2011 at 10:22:01PM -0700, Ben Widawsky wrote:
> > > > > On Mon, 26 Sep 2011 19:59:50 +0200
> > > > > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > > > > b/drivers/gpu/drm/i915/i915_irq.c index da5d607..09c11e4 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > > > @@ -1694,7 +1694,7 @@ void i915_hangcheck_elapsed(unsigned long data)
> > > > > >  		if (dev_priv->hangcheck_count++ > 1) {
> > > > > >  			DRM_ERROR("Hangcheck timer elapsed... GPU
> > > > > > hung\n"); 
> > > > > > -			if (!IS_GEN2(dev)) {
> > > > > > +			if (!IS_GEN2(dev) && i915_try_reset) {
> > > > > >  				/* Is the chip hanging on a
> > > > > > WAIT_FOR_EVENT?
> > > > > >  				 * If so we can simply poke the
> > > > > > RB_WAIT bit
> > > > > >  				 * and break the hang. This should
> > > > > > work on
> > > > > 
> > > > > I think you should also be able to accomplish the same thing
> > > > > with enable_hangcheck param. I had the same problem with the
> > > > > debugger :)
> > > > 
> > > > I agree. Iirc you have some patches floating in that area to make the
> > > > hangcheck a bit more robust. Can you maybe add this to that series and
> > > > (re-)submit?
> > > > 
> > > > Cheers, Daniel
> > > 
> > > While 9/10 times daniel > ben, I'm playing my 10% card here and
> > > suggesting that mixing the reset variable and ring kick is not the right
> > > way to go about this.
> > 
> > One purpose of the i915.reset parameter is to disable any automatic
> > attempts to recover from a hang condition so that the error state is not
> > misleading. So preventing the kick ring does help in that regard.
> > 
> > A second purpose is to prevent i915_reset() from causing havoc and hanging
> > the machine. Daniel is implying that kicking the rings is instrumental in
> > making matters worse. Again using i915.reset to prevent kicking the rings
> > fits in with that purpose.
> > 
> > Since I regard kicking rings as a form of reset, I don't see it as a
> > conflation of terms and so a valid use of i915.reset.
> 
> Couldn't have said it any better. The bad effects of kicking stuck rings
> is mostly that when we have a sync problem there's a decent chance
> somebody has written garbage into our batchbuffers. Continously trying to
> execute said garbage is just tempting faith in the gpu's error resilience.
> -Daniel

If we do this we lose the possibility to kick rings, but not reset the
GPU (not that I find that terribly useful. If we do this, it does fire a
wq event, but I don't see a problem with that for this case.

I think I would rather do this:
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 012732b..803524e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1698,6 +1698,10 @@ void i915_hangcheck_elapsed(unsigned long data)
                if (dev_priv->hangcheck_count++ > 1) {
                        DRM_ERROR("Hangcheck timer elapsed... GPU hung\n");
 
+                       /* Save off error state before kicking the rings and
+                        * possibly ruining the GPU state.
+                        */
+                       i915_handle_error(dev, true);
                        if (!IS_GEN2(dev)) {
                                /* Is the chip hanging on a WAIT_FOR_EVENT?
                                 * If so we can simply poke the RB_WAIT bit
@@ -1717,7 +1721,6 @@ void i915_hangcheck_elapsed(unsigned long data)
                                        goto repeat;
                        }
 
-                       i915_handle_error(dev, true);
                        return;
                }
        } else {

Re: [PATCH] drm/i915: kicking rings considered harmful

[Chris Wilson]

On Tue, 27 Sep 2011 12:38:59 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> If we do this we lose the possibility to kick rings, but not reset the
> GPU (not that I find that terribly useful. If we do this, it does fire a
> wq event, but I don't see a problem with that for this case.
> 
> I think I would rather do this:
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 012732b..803524e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1698,6 +1698,10 @@ void i915_hangcheck_elapsed(unsigned long data)
>                 if (dev_priv->hangcheck_count++ > 1) {
>                         DRM_ERROR("Hangcheck timer elapsed... GPU hung\n");
>  
> +                       /* Save off error state before kicking the rings and
> +                        * possibly ruining the GPU state.
> +                        */
> +                       i915_handle_error(dev, true);
>                         if (!IS_GEN2(dev)) {
>                                 /* Is the chip hanging on a WAIT_FOR_EVENT?
>                                  * If so we can simply poke the RB_WAIT bit
> @@ -1717,7 +1721,6 @@ void i915_hangcheck_elapsed(unsigned long data)
>                                         goto repeat;
>                         }
>  
> -                       i915_handle_error(dev, true);
>                         return;
>                 }
>         } else {

Interesting, if we simply call i915_capture_error_state() rather than move
the i195_handle_error() earlier we do in fact get the best of both worlds.

However, it doesn't address Daniel's statement that kick_rings() provoked
an unrecoverable hang and so we still need to disable that in order to
save the error-state. The origin of ring-kicking was to try and recover
from the modesetting/vsync issues, which apart from the outstanding issue
in intel_crtc_disable() are behind us. (I hope ;-) We shouldn't be relying
on i915_reset() and i915.reset=0 tends to be either deliberate or an act of
desparation so I don't see the issue in also preventing ring-kicking with
the same parameter. Is there an issue I'm overlooking?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: kicking rings considered harmful

[Ben Widawsky]

On Tue, 27 Sep 2011 22:54:01 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue, 27 Sep 2011 12:38:59 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > If we do this we lose the possibility to kick rings, but not reset the
> > GPU (not that I find that terribly useful. If we do this, it does fire a
> > wq event, but I don't see a problem with that for this case.
> > 
> > I think I would rather do this:
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 012732b..803524e 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1698,6 +1698,10 @@ void i915_hangcheck_elapsed(unsigned long data)
> >                 if (dev_priv->hangcheck_count++ > 1) {
> >                         DRM_ERROR("Hangcheck timer elapsed... GPU hung\n");
> >  
> > +                       /* Save off error state before kicking the rings and
> > +                        * possibly ruining the GPU state.
> > +                        */
> > +                       i915_handle_error(dev, true);
> >                         if (!IS_GEN2(dev)) {
> >                                 /* Is the chip hanging on a WAIT_FOR_EVENT?
> >                                  * If so we can simply poke the RB_WAIT bit
> > @@ -1717,7 +1721,6 @@ void i915_hangcheck_elapsed(unsigned long data)
> >                                         goto repeat;
> >                         }
> >  
> > -                       i915_handle_error(dev, true);
> >                         return;
> >                 }
> >         } else {
> 
> Interesting, if we simply call i915_capture_error_state() rather than move
> the i195_handle_error() earlier we do in fact get the best of both worlds.

We can do this except i915_handle_error() is called i915_driver_irq_handler, so
we have to modify that as well. But yeah, I'm fine with that too, though I
don't think it makes much difference either way.

> 
> However, it doesn't address Daniel's statement that kick_rings() provoked
> an unrecoverable hang and so we still need to disable that in order to
> save the error-state. The origin of ring-kicking was to try and recover
> from the modesetting/vsync issues, which apart from the outstanding issue
> in intel_crtc_disable() are behind us. (I hope ;-) We shouldn't be relying
> on i915_reset() and i915.reset=0 tends to be either deliberate or an act of
> desparation so I don't see the issue in also preventing ring-kicking with
> the same parameter. Is there an issue I'm overlooking?

No issue, I just feel that this is redundant to hangcheck_enable, so to me at
least, this just adds extra confusion to an already confusing situation. I seem
to be in the minority though.

To me it's:
reset=0, don't ever try to reset
enable_hangcheck=0, don't ever check if we're hung (ie. don't reset or kick)

And now it's
reset=0, don't every try to reset or kick
enabled_hangcheck=0, don't ever check if we're hung (ie. don't reset or kick)

I'd definitely be in favo(u)r of removing the kick_ring() if it isn't really
useful anymore. It has some forcewake race if I remember correctly which I
never bothered to fix.

> -Chris
> 

Ben

Re: [PATCH] drm/i915: kicking rings considered harmful

[Chris Wilson]

On Tue, 27 Sep 2011 18:34:31 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> I'd definitely be in favo(u)r of removing the kick_ring() if it isn't really
> useful anymore. It has some forcewake race if I remember correctly which I
> never bothered to fix.

Agreed, enough brutality, kill the kick_ring!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: kicking rings considered harmful

[Daniel Vetter]

On Wed, Sep 28, 2011 at 09:47:58AM +0100, Chris Wilson wrote:
> On Tue, 27 Sep 2011 18:34:31 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > I'd definitely be in favo(u)r of removing the kick_ring() if it isn't really
> > useful anymore. It has some forcewake race if I remember correctly which I
> > never bothered to fix.
> 
> Agreed, enough brutality, kill the kick_ring!

Actually, what about moving it into the reset-handler, kinda as a
soft-reset? If we detect that the gpu hangs again, we could then reset it
for real.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: kicking rings considered harmful

[Andrew Lutomirski]

I'm up and running again.  What's the latest patch I'm supposed to test?

i915.semaphores=1 intel_iommu=off appears to work.

--Andy

Re: [PATCH] drm/i915: kicking rings considered harmful

[Daniel Vetter]

On Mon, Oct 03, 2011 at 01:21:03PM -0700, Andrew Lutomirski wrote:
> I'm up and running again.  What's the latest patch I'm supposed to test?
> 
> i915.semaphores=1 intel_iommu=off appears to work.

Patches are in the works. Thanks a lot for sticking to this and testing
all the stuff that didn't really work out in the end.

Yours, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48