Re: [PATCH v12 05/11] drm/i915/guc: Introduce intel_uc_sanitize

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On Thu, 2017-09-28 at 12:18 +0530, Sagar Arun Kamble wrote:
> Currently GPU is reset at the end of suspend via i915_gem_sanitize.
> On resume, GuC will not be loaded until intel_uc_init_hw happens
> during GEM resume flow but action to exit sleep can be sent to GuC
> considering the FW load status. To make sure we don't invoke that
> action update GuC FW load status at the end of GPU reset as NONE.
> load_status indicates HW state and it is sanitized through this new
> function intel_uc_sanitize.
> 
> v2: Rebase.
> 
> v3: Removed intel_guc_sanitize. Marking load status as NONE at the
> GPU reset point. (Chris/Michal)
> 
> v4: Reinstated the uC function intel_uc_sanitize. (Michal Wajdeczko)
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> @@ -508,6 +508,18 @@ int intel_uc_resume(struct drm_i915_private *dev_priv)
>  	return intel_guc_resume(dev_priv);
>  }
>  
> +void intel_uc_sanitize(struct drm_i915_private *dev_priv)
> +{
> +	/*
> +	 * FIXME: intel_uc_resume currently depends on load_status to resume
> +	 * GuC. Since we are resetting Full GPU at the end of suspend, let us
> +	 * mark the load status as NONE. Once intel_uc_resume is updated to take
> +	 * into consideration GuC load state based on WOPCM, we can skip this
> +	 * state update.
> +	 */
> +	dev_priv->guc.fw.load_status = INTEL_UC_FIRMWARE_NONE;

With what I suggested to Michal, this would be call to
intel_guc_sanitize() (and in future also intel_huc_sanitize()
intel_dmc_sanitize()).

> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1763,6 +1763,9 @@ int intel_gpu_reset(struct drm_i915_private *dev_priv, unsigned engine_mask)
>  	}
>  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>  
> +	if (engine_mask == ALL_ENGINES)
> +		intel_uc_sanitize(dev_priv);

We could propagate engine_mask to intel_uc_sanitize and let it decide
what it does to keep a clear top level code flow. This also doesn't
seem to depend on if GuC submission is enabled or not.

If we want to be unconditional, wouldn't intel_guc_select_fw would not
be more appropriate in intel_uc_sanitize?

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx