[PATCH 2/2] HAX enable guc submission for CI

[PATCH 2/2] HAX enable guc submission for CI

[Chris Wilson]

---
 drivers/gpu/drm/i915/i915_params.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b6a7e363d076..9dcc8a0dd3c0 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 1,
+	.enable_guc_submission = 1,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
-- 
2.11.0

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

[PATCH 2/2] HAX Enable GuC Submission for CI

[Michał Winiarski]

---
 drivers/gpu/drm/i915/i915_params.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8ab003dca113..c9d72f1b8383 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 2,
+	.enable_guc_submission = 2,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
-- 
2.13.5

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

[PATCH 2/2] HAX enable guc submission for CI

[Jackie Li]

Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.h  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0684d5d..a351ddf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3562,17 +3562,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c729226..c38cef0 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -44,8 +44,8 @@
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc_loading, 0) \
-	param(int, enable_guc_submission, 0) \
+	param(int, enable_guc_loading, 1) \
+	param(int, enable_guc_submission, 1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
-- 
2.7.4

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

[PATCH 2/2] HAX enable guc submission for CI

[Jackie Li]

Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.h  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0684d5d..a351ddf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3562,17 +3562,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c729226..c38cef0 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -44,8 +44,8 @@
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc_loading, 0) \
-	param(int, enable_guc_submission, 0) \
+	param(int, enable_guc_loading, 1) \
+	param(int, enable_guc_submission, 1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
-- 
2.7.4

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

[PATCH 2/2] HAX Enable GuC Submission for CI

[Chris Wilson]

From: Michał Winiarski <michal.winiarski@intel.com>

Also:
Revert "drm/i915/guc: Assert that we switch between known ggtt->invalidate functions"

This reverts commit 04f7b24eccdfae680a36e9825fe0d61dcd5ed528.
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.h  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 41a203e0c160..5e8614fd390b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3590,17 +3590,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 8321bd86cba5..69ee2484c6bb 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -42,8 +42,8 @@
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc_loading, 0) \
-	param(int, enable_guc_submission, 0) \
+	param(int, enable_guc_loading, 1) \
+	param(int, enable_guc_submission, 1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
-- 
2.15.0

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

[PATCH 1/2] drm/i915/guc: Fill preempt context once at init time

[Michał Winiarski]

Since we're inhibiting context save of preempt context, we're no longer
tracking the position of HEAD/TAIL. With GuC, we're adding a new
breadcrumb for each preemption, which means that the HW will do more and
more breadcrumb writes. Eventually the ring is filled, and we're
submitting the preemption context with HEAD==TAIL==0, which won't result
in breadcrumb write, but will trigger hangcheck instead.
Instead of writing a new preempt breadcrumb for each preemption, let's
just fill the ring once at init time (which also saves a couple of
instructions in the tasklet).

Fixes: 517aaffe0c1b ("drm/i915/execlists: Inhibit context save/restore for the fake preempt context")
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 68 +++++++++++++++++------------
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 586dde579903..89e5b036061d 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -28,6 +28,10 @@
 #include "intel_guc_submission.h"
 #include "i915_drv.h"
 
+#define GUC_PREEMPT_FINISHED		0x1
+#define GUC_PREEMPT_BREADCRUMB_DWORDS	0x8
+#define GUC_PREEMPT_BREADCRUMB_BYTES	(sizeof(u32) * GUC_PREEMPT_BREADCRUMB_DWORDS)
+
 /**
  * DOC: GuC-based command submission
  *
@@ -535,8 +539,6 @@ static void flush_ggtt_writes(struct i915_vma *vma)
 		POSTING_READ_FW(GUC_STATUS);
 }
 
-#define GUC_PREEMPT_FINISHED 0x1
-#define GUC_PREEMPT_BREADCRUMB_DWORDS 0x8
 static void inject_preempt_context(struct work_struct *work)
 {
 	struct guc_preempt_work *preempt_work =
@@ -546,37 +548,13 @@ static void inject_preempt_context(struct work_struct *work)
 					     preempt_work[engine->id]);
 	struct intel_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));
-	u32 *cs = ring->vaddr + ring->tail;
 	u32 data[7];
 
-	if (engine->id == RCS) {
-		cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED,
-				intel_hws_preempt_done_address(engine));
-	} else {
-		cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED,
-				intel_hws_preempt_done_address(engine));
-		*cs++ = MI_NOOP;
-		*cs++ = MI_NOOP;
-	}
-	*cs++ = MI_USER_INTERRUPT;
-	*cs++ = MI_NOOP;
-
-	GEM_BUG_ON(!IS_ALIGNED(ring->size,
-			       GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32)));
-	GEM_BUG_ON((void *)cs - (ring->vaddr + ring->tail) !=
-		   GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32));
-
-	ring->tail += GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32);
-	ring->tail &= (ring->size - 1);
-
-	flush_ggtt_writes(ring->vma);
-
 	spin_lock_irq(&client->wq_lock);
 	guc_wq_item_append(client, engine->guc_id, ctx_desc,
-			   ring->tail / sizeof(u64), 0);
+			   GUC_PREEMPT_BREADCRUMB_BYTES / sizeof(u64), 0);
 	spin_unlock_irq(&client->wq_lock);
 
 	/*
@@ -972,6 +950,40 @@ static void guc_client_free(struct intel_guc_client *client)
 	kfree(client);
 }
 
+static void guc_fill_preempt_context(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct intel_guc_client *client = guc->preempt_client;
+	struct intel_engine_cs *engine;
+	struct intel_ring *ring;
+	enum intel_engine_id id;
+	u32 *cs;
+
+	for_each_engine(engine, dev_priv, id) {
+		ring = client->owner->engine[id].ring;
+		cs = ring->vaddr;
+
+		if (id == RCS) {
+			cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED,
+					intel_hws_preempt_done_address(engine));
+		} else {
+			cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED,
+					intel_hws_preempt_done_address(engine));
+			*cs++ = MI_NOOP;
+			*cs++ = MI_NOOP;
+		}
+		*cs++ = MI_USER_INTERRUPT;
+		*cs++ = MI_NOOP;
+
+		GEM_BUG_ON(!IS_ALIGNED(ring->size,
+			   GUC_PREEMPT_BREADCRUMB_BYTES));
+		GEM_BUG_ON((void *)cs - ring->vaddr !=
+			   GUC_PREEMPT_BREADCRUMB_BYTES);
+
+		flush_ggtt_writes(ring->vma);
+	}
+}
+
 static int guc_clients_create(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -1002,6 +1014,8 @@ static int guc_clients_create(struct intel_guc *guc)
 			return PTR_ERR(client);
 		}
 		guc->preempt_client = client;
+
+		guc_fill_preempt_context(guc);
 	}
 
 	return 0;
-- 
2.14.3

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

[PATCH 2/2] HAX: Enable GuC submission for CI

[Michał Winiarski]

---
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 drivers/gpu/drm/i915/intel_uc.c    | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 430f5f9d0ff4..3deae1e22974 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -47,7 +47,7 @@ struct drm_printer;
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc, 0) \
+	param(int, enable_guc, -1) \
 	param(int, guc_log_level, 0) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9f1bac6398fb..b48056fb769d 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -63,6 +63,8 @@ static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
 		enable_guc |= ENABLE_GUC_LOAD_HUC;
 
 	/* Any platform specific fine-tuning can be done here */
+	if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
+		enable_guc = 0;
 
 	return enable_guc;
 }
-- 
2.14.3

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

Re: [PATCH 1/2] drm/i915/guc: Fill preempt context once at init time

[Chris Wilson]

Quoting Michał Winiarski (2018-02-26 13:59:59)
> Since we're inhibiting context save of preempt context, we're no longer
> tracking the position of HEAD/TAIL. With GuC, we're adding a new
> breadcrumb for each preemption, which means that the HW will do more and
> more breadcrumb writes. Eventually the ring is filled, and we're
> submitting the preemption context with HEAD==TAIL==0, which won't result
> in breadcrumb write, but will trigger hangcheck instead.
> Instead of writing a new preempt breadcrumb for each preemption, let's
> just fill the ring once at init time (which also saves a couple of
> instructions in the tasklet).
> 
> Fixes: 517aaffe0c1b ("drm/i915/execlists: Inhibit context save/restore for the fake preempt context")
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_submission.c | 68 +++++++++++++++++------------
>  1 file changed, 41 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 586dde579903..89e5b036061d 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -28,6 +28,10 @@
>  #include "intel_guc_submission.h"
>  #include "i915_drv.h"
>  
> +#define GUC_PREEMPT_FINISHED           0x1
> +#define GUC_PREEMPT_BREADCRUMB_DWORDS  0x8
> +#define GUC_PREEMPT_BREADCRUMB_BYTES   (sizeof(u32) * GUC_PREEMPT_BREADCRUMB_DWORDS)
> +
>  /**
>   * DOC: GuC-based command submission
>   *
> @@ -535,8 +539,6 @@ static void flush_ggtt_writes(struct i915_vma *vma)
>                 POSTING_READ_FW(GUC_STATUS);
>  }
>  
> -#define GUC_PREEMPT_FINISHED 0x1
> -#define GUC_PREEMPT_BREADCRUMB_DWORDS 0x8
>  static void inject_preempt_context(struct work_struct *work)
>  {
>         struct guc_preempt_work *preempt_work =
> @@ -546,37 +548,13 @@ static void inject_preempt_context(struct work_struct *work)
>                                              preempt_work[engine->id]);
>         struct intel_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));
> -       u32 *cs = ring->vaddr + ring->tail;
>         u32 data[7];
>  
> -       if (engine->id == RCS) {
> -               cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED,
> -                               intel_hws_preempt_done_address(engine));
> -       } else {
> -               cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED,
> -                               intel_hws_preempt_done_address(engine));
> -               *cs++ = MI_NOOP;
> -               *cs++ = MI_NOOP;
> -       }
> -       *cs++ = MI_USER_INTERRUPT;
> -       *cs++ = MI_NOOP;
> -
> -       GEM_BUG_ON(!IS_ALIGNED(ring->size,
> -                              GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32)));
> -       GEM_BUG_ON((void *)cs - (ring->vaddr + ring->tail) !=
> -                  GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32));
> -
> -       ring->tail += GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32);
> -       ring->tail &= (ring->size - 1);
> -
> -       flush_ggtt_writes(ring->vma);
> -
>         spin_lock_irq(&client->wq_lock);
>         guc_wq_item_append(client, engine->guc_id, ctx_desc,
> -                          ring->tail / sizeof(u64), 0);
> +                          GUC_PREEMPT_BREADCRUMB_BYTES / sizeof(u64), 0);
>         spin_unlock_irq(&client->wq_lock);
>  
>         /*
> @@ -972,6 +950,40 @@ static void guc_client_free(struct intel_guc_client *client)
>         kfree(client);
>  }
>  
> +static void guc_fill_preempt_context(struct intel_guc *guc)
> +{
> +       struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +       struct intel_guc_client *client = guc->preempt_client;
> +       struct intel_engine_cs *engine;
> +       struct intel_ring *ring;
> +       enum intel_engine_id id;
> +       u32 *cs;
> +

Whether to use preempt_client or preempt_context is your call.

> +       for_each_engine(engine, dev_priv, id) {
		struct intel_engine *ce = &client->owner->engine[id];

		/* The preempt context must be pinned on each engine);
		GEM_BUG_ON(!ce->pin_count);

		/*
		 * We rely on the context image *not* being saved after
		 * preemption. This ensures that the RING_HEAD / RING_TAIL
		 * do not advance and they remain pointing at this command
		 * sequence forever.
		 */
		GEM_BUG_ON(!(ce->reg_state[SR] & CTX_SR_SAVE_INHIBIT));

