[Intel-gfx] [PATCH] drm/i915: Guard against i915_ggtt_disable_guc() being invoked unconditionally

[Michel Thierry]

On 5/31/2017 12:05 PM, Chris Wilson wrote:
> Commit 7c3f86b6dc51 ("drm/i915: Invalidate the guc ggtt TLB upon
> insertion") added the restoration of the invalidation routine after the
> GuC was disabled, but missed that the GuC was unconditionally disabled
> when not used. This then overwrites the invalidate routine for the older
> chipsets, causing havoc and breaking resume as the most obvious victim.
>
> We place the guard inside i915_ggtt_disable_guc() to be backport
> friendly (the bug was introduced into v4.11) but it would be preferred
> to be in more control over when this was guard (i.e. do not try and
> teardown the data structures before we have enabled them). That should
> be true with the reorganisation of the guc loaders.
>
> Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Fixes: 7c3f86b6dc51 ("drm/i915: Invalidate the guc ggtt TLB upon insertion")
> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Oscar Mateo <oscar.mateo at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> Cc: <stable at vger.kernel.org> # v4.11+
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 4f581adf2fcf..6eb83684b97b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3282,7 +3282,8 @@ void i915_ggtt_enable_guc(struct drm_i915_private *i915)
>
>  void i915_ggtt_disable_guc(struct drm_i915_private *i915)
>  {
> -       i915->ggtt.invalidate = gen6_ggtt_invalidate;
> +       if (i915->ggtt.invalidate == guc_ggtt_invalidate)
> +               i915->ggtt.invalidate = gen6_ggtt_invalidate;
>  }
>
>  void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)

Reviewed-by: Michel Thierry <michel.thierry at intel.com>

btw the bug can only happen in 4.11; in 4.12+ this safeguard is already 
redundant since enable_guc_loading should be zero and ggtt_disable_guc 
is never called.

-Michel