Quoting Mika Kuoppala (2018-02-14 14:12:13)
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> v2: Rebase.
>
> v3:
> * Remove DPF, it has been removed from SKL+.
> * Fix -internal rebase wrt. execlists interrupt handling.
>
> v4: Rebase.
>
> v5:
> * Updated for POR changes. (Daniele Ceraolo Spurio)
> * Merged with irq handling fixes by Daniele Ceraolo Spurio:
> * Simplify the code by using gen8_cs_irq_handler.
> * Fix interrupt handling for the upstream kernel.
>
> v6:
> * Remove early bringup debug messages (Tvrtko)
> * Add NB about arbitrary spin wait timeout (Tvrtko)
>
> v7 (from Paulo):
> * Don't try to write RO bits to registers.
> * Don't check for PCH types that don't exist. PCH interrupts are not
> here yet.
>
> v9:
> * squashed in selector and shared register handling (Daniele)
> * skip writing of irq if data is not valid (Daniele)
> * use time_after32 (Chris)
> * use I915_MAX_VCS and I915_MAX_VECS (Daniele)
> * remove fake pm interrupt handling for later patch (Mika)
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 212 ++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_pm.c | 7 +-
> 2 files changed, 218 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index b886bd459acc..9a2d12c8c44c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -408,6 +408,37 @@ void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
> spin_unlock_irq(&dev_priv->irq_lock);
> }
>
> +static int gen11_service_shared_iir(struct drm_i915_private *dev_priv,
> + const unsigned int bank,
> + const unsigned int bit)
> +{
> + u64 wait_end;
> + u32 ident;
> + int irq;
> +
> + I915_WRITE_FW(GEN11_IIR_REG_SELECTOR(bank), BIT(bit));
> + /*
> + * NB: Specs do not specify how long to spin wait.
> + * Taking 100us as an educated guess
> + */
> + wait_end = (local_clock() >> 10) + 100;
> + do {
> + ident = I915_READ_FW(GEN11_INTR_IDENTITY_REG(bank));
> + } while (!(ident & GEN11_INTR_DATA_VALID) &&
> + !time_after32(local_clock() >> 10, wait_end));
Now you are just mixing types willy nilly :)
No need for wait_end to be 64b when we are looking at a 100 interval.
> +
> + if (!(ident & GEN11_INTR_DATA_VALID)) {
> + DRM_ERROR("INTR_IDENTITY_REG%u:%u timed out!\n", bank, bit);
> + return -ETIMEDOUT;
> + }
> +
> + irq = ident & GEN11_INTR_ENGINE_MASK;
> +
> + I915_WRITE_FW(GEN11_INTR_IDENTITY_REG(bank), ident);
> +
> + return irq;
return ident & GEN11_INTR_ENGINE_MASK;
no need for irq, and why int return type?
Why is this gen11 specific helper so far away from the irq_handler?
> +static __always_inline void
> +gen11_cs_irq_handler(struct intel_engine_cs *engine, u32 iir)
> +{
> + gen8_cs_irq_handler(engine, iir, 0);
> +}
> +
> +static void
> +gen11_gt_irq_handler(struct drm_i915_private *dev_priv, const u32 master_ctl)
> +{
> + u16 irq[2][32];
> + unsigned int bank, engine;
> +
> + memset(irq, 0, sizeof(irq));
> +
> + for (bank = 0; bank < 2; bank++) {
> + unsigned long tmp;
> + unsigned int bit;
> + u32 dw;
> + int ret;
> +
> + if (!(master_ctl & GEN11_GT_DW_IRQ(bank)))
> + continue;
> +
> + dw = I915_READ_FW(GEN11_GT_INTR_DW(bank));
> + if (!dw)
> + DRM_ERROR("GT_INTR_DW%u blank!\n", bank);
> +
> + tmp = dw;
> + for_each_set_bit(bit, &tmp, 32) {
tmp is not required here.
> + ret = gen11_service_shared_iir(dev_priv, bank, bit);
> + if (unlikely(ret < 0))
> + continue;
> +
> + irq[bank][bit] = ret;
> + }
> +
> + I915_WRITE_FW(GEN11_GT_INTR_DW(bank), dw);
If we process the banks here, we won't need to memset 128 bytes and scan
untouched cachelines!
> + }
> +
> + if (irq[0][GEN11_RCS0])
> + gen11_cs_irq_handler(dev_priv->engine[RCS],
> + irq[0][GEN11_RCS0]);
> +
> + if (irq[0][GEN11_BCS])
> + gen11_cs_irq_handler(dev_priv->engine[BCS],
> + irq[0][GEN11_BCS]);
> +
> + for (engine = 0; engine < I915_MAX_VCS; engine++)
> + if (irq[1][GEN11_VCS(engine)])
> + gen11_cs_irq_handler(dev_priv->engine[_VCS(engine)],
> + irq[1][GEN11_VCS(engine)]);
> +
> + for (engine = 0; engine < I915_MAX_VECS; engine++)
> + if (irq[1][GEN11_VECS(engine)])
> + gen11_cs_irq_handler(dev_priv->engine[_VECS(engine)],
> + irq[1][GEN11_VECS(engine)]);
Keep reminding yourself that this is the hottest function in the entire
i915.ko.
> +}
> +
> +static irqreturn_t gen11_irq_handler(int irq, void *arg)
> +{
> + struct drm_device *dev = arg;
> + struct drm_i915_private *dev_priv = dev->dev_private;
What?
> + u32 master_ctl;
> + u32 disp_ctl;
Why is this at top level scope?
> + if (!intel_irqs_enabled(dev_priv))
> + return IRQ_NONE;
> +
> + master_ctl = I915_READ_FW(GEN11_GFX_MSTR_IRQ);
> + master_ctl &= ~GEN11_MASTER_IRQ;
> + if (!master_ctl)
> + return IRQ_NONE;
> +
> + /* Disable interrupts. */
> + I915_WRITE_FW(GEN11_GFX_MSTR_IRQ, 0);
> +
> + /* IRQs are synced during runtime_suspend, we don't require a wakeref */
> + disable_rpm_wakeref_asserts(dev_priv);
> +
> + /* Find, clear, then process each source of interrupt. */
> + gen11_gt_irq_handler(dev_priv, master_ctl);
> +
> + if (master_ctl & GEN11_DISPLAY_IRQ) {
> + disp_ctl = I915_READ_FW(GEN11_DISPLAY_INT_CTL);
> + gen8_de_irq_handler(dev_priv, disp_ctl);
> + }
> +
> + /* Acknowledge and enable interrupts. */
> + I915_WRITE_FW(GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ | master_ctl);
> + POSTING_READ_FW(GEN11_GFX_MSTR_IRQ);
> +
> + enable_rpm_wakeref_asserts(dev_priv);
What happened to the onion? gen8 is broken as well, sure I sent patches
to fix that. The posting read is ott, and you don't need to disable the
asserts around the GT irq handler.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