> +               ring = client->owner->engine[id].ring;
> +               cs = ring->vaddr;
> +
> +               if (id == RCS) {
> +                       cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED,
> +                                       intel_hws_preempt_done_address(engine));
> +               } else {
> +                       cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED,
> +                                       intel_hws_preempt_done_address(engine));
> +                       *cs++ = MI_NOOP;
> +                       *cs++ = MI_NOOP;
> +               }
> +               *cs++ = MI_USER_INTERRUPT;
> +               *cs++ = MI_NOOP;
> +
> +               GEM_BUG_ON(!IS_ALIGNED(ring->size,
> +                          GUC_PREEMPT_BREADCRUMB_BYTES));
> +               GEM_BUG_ON((void *)cs - ring->vaddr !=
> +                          GUC_PREEMPT_BREADCRUMB_BYTES);

We don't care about these anymore as we don't have to worry about
instructions wrapping. Similarly, we don't need the NOOP padding after
MI_FLUSH_DW.

Keep setting ring->tail here so we can avoid the hardcoding inside the
submission. That will allow us to adapt this with ease.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/guc: Fill preempt context once at init time

[Chris Wilson]

Quoting Chris Wilson (2018-02-26 14:28:36)
> Quoting Michał Winiarski (2018-02-26 13:59:59)
> > +       for_each_engine(engine, dev_priv, id) {
>                 struct intel_engine *ce = &client->owner->engine[id];
> 
>                 /* The preempt context must be pinned on each engine);
>                 GEM_BUG_ON(!ce->pin_count);
> 
>                 /*
>                  * We rely on the context image *not* being saved after
s/the context/this context/

>                  * preemption. This ensures that the RING_HEAD / RING_TAIL
>                  * do not advance and they remain pointing at this command
>                  * sequence forever.
>                  */
>                 GEM_BUG_ON(!(ce->reg_state[SR] & CTX_SR_SAVE_INHIBIT));
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/guc: Fill preempt context once at init time

[Chris Wilson]

Quoting Chris Wilson (2018-02-26 14:28:36)
> Quoting Michał Winiarski (2018-02-26 13:59:59)
> > +       for_each_engine(engine, dev_priv, id) {
>                 struct intel_engine *ce = &client->owner->engine[id];
> 
>                 /* The preempt context must be pinned on each engine);
>                 GEM_BUG_ON(!ce->pin_count);
> 
>                 /*
>                  * We rely on the context image *not* being saved after
>                  * preemption. This ensures that the RING_HEAD / RING_TAIL
>                  * do not advance and they remain pointing at this command
>                  * sequence forever.
>                  */

Hmm, this is not true. See intel_lr_context_resume().

Does this patch justify a test case for gem_exec_schedule+suspend
specifically? Or do we just repeat every test after a S&R cycle? I think
we could do with the latter!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/guc: Fill preempt context once at init time

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/guc: Fill preempt context once at init time
URL   : https://patchwork.freedesktop.org/series/38964/
State : success

== Summary ==

Series 38964v1 series starting with [1/2] drm/i915/guc: Fill preempt context once at init time
https://patchwork.freedesktop.org/api/1.0/series/38964/revisions/1/mbox/

---- Known issues:

Test debugfs_test:
        Subgroup read_all_entries:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713 +1

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:416s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:419s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:375s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:488s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:285s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:478s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:468s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:455s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:397s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:568s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:412s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:286s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:505s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:390s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:410s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:453s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:454s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:490s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:447s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:493s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:584s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:432s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:503s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:520s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:486s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:474s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:409s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:425s
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:395s

23012f548f1fe658192a8e43ad5af139b726676e drm-tip: 2018y-02m-26d-10h-58m-50s UTC integration manifest
d7d42ef45e3e HAX: Enable GuC submission for CI
49db4e341892 drm/i915/guc: Fill preempt context once at init time

== Logs ==

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

Re: [PATCH 1/2] drm/i915/guc: Fill preempt context once at init time

[Chris Wilson]

Quoting Chris Wilson (2018-02-26 14:38:20)
> Quoting Chris Wilson (2018-02-26 14:28:36)
> > Quoting Michał Winiarski (2018-02-26 13:59:59)
> > > +       for_each_engine(engine, dev_priv, id) {
> >                 struct intel_engine *ce = &client->owner->engine[id];
> > 
> >                 /* The preempt context must be pinned on each engine);
> >                 GEM_BUG_ON(!ce->pin_count);
> > 
> >                 /*
> >                  * We rely on the context image *not* being saved after
> >                  * preemption. This ensures that the RING_HEAD / RING_TAIL
> >                  * do not advance and they remain pointing at this command
> >                  * sequence forever.
> >                  */
> 
> Hmm, this is not true. See intel_lr_context_resume().

Continuing this chain of thought, that doesn't matter. The reg state is
reset to 0, which is what we expect with context-save-inhibit anyway.
What it does do is reset ring->tail to 0 as well. That doesn't play well
with my idea to use ring->tail.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/guc: Fill preempt context once at init time

[Chris Wilson]

Quoting Chris Wilson (2018-02-26 14:28:36)
> Quoting Michał Winiarski (2018-02-26 13:59:59)
> > +               if (id == RCS) {
> > +                       cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED,
> > +                                       intel_hws_preempt_done_address(engine));
> > +               } else {
> > +                       cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED,
> > +                                       intel_hws_preempt_done_address(engine));
> > +                       *cs++ = MI_NOOP;
> > +                       *cs++ = MI_NOOP;
> > +               }
> > +               *cs++ = MI_USER_INTERRUPT;
> > +               *cs++ = MI_NOOP;
> > +
> > +               GEM_BUG_ON(!IS_ALIGNED(ring->size,
> > +                          GUC_PREEMPT_BREADCRUMB_BYTES));
> > +               GEM_BUG_ON((void *)cs - ring->vaddr !=
> > +                          GUC_PREEMPT_BREADCRUMB_BYTES);
> 
> We don't care about these anymore as we don't have to worry about
> instructions wrapping. Similarly, we don't need the NOOP padding after
> MI_FLUSH_DW.
> 
> Keep setting ring->tail here so we can avoid the hardcoding inside the
> submission. That will allow us to adapt this with ease.

Quick digression later, intel_lr_context_resume() breaks this idea to
keep ring->tail set. So that means we need to keep the fixed size
command packet and the hardcoded length. (But we can still remove the
asserts as we do not require magic alignments to avoid wraparound issues
anymore.)

But we do want a comment here for something like
GEM_BUG_ON(cs != GUC_PREEMPT_BREADCRUMB_BYTES);
to tie the magic constant here to the wq submission. And maybe a comment
in inject_preempt_context() to explain that we we submit a command
packet that writes GUC_PREEMPT_FINISHED.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/guc: Fill preempt context once at init time

[Chris Wilson]

Quoting Chris Wilson (2018-02-26 14:28:36)
> Quoting Michał Winiarski (2018-02-26 13:59:59)
> > +       for_each_engine(engine, dev_priv, id) {
>                 struct intel_engine *ce = &client->owner->engine[id];
> 
>                 /* The preempt context must be pinned on each engine);
>                 GEM_BUG_ON(!ce->pin_count);
> 
>                 /*
>                  * We rely on the context image *not* being saved after
>                  * preemption. This ensures that the RING_HEAD / RING_TAIL
>                  * do not advance and they remain pointing at this command
>                  * sequence forever.
>                  */
>                 GEM_BUG_ON(!(ce->reg_state[SR] & CTX_SR_SAVE_INHIBIT));

In fact, don't bug out, just set it here. Then it won't break again when
icl enabling lands.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] HAX: Enable GuC submission for CI

[Michał Winiarski]

---
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 drivers/gpu/drm/i915/intel_uc.c    | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 430f5f9d0ff4..3deae1e22974 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -47,7 +47,7 @@ struct drm_printer;
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc, 0) \
+	param(int, enable_guc, -1) \
 	param(int, guc_log_level, 0) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9f1bac6398fb..b48056fb769d 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -63,6 +63,8 @@ static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
 		enable_guc |= ENABLE_GUC_LOAD_HUC;
 
 	/* Any platform specific fine-tuning can be done here */
