Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Keith Packard <keithp@keithp.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	Eugeni Dodonov <eugeni.dodonov@intel.com>,
	stable@kernel.org
Subject: Re: [PATCH] drm/i915: paper over missed irq issues with force wake vodoo
Date: Sat, 14 Jan 2012 00:52:31 +0100	[thread overview]
Message-ID: <20120113235231.GE3843@phenom.ffwll.local> (raw)
In-Reply-To: <86lipb367b.fsf@sumi.keithp.com>

On Fri, Jan 13, 2012 at 08:42:00AM -0800, Keith Packard wrote:
> On Wed,  4 Jan 2012 19:40:45 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 
> > Two things seem to do the trick on my ivb machine here:
> > - prevent the gt from powering down while waiting for seqno
> >   notification interrupts by grabbing the force_wake in get_irq (and
> >   dropping it in put_irq again).
> > - ordering writes from the ring's CS by reading a CS register, ACTHD
> >   seems to work.
> 
> I've rebased this code on top of drm-intel-fixes and pushed it to my
> forcewake-spinlock branch. I'd like to get this pushed to
> drm-intel-fixes in the next couple of days, get some QA coverage and
> then get it merged to master so that we can get some community testing
> early in the 3.3 cycle.

A few quick comments:
- I think we need to amend the commit msg of the voodoo patch with the
  piece of doc I've discovered. If you want I can send out a v3 with that.
- I think the HWSTAM revert is material for -next
- Can you post your 3 patches on intel-gfx, I think I can shoot at them a
  bit ;-)

acc101d drm/i915: Hold gt_lock across forcewake register reads

Imo this is a simple cleanup (reading forcewake-protected registers isn't
really a fast-path for us), so material for -next.

0f0e134 drm/i915: Hold gt_lock during reset

I still don't see what race you're trying to protect here, after all the
gpu just died, things are confusing anyway (and anyone accessing the gpu
in such a state should take that into account). Currently that's no one
afaics. So imo at most -next material.

176b987 drm/i915: Move reset forcewake processing to gen6_do_reset

Again this is imo just a cleanup. Furthermore the commit msg is lying a
bit because it fails to mention the fix to use the forcewake function
pointer. So the cleanup is imo for -next and the bugfix is really old,
see:


> 
> -- 
> keith.packard@intel.com



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

  reply	other threads:[~2012-01-13 23:50 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-04 16:52 [PATCH] drm/i915: paper over missed irq issues with force wake vodoo Daniel Vetter
2012-01-04 18:15 ` Eugeni Dodonov
2012-01-04 18:40   ` Daniel Vetter
2012-01-05  2:27     ` Keith Packard
2012-01-05 11:13       ` Daniel Vetter
2012-01-05 11:23         ` Eugeni Dodonov
2012-01-05 22:11     ` [PATCH] drm/i915: rip out the HWSTAM missed irq workaround Daniel Vetter
2012-01-05 23:29       ` Ben Widawsky
2012-01-06 16:03       ` Eugeni Dodonov
2012-01-09 22:00       ` Keith Packard
2012-01-09 23:39         ` Daniel Vetter
2012-01-10  2:09           ` Keith Packard
2012-01-10  7:58             ` Daniel Vetter
2012-01-18  0:24       ` Ben Widawsky
2012-01-10 12:20     ` [PATCH] drm/i915: paper over missed irq issues with force wake vodoo Daniel Vetter
2012-01-11  0:51       ` Eric Anholt
2012-01-11  4:44         ` Keith Packard
2012-01-11  6:21           ` Ben Widawsky
2012-01-11  9:59           ` Daniel Vetter
2012-01-11  5:41       ` Kenneth Graunke
2012-01-13 16:42     ` Keith Packard
2012-01-13 23:52       ` Daniel Vetter [this message]
2012-01-13 23:55         ` Daniel Vetter
2012-01-14  0:11         ` Keith Packard
2012-01-14  0:31           ` Daniel Vetter
2012-01-14  0:50             ` Keith Packard
2012-01-14 12:12               ` Daniel Vetter
2012-01-15  6:35                 ` Keith Packard
2012-01-15 15:03                   ` Daniel Vetter
2012-01-16  0:06                     ` Keith Packard
2012-01-05 23:29 ` Ben Widawsky
2012-01-08 13:01   ` Daniel Vetter
2012-01-09  5:09   ` Keith Packard
2012-01-06 20:56 ` Keith Packard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120113235231.GE3843@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=daniel.vetter@ffwll.ch \
    --cc=eugeni.dodonov@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=keithp@keithp.com \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox