Execlists rmb() patchset

Execlists rmb() patchset

[Chris Wilson]

Split the previous rmb patch into two. If the synopsis is correct and
this is a CPU read-ordering bug, we need to backport the rmb patch to
v4.15.
-chris


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915/execlists: Use rmb() to order CSB reads

[Chris Wilson]

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 its ordering requirements (B), and so
we should use a complimentary rmb (C) to ensure that our read of its
WRITE pointer is completed before we start accessing the data.

The final mb (D) 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
Fixes: 767a983ab255 ("drm/i915/execlists: Read the context-status HEAD from the HWSP")
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>
Tested-by: Timo Aaltonen <tjaalton@ubuntu.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 911f288f78aa..8977600f0d81 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -992,6 +992,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,
-- 
2.17.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] Revert "drm/i915/cnl: Use mmio access to context status buffer"

[Chris Wilson]

In the previous patch (to include a rmb() after readig the CSB WRITE
pointer from the HWSP) we believe we have fixed the underlying bug, and
so can re-enable using the HWSP on Cannolake.

This reverts commit 61bf9719fa17 ("drm/i915/cnl: Use mmio access to
context status buffer").

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")
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>
Tested-by: Timo Aaltonen <tjaalton@ubuntu.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 3 ---
 1 file changed, 3 deletions(-)

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;
 }
 
-- 
2.17.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/execlists: Use rmb() to order CSB reads

[Michel Thierry]

On 05/08/2018 09:30 AM, 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 its ordering requirements (B), and so
> we should use a complimentary rmb (C) to ensure that our read of its
> WRITE pointer is completed before we start accessing the data.
> 
> The final mb (D) 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
> Fixes: 767a983ab255 ("drm/i915/execlists: Read the context-status HEAD from the HWSP")
> 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>
> Tested-by: Timo Aaltonen <tjaalton@ubuntu.com>

Acked-by: Michel Thierry <michel.thierry@intel.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 911f288f78aa..8977600f0d81 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -992,6 +992,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,
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] Revert "drm/i915/cnl: Use mmio access to context status buffer"

[Michel Thierry]

On 05/08/2018 09:30 AM, Chris Wilson wrote:
> In the previous patch (to include a rmb() after readig the CSB WRITE
> pointer from the HWSP) we believe we have fixed the underlying bug, and
> so can re-enable using the HWSP on Cannolake.
> 
> This reverts commit 61bf9719fa17 ("drm/i915/cnl: Use mmio access to
> context status buffer").
> 
> 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")
> 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>
> Tested-by: Timo Aaltonen <tjaalton@ubuntu.com>

Acked-by: Michel Thierry <michel.thierry@intel.com>

> ---
>   drivers/gpu/drm/i915/intel_engine_cs.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> 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;
>   }
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/execlists: Use rmb() to order CSB reads

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/execlists: Use rmb() to order CSB reads
URL   : https://patchwork.freedesktop.org/series/42884/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
1bab7a77636f drm/i915/execlists: Use rmb() to order CSB reads
-:32: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#32: 
References: 61bf9719fa17 ("drm/i915/cnl: Use mmio access to context status buffer")

-:32: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 61bf9719fa17 ("drm/i915/cnl: Use mmio access to context status buffer")'
#32: 
References: 61bf9719fa17 ("drm/i915/cnl: Use mmio access to context status buffer")

total: 1 errors, 1 warnings, 0 checks, 7 lines checked
16cb13fc7847 Revert "drm/i915/cnl: Use mmio access to context status buffer"
-:19: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#19: 
References: 61bf9719fa17 ("drm/i915/cnl: Use mmio access to context status buffer")

-:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 61bf9719fa17 ("drm/i915/cnl: Use mmio access to context status buffer")'
#19: 
References: 61bf9719fa17 ("drm/i915/cnl: Use mmio access to context status buffer")

total: 1 errors, 1 warnings, 0 checks, 9 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/execlists: Use rmb() to order CSB reads

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/execlists: Use rmb() to order CSB reads
URL   : https://patchwork.freedesktop.org/series/42884/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4158 -> Patchwork_8948 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42884/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8948 that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-bsw-n3050:       DMESG-FAIL (fdo#106373) -> PASS

    igt@gem_exec_suspend@basic-s3:
      fi-skl-guc:         FAIL (fdo#105900, fdo#104699) -> PASS

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-4200u:       DMESG-FAIL (fdo#102614, fdo#106103) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104699 https://bugs.freedesktop.org/show_bug.cgi?id=104699
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
  fdo#106373 https://bugs.freedesktop.org/show_bug.cgi?id=106373


== Participating hosts (40 -> 37) ==

  Additional (1): fi-byt-j1900 
  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4158 -> Patchwork_8948

  CI_DRM_4158: b4cf5831333d423c2420f167111c03e4c1729672 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4465: 41eb85f918b02918787fc59d9cb5aab93b81f323 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8948: 16cb13fc78474561cf722214275a5598353c5b85 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4465: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit


== Linux commits ==

16cb13fc7847 Revert "drm/i915/cnl: Use mmio access to context status buffer"
1bab7a77636f drm/i915/execlists: Use rmb() to order CSB reads

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8948/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/execlists: Use rmb() to order CSB reads

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/execlists: Use rmb() to order CSB reads
URL   : https://patchwork.freedesktop.org/series/42884/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4158_full -> Patchwork_8948_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8948_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8948_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42884/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8948_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
      shard-kbl:          PASS -> SKIP

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-onoff:
      shard-hsw:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_8948_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_chv_cursor_fail@pipe-b-64x64-bottom-edge:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@kms_cursor_legacy@flip-vs-cursor-toggle:
      shard-hsw:          PASS -> FAIL (fdo#102670)

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#100368) +1

    igt@kms_flip@dpms-vs-vblank-race-interruptible:
      shard-glk:          PASS -> FAIL (fdo#103060)

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103060)

    igt@kms_rotation_crc@sprite-rotation-180:
      shard-snb:          PASS -> FAIL (fdo#103925, fdo#104724)
      shard-hsw:          PASS -> FAIL (fdo#103925, fdo#104724)

    igt@kms_rotation_crc@sprite-rotation-270:
      shard-apl:          PASS -> FAIL (fdo#103925, fdo#104724)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hugepages:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
      shard-hsw:          FAIL (fdo#104873) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-hsw:          FAIL (fdo#103928) -> PASS

    igt@kms_flip@absolute-wf_vblank-interruptible:
      shard-glk:          FAIL (fdo#106087) -> PASS

    igt@kms_flip@plain-flip-ts-check-interruptible:
      shard-glk:          FAIL (fdo#105312) -> PASS

    igt@kms_flip@wf_vblank-ts-check-interruptible:
      shard-kbl:          DMESG-WARN (fdo#103558, fdo#105602) -> PASS +4

    igt@kms_rotation_crc@primary-rotation-270:
      shard-apl:          FAIL (fdo#103925, fdo#104724) -> PASS +1

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_gtt:
      shard-apl:          DMESG-FAIL -> FAIL (fdo#105347)

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
  fdo#105312 https://bugs.freedesktop.org/show_bug.cgi?id=105312
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4158 -> Patchwork_8948

  CI_DRM_4158: b4cf5831333d423c2420f167111c03e4c1729672 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4465: 41eb85f918b02918787fc59d9cb5aab93b81f323 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8948: 16cb13fc78474561cf722214275a5598353c5b85 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4465: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8948/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/execlists: Use rmb() to order CSB reads

[Chris Wilson]

Quoting Chris Wilson (2018-05-08 17:30:41)
> 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 its ordering requirements (B), and so
> we should use a complimentary rmb (C) to ensure that our read of its
> WRITE pointer is completed before we start accessing the data.
> 
> The final mb (D) 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=105064

Resolved as ba74cb10c775 ("drm/i915/execlists: Delay writing to ELSP
until HW has processed the previous write")

So the other possibility is that this fixes up the issue with VT-d
latency.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/execlists: Use rmb() to order CSB reads

[Mika Kuoppala]

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

> 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 its ordering requirements (B), and so
> we should use a complimentary rmb (C) to ensure that our read of its
> WRITE pointer is completed before we start accessing the data.
>
> The final mb (D) 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
> Fixes: 767a983ab255 ("drm/i915/execlists: Read the context-status HEAD from the HWSP")
> 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>
> Tested-by: Timo Aaltonen <tjaalton@ubuntu.com>

Acked-by: Mika Kuoppala <mika.kuoppala@intel.com>

This also makes the hwsp access work fine with iommu on (kbl).

-Mika

> ---
>  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 911f288f78aa..8977600f0d81 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -992,6 +992,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,
> -- 
> 2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] Revert "drm/i915/cnl: Use mmio access to context status buffer"

[Mika Kuoppala]

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

> In the previous patch (to include a rmb() after readig the CSB WRITE
> pointer from the HWSP) we believe we have fixed the underlying bug, and
> so can re-enable using the HWSP on Cannolake.
>
> This reverts commit 61bf9719fa17 ("drm/i915/cnl: Use mmio access to
> context status buffer").
>
> 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")
> 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>
> Tested-by: Timo Aaltonen <tjaalton@ubuntu.com>

Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> 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;
>  }
>  
> -- 
> 2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx