[Intel-gfx] [PATCH] drm/i915/gt: Skip request resubmission if lite-restore w/a already applied

[Intel-gfx] [PATCH] drm/i915/gt: Skip request resubmission if lite-restore w/a already applied

[Chris Wilson]

Be careful that we never submit the same request again if we have
already applied the LiteRestore w/a upon it -- as the HW may complete
the request as we submit the ELSP and so become confused by the request
to execute an empty ring.

To submit the same request in ELSP[0] three times (so triggering the
LiteRestore w/a and resubmitting with it already applied) would require
preemption, and on preemption we should be unwinding the request tail as
we know the HW hasn't advanced beyond the normal time. So in practice,
this should never occur...

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index c7ea8a055005..2fb761063606 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1776,16 +1776,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 
 				return;
 			}
-
-			/*
-			 * WaIdleLiteRestore:bdw,skl
-			 * Apply the wa NOOPs to prevent
-			 * ring:HEAD == rq:TAIL as we resubmit the
-			 * request. See gen8_emit_fini_breadcrumb() for
-			 * where we prepare the padding after the
-			 * end of the request.
-			 */
-			last->tail = last->wa_tail;
 		}
 	}
 
@@ -1943,6 +1933,24 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 				    ctx_single_port_submission(rq->hw_context))
 					goto done;
 
+				/*
+				 * WaIdleLiteRestore:bdw,skl
+				 *
+				 * If we have already submitted this request
+				 * using the wa_tail, we race with the HW and
+				 * HEAD may reach wa_tail before it processes
+				 * the ELSP[]. If it sees a context with an
+				 * empty ring, the HW gets confused.
+				 *
+				 * To prevent subsequent resubmission (lite
+				 * restores) with an empty ring, we emitted a
+				 * couple of NOOPs in gen8_emit_wa_tail()
+				 * beyond the normal end of the request.
+				 */
+				if (unlikely(last->tail == last->wa_tail))
+					goto done;
+
+				last->tail = last->wa_tail;
 				merge = false;
 			}
 
-- 
2.24.0

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

[Intel-gfx] [PATCH] drm/i915/gt: Skip request resubmission if lite-restore w/a already applied

[Chris Wilson]

Be careful that we never submit the same request again if we have
already applied the LiteRestore w/a upon it -- as the HW may complete
the request as we submit the ELSP and so become confused by the request
to execute an empty ring.

To submit the same request in ELSP[0] three times (so triggering the
LiteRestore w/a and resubmitting with it already applied) would require
preemption, and on preemption we should be unwinding the request tail as
we know the HW hasn't advanced beyond the normal time. So in practice,
this should never occur...

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
git add fail for the GEM_WARN_ON tell-tale
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index c7ea8a055005..fcd9bb771223 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1776,16 +1776,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 
 				return;
 			}
-
-			/*
-			 * WaIdleLiteRestore:bdw,skl
-			 * Apply the wa NOOPs to prevent
-			 * ring:HEAD == rq:TAIL as we resubmit the
-			 * request. See gen8_emit_fini_breadcrumb() for
-			 * where we prepare the padding after the
-			 * end of the request.
-			 */
-			last->tail = last->wa_tail;
 		}
 	}
 
@@ -1943,6 +1933,24 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 				    ctx_single_port_submission(rq->hw_context))
 					goto done;
 
+				/*
+				 * WaIdleLiteRestore:bdw,skl
+				 *
+				 * If we have already submitted this request
+				 * using the wa_tail, we race with the HW and
+				 * HEAD may reach wa_tail before it processes
+				 * the ELSP[]. If it sees a context with an
+				 * empty ring, the HW gets confused.
+				 *
+				 * To prevent subsequent resubmission (lite
+				 * restores) with an empty ring, we emitted a
+				 * couple of NOOPs in gen8_emit_wa_tail()
+				 * beyond the normal end of the request.
+				 */
+				if (GEM_WARN_ON(last->tail == last->wa_tail))
+					goto done;
+
+				last->tail = last->wa_tail;
 				merge = false;
 			}
 
-- 
2.24.0

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

Re: [Intel-gfx] [PATCH] drm/i915/gt: Skip request resubmission if lite-restore w/a already applied

[Chris Wilson]

Quoting Chris Wilson (2019-12-08 23:33:08)
> Be careful that we never submit the same request again if we have
> already applied the LiteRestore w/a upon it -- as the HW may complete
> the request as we submit the ELSP and so become confused by the request
> to execute an empty ring.
> 
> To submit the same request in ELSP[0] three times (so triggering the
> LiteRestore w/a and resubmitting with it already applied) would require
> preemption, and on preemption we should be unwinding the request tail as
> we know the HW hasn't advanced beyond the normal time. So in practice,
> this should never occur...
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> git add fail for the GEM_WARN_ON tell-tale
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index c7ea8a055005..fcd9bb771223 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1776,16 +1776,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  
>                                 return;
>                         }
> -
> -                       /*
> -                        * WaIdleLiteRestore:bdw,skl
> -                        * Apply the wa NOOPs to prevent
> -                        * ring:HEAD == rq:TAIL as we resubmit the
> -                        * request. See gen8_emit_fini_breadcrumb() for
> -                        * where we prepare the padding after the
> -                        * end of the request.
> -                        */
> -                       last->tail = last->wa_tail;
>                 }
>         }
>  
> @@ -1943,6 +1933,24 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>                                     ctx_single_port_submission(rq->hw_context))
>                                         goto done;
>  
> +                               /*
> +                                * WaIdleLiteRestore:bdw,skl
> +                                *
> +                                * If we have already submitted this request
> +                                * using the wa_tail, we race with the HW and
> +                                * HEAD may reach wa_tail before it processes
> +                                * the ELSP[]. If it sees a context with an
> +                                * empty ring, the HW gets confused.
> +                                *
> +                                * To prevent subsequent resubmission (lite
> +                                * restores) with an empty ring, we emitted a
> +                                * couple of NOOPs in gen8_emit_wa_tail()
> +                                * beyond the normal end of the request.
> +                                */
> +                               if (GEM_WARN_ON(last->tail == last->wa_tail))
> +                                       goto done;
> +
> +                               last->tail = last->wa_tail;

Bah, this means we update the tail before the current submission, so
while the warn is meaningful, the ELSP[] is b0rked.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Skip request resubmission if lite-restore w/a already applied

[Patchwork]

== Series Details ==

