[PATCH v4] drm/i915/execlists: Always clear preempt status on cancelling all

[PATCH v4] drm/i915/execlists: Always clear preempt status on cancelling all

[Chris Wilson]

On reset/wedging, we cancel all pending replies from the HW and we also
want to cancel an outstanding preemption event. Since we use the same
function to cancel the pending replies for reset and for a preemption
event, we can simply clear the active tracking for all.

v2: Keep execlists_user_end() markup for wedging
v3: Move assignment to inline to hide the bare assignment.
v4: Alternate version, use user_end for clearing all leftovers

Fixes: 60a943245413 ("drm/i915/execlists: Drop clear_gtiir() on GPU reset")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        | 2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6fef9d130d55..f9fb32efb97f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -388,7 +388,7 @@ execlists_user_begin(struct intel_engine_execlists *execlists,
 inline void
 execlists_user_end(struct intel_engine_execlists *execlists)
 {
-	execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
+	execlists_clear_all_active(execlists);
 }
 
 static inline void
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d1eee08e5f6b..665b59ba1f45 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -683,6 +683,12 @@ execlists_clear_active(struct intel_engine_execlists *execlists,
 	__clear_bit(bit, (unsigned long *)&execlists->active);
 }
 
+static inline void
+execlists_clear_all_active(struct intel_engine_execlists *execlists)
+{
+	execlists->active = 0;
+}
+
 static inline bool
 execlists_is_active(const struct intel_engine_execlists *execlists,
 		    unsigned int bit)
-- 
2.18.0

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

Re: [PATCH v4] drm/i915/execlists: Always clear preempt status on cancelling all

[Tvrtko Ursulin]

On 16/07/2018 13:56, Chris Wilson wrote:
> On reset/wedging, we cancel all pending replies from the HW and we also
> want to cancel an outstanding preemption event. Since we use the same
> function to cancel the pending replies for reset and for a preemption
> event, we can simply clear the active tracking for all.
> 
> v2: Keep execlists_user_end() markup for wedging
> v3: Move assignment to inline to hide the bare assignment.
> v4: Alternate version, use user_end for clearing all leftovers
> 
> Fixes: 60a943245413 ("drm/i915/execlists: Drop clear_gtiir() on GPU reset")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c        | 2 +-
>   drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++++++
>   2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 6fef9d130d55..f9fb32efb97f 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -388,7 +388,7 @@ execlists_user_begin(struct intel_engine_execlists *execlists,
>   inline void
>   execlists_user_end(struct intel_engine_execlists *execlists)
>   {
> -	execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
> +	execlists_clear_all_active(execlists);

Doesn't seem right to break separation of flag domains.

Regards,

Tvrtko

>   }
>   
>   static inline void
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index d1eee08e5f6b..665b59ba1f45 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -683,6 +683,12 @@ execlists_clear_active(struct intel_engine_execlists *execlists,
>   	__clear_bit(bit, (unsigned long *)&execlists->active);
>   }
>   
> +static inline void
> +execlists_clear_all_active(struct intel_engine_execlists *execlists)
> +{
> +	execlists->active = 0;
> +}
> +
>   static inline bool
>   execlists_is_active(const struct intel_engine_execlists *execlists,
>   		    unsigned int bit)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4] drm/i915/execlists: Always clear preempt status on cancelling all

[Chris Wilson]

Quoting Chris Wilson (2018-07-16 13:56:46)
> On reset/wedging, we cancel all pending replies from the HW and we also
> want to cancel an outstanding preemption event. Since we use the same
> function to cancel the pending replies for reset and for a preemption
> event, we can simply clear the active tracking for all.
> 
> v2: Keep execlists_user_end() markup for wedging
> v3: Move assignment to inline to hide the bare assignment.
> v4: Alternate version, use user_end for clearing all leftovers
> 
> Fixes: 60a943245413 ("drm/i915/execlists: Drop clear_gtiir() on GPU reset")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c        | 2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 6fef9d130d55..f9fb32efb97f 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -388,7 +388,7 @@ execlists_user_begin(struct intel_engine_execlists *execlists,
>  inline void
>  execlists_user_end(struct intel_engine_execlists *execlists)
>  {
> -       execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
> +       execlists_clear_all_active(execlists);

While the doing a simple assignment is nice, we lose that direct link
with user == ACTIVE_USER.

On the whole, I think it's acceptable to use assignment here rather than
clearing the single bit.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915/execlists: Always clear preempt status on cancelling all (rev7)

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Always clear preempt status on cancelling all (rev7)
URL   : https://patchwork.freedesktop.org/series/46528/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4493 -> Patchwork_9674 =

== Summary - FAILURE ==

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_hangcheck:
      fi-cfl-8700k:       PASS -> INCOMPLETE
      {fi-cfl-8109u}:     PASS -> INCOMPLETE
      fi-cfl-s3:          PASS -> INCOMPLETE
      fi-whl-u:           PASS -> INCOMPLETE

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

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

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7500u:       PASS -> INCOMPLETE (fdo#107207)
      fi-kbl-7560u:       PASS -> INCOMPLETE (fdo#107207)
      fi-skl-6700hq:      NOTRUN -> INCOMPLETE (fdo#107207)
      fi-skl-gvtdvm:      PASS -> INCOMPLETE (fdo#107207, fdo#105600)
      fi-skl-6700k2:      PASS -> INCOMPLETE (fdo#107207)
      fi-cfl-guc:         PASS -> INCOMPLETE (fdo#106693)
      fi-skl-guc:         PASS -> INCOMPLETE (fdo#107207, fdo#106693)
      fi-skl-6600u:       PASS -> INCOMPLETE (fdo#107207)
      fi-kbl-r:           PASS -> INCOMPLETE (fdo#107207)
      fi-kbl-guc:         PASS -> DMESG-FAIL (fdo#106560, fdo#106947)
      fi-skl-6770hq:      PASS -> INCOMPLETE (fdo#107207)
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)
      fi-bxt-j4205:       PASS -> INCOMPLETE (fdo#103927)
      fi-skl-6260u:       PASS -> INCOMPLETE (fdo#107207)
      fi-glk-j4005:       PASS -> INCOMPLETE (fdo#107207, k.org#198133, fdo#103359)
      fi-kbl-x1275:       PASS -> INCOMPLETE (fdo#107207)
      fi-kbl-7567u:       PASS -> INCOMPLETE (fdo#107207)
      fi-glk-dsi:         PASS -> INCOMPLETE (fdo#107207)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-bdw-5557u:       DMESG-FAIL (fdo#106560) -> PASS

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

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

  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105600 https://bugs.freedesktop.org/show_bug.cgi?id=105600
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107207 https://bugs.freedesktop.org/show_bug.cgi?id=107207
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (44 -> 41) ==

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


== Build changes ==

    * Linux: CI_DRM_4493 -> Patchwork_9674

  CI_DRM_4493: c69b4c1274cccaa270c1e4daa68228724c80603a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4558: d8e97e1710b27a3931a1c53d1dd88c0e709c085b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9674: d81d827a9d1d593800530e3ee64bda9c3b006477 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d81d827a9d1d drm/i915/execlists: Always clear preempt status on cancelling all

== Logs ==

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