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
