[v2,4/5] drm/i915/execlists: Read the context-status buffer from the HWSP

Commit Message

Wang, Zhi A July 13, 2017, 9:36 a.m. UTC

Sorry, my bad for missing it as I only saw it used on CHV before your patch because of some HW issue.

Kinda of interested in the reason why HWSP CSB gains people's interest now? :P. Anyway, it's good move to reduce the MMIO trap here.

Thanks,
Zhi.

-----Original Message-----
From: Chris Wilson [mailto:[e-mail]] 
Sent: Thursday, July 13, 2017 12:28 PM
To: Zhenyu Wang <[e-mail]>
Cc: [e-mail]; Kuoppala, Mika <[e-mail]>; Wang, Zhi A <[e-mail]>
Subject: Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/execlists: Read the context-status buffer from the HWSP

Quoting Zhenyu Wang (2017-07-13 10:14:34)
> On 2017.07.13 10:00:25 +0100, Chris Wilson wrote:
> > The engine provides a mirror of the CSB in the HWSP. If we use the 
> > cacheable reads from the HWSP, we can shave off a few mmio reads per 
> > context-switch interrupt (which are quite frequent!). Just removing 
> > a couple of mmio is not enough to actually reduce any latency, but a 
> > small reduction in overall cpu usage.
> 
> Unfortunately current gvt's execlist emulation depends on MMIO CSB 
> read for guest workload without guest HWSP update. So this can't work for guest.
> We need to fix that in gvt, also reduce MMIO trap is good benefit for vGPU too.
> But might have to fallback to mmio mode if vgpu active now, and once 
> gvt host got fixed, will notify through pvinfo to enable this.

* shakes fist at gvt, spoilsports.

Is the right test intel_vgpu_active()? i.e. something like

Intel-gfx mailing list
[e-mail]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Comments

Chris Wilson July 13, 2017, 9:43 a.m. UTC | #1

Quoting Wang, Zhi A (2017-07-13 10:36:14)
> Sorry, my bad for missing it as I only saw it used on CHV before your patch because of some HW issue.
> 
> Kinda of interested in the reason why HWSP CSB gains people's interest now? :P. Anyway, it's good move to reduce the MMIO trap here.

We've been chasing the elimination of mmio reads here for the last few
years, since the buffer is tantalising laid out inside a cacheline and
bspec says it was designed to be read in a single cacheline load.
Previously I had an alias for the CSB using ioremap_cache(), which
sort-of-worked (bxt was being a little unhappy) but still required a
clflush. Imagine then the surprise and delight that the information we
wanted was in the HWSP ever since execlists was a twinkle in the
architect's eye (back in gm45!).
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index e3e9d850cf88..a259d584e1fd 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -504,6 +504,13 @@  static void intel_lrc_irq_handler(unsigned long data)
                        &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
                unsigned int head, tail;
 
+               /* However GVT emulation depends upon intercepting CSB mmio */
+               if (unlikely(intel_vgpu_active(dev_priv))) {
+                       buf = (u32 * __force)dev_priv->regs +
+                               i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
+                       engine->csb_head = -1;
+               }
+

would do the trick, though I guess it wants to test a version if GVT starts providing CSB emulation via the HWSP.
-Chris
_______________________________________________