* [PATCH] drm/i915: Read the hardware state for the transcoder link upon error @ 2013-06-21 14:40 Chris Wilson 2013-06-24 7:59 ` [Intel-gfx] " Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Chris Wilson @ 2013-06-21 14:40 UTC (permalink / raw) To: intel-gfx; +Cc: Chris Wilson, stable Do not trust our bookkeeping when reporting errors, and instead dump the register contents. In particular, this solves one particular issue when an error is reported before we finish setting up the outputs and have a complete mapping (i.e. during initialisation we set garbage state). If an error occurs at that early stage, it is vital that we get an accurate report of the hardware state and not conflated with our own inaccurate opinions. This fixes a panic for a large number of pre-Haswell machines that currently trigger an error during KMS takeover. Reported-by: Dustin King <daking@rescomp.stanford.edu> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60021 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: stable@vger.kernel.org --- drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5988bda..7ce4588 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10273,12 +10273,43 @@ struct intel_display_error_state { } plane[I915_MAX_PIPES]; }; +static enum transcoder +read_cpu_transcoder(struct drm_i915_private *dev_priv, int pipe) +{ + if (HAS_DDI(dev_priv->dev)) { + int edp_pipe; + + switch (I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP)) & (TRANS_DDI_FUNC_ENABLE | TRANS_DDI_EDP_INPUT_MASK)) { + default: + edp_pipe = -1; + break; + + case TRANS_DDI_EDP_INPUT_A_ONOFF | TRANS_DDI_FUNC_ENABLE: + case TRANS_DDI_EDP_INPUT_A_ON | TRANS_DDI_FUNC_ENABLE: + edp_pipe = PIPE_A; + break; + + case TRANS_DDI_EDP_INPUT_B_ONOFF | TRANS_DDI_FUNC_ENABLE: + edp_pipe = PIPE_B; + break; + + case TRANS_DDI_EDP_INPUT_C_ONOFF | TRANS_DDI_FUNC_ENABLE: + edp_pipe = PIPE_C; + break; + } + + if (edp_pipe == pipe) + pipe = TRANSCODER_EDP; + } + + return pipe; +} + struct intel_display_error_state * 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 i; error = kmalloc(sizeof(*error), GFP_ATOMIC); @@ -10289,7 +10320,9 @@ 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); + enum transcoder cpu_transcoder; + + cpu_transcoder = read_cpu_transcoder(dev_priv, i); error->pipe[i].cpu_transcoder = cpu_transcoder; if (INTEL_INFO(dev)->gen <= 6 || IS_VALLEYVIEW(dev)) { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Read the hardware state for the transcoder link upon error 2013-06-21 14:40 [PATCH] drm/i915: Read the hardware state for the transcoder link upon error Chris Wilson @ 2013-06-24 7:59 ` Daniel Vetter 2013-06-25 7:03 ` Chris Wilson 2013-06-25 7:15 ` [PATCH] drm/i915: Dump all transcoder registers on error Chris Wilson 0 siblings, 2 replies; 6+ messages in thread From: Daniel Vetter @ 2013-06-24 7:59 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, stable On Fri, Jun 21, 2013 at 03:40:04PM +0100, Chris Wilson wrote: > Do not trust our bookkeeping when reporting errors, and instead dump the > register contents. In particular, this solves one particular issue when > an error is reported before we finish setting up the outputs and have a > complete mapping (i.e. during initialisation we set garbage state). If > an error occurs at that early stage, it is vital that we get an accurate > report of the hardware state and not conflated with our own inaccurate > opinions. > > This fixes a panic for a large number of pre-Haswell machines that > currently trigger an error during KMS takeover. > > Reported-by: Dustin King <daking@rescomp.stanford.edu> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60021 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: stable@vger.kernel.org Shouldn't we just dump all transcoder registers on Haswell instead of potentially fragile dances trying to reconstruct state? -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 35 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 5988bda..7ce4588 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10273,12 +10273,43 @@ struct intel_display_error_state { > } plane[I915_MAX_PIPES]; > }; > > +static enum transcoder > +read_cpu_transcoder(struct drm_i915_private *dev_priv, int pipe) > +{ > + if (HAS_DDI(dev_priv->dev)) { > + int edp_pipe; > + > + switch (I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP)) & (TRANS_DDI_FUNC_ENABLE | TRANS_DDI_EDP_INPUT_MASK)) { > + default: > + edp_pipe = -1; > + break; > + > + case TRANS_DDI_EDP_INPUT_A_ONOFF | TRANS_DDI_FUNC_ENABLE: > + case TRANS_DDI_EDP_INPUT_A_ON | TRANS_DDI_FUNC_ENABLE: > + edp_pipe = PIPE_A; > + break; > + > + case TRANS_DDI_EDP_INPUT_B_ONOFF | TRANS_DDI_FUNC_ENABLE: > + edp_pipe = PIPE_B; > + break; > + > + case TRANS_DDI_EDP_INPUT_C_ONOFF | TRANS_DDI_FUNC_ENABLE: > + edp_pipe = PIPE_C; > + break; > + } > + > + if (edp_pipe == pipe) > + pipe = TRANSCODER_EDP; > + } > + > + return pipe; > +} > + > struct intel_display_error_state * > 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 i; > > error = kmalloc(sizeof(*error), GFP_ATOMIC); > @@ -10289,7 +10320,9 @@ 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); > + enum transcoder cpu_transcoder; > + > + cpu_transcoder = read_cpu_transcoder(dev_priv, i); > error->pipe[i].cpu_transcoder = cpu_transcoder; > > if (INTEL_INFO(dev)->gen <= 6 || IS_VALLEYVIEW(dev)) { > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Read the hardware state for the transcoder link upon error 2013-06-24 7:59 ` [Intel-gfx] " Daniel Vetter @ 2013-06-25 7:03 ` Chris Wilson 2013-06-25 7:15 ` [PATCH] drm/i915: Dump all transcoder registers on error Chris Wilson 1 sibling, 0 replies; 6+ messages in thread From: Chris Wilson @ 2013-06-25 7:03 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx, stable On Mon, Jun 24, 2013 at 09:59:33AM +0200, Daniel Vetter wrote: > On Fri, Jun 21, 2013 at 03:40:04PM +0100, Chris Wilson wrote: > > Do not trust our bookkeeping when reporting errors, and instead dump the > > register contents. In particular, this solves one particular issue when > > an error is reported before we finish setting up the outputs and have a > > complete mapping (i.e. during initialisation we set garbage state). If > > an error occurs at that early stage, it is vital that we get an accurate > > report of the hardware state and not conflated with our own inaccurate > > opinions. > > > > This fixes a panic for a large number of pre-Haswell machines that > > currently trigger an error during KMS takeover. > > > > Reported-by: Dustin King <daking@rescomp.stanford.edu> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60021 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: stable@vger.kernel.org > > Shouldn't we just dump all transcoder registers on Haswell instead of > potentially fragile dances trying to reconstruct state? You still need to reconstruct the pipe->transcoder link as they are separate banks of registers. This patch is just a smaller part of the patch to dump everything. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/i915: Dump all transcoder registers on error 2013-06-24 7:59 ` [Intel-gfx] " Daniel Vetter 2013-06-25 7:03 ` Chris Wilson @ 2013-06-25 7:15 ` Chris Wilson 2013-06-25 12:43 ` Daniel Vetter 1 sibling, 1 reply; 6+ messages in thread From: Chris Wilson @ 2013-06-25 7:15 UTC (permalink / raw) To: intel-gfx 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. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 93 +++++++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7ce4588..fbc2daed 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10242,6 +10242,9 @@ struct intel_display_error_state { u32 power_well_driver; + int num_pipes; + int num_transcoders; + struct intel_cursor_error_state { u32 control; u32 position; @@ -10251,15 +10254,7 @@ struct intel_display_error_state { 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 { @@ -10271,6 +10266,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]; }; static enum transcoder @@ -10310,6 +10318,13 @@ intel_display_capture_error_state(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; struct intel_display_error_state *error; + int transcoders[] = { + TRANSCODER_A, + TRANSCODER_B, + TRANSCODER_C, + TRANSCODER_EDP, + -1 + }; int i; error = kmalloc(sizeof(*error), GFP_ATOMIC); @@ -10319,12 +10334,15 @@ intel_display_capture_error_state(struct drm_device *dev) if (HAS_POWER_WELL(dev)) error->power_well_driver = I915_READ(HSW_PWR_WELL_DRIVER); - for_each_pipe(i) { - enum transcoder cpu_transcoder; - - cpu_transcoder = read_cpu_transcoder(dev_priv, i); - error->pipe[i].cpu_transcoder = cpu_transcoder; + if (!HAS_DDI(dev_priv->dev)) + transcoders[3] = -1; /* no EDP transcoder */ + if (dev_priv->info->num_pipes < 3) + transcoders[2] = -1; + if (dev_priv->info->num_pipes < 2) + transcoders[1] = -1; + error->num_pipes = 0; + for_each_pipe(i) { 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)); @@ -10348,14 +10366,28 @@ 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].cpu_transcoder = + read_cpu_transcoder(dev_priv, i); 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_pipes++; + } + + error->num_transcoders = 0; + for (i = 0; transcoders[i] != -1; 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)); + + error->num_transcoders++; } /* In the code above we read the registers without checking if the power @@ -10381,18 +10413,11 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m, if (HAS_POWER_WELL(dev)) err_printf(m, "PWR_WELL_CTL2: %08x\n", error->power_well_driver); - for_each_pipe(i) { + for (i = 0; i < error->num_pipes; 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); @@ -10413,5 +10438,17 @@ 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); + } } #endif -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Dump all transcoder registers on error 2013-06-25 7:15 ` [PATCH] drm/i915: Dump all transcoder registers on error Chris Wilson @ 2013-06-25 12:43 ` Daniel Vetter 2013-08-08 12:37 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2013-06-25 12:43 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Tue, Jun 25, 2013 at 08:15:22AM +0100, Chris Wilson wrote: > 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. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> I think we should squash this together with the previous patch and move the cpu_transcoder->pipe readout logic into intel_error_decode, similarly to how we already augment the various ring register state with useful context information. I just generally prefer our error state capture code to be as dumb as possible, with the least amount of trust for our hw/sw state that we can get away with. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 93 +++++++++++++++++++++++++----------- > 1 file changed, 65 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7ce4588..fbc2daed 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10242,6 +10242,9 @@ struct intel_display_error_state { > > u32 power_well_driver; > > + int num_pipes; > + int num_transcoders; > + > struct intel_cursor_error_state { > u32 control; > u32 position; > @@ -10251,15 +10254,7 @@ struct intel_display_error_state { > > 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 { > @@ -10271,6 +10266,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]; > }; > > static enum transcoder > @@ -10310,6 +10318,13 @@ intel_display_capture_error_state(struct drm_device *dev) > { > drm_i915_private_t *dev_priv = dev->dev_private; > struct intel_display_error_state *error; > + int transcoders[] = { > + TRANSCODER_A, > + TRANSCODER_B, > + TRANSCODER_C, > + TRANSCODER_EDP, > + -1 > + }; > int i; > > error = kmalloc(sizeof(*error), GFP_ATOMIC); > @@ -10319,12 +10334,15 @@ intel_display_capture_error_state(struct drm_device *dev) > if (HAS_POWER_WELL(dev)) > error->power_well_driver = I915_READ(HSW_PWR_WELL_DRIVER); > > - for_each_pipe(i) { > - enum transcoder cpu_transcoder; > - > - cpu_transcoder = read_cpu_transcoder(dev_priv, i); > - error->pipe[i].cpu_transcoder = cpu_transcoder; > + if (!HAS_DDI(dev_priv->dev)) > + transcoders[3] = -1; /* no EDP transcoder */ > + if (dev_priv->info->num_pipes < 3) > + transcoders[2] = -1; > + if (dev_priv->info->num_pipes < 2) > + transcoders[1] = -1; > > + error->num_pipes = 0; > + for_each_pipe(i) { > 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)); > @@ -10348,14 +10366,28 @@ 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].cpu_transcoder = > + read_cpu_transcoder(dev_priv, i); > 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_pipes++; > + } > + > + error->num_transcoders = 0; > + for (i = 0; transcoders[i] != -1; 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)); > + > + error->num_transcoders++; > } > > /* In the code above we read the registers without checking if the power > @@ -10381,18 +10413,11 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m, > if (HAS_POWER_WELL(dev)) > err_printf(m, "PWR_WELL_CTL2: %08x\n", > error->power_well_driver); > - for_each_pipe(i) { > + for (i = 0; i < error->num_pipes; 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); > @@ -10413,5 +10438,17 @@ 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); > + } > } > #endif > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Dump all transcoder registers on error 2013-06-25 12:43 ` Daniel Vetter @ 2013-08-08 12:37 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2013-08-08 12:37 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Tue, Jun 25, 2013 at 02:43:57PM +0200, Daniel Vetter wrote: > On Tue, Jun 25, 2013 at 08:15:22AM +0100, Chris Wilson wrote: > > 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. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > I think we should squash this together with the previous patch and move > the cpu_transcoder->pipe readout logic into intel_error_decode, similarly > to how we already augment the various ring register state with useful > context information. > > I just generally prefer our error state capture code to be as dumb as > possible, with the least amount of trust for our hw/sw state that we can > get away with. I've gone ahead and done this and pushed the frobbed patch to dinq. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-08 12:37 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-21 14:40 [PATCH] drm/i915: Read the hardware state for the transcoder link upon error Chris Wilson 2013-06-24 7:59 ` [Intel-gfx] " Daniel Vetter 2013-06-25 7:03 ` Chris Wilson 2013-06-25 7:15 ` [PATCH] drm/i915: Dump all transcoder registers on error Chris Wilson 2013-06-25 12:43 ` Daniel Vetter 2013-08-08 12:37 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).