public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/guc: Clear terminated attribute bit on GuC preemption context
@ 2017-11-01 22:16 jeff.mcgee
  2017-11-01 23:29 ` Michel Thierry
  2017-11-02  8:34 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 3+ messages in thread
From: jeff.mcgee @ 2017-11-01 22:16 UTC (permalink / raw)
  To: intel-gfx

From: Jeff McGee <jeff.mcgee@intel.com>

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.

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 3049a0781b88..d14c1342f09d 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -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 |
-- 
2.14.2

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915/guc: Clear terminated attribute bit on GuC preemption context
  2017-11-01 22:16 [PATCH] drm/i915/guc: Clear terminated attribute bit on GuC preemption context jeff.mcgee
@ 2017-11-01 23:29 ` Michel Thierry
  2017-11-02  8:34 ` ✗ Fi.CI.BAT: failure for " Patchwork
  1 sibling, 0 replies; 3+ messages in thread
From: Michel Thierry @ 2017-11-01 23:29 UTC (permalink / raw)
  To: jeff.mcgee@intel.com, intel-gfx@lists.freedesktop.org

On 01/11/17 15:16, jeff.mcgee@intel.com wrote:
> From: Jeff McGee <jeff.mcgee@intel.com>
> 
> 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.
> 
> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 3049a0781b88..d14c1342f09d 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -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;
> +

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.

>          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,

Reviewed-by: Michel Thierry <michel.thierry@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/guc: Clear terminated attribute bit on GuC preemption context
  2017-11-01 22:16 [PATCH] drm/i915/guc: Clear terminated attribute bit on GuC preemption context jeff.mcgee
  2017-11-01 23:29 ` Michel Thierry
@ 2017-11-02  8:34 ` Patchwork
  1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2017-11-02  8:34 UTC (permalink / raw)
  To: jeff.mcgee; +Cc: intel-gfx

== 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-11-02  8:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-01 22:16 [PATCH] drm/i915/guc: Clear terminated attribute bit on GuC preemption context jeff.mcgee
2017-11-01 23:29 ` Michel Thierry
2017-11-02  8:34 ` ✗ Fi.CI.BAT: failure for " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox