[Intel-gfx] [PATCH 01/11] drm/i915: Export low level PM IRQ functions to control from GuC functions

[Sagar Arun Kamble]

On 10/18/2017 6:12 PM, Tvrtko Ursulin wrote:
>
> On 18/10/2017 07:46, Sagar Arun Kamble wrote:
>> In order to separate GuC IRQ handling functions from i915_irq.c we need
>> to export the low level pm irq handlers. Export pm_iir, reset_pm_iir and
>> enable/disable_pm_irq functions.
>>
>> Suggested-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> 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/i915_irq.c  | 8 ++++----
>>   drivers/gpu/drm/i915/intel_drv.h | 4 ++++
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c 
>> b/drivers/gpu/drm/i915/i915_irq.c
>> index b1296a5..caa6283 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -306,7 +306,7 @@ void gen5_disable_gt_irq(struct drm_i915_private 
>> *dev_priv, uint32_t mask)
>>       ilk_update_gt_irq(dev_priv, mask, 0);
>>   }
>>   -static i915_reg_t gen6_pm_iir(struct drm_i915_private *dev_priv)
>> +i915_reg_t gen6_pm_iir(struct drm_i915_private *dev_priv)
>>   {
>>       return INTEL_GEN(dev_priv) >= 8 ? GEN8_GT_IIR(2) : GEN6_PMIIR;
>>   }
>
> Can we pencil in for some future work to move this into the device 
> private/info/... somewhere? It would be smaller on the aggregate, and 
> more importantly more elegant that way I think.
Yes. Agree. Will add comment about this.
Thanks for review.
>
>> @@ -369,7 +369,7 @@ void gen6_mask_pm_irq(struct drm_i915_private 
>> *dev_priv, u32 mask)
>>       __gen6_mask_pm_irq(dev_priv, mask);
>>   }
>>   -static void gen6_reset_pm_iir(struct drm_i915_private *dev_priv, 
>> u32 reset_mask)
>> +void gen6_reset_pm_iir(struct drm_i915_private *dev_priv, u32 
>> reset_mask)
>>   {
>>       i915_reg_t reg = gen6_pm_iir(dev_priv);
>>   @@ -380,7 +380,7 @@ static void gen6_reset_pm_iir(struct 
>> drm_i915_private *dev_priv, u32 reset_mask)
>>       POSTING_READ(reg);
>>   }
>>   -static void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, 
>> u32 enable_mask)
>> +void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, u32 
>> enable_mask)
>>   {
>>       lockdep_assert_held(&dev_priv->irq_lock);
>>   @@ -390,7 +390,7 @@ static void gen6_enable_pm_irq(struct 
>> drm_i915_private *dev_priv, u32 enable_mas
>>       /* unmask_pm_irq provides an implicit barrier (POSTING_READ) */
>>   }
>>   -static void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, 
>> u32 disable_mask)
>> +void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 
>> disable_mask)
>>   {
>>       lockdep_assert_held(&dev_priv->irq_lock);
>
> I briefly considered, if now that these functions are exported, we 
> would benefit from expressing the lock requirement in the functions 
> name somehow, either with a double underscore prefix, or locked 
> suffix. But since they have the asserts already I think it is fine.
>
>>   diff --git a/drivers/gpu/drm/i915/intel_drv.h 
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index d61985f..792d8ea 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1237,8 +1237,12 @@ void 
>> intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>>   /* i915_irq.c */
>>   void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t 
>> mask);
>>   void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, 
>> uint32_t mask);
>> +i915_reg_t gen6_pm_iir(struct drm_i915_private *dev_priv);
>>   void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
>>   void gen6_unmask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
>> +void gen6_reset_pm_iir(struct drm_i915_private *dev_priv, u32 
>> reset_mask);
>> +void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, u32 
>> enable_mask);
>> +void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 
>> disable_mask);
>>   void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv);
>>   void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv);
>>   void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv);
>>
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Regards,
>
> Tvrtko