Re: [Intel-gfx] [PATCH 21/31] drm/i915/slpc: Send RESET event to enable SLPC during Load/TDR
On 9/21/2017 7:36 PM, Michal Wajdeczko wrote:
On Tue, 19 Sep 2017 19:41:57 +0200, Sagar Arun Kamble
wrote:
Send host2guc SLPC reset event to GuC post GuC load.
Post this, i915 can ascertain if SLPC has started running successfully
through shared data. This check is done during intel_init_gt_powersave.
This allows to get initial configuration setup by SLPC and if needed
move to Host RPS if SLPC runs into issues.
On TDR/Engine reset i915 should send extra flag
SLPC_RESET_FLAG_TDR_OCCURREDto clear SLPC state as appropriate.
v1: Extract host2guc_slpc to handle slpc status code
coding style changes (Paulo)
Removed WARN_ON for checking msb of gtt address of
shared gem obj. (ChrisW)
host2guc_action to i915_guc_action change.(Sagar)
Updating SLPC enabled status. (Sagar)
v2: Commit message update. (David)
v3: Rebase.
v4: Added DRM_INFO message when SLPC is enabled.
v5: Updated patch as host2guc_slpc is moved to earlier patch.
SLPC activation status message put after checking the
state from shared data during intel_init_gt_powersave.
v6: Added definition of host2guc_slpc and clflush the shared data only
for required size. Setting state to NOT_RUNNING before sending RESET
event. Output data for SLPC actions is to be retrieved during
intel_guc_send with lock protection so created wrapper
__intel_guc_send that outputs GuC output data if needed. Clearing
pm_rps_events on confirming SLPC RUNNING status so that even if
host touches any of the PM registers by mistake it should not have
any effect. (Sagar)
v7: Added save/restore_default_rps as Uncore sanitize will clear the
RP_CONTROL setup by BIOS. s/i915_ggtt_offset/guc_ggtt_offset.
v8: Added support for handling TDR based SLPC reset. Added functions
host2guc_slpc_tdr_reset, intel_slpc_reset_prepare and
intel_slpc_tdr_reset to handle TDR based SLPC reset.
Cc: Michal Wajdeczko
Signed-off-by: Tom O'Rourke
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/i915_drv.c | 2 +
drivers/gpu/drm/i915/i915_irq.c | 7 +-
drivers/gpu/drm/i915/intel_pm.c | 10 +++
drivers/gpu/drm/i915/intel_slpc.c | 170
++
drivers/gpu/drm/i915/intel_slpc.h | 9 ++
drivers/gpu/drm/i915/intel_uc.c | 1 +
6 files changed, 198 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index f13a3de..932f9ef 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1074,6 +1074,8 @@ static int i915_driver_init_hw(struct
drm_i915_private *dev_priv)
intel_sanitize_options(dev_priv);
+ intel_slpc_save_default_rps(&dev_priv->guc.slpc);
+
ret = i915_ggtt_probe_hw(dev_priv);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 4a1554c..2d5ad13 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2838,8 +2838,13 @@ void i915_handle_error(struct drm_i915_private
*dev_priv,
}
}
- if (!engine_mask)
+ if (!engine_mask) {
+ if (intel_slpc_active(&dev_priv->guc.slpc)) {
+ intel_slpc_reset_prepare(&dev_priv->guc.slpc);
+ intel_slpc_tdr_reset(&dev_priv->guc.slpc);
+ }
Can you just jump to single slpc function that will hide slpc internals ?
we need to have these two calls as currently SLPC gets enabled during
intel_uc_init_hw and we can reset from two places.
1. engine reset 2. full gpu reset
In 1st case we want to disable SLPC and reset it
In 2nd case we just want to disable SLPC and let it get enabled through
intel_slpc_enable based on flag tdr_reset.
I will remove intel_slpc_active check here and probably create two
functions now to handle these different types of resets.
goto out;
+ }
/* Full reset needs the mutex, stop any other user trying to do
so. */
if (test_and_set_bit(I915_RESET_BACKOFF,
&dev_priv->gpu_error.flags)) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 6b2b7f8..c2065f2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7918,6 +7918,16 @@ void intel_init_gt_powersave(struct
drm_i915_private *dev_priv)
intel_runtime_pm_get(dev_priv);
}
+ if (intel_slpc_enabled()) {
+ dev_priv->guc.slpc.active =
+ intel_slpc_get_status(&dev_priv->guc.slpc);
+ if (!intel_slpc_active(&dev_priv->guc.slpc)) {
+ i915.enable_slpc = 0;
+ intel_sanitize_gt_powersave(dev_priv);
+ } else
+ dev_priv->pm_rps_events = 0;
+ }
+
Hmm, on one hand you're trying to use friendly wrappers like
enabled() active() but at the same time you're modifying data
which these helpers were trying to hide ...
This actually is point from where on intel_slpc_active() is determined.
SLPC enabling is asynchronous and we would need to poll on the shared
memory status to k
Re: [Intel-gfx] [PATCH 21/31] drm/i915/slpc: Send RESET event to enable SLPC during Load/TDR
On Tue, 19 Sep 2017 19:41:57 +0200, Sagar Arun Kamble
wrote:
Send host2guc SLPC reset event to GuC post GuC load.
Post this, i915 can ascertain if SLPC has started running successfully
through shared data. This check is done during intel_init_gt_powersave.
This allows to get initial configuration setup by SLPC and if needed
move to Host RPS if SLPC runs into issues.
On TDR/Engine reset i915 should send extra flag
SLPC_RESET_FLAG_TDR_OCCURREDto clear SLPC state as appropriate.
v1: Extract host2guc_slpc to handle slpc status code
coding style changes (Paulo)
Removed WARN_ON for checking msb of gtt address of
shared gem obj. (ChrisW)
host2guc_action to i915_guc_action change.(Sagar)
Updating SLPC enabled status. (Sagar)
v2: Commit message update. (David)
v3: Rebase.
v4: Added DRM_INFO message when SLPC is enabled.
v5: Updated patch as host2guc_slpc is moved to earlier patch.
SLPC activation status message put after checking the
state from shared data during intel_init_gt_powersave.
v6: Added definition of host2guc_slpc and clflush the shared data only
for required size. Setting state to NOT_RUNNING before sending RESET
event. Output data for SLPC actions is to be retrieved during
intel_guc_send with lock protection so created wrapper
__intel_guc_send that outputs GuC output data if needed. Clearing
pm_rps_events on confirming SLPC RUNNING status so that even if
host touches any of the PM registers by mistake it should not have
any effect. (Sagar)
v7: Added save/restore_default_rps as Uncore sanitize will clear the
RP_CONTROL setup by BIOS. s/i915_ggtt_offset/guc_ggtt_offset.
v8: Added support for handling TDR based SLPC reset. Added functions
host2guc_slpc_tdr_reset, intel_slpc_reset_prepare and
intel_slpc_tdr_reset to handle TDR based SLPC reset.
Cc: Michal Wajdeczko
Signed-off-by: Tom O'Rourke
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/i915_drv.c | 2 +
drivers/gpu/drm/i915/i915_irq.c | 7 +-
drivers/gpu/drm/i915/intel_pm.c | 10 +++
drivers/gpu/drm/i915/intel_slpc.c | 170
++
drivers/gpu/drm/i915/intel_slpc.h | 9 ++
drivers/gpu/drm/i915/intel_uc.c | 1 +
6 files changed, 198 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index f13a3de..932f9ef 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1074,6 +1074,8 @@ static int i915_driver_init_hw(struct
drm_i915_private *dev_priv)
intel_sanitize_options(dev_priv);
+ intel_slpc_save_default_rps(&dev_priv->guc.slpc);
+
ret = i915_ggtt_probe_hw(dev_priv);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 4a1554c..2d5ad13 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2838,8 +2838,13 @@ void i915_handle_error(struct drm_i915_private
*dev_priv,
}
}
- if (!engine_mask)
+ if (!engine_mask) {
+ if (intel_slpc_active(&dev_priv->guc.slpc)) {
+ intel_slpc_reset_prepare(&dev_priv->guc.slpc);
+ intel_slpc_tdr_reset(&dev_priv->guc.slpc);
+ }
Can you just jump to single slpc function that will hide slpc internals ?
goto out;
+ }
/* Full reset needs the mutex, stop any other user trying to do so. */
if (test_and_set_bit(I915_RESET_BACKOFF, &dev_priv->gpu_error.flags)) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 6b2b7f8..c2065f2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7918,6 +7918,16 @@ void intel_init_gt_powersave(struct
drm_i915_private *dev_priv)
intel_runtime_pm_get(dev_priv);
}
+ if (intel_slpc_enabled()) {
+ dev_priv->guc.slpc.active =
+ intel_slpc_get_status(&dev_priv->guc.slpc);
+ if (!intel_slpc_active(&dev_priv->guc.slpc)) {
+ i915.enable_slpc = 0;
+ intel_sanitize_gt_powersave(dev_priv);
+ } else
+ dev_priv->pm_rps_events = 0;
+ }
+
Hmm, on one hand you're trying to use friendly wrappers like
enabled() active() but at the same time you're modifying data
which these helpers were trying to hide ...
mutex_lock(&dev_priv->drm.struct_mutex);
mutex_lock(&dev_priv->pm.pcu_lock);
diff --git a/drivers/gpu/drm/i915/intel_slpc.c
b/drivers/gpu/drm/i915/intel_slpc.c
index f47d81e..57e69d4 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -390,6 +390,140 @@ static void slpc_shared_data_init(struct
intel_slpc *slpc)
kunmap_atomic(data);
}
+static void host2guc_slpc_reset(struct intel_slpc *slpc)
+{
+
[Intel-gfx] [PATCH 21/31] drm/i915/slpc: Send RESET event to enable SLPC during Load/TDR
Send host2guc SLPC reset event to GuC post GuC load.
Post this, i915 can ascertain if SLPC has started running successfully
through shared data. This check is done during intel_init_gt_powersave.
This allows to get initial configuration setup by SLPC and if needed
move to Host RPS if SLPC runs into issues.
On TDR/Engine reset i915 should send extra flag
SLPC_RESET_FLAG_TDR_OCCURREDto clear SLPC state as appropriate.
v1: Extract host2guc_slpc to handle slpc status code
coding style changes (Paulo)
Removed WARN_ON for checking msb of gtt address of
shared gem obj. (ChrisW)
host2guc_action to i915_guc_action change.(Sagar)
Updating SLPC enabled status. (Sagar)
v2: Commit message update. (David)
v3: Rebase.
v4: Added DRM_INFO message when SLPC is enabled.
v5: Updated patch as host2guc_slpc is moved to earlier patch.
SLPC activation status message put after checking the
state from shared data during intel_init_gt_powersave.
v6: Added definition of host2guc_slpc and clflush the shared data only
for required size. Setting state to NOT_RUNNING before sending RESET
event. Output data for SLPC actions is to be retrieved during
intel_guc_send with lock protection so created wrapper
__intel_guc_send that outputs GuC output data if needed. Clearing
pm_rps_events on confirming SLPC RUNNING status so that even if
host touches any of the PM registers by mistake it should not have
any effect. (Sagar)
v7: Added save/restore_default_rps as Uncore sanitize will clear the
RP_CONTROL setup by BIOS. s/i915_ggtt_offset/guc_ggtt_offset.
v8: Added support for handling TDR based SLPC reset. Added functions
host2guc_slpc_tdr_reset, intel_slpc_reset_prepare and
intel_slpc_tdr_reset to handle TDR based SLPC reset.
Cc: Michal Wajdeczko
Signed-off-by: Tom O'Rourke
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/i915_drv.c | 2 +
drivers/gpu/drm/i915/i915_irq.c | 7 +-
drivers/gpu/drm/i915/intel_pm.c | 10 +++
drivers/gpu/drm/i915/intel_slpc.c | 170 ++
drivers/gpu/drm/i915/intel_slpc.h | 9 ++
drivers/gpu/drm/i915/intel_uc.c | 1 +
6 files changed, 198 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f13a3de..932f9ef 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1074,6 +1074,8 @@ static int i915_driver_init_hw(struct drm_i915_private
*dev_priv)
intel_sanitize_options(dev_priv);
+ intel_slpc_save_default_rps(&dev_priv->guc.slpc);
+
ret = i915_ggtt_probe_hw(dev_priv);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4a1554c..2d5ad13 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2838,8 +2838,13 @@ void i915_handle_error(struct drm_i915_private *dev_priv,
}
}
- if (!engine_mask)
+ if (!engine_mask) {
+ if (intel_slpc_active(&dev_priv->guc.slpc)) {
+ intel_slpc_reset_prepare(&dev_priv->guc.slpc);
+ intel_slpc_tdr_reset(&dev_priv->guc.slpc);
+ }
goto out;
+ }
/* Full reset needs the mutex, stop any other user trying to do so. */
if (test_and_set_bit(I915_RESET_BACKOFF, &dev_priv->gpu_error.flags)) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6b2b7f8..c2065f2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7918,6 +7918,16 @@ void intel_init_gt_powersave(struct drm_i915_private
*dev_priv)
intel_runtime_pm_get(dev_priv);
}
+ if (intel_slpc_enabled()) {
+ dev_priv->guc.slpc.active =
+ intel_slpc_get_status(&dev_priv->guc.slpc);
+ if (!intel_slpc_active(&dev_priv->guc.slpc)) {
+ i915.enable_slpc = 0;
+ intel_sanitize_gt_powersave(dev_priv);
+ } else
+ dev_priv->pm_rps_events = 0;
+ }
+
mutex_lock(&dev_priv->drm.struct_mutex);
mutex_lock(&dev_priv->pm.pcu_lock);
diff --git a/drivers/gpu/drm/i915/intel_slpc.c
b/drivers/gpu/drm/i915/intel_slpc.c
index f47d81e..57e69d4 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -390,6 +390,140 @@ static void slpc_shared_data_init(struct intel_slpc *slpc)
kunmap_atomic(data);
}
+static void host2guc_slpc_reset(struct intel_slpc *slpc)
+{
+ struct slpc_event_input data = {0};
+ u32 shared_data_gtt_offset = guc_ggtt_offset(slpc->vma);
+
+ data.header.value = SLPC_EVENT(SLPC_EVENT_RESET, 2);
+ data.args[0] = shared_data_gtt_offset;
+ data.args[1] = 0;
+
+ host2guc_slpc(slpc, &data, 4);
+}
+
+static void host2guc_slpc_tdr_reset(
