From: Ben Widawsky <ben@bwidawsk.net>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 4/5] drm/i915: refactor ring error state capture to use arrays
Date: Sun, 30 Oct 2011 18:50:05 -0700 [thread overview]
Message-ID: <20111030185005.5997ff4b@bwidawsk.net> (raw)
In-Reply-To: <20111030184750.04997faf@bwidawsk.net>
On Sun, 30 Oct 2011 18:47:50 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:
> On Sun, 30 Oct 2011 20:12:11 +0100
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > The code already got unwindy and we want to dump more per-ring
> > registers.
> >
> > Only functional change is that we now also capture the video
> > ring registers on ilk.
> >
> > v2: fixup a refactor fumble spotted by Chris Wilson.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 55 ++++++++++++++-------------
> > drivers/gpu/drm/i915/i915_drv.h | 20 ++-------
> > drivers/gpu/drm/i915/i915_irq.c | 70 ++++++++++++++++++-----------------
> > drivers/gpu/drm/i915/i915_reg.h | 11 +----
> > 4 files changed, 73 insertions(+), 83 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 9e6cd50..290aece 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -735,6 +735,26 @@ static void print_error_buffers(struct seq_file *m,
> > }
> > }
> >
> > +static void i915_ring_error_state(struct seq_file *m,
> > + struct drm_device *dev,
> > + struct drm_i915_error_state *error,
> > + unsigned ring)
> > +{
> > + seq_printf(m, "%s command stream:\n", ring_str(ring));
> > + seq_printf(m, " ACTHD: 0x%08x\n", error->acthd[ring]);
> > + seq_printf(m, " IPEIR: 0x%08x\n", error->ipeir[ring]);
> > + seq_printf(m, " IPEHR: 0x%08x\n", error->ipehr[ring]);
> > + seq_printf(m, " INSTDONE: 0x%08x\n", error->instdone[ring]);
> > + if (ring == RCS) {
> > + if (INTEL_INFO(dev)->gen >= 4) {
> > + seq_printf(m, " INSTDONE1: 0x%08x\n", error->instdone1);
> > + seq_printf(m, " INSTPS: 0x%08x\n", error->instps);
> > + }
> > + seq_printf(m, " INSTPM: 0x%08x\n", error->instpm);
> > + }
> > + seq_printf(m, " seqno: 0x%08x\n", error->seqno[ring]);
> > +}
> > +
> > static int i915_error_state(struct seq_file *m, void *unused)
> > {
> > struct drm_info_node *node = (struct drm_info_node *) m->private;
> > @@ -757,36 +777,19 @@ static int i915_error_state(struct seq_file *m, void *unused)
> > seq_printf(m, "PCI ID: 0x%04x\n", dev->pci_device);
> > seq_printf(m, "EIR: 0x%08x\n", error->eir);
> > seq_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
> > - if (INTEL_INFO(dev)->gen >= 6) {
> > - seq_printf(m, "ERROR: 0x%08x\n", error->error);
> > - seq_printf(m, "Blitter command stream:\n");
> > - seq_printf(m, " ACTHD: 0x%08x\n", error->bcs_acthd);
> > - seq_printf(m, " IPEIR: 0x%08x\n", error->bcs_ipeir);
> > - seq_printf(m, " IPEHR: 0x%08x\n", error->bcs_ipehr);
> > - seq_printf(m, " INSTDONE: 0x%08x\n", error->bcs_instdone);
> > - seq_printf(m, " seqno: 0x%08x\n", error->bcs_seqno);
> > - seq_printf(m, "Video (BSD) command stream:\n");
> > - seq_printf(m, " ACTHD: 0x%08x\n", error->vcs_acthd);
> > - seq_printf(m, " IPEIR: 0x%08x\n", error->vcs_ipeir);
> > - seq_printf(m, " IPEHR: 0x%08x\n", error->vcs_ipehr);
> > - seq_printf(m, " INSTDONE: 0x%08x\n", error->vcs_instdone);
> > - seq_printf(m, " seqno: 0x%08x\n", error->vcs_seqno);
> > - }
> > - seq_printf(m, "Render command stream:\n");
> > - seq_printf(m, " ACTHD: 0x%08x\n", error->acthd);
> > - seq_printf(m, " IPEIR: 0x%08x\n", error->ipeir);
> > - seq_printf(m, " IPEHR: 0x%08x\n", error->ipehr);
> > - seq_printf(m, " INSTDONE: 0x%08x\n", error->instdone);
> > - if (INTEL_INFO(dev)->gen >= 4) {
> > - seq_printf(m, " INSTDONE1: 0x%08x\n", error->instdone1);
> > - seq_printf(m, " INSTPS: 0x%08x\n", error->instps);
> > - }
> > - seq_printf(m, " INSTPM: 0x%08x\n", error->instpm);
> > - seq_printf(m, " seqno: 0x%08x\n", error->seqno);
> >
> > for (i = 0; i < dev_priv->num_fence_regs; i++)
> > seq_printf(m, " fence[%d] = %08llx\n", i, error->fence[i]);
> >
> > + if (INTEL_INFO(dev)->gen >= 6)
> > + seq_printf(m, "ERROR: 0x%08x\n", error->error);
> > +
> > + i915_ring_error_state(m, dev, error, RCS);
> > + if (HAS_BLT(dev))
> > + i915_ring_error_state(m, dev, error, BCS);
> > + if (HAS_BSD(dev))
> > + i915_ring_error_state(m, dev, error, VCS);
> > +
> > if (error->active_bo)
> > print_error_buffers(m, "Active",
> > error->active_bo,
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index d2da91f..17617c1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -151,25 +151,15 @@ struct drm_i915_error_state {
> > u32 eir;
> > u32 pgtbl_er;
> > u32 pipestat[I915_MAX_PIPES];
> > - u32 ipeir;
> > - u32 ipehr;
> > - u32 instdone;
> > - u32 acthd;
> > + u32 ipeir[I915_NUM_RINGS];
> > + u32 ipehr[I915_NUM_RINGS];
> > + u32 instdone[I915_NUM_RINGS];
> > + u32 acthd[I915_NUM_RINGS];
> > u32 error; /* gen6+ */
> > - u32 bcs_acthd; /* gen6+ blt engine */
> > - u32 bcs_ipehr;
> > - u32 bcs_ipeir;
> > - u32 bcs_instdone;
> > - u32 bcs_seqno;
> > - u32 vcs_acthd; /* gen6+ bsd engine */
> > - u32 vcs_ipehr;
> > - u32 vcs_ipeir;
> > - u32 vcs_instdone;
> > - u32 vcs_seqno;
> > u32 instpm;
> > u32 instps;
> > u32 instdone1;
> > - u32 seqno;
> > + u32 seqno[I915_NUM_RINGS];
> > u64 bbaddr;
> > u64 fence[I915_MAX_NUM_FENCES];
> > struct timeval time;
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 3cd85dd..70e67f1 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -876,6 +876,32 @@ i915_error_first_batchbuffer(struct drm_i915_private *dev_priv,
> > return NULL;
> > }
> >
> > +static void i915_record_ring_state(struct drm_device *dev,
> > + struct drm_i915_error_state *error,
> > + struct intel_ring_buffer *ring)
> > +{
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> > + if (INTEL_INFO(dev)->gen >= 4) {
> > + error->ipeir[ring->id] = I915_READ(RING_IPEIR(ring->mmio_base));
> > + error->ipehr[ring->id] = I915_READ(RING_IPEHR(ring->mmio_base));
> > + error->instdone[ring->id] = I915_READ(RING_INSTDONE(ring->mmio_base));
> > + if (ring->id == RCS) {
> > + error->instps = I915_READ(INSTPS);
> > + error->instdone1 = I915_READ(INSTDONE1);
> > + error->bbaddr = I915_READ64(BB_ADDR);
> > + }
> > + } else {
> > + error->ipeir[ring->id] = I915_READ(IPEIR);
> > + error->ipehr[ring->id] = I915_READ(IPEHR);
> > + error->instdone[ring->id] = I915_READ(INSTDONE);
> > + error->bbaddr = 0;
> > + }
> > +
> > + error->seqno[ring->id] = ring->get_seqno(ring);
> > + error->acthd[ring->id] = intel_ring_get_active_head(ring);
> > +}
> > +
> > /**
> > * i915_capture_error_state - capture an error record for later analysis
> > * @dev: drm device
> > @@ -909,47 +935,23 @@ static void i915_capture_error_state(struct drm_device *dev)
> > DRM_INFO("capturing error event; look for more information in /debug/dri/%d/i915_error_state\n",
> > dev->primary->index);
> >
> > - error->seqno = dev_priv->ring[RCS].get_seqno(&dev_priv->ring[RCS]);
> > error->eir = I915_READ(EIR);
> > error->pgtbl_er = I915_READ(PGTBL_ER);
> > for_each_pipe(pipe)
> > error->pipestat[pipe] = I915_READ(PIPESTAT(pipe));
> > error->instpm = I915_READ(INSTPM);
> > - error->error = 0;
> > - if (INTEL_INFO(dev)->gen >= 6) {
> > +
> > + if (INTEL_INFO(dev)->gen >= 6)
> > error->error = I915_READ(ERROR_GEN6);
> > + else
> > + error->error = 0;
> > +
> > + i915_record_ring_state(dev, error, &dev_priv->ring[RCS]);
> > + if (HAS_BLT(dev))
> > + i915_record_ring_state(dev, error, &dev_priv->ring[BCS]);
> > + if (HAS_BSD(dev))
> > + i915_record_ring_state(dev, error, &dev_priv->ring[VCS]);
> >
> > - error->bcs_acthd = I915_READ(BCS_ACTHD);
> > - error->bcs_ipehr = I915_READ(BCS_IPEHR);
> > - error->bcs_ipeir = I915_READ(BCS_IPEIR);
> > - error->bcs_instdone = I915_READ(BCS_INSTDONE);
> > - error->bcs_seqno = 0;
> > - if (dev_priv->ring[BCS].get_seqno)
> > - error->bcs_seqno = dev_priv->ring[BCS].get_seqno(&dev_priv->ring[BCS]);
> > -
> > - error->vcs_acthd = I915_READ(VCS_ACTHD);
> > - error->vcs_ipehr = I915_READ(VCS_IPEHR);
> > - error->vcs_ipeir = I915_READ(VCS_IPEIR);
> > - error->vcs_instdone = I915_READ(VCS_INSTDONE);
> > - error->vcs_seqno = 0;
> > - if (dev_priv->ring[VCS].get_seqno)
> > - error->vcs_seqno = dev_priv->ring[VCS].get_seqno(&dev_priv->ring[VCS]);
> > - }
> > - if (INTEL_INFO(dev)->gen >= 4) {
> > - error->ipeir = I915_READ(IPEIR_I965);
> > - error->ipehr = I915_READ(IPEHR_I965);
> > - error->instdone = I915_READ(INSTDONE_I965);
> > - error->instps = I915_READ(INSTPS);
> > - error->instdone1 = I915_READ(INSTDONE1);
> > - error->acthd = I915_READ(ACTHD_I965);
> > - error->bbaddr = I915_READ64(BB_ADDR);
> > - } else {
> > - error->ipeir = I915_READ(IPEIR);
> > - error->ipehr = I915_READ(IPEHR);
> > - error->instdone = I915_READ(INSTDONE);
> > - error->acthd = I915_READ(ACTHD);
> > - error->bbaddr = 0;
> > - }
> > i915_gem_record_fences(dev, error);
> >
> > /* Record the active batch and ring buffers */
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 5a09416..c292957 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -352,6 +352,9 @@
> > #define IPEIR_I965 0x02064
> > #define IPEHR_I965 0x02068
> > #define INSTDONE_I965 0x0206c
> > +#define RING_IPEIR(base) ((base)+0x64)
> > +#define RING_IPEHR(base) ((base)+0x68)
> > +#define RING_INSTDONE(base) ((base)+0x6c)
> > #define INSTPS 0x02070 /* 965+ only */
> > #define INSTDONE1 0x0207c /* 965+ only */
> > #define ACTHD_I965 0x02074
> > @@ -365,14 +368,6 @@
> > #define INSTDONE 0x02090
> > #define NOPID 0x02094
> > #define HWSTAM 0x02098
> > -#define VCS_INSTDONE 0x1206C
> > -#define VCS_IPEIR 0x12064
> > -#define VCS_IPEHR 0x12068
> > -#define VCS_ACTHD 0x12074
> > -#define BCS_INSTDONE 0x2206C
> > -#define BCS_IPEIR 0x22064
> > -#define BCS_IPEHR 0x22068
> > -#define BCS_ACTHD 0x22074
> >
> > #define ERROR_GEN6 0x040a0
> >
>
> This patch looks a lot like an earlier patch of mine except your ring ID
> changes made it quite nice. I wonder why you didn't keep my INSTPS, and INSTPM
> per ring?
>
> Ben
Ah, just saw patch 5... I guess I find this a little weird way to break
it up, but I think I did a much worse job in my patches.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
next prev parent reply other threads:[~2011-10-31 1:50 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-30 19:12 [PATCH 1/5] drm/i915: kicking rings stuck on semaphores considered harmful Daniel Vetter
2011-10-30 19:12 ` [PATCH 2/5] drm/i915: don't bail out of intel_wait_ring_buffer too early Daniel Vetter
2011-10-31 1:29 ` Ben Widawsky
2011-10-31 7:37 ` Daniel Vetter
2011-11-01 15:31 ` Eugeni Dodonov
2011-10-30 19:12 ` [PATCH 3/5] drm/i915: switch ring->id to be a real id Daniel Vetter
2011-10-31 1:48 ` Ben Widawsky
2011-11-01 15:29 ` Eugeni Dodonov
2011-10-30 19:12 ` [PATCH 4/5] drm/i915: refactor ring error state capture to use arrays Daniel Vetter
2011-10-30 19:32 ` Chris Wilson
2011-10-31 1:47 ` Ben Widawsky
2011-10-31 1:50 ` Ben Widawsky [this message]
2011-10-31 7:53 ` Daniel Vetter
2011-10-30 19:12 ` [PATCH 5/5] drm/i915: collect more per ring error state Daniel Vetter
2011-10-31 1:51 ` Ben Widawsky
2011-10-31 1:25 ` [PATCH 1/5] drm/i915: kicking rings stuck on semaphores considered harmful Ben Widawsky
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=20111030185005.5997ff4b@bwidawsk.net \
--to=ben@bwidawsk.net \
--cc=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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.