[Intel-gfx] [PATCH 05/43] drm/i915: Refactor common code to load initial power context

[Tvrtko Ursulin]

On 06/03/2019 14:24, Chris Wilson wrote:
> We load a context (the kernel context) on both module load and resume in
> order to initialise some logical state onto the GPU. We can use the same
> routine for both operations, which will become more useful as we
> refactor rc6/rps enabling.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 48 ++++++++++++++++-----------------
>   1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0ae375b4e586..d4800e7d6f2c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2860,6 +2860,22 @@ static bool switch_to_kernel_context_sync(struct drm_i915_private *i915)
>   	return true;
>   }
>   
> +static bool load_power_context(struct drm_i915_private *i915)
> +{
> +	if (!switch_to_kernel_context_sync(i915))
> +		return false;
> +
> +	/*
> +	 * Immediately park the GPU so that we enable powersaving and
> +	 * treat it as idle. The next time we issue a request, we will
> +	 * unpark and start using the engine->pinned_default_state, otherwise
> +	 * it is in limbo and an early reset may fail.
> +	 */
> +	__i915_gem_park(i915);
> +
> +	return true;
> +}
> +
>   static void
>   i915_gem_idle_work_handler(struct work_struct *work)
>   {
> @@ -4444,7 +4460,7 @@ void i915_gem_resume(struct drm_i915_private *i915)
>   	intel_uc_resume(i915);
>   
>   	/* Always reload a context for powersaving. */
> -	if (i915_gem_switch_to_kernel_context(i915))
> +	if (!load_power_context(i915))
>   		goto err_wedged;
>   
>   out_unlock:
> @@ -4609,7 +4625,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
>   	struct i915_gem_context *ctx;
>   	struct intel_engine_cs *engine;
>   	enum intel_engine_id id;
> -	int err;
> +	int err = 0;
>   
>   	/*
>   	 * As we reset the gpu during very early sanitisation, the current
> @@ -4642,19 +4658,12 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
>   			goto err_active;
>   	}
>   
> -	if (!switch_to_kernel_context_sync(i915)) {
> -		err = -EIO; /* Caller will declare us wedged */
> +	/* Flush the default context image to memory, and enable powersaving. */
> +	if (!load_power_context(i915)) {
> +		err = -EIO;
>   		goto err_active;
>   	}
>   
> -	/*
> -	 * Immediately park the GPU so that we enable powersaving and
> -	 * treat it as idle. The next time we issue a request, we will
> -	 * unpark and start using the engine->pinned_default_state, otherwise
> -	 * it is in limbo and an early reset may fail.
> -	 */
> -	__i915_gem_park(i915);
> -
>   	for_each_engine(engine, i915, id) {
>   		struct i915_vma *state;
>   		void *vaddr;
> @@ -4720,19 +4729,10 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
>   err_active:
>   	/*
>   	 * If we have to abandon now, we expect the engines to be idle
> -	 * and ready to be torn-down. First try to flush any remaining
> -	 * request, ensure we are pointing at the kernel context and
> -	 * then remove it.
> +	 * and ready to be torn-down. The quickest way we can accomplish
> +	 * this is by declaring ourselves wedged.
>   	 */
> -	if (WARN_ON(i915_gem_switch_to_kernel_context(i915)))
> -		goto out_ctx;
> -
> -	if (WARN_ON(i915_gem_wait_for_idle(i915,
> -					   I915_WAIT_LOCKED,
> -					   MAX_SCHEDULE_TIMEOUT)))
> -		goto out_ctx;
> -
> -	i915_gem_contexts_lost(i915);

Because __intel_engines_record_defaults is only called on module load 
all of this doesn't matter before wedging?

> +	i915_gem_set_wedged(i915);
>   	goto out_ctx;
>   }
>   
> 

Regards,

Tvrtko