On Wed, Mar 16, 2016 at 01:52:04PM +0200, Mika Kuoppala wrote:
> 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)
>
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index b854af2c4141..68d766af2ed0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5018,7 +5018,7 @@ i915_gem_cleanup_ringbuffer(struct drm_device *dev)
> * 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);
> + intel_gpu_reset(dev, ALL_ENGINES);
Hmm, whitespace cleanup required around here?
> +/**
> + * 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_RINGS] = {
> + [RCS] = GEN6_GRDOM_RENDER,
> + [BCS] = GEN6_GRDOM_BLT,
> + [VCS] = GEN6_GRDOM_MEDIA,
> + [VCS2] = GEN8_GRDOM_MEDIA2,
> + [VECS] = GEN6_GRDOM_VECS,
> + };
> + int ret;
> + u32 hw_mask;
u32 hw_mask;
int ret;
looks neater;
>
> - /* 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);
> + if (engine_mask == ALL_ENGINES) {
> + hw_mask = GEN6_GRDOM_FULL;
> + } else {
> + hw_mask = 0;
> +
and this whitespace doesn't help since this block is entirely about the
computation of hw_mask
That's the sum total of my criticism.
Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
