[PATCH] drm/i915/execlists: Move irq state manipulation inside irq disabled region

[PATCH] drm/i915/execlists: Move irq state manipulation inside irq disabled region

[Chris Wilson]

Although this state (execlists->active and engine->irq_posted) itself is
not protected by the engine->timeline spinlock, it does conveniently
ensure that irqs are disabled. We can use this to protect our
manipulation of the state and so ensure that the next IRQ to arrive sees
consistent state and (hopefully) ignores the reset engine.

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: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index c1a3636e94fc..0482e54c94f0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1618,10 +1618,10 @@ static void reset_common_ring(struct intel_engine_cs *engine,
 	GEM_TRACE("%s seqno=%x\n",
 		  engine->name, request ? request->global_seqno : 0);
 
-	reset_irq(engine);
-
 	spin_lock_irqsave(&engine->timeline->lock, flags);
 
+	reset_irq(engine);
+
 	/*
 	 * Catch up with any missed context-switch interrupts.
 	 *
@@ -1636,11 +1636,11 @@ static void reset_common_ring(struct intel_engine_cs *engine,
 	/* Push back any incomplete requests for replay after the reset. */
 	__unwind_incomplete_requests(engine);
 
-	spin_unlock_irqrestore(&engine->timeline->lock, flags);
-
 	/* Mark all CS interrupts as complete */
 	execlists->active = 0;
 
+	spin_unlock_irqrestore(&engine->timeline->lock, flags);
+
 	/* If the request was innocent, we leave the request in the ELSP
 	 * and will try to replay it on restarting. The context image may
 	 * have been corrupted by the reset, in which case we may have
-- 
2.16.2

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

Re: [PATCH] drm/i915/execlists: Move irq state manipulation inside irq disabled region

[Mika Kuoppala]

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

> Although this state (execlists->active and engine->irq_posted) itself is
> not protected by the engine->timeline spinlock, it does conveniently
> ensure that irqs are disabled. We can use this to protect our
> manipulation of the state and so ensure that the next IRQ to arrive sees
> consistent state and (hopefully) ignores the reset engine.
>
> 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: Michel Thierry <michel.thierry@intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index c1a3636e94fc..0482e54c94f0 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1618,10 +1618,10 @@ static void reset_common_ring(struct intel_engine_cs *engine,
>  	GEM_TRACE("%s seqno=%x\n",
>  		  engine->name, request ? request->global_seqno : 0);
>  
> -	reset_irq(engine);
> -
>  	spin_lock_irqsave(&engine->timeline->lock, flags);
>  
> +	reset_irq(engine);
> +
>  	/*
>  	 * Catch up with any missed context-switch interrupts.
>  	 *
> @@ -1636,11 +1636,11 @@ static void reset_common_ring(struct intel_engine_cs *engine,
>  	/* Push back any incomplete requests for replay after the reset. */
>  	__unwind_incomplete_requests(engine);
>  
> -	spin_unlock_irqrestore(&engine->timeline->lock, flags);
> -
>  	/* Mark all CS interrupts as complete */
>  	execlists->active = 0;
>  
> +	spin_unlock_irqrestore(&engine->timeline->lock, flags);
> +
>  	/* If the request was innocent, we leave the request in the ELSP
>  	 * and will try to replay it on restarting. The context image may
>  	 * have been corrupted by the reset, in which case we may have
> -- 
> 2.16.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/execlists: Move irq state manipulation inside irq disabled region

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Move irq state manipulation inside irq disabled region
URL   : https://patchwork.freedesktop.org/series/39276/
State : success

== Summary ==

Series 39276v1 drm/i915/execlists: Move irq state manipulation inside irq disabled region
https://patchwork.freedesktop.org/api/1.0/series/39276/revisions/1/mbox/

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:417s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:420s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:371s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:477s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:277s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:475s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:482s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:475s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:460s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:391s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:568s
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:494s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:582s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:413s
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:289s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:506s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:387s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:407s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:453s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:410s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:448s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:489s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:495s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:587s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:423s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:498s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:519s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:487s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:480s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:406s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:429s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:393s

7075ab436b3bb8b97dfde3eb16b2545398938f83 drm-tip: 2018y-03m-02d-13h-04m-00s UTC integration manifest
fa87757ad944 drm/i915/execlists: Move irq state manipulation inside irq disabled region

== Logs ==

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

Re: [PATCH] drm/i915/execlists: Move irq state manipulation inside irq disabled region

[Chris Wilson]

Quoting Chris Wilson (2018-03-02 13:12:46)
> Although this state (execlists->active and engine->irq_posted) itself is
> not protected by the engine->timeline spinlock, it does conveniently
> ensure that irqs are disabled. We can use this to protect our
> manipulation of the state and so ensure that the next IRQ to arrive sees
> consistent state and (hopefully) ignores the reset engine.
> 
> 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: Michel Thierry <michel.thierry@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index c1a3636e94fc..0482e54c94f0 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1618,10 +1618,10 @@ static void reset_common_ring(struct intel_engine_cs *engine,
>         GEM_TRACE("%s seqno=%x\n",
>                   engine->name, request ? request->global_seqno : 0);
>  
> -       reset_irq(engine);
> -
>         spin_lock_irqsave(&engine->timeline->lock, flags);
>  
> +       reset_irq(engine);

Alternatively, we split this up with

local_irq_save(flags);

reset_irq(engine);

spin_lock(&engine->timline->lock);
...

Worth the exercise?
-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/execlists: Move irq state manipulation inside irq disabled region

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Move irq state manipulation inside irq disabled region
URL   : https://patchwork.freedesktop.org/series/39276/
State : success

== Summary ==

---- Known issues:

Test kms_chv_cursor_fail:
        Subgroup pipe-b-128x128-left-edge:
                dmesg-warn -> PASS       (shard-snb) fdo#105185 +1
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-pri-indfb-draw-mmap-gtt:
                pass       -> FAIL       (shard-apl) fdo#101623 +1
Test kms_plane_lowres:
        Subgroup pipe-c-tiling-yf:
                pass       -> FAIL       (shard-apl) fdo#103166
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047

fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:3463 pass:1819 dwarn:1   dfail:0   fail:10  skip:1632 time:12439s
shard-hsw        total:3463 pass:1770 dwarn:1   dfail:0   fail:1   skip:1690 time:12146s
shard-snb        total:3463 pass:1361 dwarn:2   dfail:0   fail:1   skip:2099 time:6996s
Blacklisted hosts:
shard-kbl        total:3463 pass:1944 dwarn:3   dfail:0   fail:7   skip:1509 time:9783s

== Logs ==

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