Having completed a test run of gem_eio across all machines in CI we also observe the phenomenon (of lost interrupts after resetting the GPU) on gen3 machines as well as the previously sighted gen6/gen7. Let's apply the same HWSTAM workaround that was effective for gen6+ for all, as although we haven't seen the same failure on gen4/5 it seems prudent to keep the code the same. As a consequence we can remove the extra setting of HWSTAM and apply the register from a single site. v2: Delazy and move the HWSTAM into its own function v3: Mask off all HWSP writes on driver unload and engine cleanup. v4: And what about the physical hwsp? v5: No, engine->init_hw() is not called from driver_init_hw(), don't be daft. Really scrub HWSTAM as early as we can in driver_init_mmio() References: https://bugs.freedesktop.org/show_bug.cgi?id=108735 Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/i915_irq.c | 9 -- drivers/gpu/drm/i915/intel_engine_cs.c | 24 ++++++ drivers/gpu/drm/i915/intel_ringbuffer.c | 107 +++++++++++++++--------- 3 files changed, 92 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e2dac9b5f4ce..0c7fc9890891 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3586,9 +3586,6 @@ static void ironlake_irq_reset(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); - if (IS_GEN(dev_priv, 5)) - I915_WRITE(HWSTAM, 0xffffffff); - GEN3_IRQ_RESET(DE); if (IS_GEN(dev_priv, 7)) I915_WRITE(GEN7_ERR_INT, 0xffffffff); @@ -4368,8 +4365,6 @@ static void i8xx_irq_reset(struct drm_device *dev) i9xx_pipestat_irq_reset(dev_priv); - I915_WRITE16(HWSTAM, 0xffff); - GEN2_IRQ_RESET(); } @@ -4537,8 +4532,6 @@ static void i915_irq_reset(struct drm_device *dev) i9xx_pipestat_irq_reset(dev_priv); - I915_WRITE(HWSTAM, 0xffffffff); - GEN3_IRQ_RESET(); } @@ -4648,8 +4641,6 @@ static void i965_irq_reset(struct drm_device *dev) i9xx_pipestat_irq_reset(dev_priv); - I915_WRITE(HWSTAM, 0xffffffff); - GEN3_IRQ_RESET(); } diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 6f165f9ad2bf..24245dba3207 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -261,6 +261,24 @@ static void __sprint_engine_name(char *name, const struct engine_info *info) info->instance) >= INTEL_ENGINE_CS_MAX_NAME); } +static void set_hwsp(struct intel_engine_cs *engine, u32 mask) +{ + struct drm_i915_private *dev_priv = engine->i915; + i915_reg_t hwstam; + + hwstam = RING_HWSTAM(engine->mmio_base); + if (INTEL_GEN(dev_priv) >= 3) + I915_WRITE(hwstam, mask); + else + I915_WRITE16(hwstam, mask); +} + +static void intel_engine_sanitize_mmio(struct intel_engine_cs *engine) +{ + /* Mask off all writes into the unknown HWSP */ + set_hwsp(engine, ~0u); +} + static int intel_engine_setup(struct drm_i915_private *dev_priv, enum intel_engine_id id) @@ -312,6 +330,9 @@ intel_engine_setup(struct drm_i915_private *dev_priv, ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier); + /* Scrub mmio state on takeover */ + intel_engine_sanitize_mmio(engine); + dev_priv->engine_class[info->class][info->instance] = engine; dev_priv->engine[id] = engine; return 0; @@ -495,6 +516,9 @@ void intel_engine_setup_common(struct intel_engine_cs *engine) static void cleanup_status_page(struct intel_engine_cs *engine) { + /* Stop writing into the status page before returnig it to the system */ + set_hwsp(engine, ~0u); + if (HWS_NEEDS_PHYSICAL(engine->i915)) { void *addr = fetch_and_zero(&engine->status_page.page_addr); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index fdeca2b877c9..88f50e30a0c4 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -379,11 +379,31 @@ gen7_render_ring_flush(struct i915_request *rq, u32 mode) return 0; } -static void ring_setup_phys_status_page(struct intel_engine_cs *engine) +static void set_hwstam(struct intel_engine_cs *engine, u32 mask) +{ + struct drm_i915_private *dev_priv = engine->i915; + i915_reg_t hwstam = RING_HWSTAM(engine->mmio_base); + + /* + * Keep the render interrupt unmasked as this papers over + * lost interrupts following a reset. + */ + if (engine->id == RCS) { + if (INTEL_GEN(engine->i915) >= 6) + mask &= ~BIT(0); + else + mask &= ~I915_USER_INTERRUPT; + } + + if (INTEL_GEN(dev_priv) >= 3) + I915_WRITE(hwstam, mask); + else + I915_WRITE16(hwstam, mask); +} + +static void set_hws_pga(struct intel_engine_cs *engine, phys_addr_t phys) { struct drm_i915_private *dev_priv = engine->i915; - struct page *page = virt_to_page(engine->status_page.page_addr); - phys_addr_t phys = PFN_PHYS(page_to_pfn(page)); u32 addr; addr = lower_32_bits(phys); @@ -393,12 +413,22 @@ static void ring_setup_phys_status_page(struct intel_engine_cs *engine) I915_WRITE(HWS_PGA, addr); } -static void intel_ring_setup_status_page(struct intel_engine_cs *engine) +static void ring_setup_phys_status_page(struct intel_engine_cs *engine) +{ + struct page *page = virt_to_page(engine->status_page.page_addr); + phys_addr_t phys = PFN_PHYS(page_to_pfn(page)); + + set_hws_pga(engine, phys); + set_hwstam(engine, ~0u); +} + +static void set_hwsp(struct intel_engine_cs *engine, u32 offset) { struct drm_i915_private *dev_priv = engine->i915; - i915_reg_t mmio; + i915_reg_t hwsp; - /* The ring status page addresses are no longer next to the rest of + /* + * The ring status page addresses are no longer next to the rest of * the ring registers as of gen7. */ if (IS_GEN(dev_priv, 7)) { @@ -410,56 +440,55 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine) default: GEM_BUG_ON(engine->id); case RCS: - mmio = RENDER_HWS_PGA_GEN7; + hwsp = RENDER_HWS_PGA_GEN7; break; case BCS: - mmio = BLT_HWS_PGA_GEN7; + hwsp = BLT_HWS_PGA_GEN7; break; case VCS: - mmio = BSD_HWS_PGA_GEN7; + hwsp = BSD_HWS_PGA_GEN7; break; case VECS: - mmio = VEBOX_HWS_PGA_GEN7; + hwsp = VEBOX_HWS_PGA_GEN7; break; } } else if (IS_GEN(dev_priv, 6)) { - mmio = RING_HWS_PGA_GEN6(engine->mmio_base); + hwsp = RING_HWS_PGA_GEN6(engine->mmio_base); } else { - mmio = RING_HWS_PGA(engine->mmio_base); + hwsp = RING_HWS_PGA(engine->mmio_base); } - if (INTEL_GEN(dev_priv) >= 6) { - u32 mask = ~0u; + I915_WRITE(hwsp, offset); + POSTING_READ(hwsp); +} - /* - * Keep the render interrupt unmasked as this papers over - * lost interrupts following a reset. - */ - if (engine->id == RCS) - mask &= ~BIT(0); +static void flush_cs_tlb(struct intel_engine_cs *engine) +{ + struct drm_i915_private *dev_priv = engine->i915; + i915_reg_t instpm = RING_INSTPM(engine->mmio_base); - I915_WRITE(RING_HWSTAM(engine->mmio_base), mask); - } + if (!IS_GEN_RANGE(dev_priv, 6, 7)) + return; - I915_WRITE(mmio, engine->status_page.ggtt_offset); - POSTING_READ(mmio); + /* ring should be idle before issuing a sync flush*/ + WARN_ON((I915_READ_MODE(engine) & MODE_IDLE) == 0); - /* Flush the TLB for this page */ - if (IS_GEN_RANGE(dev_priv, 6, 7)) { - i915_reg_t reg = RING_INSTPM(engine->mmio_base); + I915_WRITE(instpm, + _MASKED_BIT_ENABLE(INSTPM_TLB_INVALIDATE | + INSTPM_SYNC_FLUSH)); + if (intel_wait_for_register(dev_priv, + instpm, INSTPM_SYNC_FLUSH, 0, + 1000)) + DRM_ERROR("%s: wait for SyncFlush to complete for TLB invalidation timed out\n", + engine->name); +} - /* ring should be idle before issuing a sync flush*/ - WARN_ON((I915_READ_MODE(engine) & MODE_IDLE) == 0); +static void ring_setup_status_page(struct intel_engine_cs *engine) +{ + set_hwsp(engine, engine->status_page.ggtt_offset); + set_hwstam(engine, ~0u); - I915_WRITE(reg, - _MASKED_BIT_ENABLE(INSTPM_TLB_INVALIDATE | - INSTPM_SYNC_FLUSH)); - if (intel_wait_for_register(dev_priv, - reg, INSTPM_SYNC_FLUSH, 0, - 1000)) - DRM_ERROR("%s: wait for SyncFlush to complete for TLB invalidation timed out\n", - engine->name); - } + flush_cs_tlb(engine); } static bool stop_ring(struct intel_engine_cs *engine) @@ -529,7 +558,7 @@ static int init_ring_common(struct intel_engine_cs *engine) if (HWS_NEEDS_PHYSICAL(dev_priv)) ring_setup_phys_status_page(engine); else - intel_ring_setup_status_page(engine); + ring_setup_status_page(engine); intel_engine_reset_breadcrumbs(engine); -- 2.20.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
