[PATCH 3/6] drm/i915: Tweak ironlake_irq_handler to reduce GT interrupt latency
Chris Wilson
chris at chris-wilson.co.uk
Tue Dec 18 12:40:37 UTC 2018
As we depend on interrupts for engine and client synchronisation, we are
extremely sensitive to latency in the interrupt handler. Each mmio
access take an eternity and processing an unlikely display interrupt
delays request submission by several microseconds.
For example using patch,
ivb-3720qm, gem_exec_nop/sequential: 15.0us -> 9.5us
(baseline with HW semaphores: 4.4us)
A very significant reduction in the overhead imposed by removal of using
HW semaphores for inter-engine synchronisation.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 150 +++++++++++++++++++-------------
2 files changed, 91 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7e2cf3275bb7..d9e7543c9fa4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1517,6 +1517,7 @@ struct drm_i915_private {
/** Cached value of IMR to avoid reads in updating the bitfield */
union {
u32 irq_mask;
+ u32 irq_enabled;
u32 de_irq_mask[I915_MAX_PIPES];
};
u32 gt_irq_mask;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0c7fc9890891..6ad15264cd65 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2592,31 +2592,14 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
}
}
-/*
- * To handle irqs with the minimum potential races with fresh interrupts, we:
- * 1 - Disable Master Interrupt Control.
- * 2 - Find the source(s) of the interrupt.
- * 3 - Clear the Interrupt Identity bits (IIR).
- * 4 - Process the interrupt(s) that had bits set in the IIRs.
- * 5 - Re-enable Master Interrupt Control.
- */
-static irqreturn_t ironlake_irq_handler(int irq, void *arg)
+static noinline irqreturn_t ironlake_irq_de(struct drm_i915_private *dev_priv)
{
- struct drm_device *dev = arg;
- struct drm_i915_private *dev_priv = to_i915(dev);
- u32 de_iir, gt_iir, de_ier, sde_ier = 0;
irqreturn_t ret = IRQ_NONE;
-
- if (!intel_irqs_enabled(dev_priv))
- return IRQ_NONE;
+ u32 de_iir, sde_ier = 0;
/* IRQs are synced during runtime_suspend, we don't require a wakeref */
disable_rpm_wakeref_asserts(dev_priv);
- /* disable master interrupt before clearing iir */
- de_ier = I915_READ(DEIER);
- I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
-
/* Disable south interrupts. We'll only write to SDEIIR once, so further
* interrupts will will be stored on its back queue, and then we'll be
* able to process them after we restore SDEIER (as soon as we restore
@@ -2627,38 +2610,17 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
I915_WRITE(SDEIER, 0);
}
- /* Find, clear, then process each source of interrupt */
-
- gt_iir = I915_READ(GTIIR);
- if (gt_iir) {
- I915_WRITE(GTIIR, gt_iir);
- ret = IRQ_HANDLED;
- if (INTEL_GEN(dev_priv) >= 6)
- snb_gt_irq_handler(dev_priv, gt_iir);
- else
- ilk_gt_irq_handler(dev_priv, gt_iir);
- }
-
de_iir = I915_READ(DEIIR);
if (de_iir) {
I915_WRITE(DEIIR, de_iir);
- ret = IRQ_HANDLED;
if (INTEL_GEN(dev_priv) >= 7)
ivb_display_irq_handler(dev_priv, de_iir);
else
ilk_display_irq_handler(dev_priv, de_iir);
+ ret = IRQ_HANDLED;
}
- if (INTEL_GEN(dev_priv) >= 6) {
- u32 pm_iir = I915_READ(GEN6_PMIIR);
- if (pm_iir) {
- I915_WRITE(GEN6_PMIIR, pm_iir);
- ret = IRQ_HANDLED;
- gen6_rps_irq_handler(dev_priv, pm_iir);
- }
- }
-
- I915_WRITE(DEIER, de_ier);
+ I915_WRITE(DEIER, dev_priv->irq_enabled);
if (!HAS_PCH_NOP(dev_priv))
I915_WRITE(SDEIER, sde_ier);
@@ -2668,6 +2630,73 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
return ret;
}
+/*
+ * To handle irqs with the minimum potential races with fresh interrupts, we:
+ * 1 - Disable Master Interrupt Control.
+ * 2 - Find the source(s) of the interrupt.
+ * 3 - Clear the Interrupt Identity bits (IIR).
+ * 4 - Process the interrupt(s) that had bits set in the IIRs.
+ * 5 - Re-enable Master Interrupt Control.
+ */
+static irqreturn_t ironlake_irq_handler(int irq, void *arg)
+{
+ struct drm_i915_private *dev_priv = arg;
+ u32 iir;
+
+ if (!intel_irqs_enabled(dev_priv))
+ return IRQ_NONE;
+
+ /* disable master interrupt before clearing iir */
+ I915_WRITE_FW(DEIER, dev_priv->irq_enabled & ~DE_MASTER_IRQ_CONTROL);
+
+ /* Find, clear, then process each source of interrupt */
+
+ iir = I915_READ_FW(GTIIR);
+ if (likely(iir)) { /* prioritise GT-interrupts for low latency */
+ I915_WRITE_FW(GTIIR, iir);
+ I915_WRITE_FW(DEIER, dev_priv->irq_enabled);
+
+ ilk_gt_irq_handler(dev_priv, iir);
+ return IRQ_HANDLED;
+ }
+
+ return ironlake_irq_de(dev_priv);
+}
+
+static irqreturn_t sandybridge_irq_handler(int irq, void *arg)
+{
+ struct drm_i915_private *dev_priv = arg;
+ u32 iir;
+
+ if (!intel_irqs_enabled(dev_priv))
+ return IRQ_NONE;
+
+ /* disable master interrupt before clearing iir */
+ I915_WRITE_FW(DEIER, dev_priv->irq_enabled & ~DE_MASTER_IRQ_CONTROL);
+
+ /* Find, clear, then process each source of interrupt */
+
+ iir = I915_READ_FW(GTIIR);
+ if (likely(iir)) { /* prioritise GT-interrupts for low latency */
+ I915_WRITE_FW(GTIIR, iir);
+ I915_WRITE_FW(DEIER, dev_priv->irq_enabled);
+
+ snb_gt_irq_handler(dev_priv, iir);
+ return IRQ_HANDLED;
+ }
+
+ iir = I915_READ_FW(GEN6_PMIIR);
+ if (iir) {
+ I915_WRITE_FW(GEN6_PMIIR, iir);
+ I915_WRITE_FW(DEIER, dev_priv->irq_enabled);
+
+ gen6_rps_irq_handler(dev_priv,iir);
+ return IRQ_HANDLED;
+ }
+
+ return ironlake_irq_de(dev_priv);
+}
+
static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
u32 hotplug_trigger,
const u32 hpd[HPD_NUM_PINS])
@@ -3554,7 +3583,6 @@ static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
{
u32 pipestat_mask;
- u32 enable_mask;
enum pipe pipe;
pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS;
@@ -3563,21 +3591,22 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
for_each_pipe(dev_priv, pipe)
i915_enable_pipestat(dev_priv, pipe, pipestat_mask);
- enable_mask = I915_DISPLAY_PORT_INTERRUPT |
+ dev_priv->irq_enabled =
+ I915_DISPLAY_PORT_INTERRUPT |
I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
I915_LPE_PIPE_A_INTERRUPT |
I915_LPE_PIPE_B_INTERRUPT;
if (IS_CHERRYVIEW(dev_priv))
- enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT |
+ dev_priv->irq_enabled |=
+ I915_DISPLAY_PIPE_C_EVENT_INTERRUPT |
I915_LPE_PIPE_C_INTERRUPT;
WARN_ON(dev_priv->irq_mask != ~0u);
+ dev_priv->irq_mask = ~dev_priv->irq_enabled;
- dev_priv->irq_mask = ~enable_mask;
-
- GEN3_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
+ GEN3_IRQ_INIT(VLV_, dev_priv->irq_mask, dev_priv->irq_enabled);
}
/* drm_dma.h hooks
@@ -4092,6 +4121,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
}
dev_priv->irq_mask = ~display_mask;
+ dev_priv->irq_enabled = display_mask | extra_mask;
ibx_irq_pre_postinstall(dev);
@@ -4371,7 +4401,6 @@ static void i8xx_irq_reset(struct drm_device *dev)
static int i8xx_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- u16 enable_mask;
I915_WRITE16(EMR, ~(I915_ERROR_PAGE_TABLE |
I915_ERROR_MEMORY_REFRESH));
@@ -4382,13 +4411,13 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
I915_MASTER_ERROR_INTERRUPT);
- enable_mask =
+ dev_priv->irq_enabled =
I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
I915_MASTER_ERROR_INTERRUPT |
I915_USER_INTERRUPT;
- GEN2_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
+ GEN2_IRQ_INIT(, dev_priv->irq_mask, dev_priv->irq_enabled);
/* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked check happy. */
@@ -4538,7 +4567,6 @@ static void i915_irq_reset(struct drm_device *dev)
static int i915_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- u32 enable_mask;
I915_WRITE(EMR, ~(I915_ERROR_PAGE_TABLE |
I915_ERROR_MEMORY_REFRESH));
@@ -4550,7 +4578,7 @@ static int i915_irq_postinstall(struct drm_device *dev)
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
I915_MASTER_ERROR_INTERRUPT);
- enable_mask =
+ dev_priv->irq_enabled =
I915_ASLE_INTERRUPT |
I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
@@ -4559,12 +4587,12 @@ static int i915_irq_postinstall(struct drm_device *dev)
if (I915_HAS_HOTPLUG(dev_priv)) {
/* Enable in IER... */
- enable_mask |= I915_DISPLAY_PORT_INTERRUPT;
+ dev_priv->irq_enabled |= I915_DISPLAY_PORT_INTERRUPT;
/* and unmask in IMR */
dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT;
}
- GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
+ GEN3_IRQ_INIT(, dev_priv->irq_mask, dev_priv->irq_enabled);
/* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked check happy. */
@@ -4647,7 +4675,6 @@ static void i965_irq_reset(struct drm_device *dev)
static int i965_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- u32 enable_mask;
u32 error_mask;
/*
@@ -4673,7 +4700,7 @@ static int i965_irq_postinstall(struct drm_device *dev)
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
I915_MASTER_ERROR_INTERRUPT);
- enable_mask =
+ dev_priv->irq_enabled =
I915_ASLE_INTERRUPT |
I915_DISPLAY_PORT_INTERRUPT |
I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
@@ -4682,9 +4709,9 @@ static int i965_irq_postinstall(struct drm_device *dev)
I915_USER_INTERRUPT;
if (IS_G4X(dev_priv))
- enable_mask |= I915_BSD_USER_INTERRUPT;
+ dev_priv->irq_enabled |= I915_BSD_USER_INTERRUPT;
- GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
+ GEN3_IRQ_INIT(, dev_priv->irq_mask, dev_priv->irq_enabled);
/* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked check happy. */
@@ -4907,7 +4934,10 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
else
dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
} else if (HAS_PCH_SPLIT(dev_priv)) {
- dev->driver->irq_handler = ironlake_irq_handler;
+ if (INTEL_GEN(dev_priv) >= 6)
+ dev->driver->irq_handler = sandybridge_irq_handler;
+ else
+ dev->driver->irq_handler = ironlake_irq_handler;
dev->driver->irq_preinstall = ironlake_irq_reset;
dev->driver->irq_postinstall = ironlake_irq_postinstall;
dev->driver->irq_uninstall = ironlake_irq_reset;
--
2.20.0
More information about the Intel-gfx-trybot
mailing list