Re: [PATCH] drm/i915: Modify reset func to handle per engine resets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> writes:

> [ text/plain ]
> In full gpu reset we prime all engines and reset domains corresponding to
> each engine. Per engine reset is just a special case of this process
> wherein only a single engine is reset. This change is aimed to modify
> relevant functions to achieve this. There are some other steps we carry out
> in case of engine reset which are addressed in later patches.
>
> Reset func now accepts a mask of all engines that need to be reset. Where
> per engine resets are supported, error handler populates the mask
> accordingly otherwise all engines are specified.
>
> v2: ALL_ENGINES mask fixup, better for_each_ring_masked (Chris)
> v3: Whitespace fixes (Chris)
> v4: Rebase due to s/ring/engine
>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> Signed-off-by: Arun Siluvery <arun.siluvery@xxxxxxxxxxxxxxx>
> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>

Pushed to dinq. Thank you all.

-Mika

> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h         |  8 ++-
>  drivers/gpu/drm/i915/i915_gem.c         | 14 +++---
>  drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
>  drivers/gpu/drm/i915/i915_reg.h         |  2 +
>  drivers/gpu/drm/i915/intel_uncore.c     | 87 ++++++++++++++++++++++++---------
>  6 files changed, 82 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 20e82008b8b6..3648b73b48da 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -881,7 +881,7 @@ int i915_reset(struct drm_device *dev)
>  
>  	simulated = dev_priv->gpu_error.stop_rings != 0;
>  
> -	ret = intel_gpu_reset(dev);
> +	ret = intel_gpu_reset(dev, ALL_ENGINES);
>  
>  	/* Also reset the gpu hangman. */
>  	if (simulated) {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index fd1ed66dd298..8940b38ed57f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1971,6 +1971,10 @@ static inline struct drm_i915_private *guc_to_i915(struct intel_guc *guc)
>  	for ((i__) = 0; (i__) < I915_NUM_ENGINES; (i__)++) \
>  		for_each_if ((((ring__) = &(dev_priv__)->engine[(i__)]), intel_engine_initialized((ring__))))
>  
> +#define for_each_engine_masked(engine__, dev_priv__, mask__) \
> +	for ((engine__) = &dev_priv->engine[0]; (engine__) < &dev_priv->engine[I915_NUM_ENGINES]; (engine__)++) \
> +		for_each_if (intel_engine_flag((engine__)) & (mask__) && intel_engine_initialized((engine__)))
> +
>  enum hdmi_force_audio {
>  	HDMI_AUDIO_OFF_DVI = -2,	/* no aux data for HDMI-DVI converter */
>  	HDMI_AUDIO_OFF,			/* force turn off HDMI audio */
> @@ -2553,6 +2557,8 @@ struct drm_i915_cmd_table {
>  #define BLT_RING		(1<<BCS)
>  #define VEBOX_RING		(1<<VECS)
>  #define BSD2_RING		(1<<VCS2)
> +#define ALL_ENGINES		(~0)
> +
>  #define HAS_BSD(dev)		(INTEL_INFO(dev)->ring_mask & BSD_RING)
>  #define HAS_BSD2(dev)		(INTEL_INFO(dev)->ring_mask & BSD2_RING)
>  #define HAS_BLT(dev)		(INTEL_INFO(dev)->ring_mask & BLT_RING)
> @@ -2681,7 +2687,7 @@ extern void i915_driver_postclose(struct drm_device *dev,
>  extern long i915_compat_ioctl(struct file *filp, unsigned int cmd,
>  			      unsigned long arg);
>  #endif
> -extern int intel_gpu_reset(struct drm_device *dev);
> +extern int intel_gpu_reset(struct drm_device *dev, u32 engine_mask);
>  extern bool intel_has_gpu_reset(struct drm_device *dev);
>  extern int i915_reset(struct drm_device *dev);
>  extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 31652c1da761..a25109aa033c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5012,13 +5012,13 @@ i915_gem_cleanup_engines(struct drm_device *dev)
>  	for_each_engine(engine, dev_priv, i)
>  		dev_priv->gt.cleanup_engine(engine);
>  
> -    if (i915.enable_execlists)
> -            /*
> -             * Neither the BIOS, ourselves or any other kernel
> -             * expects the system to be in execlists mode on startup,
> -             * so we need to reset the GPU back to legacy mode.
> -             */
> -            intel_gpu_reset(dev);
> +	if (i915.enable_execlists)
> +		/*
> +		 * Neither the BIOS, ourselves or any other kernel
> +		 * expects the system to be in execlists mode on startup,
> +		 * so we need to reset the GPU back to legacy mode.
> +		 */
> +		intel_gpu_reset(dev, ALL_ENGINES);
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 1993449ab7c5..c114665a24b3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -413,7 +413,7 @@ void i915_gem_context_fini(struct drm_device *dev)
>  		/* The only known way to stop the gpu from accessing the hw context is
>  		 * to reset it. Do this as the very last operation to avoid confusing
>  		 * other code, leading to spurious errors. */
> -		intel_gpu_reset(dev);
> +		intel_gpu_reset(dev, ALL_ENGINES);
>  
>  		/* When default context is created and switched to, base object refcount
>  		 * will be 2 (+1 from object creation and +1 from do_switch()).
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7dfc4007f3fa..69359d918a4a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -164,6 +164,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define  GEN6_GRDOM_RENDER		(1 << 1)
>  #define  GEN6_GRDOM_MEDIA		(1 << 2)
>  #define  GEN6_GRDOM_BLT			(1 << 3)
> +#define  GEN6_GRDOM_VECS		(1 << 4)
> +#define  GEN8_GRDOM_MEDIA2		(1 << 7)
>  
>  #define RING_PP_DIR_BASE(ring)		_MMIO((ring)->mmio_base+0x228)
>  #define RING_PP_DIR_BASE_READ(ring)	_MMIO((ring)->mmio_base+0x518)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 02add02e0ce4..512b7faedefd 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1433,7 +1433,7 @@ static int i915_reset_complete(struct drm_device *dev)
>  	return (gdrst & GRDOM_RESET_STATUS) == 0;
>  }
>  
> -static int i915_do_reset(struct drm_device *dev)
> +static int i915_do_reset(struct drm_device *dev, unsigned engine_mask)
>  {
>  	/* assert reset for at least 20 usec */
>  	pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
> @@ -1450,13 +1450,13 @@ static int g4x_reset_complete(struct drm_device *dev)
>  	return (gdrst & GRDOM_RESET_ENABLE) == 0;
>  }
>  
> -static int g33_do_reset(struct drm_device *dev)
> +static int g33_do_reset(struct drm_device *dev, unsigned engine_mask)
>  {
>  	pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
>  	return wait_for(g4x_reset_complete(dev), 500);
>  }
>  
> -static int g4x_do_reset(struct drm_device *dev)
> +static int g4x_do_reset(struct drm_device *dev, unsigned engine_mask)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
> @@ -1486,7 +1486,7 @@ static int g4x_do_reset(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static int ironlake_do_reset(struct drm_device *dev)
> +static int ironlake_do_reset(struct drm_device *dev, unsigned engine_mask)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
> @@ -1510,21 +1510,62 @@ static int ironlake_do_reset(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static int gen6_do_reset(struct drm_device *dev)
> +/* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
> +static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
> +				u32 hw_domain_mask)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int	ret;
> -
> -	/* Reset the chip */
> +	int ret;
>  
>  	/* GEN6_GDRST is not in the gt power well, no need to check
>  	 * for fifo space for the write or forcewake the chip for
>  	 * the read
>  	 */
> -	__raw_i915_write32(dev_priv, GEN6_GDRST, GEN6_GRDOM_FULL);
> +	__raw_i915_write32(dev_priv, GEN6_GDRST, hw_domain_mask);
>  
> -	/* Spin waiting for the device to ack the reset request */
> -	ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) & GEN6_GRDOM_FULL) == 0, 500);
> +#define ACKED ((__raw_i915_read32(dev_priv, GEN6_GDRST) & hw_domain_mask) == 0)
> +	/* Spin waiting for the device to ack the reset requests */
> +	ret = wait_for(ACKED, 500);
> +#undef ACKED
> +
> +	return ret;
> +}
> +
> +/**
> + * gen6_reset_engines - reset individual engines
> + * @dev: DRM device
> + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
> + *
> + * This function will reset the individual engines that are set in engine_mask.
> + * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
> + *
> + * Note: It is responsibility of the caller to handle the difference between
> + * asking full domain reset versus reset for all available individual engines.
> + *
> + * Returns 0 on success, nonzero on error.
> + */
> +static int gen6_reset_engines(struct drm_device *dev, unsigned engine_mask)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_engine_cs *engine;
> +	const u32 hw_engine_mask[I915_NUM_ENGINES] = {
> +		[RCS] = GEN6_GRDOM_RENDER,
> +		[BCS] = GEN6_GRDOM_BLT,
> +		[VCS] = GEN6_GRDOM_MEDIA,
> +		[VCS2] = GEN8_GRDOM_MEDIA2,
> +		[VECS] = GEN6_GRDOM_VECS,
> +	};
> +	u32 hw_mask;
> +	int ret;
> +
> +	if (engine_mask == ALL_ENGINES) {
> +		hw_mask = GEN6_GRDOM_FULL;
> +	} else {
> +		hw_mask = 0;
> +		for_each_engine_masked(engine, dev_priv, engine_mask)
> +			hw_mask |= hw_engine_mask[engine->id];
> +	}
> +
> +	ret = gen6_hw_domain_reset(dev_priv, hw_mask);
>  
>  	intel_uncore_forcewake_reset(dev, true);
>  
> @@ -1567,34 +1608,34 @@ static void gen8_unrequest_engine_reset(struct intel_engine_cs *engine)
>  		      _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
>  }
>  
> -static int gen8_do_reset(struct drm_device *dev)
> +static int gen8_reset_engines(struct drm_device *dev, unsigned engine_mask)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_engine_cs *engine;
> -	int i;
>  
> -	for_each_engine(engine, dev_priv, i)
> +	for_each_engine_masked(engine, dev_priv, engine_mask)
>  		if (gen8_request_engine_reset(engine))
>  			goto not_ready;
>  
> -	return gen6_do_reset(dev);
> +	return gen6_reset_engines(dev, engine_mask);
>  
>  not_ready:
> -	for_each_engine(engine, dev_priv, i)
> +	for_each_engine_masked(engine, dev_priv, engine_mask)
>  		gen8_unrequest_engine_reset(engine);
>  
>  	return -EIO;
>  }
>  
> -static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
> +static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *,
> +							  unsigned engine_mask)
>  {
>  	if (!i915.reset)
>  		return NULL;
>  
>  	if (INTEL_INFO(dev)->gen >= 8)
> -		return gen8_do_reset;
> +		return gen8_reset_engines;
>  	else if (INTEL_INFO(dev)->gen >= 6)
> -		return gen6_do_reset;
> +		return gen6_reset_engines;
>  	else if (IS_GEN5(dev))
>  		return ironlake_do_reset;
>  	else if (IS_G4X(dev))
> @@ -1607,10 +1648,10 @@ static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
>  		return NULL;
>  }
>  
> -int intel_gpu_reset(struct drm_device *dev)
> +int intel_gpu_reset(struct drm_device *dev, unsigned engine_mask)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int (*reset)(struct drm_device *);
> +	int (*reset)(struct drm_device *, unsigned);
>  	int ret;
>  
>  	reset = intel_get_gpu_reset(dev);
> @@ -1621,7 +1662,7 @@ int intel_gpu_reset(struct drm_device *dev)
>  	 * request may be dropped and never completes (causing -EIO).
>  	 */
>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> -	ret = reset(dev);
> +	ret = reset(dev, engine_mask);
>  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>  
>  	return ret;
> -- 
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux