Intel-GFX Archive on lore.kernel.org
 help / color / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH 0/6] robustify reset transitions
Date: Wed, 14 Nov 2012 17:14:02 +0100
Message-ID: <1352909648-21514-1-git-send-email-daniel.vetter@ffwll.ch> (raw)

Hi all,

Pretty much still the same approach, but with a few changes compared to last
time around:
- split out the throttle fix
- fixed a bug in the wait_error EXIT_COND
- drop a unecessary barrier and update the comments&commit message to explain
  why that's safe

Chris Wilson expressed some concerns about the approach, pushing more for his
rwlock trick to enforce proper ordering. I still think that handling this as a
generational event which invalidates all seqno waiting makes more sense, instead
of playing tricks with locking. Imo grabbing the generation number together with
the data we're blocking on makes it much clearer that reset is an exceptional
event, and also (with the explicit passing of the reset_counter down the
callchain) obvious from where to where exactly the critical section is. Abusing
locking just doesn't quite feel right.

His second concern is about the double atomic_read now required. Atomic reads
don't insert any barriers or other stuff (they only differ in the typechecking
compared to normal loads), and with the updated patches no additional barriers
are inserted in any fastpaths. So imo no concern for performance, and imo it
yields cleaner code to separate the states of the reset machine from the
generational "invalidate everthing" counter.

YMMV, so comments&flames on the approach highly welcome. Also, if people have
ideas how to better test this ...

Cheers, Daniel

Daniel Vetter (6):
  drm/i915: move dev_priv->mm out of line
  drm/i915: extract hangcheck/reset/error_state state into substruct
  drm/i915: move wedged to the other gpu error handling stuff
  drm/i915: fix reset handling in the throttle ioctl
  drm/i915: clear up wedged transitions
  drm/i915: create a race-free reset detection

 drivers/gpu/drm/i915/i915_debugfs.c     |  12 +-
 drivers/gpu/drm/i915/i915_dma.c         |   9 +-
 drivers/gpu/drm/i915/i915_drv.c         |   8 +-
 drivers/gpu/drm/i915/i915_drv.h         | 274 ++++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_gem.c         | 104 ++++++------
 drivers/gpu/drm/i915/i915_irq.c         |  89 +++++++----
 drivers/gpu/drm/i915/intel_display.c    |   4 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c |   8 +-
 8 files changed, 291 insertions(+), 217 deletions(-)

-- 
1.7.11.4

             reply index

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-14 16:14 Daniel Vetter [this message]
2012-11-14 16:14 ` [PATCH 1/6] drm/i915: move dev_priv->mm out of line Daniel Vetter
2012-12-04 16:31   ` Damien Lespiau
2012-11-14 16:14 ` [PATCH 2/6] drm/i915: extract hangcheck/reset/error_state state into substruct Daniel Vetter
2012-12-04 17:20   ` Damien Lespiau
2012-11-14 16:14 ` [PATCH 3/6] drm/i915: move wedged to the other gpu error handling stuff Daniel Vetter
2012-12-04 17:24   ` Damien Lespiau
2012-11-14 16:14 ` [PATCH 4/6] drm/i915: fix reset handling in the throttle ioctl Daniel Vetter
2012-12-05 14:08   ` Damien Lespiau
2012-11-14 16:14 ` [PATCH 5/6] drm/i915: clear up wedged transitions Daniel Vetter
2012-11-14 16:14 ` [PATCH 6/6] drm/i915: create a race-free reset detection Daniel Vetter
2012-11-15 16:17 ` [PATCH 1/2] drm/i915: clear up wedged transitions Daniel Vetter
2012-11-15 16:17   ` [PATCH 2/2] drm/i915: create a race-free reset detection Daniel Vetter
2012-12-05 16:35     ` Damien Lespiau
2012-12-06  8:01       ` [PATCH] " Daniel Vetter
2012-12-06 15:23       ` [PATCH] drm/i915: clarify concurrent hang detect/gpu reset consistency Daniel Vetter
2013-01-18 20:48       ` [PATCH 2/2] drm/i915: create a race-free reset detection Daniel Vetter
2013-01-21 12:06         ` Damien Lespiau
2013-01-21 19:15           ` Daniel Vetter
2012-12-05 14:54   ` [PATCH 1/2] drm/i915: clear up wedged transitions Damien Lespiau
2012-12-05 16:38   ` Damien Lespiau
2012-12-05 17:14     ` Daniel Vetter
2014-09-03 20:26   ` Chris Wilson
2014-09-04  6:03     ` Daniel Vetter
2014-09-04  6:11       ` Chris Wilson

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=1352909648-21514-1-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.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

Intel-GFX Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/intel-gfx/0 intel-gfx/git/0.git
	git clone --mirror https://lore.kernel.org/intel-gfx/1 intel-gfx/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 intel-gfx intel-gfx/ https://lore.kernel.org/intel-gfx \
		intel-gfx@lists.freedesktop.org
	public-inbox-index intel-gfx

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.intel-gfx


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git