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

[Michal Wajdeczko]

On Tue, 07 Nov 2017 10:21:17 +0100, Chris Wilson  
<chris at chris-wilson.co.uk> wrote:

> 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.

So if GuC submission is not i915 than maybe to avoid confusion we should
rename i915_guc_submission.c to intel_guc_submission.c ?

Michal