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:chris@xxxxxxxxxxxxxxxxxx] Sent: Thursday, July 13, 2017 12:28 PM To: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Kuoppala, Mika <mika.kuoppala@xxxxxxxxx>; Wang, Zhi A <zhi.a.wang@xxxxxxxxx> Subject: Re: [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 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