+	if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
+		enable_guc = 0;
 
 	return enable_guc;
 }
-- 
2.14.3

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

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/guc: Fill preempt context once at init time

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/guc: Fill preempt context once at init time
URL   : https://patchwork.freedesktop.org/series/38964/
State : success

== Summary ==

---- Possible new issues:

Test drv_missed_irq:
                pass       -> SKIP       (shard-apl)
Test drv_selftest:
        Subgroup live_guc:
                pass       -> DMESG-WARN (shard-apl)
Test perf:
        Subgroup gen8-unprivileged-single-ctx-counters:
                pass       -> FAIL       (shard-apl)

---- Known issues:

Test gem_eio:
        Subgroup in-flight:
                pass       -> INCOMPLETE (shard-apl) fdo#104945
Test kms_flip:
        Subgroup 2x-plain-flip-fb-recreate:
                fail       -> PASS       (shard-hsw) fdo#100368 +1
Test kms_frontbuffer_tracking:
        Subgroup fbc-rgb565-draw-blt:
                pass       -> FAIL       (shard-apl) fdo#101623
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047

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

shard-apl        total:3434 pass:1793 dwarn:2   dfail:0   fail:16  skip:1621 time:12059s
shard-hsw        total:3460 pass:1767 dwarn:1   dfail:0   fail:1   skip:1690 time:11691s
shard-snb        total:3558 pass:1392 dwarn:1   dfail:0   fail:2   skip:2163 time:6839s
Blacklisted hosts:
shard-kbl        total:3258 pass:1808 dwarn:2   dfail:1   fail:15  skip:1429 time:8828s

== Logs ==

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