From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 4/5] drm/i915: refactor ring error state capture to use arrays Date: Sun, 30 Oct 2011 18:47:50 -0700 Message-ID: <20111030184750.04997faf@bwidawsk.net> References: <1320001932-1846-1-git-send-email-daniel.vetter@ffwll.ch> <1320001932-1846-4-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 87EB39E8BC for ; Sun, 30 Oct 2011 18:48:03 -0700 (PDT) In-Reply-To: <1320001932-1846-4-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: intel-gfx List-Id: intel-gfx@lists.freedesktop.org On Sun, 30 Oct 2011 20:12:11 +0100 Daniel Vetter 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 > --- > 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