[Intel-gfx] [PATCH 02/11] drm/i915/guc: Move GuC interrupts related functions from i915_irq.c to intel_guc.c
Sagar Arun Kamble
sagar.a.kamble at intel.com
Wed Oct 18 14:06:09 UTC 2017
On 10/18/2017 6:16 PM, Tvrtko Ursulin wrote:
>
> On 18/10/2017 07:46, Sagar Arun Kamble wrote:
>> GuC interrupts handling is core GuC functionality. Better to keep it
>> with other core functions in intel_guc.c. Since they are used from
>> uC functions, GuC logging, i915 irq handling keeping them grouped in
>> intel_guc.c instead of intel_uc.c.
>>
>> 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 | 70
>> +----------------------------------
>> drivers/gpu/drm/i915/intel_drv.h | 3 --
>> drivers/gpu/drm/i915/intel_guc.c | 71
>> +++++++++++++++++++++++++++++++++++-
>> drivers/gpu/drm/i915/intel_guc.h | 4 ++
>> drivers/gpu/drm/i915/intel_guc_log.c | 6 +--
>> drivers/gpu/drm/i915/intel_uc.c | 8 ++--
>> 6 files changed, 81 insertions(+), 81 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c
>> b/drivers/gpu/drm/i915/i915_irq.c
>> index caa6283..84a4bf3 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -203,7 +203,6 @@ static void gen2_assert_iir_is_zero(struct
>> drm_i915_private *dev_priv,
>> } while (0)
>> static void gen6_rps_irq_handler(struct drm_i915_private
>> *dev_priv, u32 pm_iir);
>> -static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv,
>> u32 pm_iir);
>> /* For display hotplug interrupt */
>> static inline void
>> @@ -450,38 +449,6 @@ void gen6_disable_rps_interrupts(struct
>> drm_i915_private *dev_priv)
>> gen6_reset_rps_interrupts(dev_priv);
>> }
>> -void gen9_reset_guc_interrupts(struct drm_i915_private *dev_priv)
>> -{
>> - spin_lock_irq(&dev_priv->irq_lock);
>> - gen6_reset_pm_iir(dev_priv, dev_priv->pm_guc_events);
>> - spin_unlock_irq(&dev_priv->irq_lock);
>> -}
>> -
>> -void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv)
>> -{
>> - spin_lock_irq(&dev_priv->irq_lock);
>> - if (!dev_priv->guc.interrupts_enabled) {
>> - WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) &
>> - dev_priv->pm_guc_events);
>> - dev_priv->guc.interrupts_enabled = true;
>> - gen6_enable_pm_irq(dev_priv, dev_priv->pm_guc_events);
>> - }
>> - spin_unlock_irq(&dev_priv->irq_lock);
>> -}
>> -
>> -void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv)
>> -{
>> - spin_lock_irq(&dev_priv->irq_lock);
>> - dev_priv->guc.interrupts_enabled = false;
>> -
>> - gen6_disable_pm_irq(dev_priv, dev_priv->pm_guc_events);
>> -
>> - spin_unlock_irq(&dev_priv->irq_lock);
>> - synchronize_irq(dev_priv->drm.irq);
>> -
>> - gen9_reset_guc_interrupts(dev_priv);
>> -}
>> -
>> /**
>> * bdw_update_port_irq - update DE port interrupt
>> * @dev_priv: driver private
>> @@ -1474,7 +1441,7 @@ static void gen8_gt_irq_handler(struct
>> drm_i915_private *dev_priv,
>> gen6_rps_irq_handler(dev_priv, gt_iir[2]);
>> if (gt_iir[2] & dev_priv->pm_guc_events)
>> - gen9_guc_irq_handler(dev_priv, gt_iir[2]);
>> + intel_guc_irq_handler(dev_priv, gt_iir[2]);
>
> Slight downside that it cannot be inlined any longer, should the
> compiler decide to do so. But it is probably not a relevant
> consideration for the frequency of these.
Ok. If needed we can force by making these static inline from header.
Thanks for review.
>
>> }
>> static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
>> @@ -1751,41 +1718,6 @@ static void gen6_rps_irq_handler(struct
>> drm_i915_private *dev_priv, u32 pm_iir)
>> }
>> }
>> -static void gen9_guc_irq_handler(struct drm_i915_private
>> *dev_priv, u32 gt_iir)
>> -{
>> - if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) {
>> - /* Sample the log buffer flush related bits & clear them out
>> now
>> - * itself from the message identity register to minimize the
>> - * probability of losing a flush interrupt, when there are back
>> - * to back flush interrupts.
>> - * There can be a new flush interrupt, for different log buffer
>> - * type (like for ISR), whilst Host is handling one (for DPC).
>> - * Since same bit is used in message register for ISR & DPC, it
>> - * could happen that GuC sets the bit for 2nd interrupt but
>> Host
>> - * clears out the bit on handling the 1st interrupt.
>> - */
>> - u32 msg, flush;
>> -
>> - msg = I915_READ(SOFT_SCRATCH(15));
>> - flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
>> - INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
>> - if (flush) {
>> - /* Clear the message bits that are handled */
>> - I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
>> -
>> - /* Handle flush interrupt in bottom half */
>> - queue_work(dev_priv->guc.log.runtime.flush_wq,
>> - &dev_priv->guc.log.runtime.flush_work);
>> -
>> - dev_priv->guc.log.flush_interrupt_count++;
>> - } else {
>> - /* Not clearing of unhandled event bits won't result in
>> - * re-triggering of the interrupt.
>> - */
>> - }
>> - }
>> -}
>> -
>> static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
>> {
>> enum pipe pipe;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 792d8ea..39c76ba 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1269,9 +1269,6 @@ void gen8_irq_power_well_post_enable(struct
>> drm_i915_private *dev_priv,
>> u8 pipe_mask);
>> void gen8_irq_power_well_pre_disable(struct drm_i915_private
>> *dev_priv,
>> u8 pipe_mask);
>> -void gen9_reset_guc_interrupts(struct drm_i915_private *dev_priv);
>> -void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv);
>> -void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
>> /* intel_crt.c */
>> void intel_crt_init(struct drm_i915_private *dev_priv);
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c
>> b/drivers/gpu/drm/i915/intel_guc.c
>> index 10037c0..3a64ae1 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -274,7 +274,7 @@ 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);
>> + intel_disable_guc_interrupts(dev_priv);
>> ctx = dev_priv->kernel_context;
>> @@ -302,7 +302,7 @@ int intel_guc_resume(struct drm_i915_private
>> *dev_priv)
>> return 0;
>> if (i915_modparams.guc_log_level >= 0)
>> - gen9_enable_guc_interrupts(dev_priv);
>> + intel_enable_guc_interrupts(dev_priv);
>> ctx = dev_priv->kernel_context;
>> @@ -367,3 +367,70 @@ u32 intel_guc_wopcm_size(struct
>> drm_i915_private *dev_priv)
>> return wopcm_size;
>> }
>> +
>> +void intel_reset_guc_interrupts(struct drm_i915_private *dev_priv)
>> +{
>> + spin_lock_irq(&dev_priv->irq_lock);
>> + gen6_reset_pm_iir(dev_priv, dev_priv->pm_guc_events);
>> + spin_unlock_irq(&dev_priv->irq_lock);
>> +}
>> +
>> +void intel_enable_guc_interrupts(struct drm_i915_private *dev_priv)
>> +{
>> + spin_lock_irq(&dev_priv->irq_lock);
>> + if (!dev_priv->guc.interrupts_enabled) {
>> + WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) &
>> + dev_priv->pm_guc_events);
>> + dev_priv->guc.interrupts_enabled = true;
>> + gen6_enable_pm_irq(dev_priv, dev_priv->pm_guc_events);
>> + }
>> + spin_unlock_irq(&dev_priv->irq_lock);
>> +}
>> +
>> +void intel_disable_guc_interrupts(struct drm_i915_private *dev_priv)
>> +{
>> + spin_lock_irq(&dev_priv->irq_lock);
>> + dev_priv->guc.interrupts_enabled = false;
>> +
>> + gen6_disable_pm_irq(dev_priv, dev_priv->pm_guc_events);
>> +
>> + spin_unlock_irq(&dev_priv->irq_lock);
>> + synchronize_irq(dev_priv->drm.irq);
>> +
>> + intel_reset_guc_interrupts(dev_priv);
>> +}
>> +
>> +void intel_guc_irq_handler(struct drm_i915_private *dev_priv, u32
>> gt_iir)
>> +{
>> + if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) {
>> + /* Sample the log buffer flush related bits & clear them out
>> now
>> + * itself from the message identity register to minimize the
>> + * probability of losing a flush interrupt, when there are back
>> + * to back flush interrupts.
>> + * There can be a new flush interrupt, for different log buffer
>> + * type (like for ISR), whilst Host is handling one (for DPC).
>> + * Since same bit is used in message register for ISR & DPC, it
>> + * could happen that GuC sets the bit for 2nd interrupt but
>> Host
>> + * clears out the bit on handling the 1st interrupt.
>> + */
>> + u32 msg, flush;
>> +
>> + msg = I915_READ(SOFT_SCRATCH(15));
>> + flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
>> + INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
>> + if (flush) {
>> + /* Clear the message bits that are handled */
>> + I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
>> +
>> + /* Handle flush interrupt in bottom half */
>> + queue_work(dev_priv->guc.log.runtime.flush_wq,
>> + &dev_priv->guc.log.runtime.flush_work);
>> +
>> + dev_priv->guc.log.flush_interrupt_count++;
>> + } else {
>> + /* Not clearing of unhandled event bits won't result in
>> + * re-triggering of the interrupt.
>> + */
>> + }
>> + }
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h
>> b/drivers/gpu/drm/i915/intel_guc.h
>> index 418450b..8b26505 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -116,5 +116,9 @@ static inline u32 guc_ggtt_offset(struct i915_vma
>> *vma)
>> int intel_guc_resume(struct drm_i915_private *dev_priv);
>> struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32
>> size);
>> u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>> +void intel_reset_guc_interrupts(struct drm_i915_private *dev_priv);
>> +void intel_enable_guc_interrupts(struct drm_i915_private *dev_priv);
>> +void intel_disable_guc_interrupts(struct drm_i915_private *dev_priv);
>> +void intel_guc_irq_handler(struct drm_i915_private *dev_priv, u32
>> pm_iir);
>> #endif
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c
>> b/drivers/gpu/drm/i915/intel_guc_log.c
>> index 76d3eb1..8120208 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -510,7 +510,7 @@ static void guc_flush_logs(struct intel_guc *guc)
>> return;
>> /* First disable the interrupts, will be renabled afterwards */
>> - gen9_disable_guc_interrupts(dev_priv);
>> + intel_disable_guc_interrupts(dev_priv);
>> /* Before initiating the forceful flush, wait for any
>> pending/ongoing
>> * flush to complete otherwise forceful flush may not actually
>> happen.
>> @@ -628,7 +628,7 @@ int i915_guc_log_control(struct drm_i915_private
>> *dev_priv, u64 control_val)
>> }
>> /* GuC logging is currently the only user of Guc2Host
>> interrupts */
>> - gen9_enable_guc_interrupts(dev_priv);
>> + intel_enable_guc_interrupts(dev_priv);
>> } else {
>> /* Once logging is disabled, GuC won't generate logs & send an
>> * interrupt. But there could be some data in the log buffer
>> @@ -662,7 +662,7 @@ void i915_guc_log_unregister(struct
>> drm_i915_private *dev_priv)
>> mutex_lock(&dev_priv->drm.struct_mutex);
>> /* GuC logging is currently the only user of Guc2Host
>> interrupts */
>> - gen9_disable_guc_interrupts(dev_priv);
>> + intel_disable_guc_interrupts(dev_priv);
>> guc_log_runtime_destroy(&dev_priv->guc);
>> mutex_unlock(&dev_priv->drm.struct_mutex);
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 25bd162..b9b9947a0 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -158,7 +158,7 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> return 0;
>> guc_disable_communication(guc);
>> - gen9_reset_guc_interrupts(dev_priv);
>> + intel_reset_guc_interrupts(dev_priv);
>> /* We need to notify the guc whenever we change the GGTT */
>> i915_ggtt_enable_guc(dev_priv);
>> @@ -215,7 +215,7 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> intel_huc_auth(&dev_priv->huc);
>> if (i915_modparams.enable_guc_submission) {
>> if (i915_modparams.guc_log_level >= 0)
>> - gen9_enable_guc_interrupts(dev_priv);
>> + intel_enable_guc_interrupts(dev_priv);
>> ret = i915_guc_submission_enable(dev_priv);
>> if (ret)
>> @@ -241,7 +241,7 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> */
>> err_interrupts:
>> guc_disable_communication(guc);
>> - gen9_disable_guc_interrupts(dev_priv);
>> + intel_disable_guc_interrupts(dev_priv);
>> err_log_capture:
>> guc_capture_load_err_log(guc);
>> err_submission:
>> @@ -282,7 +282,7 @@ void intel_uc_fini_hw(struct drm_i915_private
>> *dev_priv)
>> guc_disable_communication(&dev_priv->guc);
>> if (i915_modparams.enable_guc_submission) {
>> - gen9_disable_guc_interrupts(dev_priv);
>> + intel_disable_guc_interrupts(dev_priv);
>> i915_guc_submission_fini(dev_priv);
>> }
>>
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Regards,
>
> Tvrtko
More information about the Intel-gfx
mailing list