[SRU,OEM-B] UBUNTU: SAUCE: drm/i915/execlists: Use rmb() to order CSB reads

From: Chris Wilson <chris@chris-wilson.co.uk>

BugLink: http://bugs.launchpad.net/bugs/1769843

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>
Signed-off-by: Timo Aaltonen <timo.aaltonen@canonical.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 1 +
 1 file changed, 1 insertion(+)

On 08.05.2018 16:00, Timo Aaltonen wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> BugLink: http://bugs.launchpad.net/bugs/1769843
> 
> 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>
> Signed-off-by: Timo Aaltonen <timo.aaltonen@canonical.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index e8af93e9235a..c4bcaa0cf677 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -838,6 +838,7 @@ static void intel_lrc_irq_handler(unsigned long data)
>  
>  			head = execlists->csb_head;
>  			tail = READ_ONCE(buf[write_idx]);
> +			rmb(); /* Hopefully paired with a wmb() in HW */
>  		}
>  
>  		while (head != tail) {
> 

and for the record it should be mentioned that this got applied to
oem-next already back then