On 11/7/2017 3:57 PM, Michal Wajdeczko wrote:
On Tue, 07 Nov 2017 10:21:17 +0100, Chris Wilson
<chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
>> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
>> Cc: Michel Thierry <michel.thierry@xxxxxxxxx>
>> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
>> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
>> ---
>> 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 ?
Yes. Will need to update functions that are i915_guc* as well. (spotted
some in guc_log.c as well.)
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
