[Intel-gfx] [PATCH v2 1/1] drm/i915: Suspend GuC prior to GPU Reset during GEM suspend

[Daniele Ceraolo Spurio]

On 05/04/17 05:24, Chris Wilson wrote:
> On Wed, Apr 05, 2017 at 03:51:50PM +0530, Sagar Arun Kamble wrote:
>> i915 is currently doing Full GPU reset at the end of suspend followed by
>> GuC suspend. This reset bypasses the GuC. We need to tell the GuC to
>> suspend before we do a direct intel_gpu_reset, Otherwise the gpu state will
>> no longer match the GuC's expectations and its suspend will not be
>> successful. With this change, i915 suspends GuC after suspending GEM and

Here I would say here that GuC suspend fails because we're resetting GuC 
as part of the GPU reset and not because the expectation doesn't match.

>> before doing Full GPU reset.
>
> I'll massage the commit message slightly (much easier when proofreading
> someone else's writing). It makes sense to me, any takers from those who
> know guc more intimately or at least have observed and tested this patch?
> -Chris
>

Change looks good to me:
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>

On a related note, we only allocate the pages for saving the GuC state 
if i915.enable_guc_submission=1 (they're part of the ADS), but from what 
I can see we call intel_guc_suspend/resume unconditionally.
Since the ADS are not designed to be submission-only structs I think we 
should move the ADS allocation to always happen when the GuC is loaded 
without looking at i915.enable_guc_submission.

Daniele