Re: [PATCH 1/2] drm/i915: Pull wait-for-idle into i915_gem_switch_to_kernel_context()

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> All callers do want a synchronous switch to the kernel context, that is
> by the time the call returns, the GPU has evicted all user contexts and
> now has the kernel context pinned. As all callers want this behaviour,
> refactor the common wait-for-idle into the switch.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c         |  6 ------
>  drivers/gpu/drm/i915/i915_gem_context.c |  4 +++-
>  drivers/gpu/drm/i915/i915_gem_evict.c   | 14 +-------------
>  3 files changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 890fe2802973..18ba74be286c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4564,12 +4564,6 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
>  	if (ret)
>  		goto err_unlock;
>  
> -	ret = i915_gem_wait_for_idle(dev_priv,
> -				     I915_WAIT_INTERRUPTIBLE |
> -				     I915_WAIT_LOCKED);
> -	if (ret)
> -		goto err_unlock;
> -
>  	assert_kernel_context_is_current(dev_priv);
>  	i915_gem_contexts_lost(dev_priv);
>  	mutex_unlock(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 58a2a44f88bd..f70b05e682ac 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -924,7 +924,9 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
>  			return ret;
>  	}
>  
> -	return 0;
> +	return i915_gem_wait_for_idle(dev_priv,
> +				     I915_WAIT_INTERRUPTIBLE |
> +				     I915_WAIT_LOCKED);

This wont apply due to special case EIO handling the previous
suspend hardening patches introduced. Please explain why the EIO
needs to passthrough and not return.

-Mika

>  }
>  
>  static bool client_is_banned(struct drm_i915_file_private *file_priv)
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index 4df039ef2ce3..5cf73ad4801a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -52,25 +52,13 @@ static bool ggtt_is_idle(struct drm_i915_private *dev_priv)
>  
>  static int ggtt_flush(struct drm_i915_private *i915)
>  {
> -	int err;
> -
>  	/* Not everything in the GGTT is tracked via vma (otherwise we
>  	 * could evict as required with minimal stalling) so we are forced
>  	 * to idle the GPU and explicitly retire outstanding requests in
>  	 * the hopes that we can then remove contexts and the like only
>  	 * bound by their active reference.
>  	 */
> -	err = i915_gem_switch_to_kernel_context(i915);
> -	if (err)
> -		return err;
> -
> -	err = i915_gem_wait_for_idle(i915,
> -				     I915_WAIT_INTERRUPTIBLE |
> -				     I915_WAIT_LOCKED);
> -	if (err)
> -		return err;
> -
> -	return 0;
> +	return i915_gem_switch_to_kernel_context(i915);
>  }
>  
>  static bool
> -- 
> 2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx