[Intel-gfx] [PATCH 1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values

[Intel-gfx] [PATCH 1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values

[Chris Wilson]

Since we want to read the values from the HWSP as written to by the GPU,
warn the compiler that the values are volatile.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_timeline.c | 22 ++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index fb5b7d3498a6..aabd46cea511 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -491,7 +491,7 @@ checked_intel_timeline_create(struct intel_gt *gt)
 	if (IS_ERR(tl))
 		return tl;
 
-	if (*tl->hwsp_seqno != tl->seqno) {
+	if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
 		pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
 		       *tl->hwsp_seqno, tl->seqno);
 		intel_timeline_put(tl);
@@ -561,9 +561,9 @@ static int live_hwsp_engine(void *arg)
 	for (n = 0; n < count; n++) {
 		struct intel_timeline *tl = timelines[n];
 
-		if (!err && *tl->hwsp_seqno != n) {
-			pr_err("Invalid seqno stored in timeline %lu @ %x, found 0x%x\n",
-			       n, tl->hwsp_offset, *tl->hwsp_seqno);
+		if (!err && READ_ONCE(*tl->hwsp_seqno) != n) {
+			GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x, found 0x%x\n",
+			       n, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);
 			GEM_TRACE_DUMP();
 			err = -EINVAL;
 		}
@@ -633,9 +633,9 @@ static int live_hwsp_alternate(void *arg)
 	for (n = 0; n < count; n++) {
 		struct intel_timeline *tl = timelines[n];
 
-		if (!err && *tl->hwsp_seqno != n) {
-			pr_err("Invalid seqno stored in timeline %lu @ %x, found 0x%x\n",
-			       n, tl->hwsp_offset, *tl->hwsp_seqno);
+		if (!err && READ_ONCE(*tl->hwsp_seqno) != n) {
+			GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x, found 0x%x\n",
+			       n, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);
 			GEM_TRACE_DUMP();
 			err = -EINVAL;
 		}
@@ -733,7 +733,7 @@ static int live_hwsp_wrap(void *arg)
 			goto out;
 		}
 
-		if (*hwsp_seqno[0] != seqno[0] || *hwsp_seqno[1] != seqno[1]) {
+		if (READ_ONCE(*hwsp_seqno[0]) != seqno[0] || READ_ONCE(*hwsp_seqno[1]) != seqno[1]) {
 			pr_err("Bad timeline values: found (%x, %x), expected (%x, %x)\n",
 			       *hwsp_seqno[0], *hwsp_seqno[1],
 			       seqno[0], seqno[1]);
@@ -966,9 +966,9 @@ static int live_hwsp_recycle(void *arg)
 				break;
 			}
 
-			if (*tl->hwsp_seqno != count) {
-				pr_err("Invalid seqno stored in timeline %lu @ tl->hwsp_offset, found 0x%x\n",
-				       count, *tl->hwsp_seqno);
+			if (READ_ONCE(*tl->hwsp_seqno) != count) {
+				GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x found 0x%x\n",
+				       count, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);
 				GEM_TRACE_DUMP();
 				err = -EINVAL;
 			}
-- 
2.20.1

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

[Intel-gfx] [PATCH 2/2] drm/i915/gt: Wait for aux invalidation on Tigerlake

[Chris Wilson]

Add a SRM read back of the aux invalidation register after poking
hsdes: 1809175790, as failing to do so leads to writes going astray.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2169
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 31 ++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index e0280a672f1d..c9e46792b976 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -4757,14 +4757,21 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
 	intel_engine_mask_t aux_inv = 0;
 	u32 cmd, *cs;
 
+	cmd = 4;
+	if (mode & EMIT_INVALIDATE)
+		cmd += 2;
 	if (mode & EMIT_INVALIDATE)
 		aux_inv = request->engine->mask & ~BIT(BCS0);
+	if (aux_inv)
+		cmd += 2 * hweight8(aux_inv) + 6;
 
-	cs = intel_ring_begin(request,
-			      4 + (aux_inv ? 2 * hweight8(aux_inv) + 2 : 0));
+	cs = intel_ring_begin(request, cmd);
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
+	if (mode & EMIT_INVALIDATE)
+		*cs++ = preparser_disable(true);
+
 	cmd = MI_FLUSH_DW + 1;
 
 	/* We always require a command barrier so that subsequent
@@ -4780,11 +4787,6 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
 			cmd |= MI_INVALIDATE_BSD;
 	}
 
-	*cs++ = cmd;
-	*cs++ = LRC_PPHWSP_SCRATCH_ADDR;
-	*cs++ = 0; /* upper addr */
-	*cs++ = 0; /* value */
-
 	if (aux_inv) { /* hsdes: 1809175790 */
 		struct intel_engine_cs *engine;
 		unsigned int tmp;
@@ -4796,7 +4798,22 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
 			*cs++ = AUX_INV;
 		}
 		*cs++ = MI_NOOP;
+
+		*cs++ = MI_STORE_REGISTER_MEM | MI_USE_GGTT;
+		*cs++ = i915_mmio_reg_offset(aux_inv_reg(request->engine));
+		*cs++ = i915_ggtt_offset(engine->status_page.vma) +
+			I915_GEM_HWS_SCRATCH * sizeof(u32);
+		*cs++ = 0;
 	}
+
+	*cs++ = cmd;
+	*cs++ = LRC_PPHWSP_SCRATCH_ADDR;
+	*cs++ = 0; /* upper addr */
+	*cs++ = 0; /* value */
+
+	if (mode & EMIT_INVALIDATE)
+		*cs++ = preparser_disable(false);
+
 	intel_ring_advance(request, cs);
 
 	return 0;
-- 
2.20.1

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values
URL   : https://patchwork.freedesktop.org/series/79565/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
31bcd6fa4b2a drm/i915/selftests: Add compiler paranoia for checking HWSP values
-:35: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#35: FILE: drivers/gpu/drm/i915/gt/selftest_timeline.c:566:
+			GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x, found 0x%x\n",
+			       n, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);

-:48: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#48: FILE: drivers/gpu/drm/i915/gt/selftest_timeline.c:638:
+			GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x, found 0x%x\n",
+			       n, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);

-:57: WARNING:LONG_LINE: line length of 101 exceeds 100 columns
#57: FILE: drivers/gpu/drm/i915/gt/selftest_timeline.c:736:
+		if (READ_ONCE(*hwsp_seqno[0]) != seqno[0] || READ_ONCE(*hwsp_seqno[1]) != seqno[1]) {

-:70: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#70: FILE: drivers/gpu/drm/i915/gt/selftest_timeline.c:971:
+				GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x found 0x%x\n",
+				       count, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);

total: 0 errors, 1 warnings, 3 checks, 52 lines checked
58fd41068987 drm/i915/gt: Wait for aux invalidation on Tigerlake


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

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values
URL   : https://patchwork.freedesktop.org/series/79565/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.0
Fast mode used, each commit won't be checked separately.


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

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 5709 bytes --]

== Series Details ==

Series: series starting with [1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values
URL   : https://patchwork.freedesktop.org/series/79565/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8757 -> Patchwork_18195
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/index.html

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-tgl-y:           NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-tgl-y/igt@runner@aborted.html
    - fi-tgl-u2:          NOTRUN -> [FAIL][2]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-tgl-u2/igt@runner@aborted.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@runner@aborted:
    - {fi-tgl-dsi}:       NOTRUN -> [FAIL][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-tgl-dsi/igt@runner@aborted.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-icl-u2:          [PASS][4] -> [DMESG-WARN][5] ([i915#1982])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8757/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - fi-icl-y:           [DMESG-WARN][6] ([i915#1982]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8757/fi-icl-y/igt@i915_module_load@reload.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-icl-y/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@module-reload:
    - fi-cml-s:           [DMESG-WARN][8] ([i915#1982]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8757/fi-cml-s/igt@i915_pm_rpm@module-reload.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-cml-s/igt@i915_pm_rpm@module-reload.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
    - fi-icl-u2:          [DMESG-WARN][10] ([i915#1982]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8757/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [SKIP][12] ([fdo#109271]) -> [DMESG-FAIL][13] ([i915#62])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8757/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  * igt@kms_force_connector_basic@force-edid:
    - fi-kbl-x1275:       [DMESG-WARN][14] ([i915#62] / [i915#92]) -> [DMESG-WARN][15] ([i915#62] / [i915#92] / [i915#95]) +4 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8757/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-kbl-x1275:       [DMESG-WARN][16] ([i915#1982] / [i915#62] / [i915#92]) -> [DMESG-WARN][17] ([i915#62] / [i915#92])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8757/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (45 -> 40)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper 


Build changes
-------------

  * Linux: CI_DRM_8757 -> Patchwork_18195

  CI-20190529: 20190529
  CI_DRM_8757: 6802049b80a49f5f45c2bc2dd3e6d189204dc2bb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5738: bc8b56fe177af34fbde7b96f1f66614a0014c6ef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18195: 58fd410689871a4b20807aaa0c31c49baa7c2f1f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

58fd41068987 drm/i915/gt: Wait for aux invalidation on Tigerlake
31bcd6fa4b2a drm/i915/selftests: Add compiler paranoia for checking HWSP values

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18195/index.html

[-- Attachment #1.2: Type: text/html, Size: 7311 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/gt: Wait for aux invalidation on Tigerlake

[Chris Wilson]

Quoting Chris Wilson (2020-07-16 21:32:01)
> Add a SRM read back of the aux invalidation register after poking
> hsdes: 1809175790, as failing to do so leads to writes going astray.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2169
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 31 ++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index e0280a672f1d..c9e46792b976 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -4757,14 +4757,21 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>         intel_engine_mask_t aux_inv = 0;
>         u32 cmd, *cs;
>  
> +       cmd = 4;
> +       if (mode & EMIT_INVALIDATE)
> +               cmd += 2;
>         if (mode & EMIT_INVALIDATE)
>                 aux_inv = request->engine->mask & ~BIT(BCS0);
> +       if (aux_inv)
> +               cmd += 2 * hweight8(aux_inv) + 6;
>  
> -       cs = intel_ring_begin(request,
> -                             4 + (aux_inv ? 2 * hweight8(aux_inv) + 2 : 0));
> +       cs = intel_ring_begin(request, cmd);
>         if (IS_ERR(cs))
>                 return PTR_ERR(cs);
>  
> +       if (mode & EMIT_INVALIDATE)
> +               *cs++ = preparser_disable(true);
> +
>         cmd = MI_FLUSH_DW + 1;
>  
>         /* We always require a command barrier so that subsequent
> @@ -4780,11 +4787,6 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>                         cmd |= MI_INVALIDATE_BSD;
>         }
>  
> -       *cs++ = cmd;
> -       *cs++ = LRC_PPHWSP_SCRATCH_ADDR;
> -       *cs++ = 0; /* upper addr */
> -       *cs++ = 0; /* value */
> -
>         if (aux_inv) { /* hsdes: 1809175790 */
>                 struct intel_engine_cs *engine;
>                 unsigned int tmp;
> @@ -4796,7 +4798,22 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>                         *cs++ = AUX_INV;
>                 }
>                 *cs++ = MI_NOOP;
> +
> +               *cs++ = MI_STORE_REGISTER_MEM | MI_USE_GGTT;

Sigh. So I fixed this to MI_SRM_GEN8 and tgl failed the test again.

> +               *cs++ = i915_mmio_reg_offset(aux_inv_reg(request->engine));
> +               *cs++ = i915_ggtt_offset(engine->status_page.vma) +
> +                       I915_GEM_HWS_SCRATCH * sizeof(u32);
> +               *cs++ = 0;

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values

[Mika Kuoppala]

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

> Since we want to read the values from the HWSP as written to by the GPU,
> warn the compiler that the values are volatile.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Yes, for consistency (and accuracy) this should be nudged also, even
tho it is on the safe side.

    GEM_BUG_ON(i915_seqno_passed(*tl->hwsp_seqno, *seqno));

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

> ---
>  drivers/gpu/drm/i915/gt/selftest_timeline.c | 22 ++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> index fb5b7d3498a6..aabd46cea511 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> @@ -491,7 +491,7 @@ checked_intel_timeline_create(struct intel_gt *gt)
>  	if (IS_ERR(tl))
>  		return tl;
>  
> -	if (*tl->hwsp_seqno != tl->seqno) {
> +	if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
>  		pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
>  		       *tl->hwsp_seqno, tl->seqno);
>  		intel_timeline_put(tl);
> @@ -561,9 +561,9 @@ static int live_hwsp_engine(void *arg)
>  	for (n = 0; n < count; n++) {
>  		struct intel_timeline *tl = timelines[n];
>  
> -		if (!err && *tl->hwsp_seqno != n) {
> -			pr_err("Invalid seqno stored in timeline %lu @ %x, found 0x%x\n",
> -			       n, tl->hwsp_offset, *tl->hwsp_seqno);
> +		if (!err && READ_ONCE(*tl->hwsp_seqno) != n) {
> +			GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x, found 0x%x\n",
> +			       n, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);
>  			GEM_TRACE_DUMP();
>  			err = -EINVAL;
>  		}
> @@ -633,9 +633,9 @@ static int live_hwsp_alternate(void *arg)
>  	for (n = 0; n < count; n++) {
>  		struct intel_timeline *tl = timelines[n];
>  
> -		if (!err && *tl->hwsp_seqno != n) {
> -			pr_err("Invalid seqno stored in timeline %lu @ %x, found 0x%x\n",
> -			       n, tl->hwsp_offset, *tl->hwsp_seqno);
> +		if (!err && READ_ONCE(*tl->hwsp_seqno) != n) {
> +			GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x, found 0x%x\n",
> +			       n, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);
>  			GEM_TRACE_DUMP();
>  			err = -EINVAL;
>  		}
> @@ -733,7 +733,7 @@ static int live_hwsp_wrap(void *arg)
>  			goto out;
>  		}
>  
> -		if (*hwsp_seqno[0] != seqno[0] || *hwsp_seqno[1] != seqno[1]) {
> +		if (READ_ONCE(*hwsp_seqno[0]) != seqno[0] || READ_ONCE(*hwsp_seqno[1]) != seqno[1]) {
>  			pr_err("Bad timeline values: found (%x, %x), expected (%x, %x)\n",
>  			       *hwsp_seqno[0], *hwsp_seqno[1],
>  			       seqno[0], seqno[1]);
> @@ -966,9 +966,9 @@ static int live_hwsp_recycle(void *arg)
>  				break;
>  			}
>  
> -			if (*tl->hwsp_seqno != count) {
> -				pr_err("Invalid seqno stored in timeline %lu @ tl->hwsp_offset, found 0x%x\n",
> -				       count, *tl->hwsp_seqno);
> +			if (READ_ONCE(*tl->hwsp_seqno) != count) {
> +				GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x found 0x%x\n",
> +				       count, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);
>  				GEM_TRACE_DUMP();
>  				err = -EINVAL;
>  			}
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/gt: Wait for aux invalidation on Tigerlake

[Mika Kuoppala]

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

> Add a SRM read back of the aux invalidation register after poking
> hsdes: 1809175790, as failing to do so leads to writes going astray.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2169
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 31 ++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index e0280a672f1d..c9e46792b976 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -4757,14 +4757,21 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>  	intel_engine_mask_t aux_inv = 0;
>  	u32 cmd, *cs;
>  
> +	cmd = 4;
> +	if (mode & EMIT_INVALIDATE)
> +		cmd += 2;
>  	if (mode & EMIT_INVALIDATE)
>  		aux_inv = request->engine->mask & ~BIT(BCS0);
> +	if (aux_inv)
> +		cmd += 2 * hweight8(aux_inv) + 6;
>  
> -	cs = intel_ring_begin(request,
> -			      4 + (aux_inv ? 2 * hweight8(aux_inv) + 2 : 0));
> +	cs = intel_ring_begin(request, cmd);
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> +	if (mode & EMIT_INVALIDATE)
> +		*cs++ = preparser_disable(true);

This makes sense. Could be even separate patch.

On invalidate, care to try if the actual invalidate LRI
with POSTED set (after disabling preparser) could also fix this?

It feels like the posted was missing from the start.

-Mika

> +
>  	cmd = MI_FLUSH_DW + 1;
>  
>  	/* We always require a command barrier so that subsequent
> @@ -4780,11 +4787,6 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>  			cmd |= MI_INVALIDATE_BSD;
>  	}
>  
> -	*cs++ = cmd;
> -	*cs++ = LRC_PPHWSP_SCRATCH_ADDR;
> -	*cs++ = 0; /* upper addr */
> -	*cs++ = 0; /* value */
> -
>  	if (aux_inv) { /* hsdes: 1809175790 */
>  		struct intel_engine_cs *engine;
>  		unsigned int tmp;
> @@ -4796,7 +4798,22 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>  			*cs++ = AUX_INV;
>  		}
>  		*cs++ = MI_NOOP;
> +
> +		*cs++ = MI_STORE_REGISTER_MEM | MI_USE_GGTT;
> +		*cs++ = i915_mmio_reg_offset(aux_inv_reg(request->engine));
> +		*cs++ = i915_ggtt_offset(engine->status_page.vma) +
> +			I915_GEM_HWS_SCRATCH * sizeof(u32);
> +		*cs++ = 0;
>  	}
> +
> +	*cs++ = cmd;
> +	*cs++ = LRC_PPHWSP_SCRATCH_ADDR;
> +	*cs++ = 0; /* upper addr */
> +	*cs++ = 0; /* value */
> +
> +	if (mode & EMIT_INVALIDATE)
> +		*cs++ = preparser_disable(false);
> +
>  	intel_ring_advance(request, cs);
>  
>  	return 0;
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/gt: Wait for aux invalidation on Tigerlake

[Chris Wilson]

Quoting Mika Kuoppala (2020-07-17 09:30:07)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Add a SRM read back of the aux invalidation register after poking
> > hsdes: 1809175790, as failing to do so leads to writes going astray.
> >
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2169
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_lrc.c | 31 ++++++++++++++++++++++-------
> >  1 file changed, 24 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index e0280a672f1d..c9e46792b976 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -4757,14 +4757,21 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
> >       intel_engine_mask_t aux_inv = 0;
> >       u32 cmd, *cs;
> >  
> > +     cmd = 4;
> > +     if (mode & EMIT_INVALIDATE)
> > +             cmd += 2;
> >       if (mode & EMIT_INVALIDATE)
> >               aux_inv = request->engine->mask & ~BIT(BCS0);
> > +     if (aux_inv)
> > +             cmd += 2 * hweight8(aux_inv) + 6;
> >  
> > -     cs = intel_ring_begin(request,
> > -                           4 + (aux_inv ? 2 * hweight8(aux_inv) + 2 : 0));
> > +     cs = intel_ring_begin(request, cmd);
> >       if (IS_ERR(cs))
> >               return PTR_ERR(cs);
> >  
> > +     if (mode & EMIT_INVALIDATE)
> > +             *cs++ = preparser_disable(true);
> 
> This makes sense. Could be even separate patch.
> 
> On invalidate, care to try if the actual invalidate LRI
> with POSTED set (after disabling preparser) could also fix this?

I may have accidentally broke tgl1-gem and it needs some tlc ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx