[Intel-gfx] [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume

[Chris Wilson]

Quoting Sagar Arun Kamble (2017-11-07 06:05:01)
> 
> 
> On 11/6/2017 5:43 PM, Chris Wilson wrote:
> > Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
> >> Before GT device suspend, i915 should release GuC client doorbells by
> >> stopping doorbell controller snooping and doorbell deallocation through
> >> GuC. They need to be acquired again on resume. This will also resolve
> >> the driver unload issue with GuC, where doorbell deallocation was being
> >> done post GEM suspend.
> >> Release function is called from guc_suspend prior to sending sleep action
> >> during runtime and drm suspend. Acquiral is different in runtime and drm
> >> resume paths as on drm resume we are currently doing full reinit. Release
> >> should be done in synchronization with acquire hence GuC suspend/resume
> >> along with doorbell release/acquire should be under struct_mutex. Upcoming
> >> suspend and resume restructuring for GuC will update these changes.
> >>
> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> >> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >> Cc: Michał Winiarski <michal.winiarski at intel.com>
> >> Cc: Michel Thierry <michel.thierry at intel.com>
> >> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> >> Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/i915_drv.c            |  3 +++
> >>   drivers/gpu/drm/i915/i915_gem.c            |  5 +++--
> >>   drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
> >>   drivers/gpu/drm/i915/i915_guc_submission.h |  2 ++
> >>   drivers/gpu/drm/i915/intel_guc.c           |  2 ++
> >>   5 files changed, 26 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index e7e9e06..3df8a7d 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -47,6 +47,7 @@
> >>   #include <drm/i915_drm.h>
> >>   
> >>   #include "i915_drv.h"
> >> +#include "i915_guc_submission.h"
> >>   #include "i915_trace.h"
> >>   #include "i915_vgpu.h"
> >>   #include "intel_drv.h"
> >> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
> >>   
> >>          intel_guc_resume(dev_priv);
> >>   
> >> +       i915_guc_clients_acquire_doorbells(&dev_priv->guc);
> > intel_guc_acquire_doorbells();
> Prefixed "i915_guc_clients" since this modifies submission state 
> (clients/doorbells). Should have kept dev_priv as parameter.
> what should be the correct option here: intel_guc*(guc) or 
> i915_guc*(dev_priv)

GuC submission is not i915. It is not part of the user facing api.
Operate on intel_guc as you were.
-Chris