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

[Timo Aaltonen]

On 08.05.2018 16:00, Timo Aaltonen wrote:
> From: Chris Wilson <chris at 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 at linux.intel.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Rafael Antognolli <rafael.antognolli at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Cc: Timo Aaltonen <tjaalton at ubuntu.com>
> Signed-off-by: Timo Aaltonen <timo.aaltonen at 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


-- 
t