| Message ID | 1508309222-26406-2-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: > 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@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 | 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. > @@ -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@intel.com> Regards, Tvrtko
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@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 | 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@intel.com> > > Regards, > > Tvrtko
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; } @@ -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); 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);
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@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 | 8 ++++---- drivers/gpu/drm/i915/intel_drv.h | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-)