Re: [PATCH v2 11/25] drm/i915: add missing error capturing of the PIPESTAT reg

Imre Deak
2014-04-18T11:44:45Z

On Wed, 2014-04-16 at 15:17 +0300, Ville Syrjälä wrote:
> On Mon, Apr 14, 2014 at 08:24:32PM +0300, Imre Deak wrote:
> > While checking the error capture path I noticed that we lacked the
> > power domain-on check for PIPESTAT so fix this by moving that to where
> > the rest of pipe registers are captured.
> >
> > The move also revealed that we actually don't include this register in
> > the error report, so fix that too.
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 -
> > drivers/gpu/drm/i915/i915_gpu_error.c | 3 ---
> > drivers/gpu/drm/i915/intel_display.c | 3 +++
> > 3 files changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 7d6acb4..5254f4b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -325,7 +325,6 @@ struct drm_i915_error_state {
> > u32 gab_ctl;
> > u32 gfx_mode;
> > u32 extra_instdone[I915_NUM_INSTDONE_REG];
> > - u32 pipestat[I915_MAX_PIPES];
> > u64 fence[I915_MAX_NUM_FENCES];
> > struct intel_overlay_error_state *overlay;
> > struct intel_display_error_state *display;
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index ba79b59..7b5cc08 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -1028,7 +1028,6 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> > struct drm_i915_error_state *error)
> > {
> > struct drm_device *dev = dev_priv->dev;
> > - int pipe;
> >
> > /* General organization
> > * 1. Registers specific to a single generation
> > @@ -1080,8 +1079,6 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> > error->ier = I915_READ16(IER);
> > else
> > error->ier = I915_READ(IER);
> > - for_each_pipe(pipe)
> > - error->pipestat[pipe] = I915_READ(PIPESTAT(pipe));
> > }
> >
> > /* 4: Everything else */
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 1390ab5..4d8d875 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11925,6 +11925,7 @@ struct intel_display_error_state {
> > struct intel_pipe_error_state {
> > bool power_domain_on;
> > u32 source;
> > + u32 stat;
> > } pipe[I915_MAX_PIPES];
> >
> > struct intel_plane_error_state {
> > @@ -12006,6 +12007,7 @@ intel_display_capture_error_state(struct drm_device *dev)
> > }
> >
> > error->pipe[i].source = I915_READ(PIPESRC(i));
> > + error->pipe[i].stat = I915_READ(PIPESTAT(i));
>
> This needs a !HAS_PCH_SPLIT check

Ok, forgot that. Also noticed now that it should be actually gen<=5 ||
vlv, I can fix that too while at it.

>
> > }
> >
> > error->num_transcoders = INTEL_INFO(dev)->num_pipes;
> > @@ -12056,6 +12058,7 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
> > err_printf(m, " Power: %s\n",
> > error->pipe[i].power_domain_on ? "on" : "off");
> > err_printf(m, " SRC: %08x\n", error->pipe[i].source);
> > + err_printf(m, " STAT: %08x\n", error->pipe[i].stat);
> >
> > err_printf(m, "Plane [%d]:\n", i);
> > err_printf(m, " CNTR: %08x\n", error->plane[i].control);
> > --
> > 1.8.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org