* [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
@ 2017-11-05 13:39 Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume Sagar Arun Kamble
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Sagar Arun Kamble @ 2017-11-05 13:39 UTC (permalink / raw)
To: intel-gfx
Releasing GuC doorbell involves stopping the doorbell unit snooping and
executing doorbell deallocation flow in GuC firmware. When GuC is in sane
state, both steps will be necessary to release doorbell.
If GuC is hung (possibly leading to i915 reset), only doorbell unit
snooping is to be stopped. destroy_doorbell will be called in suspend and
reset flows differently in upcoming patches.
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index d14c134..b6486dc 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -207,7 +207,7 @@ static int __create_doorbell(struct i915_guc_client *client)
return err;
}
-static int __destroy_doorbell(struct i915_guc_client *client)
+static int __destroy_doorbell(struct i915_guc_client *client, bool notify_guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(client->guc);
struct guc_doorbell_info *doorbell;
@@ -225,7 +225,10 @@ static int __destroy_doorbell(struct i915_guc_client *client)
if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID), 10))
WARN_ONCE(true, "Doorbell never became invalid after disable\n");
- return __guc_deallocate_doorbell(client->guc, client->stage_id);
+ if (notify_guc)
+ return __guc_deallocate_doorbell(client->guc, client->stage_id);
+
+ return 0;
}
static int create_doorbell(struct i915_guc_client *client)
@@ -250,7 +253,11 @@ static int create_doorbell(struct i915_guc_client *client)
return ret;
}
-static int destroy_doorbell(struct i915_guc_client *client)
+/*
+ * This function deactivates doorbell monitoring through doorbell unit and
+ * optionally notifies GuC to deallocate the doorbell.
+ */
+static int destroy_doorbell(struct i915_guc_client *client, bool notify_guc)
{
int err;
@@ -259,7 +266,7 @@ static int destroy_doorbell(struct i915_guc_client *client)
/* XXX: wait for any interrupts */
/* XXX: wait for workqueue to drain */
- err = __destroy_doorbell(client);
+ err = __destroy_doorbell(client, notify_guc);
if (err)
return err;
@@ -873,7 +880,7 @@ static int __reset_doorbell(struct i915_guc_client* client, u16 db_id)
__update_doorbell_desc(client, db_id);
err = __create_doorbell(client);
if (!err)
- err = __destroy_doorbell(client);
+ err = __destroy_doorbell(client, true);
return err;
}
@@ -899,7 +906,7 @@ static int guc_init_doorbell_hw(struct intel_guc *guc)
if (has_doorbell(client)) {
/* Borrow execbuf_client (we will recreate it later) */
- destroy_doorbell(client);
+ destroy_doorbell(client, true);
recreate_first_client = true;
}
@@ -925,7 +932,7 @@ static int guc_init_doorbell_hw(struct intel_guc *guc)
ret = __create_doorbell(guc->preempt_client);
if (ret) {
- __destroy_doorbell(guc->execbuf_client);
+ __destroy_doorbell(guc->execbuf_client, true);
return ret;
}
@@ -1043,7 +1050,7 @@ static void guc_client_free(struct i915_guc_client *client)
/* FIXME: in many cases, by the time we get here the GuC has been
* reset, so we cannot destroy the doorbell properly. Ignore the
* error message for now */
- destroy_doorbell(client);
+ destroy_doorbell(client, true);
guc_stage_desc_fini(client->guc, client);
i915_gem_object_unpin_map(client->vma->obj);
i915_vma_unpin_and_release(&client->vma);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume
2017-11-05 13:39 [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Sagar Arun Kamble
@ 2017-11-05 13:39 ` Sagar Arun Kamble
2017-11-06 12:13 ` Chris Wilson
2017-11-05 13:39 ` [PATCH 3/5] drm/i915/guc: Deactivate all client doorbells before reset Sagar Arun Kamble
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Sagar Arun Kamble @ 2017-11-05 13:39 UTC (permalink / raw)
To: intel-gfx
Before GT device suspend, i915 should release GuC client doorbells by
stopping doorbell controller snooping and doorbell deallocation through
GuC. They need to be acquired again on resume. This will also resolve
the driver unload issue with GuC, where doorbell deallocation was being
done post GEM suspend.
Release function is called from guc_suspend prior to sending sleep action
during runtime and drm suspend. Acquiral is different in runtime and drm
resume paths as on drm resume we are currently doing full reinit. Release
should be done in synchronization with acquire hence GuC suspend/resume
along with doorbell release/acquire should be under struct_mutex. Upcoming
suspend and resume restructuring for GuC will update these changes.
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 3 +++
drivers/gpu/drm/i915/i915_gem.c | 5 +++--
drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
drivers/gpu/drm/i915/i915_guc_submission.h | 2 ++
drivers/gpu/drm/i915/intel_guc.c | 2 ++
5 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e7e9e06..3df8a7d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -47,6 +47,7 @@
#include <drm/i915_drm.h>
#include "i915_drv.h"
+#include "i915_guc_submission.h"
#include "i915_trace.h"
#include "i915_vgpu.h"
#include "intel_drv.h"
@@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
intel_guc_resume(dev_priv);
+ i915_guc_clients_acquire_doorbells(&dev_priv->guc);
+
if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6a71805..cbce714 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -29,13 +29,14 @@
#include <drm/drm_vma_manager.h>
#include <drm/i915_drm.h>
#include "i915_drv.h"
+#include "i915_gemfs.h"
#include "i915_gem_clflush.h"
-#include "i915_vgpu.h"
+#include "i915_guc_submission.h"
#include "i915_trace.h"
+#include "i915_vgpu.h"
#include "intel_drv.h"
#include "intel_frontbuffer.h"
#include "intel_mocs.h"
-#include "i915_gemfs.h"
#include <linux/dma-fence-array.h>
#include <linux/kthread.h>
#include <linux/reservation.h>
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b6486dc..b989edf 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1047,10 +1047,6 @@ static void guc_client_free(struct i915_guc_client *client)
* Be sure to drop any locks
*/
- /* FIXME: in many cases, by the time we get here the GuC has been
- * reset, so we cannot destroy the doorbell properly. Ignore the
- * error message for now */
- destroy_doorbell(client, true);
guc_stage_desc_fini(client->guc, client);
i915_gem_object_unpin_map(client->vma->obj);
i915_vma_unpin_and_release(&client->vma);
@@ -1102,6 +1098,21 @@ static void guc_clients_destroy(struct intel_guc *guc)
guc_client_free(client);
}
+void i915_guc_clients_acquire_doorbells(struct intel_guc *guc)
+{
+ if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+ return;
+
+ create_doorbell(guc->execbuf_client);
+ create_doorbell(guc->preempt_client);
+}
+
+void i915_guc_clients_release_doorbells(struct intel_guc *guc)
+{
+ destroy_doorbell(guc->preempt_client, true);
+ destroy_doorbell(guc->execbuf_client, true);
+}
+
static void guc_policy_init(struct guc_policy *policy)
{
policy->execution_quantum = POLICY_DEFAULT_EXECUTION_QUANTUM_US;
@@ -1423,6 +1434,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
} else {
guc_reset_wq(guc->execbuf_client);
guc_reset_wq(guc->preempt_client);
+ i915_guc_clients_acquire_doorbells(guc);
}
err = intel_guc_sample_forcewake(guc);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.h b/drivers/gpu/drm/i915/i915_guc_submission.h
index cb4353b..1d5cf22 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.h
+++ b/drivers/gpu/drm/i915/i915_guc_submission.h
@@ -72,6 +72,8 @@ struct i915_guc_client {
u64 submissions[I915_NUM_ENGINES];
};
+void i915_guc_clients_acquire_doorbells(struct intel_guc *guc);
+void i915_guc_clients_release_doorbells(struct intel_guc *guc);
int i915_guc_submission_init(struct drm_i915_private *dev_priv);
int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 9678630..b6eb5ac 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -274,6 +274,8 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
return 0;
+ i915_guc_clients_release_doorbells(&dev_priv->guc);
+
gen9_disable_guc_interrupts(dev_priv);
data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/5] drm/i915/guc: Deactivate all client doorbells before reset
2017-11-05 13:39 [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume Sagar Arun Kamble
@ 2017-11-05 13:39 ` Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update Sagar Arun Kamble
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Sagar Arun Kamble @ 2017-11-05 13:39 UTC (permalink / raw)
To: intel-gfx
Prior to i915 reset, we need to deactivate all client doorbells
as they will need to be reacquired again post reset. We should
not execute GuC doorbell deallocation flow as GuC might be in
bad state (possibly reason for i915 reset).
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 2 ++
drivers/gpu/drm/i915/i915_guc_submission.c | 14 ++++++++++++++
drivers/gpu/drm/i915/i915_guc_submission.h | 1 +
3 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cbce714..879e318 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2963,6 +2963,8 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
i915_gem_revoke_fences(dev_priv);
+ i915_guc_clients_reset_prepare(&dev_priv->guc);
+
return err;
}
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b989edf..21f7fa7 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1113,6 +1113,20 @@ void i915_guc_clients_release_doorbells(struct intel_guc *guc)
destroy_doorbell(guc->execbuf_client, true);
}
+void i915_guc_clients_reset_prepare(struct intel_guc *guc)
+{
+ if (!i915_modparams.enable_guc_submission)
+ return;
+
+ /*
+ * We are only deactivating doorbell unit monitoring. GuC might
+ * be in bad state and will be reset hence don't execute GuC
+ * doorbell deallocation flow.
+ */
+ destroy_doorbell(guc->preempt_client, false);
+ destroy_doorbell(guc->execbuf_client, false);
+}
+
static void guc_policy_init(struct guc_policy *policy)
{
policy->execution_quantum = POLICY_DEFAULT_EXECUTION_QUANTUM_US;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.h b/drivers/gpu/drm/i915/i915_guc_submission.h
index 1d5cf22..91ad505 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.h
+++ b/drivers/gpu/drm/i915/i915_guc_submission.h
@@ -74,6 +74,7 @@ struct i915_guc_client {
void i915_guc_clients_acquire_doorbells(struct intel_guc *guc);
void i915_guc_clients_release_doorbells(struct intel_guc *guc);
+void i915_guc_clients_reset_prepare(struct intel_guc *guc);
int i915_guc_submission_init(struct drm_i915_private *dev_priv);
int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update
2017-11-05 13:39 [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 3/5] drm/i915/guc: Deactivate all client doorbells before reset Sagar Arun Kamble
@ 2017-11-05 13:39 ` Sagar Arun Kamble
2017-11-05 15:55 ` Michal Wajdeczko
2017-11-05 13:39 ` [PATCH 5/5] HAX enable GuC submission for CI Sagar Arun Kamble
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Sagar Arun Kamble @ 2017-11-05 13:39 UTC (permalink / raw)
To: intel-gfx
Current wait timeout of 10us is very tight as seen on SKL/KBL randomly
for pm_rpm@basic-pci-d3-state. Increase it to 50us.
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 21f7fa7..3914415 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -222,7 +222,8 @@ static int __destroy_doorbell(struct i915_guc_client *client, bool notify_guc)
/* Doorbell release flow requires that we wait for GEN8_DRB_VALID bit
* to go to zero after updating db_status before we call the GuC to
* release the doorbell */
- if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID), 10))
+ if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID),
+ 50))
WARN_ONCE(true, "Doorbell never became invalid after disable\n");
if (notify_guc)
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/5] HAX enable GuC submission for CI
2017-11-05 13:39 [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Sagar Arun Kamble
` (2 preceding siblings ...)
2017-11-05 13:39 ` [PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update Sagar Arun Kamble
@ 2017-11-05 13:39 ` Sagar Arun Kamble
2017-11-05 14:18 ` ✗ Fi.CI.BAT: warning for series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Patchwork
2017-11-08 12:32 ` ✓ Fi.CI.BAT: success " Patchwork
5 siblings, 0 replies; 14+ messages in thread
From: Sagar Arun Kamble @ 2017-11-05 13:39 UTC (permalink / raw)
To: intel-gfx
Also revert ("drm/i915/guc: Assert that we switch between
known ggtt->invalidate functions")
---
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) \
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
2017-11-05 13:39 [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Sagar Arun Kamble
` (3 preceding siblings ...)
2017-11-05 13:39 ` [PATCH 5/5] HAX enable GuC submission for CI Sagar Arun Kamble
@ 2017-11-05 14:18 ` Patchwork
2017-11-08 12:32 ` ✓ Fi.CI.BAT: success " Patchwork
5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-11-05 14:18 UTC (permalink / raw)
To: Sagar Arun Kamble; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
URL : https://patchwork.freedesktop.org/series/33209/
State : warning
== Summary ==
Series 33209v1 series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
https://patchwork.freedesktop.org/api/1.0/series/33209/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-c:
dmesg-warn -> PASS (fi-bsw-n3050)
Test drv_module_reload:
Subgroup basic-reload:
pass -> DMESG-WARN (fi-cnl-y)
Subgroup basic-no-display:
pass -> DMESG-WARN (fi-cnl-y)
Subgroup basic-reload-inject:
pass -> DMESG-WARN (fi-cnl-y)
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:444s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:454s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:538s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:276s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:510s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:512s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:503s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:486s
fi-cnl-y total:289 pass:259 dwarn:3 dfail:0 fail:0 skip:27 time:626s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:425s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:265s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:588s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:435s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:430s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:428s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:497s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:466s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:574s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:479s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:590s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:577s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:450s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:592s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:649s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:517s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:502s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:464s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:570s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:428s
8b0ae6b50a229dc661a02f4034252ee854cc9b83 drm-tip: 2017y-11m-03d-17h-15m-57s UTC integration manifest
a91c31b1a24c HAX enable GuC submission for CI
32d332ce5a44 drm/i915/guc: Increase wait timeout for doorbell release status update
be93da66d7df drm/i915/guc: Deactivate all client doorbells before reset
b6515b105ab5 drm/i915/guc: Release all client doorbells on suspend and acquire on resume
9e190e74bacd drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6966/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update
2017-11-05 13:39 ` [PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update Sagar Arun Kamble
@ 2017-11-05 15:55 ` Michal Wajdeczko
2017-11-06 5:43 ` Sagar Arun Kamble
0 siblings, 1 reply; 14+ messages in thread
From: Michal Wajdeczko @ 2017-11-05 15:55 UTC (permalink / raw)
To: intel-gfx, Sagar Arun Kamble
On Sun, 05 Nov 2017 14:39:42 +0100, Sagar Arun Kamble
<sagar.a.kamble@intel.com> wrote:
> Current wait timeout of 10us is very tight as seen on SKL/KBL randomly
> for pm_rpm@basic-pci-d3-state. Increase it to 50us.
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 21f7fa7..3914415 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -222,7 +222,8 @@ static int __destroy_doorbell(struct i915_guc_client
> *client, bool notify_guc)
> /* Doorbell release flow requires that we wait for GEN8_DRB_VALID bit
> * to go to zero after updating db_status before we call the GuC to
> * release the doorbell */
> - if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID),
> 10))
> + if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID),
> + 50))
Can we use intel_wait_for_register() here ?
> WARN_ONCE(true, "Doorbell never became invalid after disable\n");
> if (notify_guc)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update
2017-11-05 15:55 ` Michal Wajdeczko
@ 2017-11-06 5:43 ` Sagar Arun Kamble
0 siblings, 0 replies; 14+ messages in thread
From: Sagar Arun Kamble @ 2017-11-06 5:43 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On 11/5/2017 9:25 PM, Michal Wajdeczko wrote:
> On Sun, 05 Nov 2017 14:39:42 +0100, Sagar Arun Kamble
> <sagar.a.kamble@intel.com> wrote:
>
>> Current wait timeout of 10us is very tight as seen on SKL/KBL randomly
>> for pm_rpm@basic-pci-d3-state. Increase it to 50us.
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_guc_submission.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index 21f7fa7..3914415 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -222,7 +222,8 @@ static int __destroy_doorbell(struct
>> i915_guc_client *client, bool notify_guc)
>> /* Doorbell release flow requires that we wait for
>> GEN8_DRB_VALID bit
>> * to go to zero after updating db_status before we call the GuC to
>> * release the doorbell */
>> - if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) &
>> GEN8_DRB_VALID), 10))
>> + if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID),
>> + 50))
>
> Can we use intel_wait_for_register() here ?
Yes. Since wait timeout is expected to be in the order of uS (50us for
now) will be using __intel_wait_for_register_fw.
+ if (__intel_wait_for_register_fw(dev_priv, GEN8_DRBREGL(db_id),
GEN8_DRB_VALID, 0,
+ 50, 1, NULL))
Is this fine (with slow timeout of 1ms)?
>> WARN_ONCE(true, "Doorbell never became invalid after
>> disable\n");
>> if (notify_guc)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume
2017-11-05 13:39 ` [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume Sagar Arun Kamble
@ 2017-11-06 12:13 ` Chris Wilson
2017-11-07 6:05 ` Sagar Arun Kamble
0 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2017-11-06 12:13 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx; +Cc: Sagar
Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
> Before GT device suspend, i915 should release GuC client doorbells by
> stopping doorbell controller snooping and doorbell deallocation through
> GuC. They need to be acquired again on resume. This will also resolve
> the driver unload issue with GuC, where doorbell deallocation was being
> done post GEM suspend.
> Release function is called from guc_suspend prior to sending sleep action
> during runtime and drm suspend. Acquiral is different in runtime and drm
> resume paths as on drm resume we are currently doing full reinit. Release
> should be done in synchronization with acquire hence GuC suspend/resume
> along with doorbell release/acquire should be under struct_mutex. Upcoming
> suspend and resume restructuring for GuC will update these changes.
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 3 +++
> drivers/gpu/drm/i915/i915_gem.c | 5 +++--
> drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
> drivers/gpu/drm/i915/i915_guc_submission.h | 2 ++
> drivers/gpu/drm/i915/intel_guc.c | 2 ++
> 5 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e7e9e06..3df8a7d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -47,6 +47,7 @@
> #include <drm/i915_drm.h>
>
> #include "i915_drv.h"
> +#include "i915_guc_submission.h"
> #include "i915_trace.h"
> #include "i915_vgpu.h"
> #include "intel_drv.h"
> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
>
> intel_guc_resume(dev_priv);
>
> + i915_guc_clients_acquire_doorbells(&dev_priv->guc);
intel_guc_acquire_doorbells();
Though, if we need to specialise between resume and runtime_resume, I
suggest adding intel_guc_resume() and intel_guc_runtime_resume() entry
points. (We probably should find a better way to distinguish those two
paths, system_resume vs runtime_resume?)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume
2017-11-06 12:13 ` Chris Wilson
@ 2017-11-07 6:05 ` Sagar Arun Kamble
2017-11-07 9:21 ` Chris Wilson
0 siblings, 1 reply; 14+ messages in thread
From: Sagar Arun Kamble @ 2017-11-07 6:05 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Cc: Sagar Arun Kamble , " Michał Winiarski , Michel Thierry ,
Michal Wajdeczko , Arkadiusz Hiler ,
Joonas Lahtinen
On 11/6/2017 5:43 PM, Chris Wilson wrote:
> Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
>> Before GT device suspend, i915 should release GuC client doorbells by
>> stopping doorbell controller snooping and doorbell deallocation through
>> GuC. They need to be acquired again on resume. This will also resolve
>> the driver unload issue with GuC, where doorbell deallocation was being
>> done post GEM suspend.
>> Release function is called from guc_suspend prior to sending sleep action
>> during runtime and drm suspend. Acquiral is different in runtime and drm
>> resume paths as on drm resume we are currently doing full reinit. Release
>> should be done in synchronization with acquire hence GuC suspend/resume
>> along with doorbell release/acquire should be under struct_mutex. Upcoming
>> suspend and resume restructuring for GuC will update these changes.
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 3 +++
>> drivers/gpu/drm/i915/i915_gem.c | 5 +++--
>> drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
>> drivers/gpu/drm/i915/i915_guc_submission.h | 2 ++
>> drivers/gpu/drm/i915/intel_guc.c | 2 ++
>> 5 files changed, 26 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index e7e9e06..3df8a7d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -47,6 +47,7 @@
>> #include <drm/i915_drm.h>
>>
>> #include "i915_drv.h"
>> +#include "i915_guc_submission.h"
>> #include "i915_trace.h"
>> #include "i915_vgpu.h"
>> #include "intel_drv.h"
>> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
>>
>> intel_guc_resume(dev_priv);
>>
>> + i915_guc_clients_acquire_doorbells(&dev_priv->guc);
> intel_guc_acquire_doorbells();
Prefixed "i915_guc_clients" since this modifies submission state
(clients/doorbells). Should have kept dev_priv as parameter.
what should be the correct option here: intel_guc*(guc) or
i915_guc*(dev_priv)
>
> Though, if we need to specialise between resume and runtime_resume, I
> suggest adding intel_guc_resume() and intel_guc_runtime_resume() entry
> points. (We probably should find a better way to distinguish those two
> paths, system_resume vs runtime_resume?)
Yes. these will be added in the upcoming GuC suspend/resume
restructuring series.
Functionality along runtime/system suspend path will be same for GuC.
Will defer in
the resume path since we are doing full gem reinit on resume from system
suspend.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume
2017-11-07 6:05 ` Sagar Arun Kamble
@ 2017-11-07 9:21 ` Chris Wilson
2017-11-07 10:27 ` Michal Wajdeczko
0 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2017-11-07 9:21 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx
Cc: =?utf-8?q?Sagar_Arun_Kamble_=2C__=22_Micha=C5=82_Winiarski_=2C__Michel_Th?=
Quoting Sagar Arun Kamble (2017-11-07 06:05:01)
>
>
> On 11/6/2017 5:43 PM, Chris Wilson wrote:
> > Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
> >> Before GT device suspend, i915 should release GuC client doorbells by
> >> stopping doorbell controller snooping and doorbell deallocation through
> >> GuC. They need to be acquired again on resume. This will also resolve
> >> the driver unload issue with GuC, where doorbell deallocation was being
> >> done post GEM suspend.
> >> Release function is called from guc_suspend prior to sending sleep action
> >> during runtime and drm suspend. Acquiral is different in runtime and drm
> >> resume paths as on drm resume we are currently doing full reinit. Release
> >> should be done in synchronization with acquire hence GuC suspend/resume
> >> along with doorbell release/acquire should be under struct_mutex. Upcoming
> >> suspend and resume restructuring for GuC will update these changes.
> >>
> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Michał Winiarski <michal.winiarski@intel.com>
> >> Cc: Michel Thierry <michel.thierry@intel.com>
> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.c | 3 +++
> >> drivers/gpu/drm/i915/i915_gem.c | 5 +++--
> >> drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
> >> drivers/gpu/drm/i915/i915_guc_submission.h | 2 ++
> >> drivers/gpu/drm/i915/intel_guc.c | 2 ++
> >> 5 files changed, 26 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index e7e9e06..3df8a7d 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -47,6 +47,7 @@
> >> #include <drm/i915_drm.h>
> >>
> >> #include "i915_drv.h"
> >> +#include "i915_guc_submission.h"
> >> #include "i915_trace.h"
> >> #include "i915_vgpu.h"
> >> #include "intel_drv.h"
> >> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
> >>
> >> intel_guc_resume(dev_priv);
> >>
> >> + i915_guc_clients_acquire_doorbells(&dev_priv->guc);
> > intel_guc_acquire_doorbells();
> Prefixed "i915_guc_clients" since this modifies submission state
> (clients/doorbells). Should have kept dev_priv as parameter.
> what should be the correct option here: intel_guc*(guc) or
> i915_guc*(dev_priv)
GuC submission is not i915. It is not part of the user facing api.
Operate on intel_guc as you were.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume
2017-11-07 9:21 ` Chris Wilson
@ 2017-11-07 10:27 ` Michal Wajdeczko
2017-11-07 10:33 ` Sagar Arun Kamble
0 siblings, 1 reply; 14+ messages in thread
From: Michal Wajdeczko @ 2017-11-07 10:27 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx, Chris Wilson
On Tue, 07 Nov 2017 10:21:17 +0100, Chris Wilson
<chris@chris-wilson.co.uk> wrote:
> Quoting Sagar Arun Kamble (2017-11-07 06:05:01)
>>
>>
>> On 11/6/2017 5:43 PM, Chris Wilson wrote:
>> > Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
>> >> Before GT device suspend, i915 should release GuC client doorbells by
>> >> stopping doorbell controller snooping and doorbell deallocation
>> through
>> >> GuC. They need to be acquired again on resume. This will also resolve
>> >> the driver unload issue with GuC, where doorbell deallocation was
>> being
>> >> done post GEM suspend.
>> >> Release function is called from guc_suspend prior to sending sleep
>> action
>> >> during runtime and drm suspend. Acquiral is different in runtime and
>> drm
>> >> resume paths as on drm resume we are currently doing full reinit.
>> Release
>> >> should be done in synchronization with acquire hence GuC
>> suspend/resume
>> >> along with doorbell release/acquire should be under struct_mutex.
>> Upcoming
>> >> suspend and resume restructuring for GuC will update these changes.
>> >>
>> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> >> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> >> Cc: Michel Thierry <michel.thierry@intel.com>
>> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> >> ---
>> >> drivers/gpu/drm/i915/i915_drv.c | 3 +++
>> >> drivers/gpu/drm/i915/i915_gem.c | 5 +++--
>> >> drivers/gpu/drm/i915/i915_guc_submission.c | 20
>> ++++++++++++++++----
>> >> drivers/gpu/drm/i915/i915_guc_submission.h | 2 ++
>> >> drivers/gpu/drm/i915/intel_guc.c | 2 ++
>> >> 5 files changed, 26 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c
>> >> index e7e9e06..3df8a7d 100644
>> >> --- a/drivers/gpu/drm/i915/i915_drv.c
>> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> >> @@ -47,6 +47,7 @@
>> >> #include <drm/i915_drm.h>
>> >>
>> >> #include "i915_drv.h"
>> >> +#include "i915_guc_submission.h"
>> >> #include "i915_trace.h"
>> >> #include "i915_vgpu.h"
>> >> #include "intel_drv.h"
>> >> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device
>> *kdev)
>> >>
>> >> intel_guc_resume(dev_priv);
>> >>
>> >> + i915_guc_clients_acquire_doorbells(&dev_priv->guc);
>> > intel_guc_acquire_doorbells();
>> Prefixed "i915_guc_clients" since this modifies submission state
>> (clients/doorbells). Should have kept dev_priv as parameter.
>> what should be the correct option here: intel_guc*(guc) or
>> i915_guc*(dev_priv)
>
> GuC submission is not i915. It is not part of the user facing api.
> Operate on intel_guc as you were.
So if GuC submission is not i915 than maybe to avoid confusion we should
rename i915_guc_submission.c to intel_guc_submission.c ?
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume
2017-11-07 10:27 ` Michal Wajdeczko
@ 2017-11-07 10:33 ` Sagar Arun Kamble
0 siblings, 0 replies; 14+ messages in thread
From: Sagar Arun Kamble @ 2017-11-07 10:33 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx, Chris Wilson
On 11/7/2017 3:57 PM, Michal Wajdeczko wrote:
> On Tue, 07 Nov 2017 10:21:17 +0100, Chris Wilson
> <chris@chris-wilson.co.uk> wrote:
>
>> Quoting Sagar Arun Kamble (2017-11-07 06:05:01)
>>>
>>>
>>> On 11/6/2017 5:43 PM, Chris Wilson wrote:
>>> > Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
>>> >> Before GT device suspend, i915 should release GuC client
>>> doorbells by
>>> >> stopping doorbell controller snooping and doorbell deallocation
>>> through
>>> >> GuC. They need to be acquired again on resume. This will also
>>> resolve
>>> >> the driver unload issue with GuC, where doorbell deallocation was
>>> being
>>> >> done post GEM suspend.
>>> >> Release function is called from guc_suspend prior to sending
>>> sleep action
>>> >> during runtime and drm suspend. Acquiral is different in runtime
>>> and drm
>>> >> resume paths as on drm resume we are currently doing full reinit.
>>> Release
>>> >> should be done in synchronization with acquire hence GuC
>>> suspend/resume
>>> >> along with doorbell release/acquire should be under struct_mutex.
>>> Upcoming
>>> >> suspend and resume restructuring for GuC will update these changes.
>>> >>
>>> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> >> Cc: Michał Winiarski <michal.winiarski@intel.com>
>>> >> Cc: Michel Thierry <michel.thierry@intel.com>
>>> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>>> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> >> ---
>>> >> drivers/gpu/drm/i915/i915_drv.c | 3 +++
>>> >> drivers/gpu/drm/i915/i915_gem.c | 5 +++--
>>> >> drivers/gpu/drm/i915/i915_guc_submission.c | 20
>>> ++++++++++++++++----
>>> >> drivers/gpu/drm/i915/i915_guc_submission.h | 2 ++
>>> >> drivers/gpu/drm/i915/intel_guc.c | 2 ++
>>> >> 5 files changed, 26 insertions(+), 6 deletions(-)
>>> >>
>>> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>>> b/drivers/gpu/drm/i915/i915_drv.c
>>> >> index e7e9e06..3df8a7d 100644
>>> >> --- a/drivers/gpu/drm/i915/i915_drv.c
>>> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>> >> @@ -47,6 +47,7 @@
>>> >> #include <drm/i915_drm.h>
>>> >>
>>> >> #include "i915_drv.h"
>>> >> +#include "i915_guc_submission.h"
>>> >> #include "i915_trace.h"
>>> >> #include "i915_vgpu.h"
>>> >> #include "intel_drv.h"
>>> >> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct
>>> device *kdev)
>>> >>
>>> >> intel_guc_resume(dev_priv);
>>> >>
>>> >> + i915_guc_clients_acquire_doorbells(&dev_priv->guc);
>>> > intel_guc_acquire_doorbells();
>>> Prefixed "i915_guc_clients" since this modifies submission state
>>> (clients/doorbells). Should have kept dev_priv as parameter.
>>> what should be the correct option here: intel_guc*(guc) or
>>> i915_guc*(dev_priv)
>>
>> GuC submission is not i915. It is not part of the user facing api.
>> Operate on intel_guc as you were.
>
> So if GuC submission is not i915 than maybe to avoid confusion we should
> rename i915_guc_submission.c to intel_guc_submission.c ?
Yes. Will need to update functions that are i915_guc* as well. (spotted
some in guc_log.c as well.)
>
> Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
2017-11-05 13:39 [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Sagar Arun Kamble
` (4 preceding siblings ...)
2017-11-05 14:18 ` ✗ Fi.CI.BAT: warning for series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Patchwork
@ 2017-11-08 12:32 ` Patchwork
5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-11-08 12:32 UTC (permalink / raw)
To: Sagar Arun Kamble; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
URL : https://patchwork.freedesktop.org/series/33209/
State : success
== Summary ==
Series 33209v1 series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
https://patchwork.freedesktop.org/api/1.0/series/33209/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-c:
dmesg-warn -> PASS (fi-bsw-n3050) fdo#103479
fdo#103479 https://bugs.freedesktop.org/show_bug.cgi?id=103479
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:444s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:454s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:538s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:276s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:510s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:512s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:503s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:486s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:425s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:265s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:588s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:435s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:430s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:428s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:497s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:466s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:574s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:479s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:590s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:577s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:450s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:592s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:649s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:517s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:502s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:464s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:570s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:428s
Blacklisted hosts:
fi-cnl-y total:289 pass:259 dwarn:3 dfail:0 fail:0 skip:27 time:626s
8b0ae6b50a229dc661a02f4034252ee854cc9b83 drm-tip: 2017y-11m-03d-17h-15m-57s UTC integration manifest
a91c31b1a24c HAX enable GuC submission for CI
32d332ce5a44 drm/i915/guc: Increase wait timeout for doorbell release status update
be93da66d7df drm/i915/guc: Deactivate all client doorbells before reset
b6515b105ab5 drm/i915/guc: Release all client doorbells on suspend and acquire on resume
9e190e74bacd drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6966/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-11-08 12:32 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-05 13:39 [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume Sagar Arun Kamble
2017-11-06 12:13 ` Chris Wilson
2017-11-07 6:05 ` Sagar Arun Kamble
2017-11-07 9:21 ` Chris Wilson
2017-11-07 10:27 ` Michal Wajdeczko
2017-11-07 10:33 ` Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 3/5] drm/i915/guc: Deactivate all client doorbells before reset Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update Sagar Arun Kamble
2017-11-05 15:55 ` Michal Wajdeczko
2017-11-06 5:43 ` Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 5/5] HAX enable GuC submission for CI Sagar Arun Kamble
2017-11-05 14:18 ` ✗ Fi.CI.BAT: warning for series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Patchwork
2017-11-08 12:32 ` ✓ Fi.CI.BAT: success " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.