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@xxxxxxxxx>
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