Series: drm/i915/gt: Skip request resubmission if lite-restore w/a already applied
URL   : https://patchwork.freedesktop.org/series/70602/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7511 -> Patchwork_15647
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15647 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15647, 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_15647/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@debugfs_test@read_all_entries:
    - fi-ivb-3770:        NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15647/fi-ivb-3770/igt@debugfs_test@read_all_entries.html

  * igt@gem_ctx_create@basic:
    - fi-bsw-kefka:       [PASS][2] -> [INCOMPLETE][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-bsw-kefka/igt@gem_ctx_create@basic.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15647/fi-bsw-kefka/igt@gem_ctx_create@basic.html

  * igt@gem_exec_parallel@basic:
    - fi-hsw-4770:        NOTRUN -> [INCOMPLETE][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15647/fi-hsw-4770/igt@gem_exec_parallel@basic.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-snb-2600:        [PASS][5] -> [INCOMPLETE][6] ([i915#82])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-snb-2600/igt@gem_ctx_create@basic-files.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15647/fi-snb-2600/igt@gem_ctx_create@basic-files.html

  
#### Possible fixes ####

  * igt@core_auth@basic-auth:
    - fi-pnv-d510:        [INCOMPLETE][7] ([i915#299]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-pnv-d510/igt@core_auth@basic-auth.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15647/fi-pnv-d510/igt@core_auth@basic-auth.html

  * igt@gem_close_race@basic-threads:
    - fi-hsw-4770:        [INCOMPLETE][9] ([i915#435]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-hsw-4770/igt@gem_close_race@basic-threads.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15647/fi-hsw-4770/igt@gem_close_race@basic-threads.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-byt-n2820:       [INCOMPLETE][11] ([i915#45]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15647/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html

  
  [i915#299]: https://gitlab.freedesktop.org/drm/intel/issues/299
  [i915#435]: https://gitlab.freedesktop.org/drm/intel/issues/435
  [i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82


Participating hosts (26 -> 20)
------------------------------

  Additional (5): fi-hsw-peppy fi-byt-j1900 fi-gdg-551 fi-kbl-8809g fi-blb-e6850 
  Missing    (11): fi-ilk-m540 fi-bxt-dsi fi-glk-dsi fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-kbl-guc fi-ctg-p8600 fi-elk-e7500 fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7511 -> Patchwork_15647

  CI-20190529: 20190529
  CI_DRM_7511: 04ca2c0fb47c3d394a364f5f71943ca9f440dda7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5339: d45e11f8e10a4c5775372002ec89974ce3d97b27 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15647: 220008fe81b0bae610b3a496a9dd329715fb2933 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

220008fe81b0 drm/i915/gt: Skip request resubmission if lite-restore w/a already applied

== Logs ==

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Skip request resubmission if lite-restore w/a already applied (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/gt: Skip request resubmission if lite-restore w/a already applied (rev2)
URL   : https://patchwork.freedesktop.org/series/70602/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7511 -> Patchwork_15648
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15648 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15648, 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_15648/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_ctx_create@basic-files:
    - fi-bdw-5557u:       NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15648/fi-bdw-5557u/igt@gem_ctx_create@basic-files.html

  * igt@i915_selftest@live_execlists:
    - fi-kbl-soraka:      [PASS][2] -> [DMESG-WARN][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-kbl-soraka/igt@i915_selftest@live_execlists.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15648/fi-kbl-soraka/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_hangcheck:
    - fi-bsw-kefka:       [PASS][4] -> [DMESG-WARN][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-bsw-kefka/igt@i915_selftest@live_hangcheck.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15648/fi-bsw-kefka/igt@i915_selftest@live_hangcheck.html
    - fi-glk-dsi:         [PASS][6] -> [DMESG-WARN][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-glk-dsi/igt@i915_selftest@live_hangcheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15648/fi-glk-dsi/igt@i915_selftest@live_hangcheck.html

  * igt@runner@aborted:
    - fi-kbl-soraka:      NOTRUN -> [FAIL][8]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15648/fi-kbl-soraka/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@read_all_entries:
    - fi-bxt-dsi:         [PASS][9] -> [INCOMPLETE][10] ([fdo#103927])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-bxt-dsi/igt@debugfs_test@read_all_entries.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15648/fi-bxt-dsi/igt@debugfs_test@read_all_entries.html

  
#### Possible fixes ####

  * igt@core_auth@basic-auth:
    - fi-pnv-d510:        [INCOMPLETE][11] ([i915#299]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-pnv-d510/igt@core_auth@basic-auth.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15648/fi-pnv-d510/igt@core_auth@basic-auth.html

  * igt@debugfs_test@read_all_entries:
    - fi-apl-guc:         [INCOMPLETE][13] ([fdo#103927]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-apl-guc/igt@debugfs_test@read_all_entries.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15648/fi-apl-guc/igt@debugfs_test@read_all_entries.html

  * igt@gem_close_race@basic-threads:
    - fi-hsw-4770:        [INCOMPLETE][15] ([i915#435]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7511/fi-hsw-4770/igt@gem_close_race@basic-threads.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15648/fi-hsw-4770/igt@gem_close_race@basic-threads.html

  
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [i915#299]: https://gitlab.freedesktop.org/drm/intel/issues/299
  [i915#435]: https://gitlab.freedesktop.org/drm/intel/issues/435


Participating hosts (26 -> 21)
------------------------------

  Additional (4): fi-byt-j1900 fi-blb-e6850 fi-gdg-551 fi-hsw-peppy 
  Missing    (9): fi-ilk-m540 fi-bsw-n3050 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bsw-nick fi-bdw-samus fi-snb-2600 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7511 -> Patchwork_15648

  CI-20190529: 20190529
  CI_DRM_7511: 04ca2c0fb47c3d394a364f5f71943ca9f440dda7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5339: d45e11f8e10a4c5775372002ec89974ce3d97b27 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15648: cb7d326696f60603d3c132ba51d2bbb731979348 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

cb7d326696f6 drm/i915/gt: Skip request resubmission if lite-restore w/a already applied

== Logs ==

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