[PATCH 1/3] drm/i915/guc: Clear preempt status before use

[PATCH 1/3] drm/i915/guc: Clear preempt status before use

[Chris Wilson]

---
 drivers/gpu/drm/i915/intel_guc.h            |  4 ++-
 drivers/gpu/drm/i915/intel_guc_submission.c | 33 +++++++++++----------
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 0f1c4f9ebfd8..5703e62a3d33 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -34,6 +34,8 @@
 #include "intel_uc_fw.h"
 #include "i915_vma.h"
 
+struct guc_shared_ctx_data;
+
 struct guc_preempt_work {
 	struct work_struct work;
 	struct intel_engine_cs *engine;
@@ -62,7 +64,7 @@ struct intel_guc {
 	void *stage_desc_pool_vaddr;
 	struct ida stage_ids;
 	struct i915_vma *shared_data;
-	void *shared_data_vaddr;
+	struct guc_shared_ctx_data *shared_data_vaddr;
 
 	struct intel_guc_client *execbuf_client;
 	struct intel_guc_client *preempt_client;
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index eae668442ebe..fb0499f80b62 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -555,6 +555,8 @@ static void inject_preempt_context(struct work_struct *work)
 	struct intel_guc_client *client = guc->preempt_client;
 	struct guc_stage_desc *stage_desc = __get_stage_desc(client);
 	struct intel_context *ce = to_intel_context(client->owner, engine);
+	struct guc_ctx_report *report =
+		&guc->shared_data_vaddr->preempt_ctx_report[engine->guc_id];
 	u32 data[7];
 
 	if (!ce->ring->emit) { /* recreate upon load/resume */
@@ -587,6 +589,14 @@ static void inject_preempt_context(struct work_struct *work)
 			   GUC_PREEMPT_BREADCRUMB_BYTES / sizeof(u64), 0);
 	spin_unlock_irq(&client->wq_lock);
 
+	/*
+	 * GuC is expecting that we're also going to clear the affected context
+	 * counter, let's also reset the return status to not depend on GuC
+	 * resetting it after recieving another preempt action
+	 */
+	report->affected_count = 0;
+	report->report_return_status = INTEL_GUC_REPORT_STATUS_UNKNOWN;
+
 	/*
 	 * If GuC firmware performs an engine reset while that engine had
 	 * a preemption pending, it will set the terminated attribute bit
@@ -629,20 +639,12 @@ static void inject_preempt_context(struct work_struct *work)
 static void wait_for_guc_preempt_report(struct intel_engine_cs *engine)
 {
 	struct intel_guc *guc = &engine->i915->guc;
-	struct guc_shared_ctx_data *data = guc->shared_data_vaddr;
 	struct guc_ctx_report *report =
-		&data->preempt_ctx_report[engine->guc_id];
+		&guc->shared_data_vaddr->preempt_ctx_report[engine->guc_id];
 
-	WARN_ON(wait_for_atomic(report->report_return_status ==
+	WARN_ON(wait_for_atomic(READ_ONCE(report->report_return_status) ==
 				INTEL_GUC_REPORT_STATUS_COMPLETE,
 				GUC_PREEMPT_POSTPROCESS_DELAY_MS));
-	/*
-	 * GuC is expecting that we're also going to clear the affected context
-	 * counter, let's also reset the return status to not depend on GuC
-	 * resetting it after recieving another preempt action
-	 */
-	report->affected_count = 0;
-	report->report_return_status = INTEL_GUC_REPORT_STATUS_UNKNOWN;
 }
 
 static void complete_preempt_context(struct intel_engine_cs *engine)
@@ -654,11 +656,10 @@ static void complete_preempt_context(struct intel_engine_cs *engine)
 	if (inject_preempt_hang(execlists))
 		return;
 
+	wait_for_guc_preempt_report(engine);
+
 	execlists_cancel_port_requests(execlists);
 	execlists_unwind_incomplete_requests(execlists);
-
-	wait_for_guc_preempt_report(engine);
-	intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0);
 }
 
 /**
@@ -726,6 +727,9 @@ static bool __guc_dequeue(struct intel_engine_cs *engine)
 			int prio = execlists->queue_priority;
 
 			if (__execlists_need_preempt(prio, port_prio(port))) {
+				intel_write_status_page(engine,
+							I915_GEM_HWS_PREEMPT_INDEX,
+							0);
 				execlists_set_active(execlists,
 						     EXECLISTS_ACTIVE_PREEMPT);
 				queue_work(engine->i915->guc.preempt_wq,
@@ -816,8 +820,7 @@ static void guc_submission_tasklet(unsigned long data)
 	}
 
 	if (execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT) &&
-	    intel_read_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX) ==
-	    GUC_PREEMPT_FINISHED)
+	    intel_read_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX))
 		complete_preempt_context(engine);
 
 	if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT))
-- 
2.19.1

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

[PATCH 2/3] guc-send

[Chris Wilson]

---
 drivers/gpu/drm/i915/intel_guc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 230aea69385d..20d1a6c7d20f 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -31,7 +31,7 @@ static void gen8_guc_raise_irq(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
+	I915_WRITE_FW(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
 }
 
 static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
@@ -404,9 +404,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 	intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
 
 	for (i = 0; i < len; i++)
-		I915_WRITE(guc_send_reg(guc, i), action[i]);
+		I915_WRITE_FW(guc_send_reg(guc, i), action[i]);
 
-	POSTING_READ(guc_send_reg(guc, i - 1));
+	POSTING_READ_FW(guc_send_reg(guc, i - 1));
 
 	intel_guc_notify(guc);
 
@@ -419,7 +419,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 					   INTEL_GUC_MSG_TYPE_MASK,
 					   INTEL_GUC_MSG_TYPE_RESPONSE <<
 					   INTEL_GUC_MSG_TYPE_SHIFT,
-					   10, 10, &status);
+					   10, 50, &status);
 	/* If GuC explicitly returned an error, convert it to -EIO */
 	if (!ret && !INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(status))
 		ret = -EIO;
@@ -434,7 +434,8 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 		int count = min(response_buf_size, guc->send_regs.count - 1);
 
 		for (i = 0; i < count; i++)
-			response_buf[i] = I915_READ(guc_send_reg(guc, i + 1));
+			response_buf[i] =
+			       	I915_READ_FW(guc_send_reg(guc, i + 1));
 	}
 
 	/* Use data from the GuC response as our return value */
-- 
2.19.1

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

[PATCH 3/3] drm/i915/guc: sleep on enable

[Chris Wilson]

Seems like there's a missing ack before the guc is ready for commands.

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index fb0499f80b62..b7fd3422cb28 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -1307,6 +1307,8 @@ int intel_guc_submission_enable(struct intel_guc *guc)
 
 	GEM_BUG_ON(!guc->execbuf_client);
 
+	usleep_range(1000, 10000);
+
 	err = intel_guc_sample_forcewake(guc);
 	if (err)
 		return err;
-- 
2.19.1

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/guc: Clear preempt status before use

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/guc: Clear preempt status before use
URL   : https://patchwork.freedesktop.org/series/50980/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
be690e2c317d drm/i915/guc: Clear preempt status before use
-:8: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

-:113: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)

total: 1 errors, 1 warnings, 0 checks, 91 lines checked
16b5c27313f1 guc-send
-:8: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

-:47: ERROR:CODE_INDENT: code indent should use tabs where possible
#47: FILE: drivers/gpu/drm/i915/intel_guc.c:438:
+^I^I^I       ^II915_READ_FW(guc_send_reg(guc, i + 1));$

-:47: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#47: FILE: drivers/gpu/drm/i915/intel_guc.c:438:
+^I^I^I       ^II915_READ_FW(guc_send_reg(guc, i + 1));$

-:50: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)

total: 2 errors, 2 warnings, 0 checks, 36 lines checked
f8b28681abd3 drm/i915/guc: sleep on enable
-:22: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 0 checks, 8 lines checked

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

✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/guc: Clear preempt status before use

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/guc: Clear preempt status before use
URL   : https://patchwork.freedesktop.org/series/50980/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/guc: Clear preempt status before use
Okay!

Commit: guc-send
-O:drivers/gpu/drm/i915/intel_guc.c:434:29: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_guc.c:434:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_guc.c:434:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_guc.c:434:29: warning: expression using sizeof(void)

Commit: drm/i915/guc: sleep on enable
Okay!

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

✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/guc: Clear preempt status before use

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/guc: Clear preempt status before use
URL   : https://patchwork.freedesktop.org/series/50980/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4979 -> Patchwork_10453 =

== Summary - FAILURE ==

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_hangcheck:
      fi-icl-u2:          PASS -> INCOMPLETE

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_guc:
      {fi-apl-guc}:       NOTRUN -> DMESG-WARN (fdo#107258)

    igt@gem_mmap_gtt@basic-read-no-prefault:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#105719)

    igt@kms_busy@basic-flip-b:
      fi-skl-6700hq:      PASS -> DMESG-WARN (fdo#105998) +2

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    igt@pm_rpm@module-reload:
      fi-skl-6600u:       PASS -> INCOMPLETE (fdo#107807)
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#107726)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gem:
      {fi-apl-guc}:       INCOMPLETE (fdo#106693) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-icl-u:           INCOMPLETE (fdo#108315) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       DMESG-WARN (fdo#106000) -> PASS

    igt@kms_flip@basic-plain-flip:
      fi-ilk-650:         DMESG-WARN (fdo#106387) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362, fdo#103191) -> PASS +1

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

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
  fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
  fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107726 https://bugs.freedesktop.org/show_bug.cgi?id=107726
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315


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

  Additional (1): fi-pnv-d510 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-bwr-2160 


== Build changes ==

    * Linux: CI_DRM_4979 -> Patchwork_10453

  CI_DRM_4979: 2c411746783a4db33844f298ee88f0301cf0453e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4674: 93871c6fb3c25e5d350c9faf36ded917174214de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10453: f8b28681abd3ea293d1c40107a0f6df4f6c1fe4e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f8b28681abd3 drm/i915/guc: sleep on enable
16b5c27313f1 guc-send
be690e2c317d drm/i915/guc: Clear preempt status before use

== Logs ==

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

Re: [PATCH 3/3] drm/i915/guc: sleep on enable

[Daniele Ceraolo Spurio]

On 14/10/18 10:02, Chris Wilson wrote:
> Seems like there's a missing ack before the guc is ready for commands.
> 

I'm assuming you're running without HuC since the HuC auth H2G comes 
before this one.
What we're polling to indicate load completion (GS_UKERNEL_READY) is 
definitely what the firmware uses to signal readiness. The other check 
we do (GS_MIA_CORE_STATE) should only apply for rc6 scenarios. From what 
I can see from the firmware code, all the initialization steps are done 
before GS_UKERNEL_READY is written to the status register so there 
shouldn't be any missing acks in principle.
Is the GuC returning anything in the scratch 0 register? It should be 
printed out by the H2G error message. The value of the status register 
(0xc000) could also provide interesting debug info.

Thanks,
Daniele

> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_submission.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index fb0499f80b62..b7fd3422cb28 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -1307,6 +1307,8 @@ int intel_guc_submission_enable(struct intel_guc *guc)
>   
>   	GEM_BUG_ON(!guc->execbuf_client);
>   
> +	usleep_range(1000, 10000);
> +
>   	err = intel_guc_sample_forcewake(guc);
>   	if (err)
>   		return err;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/guc: sleep on enable

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2018-10-15 19:33:26)
> 
> 
> On 14/10/18 10:02, Chris Wilson wrote:
> > Seems like there's a missing ack before the guc is ready for commands.
> > 
> 
> I'm assuming you're running without HuC since the HuC auth H2G comes 
> before this one.

https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_4981/fi-apl-guc/boot0.log
i915.enable_guc=3 
<7>[    6.877175] [drm:intel_uc_fw_fetch [i915]] GuC fw fetch i915/bxt_guc_ver9_29.bin
<7>[    6.877268] [drm:intel_uc_fw_fetch [i915]] GuC fw fetch PENDING
<7>[    6.879780] [drm:intel_uc_fw_fetch [i915]] GuC fw size 146432 ptr 000000003fdb20d0
<7>[    6.879869] [drm:intel_uc_fw_fetch [i915]] GuC fw version 9.29 (wanted 9.29)
<7>[    6.880425] [drm:intel_uc_fw_fetch [i915]] GuC fw fetch SUCCESS
<7>[    6.880723] [drm:intel_uc_fw_fetch [i915]] HuC fw fetch i915/bxt_huc_ver01_07_1398.bin
<7>[    6.880807] [drm:intel_uc_fw_fetch [i915]] HuC fw fetch PENDING
<7>[    6.882529] [drm:intel_uc_fw_fetch [i915]] HuC fw size 154432 ptr 000000000aad61c4
<7>[    6.882621] [drm:intel_uc_fw_fetch [i915]] HuC fw version 1.7 (wanted 1.7)
<7>[    6.883098] [drm:intel_uc_fw_fetch [i915]] HuC fw fetch SUCCESS

> What we're polling to indicate load completion (GS_UKERNEL_READY) is 
> definitely what the firmware uses to signal readiness. The other check 
> we do (GS_MIA_CORE_STATE) should only apply for rc6 scenarios. From what 
> I can see from the firmware code, all the initialization steps are done 
> before GS_UKERNEL_READY is written to the status register so there 
> shouldn't be any missing acks in principle.

> Is the GuC returning anything in the scratch 0 register? It should be 
> printed out by the H2G error message. The value of the status register 
> (0xc000) could also provide interesting debug info.

When do you want to know? As you are probably aware, our first
indication of failure is from wait_for_guc_preempt_report() and
the wait there on report->report_return_status timing out.

Michel asked what was the value when it timed out, but alas apl-guc was
not available for comment.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/guc: sleep on enable

[Daniele Ceraolo Spurio]

On 15/10/18 12:23, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2018-10-15 19:33:26)
>>
>>
>> On 14/10/18 10:02, Chris Wilson wrote:
>>> Seems like there's a missing ack before the guc is ready for commands.
>>>
>>
>> I'm assuming you're running without HuC since the HuC auth H2G comes
>> before this one.
> 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_4981/fi-apl-guc/boot0.log
> i915.enable_guc=3
> <7>[    6.877175] [drm:intel_uc_fw_fetch [i915]] GuC fw fetch i915/bxt_guc_ver9_29.bin
> <7>[    6.877268] [drm:intel_uc_fw_fetch [i915]] GuC fw fetch PENDING
> <7>[    6.879780] [drm:intel_uc_fw_fetch [i915]] GuC fw size 146432 ptr 000000003fdb20d0
> <7>[    6.879869] [drm:intel_uc_fw_fetch [i915]] GuC fw version 9.29 (wanted 9.29)
> <7>[    6.880425] [drm:intel_uc_fw_fetch [i915]] GuC fw fetch SUCCESS
> <7>[    6.880723] [drm:intel_uc_fw_fetch [i915]] HuC fw fetch i915/bxt_huc_ver01_07_1398.bin
> <7>[    6.880807] [drm:intel_uc_fw_fetch [i915]] HuC fw fetch PENDING
> <7>[    6.882529] [drm:intel_uc_fw_fetch [i915]] HuC fw size 154432 ptr 000000000aad61c4
> <7>[    6.882621] [drm:intel_uc_fw_fetch [i915]] HuC fw version 1.7 (wanted 1.7)
> <7>[    6.883098] [drm:intel_uc_fw_fetch [i915]] HuC fw fetch SUCCESS
> 
>> What we're polling to indicate load completion (GS_UKERNEL_READY) is
>> definitely what the firmware uses to signal readiness. The other check
>> we do (GS_MIA_CORE_STATE) should only apply for rc6 scenarios. From what
>> I can see from the firmware code, all the initialization steps are done
>> before GS_UKERNEL_READY is written to the status register so there
>> shouldn't be any missing acks in principle.
> 
>> Is the GuC returning anything in the scratch 0 register? It should be
>> printed out by the H2G error message. The value of the status register
>> (0xc000) could also provide interesting debug info.
> 
> When do you want to know? As you are probably aware, our first
> indication of failure is from wait_for_guc_preempt_report() and
> the wait there on report->report_return_status timing out.
> 
> Michel asked what was the value when it timed out, but alas apl-guc was
> not available for comment.
> -Chris
> 

I think found the root cause of the issue (with the help of one of the 
GuC devs). The guc suspend/resume protocol requires us to do an extra 
couple of steps to make sure GuC is done managing its state, waiting on 
the H2G return is not enough; since we're not correctly doing those GuC 
is still in the middle of the resume process when the preemption request 
arrives, thus causing the failure. Patch to fix this incoming.

Note that since we ensure the HW is idle before suspend we could 
theoretically skip the guc_resume step as there is nothing to restore, 
but this is untested from the GuC side so not recommended yet. We still 
need to do guc_suspend since that step ensures that all guc timers are 
correctly disabled.

I think you mentioned you were also seeing issues even outside of the 
suspend/resume path, so we probably have a different issue as well :(

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