From: Ben Widawsky <ben@bwidawsk.net>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>,
Ben Widawsky <benjamin.widawsky@intel.com>
Subject: Re: [PATCH 10/13] drm/i915/bdw: collect semaphore error state
Date: Tue, 29 Apr 2014 14:43:43 -0700 [thread overview]
Message-ID: <20140429214343.GA14824@bwidawsk.net> (raw)
In-Reply-To: <20140224130804.GQ3852@intel.com>
On Mon, Feb 24, 2014 at 03:08:04PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 19, 2014 at 10:19:23PM -0800, Ben Widawsky wrote:
> > Since the semaphore information is in an object, just dump it, and let
> > the user parse it later.
> >
> > NOTE: The page being used for the semaphores are incoherent with the
> > CPU. No matter what I do, I cannot figure out a way to read anything but
> > 0s. Note that the semaphore waits are indeed working.
> >
> > v2: Don't print signal, and wait (they should be the same). Instead,
> > print sync_seqno (Chris)
> >
> > v3: Free the semaphore error object (Chris)
> >
> > v4: Fix semaphore offset calculation during error state collection
> > (Ville)
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/i915_gpu_error.c | 52 ++++++++++++++++++++++++++++++---
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 12 ++++----
> > 3 files changed, 55 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 028ce5a..9b86874 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -322,6 +322,7 @@ struct drm_i915_error_state {
> > u64 fence[I915_MAX_NUM_FENCES];
> > struct intel_overlay_error_state *overlay;
> > struct intel_display_error_state *display;
> > + struct drm_i915_error_object *semaphore_obj;
> >
> > struct drm_i915_error_ring {
> > bool valid;
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 2b64493..25fc2ea 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -310,6 +310,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> > struct drm_device *dev = error_priv->dev;
> > drm_i915_private_t *dev_priv = dev->dev_private;
> > struct drm_i915_error_state *error = error_priv->error;
> > + struct drm_i915_error_object *obj;
> > int i, j, page, offset, elt;
> >
> > if (!error) {
> > @@ -360,8 +361,6 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> > error->pinned_bo_count[0]);
> >
> > for (i = 0; i < ARRAY_SIZE(error->ring); i++) {
> > - struct drm_i915_error_object *obj;
> > -
> > if ((obj = error->ring[i].batchbuffer)) {
> > err_printf(m, "%s --- gtt_offset = 0x%08x\n",
> > dev_priv->ring[i].name,
> > @@ -436,6 +435,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> > }
> > }
> >
> > + obj = error->semaphore_obj;
> > + if (obj) {
>
> Still won't pass under Chris's consitency radar.
>
> > + err_printf(m, "Semaphore page = 0x%08x\n", obj->gtt_offset);
> > + for (elt = 0; elt < PAGE_SIZE/16; elt += 4) {
> > + err_printf(m, "[%04x] %08x %08x %08x %08x\n",
> > + elt * 4,
> > + obj->pages[0][elt],
> > + obj->pages[0][elt+1],
> > + obj->pages[0][elt+2],
> > + obj->pages[0][elt+3]);
> > + }
> > + }
> > +
> > if (error->overlay)
> > intel_overlay_print_error_state(m, error->overlay);
> >
> > @@ -506,6 +518,7 @@ static void i915_error_state_free(struct kref *error_ref)
> > kfree(error->ring[i].requests);
> > }
> >
> > + i915_error_object_free(error->semaphore_obj);
> > kfree(error->active_bo);
> > kfree(error->overlay);
> > kfree(error->display);
> > @@ -797,6 +810,33 @@ i915_error_first_batchbuffer(struct drm_i915_private *dev_priv,
> > return NULL;
> > }
> >
> > +static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv,
> > + struct drm_i915_error_state *error,
> > + struct intel_ring_buffer *ring,
> > + struct drm_i915_error_ring *ering)
> > +{
> > + struct intel_ring_buffer *useless;
> > + int i;
> > +
> > + if (!i915_semaphore_is_enabled(dev_priv->dev))
> > + return;
> > +
> > + if (!error->semaphore_obj)
> > + error->semaphore_obj =
> > + i915_error_object_create(dev_priv,
> > + dev_priv->semaphore_obj,
> > + &dev_priv->gtt.base);
> > +
> > + for_each_ring(useless, dev_priv, i) {
> > + u16 signal_offset =
> > + (GEN8_SIGNAL_OFFSET(ring, i) / 4) & PAGE_MASK;
>
> (... & PAGE_MASK) / 4;
>
Nice catch.
> > + u32 *tmp = error->semaphore_obj->pages[0];
> > +
> > + ering->semaphore_mboxes[i] = tmp[signal_offset];
> > + ering->semaphore_seqno[i] = ring->semaphore.sync_seqno[i];
> > + }
> > +}
> > +
> > static void gen6_record_semaphore_state(struct drm_i915_private *dev_priv,
> > struct intel_ring_buffer *ring,
> > struct drm_i915_error_ring *ering)
> > @@ -814,6 +854,7 @@ static void gen6_record_semaphore_state(struct drm_i915_private *dev_priv,
> > }
> >
> > static void i915_record_ring_state(struct drm_device *dev,
> > + struct drm_i915_error_state *error,
> > struct intel_ring_buffer *ring,
> > struct drm_i915_error_ring *ering)
> > {
> > @@ -822,7 +863,10 @@ static void i915_record_ring_state(struct drm_device *dev,
> > if (INTEL_INFO(dev)->gen >= 6) {
> > ering->rc_psmi = I915_READ(ring->mmio_base + 0x50);
> > ering->fault_reg = I915_READ(RING_FAULT_REG(ring));
> > - gen6_record_semaphore_state(dev_priv, ring, ering);
> > + if (INTEL_INFO(dev)->gen >= 8)
> > + gen8_record_semaphore_state(dev_priv, error, ring, ering);
> > + else
> > + gen6_record_semaphore_state(dev_priv, ring, ering);
> > }
> >
> > if (INTEL_INFO(dev)->gen >= 4) {
> > @@ -948,7 +992,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
> >
> > error->ring[i].valid = true;
> >
> > - i915_record_ring_state(dev, ring, &error->ring[i]);
> > + i915_record_ring_state(dev, error, ring, &error->ring[i]);
> >
> > error->ring[i].batchbuffer =
> > i915_error_first_batchbuffer(dev_priv, ring);
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index 5d56c95..01aad54 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -37,9 +37,9 @@ struct intel_hw_status_page {
> > * do the writes, and that must have qw aligned offsets, simply pretend it's 8b.
> > */
> > #define i915_semaphore_seqno_size sizeof(uint64_t)
> > -#define GEN8_SIGNAL_OFFSET(to) \
> > +#define GEN8_SIGNAL_OFFSET(__ring, to) \
> > (i915_gem_obj_ggtt_offset(dev_priv->semaphore_obj) + \
> > - (ring->id * I915_NUM_RINGS * i915_semaphore_seqno_size) + \
> > + ((__ring)->id * I915_NUM_RINGS * i915_semaphore_seqno_size) + \
> > (i915_semaphore_seqno_size * (to)))
> >
> > #define GEN8_WAIT_OFFSET(__ring, from) \
> > @@ -51,10 +51,10 @@ struct intel_hw_status_page {
> > if (!dev_priv->semaphore_obj) { \
> > break; \
> > } \
> > - ring->semaphore.signal_ggtt[RCS] = GEN8_SIGNAL_OFFSET(RCS); \
> > - ring->semaphore.signal_ggtt[VCS] = GEN8_SIGNAL_OFFSET(VCS); \
> > - ring->semaphore.signal_ggtt[BCS] = GEN8_SIGNAL_OFFSET(BCS); \
> > - ring->semaphore.signal_ggtt[VECS] = GEN8_SIGNAL_OFFSET(VECS); \
> > + ring->semaphore.signal_ggtt[RCS] = GEN8_SIGNAL_OFFSET(ring, RCS); \
> > + ring->semaphore.signal_ggtt[VCS] = GEN8_SIGNAL_OFFSET(ring, VCS); \
> > + ring->semaphore.signal_ggtt[BCS] = GEN8_SIGNAL_OFFSET(ring, BCS); \
> > + ring->semaphore.signal_ggtt[VECS] = GEN8_SIGNAL_OFFSET(ring, VECS); \
> > ring->semaphore.signal_ggtt[ring->id] = MI_SEMAPHORE_SYNC_INVALID; \
> > } while(0)
> >
> > --
> > 1.9.0
>
> --
> Ville Syrjälä
> Intel OTC
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2014-04-29 21:43 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-20 6:19 [PATCH 01/13] drm/i915: Move semaphore specific ring members to struct Ben Widawsky
2014-02-20 6:19 ` [PATCH 02/13] drm/i915: Virtualize the ringbuffer signal func Ben Widawsky
2014-02-20 6:19 ` [PATCH 03/13] drm/i915: Move ring_begin to signal() Ben Widawsky
2014-02-20 6:19 ` [PATCH 04/13] drm/i915: Make semaphore updates more precise Ben Widawsky
2014-02-24 13:09 ` Ville Syrjälä
2014-02-20 6:19 ` [PATCH 05/13] drm/i915: gen specific ring init Ben Widawsky
2014-02-24 13:05 ` Ville Syrjälä
2014-02-20 6:19 ` [PATCH 06/13] drm/i915/bdw: implement semaphore signal Ben Widawsky
2014-02-24 13:10 ` Ville Syrjälä
2014-03-05 12:24 ` Ville Syrjälä
2014-02-20 6:19 ` [PATCH 07/13] drm/i915/bdw: implement semaphore wait Ben Widawsky
2014-02-20 6:19 ` [PATCH 08/13] drm/i915/bdw: poll semaphores Ben Widawsky
2014-02-24 13:06 ` Ville Syrjälä
2014-02-20 6:19 ` [PATCH 09/13] drm/i915: Extract semaphore error collection Ben Widawsky
2014-02-20 6:19 ` [PATCH 10/13] drm/i915/bdw: collect semaphore error state Ben Widawsky
2014-02-24 13:08 ` Ville Syrjälä
2014-04-29 21:43 ` Ben Widawsky [this message]
2014-02-20 6:19 ` [PATCH 11/13] drm/i915: unleash semaphores on gen8 Ben Widawsky
2014-02-20 6:19 ` [PATCH 12/13] drm/i915: semaphore debugfs Ben Widawsky
2014-02-20 6:19 ` [PATCH 13/13] DONT_MERGE drm/i915: FORCE_RESTORE for gen8 semaphores Ben Widawsky
2014-03-05 12:29 ` [PATCH 01/13] drm/i915: Move semaphore specific ring members to struct Ville Syrjälä
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=20140429214343.GA14824@bwidawsk.net \
--to=ben@bwidawsk.net \
--cc=benjamin.widawsky@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/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