[Intel-gfx] [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers

[Sagar Arun Kamble]

On 9/22/2017 12:41 PM, Michal Wajdeczko wrote:
> On Fri, 22 Sep 2017 06:49:12 +0200, Sagar Arun Kamble 
> <sagar.a.kamble at intel.com> wrote:
>
>> Prepared generic helpers intel_uc_suspend, intel_uc_resume,
>> intel_uc_runtime_suspend, intel_uc_runtime_resume. Added
>> error handling to all the calls for suspend/resume.
>>
>> v2: Rebase w.r.t removal of GuC code restructuring.
>>
>> 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_drv.c | 23 ++++++++++++++++++++---
>>  drivers/gpu/drm/i915/i915_gem.c |  7 ++++++-
>>  drivers/gpu/drm/i915/intel_uc.c | 20 ++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_uc.h |  4 ++++
>>  4 files changed, 50 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index 5c111ea..8635f40 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1691,7 +1691,15 @@ static int i915_drm_resume(struct drm_device 
>> *dev)
>>      }
>>      mutex_unlock(&dev->struct_mutex);
>> -    intel_guc_resume(dev_priv);
>> +    /*
>> +     * NB: Full gem reinitialization is being done above, hence
>> +     * intel_uc_resume will be of no use. Currently intel_uc_resume
>> +     * is nop. If full reinitialization is removed, will  need to put
>> +     * functionality to resume from sleep in intel_uc_resume.
>> +     */
>> +    ret = intel_uc_resume(dev_priv);
>> +    if (ret)
>> +        DRM_ERROR("failed to resume uc\n");
>>     intel_modeset_init_hw(dev);
>> @@ -2493,7 +2501,12 @@ static int intel_runtime_suspend(struct device 
>> *kdev)
>>       */
>>      i915_gem_runtime_suspend(dev_priv);
>> -    intel_guc_suspend(dev_priv);
>> +    ret = intel_uc_runtime_suspend(dev_priv);
>> +    if (ret) {
>> +        DRM_ERROR("uc runtime suspend failed, disabling it(%d)\n", 
>> ret);
>> +        enable_rpm_wakeref_asserts(dev_priv);
>> +        return ret;
>> +    }
>>     intel_runtime_pm_disable_interrupts(dev_priv);
>> @@ -2578,7 +2591,11 @@ static int intel_runtime_resume(struct device 
>> *kdev)
>>      if (intel_uncore_unclaimed_mmio(dev_priv))
>>          DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>> -    intel_guc_resume(dev_priv);
>> +    ret = intel_uc_runtime_resume(dev_priv);
>> +    if (ret) {
>> +        DRM_ERROR("uc runtime resume failed (%d)\n", ret);
>> +        return ret;
>> +    }
>>     if (IS_GEN9_LP(dev_priv)) {
>>          bxt_disable_dc9(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c 
>> b/drivers/gpu/drm/i915/i915_gem.c
>> index c4bf348..dd56d45 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4539,7 +4539,11 @@ int i915_gem_suspend(struct drm_i915_private 
>> *dev_priv)
>>      i915_gem_contexts_lost(dev_priv);
>>      mutex_unlock(&dev->struct_mutex);
>> -    intel_guc_suspend(dev_priv);
>> +    ret = intel_uc_suspend(dev_priv);
>
> Is it ok that we suspend uc in 'gem_suspend' but resume in 'drm_resume' ?
>
> Michal
Currently it is fine as we are doing full reinit. But in future, it is 
better to have it in i915_gem_resume.
Will move it there. Thanks.
>
>> +    if (ret) {
>> +        DRM_ERROR("uc suspend failed (%d)\n", ret);
>> +        goto out;
>> +    }
>>     cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
>>      cancel_delayed_work_sync(&dev_priv->gt.retire_work);
>> @@ -4583,6 +4587,7 @@ int i915_gem_suspend(struct drm_i915_private 
>> *dev_priv)
>> err_unlock:
>>      mutex_unlock(&dev->struct_mutex);
>> +out:
>>      intel_runtime_pm_put(dev_priv);
>>      return ret;
>>  }
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 0178ba4..8e4d8b0 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -537,3 +537,23 @@ int intel_guc_sample_forcewake(struct intel_guc 
>> *guc)
>>     return intel_guc_send(guc, action, ARRAY_SIZE(action));
>>  }
>> +
>> +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
>> +{
>> +    return intel_guc_suspend(dev_priv);
>> +}
>> +
>> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
>> +{
>> +    return intel_guc_resume(dev_priv);
>> +}
>> +
>> +int intel_uc_suspend(struct drm_i915_private *dev_priv)
>> +{
>> +    return intel_guc_suspend(dev_priv);
>> +}
>> +
>> +int intel_uc_resume(struct drm_i915_private *dev_priv)
>> +{
>> +    return 0;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h 
>> b/drivers/gpu/drm/i915/intel_uc.h
>> index 7703c9a..3d33a51 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -208,6 +208,10 @@ struct intel_huc {
>>  void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
>>  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
>> +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv);
>> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
>> +int intel_uc_suspend(struct drm_i915_private *dev_priv);
>> +int intel_uc_resume(struct drm_i915_private *dev_priv);
>>  int intel_guc_sample_forcewake(struct intel_guc *guc);
>>  int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len);
>>  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, 
>> u32 len);