Skip to contentSkip to conversation

drm/i915/guc: Clear terminated attribute bit on GuC preemption context

PATCH In review [email protected] 3 participants 3 messages Nov 1, 2017 – Nov 2, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/i915/guc: Clear terminated attribute bit on GuC preemption context

From: Jeff McGee <[email protected]>

If GuC firmware performs an engine reset while that engine had a preemption pending, it will set the terminated attribute bit on our preemption stage descriptor. GuC firmware retains all pending work items for a high-priority GuC client, unlike the normal-priority GuC client where work items are dropped. It wants to make sure the preempt- to-idle work doesn't run when scheduling resumes, and uses this bit to inform its scheduler and presumably us as well. Our job is to clear it for the next preemption after reset, otherwise that and future preemptions will never complete. We'll just clear it every time.

---
 drivers/gpu/drm/i915/i915_guc_submission.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
drivers/gpu/drm/i915/i915_guc_submission.c +15 -0
index 3049a0781b88..d14c1342f09d 100644
@@ -590,6 +590,7 @@ static void inject_preempt_context(struct work_struct *work)
 	struct intel_guc *guc = container_of(preempt_work, typeof(*guc),
 					     preempt_work[engine->id]);
 	struct i915_guc_client *client = guc->preempt_client;
+	struct guc_stage_desc *stage_desc = __get_stage_desc(client);
 	struct intel_ring *ring = client->owner->engine[engine->id].ring;
 	u32 ctx_desc = lower_32_bits(intel_lr_context_descriptor(client->owner,
 								 engine));
@@ -623,6 +624,20 @@ static void inject_preempt_context(struct work_struct *work)
 			   ring->tail / sizeof(u64), 0);
 	spin_unlock_irq(&client->wq_lock);
 
+	/*
+	 * If GuC firmware performs an engine reset while that engine had
+	 * a preemption pending, it will set the terminated attribute bit
+	 * on our preemption stage descriptor. GuC firmware retains all
+	 * pending work items for a high-priority GuC client, unlike the
+	 * normal-priority GuC client where work items are dropped. It
+	 * wants to make sure the preempt-to-idle work doesn't run when
+	 * scheduling resumes, and uses this bit to inform its scheduler
+	 * and presumably us as well. Our job is to clear it for the next
+	 * preemption after reset, otherwise that and future preemptions
+	 * will never complete. We'll just clear it every time.
+	 */
+	stage_desc->attribute &= ~GUC_STAGE_DESC_ATTR_TERMINATED;
+
 	data[0] = INTEL_GUC_ACTION_REQUEST_PREEMPTION;
 	data[1] = client->stage_id;
 	data[2] = INTEL_GUC_PREEMPT_OPTION_DROP_WORK_Q |
Signature · 6 lines
2.14.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 1/1
#1 lore raw
Quoted 37 lines · On 01/11/17 15:16, [email protected] wrote:

From: Jeff McGee <[email protected]>

If GuC firmware performs an engine reset while that engine had a preemption pending, it will set the terminated attribute bit on our preemption stage descriptor. GuC firmware retains all pending work items for a high-priority GuC client, unlike the normal-priority GuC client where work items are dropped. It wants to make sure the preempt- to-idle work doesn't run when scheduling resumes, and uses this bit to inform its scheduler and presumably us as well. Our job is to clear it for the next preemption after reset, otherwise that and future preemptions will never complete. We'll just clear it every time.

---
  drivers/gpu/drm/i915/i915_guc_submission.c | 15 +++++++++++++++
  1 file changed, 15 insertions(+)
drivers/gpu/drm/i915/i915_guc_submission.c +5 -0
index 3049a0781b88..d14c1342f09d 100644
@@ -590,6 +590,7 @@ static void inject_preempt_context(struct work_struct *work)
         struct intel_guc *guc = container_of(preempt_work, typeof(*guc),
                                              preempt_work[engine->id]);
         struct i915_guc_client *client = guc->preempt_client;
+       struct guc_stage_desc *stage_desc = __get_stage_desc(client);
         struct intel_ring *ring = client->owner->engine[engine->id].ring;
         u32 ctx_desc = lower_32_bits(intel_lr_context_descriptor(client->owner,
                                                                  engine));
@@ -623,6 +624,20 @@ static void inject_preempt_context(struct work_struct *work)
                            ring->tail / sizeof(u64), 0);
         spin_unlock_irq(&client->wq_lock);
 
+       /*
+        * If GuC firmware performs an engine reset while that engine had
+        * a preemption pending, it will set the terminated attribute bit
+        * on our preemption stage descriptor. GuC firmware retains all

drivers/gpu/drm/i915/i915_guc_submission.c static void inject_preempt_context(struct work_struct *work) · line 640

Quoted 10 lines
drivers/gpu/drm/i915/i915_guc_submission.c +10 -0
+        * pending work items for a high-priority GuC client, unlike the
+        * normal-priority GuC client where work items are dropped. It
+        * wants to make sure the preempt-to-idle work doesn't run when
+        * scheduling resumes, and uses this bit to inform its scheduler
+        * and presumably us as well. Our job is to clear it for the next
+        * preemption after reset, otherwise that and future preemptions
+        * will never complete. We'll just clear it every time.
+        */
+       stage_desc->attribute &= ~GUC_STAGE_DESC_ATTR_TERMINATED;
+

I looked around and yes, the firmware will set the terminated bit in the stage_desc of the preempt client if it had a pending preemption request. No harm in clearing it unconditionally.

Quoted 5 lines
drivers/gpu/drm/i915/i915_guc_submission.c +0 -1
         data[0] = INTEL_GUC_ACTION_REQUEST_PREEMPTION;
         data[1] = client->stage_id;
         data[2] = INTEL_GUC_PREEMPT_OPTION_DROP_WORK_Q |
--

2.14.2

Since Michał is not around,

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#2 lore raw

✗ Fi.CI.BAT: failure for drm/i915/guc: Clear terminated attribute bit on GuC preemption context

In reply to [email protected]
== Series Details ==

== Series Details ==

Series: drm/i915/guc: Clear terminated attribute bit on GuC preemption context
URL : https://patchwork.freedesktop.org/series/33013/
State : failure

== Summary ==

Series 33013v1 drm/i915/guc: Clear terminated attribute bit on GuC preemption context
https://patchwork.freedesktop.org/api/1.0/series/33013/revisions/1/mbox/

Test chamelium:

        Subgroup dp-hpd-fast:
                skip       -> INCOMPLETE (fi-bdw-5557u)
fi-bdw-5557u     total:289  pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:461s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:378s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:536s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:279s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:508s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:507s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:507s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:490s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:562s
fi-cnl-y         total:217  pass:196  dwarn:0   dfail:0   fail:0   skip:20 
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:433s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:261s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:583s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:494s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:429s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:427s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:433s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:494s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:468s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:494s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:576s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:480s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:588s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:568s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:598s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:653s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:521s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:501s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:460s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:423s

db0e5dd56ceaf2b071b7f4bc749445588c1d5812 drm-tip: 2017y-11m-01d-21h-18m-43s UTC integration manifest d0992e9d73f0 drm/i915/guc: Clear terminated attribute bit on GuC preemption context

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6917/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx