[Intel-gfx] [PATCH 06/12] drm/i915/guc: unify guc irq handling
Michal Wajdeczko
michal.wajdeczko at intel.com
Wed Jul 10 17:04:16 UTC 2019
On Wed, 10 Jul 2019 02:54:31 +0200, Daniele Ceraolo Spurio
<daniele.ceraolospurio at intel.com> wrote:
> The 16-bit guc irq vector is unchanged across gens, the only thing that
> moved is its position (from the upper 16 bits of the PM regs to its own
> register). Instead of duplicating all defines and functions to handle
> the 2 different positions, we can work on the vector and shift it as
> appropriate. While at it, update the handler to work on intel_guc.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 24 ++++++++-------------
> drivers/gpu/drm/i915/i915_reg.h | 10 ---------
> drivers/gpu/drm/i915/intel_guc_reg.h | 32 ++++++++++++++--------------
> 3 files changed, 25 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 831d185c07d2..42d6d8bfac70 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -264,7 +264,7 @@ static void gen2_irq_init(struct intel_uncore
> *uncore,
> gen2_irq_init((uncore), imr_val, ier_val)
> 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);
> +static void guc_irq_handler(struct intel_guc *guc, u16 guc_iir);
> /* For display hotplug interrupt */
> static inline void
> @@ -658,8 +658,7 @@ void gen11_enable_guc_interrupts(struct intel_guc
> *guc)
> spin_lock_irq(&dev_priv->irq_lock);
> if (!guc->interrupts.enabled) {
> - u32 events = REG_FIELD_PREP(ENGINE1_MASK,
> - GEN11_GUC_INTR_GUC2HOST);
> + u32 events = REG_FIELD_PREP(ENGINE1_MASK, GUC_INTR_GUC2HOST);
> WARN_ON_ONCE(gen11_reset_one_iir(&dev_priv->gt, 0, GEN11_GUC));
> I915_WRITE(GEN11_GUC_SG_INTR_ENABLE, events);
> @@ -1656,7 +1655,7 @@ static void gen8_gt_irq_handler(struct
> drm_i915_private *i915,
> if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) {
> gen6_rps_irq_handler(i915, gt_iir[2]);
> - gen9_guc_irq_handler(i915, gt_iir[2]);
> + guc_irq_handler(&i915->guc, gt_iir[2] >> 16);
> }
> }
> @@ -1955,16 +1954,10 @@ static void gen6_rps_irq_handler(struct
> drm_i915_private *dev_priv, u32 pm_iir)
> DRM_DEBUG("Command parser error, pm_iir 0x%08x\n", pm_iir);
> }
> -static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32
> gt_iir)
> +static void guc_irq_handler(struct intel_guc *guc, u16 iir)
maybe it's the good time to move this handler out to intel_guc.c ?
> {
> - if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT)
> - intel_guc_to_host_event_handler(&dev_priv->guc);
> -}
> -
> -static void gen11_guc_irq_handler(struct drm_i915_private *i915, u16
> iir)
> -{
> - if (iir & GEN11_GUC_INTR_GUC2HOST)
> - intel_guc_to_host_event_handler(&i915->guc);
> + if (iir & GUC_INTR_GUC2HOST)
> + intel_guc_to_host_event_handler(guc);
> }
> static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv)
> @@ -3092,7 +3085,7 @@ gen11_other_irq_handler(struct intel_gt *gt, const
> u8 instance,
> struct drm_i915_private *i915 = gt->i915;
> if (instance == OTHER_GUC_INSTANCE)
> - return gen11_guc_irq_handler(i915, iir);
> + return guc_irq_handler(&i915->guc, iir);
> if (instance == OTHER_GTPM_INSTANCE)
> return gen11_rps_irq_handler(gt, iir);
> @@ -4764,8 +4757,9 @@ void intel_irq_init(struct drm_i915_private
> *dev_priv)
> for (i = 0; i < MAX_L3_SLICES; ++i)
> dev_priv->l3_parity.remap_info[i] = NULL;
> + /* pre-gen11 the guc irqs bits are in the upper 16 bits of the pm reg
> */
s/gen11/Gen11
s/guc/GuC
> if (HAS_GUC_SCHED(dev_priv) && INTEL_GEN(dev_priv) < 11)
> - dev_priv->pm_guc_events = GEN9_GUC_TO_HOST_INT_EVENT;
> + dev_priv->pm_guc_events = GUC_INTR_GUC2HOST << 16;
maybe we should add definition for this mask in PM register
#define GEN8_PM_IIR_GUC_MASK 0xFFFF0000
and then
dev_priv->pm_guc_events = REG_FIELD_PREP(GEN8_PM_IIR_GUC_MASK,
GUC_INTR_GUC2HOST)
and in gen8_gt_irq_handler() earlier above
guc_irq_handler(&i915->guc, REG_FIELD_GET(GEN8_PM_IIR_GUC_MASK,
gt_iir[2]));
> /* Let's track the enabled rps events */
> if (IS_VALLEYVIEW(dev_priv))
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 5898f59e3dd7..4dc31e488b80 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7342,16 +7342,6 @@ enum {
> #define GEN8_GT_IIR(which) _MMIO(0x44308 + (0x10 * (which)))
> #define GEN8_GT_IER(which) _MMIO(0x4430c + (0x10 * (which)))
> -#define GEN9_GUC_TO_HOST_INT_EVENT (1 << 31)
> -#define GEN9_GUC_EXEC_ERROR_EVENT (1 << 30)
> -#define GEN9_GUC_DISPLAY_EVENT (1 << 29)
> -#define GEN9_GUC_SEMA_SIGNAL_EVENT (1 << 28)
> -#define GEN9_GUC_IOMMU_MSG_EVENT (1 << 27)
> -#define GEN9_GUC_DB_RING_EVENT (1 << 26)
> -#define GEN9_GUC_DMA_DONE_EVENT (1 << 25)
> -#define GEN9_GUC_FATAL_ERROR_EVENT (1 << 24)
> -#define GEN9_GUC_NOTIFICATION_EVENT (1 << 23)
> -
> #define GEN8_RCS_IRQ_SHIFT 0
> #define GEN8_BCS_IRQ_SHIFT 16
> #define GEN8_VCS0_IRQ_SHIFT 0 /* NB: VCS1 in bspec! */
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h
> b/drivers/gpu/drm/i915/intel_guc_reg.h
> index a5ab7bc5504c..e3cbb23299ce 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -141,21 +141,21 @@ struct guc_doorbell_info {
> #define GUC_PM_P24C_IER _MMIO(0xC55C)
> /* GuC Interrupt Vector */
> -#define GEN11_GUC_INTR_GUC2HOST (1 << 15)
> -#define GEN11_GUC_INTR_EXEC_ERROR (1 << 14)
> -#define GEN11_GUC_INTR_DISPLAY_EVENT (1 << 13)
> -#define GEN11_GUC_INTR_SEM_SIG (1 << 12)
> -#define GEN11_GUC_INTR_IOMMU2GUC (1 << 11)
> -#define GEN11_GUC_INTR_DOORBELL_RANG (1 << 10)
> -#define GEN11_GUC_INTR_DMA_DONE (1 << 9)
> -#define GEN11_GUC_INTR_FATAL_ERROR (1 << 8)
> -#define GEN11_GUC_INTR_NOTIF_ERROR (1 << 7)
> -#define GEN11_GUC_INTR_SW_INT_6 (1 << 6)
> -#define GEN11_GUC_INTR_SW_INT_5 (1 << 5)
> -#define GEN11_GUC_INTR_SW_INT_4 (1 << 4)
> -#define GEN11_GUC_INTR_SW_INT_3 (1 << 3)
> -#define GEN11_GUC_INTR_SW_INT_2 (1 << 2)
> -#define GEN11_GUC_INTR_SW_INT_1 (1 << 1)
> -#define GEN11_GUC_INTR_SW_INT_0 (1 << 0)
> +#define GUC_INTR_GUC2HOST BIT(15)
> +#define GUC_INTR_EXEC_ERROR BIT(14)
> +#define GUC_INTR_DISPLAY_EVENT BIT(13)
> +#define GUC_INTR_SEM_SIG BIT(12)
> +#define GUC_INTR_IOMMU2GUC BIT(11)
> +#define GUC_INTR_DOORBELL_RANG BIT(10)
> +#define GUC_INTR_DMA_DONE BIT(9)
> +#define GUC_INTR_FATAL_ERROR BIT(8)
> +#define GUC_INTR_NOTIF_ERROR BIT(7)
> +#define GUC_INTR_SW_INT_6 BIT(6)
> +#define GUC_INTR_SW_INT_5 BIT(5)
> +#define GUC_INTR_SW_INT_4 BIT(4)
> +#define GUC_INTR_SW_INT_3 BIT(3)
> +#define GUC_INTR_SW_INT_2 BIT(2)
> +#define GUC_INTR_SW_INT_1 BIT(1)
> +#define GUC_INTR_SW_INT_0 BIT(0)
shouldn't we use REG_BIT here?
with/without above nits,
Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
More information about the Intel-gfx
mailing list