[PATCH] drm/i915: Kick waiters on resetting legacy rings

[PATCH] drm/i915: Kick waiters on resetting legacy rings

[Chris Wilson]

This reapplies commit 39f3be162c46 ("drm/i915: Kick waiters on resetting
legacy rings") after the improved gem_eio was run across all machines we
found that gen3 and early gen4 still lost the immediate interrupt
following reset, and the HWSTAM w/a applied to gen6+ is inadequate.

Unlike the later gen, on gen3/4 the principle (and only tests to fail so
far) are the wait vs reset test cases, whereas the reset stress case
works fine (which was the predominantly failing case for gen6+). That is
enough to suggest the underlying issue is sufficiently different to
support the difference in HWSTAM efficacy.

Testcase: igt/gem_eio/wait-10ms
References: 39f3be162c46 ("drm/i915: Kick waiters on resetting legacy rings")
References: a69ab52b0358 ("drm/i915: Remove extra waiter kick on legacy resets")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b65cf7832b39..d40f55a8dc34 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -537,6 +537,8 @@ static int init_ring_common(struct intel_engine_cs *engine)
 	if (INTEL_GEN(dev_priv) > 2)
 		I915_WRITE_MODE(engine, _MASKED_BIT_DISABLE(STOP_RING));
 
+	/* Papering over lost _interrupts_ immediately following the restart */
+	intel_engine_wakeup(engine);
 out:
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 
-- 
2.18.0

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Kick waiters on resetting legacy rings (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Kick waiters on resetting legacy rings (rev2)
URL   : https://patchwork.freedesktop.org/series/47362/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f715468550d1 drm/i915: Kick waiters on resetting legacy rings
-:18: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#18: 
References: 39f3be162c46 ("drm/i915: Kick waiters on resetting legacy rings")

-:18: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 39f3be162c46 ("drm/i915: Kick waiters on resetting legacy rings")'
#18: 
References: 39f3be162c46 ("drm/i915: Kick waiters on resetting legacy rings")

-:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit a69ab52b0358 ("drm/i915: Remove extra waiter kick on legacy resets")'
#19: 
References: a69ab52b0358 ("drm/i915: Remove extra waiter kick on legacy resets")

total: 2 errors, 1 warnings, 0 checks, 8 lines checked

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

✓ Fi.CI.BAT: success for drm/i915: Kick waiters on resetting legacy rings (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Kick waiters on resetting legacy rings (rev2)
URL   : https://patchwork.freedesktop.org/series/47362/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4664 -> Patchwork_9936 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47362/revisions/2/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_workarounds:
      fi-cnl-psr:         PASS -> DMESG-FAIL (fdo#107292)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         DMESG-FAIL (fdo#107164) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         DMESG-FAIL (fdo#106685, fdo#107174) -> PASS

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

    igt@kms_chamelium@dp-crc-fast:
      fi-kbl-7500u:       DMESG-FAIL (fdo#103841) -> PASS

    
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106685 https://bugs.freedesktop.org/show_bug.cgi?id=106685
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292


== Participating hosts (54 -> 49) ==

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


== Build changes ==

    * Linux: CI_DRM_4664 -> Patchwork_9936

  CI_DRM_4664: 19e458884fe1d8d10e453529933199250cc8821f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9936: f715468550d11fafc82dda24553e61664bae43ce @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f715468550d1 drm/i915: Kick waiters on resetting legacy rings

== Logs ==

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

Re: [PATCH] drm/i915: Kick waiters on resetting legacy rings

[Mika Kuoppala]

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

> This reapplies commit 39f3be162c46 ("drm/i915: Kick waiters on resetting
> legacy rings") after the improved gem_eio was run across all machines we
> found that gen3 and early gen4 still lost the immediate interrupt
> following reset, and the HWSTAM w/a applied to gen6+ is inadequate.
>
> Unlike the later gen, on gen3/4 the principle (and only tests to fail so
> far) are the wait vs reset test cases, whereas the reset stress case
> works fine (which was the predominantly failing case for gen6+). That is
> enough to suggest the underlying issue is sufficiently different to
> support the difference in HWSTAM efficacy.
>
> Testcase: igt/gem_eio/wait-10ms
> References: 39f3be162c46 ("drm/i915: Kick waiters on resetting legacy rings")
> References: a69ab52b0358 ("drm/i915: Remove extra waiter kick on legacy resets")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index b65cf7832b39..d40f55a8dc34 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -537,6 +537,8 @@ static int init_ring_common(struct intel_engine_cs *engine)
>  	if (INTEL_GEN(dev_priv) > 2)
>  		I915_WRITE_MODE(engine, _MASKED_BIT_DISABLE(STOP_RING));
>  
> +	/* Papering over lost _interrupts_ immediately following the restart */
> +	intel_engine_wakeup(engine);

We came, we toggled, we learned. Onwards!

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

>  out:
>  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>  
> -- 
> 2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Kick waiters on resetting legacy rings

[Chris Wilson]

Quoting Mika Kuoppala (2018-08-14 12:38:18)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > This reapplies commit 39f3be162c46 ("drm/i915: Kick waiters on resetting
> > legacy rings") after the improved gem_eio was run across all machines we
> > found that gen3 and early gen4 still lost the immediate interrupt
> > following reset, and the HWSTAM w/a applied to gen6+ is inadequate.
> >
> > Unlike the later gen, on gen3/4 the principle (and only tests to fail so
> > far) are the wait vs reset test cases, whereas the reset stress case
> > works fine (which was the predominantly failing case for gen6+). That is
> > enough to suggest the underlying issue is sufficiently different to
> > support the difference in HWSTAM efficacy.
> >
> > Testcase: igt/gem_eio/wait-10ms
> > References: 39f3be162c46 ("drm/i915: Kick waiters on resetting legacy rings")
> > References: a69ab52b0358 ("drm/i915: Remove extra waiter kick on legacy resets")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index b65cf7832b39..d40f55a8dc34 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -537,6 +537,8 @@ static int init_ring_common(struct intel_engine_cs *engine)
> >       if (INTEL_GEN(dev_priv) > 2)
> >               I915_WRITE_MODE(engine, _MASKED_BIT_DISABLE(STOP_RING));
> >  
> > +     /* Papering over lost _interrupts_ immediately following the restart */
> > +     intel_engine_wakeup(engine);
> 
> We came, we toggled, we learned. Onwards!

The pessimist in me says if this again boils down to a timing bug, we
need to find another w/a. It's a shame that HWSTAM wasn't it (even
remembering the bit differences between gen, as a last resort I tested
with HWSTAM==0 i.e. no mask).

Until then, ignorance is bliss.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Kick waiters on resetting legacy rings (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Kick waiters on resetting legacy rings (rev2)
URL   : https://patchwork.freedesktop.org/series/47362/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4664_full -> Patchwork_9936_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-snb:          FAIL (fdo#106886) -> PASS
      shard-glk:          INCOMPLETE (fdo#103359, k.org#198133, fdo#106886) -> PASS

    igt@perf@polling:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4664 -> Patchwork_9936

  CI_DRM_4664: 19e458884fe1d8d10e453529933199250cc8821f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9936: f715468550d11fafc82dda24553e61664bae43ce @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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