On Thu, 08 Aug 2013, Daniel Vetter <[email protected]> wrote: > From: Chris Wilson <[email protected]> > > If we get an error event really early in the driver setup sequence, > which gen3 is especially prone to with various display GTT faults we > Oops. So try to avoid this. > > Additionally with Haswell the transcoders are a separate bank of > registers from the pipes (4 transcoders, 3 pipes). In event of an > error, we want to be sure we have a complete and accurate picture of > the machine state, so record all the transcoders in addition to all > the active pipes. > > This regression has been introduced in > > commit 702e7a56af3780d8b3a717f698209bef44187bb0 > Author: Paulo Zanoni <[email protected]> > Date: Tue Oct 23 18:29:59 2012 -0200 > > drm/i915: convert PIPECONF to use transcoder instead of pipe > > Based on the patch "drm/i915: Dump all transcoder registers on error" > from Chris Wilson: > > v2: Rebase so that we don't try to be clever and try to figure out the > cpu transcoder from hw state. That exercise should be done when we > analyze the error state offline. > > The actual bugfix is to not call intel_pipe_to_cpu_transcoder in the > error state capture code in case the pipes aren't fully set up yet. > > v3: Simplifiy the err->num_transcoders computation a bit. While at it > make the error capture stuff save on systems without a display block. > > v4: Fix fail, spotted by Jani. > > v5: Completely new commit message, cc: stable. > > Cc: Paulo Zanoni <[email protected]> > Cc: Damien Lespiau <[email protected]> > Cc: Jani Nikula <[email protected]>
s/Cc/Reviewed-by/ Cheers, Jani. > Cc: Chris Wilson <[email protected]> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60021 > Cc: [email protected] > Signed-off-by: Daniel Vetter <[email protected]> > --- > drivers/gpu/drm/i915/intel_display.c | 86 > ++++++++++++++++++++++++------------ > 1 file changed, 57 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 4127ad2..0b11405 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10380,6 +10380,8 @@ struct intel_display_error_state { > > u32 power_well_driver; > > + int num_transcoders; > + > struct intel_cursor_error_state { > u32 control; > u32 position; > @@ -10388,16 +10390,7 @@ struct intel_display_error_state { > } cursor[I915_MAX_PIPES]; > > struct intel_pipe_error_state { > - enum transcoder cpu_transcoder; > - u32 conf; > u32 source; > - > - u32 htotal; > - u32 hblank; > - u32 hsync; > - u32 vtotal; > - u32 vblank; > - u32 vsync; > } pipe[I915_MAX_PIPES]; > > struct intel_plane_error_state { > @@ -10409,6 +10402,19 @@ struct intel_display_error_state { > u32 surface; > u32 tile_offset; > } plane[I915_MAX_PIPES]; > + > + struct intel_transcoder_error_state { > + enum transcoder cpu_transcoder; > + > + u32 conf; > + > + u32 htotal; > + u32 hblank; > + u32 hsync; > + u32 vtotal; > + u32 vblank; > + u32 vsync; > + } transcoder[4]; > }; > > struct intel_display_error_state * > @@ -10416,9 +10422,17 @@ intel_display_capture_error_state(struct drm_device > *dev) > { > drm_i915_private_t *dev_priv = dev->dev_private; > struct intel_display_error_state *error; > - enum transcoder cpu_transcoder; > + int transcoders[] = { > + TRANSCODER_A, > + TRANSCODER_B, > + TRANSCODER_C, > + TRANSCODER_EDP, > + }; > int i; > > + if (INTEL_INFO(dev)->num_pipes == 0) > + return NULL; > + > error = kmalloc(sizeof(*error), GFP_ATOMIC); > if (error == NULL) > return NULL; > @@ -10427,9 +10441,6 @@ intel_display_capture_error_state(struct drm_device > *dev) > error->power_well_driver = I915_READ(HSW_PWR_WELL_DRIVER); > > for_each_pipe(i) { > - cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv, i); > - error->pipe[i].cpu_transcoder = cpu_transcoder; > - > if (INTEL_INFO(dev)->gen <= 6 || IS_VALLEYVIEW(dev)) { > error->cursor[i].control = I915_READ(CURCNTR(i)); > error->cursor[i].position = I915_READ(CURPOS(i)); > @@ -10453,14 +10464,25 @@ intel_display_capture_error_state(struct drm_device > *dev) > error->plane[i].tile_offset = I915_READ(DSPTILEOFF(i)); > } > > - error->pipe[i].conf = I915_READ(PIPECONF(cpu_transcoder)); > error->pipe[i].source = I915_READ(PIPESRC(i)); > - error->pipe[i].htotal = I915_READ(HTOTAL(cpu_transcoder)); > - error->pipe[i].hblank = I915_READ(HBLANK(cpu_transcoder)); > - error->pipe[i].hsync = I915_READ(HSYNC(cpu_transcoder)); > - error->pipe[i].vtotal = I915_READ(VTOTAL(cpu_transcoder)); > - error->pipe[i].vblank = I915_READ(VBLANK(cpu_transcoder)); > - error->pipe[i].vsync = I915_READ(VSYNC(cpu_transcoder)); > + } > + > + error->num_transcoders = INTEL_INFO(dev)->num_pipes; > + if (HAS_DDI(dev_priv->dev)) > + error->num_transcoders++; /* Account for eDP. */ > + > + for (i = 0; i < error->num_transcoders; i++) { > + enum transcoder cpu_transcoder = transcoders[i]; > + > + error->transcoder[i].cpu_transcoder = cpu_transcoder; > + > + error->transcoder[i].conf = I915_READ(PIPECONF(cpu_transcoder)); > + error->transcoder[i].htotal = I915_READ(HTOTAL(cpu_transcoder)); > + error->transcoder[i].hblank = I915_READ(HBLANK(cpu_transcoder)); > + error->transcoder[i].hsync = I915_READ(HSYNC(cpu_transcoder)); > + error->transcoder[i].vtotal = I915_READ(VTOTAL(cpu_transcoder)); > + error->transcoder[i].vblank = I915_READ(VBLANK(cpu_transcoder)); > + error->transcoder[i].vsync = I915_READ(VSYNC(cpu_transcoder)); > } > > /* In the code above we read the registers without checking if the power > @@ -10481,22 +10503,16 @@ intel_display_print_error_state(struct > drm_i915_error_state_buf *m, > { > int i; > > + if (!error) > + return; > + > err_printf(m, "Num Pipes: %d\n", INTEL_INFO(dev)->num_pipes); > if (HAS_POWER_WELL(dev)) > err_printf(m, "PWR_WELL_CTL2: %08x\n", > error->power_well_driver); > for_each_pipe(i) { > err_printf(m, "Pipe [%d]:\n", i); > - err_printf(m, " CPU transcoder: %c\n", > - transcoder_name(error->pipe[i].cpu_transcoder)); > - err_printf(m, " CONF: %08x\n", error->pipe[i].conf); > err_printf(m, " SRC: %08x\n", error->pipe[i].source); > - err_printf(m, " HTOTAL: %08x\n", error->pipe[i].htotal); > - err_printf(m, " HBLANK: %08x\n", error->pipe[i].hblank); > - err_printf(m, " HSYNC: %08x\n", error->pipe[i].hsync); > - err_printf(m, " VTOTAL: %08x\n", error->pipe[i].vtotal); > - err_printf(m, " VBLANK: %08x\n", error->pipe[i].vblank); > - err_printf(m, " VSYNC: %08x\n", error->pipe[i].vsync); > > err_printf(m, "Plane [%d]:\n", i); > err_printf(m, " CNTR: %08x\n", error->plane[i].control); > @@ -10517,4 +10533,16 @@ intel_display_print_error_state(struct > drm_i915_error_state_buf *m, > err_printf(m, " POS: %08x\n", error->cursor[i].position); > err_printf(m, " BASE: %08x\n", error->cursor[i].base); > } > + > + for (i = 0; i < error->num_transcoders; i++) { > + err_printf(m, " CPU transcoder: %c\n", > + > transcoder_name(error->transcoder[i].cpu_transcoder)); > + err_printf(m, " CONF: %08x\n", error->transcoder[i].conf); > + err_printf(m, " HTOTAL: %08x\n", error->transcoder[i].htotal); > + err_printf(m, " HBLANK: %08x\n", error->transcoder[i].hblank); > + err_printf(m, " HSYNC: %08x\n", error->transcoder[i].hsync); > + err_printf(m, " VTOTAL: %08x\n", error->transcoder[i].vtotal); > + err_printf(m, " VBLANK: %08x\n", error->transcoder[i].vblank); > + err_printf(m, " VSYNC: %08x\n", error->transcoder[i].vsync); > + } > } > -- > 1.8.3.2 > -- Jani Nikula, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
