[Intel-gfx] [PATCH v2 1/1] drm/i915: Save PM interrupt register offsets in device info
Michal Wajdeczko
michal.wajdeczko at intel.com
Tue Oct 24 10:46:09 UTC 2017
On Tue, 24 Oct 2017 12:41:13 +0200, Sagar Arun Kamble
<sagar.a.kamble at intel.com> wrote:
> PM interrupt register offsets are constant per platforms and saving those
> in device info is more appropriate than getting those through functions.
> This patch removes functions gen6_pm_iir/imr/ier and saves those offsets
> in device info.
>
> v2: Use INTEL_INFO() to access device info. (Chris)
>
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 +++++
> drivers/gpu/drm/i915/i915_irq.c | 31
> +++++++++----------------------
> drivers/gpu/drm/i915/intel_device_info.c | 11 +++++++++++
> 3 files changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 54b5d4c..2f77d26 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -888,6 +888,11 @@ struct intel_device_info {
> u16 degamma_lut_size;
> u16 gamma_lut_size;
> } color;
> +
> + /* PM interrupt register offsets */
> + i915_reg_t pm_iir_offset;
> + i915_reg_t pm_imr_offset;
> + i915_reg_t pm_ier_offset;
> };
> struct intel_display_error_state;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index b1296a5..5d448af 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -306,21 +306,6 @@ 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)
> -{
> - return INTEL_GEN(dev_priv) >= 8 ? GEN8_GT_IIR(2) : GEN6_PMIIR;
> -}
> -
> -static i915_reg_t gen6_pm_imr(struct drm_i915_private *dev_priv)
> -{
> - return INTEL_GEN(dev_priv) >= 8 ? GEN8_GT_IMR(2) : GEN6_PMIMR;
> -}
> -
> -static i915_reg_t gen6_pm_ier(struct drm_i915_private *dev_priv)
> -{
> - return INTEL_GEN(dev_priv) >= 8 ? GEN8_GT_IER(2) : GEN6_PMIER;
> -}
> -
btw, if you keep these functions but modify them into:
return INTEL_INFO(dev_priv)->pm_xxx_offset;
then most of below changes will not be needed
> /**
> * snb_update_pm_irq - update GEN6_PMIMR
> * @dev_priv: driver private
> @@ -332,6 +317,7 @@ static void snb_update_pm_irq(struct
> drm_i915_private *dev_priv,
> uint32_t enabled_irq_mask)
> {
> uint32_t new_val;
> + i915_reg_t reg = INTEL_INFO(dev_priv)->pm_imr_offset;
s/reg/imr ?
> WARN_ON(enabled_irq_mask & ~interrupt_mask);
> @@ -343,8 +329,8 @@ static void snb_update_pm_irq(struct
> drm_i915_private *dev_priv,
> if (new_val != dev_priv->pm_imr) {
> dev_priv->pm_imr = new_val;
> - I915_WRITE(gen6_pm_imr(dev_priv), dev_priv->pm_imr);
> - POSTING_READ(gen6_pm_imr(dev_priv));
> + I915_WRITE(reg, dev_priv->pm_imr);
> + POSTING_READ(reg);
> }
> }
> @@ -371,7 +357,7 @@ void gen6_mask_pm_irq(struct drm_i915_private
> *dev_priv, u32 mask)
> static void gen6_reset_pm_iir(struct drm_i915_private *dev_priv, u32
> reset_mask)
> {
> - i915_reg_t reg = gen6_pm_iir(dev_priv);
> + i915_reg_t reg = INTEL_INFO(dev_priv)->pm_iir_offset;
s/reg/iir ?
> lockdep_assert_held(&dev_priv->irq_lock);
> @@ -385,7 +371,7 @@ static void gen6_enable_pm_irq(struct
> drm_i915_private *dev_priv, u32 enable_mas
> lockdep_assert_held(&dev_priv->irq_lock);
> dev_priv->pm_ier |= enable_mask;
> - I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier);
> + I915_WRITE(INTEL_INFO(dev_priv)->pm_ier_offset, dev_priv->pm_ier);
> gen6_unmask_pm_irq(dev_priv, enable_mask);
> /* unmask_pm_irq provides an implicit barrier (POSTING_READ) */
> }
> @@ -396,7 +382,7 @@ static void gen6_disable_pm_irq(struct
> drm_i915_private *dev_priv, u32 disable_m
> dev_priv->pm_ier &= ~disable_mask;
> __gen6_mask_pm_irq(dev_priv, disable_mask);
> - I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier);
> + I915_WRITE(INTEL_INFO(dev_priv)->pm_ier_offset, dev_priv->pm_ier);
> /* though a barrier is missing here, but don't really need a one */
> }
> @@ -417,7 +403,8 @@ void gen6_enable_rps_interrupts(struct
> drm_i915_private *dev_priv)
> spin_lock_irq(&dev_priv->irq_lock);
> WARN_ON_ONCE(rps->pm_iir);
> - WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) &
> dev_priv->pm_rps_events);
> + WARN_ON_ONCE(I915_READ(INTEL_INFO(dev_priv)->pm_iir_offset) &
> + dev_priv->pm_rps_events);
Can you define separate iir_reg variable as in above functions to simplify
this nested statement ?
> rps->interrupts_enabled = true;
> gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
> @@ -461,7 +448,7 @@ 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)) &
> + WARN_ON_ONCE(I915_READ(INTEL_INFO(dev_priv)->pm_iir_offset) &
> dev_priv->pm_guc_events);
> dev_priv->guc.interrupts_enabled = true;
> gen6_enable_pm_irq(dev_priv, dev_priv->pm_guc_events);
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> index 875d428..d1a4911 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -462,4 +462,15 @@ void intel_device_info_runtime_init(struct
> drm_i915_private *dev_priv)
> info->sseu.has_subslice_pg ? "y" : "n");
> DRM_DEBUG_DRIVER("has EU power gating: %s\n",
> info->sseu.has_eu_pg ? "y" : "n");
> +
> + /* Initialize PM interrupt register offsets */
> + if (INTEL_GEN(dev_priv) >= 8) {
> + info->pm_iir_offset = GEN8_GT_IIR(2);
> + info->pm_imr_offset = GEN8_GT_IMR(2);
> + info->pm_ier_offset = GEN8_GT_IER(2);
> + } else {
> + info->pm_iir_offset = GEN6_PMIIR;
> + info->pm_imr_offset = GEN6_PMIMR;
> + info->pm_ier_offset = GEN6_PMIER;
> + }
> }
More information about the Intel-gfx
mailing list