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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox