[Intel-gfx] [PATCH v3 11/12] drm/i915/guc: Restore GuC interrupts across suspend/reset if enabled

[Sagar Arun Kamble]

On 1/4/2018 11:19 PM, Michal Wajdeczko wrote:
> On Thu, 04 Jan 2018 17:21:53 +0100, Sagar Arun Kamble 
> <sagar.a.kamble at intel.com> wrote:
>
>> In order to override the disable/enable control of GuC interrupts during
>> suspend/reset cycle we are creating two new functions suspend/restore
>> guc_interrupts which check if interrupts were enabled and disable them
>> on suspend and enable them on resume. They are used to restore 
>> interrupts
>> across reset as well.
>>
>> Further restructuring of runtime_pm_enable/disable_interrupts and
>> suspend/restore_guc_interrupts will be done in upcoming patches.
>>
>> v2: Rebase.
>>
>> v3: Updated suspend/restore with the new low level get/put functions.
>> (Tvrtko)
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c |  2 ++
>>  drivers/gpu/drm/i915/intel_guc.c     | 32 
>> ++++++++++++++++++++++++++++----
>>  drivers/gpu/drm/i915/intel_guc.h     |  2 ++
>>  3 files changed, 32 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 0cd3559..2e0db53 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3676,8 +3676,10 @@ void intel_finish_reset(struct 
>> drm_i915_private *dev_priv)
>>           * The display has been reset as well,
>>           * so need a full re-initialization.
>>           */
>> +        intel_suspend_guc_interrupts(&dev_priv->guc);
>>          intel_runtime_pm_disable_interrupts(dev_priv);
>>          intel_runtime_pm_enable_interrupts(dev_priv);
>> +        intel_restore_guc_interrupts(&dev_priv->guc);
>>         intel_pps_unlock_regs_wa(dev_priv);
>>          intel_modeset_init_hw(dev);
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c 
>> b/drivers/gpu/drm/i915/intel_guc.c
>> index d356c40..28a418a 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -406,8 +406,7 @@ int intel_guc_suspend(struct drm_i915_private 
>> *dev_priv)
>>      if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>>          return 0;
>> -    if (guc->log.level >= 0)
>> -        intel_put_guc_interrupts(guc, GUC_INTR_CLIENT_LOG);
>> +    intel_suspend_guc_interrupts(guc);
>
> Hmm, maybe we should introduce
>
>     intel_uc_suspend(struct drm_i915_private *dev_priv)
>
> which will call separately
>
>     intel_guc_suspend(guc); /* send suspend action */
>     intel_guc_suspend_interrupts(guc);
>
Yes. Ordering was not correct. I have this change as part of 
suspend/resume restructuring from GuC/GEM which will follow
after this. Will keep as it is for now in this patch.
>>     data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
>>      /* any value greater than GUC_POWER_D0 */
>> @@ -452,8 +451,7 @@ int intel_guc_resume(struct drm_i915_private 
>> *dev_priv)
>>      if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>>          return 0;
>> -    if (guc->log.level >= 0)
>> -        intel_get_guc_interrupts(guc, GUC_INTR_CLIENT_LOG);
>> +    intel_restore_guc_interrupts(guc);
>>     data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
>>      data[1] = GUC_POWER_D0;
>> @@ -548,6 +546,16 @@ void intel_get_guc_interrupts(struct intel_guc 
>> *guc, enum guc_intr_client id)
>>      spin_unlock_irq(&dev_priv->irq_lock);
>>  }
>> +void intel_restore_guc_interrupts(struct intel_guc *guc)
>> +{
>> +    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> +
>> +    spin_lock_irq(&dev_priv->irq_lock);
>> +    if (guc->interrupt_clients)
>> +        __intel_get_guc_interrupts(guc);
>> +    spin_unlock_irq(&dev_priv->irq_lock);
>> +}
>> +
>>  static void __intel_put_guc_interrupts(struct intel_guc *guc)
>>  {
>>      struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> @@ -576,6 +584,22 @@ void intel_put_guc_interrupts(struct intel_guc 
>> *guc, enum guc_intr_client id)
>>      intel_reset_guc_interrupts(guc);
>>  }
>> +void intel_suspend_guc_interrupts(struct intel_guc *guc)
>> +{
>> +    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> +
>> +    spin_lock_irq(&dev_priv->irq_lock);
>> +    if (!guc->interrupt_clients) {
>> +        spin_unlock_irq(&dev_priv->irq_lock);
>> +        return;
>> +    }
>> +    __intel_put_guc_interrupts(guc);
>> +    spin_unlock_irq(&dev_priv->irq_lock);
>> +    synchronize_irq(dev_priv->drm.irq);
>> +
>> +    intel_reset_guc_interrupts(guc);
>> +}
>> +
>>  void intel_guc_irq_handler(struct intel_guc *guc, u32 gt_iir)
>>  {
>>      struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
>> b/drivers/gpu/drm/i915/intel_guc.h
>> index af74392..2c14781 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -140,5 +140,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma 
>> *vma)
>>  void intel_get_guc_interrupts(struct intel_guc *guc, enum 
>> guc_intr_client id);
>>  void intel_put_guc_interrupts(struct intel_guc *guc, enum 
>> guc_intr_client id);
>>  void intel_guc_irq_handler(struct intel_guc *guc, u32 pm_iir);
>> +void intel_suspend_guc_interrupts(struct intel_guc *guc);
>> +void intel_restore_guc_interrupts(struct intel_guc *guc);
>
> To match obj-verb pattern:
>
> intel_guc_suspend_interrupts
> intel_guc_restore_interrupts
>
Yes. Will update.

Thanks
Sagar
>> #endif