| Message ID | 1508309222-26406-3-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@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. > } > > 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@intel.com> Regards, Tvrtko
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@intel.com> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Joonas Lahtinen <joonas.lahtinen@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@intel.com> > > Regards, > > Tvrtko
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]); } 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); }
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@intel.com> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@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(-)