| Message ID | 20180511121147.31915-3-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Chris Wilson (2018-05-11 13:11:47) > The original switch to use CSB from the HWSP was plagued by the effort s/effort/effect/ > of read ordering on VT-d; we would read the WRITE pointer from the HWSP > before it had completed writing the CSB contents. The mystery comes down > to the lack of rmb() for correct ordering with respect to the writes > from HW, and with that resolved we can remove the VT-d special casing. -Chris
Quoting Chris Wilson (2018-05-11 13:11:47) > The original switch to use CSB from the HWSP was plagued by the effort > of read ordering on VT-d; we would read the WRITE pointer from the HWSP > before it had completed writing the CSB contents. The mystery comes down > to the lack of rmb() for correct ordering with respect to the writes > from HW, and with that resolved we can remove the VT-d special casing. Mika's been able to reproduce the VT-d issue and is soak testing this fix, so I'll leave that until he's had a chance to confirm it survives. In the meantime, I think we are reasonably happy this is the right fix for Cannonlake and beyond, so I've pushed the first two patches. Thanks for the review and testing, -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Chris Wilson (2018-05-11 13:11:47) >> The original switch to use CSB from the HWSP was plagued by the effort >> of read ordering on VT-d; we would read the WRITE pointer from the HWSP >> before it had completed writing the CSB contents. The mystery comes down >> to the lack of rmb() for correct ordering with respect to the writes >> from HW, and with that resolved we can remove the VT-d special casing. > > Mika's been able to reproduce the VT-d issue and is soak testing this > fix, so I'll leave that until he's had a chance to confirm it survives. > In the meantime, I think we are reasonably happy this is the right fix > for Cannonlake and beyond, so I've pushed the first two patches. > My kbl was quite sensitive to this, sometimes failing to even boot without the vtd backoff. Now after weekend of gem_ctx_switch and gem_exec_whisper I am confident for, Tested-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Quoting Mika Kuoppala (2018-05-14 09:33:23) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Quoting Chris Wilson (2018-05-11 13:11:47) > >> The original switch to use CSB from the HWSP was plagued by the effort > >> of read ordering on VT-d; we would read the WRITE pointer from the HWSP > >> before it had completed writing the CSB contents. The mystery comes down > >> to the lack of rmb() for correct ordering with respect to the writes > >> from HW, and with that resolved we can remove the VT-d special casing. > > > > Mika's been able to reproduce the VT-d issue and is soak testing this > > fix, so I'll leave that until he's had a chance to confirm it survives. > > In the meantime, I think we are reasonably happy this is the right fix > > for Cannonlake and beyond, so I've pushed the first two patches. > > > > My kbl was quite sensitive to this, sometimes failing to even > boot without the vtd backoff. > > Now after weekend of gem_ctx_switch and gem_exec_whisper > I am confident for, > > Tested-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Thanks. The proof of the pudding is in the eating! Pushed to a wider audience :) -Chris
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 8303e05b0c7d..6bfd7e3ed152 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -458,14 +458,6 @@ static void intel_engine_init_batch_pool(struct intel_engine_cs *engine) static bool csb_force_mmio(struct drm_i915_private *i915) { - /* - * IOMMU adds unpredictable latency causing the CSB write (from the - * GPU into the HWSP) to only be visible some time after the interrupt - * (missed breadcrumb syndrome). - */ - if (intel_vtd_active()) - return true; - /* Older GVT emulation depends upon intercepting CSB mmio */ if (intel_vgpu_active(i915) && !intel_vgpu_has_hwsp_emulation(i915)) return true;
The original switch to use CSB from the HWSP was plagued by the effort of read ordering on VT-d; we would read the WRITE pointer from the HWSP before it had completed writing the CSB contents. The mystery comes down to the lack of rmb() for correct ordering with respect to the writes from HW, and with that resolved we can remove the VT-d special casing. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 8 -------- 1 file changed, 8 deletions(-)