Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/guc: fix GuC suspend/resume
On 27/11/2018 11:34, Daniele Ceraolo Spurio wrote:
On 26/11/2018 06:51, Michal Wajdeczko wrote:
On Wed, 17 Oct 2018 00:46:47 +0200, Daniele Ceraolo Spurio
wrote:
/snip/
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 8382d591c784..1a853cc627e3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -687,6 +687,13 @@ enum intel_guc_report_status {
INTEL_GUC_REPORT_STATUS_COMPLETE = 0x4,
};
+enum intel_guc_sleep_state_status {
+ INTEL_GUC_SLEEP_STATE_SUCCESS = 0x0,
+ INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x1,
+ INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x2
+};
btw, it used to be 0,1,2 but from some time fw defines above as:
INTEL_GUC_SLEEP_STATE_SUCCESS = 0x1,
INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x2,
INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x3,
Michal
Yeah, I think I had already mentioned in some reply that the newer
firmware does suspend/resume differently, but I haven't looked at the
details. I'm not even sure if polling the register will still be required.
Daniele
I've confirmed with the GuC team that the differences are mostly
internal to GuC and the only change from the kernel perspective is that
the enum values have changed. We still need to do the polling, but I
guess we'll be able to init the register to zero since all the return
values are > 0.
Daniele
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/guc: fix GuC suspend/resume
On 26/11/2018 06:51, Michal Wajdeczko wrote:
On Wed, 17 Oct 2018 00:46:47 +0200, Daniele Ceraolo Spurio
wrote:
/snip/
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 8382d591c784..1a853cc627e3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -687,6 +687,13 @@ enum intel_guc_report_status {
INTEL_GUC_REPORT_STATUS_COMPLETE = 0x4,
};
+enum intel_guc_sleep_state_status {
+ INTEL_GUC_SLEEP_STATE_SUCCESS = 0x0,
+ INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x1,
+ INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x2
+};
btw, it used to be 0,1,2 but from some time fw defines above as:
INTEL_GUC_SLEEP_STATE_SUCCESS = 0x1,
INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x2,
INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x3,
Michal
Yeah, I think I had already mentioned in some reply that the newer
firmware does suspend/resume differently, but I haven't looked at the
details. I'm not even sure if polling the register will still be required.
Daniele
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/guc: fix GuC suspend/resume
On Wed, 17 Oct 2018 00:46:47 +0200, Daniele Ceraolo Spurio
wrote:
/snip/
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 8382d591c784..1a853cc627e3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -687,6 +687,13 @@ enum intel_guc_report_status {
INTEL_GUC_REPORT_STATUS_COMPLETE = 0x4,
};
+enum intel_guc_sleep_state_status {
+ INTEL_GUC_SLEEP_STATE_SUCCESS = 0x0,
+ INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x1,
+ INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x2
+};
btw, it used to be 0,1,2 but from some time fw defines above as:
INTEL_GUC_SLEEP_STATE_SUCCESS = 0x1,
INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x2,
INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x3,
Michal
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/guc: fix GuC suspend/resume
Quoting Michal Wajdeczko (2018-10-17 17:40:50) > On Wed, 17 Oct 2018 00:46:47 +0200, Daniele Ceraolo Spurio > wrote: > > > The ENTER/EXIT_S_STATE actions queue the save/restore operation in GuC > > FW and then return, so waiting on the H2G is not enough to guarantee > > GuC is done. > > When all the processing is done, GuC writes 0 to scratch register 14, > > so we can poll on that. Note that GuC does not ensure that the value > > in the register is different from 0 while the action is in progress > > so we need to take care of that ourselves as well. > > > > v2: improve comment, return early on GuC error and improve error > > message (Michal) > > > > Cc: Chris Wilson > > Cc: Michal Wajdeczko > > Signed-off-by: Daniele Ceraolo Spurio > > Acked-by: Chris Wilson > > --- > > Reviewed-by: Michal Wajdeczko Applied, thanks for the fix and review. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/guc: fix GuC suspend/resume
On Wed, 17 Oct 2018 00:46:47 +0200, Daniele Ceraolo Spurio wrote: The ENTER/EXIT_S_STATE actions queue the save/restore operation in GuC FW and then return, so waiting on the H2G is not enough to guarantee GuC is done. When all the processing is done, GuC writes 0 to scratch register 14, so we can poll on that. Note that GuC does not ensure that the value in the register is different from 0 while the action is in progress so we need to take care of that ourselves as well. v2: improve comment, return early on GuC error and improve error message (Michal) Cc: Chris Wilson Cc: Michal Wajdeczko Signed-off-by: Daniele Ceraolo Spurio Acked-by: Chris Wilson --- Reviewed-by: Michal Wajdeczko Michal ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/2] drm/i915/guc: fix GuC suspend/resume
The ENTER/EXIT_S_STATE actions queue the save/restore operation in GuC
FW and then return, so waiting on the H2G is not enough to guarantee
GuC is done.
When all the processing is done, GuC writes 0 to scratch register 14,
so we can poll on that. Note that GuC does not ensure that the value
in the register is different from 0 while the action is in progress
so we need to take care of that ourselves as well.
v2: improve comment, return early on GuC error and improve error
message (Michal)
Cc: Chris Wilson
Cc: Michal Wajdeczko
Signed-off-by: Daniele Ceraolo Spurio
Acked-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_guc.c | 42 +--
drivers/gpu/drm/i915/intel_guc_fwif.h | 7 +
2 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 230aea69385d..4c61eb94527a 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -521,6 +521,44 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32
rsa_offset)
return intel_guc_send(guc, action, ARRAY_SIZE(action));
}
+/*
+ * The ENTER/EXIT_S_STATE actions queue the save/restore operation in GuC FW
and
+ * then return, so waiting on the H2G is not enough to guarantee GuC is done.
+ * When all the processing is done, GuC writes INTEL_GUC_SLEEP_STATE_SUCCESS to
+ * scratch register 14, so we can poll on that. Note that GuC does not ensure
+ * that the value in the register is different from
+ * INTEL_GUC_SLEEP_STATE_SUCCESS while the action is in progress so we need to
+ * take care of that ourselves as well.
+ */
+static int guc_sleep_state_action(struct intel_guc *guc,
+ const u32 *action, u32 len)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ int ret;
+ u32 status;
+
+ I915_WRITE(SOFT_SCRATCH(14), INTEL_GUC_SLEEP_STATE_INVALID_MASK);
+
+ ret = intel_guc_send(guc, action, len);
+ if (ret)
+ return ret;
+
+ ret = __intel_wait_for_register(dev_priv, SOFT_SCRATCH(14),
+ INTEL_GUC_SLEEP_STATE_INVALID_MASK,
+ 0, 0, 10, );
+ if (ret)
+ return ret;
+
+ if (status != INTEL_GUC_SLEEP_STATE_SUCCESS) {
+ DRM_ERROR("GuC failed to change sleep state. "
+ "action=0x%x, err=%u\n",
+ action[0], status);
+ return -EIO;
+ }
+
+ return 0;
+}
+
/**
* intel_guc_suspend() - notify GuC entering suspend state
* @guc: the guc
@@ -533,7 +571,7 @@ int intel_guc_suspend(struct intel_guc *guc)
intel_guc_ggtt_offset(guc, guc->shared_data)
};
- return intel_guc_send(guc, data, ARRAY_SIZE(data));
+ return guc_sleep_state_action(guc, data, ARRAY_SIZE(data));
}
/**
@@ -571,7 +609,7 @@ int intel_guc_resume(struct intel_guc *guc)
intel_guc_ggtt_offset(guc, guc->shared_data)
};
- return intel_guc_send(guc, data, ARRAY_SIZE(data));
+ return guc_sleep_state_action(guc, data, ARRAY_SIZE(data));
}
/**
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 8382d591c784..1a853cc627e3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -687,6 +687,13 @@ enum intel_guc_report_status {
INTEL_GUC_REPORT_STATUS_COMPLETE = 0x4,
};
+enum intel_guc_sleep_state_status {
+ INTEL_GUC_SLEEP_STATE_SUCCESS = 0x0,
+ INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x1,
+ INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x2
+};
+#define INTEL_GUC_SLEEP_STATE_INVALID_MASK 0x8000
+
#define GUC_LOG_CONTROL_LOGGING_ENABLED(1 << 0)
#define GUC_LOG_CONTROL_VERBOSITY_SHIFT4
#define GUC_LOG_CONTROL_VERBOSITY_MASK (0xF << GUC_LOG_CONTROL_VERBOSITY_SHIFT)
--
2.19.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
