[Intel-gfx] [PATCH v6 2/7] drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume

[Sagar Arun Kamble]

On 9/22/2017 1:08 PM, Michal Wajdeczko wrote:
> On Fri, 22 Sep 2017 06:49:13 +0200, Sagar Arun Kamble 
> <sagar.a.kamble at intel.com> wrote:
>
>> Apart from configuring interrupts, we need to update the ggtt invalidate
>> interface and GuC communication on suspend. This functionality can be
>> reused for other suspend and reset paths.
>>
>> v2: Rebase w.r.t removal of GuC code restructuring.
>>
>> v3: Removed GuC specific helpers as tasks other than send H2G for
>> sleep/resume are to be done from uc generic functions. (Michal)
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Michał Winiarski <michal.winiarski at intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_guc_submission.c |  5 ----
>>  drivers/gpu/drm/i915/intel_uc.c            | 39 
>> ++++++++++++++++++++++++++++--
>>  2 files changed, 37 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index e191d56..cd5ef8b 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -1219,8 +1219,6 @@ int intel_guc_suspend(struct drm_i915_private 
>> *dev_priv)
>>      if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>>          return 0;
>> -    gen9_disable_guc_interrupts(dev_priv);
>> -
>>      ctx = dev_priv->kernel_context;
>>     data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
>> @@ -1245,9 +1243,6 @@ int intel_guc_resume(struct drm_i915_private 
>> *dev_priv)
>>      if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>>          return 0;
>> -    if (i915.guc_log_level >= 0)
>> -        gen9_enable_guc_interrupts(dev_priv);
>> -
>>      ctx = dev_priv->kernel_context;
>>     data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 8e4d8b0..66d2cff 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -540,12 +540,47 @@ int intel_guc_sample_forcewake(struct intel_guc 
>> *guc)
>> int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
>>  {
>> -    return intel_guc_suspend(dev_priv);
>> +    int ret;
>> +
>> +    if (!i915.enable_guc_loading)
>> +        return 0;
>> +
>> +    ret = intel_guc_suspend(dev_priv);
>> +    if (ret) {
>> +        DRM_ERROR("GuC runtime suspend failed (%d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    i915_ggtt_disable_guc(dev_priv);
>> +    gen9_disable_guc_interrupts(dev_priv);
>> +    guc_disable_communication(&dev_priv->guc);
>> +
>> +    return 0;
>>  }
>> int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
>>  {
>> -    return intel_guc_resume(dev_priv);
>> +    int ret;
>> +
>> +    if (!i915.enable_guc_loading)
>> +        return 0;
>> +
>> +    ret = guc_enable_communication(&dev_priv->guc);
>> +    if (ret) {
>> +        DRM_ERROR("GuC enable communication failed (%d)\n", ret);
>
> Not sure if we need extra message here, as Guc CT has its own error
> messages
Ok. Will remove these.
>
>> +        return ret;
>> +    }
>> +    if (i915.guc_log_level >= 0)
>> +        gen9_enable_guc_interrupts(dev_priv);
>> +    i915_ggtt_enable_guc(dev_priv);
>> +
>> +    ret = intel_guc_resume(dev_priv);
>> +    if (ret) {
>> +        DRM_ERROR("GuC runtime resume failed (%d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>
> Maybe better would be to have single error message:
>
> out:
>     if (ret)
>         DRM_ERROR("Failed to resume uC runtime (%d)\n", ret);
>     return ret;
Will update like this. Thanks.
>
>>  }
>> int intel_uc_suspend(struct drm_i915_private *dev_priv)