| Message ID | 20180508101506.22759-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08.05.2018 13:15, Chris Wilson wrote: > We assume that the CSB is written using the normal ringbuffer > coherency protocols, as outlined in kernel/events/ring_buffer.c: > > * (HW) (DRIVER) > * > * if (LOAD ->data_tail) { LOAD ->data_head > * (A) smp_rmb() (C) > * STORE $data LOAD $data > * smp_wmb() (B) smp_mb() (D) > * STORE ->data_head STORE ->data_tail > * } > > So we assume that the HW fulfils it's ordering requirements, and so we > should use a complimentary rmb() to ensure that our read of its WRITE > pointer is completed before we start accessing the data. > > The final mb() is implied by the uncached mmio we perform to inform the > HW of our READ pointer. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=105064 > References: https://bugs.freedesktop.org/show_bug.cgi?id=105888 > References: https://bugs.freedesktop.org/show_bug.cgi?id=106185 > References: 61bf9719fa17 ("drm/i915/cnl: Use mmio access to context status buffer") > Suggested-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > 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: Rafael Antognolli <rafael.antognolli@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > Cc: Timo Aaltonen <tjaalton@ubuntu.com> Seems to work here on CNL-Y just like 61bf9719fa17. Tested-by: Timo Aaltonen <tjaalton@ubuntu.com>
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 70325e0824e3..8303e05b0c7d 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -470,9 +470,6 @@ static bool csb_force_mmio(struct drm_i915_private *i915) if (intel_vgpu_active(i915) && !intel_vgpu_has_hwsp_emulation(i915)) return true; - if (IS_CANNONLAKE(i915)) - return true; - return false; } diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index f9f4064dec0e..81dd5da7d055 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -993,6 +993,7 @@ static void execlists_submission_tasklet(unsigned long data) head = execlists->csb_head; tail = READ_ONCE(buf[write_idx]); + rmb(); /* Hopefully paired with a wmb() in HW */ } GEM_TRACE("%s cs-irq head=%d [%d%s], tail=%d [%d%s]\n", engine->name,
We assume that the CSB is written using the normal ringbuffer coherency protocols, as outlined in kernel/events/ring_buffer.c: * (HW) (DRIVER) * * if (LOAD ->data_tail) { LOAD ->data_head * (A) smp_rmb() (C) * STORE $data LOAD $data * smp_wmb() (B) smp_mb() (D) * STORE ->data_head STORE ->data_tail * } So we assume that the HW fulfils it's ordering requirements, and so we should use a complimentary rmb() to ensure that our read of its WRITE pointer is completed before we start accessing the data. The final mb() is implied by the uncached mmio we perform to inform the HW of our READ pointer. References: https://bugs.freedesktop.org/show_bug.cgi?id=105064 References: https://bugs.freedesktop.org/show_bug.cgi?id=105888 References: https://bugs.freedesktop.org/show_bug.cgi?id=106185 References: 61bf9719fa17 ("drm/i915/cnl: Use mmio access to context status buffer") Suggested-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> 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: Rafael Antognolli <rafael.antognolli@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Timo Aaltonen <tjaalton@ubuntu.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 3 --- drivers/gpu/drm/i915/intel_lrc.c | 1 + 2 files changed, 1 insertion(+), 3 deletions(-)