Re: [Intel-gfx] [PATCH] drm/i915: Nuke display error state

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 05 May 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I doubt anyone has used the display error state since CS flips
> went the way of the dodo. Just nuke it.

FWIW, I've never used it.

Acked-by: Jani Nikula <jani.nikula@intel.com>

>
> It might be semi interesting to have something like this for
> FIFO underruns and the like, but as it stands this wouldn't
> provide a sufficient amount of information. So would need
> an extensive rewrite anyway.
>
> The lockless power well handling is also racy, so this could
> just be contributing noise to test results if we end up
> accessing something with the relevant power well already
> disabled.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 204 -------------------
>  drivers/gpu/drm/i915/display/intel_display.h |   6 -
>  drivers/gpu/drm/i915/i915_gpu_error.c        |   6 -
>  drivers/gpu/drm/i915/i915_gpu_error.h        |   2 -
>  4 files changed, 218 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index fcd8123ede8e..2ae31c47b2a9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12988,207 +12988,3 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
>  	acpi_video_unregister();
>  	intel_opregion_unregister(i915);
>  }
> -
> -#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
> -
> -struct intel_display_error_state {
> -
> -	u32 power_well_driver;
> -
> -	struct intel_cursor_error_state {
> -		u32 control;
> -		u32 position;
> -		u32 base;
> -		u32 size;
> -	} cursor[I915_MAX_PIPES];
> -
> -	struct intel_pipe_error_state {
> -		bool power_domain_on;
> -		u32 source;
> -		u32 stat;
> -	} pipe[I915_MAX_PIPES];
> -
> -	struct intel_plane_error_state {
> -		u32 control;
> -		u32 stride;
> -		u32 size;
> -		u32 pos;
> -		u32 addr;
> -		u32 surface;
> -		u32 tile_offset;
> -	} plane[I915_MAX_PIPES];
> -
> -	struct intel_transcoder_error_state {
> -		bool available;
> -		bool power_domain_on;
> -		enum transcoder cpu_transcoder;
> -
> -		u32 conf;
> -
> -		u32 htotal;
> -		u32 hblank;
> -		u32 hsync;
> -		u32 vtotal;
> -		u32 vblank;
> -		u32 vsync;
> -	} transcoder[5];
> -};
> -
> -struct intel_display_error_state *
> -intel_display_capture_error_state(struct drm_i915_private *dev_priv)
> -{
> -	struct intel_display_error_state *error;
> -	int transcoders[] = {
> -		TRANSCODER_A,
> -		TRANSCODER_B,
> -		TRANSCODER_C,
> -		TRANSCODER_D,
> -		TRANSCODER_EDP,
> -	};
> -	int i;
> -
> -	BUILD_BUG_ON(ARRAY_SIZE(transcoders) != ARRAY_SIZE(error->transcoder));
> -
> -	if (!HAS_DISPLAY(dev_priv))
> -		return NULL;
> -
> -	error = kzalloc(sizeof(*error), GFP_ATOMIC);
> -	if (error == NULL)
> -		return NULL;
> -
> -	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -		error->power_well_driver = intel_de_read(dev_priv,
> -							 HSW_PWR_WELL_CTL2);
> -
> -	for_each_pipe(dev_priv, i) {
> -		error->pipe[i].power_domain_on =
> -			__intel_display_power_is_enabled(dev_priv,
> -							 POWER_DOMAIN_PIPE(i));
> -		if (!error->pipe[i].power_domain_on)
> -			continue;
> -
> -		error->cursor[i].control = intel_de_read(dev_priv, CURCNTR(i));
> -		error->cursor[i].position = intel_de_read(dev_priv, CURPOS(i));
> -		error->cursor[i].base = intel_de_read(dev_priv, CURBASE(i));
> -
> -		error->plane[i].control = intel_de_read(dev_priv, DSPCNTR(i));
> -		error->plane[i].stride = intel_de_read(dev_priv, DSPSTRIDE(i));
> -		if (DISPLAY_VER(dev_priv) <= 3) {
> -			error->plane[i].size = intel_de_read(dev_priv,
> -							     DSPSIZE(i));
> -			error->plane[i].pos = intel_de_read(dev_priv,
> -							    DSPPOS(i));
> -		}
> -		if (DISPLAY_VER(dev_priv) <= 7 && !IS_HASWELL(dev_priv))
> -			error->plane[i].addr = intel_de_read(dev_priv,
> -							     DSPADDR(i));
> -		if (DISPLAY_VER(dev_priv) >= 4) {
> -			error->plane[i].surface = intel_de_read(dev_priv,
> -								DSPSURF(i));
> -			error->plane[i].tile_offset = intel_de_read(dev_priv,
> -								    DSPTILEOFF(i));
> -		}
> -
> -		error->pipe[i].source = intel_de_read(dev_priv, PIPESRC(i));
> -
> -		if (HAS_GMCH(dev_priv))
> -			error->pipe[i].stat = intel_de_read(dev_priv,
> -							    PIPESTAT(i));
> -	}
> -
> -	for (i = 0; i < ARRAY_SIZE(error->transcoder); i++) {
> -		enum transcoder cpu_transcoder = transcoders[i];
> -
> -		if (!HAS_TRANSCODER(dev_priv, cpu_transcoder))
> -			continue;
> -
> -		error->transcoder[i].available = true;
> -		error->transcoder[i].power_domain_on =
> -			__intel_display_power_is_enabled(dev_priv,
> -				POWER_DOMAIN_TRANSCODER(cpu_transcoder));
> -		if (!error->transcoder[i].power_domain_on)
> -			continue;
> -
> -		error->transcoder[i].cpu_transcoder = cpu_transcoder;
> -
> -		error->transcoder[i].conf = intel_de_read(dev_priv,
> -							  PIPECONF(cpu_transcoder));
> -		error->transcoder[i].htotal = intel_de_read(dev_priv,
> -							    HTOTAL(cpu_transcoder));
> -		error->transcoder[i].hblank = intel_de_read(dev_priv,
> -							    HBLANK(cpu_transcoder));
> -		error->transcoder[i].hsync = intel_de_read(dev_priv,
> -							   HSYNC(cpu_transcoder));
> -		error->transcoder[i].vtotal = intel_de_read(dev_priv,
> -							    VTOTAL(cpu_transcoder));
> -		error->transcoder[i].vblank = intel_de_read(dev_priv,
> -							    VBLANK(cpu_transcoder));
> -		error->transcoder[i].vsync = intel_de_read(dev_priv,
> -							   VSYNC(cpu_transcoder));
> -	}
> -
> -	return error;
> -}
> -
> -#define err_printf(e, ...) i915_error_printf(e, __VA_ARGS__)
> -
> -void
> -intel_display_print_error_state(struct drm_i915_error_state_buf *m,
> -				struct intel_display_error_state *error)
> -{
> -	struct drm_i915_private *dev_priv = m->i915;
> -	int i;
> -
> -	if (!error)
> -		return;
> -
> -	err_printf(m, "Num Pipes: %d\n", INTEL_NUM_PIPES(dev_priv));
> -	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -		err_printf(m, "PWR_WELL_CTL2: %08x\n",
> -			   error->power_well_driver);
> -	for_each_pipe(dev_priv, i) {
> -		err_printf(m, "Pipe [%d]:\n", i);
> -		err_printf(m, "  Power: %s\n",
> -			   onoff(error->pipe[i].power_domain_on));
> -		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);
> -		err_printf(m, "  STRIDE: %08x\n", error->plane[i].stride);
> -		if (DISPLAY_VER(dev_priv) <= 3) {
> -			err_printf(m, "  SIZE: %08x\n", error->plane[i].size);
> -			err_printf(m, "  POS: %08x\n", error->plane[i].pos);
> -		}
> -		if (DISPLAY_VER(dev_priv) <= 7 && !IS_HASWELL(dev_priv))
> -			err_printf(m, "  ADDR: %08x\n", error->plane[i].addr);
> -		if (DISPLAY_VER(dev_priv) >= 4) {
> -			err_printf(m, "  SURF: %08x\n", error->plane[i].surface);
> -			err_printf(m, "  TILEOFF: %08x\n", error->plane[i].tile_offset);
> -		}
> -
> -		err_printf(m, "Cursor [%d]:\n", i);
> -		err_printf(m, "  CNTR: %08x\n", error->cursor[i].control);
> -		err_printf(m, "  POS: %08x\n", error->cursor[i].position);
> -		err_printf(m, "  BASE: %08x\n", error->cursor[i].base);
> -	}
> -
> -	for (i = 0; i < ARRAY_SIZE(error->transcoder); i++) {
> -		if (!error->transcoder[i].available)
> -			continue;
> -
> -		err_printf(m, "CPU transcoder: %s\n",
> -			   transcoder_name(error->transcoder[i].cpu_transcoder));
> -		err_printf(m, "  Power: %s\n",
> -			   onoff(error->transcoder[i].power_domain_on));
> -		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
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index fc0df4c63e8d..3e11cf3dfa65 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -37,7 +37,6 @@ struct drm_encoder;
>  struct drm_file;
>  struct drm_format_info;
>  struct drm_framebuffer;
> -struct drm_i915_error_state_buf;
>  struct drm_i915_gem_object;
>  struct drm_i915_private;
>  struct drm_mode_fb_cmd2;
> @@ -611,11 +610,6 @@ void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state);
>  int bdw_get_pipemisc_bpp(struct intel_crtc *crtc);
>  unsigned int intel_plane_fence_y_offset(const struct intel_plane_state *plane_state);
>  
> -struct intel_display_error_state *
> -intel_display_capture_error_state(struct drm_i915_private *dev_priv);
> -void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
> -				     struct intel_display_error_state *error);
> -
>  bool
>  intel_format_info_is_yuv_semiplanar(const struct drm_format_info *info,
>  				    u64 modifier);
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index bb181fe5d47e..99ca242ec13b 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -36,7 +36,6 @@
>  
>  #include <drm/drm_print.h>
>  
> -#include "display/intel_atomic.h"
>  #include "display/intel_csr.h"
>  #include "display/intel_overlay.h"
>  
> @@ -808,9 +807,6 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
>  	if (error->overlay)
>  		intel_overlay_print_error_state(m, error->overlay);
>  
> -	if (error->display)
> -		intel_display_print_error_state(m, error->display);
> -
>  	err_print_capabilities(m, error);
>  	err_print_params(m, &error->params);
>  }
> @@ -974,7 +970,6 @@ void __i915_gpu_coredump_free(struct kref *error_ref)
>  	}
>  
>  	kfree(error->overlay);
> -	kfree(error->display);
>  
>  	cleanup_params(error);
>  
> @@ -1826,7 +1821,6 @@ i915_gpu_coredump(struct intel_gt *gt, intel_engine_mask_t engine_mask)
>  	}
>  
>  	error->overlay = intel_overlay_capture_error_state(i915);
> -	error->display = intel_display_capture_error_state(i915);
>  
>  	return error;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
> index 16bc42de4b84..eb435f9e0220 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.h
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.h
> @@ -29,7 +29,6 @@ struct drm_i915_private;
>  struct i915_vma_compress;
>  struct intel_engine_capture_vma;
>  struct intel_overlay_error_state;
> -struct intel_display_error_state;
>  
>  struct i915_vma_coredump {
>  	struct i915_vma_coredump *next;
> @@ -182,7 +181,6 @@ struct i915_gpu_coredump {
>  	struct i915_params params;
>  
>  	struct intel_overlay_error_state *overlay;
> -	struct intel_display_error_state *display;
>  
>  	struct scatterlist *sgl, *fit;
>  };

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx