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 at 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 at 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
