* [CI 1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
Remove unused enums and ctx_save_restore_disabled() function, leftover
from the legacy preemption removal.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 22 -------------------
1 file changed, 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 902e25eb6b9d..6d4eb7690db1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -18,15 +18,6 @@
#include "i915_drv.h"
#include "i915_trace.h"
-enum {
- GUC_PREEMPT_NONE = 0,
- GUC_PREEMPT_INPROGRESS,
- GUC_PREEMPT_FINISHED,
-};
-#define GUC_PREEMPT_BREADCRUMB_DWORDS 0x8
-#define GUC_PREEMPT_BREADCRUMB_BYTES \
- (sizeof(u32) * GUC_PREEMPT_BREADCRUMB_DWORDS)
-
/**
* DOC: GuC-based command submission
*
@@ -884,19 +875,6 @@ static void guc_client_free(struct intel_guc_client *client)
kfree(client);
}
-static inline bool ctx_save_restore_disabled(struct intel_context *ce)
-{
- u32 sr = ce->lrc_reg_state[CTX_CONTEXT_CONTROL + 1];
-
-#define SR_DISABLED \
- _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | \
- CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT)
-
- return (sr & SR_DISABLED) == SR_DISABLED;
-
-#undef SR_DISABLED
-}
-
static int guc_clients_create(struct intel_guc *guc)
{
struct intel_guc_client *client;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Intel-gfx] [CI 1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
Remove unused enums and ctx_save_restore_disabled() function, leftover
from the legacy preemption removal.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 22 -------------------
1 file changed, 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 902e25eb6b9d..6d4eb7690db1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -18,15 +18,6 @@
#include "i915_drv.h"
#include "i915_trace.h"
-enum {
- GUC_PREEMPT_NONE = 0,
- GUC_PREEMPT_INPROGRESS,
- GUC_PREEMPT_FINISHED,
-};
-#define GUC_PREEMPT_BREADCRUMB_DWORDS 0x8
-#define GUC_PREEMPT_BREADCRUMB_BYTES \
- (sizeof(u32) * GUC_PREEMPT_BREADCRUMB_DWORDS)
-
/**
* DOC: GuC-based command submission
*
@@ -884,19 +875,6 @@ static void guc_client_free(struct intel_guc_client *client)
kfree(client);
}
-static inline bool ctx_save_restore_disabled(struct intel_context *ce)
-{
- u32 sr = ce->lrc_reg_state[CTX_CONTEXT_CONTROL + 1];
-
-#define SR_DISABLED \
- _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT | \
- CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT)
-
- return (sr & SR_DISABLED) == SR_DISABLED;
-
-#undef SR_DISABLED
-}
-
static int guc_clients_create(struct intel_guc *guc)
{
struct intel_guc_client *client;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [CI 2/5] drm/i915/guc: add a helper to allocate and map guc vma
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
We already have a couple of use-cases in the code and another one will
come in one of the later patches in the series.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 34 +++++++++++++++++++
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 2 ++
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 21 +++---------
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 22 ++++--------
4 files changed, 47 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 3ee4a4e7689d..922a19635d20 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -704,3 +704,37 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
i915_gem_object_put(obj);
return vma;
}
+
+/**
+ * intel_guc_allocate_and_map_vma() - Allocate and map VMA for GuC usage
+ * @guc: the guc
+ * @size: size of area to allocate (both virtual space and memory)
+ * @out_vma: return variable for the allocated vma pointer
+ * @out_vaddr: return variable for the obj mapping
+ *
+ * This wrapper calls intel_guc_allocate_vma() and then maps the allocated
+ * object with I915_MAP_WB.
+ *
+ * Return: 0 if successful, a negative errno code otherwise.
+ */
+int intel_guc_allocate_and_map_vma(struct intel_guc *guc, u32 size,
+ struct i915_vma **out_vma, void **out_vaddr)
+{
+ struct i915_vma *vma;
+ void *vaddr;
+
+ vma = intel_guc_allocate_vma(guc, size);
+ if (IS_ERR(vma))
+ return PTR_ERR(vma);
+
+ vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
+ if (IS_ERR(vaddr)) {
+ i915_vma_unpin_and_release(&vma, 0);
+ return PTR_ERR(vaddr);
+ }
+
+ *out_vma = vma;
+ *out_vaddr = vaddr;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index e6400204a2bd..bf438f820c35 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -149,6 +149,8 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
int intel_guc_suspend(struct intel_guc *guc);
int intel_guc_resume(struct intel_guc *guc);
struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
+int intel_guc_allocate_and_map_vma(struct intel_guc *guc, u32 size,
+ struct i915_vma **out_vma, void **out_vaddr);
static inline bool intel_guc_is_supported(struct intel_guc *guc)
{
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
index ca6674b8e00c..6334d773c3e1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
@@ -135,32 +135,19 @@ static void __guc_ads_init(struct intel_guc *guc)
int intel_guc_ads_create(struct intel_guc *guc)
{
const u32 size = PAGE_ALIGN(sizeof(struct __guc_ads_blob));
- struct i915_vma *vma;
- void *blob;
int ret;
GEM_BUG_ON(guc->ads_vma);
- vma = intel_guc_allocate_vma(guc, size);
- if (IS_ERR(vma))
- return PTR_ERR(vma);
+ ret = intel_guc_allocate_and_map_vma(guc, size, &guc->ads_vma,
+ (void **)&guc->ads_blob);
- blob = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
- if (IS_ERR(blob)) {
- ret = PTR_ERR(blob);
- goto err_vma;
- }
-
- guc->ads_vma = vma;
- guc->ads_blob = blob;
+ if (ret)
+ return ret;
__guc_ads_init(guc);
return 0;
-
-err_vma:
- i915_vma_unpin_and_release(&guc->ads_vma, 0);
- return ret;
}
void intel_guc_ads_destroy(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 6d4eb7690db1..445e0e3c4f11 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -308,23 +308,15 @@ static void guc_proc_desc_fini(struct intel_guc_client *client)
static int guc_stage_desc_pool_create(struct intel_guc *guc)
{
- struct i915_vma *vma;
- void *vaddr;
-
- vma = intel_guc_allocate_vma(guc,
- PAGE_ALIGN(sizeof(struct guc_stage_desc) *
- GUC_MAX_STAGE_DESCRIPTORS));
- if (IS_ERR(vma))
- return PTR_ERR(vma);
+ u32 size = PAGE_ALIGN(sizeof(struct guc_stage_desc) *
+ GUC_MAX_STAGE_DESCRIPTORS);
+ int ret;
- vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
- if (IS_ERR(vaddr)) {
- i915_vma_unpin_and_release(&vma, 0);
- return PTR_ERR(vaddr);
- }
+ ret = intel_guc_allocate_and_map_vma(guc, size, &guc->stage_desc_pool,
+ &guc->stage_desc_pool_vaddr);
+ if (ret)
+ return ret;
- guc->stage_desc_pool = vma;
- guc->stage_desc_pool_vaddr = vaddr;
ida_init(&guc->stage_ids);
return 0;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Intel-gfx] [CI 2/5] drm/i915/guc: add a helper to allocate and map guc vma
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
We already have a couple of use-cases in the code and another one will
come in one of the later patches in the series.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 34 +++++++++++++++++++
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 2 ++
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 21 +++---------
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 22 ++++--------
4 files changed, 47 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 3ee4a4e7689d..922a19635d20 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -704,3 +704,37 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
i915_gem_object_put(obj);
return vma;
}
+
+/**
+ * intel_guc_allocate_and_map_vma() - Allocate and map VMA for GuC usage
+ * @guc: the guc
+ * @size: size of area to allocate (both virtual space and memory)
+ * @out_vma: return variable for the allocated vma pointer
+ * @out_vaddr: return variable for the obj mapping
+ *
+ * This wrapper calls intel_guc_allocate_vma() and then maps the allocated
+ * object with I915_MAP_WB.
+ *
+ * Return: 0 if successful, a negative errno code otherwise.
+ */
+int intel_guc_allocate_and_map_vma(struct intel_guc *guc, u32 size,
+ struct i915_vma **out_vma, void **out_vaddr)
+{
+ struct i915_vma *vma;
+ void *vaddr;
+
+ vma = intel_guc_allocate_vma(guc, size);
+ if (IS_ERR(vma))
+ return PTR_ERR(vma);
+
+ vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
+ if (IS_ERR(vaddr)) {
+ i915_vma_unpin_and_release(&vma, 0);
+ return PTR_ERR(vaddr);
+ }
+
+ *out_vma = vma;
+ *out_vaddr = vaddr;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index e6400204a2bd..bf438f820c35 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -149,6 +149,8 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
int intel_guc_suspend(struct intel_guc *guc);
int intel_guc_resume(struct intel_guc *guc);
struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
+int intel_guc_allocate_and_map_vma(struct intel_guc *guc, u32 size,
+ struct i915_vma **out_vma, void **out_vaddr);
static inline bool intel_guc_is_supported(struct intel_guc *guc)
{
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
index ca6674b8e00c..6334d773c3e1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
@@ -135,32 +135,19 @@ static void __guc_ads_init(struct intel_guc *guc)
int intel_guc_ads_create(struct intel_guc *guc)
{
const u32 size = PAGE_ALIGN(sizeof(struct __guc_ads_blob));
- struct i915_vma *vma;
- void *blob;
int ret;
GEM_BUG_ON(guc->ads_vma);
- vma = intel_guc_allocate_vma(guc, size);
- if (IS_ERR(vma))
- return PTR_ERR(vma);
+ ret = intel_guc_allocate_and_map_vma(guc, size, &guc->ads_vma,
+ (void **)&guc->ads_blob);
- blob = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
- if (IS_ERR(blob)) {
- ret = PTR_ERR(blob);
- goto err_vma;
- }
-
- guc->ads_vma = vma;
- guc->ads_blob = blob;
+ if (ret)
+ return ret;
__guc_ads_init(guc);
return 0;
-
-err_vma:
- i915_vma_unpin_and_release(&guc->ads_vma, 0);
- return ret;
}
void intel_guc_ads_destroy(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 6d4eb7690db1..445e0e3c4f11 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -308,23 +308,15 @@ static void guc_proc_desc_fini(struct intel_guc_client *client)
static int guc_stage_desc_pool_create(struct intel_guc *guc)
{
- struct i915_vma *vma;
- void *vaddr;
-
- vma = intel_guc_allocate_vma(guc,
- PAGE_ALIGN(sizeof(struct guc_stage_desc) *
- GUC_MAX_STAGE_DESCRIPTORS));
- if (IS_ERR(vma))
- return PTR_ERR(vma);
+ u32 size = PAGE_ALIGN(sizeof(struct guc_stage_desc) *
+ GUC_MAX_STAGE_DESCRIPTORS);
+ int ret;
- vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
- if (IS_ERR(vaddr)) {
- i915_vma_unpin_and_release(&vma, 0);
- return PTR_ERR(vaddr);
- }
+ ret = intel_guc_allocate_and_map_vma(guc, size, &guc->stage_desc_pool,
+ &guc->stage_desc_pool_vaddr);
+ if (ret)
+ return ret;
- guc->stage_desc_pool = vma;
- guc->stage_desc_pool_vaddr = vaddr;
ida_init(&guc->stage_ids);
return 0;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [CI 3/5] drm/i915/guc: kill doorbell code and selftests
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
Instead of relying on the workqueue, the upcoming reworked GuC
submission flow will offer the host driver indipendent control over
the execution status of each context submitted to GuC. As part of this,
the doorbell usage model has been reworked, with each doorbell being
paired to a single lrc and a doorbell ring representing new work
available for that specific context. This mechanism, however, limits
the number of contexts that can be registered with GuC to the number of
doorbells, which is an undesired limitation. To avoid this limitation,
we requested the GuC team to also provide a H2G that will allow the host
to notify the GuC of work available for a specified lrc, so we can use
that mechanism instead of relying on the doorbells. We can therefore drop
the doorbell code we currently have, also given the fact that in the
unlikely case we'd want to switch back to using doorbells we'd have to
heavily rework it.
The workqueue will still have a use in the new interface to pass special
commands, so that code has been retained for now.
With the doorbells gone and the GuC client becoming even simpler, the
existing GuC selftests don't give us any meaningful coverage so we can
remove them as well. Some selftests might come with the new code, but
they will look different from what we have now so if doesn't seem worth
it to keep the file around in the meantime.
v2: fix comments and commit message (John)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 8 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 2 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 376 ++----------------
.../gpu/drm/i915/gt/uc/intel_guc_submission.h | 9 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 7 +-
drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 299 --------------
drivers/gpu/drm/i915/i915_debugfs.c | 11 +-
.../drm/i915/selftests/i915_live_selftests.h | 1 -
8 files changed, 44 insertions(+), 669 deletions(-)
delete mode 100644 drivers/gpu/drm/i915/gt/uc/selftest_guc.c
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index bf438f820c35..b2d1766e689a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -20,8 +20,8 @@ struct __guc_ads_blob;
/*
* Top level structure of GuC. It handles firmware loading and manages client
- * pool and doorbells. intel_guc owns a intel_guc_client to replace the legacy
- * ExecList submission.
+ * pool. intel_guc owns a intel_guc_client to replace the legacy ExecList
+ * submission.
*/
struct intel_guc {
struct intel_uc_fw fw;
@@ -50,10 +50,6 @@ struct intel_guc {
struct intel_guc_client *execbuf_client;
- DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
- /* Cyclic counter mod pagesize */
- u32 db_cacheline;
-
/* Control params for fw initialization */
u32 params[GUC_CTL_MAX_DWORDS];
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
index a26a85d50209..1e8e4af7d9ca 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
@@ -31,7 +31,7 @@
#define GUC_DOORBELL_INVALID 256
-#define GUC_DB_SIZE (PAGE_SIZE)
+#define GUC_PD_SIZE (PAGE_SIZE)
#define GUC_WQ_SIZE (PAGE_SIZE * 2)
/* Work queue item header definitions */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 445e0e3c4f11..0b89da85885c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -30,8 +30,8 @@
* GuC client:
* A intel_guc_client refers to a submission path through GuC. Currently, there
* is only one client, which is charged with all submissions to the GuC. This
- * struct is the owner of a doorbell, a process descriptor and a workqueue (all
- * of them inside a single gem object that contains all required pages for these
+ * struct is the owner of a process descriptor and a workqueue (both of them
+ * inside a single gem object that contains all required pages for these
* elements).
*
* GuC stage descriptor:
@@ -39,13 +39,13 @@
* descriptors, and shares them with the GuC.
* Currently, there exists a 1:1 mapping between a intel_guc_client and a
* guc_stage_desc (via the client's stage_id), so effectively only one
- * gets used. This stage descriptor lets the GuC know about the doorbell,
- * workqueue and process descriptor. Theoretically, it also lets the GuC
- * know about our HW contexts (context ID, etc...), but we actually
- * employ a kind of submission where the GuC uses the LRCA sent via the work
- * item instead (the single guc_stage_desc associated to execbuf client
- * contains information about the default kernel context only, but this is
- * essentially unused). This is called a "proxy" submission.
+ * gets used. This stage descriptor lets the GuC know about the workqueue and
+ * process descriptor. Theoretically, it also lets the GuC know about our HW
+ * contexts (context ID, etc...), but we actually employ a kind of submission
+ * where the GuC uses the LRCA sent via the work item instead (the single
+ * guc_stage_desc associated to execbuf client contains information about the
+ * default kernel context only, but this is essentially unused). This is called
+ * a "proxy" submission.
*
* The Scratch registers:
* There are 16 MMIO-based registers start from 0xC180. The kernel driver writes
@@ -56,10 +56,6 @@
* then proceeds.
* See intel_guc_send()
*
- * Doorbells:
- * Doorbells are interrupts to uKernel. A doorbell is a single cache line (QW)
- * mapped into process space.
- *
* Work Items:
* There are several types of work items that the host may place into a
* workqueue, each with its own requirements and limitations. Currently only
@@ -81,78 +77,6 @@ static inline bool is_high_priority(struct intel_guc_client *client)
client->priority == GUC_CLIENT_PRIORITY_HIGH);
}
-static int reserve_doorbell(struct intel_guc_client *client)
-{
- unsigned long offset;
- unsigned long end;
- u16 id;
-
- GEM_BUG_ON(client->doorbell_id != GUC_DOORBELL_INVALID);
-
- /*
- * The bitmap tracks which doorbell registers are currently in use.
- * It is split into two halves; the first half is used for normal
- * priority contexts, the second half for high-priority ones.
- */
- offset = 0;
- end = GUC_NUM_DOORBELLS / 2;
- if (is_high_priority(client)) {
- offset = end;
- end += offset;
- }
-
- id = find_next_zero_bit(client->guc->doorbell_bitmap, end, offset);
- if (id == end)
- return -ENOSPC;
-
- __set_bit(id, client->guc->doorbell_bitmap);
- client->doorbell_id = id;
- DRM_DEBUG_DRIVER("client %u (high prio=%s) reserved doorbell: %d\n",
- client->stage_id, yesno(is_high_priority(client)),
- id);
- return 0;
-}
-
-static bool has_doorbell(struct intel_guc_client *client)
-{
- if (client->doorbell_id == GUC_DOORBELL_INVALID)
- return false;
-
- return test_bit(client->doorbell_id, client->guc->doorbell_bitmap);
-}
-
-static void unreserve_doorbell(struct intel_guc_client *client)
-{
- GEM_BUG_ON(!has_doorbell(client));
-
- __clear_bit(client->doorbell_id, client->guc->doorbell_bitmap);
- client->doorbell_id = GUC_DOORBELL_INVALID;
-}
-
-/*
- * Tell the GuC to allocate or deallocate a specific doorbell
- */
-
-static int __guc_allocate_doorbell(struct intel_guc *guc, u32 stage_id)
-{
- u32 action[] = {
- INTEL_GUC_ACTION_ALLOCATE_DOORBELL,
- stage_id
- };
-
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
-}
-
-static int __guc_deallocate_doorbell(struct intel_guc *guc, u32 stage_id)
-{
- u32 action[] = {
- INTEL_GUC_ACTION_DEALLOCATE_DOORBELL,
- stage_id
- };
-
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
-}
-
static struct guc_stage_desc *__get_stage_desc(struct intel_guc_client *client)
{
struct guc_stage_desc *base = client->guc->stage_desc_pool_vaddr;
@@ -160,118 +84,10 @@ static struct guc_stage_desc *__get_stage_desc(struct intel_guc_client *client)
return &base[client->stage_id];
}
-/*
- * Initialise, update, or clear doorbell data shared with the GuC
- *
- * These functions modify shared data and so need access to the mapped
- * client object which contains the page being used for the doorbell
- */
-
-static void __update_doorbell_desc(struct intel_guc_client *client, u16 new_id)
-{
- struct guc_stage_desc *desc;
-
- /* Update the GuC's idea of the doorbell ID */
- desc = __get_stage_desc(client);
- desc->db_id = new_id;
-}
-
-static struct guc_doorbell_info *__get_doorbell(struct intel_guc_client *client)
-{
- return client->vaddr + client->doorbell_offset;
-}
-
-static bool __doorbell_valid(struct intel_guc *guc, u16 db_id)
-{
- struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
-
- GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
- return intel_uncore_read(uncore, GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID;
-}
-
-static void __init_doorbell(struct intel_guc_client *client)
-{
- struct guc_doorbell_info *doorbell;
-
- doorbell = __get_doorbell(client);
- doorbell->db_status = GUC_DOORBELL_ENABLED;
- doorbell->cookie = 0;
-}
-
-static void __fini_doorbell(struct intel_guc_client *client)
-{
- struct guc_doorbell_info *doorbell;
- u16 db_id = client->doorbell_id;
-
- doorbell = __get_doorbell(client);
- doorbell->db_status = GUC_DOORBELL_DISABLED;
-
- /* 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(!__doorbell_valid(client->guc, db_id), 10))
- WARN_ONCE(true, "Doorbell never became invalid after disable\n");
-}
-
-static int create_doorbell(struct intel_guc_client *client)
-{
- int ret;
-
- if (WARN_ON(!has_doorbell(client)))
- return -ENODEV; /* internal setup error, should never happen */
-
- __update_doorbell_desc(client, client->doorbell_id);
- __init_doorbell(client);
-
- ret = __guc_allocate_doorbell(client->guc, client->stage_id);
- if (ret) {
- __fini_doorbell(client);
- __update_doorbell_desc(client, GUC_DOORBELL_INVALID);
- DRM_DEBUG_DRIVER("Couldn't create client %u doorbell: %d\n",
- client->stage_id, ret);
- return ret;
- }
-
- return 0;
-}
-
-static int destroy_doorbell(struct intel_guc_client *client)
-{
- int ret;
-
- GEM_BUG_ON(!has_doorbell(client));
-
- __fini_doorbell(client);
- ret = __guc_deallocate_doorbell(client->guc, client->stage_id);
- if (ret)
- DRM_ERROR("Couldn't destroy client %u doorbell: %d\n",
- client->stage_id, ret);
-
- __update_doorbell_desc(client, GUC_DOORBELL_INVALID);
-
- return ret;
-}
-
-static unsigned long __select_cacheline(struct intel_guc *guc)
-{
- unsigned long offset;
-
- /* Doorbell uses a single cache line within a page */
- offset = offset_in_page(guc->db_cacheline);
-
- /* Moving to next cache line to reduce contention */
- guc->db_cacheline += cache_line_size();
-
- DRM_DEBUG_DRIVER("reserved cacheline 0x%lx, next 0x%x, linesize %u\n",
- offset, guc->db_cacheline, cache_line_size());
- return offset;
-}
-
static inline struct guc_process_desc *
__get_process_desc(struct intel_guc_client *client)
{
- return client->vaddr + client->proc_desc_offset;
+ return client->vaddr;
}
/*
@@ -332,8 +148,8 @@ static void guc_stage_desc_pool_destroy(struct intel_guc *guc)
* Initialise/clear the stage descriptor shared with the GuC firmware.
*
* This descriptor tells the GuC where (in GGTT space) to find the important
- * data structures relating to this client (doorbell, process descriptor,
- * write queue, etc).
+ * data structures relating to this client (process descriptor, write queue,
+ * etc).
*/
static void guc_stage_desc_init(struct intel_guc_client *client)
{
@@ -350,19 +166,14 @@ static void guc_stage_desc_init(struct intel_guc_client *client)
desc->attribute |= GUC_STAGE_DESC_ATTR_PREEMPT;
desc->stage_id = client->stage_id;
desc->priority = client->priority;
- desc->db_id = client->doorbell_id;
/*
- * The doorbell, process descriptor, and workqueue are all parts
- * of the client object, which the GuC will reference via the GGTT
+ * The process descriptor and workqueue are all parts of the client
+ * object, which the GuC will reference via the GGTT
*/
gfx_addr = intel_guc_ggtt_offset(guc, client->vma);
- desc->db_trigger_phy = sg_dma_address(client->vma->pages->sgl) +
- client->doorbell_offset;
- desc->db_trigger_cpu = ptr_to_u64(__get_doorbell(client));
- desc->db_trigger_uk = gfx_addr + client->doorbell_offset;
- desc->process_desc = gfx_addr + client->proc_desc_offset;
- desc->wq_addr = gfx_addr + GUC_DB_SIZE;
+ desc->process_desc = gfx_addr;
+ desc->wq_addr = gfx_addr + GUC_PD_SIZE;
desc->wq_size = GUC_WQ_SIZE;
desc->desc_private = ptr_to_u64(client);
@@ -408,48 +219,23 @@ static void guc_wq_item_append(struct intel_guc_client *client,
GUC_WQ_SIZE) < wqi_size);
GEM_BUG_ON(wq_off & (wqi_size - 1));
- /* WQ starts from the page after doorbell / process_desc */
- wqi = client->vaddr + wq_off + GUC_DB_SIZE;
-
- if (I915_SELFTEST_ONLY(client->use_nop_wqi)) {
- wqi->header = WQ_TYPE_NOOP | (wqi_len << WQ_LEN_SHIFT);
- } else {
- /* Now fill in the 4-word work queue item */
- wqi->header = WQ_TYPE_INORDER |
- (wqi_len << WQ_LEN_SHIFT) |
- (target_engine << WQ_TARGET_SHIFT) |
- WQ_NO_WCFLUSH_WAIT;
- wqi->context_desc = context_desc;
- wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
- GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
- wqi->fence_id = fence_id;
- }
+ /* WQ starts from the page after process_desc */
+ wqi = client->vaddr + wq_off + GUC_PD_SIZE;
+
+ /* Now fill in the 4-word work queue item */
+ wqi->header = WQ_TYPE_INORDER |
+ (wqi_len << WQ_LEN_SHIFT) |
+ (target_engine << WQ_TARGET_SHIFT) |
+ WQ_NO_WCFLUSH_WAIT;
+ wqi->context_desc = context_desc;
+ wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
+ GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
+ wqi->fence_id = fence_id;
/* Make the update visible to GuC */
WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
}
-static void guc_ring_doorbell(struct intel_guc_client *client)
-{
- struct guc_doorbell_info *db;
- u32 cookie;
-
- lockdep_assert_held(&client->wq_lock);
-
- /* pointer of current doorbell cacheline */
- db = __get_doorbell(client);
-
- /*
- * We're not expecting the doorbell cookie to change behind our back,
- * we also need to treat 0 as a reserved value.
- */
- cookie = READ_ONCE(db->cookie);
- WARN_ON_ONCE(xchg(&db->cookie, cookie + 1 ?: cookie + 2) != cookie);
-
- /* XXX: doorbell was lost and need to acquire it again */
- GEM_BUG_ON(db->db_status != GUC_DOORBELL_ENABLED);
-}
-
static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
{
struct intel_guc_client *client = guc->execbuf_client;
@@ -459,7 +245,6 @@ static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
guc_wq_item_append(client, engine->guc_id, ctx_desc,
ring_tail, rq->fence.seqno);
- guc_ring_doorbell(client);
}
/*
@@ -744,36 +529,6 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
* path of guc_submit() above.
*/
-/* Check that a doorbell register is in the expected state */
-static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
-{
- bool valid;
-
- GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
-
- valid = __doorbell_valid(guc, db_id);
-
- if (test_bit(db_id, guc->doorbell_bitmap) == valid)
- return true;
-
- DRM_DEBUG_DRIVER("Doorbell %u has unexpected state: valid=%s\n",
- db_id, yesno(valid));
-
- return false;
-}
-
-static bool guc_verify_doorbells(struct intel_guc *guc)
-{
- bool doorbells_ok = true;
- u16 db_id;
-
- for (db_id = 0; db_id < GUC_NUM_DOORBELLS; ++db_id)
- if (!doorbell_ok(guc, db_id))
- doorbells_ok = false;
-
- return doorbells_ok;
-}
-
/**
* guc_client_alloc() - Allocate an intel_guc_client
* @guc: the intel_guc structure
@@ -798,7 +553,6 @@ guc_client_alloc(struct intel_guc *guc, u32 priority)
client->guc = guc;
client->priority = priority;
- client->doorbell_id = GUC_DOORBELL_INVALID;
spin_lock_init(&client->wq_lock);
ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
@@ -808,8 +562,8 @@ guc_client_alloc(struct intel_guc *guc, u32 priority)
client->stage_id = ret;
- /* The first page is doorbell/proc_desc. Two followed pages are wq. */
- vma = intel_guc_allocate_vma(guc, GUC_DB_SIZE + GUC_WQ_SIZE);
+ /* The first page is proc_desc. Two following pages are wq. */
+ vma = intel_guc_allocate_vma(guc, GUC_PD_SIZE + GUC_WQ_SIZE);
if (IS_ERR(vma)) {
ret = PTR_ERR(vma);
goto err_id;
@@ -825,31 +579,11 @@ guc_client_alloc(struct intel_guc *guc, u32 priority)
}
client->vaddr = vaddr;
- ret = reserve_doorbell(client);
- if (ret)
- goto err_vaddr;
-
- client->doorbell_offset = __select_cacheline(guc);
-
- /*
- * Since the doorbell only requires a single cacheline, we can save
- * space by putting the application process descriptor in the same
- * page. Use the half of the page that doesn't include the doorbell.
- */
- if (client->doorbell_offset >= (GUC_DB_SIZE / 2))
- client->proc_desc_offset = 0;
- else
- client->proc_desc_offset = (GUC_DB_SIZE / 2);
-
DRM_DEBUG_DRIVER("new priority %u client %p: stage_id %u\n",
priority, client, client->stage_id);
- DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%lx\n",
- client->doorbell_id, client->doorbell_offset);
return client;
-err_vaddr:
- i915_gem_object_unpin_map(client->vma->obj);
err_vma:
i915_vma_unpin_and_release(&client->vma, 0);
err_id:
@@ -861,7 +595,6 @@ guc_client_alloc(struct intel_guc *guc, u32 priority)
static void guc_client_free(struct intel_guc_client *client)
{
- unreserve_doorbell(client);
i915_vma_unpin_and_release(&client->vma, I915_VMA_RELEASE_MAP);
ida_simple_remove(&client->guc->stage_ids, client->stage_id);
kfree(client);
@@ -892,44 +625,22 @@ static void guc_clients_destroy(struct intel_guc *guc)
guc_client_free(client);
}
-static int __guc_client_enable(struct intel_guc_client *client)
+static void __guc_client_enable(struct intel_guc_client *client)
{
- int ret;
-
guc_proc_desc_init(client);
guc_stage_desc_init(client);
-
- ret = create_doorbell(client);
- if (ret)
- goto fail;
-
- return 0;
-
-fail:
- guc_stage_desc_fini(client);
- guc_proc_desc_fini(client);
- return ret;
}
static void __guc_client_disable(struct intel_guc_client *client)
{
- /*
- * By the time we're here, GuC may have already been reset. if that is
- * the case, instead of trying (in vain) to communicate with it, let's
- * just cleanup the doorbell HW and our internal state.
- */
- if (intel_guc_is_running(client->guc))
- destroy_doorbell(client);
- else
- __fini_doorbell(client);
-
+ /* Note: By the time we're here, GuC may have already been reset */
guc_stage_desc_fini(client);
guc_proc_desc_fini(client);
}
-static int guc_clients_enable(struct intel_guc *guc)
+static void guc_clients_enable(struct intel_guc *guc)
{
- return __guc_client_enable(guc->execbuf_client);
+ __guc_client_enable(guc->execbuf_client);
}
static void guc_clients_disable(struct intel_guc *guc)
@@ -958,7 +669,6 @@ int intel_guc_submission_init(struct intel_guc *guc)
*/
GEM_BUG_ON(!guc->stage_desc_pool);
- WARN_ON(!guc_verify_doorbells(guc));
ret = guc_clients_create(guc);
if (ret)
goto err_pool;
@@ -973,7 +683,6 @@ int intel_guc_submission_init(struct intel_guc *guc)
void intel_guc_submission_fini(struct intel_guc *guc)
{
guc_clients_destroy(guc);
- WARN_ON(!guc_verify_doorbells(guc));
if (guc->stage_desc_pool)
guc_stage_desc_pool_destroy(guc);
@@ -1043,16 +752,11 @@ static void guc_set_default_submission(struct intel_engine_cs *engine)
GEM_BUG_ON(engine->irq_enable || engine->irq_disable);
}
-int intel_guc_submission_enable(struct intel_guc *guc)
+void intel_guc_submission_enable(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
struct intel_engine_cs *engine;
enum intel_engine_id id;
- int err;
-
- err = i915_inject_probe_error(gt->i915, -ENXIO);
- if (err)
- return err;
/*
* We're using GuC work items for submitting work through GuC. Since
@@ -1069,9 +773,7 @@ int intel_guc_submission_enable(struct intel_guc *guc)
GEM_BUG_ON(!guc->execbuf_client);
- err = guc_clients_enable(guc);
- if (err)
- return err;
+ guc_clients_enable(guc);
/* Take over from manual control of ELSP (execlists) */
guc_interrupts_capture(gt);
@@ -1080,8 +782,6 @@ int intel_guc_submission_enable(struct intel_guc *guc)
engine->set_default_submission = guc_set_default_submission;
engine->set_default_submission(engine);
}
-
- return 0;
}
void intel_guc_submission_disable(struct intel_guc *guc)
@@ -1109,7 +809,3 @@ void intel_guc_submission_init_early(struct intel_guc *guc)
{
guc->submission_supported = __guc_submission_support(guc);
}
-
-#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
-#include "selftest_guc.c"
-#endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index 54d716828352..e2deb4e6f42a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -44,21 +44,14 @@ struct intel_guc_client {
/* bitmap of (host) engine ids */
u32 priority;
u32 stage_id;
- u32 proc_desc_offset;
-
- u16 doorbell_id;
- unsigned long doorbell_offset;
/* Protects GuC client's WQ access */
spinlock_t wq_lock;
-
- /* For testing purposes, use nop WQ items instead of real ones */
- I915_SELFTEST_DECLARE(bool use_nop_wqi);
};
void intel_guc_submission_init_early(struct intel_guc *guc);
int intel_guc_submission_init(struct intel_guc *guc);
-int intel_guc_submission_enable(struct intel_guc *guc);
+void intel_guc_submission_enable(struct intel_guc *guc);
void intel_guc_submission_disable(struct intel_guc *guc);
void intel_guc_submission_fini(struct intel_guc *guc);
int intel_guc_preempt_work_create(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 629b19377a29..c6519066a0f6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -486,11 +486,8 @@ int intel_uc_init_hw(struct intel_uc *uc)
if (ret)
goto err_communication;
- if (intel_uc_supports_guc_submission(uc)) {
- ret = intel_guc_submission_enable(guc);
- if (ret)
- goto err_communication;
- }
+ if (intel_uc_supports_guc_submission(uc))
+ intel_guc_submission_enable(guc);
dev_info(i915->drm.dev, "%s firmware %s version %u.%u %s:%s\n",
intel_uc_fw_type_repr(INTEL_UC_FW_TYPE_GUC), guc->fw.path,
diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
deleted file mode 100644
index d8a80388bd31..000000000000
--- a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
+++ /dev/null
@@ -1,299 +0,0 @@
-// SPDX-License-Identifier: MIT
-/*
- * Copyright © 2017 Intel Corporation
- */
-
-#include "i915_selftest.h"
-#include "gem/i915_gem_pm.h"
-
-/* max doorbell number + negative test for each client type */
-#define ATTEMPTS (GUC_NUM_DOORBELLS + GUC_CLIENT_PRIORITY_NUM)
-
-static struct intel_guc_client *clients[ATTEMPTS];
-
-static bool available_dbs(struct intel_guc *guc, u32 priority)
-{
- unsigned long offset;
- unsigned long end;
- u16 id;
-
- /* first half is used for normal priority, second half for high */
- offset = 0;
- end = GUC_NUM_DOORBELLS / 2;
- if (priority <= GUC_CLIENT_PRIORITY_HIGH) {
- offset = end;
- end += offset;
- }
-
- id = find_next_zero_bit(guc->doorbell_bitmap, end, offset);
- if (id < end)
- return true;
-
- return false;
-}
-
-static int check_all_doorbells(struct intel_guc *guc)
-{
- u16 db_id;
-
- pr_info_once("Max number of doorbells: %d", GUC_NUM_DOORBELLS);
- for (db_id = 0; db_id < GUC_NUM_DOORBELLS; ++db_id) {
- if (!doorbell_ok(guc, db_id)) {
- pr_err("doorbell %d, not ok\n", db_id);
- return -EIO;
- }
- }
-
- return 0;
-}
-
-static int ring_doorbell_nop(struct intel_guc_client *client)
-{
- struct guc_process_desc *desc = __get_process_desc(client);
- int err;
-
- client->use_nop_wqi = true;
-
- spin_lock_irq(&client->wq_lock);
-
- guc_wq_item_append(client, 0, 0, 0, 0);
- guc_ring_doorbell(client);
-
- spin_unlock_irq(&client->wq_lock);
-
- client->use_nop_wqi = false;
-
- /* if there are no issues GuC will update the WQ head and keep the
- * WQ in active status
- */
- err = wait_for(READ_ONCE(desc->head) == READ_ONCE(desc->tail), 10);
- if (err) {
- pr_err("doorbell %u ring failed!\n", client->doorbell_id);
- return -EIO;
- }
-
- if (desc->wq_status != WQ_STATUS_ACTIVE) {
- pr_err("doorbell %u ring put WQ in bad state (%u)!\n",
- client->doorbell_id, desc->wq_status);
- return -EIO;
- }
-
- return 0;
-}
-
-/*
- * Basic client sanity check, handy to validate create_clients.
- */
-static int validate_client(struct intel_guc_client *client, int client_priority)
-{
- if (client->priority != client_priority ||
- client->doorbell_id == GUC_DOORBELL_INVALID)
- return -EINVAL;
- else
- return 0;
-}
-
-static bool client_doorbell_in_sync(struct intel_guc_client *client)
-{
- return !client || doorbell_ok(client->guc, client->doorbell_id);
-}
-
-/*
- * Check that we're able to synchronize guc_clients with their doorbells
- *
- * We're creating clients and reserving doorbells once, at module load. During
- * module lifetime, GuC, doorbell HW, and i915 state may go out of sync due to
- * GuC being reset. In other words - GuC clients are still around, but the
- * status of their doorbells may be incorrect. This is the reason behind
- * validating that the doorbells status expected by the driver matches what the
- * GuC/HW have.
- */
-static int igt_guc_clients(void *arg)
-{
- struct intel_gt *gt = arg;
- struct intel_guc *guc = >->uc.guc;
- intel_wakeref_t wakeref;
- int err = 0;
-
- GEM_BUG_ON(!HAS_GT_UC(gt->i915));
- wakeref = intel_runtime_pm_get(gt->uncore->rpm);
-
- err = check_all_doorbells(guc);
- if (err)
- goto unlock;
-
- /*
- * Get rid of clients created during driver load because the test will
- * recreate them.
- */
- guc_clients_disable(guc);
- guc_clients_destroy(guc);
- if (guc->execbuf_client) {
- pr_err("guc_clients_destroy lied!\n");
- err = -EINVAL;
- goto unlock;
- }
-
- err = guc_clients_create(guc);
- if (err) {
- pr_err("Failed to create clients\n");
- goto unlock;
- }
- GEM_BUG_ON(!guc->execbuf_client);
-
- err = validate_client(guc->execbuf_client,
- GUC_CLIENT_PRIORITY_KMD_NORMAL);
- if (err) {
- pr_err("execbug client validation failed\n");
- goto out;
- }
-
- /* the client should now have reserved a doorbell */
- if (!has_doorbell(guc->execbuf_client)) {
- pr_err("guc_clients_create didn't reserve doorbells\n");
- err = -EINVAL;
- goto out;
- }
-
- /* Now enable the clients */
- guc_clients_enable(guc);
-
- /* each client should now have received a doorbell */
- if (!client_doorbell_in_sync(guc->execbuf_client)) {
- pr_err("failed to initialize the doorbells\n");
- err = -EINVAL;
- goto out;
- }
-
- /*
- * Basic test - an attempt to reallocate a valid doorbell to the
- * client it is currently assigned should not cause a failure.
- */
- err = create_doorbell(guc->execbuf_client);
-
-out:
- /*
- * Leave clean state for other test, plus the driver always destroy the
- * clients during unload.
- */
- guc_clients_disable(guc);
- guc_clients_destroy(guc);
- guc_clients_create(guc);
- guc_clients_enable(guc);
-unlock:
- intel_runtime_pm_put(gt->uncore->rpm, wakeref);
- return err;
-}
-
-/*
- * Create as many clients as number of doorbells. Note that there's already
- * client(s)/doorbell(s) created during driver load, but this test creates
- * its own and do not interact with the existing ones.
- */
-static int igt_guc_doorbells(void *arg)
-{
- struct intel_gt *gt = arg;
- struct intel_guc *guc = >->uc.guc;
- intel_wakeref_t wakeref;
- int i, err = 0;
- u16 db_id;
-
- GEM_BUG_ON(!HAS_GT_UC(gt->i915));
- wakeref = intel_runtime_pm_get(gt->uncore->rpm);
-
- err = check_all_doorbells(guc);
- if (err)
- goto unlock;
-
- for (i = 0; i < ATTEMPTS; i++) {
- clients[i] = guc_client_alloc(guc, i % GUC_CLIENT_PRIORITY_NUM);
-
- if (!clients[i]) {
- pr_err("[%d] No guc client\n", i);
- err = -EINVAL;
- goto out;
- }
-
- if (IS_ERR(clients[i])) {
- if (PTR_ERR(clients[i]) != -ENOSPC) {
- pr_err("[%d] unexpected error\n", i);
- err = PTR_ERR(clients[i]);
- goto out;
- }
-
- if (available_dbs(guc, i % GUC_CLIENT_PRIORITY_NUM)) {
- pr_err("[%d] non-db related alloc fail\n", i);
- err = -EINVAL;
- goto out;
- }
-
- /* expected, ran out of dbs for this client type */
- continue;
- }
-
- /*
- * The check below is only valid because we keep a doorbell
- * assigned during the whole life of the client.
- */
- if (clients[i]->stage_id >= GUC_NUM_DOORBELLS) {
- pr_err("[%d] more clients than doorbells (%d >= %d)\n",
- i, clients[i]->stage_id, GUC_NUM_DOORBELLS);
- err = -EINVAL;
- goto out;
- }
-
- err = validate_client(clients[i], i % GUC_CLIENT_PRIORITY_NUM);
- if (err) {
- pr_err("[%d] client_alloc sanity check failed!\n", i);
- err = -EINVAL;
- goto out;
- }
-
- db_id = clients[i]->doorbell_id;
-
- err = __guc_client_enable(clients[i]);
- if (err) {
- pr_err("[%d] Failed to create a doorbell\n", i);
- goto out;
- }
-
- /* doorbell id shouldn't change, we are holding the mutex */
- if (db_id != clients[i]->doorbell_id) {
- pr_err("[%d] doorbell id changed (%d != %d)\n",
- i, db_id, clients[i]->doorbell_id);
- err = -EINVAL;
- goto out;
- }
-
- err = check_all_doorbells(guc);
- if (err)
- goto out;
-
- err = ring_doorbell_nop(clients[i]);
- if (err)
- goto out;
- }
-
-out:
- for (i = 0; i < ATTEMPTS; i++)
- if (!IS_ERR_OR_NULL(clients[i])) {
- __guc_client_disable(clients[i]);
- guc_client_free(clients[i]);
- }
-unlock:
- intel_runtime_pm_put(gt->uncore->rpm, wakeref);
- return err;
-}
-
-int intel_guc_live_selftest(struct drm_i915_private *i915)
-{
- static const struct i915_subtest tests[] = {
- SUBTEST(igt_guc_clients),
- SUBTEST(igt_guc_doorbells),
- };
-
- if (!USES_GUC_SUBMISSION(i915))
- return 0;
-
- return intel_gt_live_subtests(tests, &i915->gt);
-}
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index cab632791f73..5d5974e7f3ed 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1815,17 +1815,10 @@ static int i915_guc_info(struct seq_file *m, void *data)
GEM_BUG_ON(!guc->execbuf_client);
- seq_printf(m, "\nDoorbell map:\n");
- seq_printf(m, "\t%*pb\n", GUC_NUM_DOORBELLS, guc->doorbell_bitmap);
- seq_printf(m, "Doorbell next cacheline: 0x%x\n", guc->db_cacheline);
-
seq_printf(m, "\nGuC execbuf client @ %p:\n", client);
- seq_printf(m, "\tPriority %d, GuC stage index: %u, PD offset 0x%x\n",
+ seq_printf(m, "\tPriority %d, GuC stage index: %u\n",
client->priority,
- client->stage_id,
- client->proc_desc_offset);
- seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
- client->doorbell_id, client->doorbell_offset);
+ client->stage_id);
/* Add more as required ... */
return 0;
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index 11b40bc58e6d..e0adabc614e9 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -37,5 +37,4 @@ selftest(reset, intel_reset_live_selftests)
selftest(memory_region, intel_memory_region_live_selftests)
selftest(hangcheck, intel_hangcheck_live_selftests)
selftest(execlists, intel_execlists_live_selftests)
-selftest(guc, intel_guc_live_selftest)
selftest(perf, i915_perf_live_selftests)
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Intel-gfx] [CI 3/5] drm/i915/guc: kill doorbell code and selftests
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
Instead of relying on the workqueue, the upcoming reworked GuC
submission flow will offer the host driver indipendent control over
the execution status of each context submitted to GuC. As part of this,
the doorbell usage model has been reworked, with each doorbell being
paired to a single lrc and a doorbell ring representing new work
available for that specific context. This mechanism, however, limits
the number of contexts that can be registered with GuC to the number of
doorbells, which is an undesired limitation. To avoid this limitation,
we requested the GuC team to also provide a H2G that will allow the host
to notify the GuC of work available for a specified lrc, so we can use
that mechanism instead of relying on the doorbells. We can therefore drop
the doorbell code we currently have, also given the fact that in the
unlikely case we'd want to switch back to using doorbells we'd have to
heavily rework it.
The workqueue will still have a use in the new interface to pass special
commands, so that code has been retained for now.
With the doorbells gone and the GuC client becoming even simpler, the
existing GuC selftests don't give us any meaningful coverage so we can
remove them as well. Some selftests might come with the new code, but
they will look different from what we have now so if doesn't seem worth
it to keep the file around in the meantime.
v2: fix comments and commit message (John)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 8 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 2 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 376 ++----------------
.../gpu/drm/i915/gt/uc/intel_guc_submission.h | 9 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 7 +-
drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 299 --------------
drivers/gpu/drm/i915/i915_debugfs.c | 11 +-
.../drm/i915/selftests/i915_live_selftests.h | 1 -
8 files changed, 44 insertions(+), 669 deletions(-)
delete mode 100644 drivers/gpu/drm/i915/gt/uc/selftest_guc.c
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index bf438f820c35..b2d1766e689a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -20,8 +20,8 @@ struct __guc_ads_blob;
/*
* Top level structure of GuC. It handles firmware loading and manages client
- * pool and doorbells. intel_guc owns a intel_guc_client to replace the legacy
- * ExecList submission.
+ * pool. intel_guc owns a intel_guc_client to replace the legacy ExecList
+ * submission.
*/
struct intel_guc {
struct intel_uc_fw fw;
@@ -50,10 +50,6 @@ struct intel_guc {
struct intel_guc_client *execbuf_client;
- DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
- /* Cyclic counter mod pagesize */
- u32 db_cacheline;
-
/* Control params for fw initialization */
u32 params[GUC_CTL_MAX_DWORDS];
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
index a26a85d50209..1e8e4af7d9ca 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
@@ -31,7 +31,7 @@
#define GUC_DOORBELL_INVALID 256
-#define GUC_DB_SIZE (PAGE_SIZE)
+#define GUC_PD_SIZE (PAGE_SIZE)
#define GUC_WQ_SIZE (PAGE_SIZE * 2)
/* Work queue item header definitions */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 445e0e3c4f11..0b89da85885c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -30,8 +30,8 @@
* GuC client:
* A intel_guc_client refers to a submission path through GuC. Currently, there
* is only one client, which is charged with all submissions to the GuC. This
- * struct is the owner of a doorbell, a process descriptor and a workqueue (all
- * of them inside a single gem object that contains all required pages for these
+ * struct is the owner of a process descriptor and a workqueue (both of them
+ * inside a single gem object that contains all required pages for these
* elements).
*
* GuC stage descriptor:
@@ -39,13 +39,13 @@
* descriptors, and shares them with the GuC.
* Currently, there exists a 1:1 mapping between a intel_guc_client and a
* guc_stage_desc (via the client's stage_id), so effectively only one
- * gets used. This stage descriptor lets the GuC know about the doorbell,
- * workqueue and process descriptor. Theoretically, it also lets the GuC
- * know about our HW contexts (context ID, etc...), but we actually
- * employ a kind of submission where the GuC uses the LRCA sent via the work
- * item instead (the single guc_stage_desc associated to execbuf client
- * contains information about the default kernel context only, but this is
- * essentially unused). This is called a "proxy" submission.
+ * gets used. This stage descriptor lets the GuC know about the workqueue and
+ * process descriptor. Theoretically, it also lets the GuC know about our HW
+ * contexts (context ID, etc...), but we actually employ a kind of submission
+ * where the GuC uses the LRCA sent via the work item instead (the single
+ * guc_stage_desc associated to execbuf client contains information about the
+ * default kernel context only, but this is essentially unused). This is called
+ * a "proxy" submission.
*
* The Scratch registers:
* There are 16 MMIO-based registers start from 0xC180. The kernel driver writes
@@ -56,10 +56,6 @@
* then proceeds.
* See intel_guc_send()
*
- * Doorbells:
- * Doorbells are interrupts to uKernel. A doorbell is a single cache line (QW)
- * mapped into process space.
- *
* Work Items:
* There are several types of work items that the host may place into a
* workqueue, each with its own requirements and limitations. Currently only
@@ -81,78 +77,6 @@ static inline bool is_high_priority(struct intel_guc_client *client)
client->priority == GUC_CLIENT_PRIORITY_HIGH);
}
-static int reserve_doorbell(struct intel_guc_client *client)
-{
- unsigned long offset;
- unsigned long end;
- u16 id;
-
- GEM_BUG_ON(client->doorbell_id != GUC_DOORBELL_INVALID);
-
- /*
- * The bitmap tracks which doorbell registers are currently in use.
- * It is split into two halves; the first half is used for normal
- * priority contexts, the second half for high-priority ones.
- */
- offset = 0;
- end = GUC_NUM_DOORBELLS / 2;
- if (is_high_priority(client)) {
- offset = end;
- end += offset;
- }
-
- id = find_next_zero_bit(client->guc->doorbell_bitmap, end, offset);
- if (id == end)
- return -ENOSPC;
-
- __set_bit(id, client->guc->doorbell_bitmap);
- client->doorbell_id = id;
- DRM_DEBUG_DRIVER("client %u (high prio=%s) reserved doorbell: %d\n",
- client->stage_id, yesno(is_high_priority(client)),
- id);
- return 0;
-}
-
-static bool has_doorbell(struct intel_guc_client *client)
-{
- if (client->doorbell_id == GUC_DOORBELL_INVALID)
- return false;
-
- return test_bit(client->doorbell_id, client->guc->doorbell_bitmap);
-}
-
-static void unreserve_doorbell(struct intel_guc_client *client)
-{
- GEM_BUG_ON(!has_doorbell(client));
-
- __clear_bit(client->doorbell_id, client->guc->doorbell_bitmap);
- client->doorbell_id = GUC_DOORBELL_INVALID;
-}
-
-/*
- * Tell the GuC to allocate or deallocate a specific doorbell
- */
-
-static int __guc_allocate_doorbell(struct intel_guc *guc, u32 stage_id)
-{
- u32 action[] = {
- INTEL_GUC_ACTION_ALLOCATE_DOORBELL,
- stage_id
- };
-
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
-}
-
-static int __guc_deallocate_doorbell(struct intel_guc *guc, u32 stage_id)
-{
- u32 action[] = {
- INTEL_GUC_ACTION_DEALLOCATE_DOORBELL,
- stage_id
- };
-
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
-}
-
static struct guc_stage_desc *__get_stage_desc(struct intel_guc_client *client)
{
struct guc_stage_desc *base = client->guc->stage_desc_pool_vaddr;
@@ -160,118 +84,10 @@ static struct guc_stage_desc *__get_stage_desc(struct intel_guc_client *client)
return &base[client->stage_id];
}
-/*
- * Initialise, update, or clear doorbell data shared with the GuC
- *
- * These functions modify shared data and so need access to the mapped
- * client object which contains the page being used for the doorbell
- */
-
-static void __update_doorbell_desc(struct intel_guc_client *client, u16 new_id)
-{
- struct guc_stage_desc *desc;
-
- /* Update the GuC's idea of the doorbell ID */
- desc = __get_stage_desc(client);
- desc->db_id = new_id;
-}
-
-static struct guc_doorbell_info *__get_doorbell(struct intel_guc_client *client)
-{
- return client->vaddr + client->doorbell_offset;
-}
-
-static bool __doorbell_valid(struct intel_guc *guc, u16 db_id)
-{
- struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
-
- GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
- return intel_uncore_read(uncore, GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID;
-}
-
-static void __init_doorbell(struct intel_guc_client *client)
-{
- struct guc_doorbell_info *doorbell;
-
- doorbell = __get_doorbell(client);
- doorbell->db_status = GUC_DOORBELL_ENABLED;
- doorbell->cookie = 0;
-}
-
-static void __fini_doorbell(struct intel_guc_client *client)
-{
- struct guc_doorbell_info *doorbell;
- u16 db_id = client->doorbell_id;
-
- doorbell = __get_doorbell(client);
- doorbell->db_status = GUC_DOORBELL_DISABLED;
-
- /* 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(!__doorbell_valid(client->guc, db_id), 10))
- WARN_ONCE(true, "Doorbell never became invalid after disable\n");
-}
-
-static int create_doorbell(struct intel_guc_client *client)
-{
- int ret;
-
- if (WARN_ON(!has_doorbell(client)))
- return -ENODEV; /* internal setup error, should never happen */
-
- __update_doorbell_desc(client, client->doorbell_id);
- __init_doorbell(client);
-
- ret = __guc_allocate_doorbell(client->guc, client->stage_id);
- if (ret) {
- __fini_doorbell(client);
- __update_doorbell_desc(client, GUC_DOORBELL_INVALID);
- DRM_DEBUG_DRIVER("Couldn't create client %u doorbell: %d\n",
- client->stage_id, ret);
- return ret;
- }
-
- return 0;
-}
-
-static int destroy_doorbell(struct intel_guc_client *client)
-{
- int ret;
-
- GEM_BUG_ON(!has_doorbell(client));
-
- __fini_doorbell(client);
- ret = __guc_deallocate_doorbell(client->guc, client->stage_id);
- if (ret)
- DRM_ERROR("Couldn't destroy client %u doorbell: %d\n",
- client->stage_id, ret);
-
- __update_doorbell_desc(client, GUC_DOORBELL_INVALID);
-
- return ret;
-}
-
-static unsigned long __select_cacheline(struct intel_guc *guc)
-{
- unsigned long offset;
-
- /* Doorbell uses a single cache line within a page */
- offset = offset_in_page(guc->db_cacheline);
-
- /* Moving to next cache line to reduce contention */
- guc->db_cacheline += cache_line_size();
-
- DRM_DEBUG_DRIVER("reserved cacheline 0x%lx, next 0x%x, linesize %u\n",
- offset, guc->db_cacheline, cache_line_size());
- return offset;
-}
-
static inline struct guc_process_desc *
__get_process_desc(struct intel_guc_client *client)
{
- return client->vaddr + client->proc_desc_offset;
+ return client->vaddr;
}
/*
@@ -332,8 +148,8 @@ static void guc_stage_desc_pool_destroy(struct intel_guc *guc)
* Initialise/clear the stage descriptor shared with the GuC firmware.
*
* This descriptor tells the GuC where (in GGTT space) to find the important
- * data structures relating to this client (doorbell, process descriptor,
- * write queue, etc).
+ * data structures relating to this client (process descriptor, write queue,
+ * etc).
*/
static void guc_stage_desc_init(struct intel_guc_client *client)
{
@@ -350,19 +166,14 @@ static void guc_stage_desc_init(struct intel_guc_client *client)
desc->attribute |= GUC_STAGE_DESC_ATTR_PREEMPT;
desc->stage_id = client->stage_id;
desc->priority = client->priority;
- desc->db_id = client->doorbell_id;
/*
- * The doorbell, process descriptor, and workqueue are all parts
- * of the client object, which the GuC will reference via the GGTT
+ * The process descriptor and workqueue are all parts of the client
+ * object, which the GuC will reference via the GGTT
*/
gfx_addr = intel_guc_ggtt_offset(guc, client->vma);
- desc->db_trigger_phy = sg_dma_address(client->vma->pages->sgl) +
- client->doorbell_offset;
- desc->db_trigger_cpu = ptr_to_u64(__get_doorbell(client));
- desc->db_trigger_uk = gfx_addr + client->doorbell_offset;
- desc->process_desc = gfx_addr + client->proc_desc_offset;
- desc->wq_addr = gfx_addr + GUC_DB_SIZE;
+ desc->process_desc = gfx_addr;
+ desc->wq_addr = gfx_addr + GUC_PD_SIZE;
desc->wq_size = GUC_WQ_SIZE;
desc->desc_private = ptr_to_u64(client);
@@ -408,48 +219,23 @@ static void guc_wq_item_append(struct intel_guc_client *client,
GUC_WQ_SIZE) < wqi_size);
GEM_BUG_ON(wq_off & (wqi_size - 1));
- /* WQ starts from the page after doorbell / process_desc */
- wqi = client->vaddr + wq_off + GUC_DB_SIZE;
-
- if (I915_SELFTEST_ONLY(client->use_nop_wqi)) {
- wqi->header = WQ_TYPE_NOOP | (wqi_len << WQ_LEN_SHIFT);
- } else {
- /* Now fill in the 4-word work queue item */
- wqi->header = WQ_TYPE_INORDER |
- (wqi_len << WQ_LEN_SHIFT) |
- (target_engine << WQ_TARGET_SHIFT) |
- WQ_NO_WCFLUSH_WAIT;
- wqi->context_desc = context_desc;
- wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
- GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
- wqi->fence_id = fence_id;
- }
+ /* WQ starts from the page after process_desc */
+ wqi = client->vaddr + wq_off + GUC_PD_SIZE;
+
+ /* Now fill in the 4-word work queue item */
+ wqi->header = WQ_TYPE_INORDER |
+ (wqi_len << WQ_LEN_SHIFT) |
+ (target_engine << WQ_TARGET_SHIFT) |
+ WQ_NO_WCFLUSH_WAIT;
+ wqi->context_desc = context_desc;
+ wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
+ GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
+ wqi->fence_id = fence_id;
/* Make the update visible to GuC */
WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
}
-static void guc_ring_doorbell(struct intel_guc_client *client)
-{
- struct guc_doorbell_info *db;
- u32 cookie;
-
- lockdep_assert_held(&client->wq_lock);
-
- /* pointer of current doorbell cacheline */
- db = __get_doorbell(client);
-
- /*
- * We're not expecting the doorbell cookie to change behind our back,
- * we also need to treat 0 as a reserved value.
- */
- cookie = READ_ONCE(db->cookie);
- WARN_ON_ONCE(xchg(&db->cookie, cookie + 1 ?: cookie + 2) != cookie);
-
- /* XXX: doorbell was lost and need to acquire it again */
- GEM_BUG_ON(db->db_status != GUC_DOORBELL_ENABLED);
-}
-
static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
{
struct intel_guc_client *client = guc->execbuf_client;
@@ -459,7 +245,6 @@ static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
guc_wq_item_append(client, engine->guc_id, ctx_desc,
ring_tail, rq->fence.seqno);
- guc_ring_doorbell(client);
}
/*
@@ -744,36 +529,6 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
* path of guc_submit() above.
*/
-/* Check that a doorbell register is in the expected state */
-static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
-{
- bool valid;
-
- GEM_BUG_ON(db_id >= GUC_NUM_DOORBELLS);
-
- valid = __doorbell_valid(guc, db_id);
-
- if (test_bit(db_id, guc->doorbell_bitmap) == valid)
- return true;
-
- DRM_DEBUG_DRIVER("Doorbell %u has unexpected state: valid=%s\n",
- db_id, yesno(valid));
-
- return false;
-}
-
-static bool guc_verify_doorbells(struct intel_guc *guc)
-{
- bool doorbells_ok = true;
- u16 db_id;
-
- for (db_id = 0; db_id < GUC_NUM_DOORBELLS; ++db_id)
- if (!doorbell_ok(guc, db_id))
- doorbells_ok = false;
-
- return doorbells_ok;
-}
-
/**
* guc_client_alloc() - Allocate an intel_guc_client
* @guc: the intel_guc structure
@@ -798,7 +553,6 @@ guc_client_alloc(struct intel_guc *guc, u32 priority)
client->guc = guc;
client->priority = priority;
- client->doorbell_id = GUC_DOORBELL_INVALID;
spin_lock_init(&client->wq_lock);
ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
@@ -808,8 +562,8 @@ guc_client_alloc(struct intel_guc *guc, u32 priority)
client->stage_id = ret;
- /* The first page is doorbell/proc_desc. Two followed pages are wq. */
- vma = intel_guc_allocate_vma(guc, GUC_DB_SIZE + GUC_WQ_SIZE);
+ /* The first page is proc_desc. Two following pages are wq. */
+ vma = intel_guc_allocate_vma(guc, GUC_PD_SIZE + GUC_WQ_SIZE);
if (IS_ERR(vma)) {
ret = PTR_ERR(vma);
goto err_id;
@@ -825,31 +579,11 @@ guc_client_alloc(struct intel_guc *guc, u32 priority)
}
client->vaddr = vaddr;
- ret = reserve_doorbell(client);
- if (ret)
- goto err_vaddr;
-
- client->doorbell_offset = __select_cacheline(guc);
-
- /*
- * Since the doorbell only requires a single cacheline, we can save
- * space by putting the application process descriptor in the same
- * page. Use the half of the page that doesn't include the doorbell.
- */
- if (client->doorbell_offset >= (GUC_DB_SIZE / 2))
- client->proc_desc_offset = 0;
- else
- client->proc_desc_offset = (GUC_DB_SIZE / 2);
-
DRM_DEBUG_DRIVER("new priority %u client %p: stage_id %u\n",
priority, client, client->stage_id);
- DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%lx\n",
- client->doorbell_id, client->doorbell_offset);
return client;
-err_vaddr:
- i915_gem_object_unpin_map(client->vma->obj);
err_vma:
i915_vma_unpin_and_release(&client->vma, 0);
err_id:
@@ -861,7 +595,6 @@ guc_client_alloc(struct intel_guc *guc, u32 priority)
static void guc_client_free(struct intel_guc_client *client)
{
- unreserve_doorbell(client);
i915_vma_unpin_and_release(&client->vma, I915_VMA_RELEASE_MAP);
ida_simple_remove(&client->guc->stage_ids, client->stage_id);
kfree(client);
@@ -892,44 +625,22 @@ static void guc_clients_destroy(struct intel_guc *guc)
guc_client_free(client);
}
-static int __guc_client_enable(struct intel_guc_client *client)
+static void __guc_client_enable(struct intel_guc_client *client)
{
- int ret;
-
guc_proc_desc_init(client);
guc_stage_desc_init(client);
-
- ret = create_doorbell(client);
- if (ret)
- goto fail;
-
- return 0;
-
-fail:
- guc_stage_desc_fini(client);
- guc_proc_desc_fini(client);
- return ret;
}
static void __guc_client_disable(struct intel_guc_client *client)
{
- /*
- * By the time we're here, GuC may have already been reset. if that is
- * the case, instead of trying (in vain) to communicate with it, let's
- * just cleanup the doorbell HW and our internal state.
- */
- if (intel_guc_is_running(client->guc))
- destroy_doorbell(client);
- else
- __fini_doorbell(client);
-
+ /* Note: By the time we're here, GuC may have already been reset */
guc_stage_desc_fini(client);
guc_proc_desc_fini(client);
}
-static int guc_clients_enable(struct intel_guc *guc)
+static void guc_clients_enable(struct intel_guc *guc)
{
- return __guc_client_enable(guc->execbuf_client);
+ __guc_client_enable(guc->execbuf_client);
}
static void guc_clients_disable(struct intel_guc *guc)
@@ -958,7 +669,6 @@ int intel_guc_submission_init(struct intel_guc *guc)
*/
GEM_BUG_ON(!guc->stage_desc_pool);
- WARN_ON(!guc_verify_doorbells(guc));
ret = guc_clients_create(guc);
if (ret)
goto err_pool;
@@ -973,7 +683,6 @@ int intel_guc_submission_init(struct intel_guc *guc)
void intel_guc_submission_fini(struct intel_guc *guc)
{
guc_clients_destroy(guc);
- WARN_ON(!guc_verify_doorbells(guc));
if (guc->stage_desc_pool)
guc_stage_desc_pool_destroy(guc);
@@ -1043,16 +752,11 @@ static void guc_set_default_submission(struct intel_engine_cs *engine)
GEM_BUG_ON(engine->irq_enable || engine->irq_disable);
}
-int intel_guc_submission_enable(struct intel_guc *guc)
+void intel_guc_submission_enable(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
struct intel_engine_cs *engine;
enum intel_engine_id id;
- int err;
-
- err = i915_inject_probe_error(gt->i915, -ENXIO);
- if (err)
- return err;
/*
* We're using GuC work items for submitting work through GuC. Since
@@ -1069,9 +773,7 @@ int intel_guc_submission_enable(struct intel_guc *guc)
GEM_BUG_ON(!guc->execbuf_client);
- err = guc_clients_enable(guc);
- if (err)
- return err;
+ guc_clients_enable(guc);
/* Take over from manual control of ELSP (execlists) */
guc_interrupts_capture(gt);
@@ -1080,8 +782,6 @@ int intel_guc_submission_enable(struct intel_guc *guc)
engine->set_default_submission = guc_set_default_submission;
engine->set_default_submission(engine);
}
-
- return 0;
}
void intel_guc_submission_disable(struct intel_guc *guc)
@@ -1109,7 +809,3 @@ void intel_guc_submission_init_early(struct intel_guc *guc)
{
guc->submission_supported = __guc_submission_support(guc);
}
-
-#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
-#include "selftest_guc.c"
-#endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index 54d716828352..e2deb4e6f42a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -44,21 +44,14 @@ struct intel_guc_client {
/* bitmap of (host) engine ids */
u32 priority;
u32 stage_id;
- u32 proc_desc_offset;
-
- u16 doorbell_id;
- unsigned long doorbell_offset;
/* Protects GuC client's WQ access */
spinlock_t wq_lock;
-
- /* For testing purposes, use nop WQ items instead of real ones */
- I915_SELFTEST_DECLARE(bool use_nop_wqi);
};
void intel_guc_submission_init_early(struct intel_guc *guc);
int intel_guc_submission_init(struct intel_guc *guc);
-int intel_guc_submission_enable(struct intel_guc *guc);
+void intel_guc_submission_enable(struct intel_guc *guc);
void intel_guc_submission_disable(struct intel_guc *guc);
void intel_guc_submission_fini(struct intel_guc *guc);
int intel_guc_preempt_work_create(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 629b19377a29..c6519066a0f6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -486,11 +486,8 @@ int intel_uc_init_hw(struct intel_uc *uc)
if (ret)
goto err_communication;
- if (intel_uc_supports_guc_submission(uc)) {
- ret = intel_guc_submission_enable(guc);
- if (ret)
- goto err_communication;
- }
+ if (intel_uc_supports_guc_submission(uc))
+ intel_guc_submission_enable(guc);
dev_info(i915->drm.dev, "%s firmware %s version %u.%u %s:%s\n",
intel_uc_fw_type_repr(INTEL_UC_FW_TYPE_GUC), guc->fw.path,
diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
deleted file mode 100644
index d8a80388bd31..000000000000
--- a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
+++ /dev/null
@@ -1,299 +0,0 @@
-// SPDX-License-Identifier: MIT
-/*
- * Copyright © 2017 Intel Corporation
- */
-
-#include "i915_selftest.h"
-#include "gem/i915_gem_pm.h"
-
-/* max doorbell number + negative test for each client type */
-#define ATTEMPTS (GUC_NUM_DOORBELLS + GUC_CLIENT_PRIORITY_NUM)
-
-static struct intel_guc_client *clients[ATTEMPTS];
-
-static bool available_dbs(struct intel_guc *guc, u32 priority)
-{
- unsigned long offset;
- unsigned long end;
- u16 id;
-
- /* first half is used for normal priority, second half for high */
- offset = 0;
- end = GUC_NUM_DOORBELLS / 2;
- if (priority <= GUC_CLIENT_PRIORITY_HIGH) {
- offset = end;
- end += offset;
- }
-
- id = find_next_zero_bit(guc->doorbell_bitmap, end, offset);
- if (id < end)
- return true;
-
- return false;
-}
-
-static int check_all_doorbells(struct intel_guc *guc)
-{
- u16 db_id;
-
- pr_info_once("Max number of doorbells: %d", GUC_NUM_DOORBELLS);
- for (db_id = 0; db_id < GUC_NUM_DOORBELLS; ++db_id) {
- if (!doorbell_ok(guc, db_id)) {
- pr_err("doorbell %d, not ok\n", db_id);
- return -EIO;
- }
- }
-
- return 0;
-}
-
-static int ring_doorbell_nop(struct intel_guc_client *client)
-{
- struct guc_process_desc *desc = __get_process_desc(client);
- int err;
-
- client->use_nop_wqi = true;
-
- spin_lock_irq(&client->wq_lock);
-
- guc_wq_item_append(client, 0, 0, 0, 0);
- guc_ring_doorbell(client);
-
- spin_unlock_irq(&client->wq_lock);
-
- client->use_nop_wqi = false;
-
- /* if there are no issues GuC will update the WQ head and keep the
- * WQ in active status
- */
- err = wait_for(READ_ONCE(desc->head) == READ_ONCE(desc->tail), 10);
- if (err) {
- pr_err("doorbell %u ring failed!\n", client->doorbell_id);
- return -EIO;
- }
-
- if (desc->wq_status != WQ_STATUS_ACTIVE) {
- pr_err("doorbell %u ring put WQ in bad state (%u)!\n",
- client->doorbell_id, desc->wq_status);
- return -EIO;
- }
-
- return 0;
-}
-
-/*
- * Basic client sanity check, handy to validate create_clients.
- */
-static int validate_client(struct intel_guc_client *client, int client_priority)
-{
- if (client->priority != client_priority ||
- client->doorbell_id == GUC_DOORBELL_INVALID)
- return -EINVAL;
- else
- return 0;
-}
-
-static bool client_doorbell_in_sync(struct intel_guc_client *client)
-{
- return !client || doorbell_ok(client->guc, client->doorbell_id);
-}
-
-/*
- * Check that we're able to synchronize guc_clients with their doorbells
- *
- * We're creating clients and reserving doorbells once, at module load. During
- * module lifetime, GuC, doorbell HW, and i915 state may go out of sync due to
- * GuC being reset. In other words - GuC clients are still around, but the
- * status of their doorbells may be incorrect. This is the reason behind
- * validating that the doorbells status expected by the driver matches what the
- * GuC/HW have.
- */
-static int igt_guc_clients(void *arg)
-{
- struct intel_gt *gt = arg;
- struct intel_guc *guc = >->uc.guc;
- intel_wakeref_t wakeref;
- int err = 0;
-
- GEM_BUG_ON(!HAS_GT_UC(gt->i915));
- wakeref = intel_runtime_pm_get(gt->uncore->rpm);
-
- err = check_all_doorbells(guc);
- if (err)
- goto unlock;
-
- /*
- * Get rid of clients created during driver load because the test will
- * recreate them.
- */
- guc_clients_disable(guc);
- guc_clients_destroy(guc);
- if (guc->execbuf_client) {
- pr_err("guc_clients_destroy lied!\n");
- err = -EINVAL;
- goto unlock;
- }
-
- err = guc_clients_create(guc);
- if (err) {
- pr_err("Failed to create clients\n");
- goto unlock;
- }
- GEM_BUG_ON(!guc->execbuf_client);
-
- err = validate_client(guc->execbuf_client,
- GUC_CLIENT_PRIORITY_KMD_NORMAL);
- if (err) {
- pr_err("execbug client validation failed\n");
- goto out;
- }
-
- /* the client should now have reserved a doorbell */
- if (!has_doorbell(guc->execbuf_client)) {
- pr_err("guc_clients_create didn't reserve doorbells\n");
- err = -EINVAL;
- goto out;
- }
-
- /* Now enable the clients */
- guc_clients_enable(guc);
-
- /* each client should now have received a doorbell */
- if (!client_doorbell_in_sync(guc->execbuf_client)) {
- pr_err("failed to initialize the doorbells\n");
- err = -EINVAL;
- goto out;
- }
-
- /*
- * Basic test - an attempt to reallocate a valid doorbell to the
- * client it is currently assigned should not cause a failure.
- */
- err = create_doorbell(guc->execbuf_client);
-
-out:
- /*
- * Leave clean state for other test, plus the driver always destroy the
- * clients during unload.
- */
- guc_clients_disable(guc);
- guc_clients_destroy(guc);
- guc_clients_create(guc);
- guc_clients_enable(guc);
-unlock:
- intel_runtime_pm_put(gt->uncore->rpm, wakeref);
- return err;
-}
-
-/*
- * Create as many clients as number of doorbells. Note that there's already
- * client(s)/doorbell(s) created during driver load, but this test creates
- * its own and do not interact with the existing ones.
- */
-static int igt_guc_doorbells(void *arg)
-{
- struct intel_gt *gt = arg;
- struct intel_guc *guc = >->uc.guc;
- intel_wakeref_t wakeref;
- int i, err = 0;
- u16 db_id;
-
- GEM_BUG_ON(!HAS_GT_UC(gt->i915));
- wakeref = intel_runtime_pm_get(gt->uncore->rpm);
-
- err = check_all_doorbells(guc);
- if (err)
- goto unlock;
-
- for (i = 0; i < ATTEMPTS; i++) {
- clients[i] = guc_client_alloc(guc, i % GUC_CLIENT_PRIORITY_NUM);
-
- if (!clients[i]) {
- pr_err("[%d] No guc client\n", i);
- err = -EINVAL;
- goto out;
- }
-
- if (IS_ERR(clients[i])) {
- if (PTR_ERR(clients[i]) != -ENOSPC) {
- pr_err("[%d] unexpected error\n", i);
- err = PTR_ERR(clients[i]);
- goto out;
- }
-
- if (available_dbs(guc, i % GUC_CLIENT_PRIORITY_NUM)) {
- pr_err("[%d] non-db related alloc fail\n", i);
- err = -EINVAL;
- goto out;
- }
-
- /* expected, ran out of dbs for this client type */
- continue;
- }
-
- /*
- * The check below is only valid because we keep a doorbell
- * assigned during the whole life of the client.
- */
- if (clients[i]->stage_id >= GUC_NUM_DOORBELLS) {
- pr_err("[%d] more clients than doorbells (%d >= %d)\n",
- i, clients[i]->stage_id, GUC_NUM_DOORBELLS);
- err = -EINVAL;
- goto out;
- }
-
- err = validate_client(clients[i], i % GUC_CLIENT_PRIORITY_NUM);
- if (err) {
- pr_err("[%d] client_alloc sanity check failed!\n", i);
- err = -EINVAL;
- goto out;
- }
-
- db_id = clients[i]->doorbell_id;
-
- err = __guc_client_enable(clients[i]);
- if (err) {
- pr_err("[%d] Failed to create a doorbell\n", i);
- goto out;
- }
-
- /* doorbell id shouldn't change, we are holding the mutex */
- if (db_id != clients[i]->doorbell_id) {
- pr_err("[%d] doorbell id changed (%d != %d)\n",
- i, db_id, clients[i]->doorbell_id);
- err = -EINVAL;
- goto out;
- }
-
- err = check_all_doorbells(guc);
- if (err)
- goto out;
-
- err = ring_doorbell_nop(clients[i]);
- if (err)
- goto out;
- }
-
-out:
- for (i = 0; i < ATTEMPTS; i++)
- if (!IS_ERR_OR_NULL(clients[i])) {
- __guc_client_disable(clients[i]);
- guc_client_free(clients[i]);
- }
-unlock:
- intel_runtime_pm_put(gt->uncore->rpm, wakeref);
- return err;
-}
-
-int intel_guc_live_selftest(struct drm_i915_private *i915)
-{
- static const struct i915_subtest tests[] = {
- SUBTEST(igt_guc_clients),
- SUBTEST(igt_guc_doorbells),
- };
-
- if (!USES_GUC_SUBMISSION(i915))
- return 0;
-
- return intel_gt_live_subtests(tests, &i915->gt);
-}
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index cab632791f73..5d5974e7f3ed 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1815,17 +1815,10 @@ static int i915_guc_info(struct seq_file *m, void *data)
GEM_BUG_ON(!guc->execbuf_client);
- seq_printf(m, "\nDoorbell map:\n");
- seq_printf(m, "\t%*pb\n", GUC_NUM_DOORBELLS, guc->doorbell_bitmap);
- seq_printf(m, "Doorbell next cacheline: 0x%x\n", guc->db_cacheline);
-
seq_printf(m, "\nGuC execbuf client @ %p:\n", client);
- seq_printf(m, "\tPriority %d, GuC stage index: %u, PD offset 0x%x\n",
+ seq_printf(m, "\tPriority %d, GuC stage index: %u\n",
client->priority,
- client->stage_id,
- client->proc_desc_offset);
- seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
- client->doorbell_id, client->doorbell_offset);
+ client->stage_id);
/* Add more as required ... */
return 0;
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index 11b40bc58e6d..e0adabc614e9 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -37,5 +37,4 @@ selftest(reset, intel_reset_live_selftests)
selftest(memory_region, intel_memory_region_live_selftests)
selftest(hangcheck, intel_hangcheck_live_selftests)
selftest(execlists, intel_execlists_live_selftests)
-selftest(guc, intel_guc_live_selftest)
selftest(perf, i915_perf_live_selftests)
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [CI 4/5] drm/i915/guc: kill the GuC client
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
We now only use 1 client without any plan to add more. The client is
also only holding information about the WQ and the process desc, so we
can just move those in the intel_guc structure and always use stage_id
0.
v2: fix comment (John)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/intel_reset.c | 6 +-
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 8 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 1 -
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 275 +++++-------------
.../gpu/drm/i915/gt/uc/intel_guc_submission.h | 45 +--
drivers/gpu/drm/i915/i915_debugfs.c | 11 -
6 files changed, 89 insertions(+), 257 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index b7007cd78c6f..a12f871435fc 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -21,6 +21,7 @@
#include "intel_reset.h"
#include "uc/intel_guc.h"
+#include "uc/intel_guc_submission.h"
#define RESET_MAX_RETRIES 3
@@ -1081,6 +1082,7 @@ static inline int intel_gt_reset_engine(struct intel_engine_cs *engine)
int intel_engine_reset(struct intel_engine_cs *engine, const char *msg)
{
struct intel_gt *gt = engine->gt;
+ bool uses_guc = intel_engine_in_guc_submission_mode(engine);
int ret;
GEM_TRACE("%s flags=%lx\n", engine->name, gt->reset.flags);
@@ -1096,14 +1098,14 @@ int intel_engine_reset(struct intel_engine_cs *engine, const char *msg)
"Resetting %s for %s\n", engine->name, msg);
atomic_inc(&engine->i915->gpu_error.reset_engine_count[engine->uabi_class]);
- if (!engine->gt->uc.guc.execbuf_client)
+ if (!uses_guc)
ret = intel_gt_reset_engine(engine);
else
ret = intel_guc_reset_engine(&engine->gt->uc.guc, engine);
if (ret) {
/* If we fail here, we expect to fallback to a global reset */
DRM_DEBUG_DRIVER("%sFailed to reset %s, ret=%d\n",
- engine->gt->uc.guc.execbuf_client ? "GuC " : "",
+ uses_guc ? "GuC " : "",
engine->name, ret);
goto out;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index b2d1766e689a..cd09c912e361 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -46,9 +46,13 @@ struct intel_guc {
struct i915_vma *stage_desc_pool;
void *stage_desc_pool_vaddr;
- struct ida stage_ids;
- struct intel_guc_client *execbuf_client;
+ struct i915_vma *workqueue;
+ void *workqueue_vaddr;
+ spinlock_t wq_lock;
+
+ struct i915_vma *proc_desc;
+ void *proc_desc_vaddr;
/* Control params for fw initialization */
u32 params[GUC_CTL_MAX_DWORDS];
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
index 1e8e4af7d9ca..a6b733c146c9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
@@ -31,7 +31,6 @@
#define GUC_DOORBELL_INVALID 256
-#define GUC_PD_SIZE (PAGE_SIZE)
#define GUC_WQ_SIZE (PAGE_SIZE * 2)
/* Work queue item header definitions */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 0b89da85885c..e35e9939b207 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -27,24 +27,13 @@
* code) matches the old submission model and will be updated as part of the
* upgrade to the new flow.
*
- * GuC client:
- * A intel_guc_client refers to a submission path through GuC. Currently, there
- * is only one client, which is charged with all submissions to the GuC. This
- * struct is the owner of a process descriptor and a workqueue (both of them
- * inside a single gem object that contains all required pages for these
- * elements).
- *
* GuC stage descriptor:
* During initialization, the driver allocates a static pool of 1024 such
- * descriptors, and shares them with the GuC.
- * Currently, there exists a 1:1 mapping between a intel_guc_client and a
- * guc_stage_desc (via the client's stage_id), so effectively only one
- * gets used. This stage descriptor lets the GuC know about the workqueue and
+ * descriptors, and shares them with the GuC. Currently, we only use one
+ * descriptor. This stage descriptor lets the GuC know about the workqueue and
* process descriptor. Theoretically, it also lets the GuC know about our HW
* contexts (context ID, etc...), but we actually employ a kind of submission
- * where the GuC uses the LRCA sent via the work item instead (the single
- * guc_stage_desc associated to execbuf client contains information about the
- * default kernel context only, but this is essentially unused). This is called
+ * where the GuC uses the LRCA sent via the work item instead. This is called
* a "proxy" submission.
*
* The Scratch registers:
@@ -71,33 +60,45 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb)
return rb_entry(rb, struct i915_priolist, node);
}
-static inline bool is_high_priority(struct intel_guc_client *client)
+static struct guc_stage_desc *__get_stage_desc(struct intel_guc *guc, u32 id)
{
- return (client->priority == GUC_CLIENT_PRIORITY_KMD_HIGH ||
- client->priority == GUC_CLIENT_PRIORITY_HIGH);
+ struct guc_stage_desc *base = guc->stage_desc_pool_vaddr;
+
+ return &base[id];
}
-static struct guc_stage_desc *__get_stage_desc(struct intel_guc_client *client)
+static int guc_workqueue_create(struct intel_guc *guc)
{
- struct guc_stage_desc *base = client->guc->stage_desc_pool_vaddr;
-
- return &base[client->stage_id];
+ return intel_guc_allocate_and_map_vma(guc, GUC_WQ_SIZE, &guc->workqueue,
+ &guc->workqueue_vaddr);
}
-static inline struct guc_process_desc *
-__get_process_desc(struct intel_guc_client *client)
+static void guc_workqueue_destroy(struct intel_guc *guc)
{
- return client->vaddr;
+ i915_vma_unpin_and_release(&guc->workqueue, I915_VMA_RELEASE_MAP);
}
/*
* Initialise the process descriptor shared with the GuC firmware.
*/
-static void guc_proc_desc_init(struct intel_guc_client *client)
+static int guc_proc_desc_create(struct intel_guc *guc)
+{
+ const u32 size = PAGE_ALIGN(sizeof(struct guc_process_desc));
+
+ return intel_guc_allocate_and_map_vma(guc, size, &guc->proc_desc,
+ &guc->proc_desc_vaddr);
+}
+
+static void guc_proc_desc_destroy(struct intel_guc *guc)
+{
+ i915_vma_unpin_and_release(&guc->proc_desc, I915_VMA_RELEASE_MAP);
+}
+
+static void guc_proc_desc_init(struct intel_guc *guc)
{
struct guc_process_desc *desc;
- desc = memset(__get_process_desc(client), 0, sizeof(*desc));
+ desc = memset(guc->proc_desc_vaddr, 0, sizeof(*desc));
/*
* XXX: pDoorbell and WQVBaseAddress are pointers in process address
@@ -108,39 +109,27 @@ static void guc_proc_desc_init(struct intel_guc_client *client)
desc->wq_base_addr = 0;
desc->db_base_addr = 0;
- desc->stage_id = client->stage_id;
desc->wq_size_bytes = GUC_WQ_SIZE;
desc->wq_status = WQ_STATUS_ACTIVE;
- desc->priority = client->priority;
+ desc->priority = GUC_CLIENT_PRIORITY_KMD_NORMAL;
}
-static void guc_proc_desc_fini(struct intel_guc_client *client)
+static void guc_proc_desc_fini(struct intel_guc *guc)
{
- struct guc_process_desc *desc;
-
- desc = __get_process_desc(client);
- memset(desc, 0, sizeof(*desc));
+ memset(guc->proc_desc_vaddr, 0, sizeof(struct guc_process_desc));
}
static int guc_stage_desc_pool_create(struct intel_guc *guc)
{
u32 size = PAGE_ALIGN(sizeof(struct guc_stage_desc) *
GUC_MAX_STAGE_DESCRIPTORS);
- int ret;
-
- ret = intel_guc_allocate_and_map_vma(guc, size, &guc->stage_desc_pool,
- &guc->stage_desc_pool_vaddr);
- if (ret)
- return ret;
-
- ida_init(&guc->stage_ids);
- return 0;
+ return intel_guc_allocate_and_map_vma(guc, size, &guc->stage_desc_pool,
+ &guc->stage_desc_pool_vaddr);
}
static void guc_stage_desc_pool_destroy(struct intel_guc *guc)
{
- ida_destroy(&guc->stage_ids);
i915_vma_unpin_and_release(&guc->stage_desc_pool, I915_VMA_RELEASE_MAP);
}
@@ -148,58 +137,49 @@ static void guc_stage_desc_pool_destroy(struct intel_guc *guc)
* Initialise/clear the stage descriptor shared with the GuC firmware.
*
* This descriptor tells the GuC where (in GGTT space) to find the important
- * data structures relating to this client (process descriptor, write queue,
+ * data structures related to work submission (process descriptor, write queue,
* etc).
*/
-static void guc_stage_desc_init(struct intel_guc_client *client)
+static void guc_stage_desc_init(struct intel_guc *guc)
{
- struct intel_guc *guc = client->guc;
struct guc_stage_desc *desc;
- u32 gfx_addr;
- desc = __get_stage_desc(client);
+ /* we only use 1 stage desc, so hardcode it to 0 */
+ desc = __get_stage_desc(guc, 0);
memset(desc, 0, sizeof(*desc));
desc->attribute = GUC_STAGE_DESC_ATTR_ACTIVE |
GUC_STAGE_DESC_ATTR_KERNEL;
- if (is_high_priority(client))
- desc->attribute |= GUC_STAGE_DESC_ATTR_PREEMPT;
- desc->stage_id = client->stage_id;
- desc->priority = client->priority;
- /*
- * The process descriptor and workqueue are all parts of the client
- * object, which the GuC will reference via the GGTT
- */
- gfx_addr = intel_guc_ggtt_offset(guc, client->vma);
- desc->process_desc = gfx_addr;
- desc->wq_addr = gfx_addr + GUC_PD_SIZE;
- desc->wq_size = GUC_WQ_SIZE;
+ desc->stage_id = 0;
+ desc->priority = GUC_CLIENT_PRIORITY_KMD_NORMAL;
- desc->desc_private = ptr_to_u64(client);
+ desc->process_desc = intel_guc_ggtt_offset(guc, guc->proc_desc);
+ desc->wq_addr = intel_guc_ggtt_offset(guc, guc->workqueue);
+ desc->wq_size = GUC_WQ_SIZE;
}
-static void guc_stage_desc_fini(struct intel_guc_client *client)
+static void guc_stage_desc_fini(struct intel_guc *guc)
{
struct guc_stage_desc *desc;
- desc = __get_stage_desc(client);
+ desc = __get_stage_desc(guc, 0);
memset(desc, 0, sizeof(*desc));
}
/* Construct a Work Item and append it to the GuC's Work Queue */
-static void guc_wq_item_append(struct intel_guc_client *client,
+static void guc_wq_item_append(struct intel_guc *guc,
u32 target_engine, u32 context_desc,
u32 ring_tail, u32 fence_id)
{
/* wqi_len is in DWords, and does not include the one-word header */
const size_t wqi_size = sizeof(struct guc_wq_item);
const u32 wqi_len = wqi_size / sizeof(u32) - 1;
- struct guc_process_desc *desc = __get_process_desc(client);
+ struct guc_process_desc *desc = guc->proc_desc_vaddr;
struct guc_wq_item *wqi;
u32 wq_off;
- lockdep_assert_held(&client->wq_lock);
+ lockdep_assert_held(&guc->wq_lock);
/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
* should not have the case where structure wqi is across page, neither
@@ -220,7 +200,7 @@ static void guc_wq_item_append(struct intel_guc_client *client,
GEM_BUG_ON(wq_off & (wqi_size - 1));
/* WQ starts from the page after process_desc */
- wqi = client->vaddr + wq_off + GUC_PD_SIZE;
+ wqi = guc->workqueue_vaddr + wq_off;
/* Now fill in the 4-word work queue item */
wqi->header = WQ_TYPE_INORDER |
@@ -238,12 +218,11 @@ static void guc_wq_item_append(struct intel_guc_client *client,
static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
{
- struct intel_guc_client *client = guc->execbuf_client;
struct intel_engine_cs *engine = rq->engine;
u32 ctx_desc = lower_32_bits(rq->hw_context->lrc_desc);
u32 ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
- guc_wq_item_append(client, engine->guc_id, ctx_desc,
+ guc_wq_item_append(guc, engine->guc_id, ctx_desc,
ring_tail, rq->fence.seqno);
}
@@ -267,9 +246,8 @@ static void guc_submit(struct intel_engine_cs *engine,
struct i915_request **end)
{
struct intel_guc *guc = &engine->gt->uc.guc;
- struct intel_guc_client *client = guc->execbuf_client;
- spin_lock(&client->wq_lock);
+ spin_lock(&guc->wq_lock);
do {
struct i915_request *rq = *out++;
@@ -278,7 +256,7 @@ static void guc_submit(struct intel_engine_cs *engine,
guc_add_request(guc, rq);
} while (out != end);
- spin_unlock(&client->wq_lock);
+ spin_unlock(&guc->wq_lock);
}
static inline int rq_prio(const struct i915_request *rq)
@@ -529,126 +507,6 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
* path of guc_submit() above.
*/
-/**
- * guc_client_alloc() - Allocate an intel_guc_client
- * @guc: the intel_guc structure
- * @priority: four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
- * The kernel client to replace ExecList submission is created with
- * NORMAL priority. Priority of a client for scheduler can be HIGH,
- * while a preemption context can use CRITICAL.
- *
- * Return: An intel_guc_client object if success, else NULL.
- */
-static struct intel_guc_client *
-guc_client_alloc(struct intel_guc *guc, u32 priority)
-{
- struct intel_guc_client *client;
- struct i915_vma *vma;
- void *vaddr;
- int ret;
-
- client = kzalloc(sizeof(*client), GFP_KERNEL);
- if (!client)
- return ERR_PTR(-ENOMEM);
-
- client->guc = guc;
- client->priority = priority;
- spin_lock_init(&client->wq_lock);
-
- ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
- GFP_KERNEL);
- if (ret < 0)
- goto err_client;
-
- client->stage_id = ret;
-
- /* The first page is proc_desc. Two following pages are wq. */
- vma = intel_guc_allocate_vma(guc, GUC_PD_SIZE + GUC_WQ_SIZE);
- if (IS_ERR(vma)) {
- ret = PTR_ERR(vma);
- goto err_id;
- }
-
- /* We'll keep just the first (doorbell/proc) page permanently kmap'd. */
- client->vma = vma;
-
- vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
- if (IS_ERR(vaddr)) {
- ret = PTR_ERR(vaddr);
- goto err_vma;
- }
- client->vaddr = vaddr;
-
- DRM_DEBUG_DRIVER("new priority %u client %p: stage_id %u\n",
- priority, client, client->stage_id);
-
- return client;
-
-err_vma:
- i915_vma_unpin_and_release(&client->vma, 0);
-err_id:
- ida_simple_remove(&guc->stage_ids, client->stage_id);
-err_client:
- kfree(client);
- return ERR_PTR(ret);
-}
-
-static void guc_client_free(struct intel_guc_client *client)
-{
- i915_vma_unpin_and_release(&client->vma, I915_VMA_RELEASE_MAP);
- ida_simple_remove(&client->guc->stage_ids, client->stage_id);
- kfree(client);
-}
-
-static int guc_clients_create(struct intel_guc *guc)
-{
- struct intel_guc_client *client;
-
- GEM_BUG_ON(guc->execbuf_client);
-
- client = guc_client_alloc(guc, GUC_CLIENT_PRIORITY_KMD_NORMAL);
- if (IS_ERR(client)) {
- DRM_ERROR("Failed to create GuC client for submission!\n");
- return PTR_ERR(client);
- }
- guc->execbuf_client = client;
-
- return 0;
-}
-
-static void guc_clients_destroy(struct intel_guc *guc)
-{
- struct intel_guc_client *client;
-
- client = fetch_and_zero(&guc->execbuf_client);
- if (client)
- guc_client_free(client);
-}
-
-static void __guc_client_enable(struct intel_guc_client *client)
-{
- guc_proc_desc_init(client);
- guc_stage_desc_init(client);
-}
-
-static void __guc_client_disable(struct intel_guc_client *client)
-{
- /* Note: By the time we're here, GuC may have already been reset */
- guc_stage_desc_fini(client);
- guc_proc_desc_fini(client);
-}
-
-static void guc_clients_enable(struct intel_guc *guc)
-{
- __guc_client_enable(guc->execbuf_client);
-}
-
-static void guc_clients_disable(struct intel_guc *guc)
-{
- if (guc->execbuf_client)
- __guc_client_disable(guc->execbuf_client);
-}
-
/*
* Set up the memory resources to be shared with the GuC (via the GGTT)
* at firmware loading time.
@@ -669,12 +527,20 @@ int intel_guc_submission_init(struct intel_guc *guc)
*/
GEM_BUG_ON(!guc->stage_desc_pool);
- ret = guc_clients_create(guc);
+ ret = guc_workqueue_create(guc);
if (ret)
goto err_pool;
+ ret = guc_proc_desc_create(guc);
+ if (ret)
+ goto err_workqueue;
+
+ spin_lock_init(&guc->wq_lock);
+
return 0;
+err_workqueue:
+ guc_workqueue_destroy(guc);
err_pool:
guc_stage_desc_pool_destroy(guc);
return ret;
@@ -682,10 +548,11 @@ int intel_guc_submission_init(struct intel_guc *guc)
void intel_guc_submission_fini(struct intel_guc *guc)
{
- guc_clients_destroy(guc);
-
- if (guc->stage_desc_pool)
+ if (guc->stage_desc_pool) {
+ guc_proc_desc_destroy(guc);
+ guc_workqueue_destroy(guc);
guc_stage_desc_pool_destroy(guc);
+ }
}
static void guc_interrupts_capture(struct intel_gt *gt)
@@ -771,9 +638,8 @@ void intel_guc_submission_enable(struct intel_guc *guc)
sizeof(struct guc_wq_item) *
I915_NUM_ENGINES > GUC_WQ_SIZE);
- GEM_BUG_ON(!guc->execbuf_client);
-
- guc_clients_enable(guc);
+ guc_proc_desc_init(guc);
+ guc_stage_desc_init(guc);
/* Take over from manual control of ELSP (execlists) */
guc_interrupts_capture(gt);
@@ -790,8 +656,12 @@ void intel_guc_submission_disable(struct intel_guc *guc)
GEM_BUG_ON(gt->awake); /* GT should be parked first */
+ /* Note: By the time we're here, GuC may have already been reset */
+
guc_interrupts_release(gt);
- guc_clients_disable(guc);
+
+ guc_stage_desc_fini(guc);
+ guc_proc_desc_fini(guc);
}
static bool __guc_submission_support(struct intel_guc *guc)
@@ -809,3 +679,8 @@ void intel_guc_submission_init_early(struct intel_guc *guc)
{
guc->submission_supported = __guc_submission_support(guc);
}
+
+bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine)
+{
+ return engine->set_default_submission == guc_set_default_submission;
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index e2deb4e6f42a..e402a2932592 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -6,48 +6,10 @@
#ifndef _INTEL_GUC_SUBMISSION_H_
#define _INTEL_GUC_SUBMISSION_H_
-#include <linux/spinlock.h>
+#include <linux/types.h>
-#include "gt/intel_engine_types.h"
-
-#include "i915_gem.h"
-#include "i915_selftest.h"
-
-struct drm_i915_private;
-
-/*
- * This structure primarily describes the GEM object shared with the GuC.
- * The specs sometimes refer to this object as a "GuC context", but we use
- * the term "client" to avoid confusion with hardware contexts. This
- * GEM object is held for the entire lifetime of our interaction with
- * the GuC, being allocated before the GuC is loaded with its firmware.
- * Because there's no way to update the address used by the GuC after
- * initialisation, the shared object must stay pinned into the GGTT as
- * long as the GuC is in use. We also keep the first page (only) mapped
- * into kernel address space, as it includes shared data that must be
- * updated on every request submission.
- *
- * The single GEM object described here is actually made up of several
- * separate areas, as far as the GuC is concerned. The first page (kept
- * kmap'd) includes the "process descriptor" which holds sequence data for
- * the doorbell, and one cacheline which actually *is* the doorbell; a
- * write to this will "ring the doorbell" (i.e. send an interrupt to the
- * GuC). The subsequent pages of the client object constitute the work
- * queue (a circular array of work items), again described in the process
- * descriptor. Work queue pages are mapped momentarily as required.
- */
-struct intel_guc_client {
- struct i915_vma *vma;
- void *vaddr;
- struct intel_guc *guc;
-
- /* bitmap of (host) engine ids */
- u32 priority;
- u32 stage_id;
-
- /* Protects GuC client's WQ access */
- spinlock_t wq_lock;
-};
+struct intel_guc;
+struct intel_engine_cs;
void intel_guc_submission_init_early(struct intel_guc *guc);
int intel_guc_submission_init(struct intel_guc *guc);
@@ -56,5 +18,6 @@ void intel_guc_submission_disable(struct intel_guc *guc);
void intel_guc_submission_fini(struct intel_guc *guc);
int intel_guc_preempt_work_create(struct intel_guc *guc);
void intel_guc_preempt_work_destroy(struct intel_guc *guc);
+bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine);
#endif
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5d5974e7f3ed..f32e7b016197 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1802,23 +1802,12 @@ static void i915_guc_log_info(struct seq_file *m,
static int i915_guc_info(struct seq_file *m, void *data)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
- const struct intel_guc *guc = &dev_priv->gt.uc.guc;
- struct intel_guc_client *client = guc->execbuf_client;
if (!USES_GUC(dev_priv))
return -ENODEV;
i915_guc_log_info(m, dev_priv);
- if (!USES_GUC_SUBMISSION(dev_priv))
- return 0;
-
- GEM_BUG_ON(!guc->execbuf_client);
-
- seq_printf(m, "\nGuC execbuf client @ %p:\n", client);
- seq_printf(m, "\tPriority %d, GuC stage index: %u\n",
- client->priority,
- client->stage_id);
/* Add more as required ... */
return 0;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Intel-gfx] [CI 4/5] drm/i915/guc: kill the GuC client
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
We now only use 1 client without any plan to add more. The client is
also only holding information about the WQ and the process desc, so we
can just move those in the intel_guc structure and always use stage_id
0.
v2: fix comment (John)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/intel_reset.c | 6 +-
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 8 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 1 -
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 275 +++++-------------
.../gpu/drm/i915/gt/uc/intel_guc_submission.h | 45 +--
drivers/gpu/drm/i915/i915_debugfs.c | 11 -
6 files changed, 89 insertions(+), 257 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index b7007cd78c6f..a12f871435fc 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -21,6 +21,7 @@
#include "intel_reset.h"
#include "uc/intel_guc.h"
+#include "uc/intel_guc_submission.h"
#define RESET_MAX_RETRIES 3
@@ -1081,6 +1082,7 @@ static inline int intel_gt_reset_engine(struct intel_engine_cs *engine)
int intel_engine_reset(struct intel_engine_cs *engine, const char *msg)
{
struct intel_gt *gt = engine->gt;
+ bool uses_guc = intel_engine_in_guc_submission_mode(engine);
int ret;
GEM_TRACE("%s flags=%lx\n", engine->name, gt->reset.flags);
@@ -1096,14 +1098,14 @@ int intel_engine_reset(struct intel_engine_cs *engine, const char *msg)
"Resetting %s for %s\n", engine->name, msg);
atomic_inc(&engine->i915->gpu_error.reset_engine_count[engine->uabi_class]);
- if (!engine->gt->uc.guc.execbuf_client)
+ if (!uses_guc)
ret = intel_gt_reset_engine(engine);
else
ret = intel_guc_reset_engine(&engine->gt->uc.guc, engine);
if (ret) {
/* If we fail here, we expect to fallback to a global reset */
DRM_DEBUG_DRIVER("%sFailed to reset %s, ret=%d\n",
- engine->gt->uc.guc.execbuf_client ? "GuC " : "",
+ uses_guc ? "GuC " : "",
engine->name, ret);
goto out;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index b2d1766e689a..cd09c912e361 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -46,9 +46,13 @@ struct intel_guc {
struct i915_vma *stage_desc_pool;
void *stage_desc_pool_vaddr;
- struct ida stage_ids;
- struct intel_guc_client *execbuf_client;
+ struct i915_vma *workqueue;
+ void *workqueue_vaddr;
+ spinlock_t wq_lock;
+
+ struct i915_vma *proc_desc;
+ void *proc_desc_vaddr;
/* Control params for fw initialization */
u32 params[GUC_CTL_MAX_DWORDS];
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
index 1e8e4af7d9ca..a6b733c146c9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
@@ -31,7 +31,6 @@
#define GUC_DOORBELL_INVALID 256
-#define GUC_PD_SIZE (PAGE_SIZE)
#define GUC_WQ_SIZE (PAGE_SIZE * 2)
/* Work queue item header definitions */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 0b89da85885c..e35e9939b207 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -27,24 +27,13 @@
* code) matches the old submission model and will be updated as part of the
* upgrade to the new flow.
*
- * GuC client:
- * A intel_guc_client refers to a submission path through GuC. Currently, there
- * is only one client, which is charged with all submissions to the GuC. This
- * struct is the owner of a process descriptor and a workqueue (both of them
- * inside a single gem object that contains all required pages for these
- * elements).
- *
* GuC stage descriptor:
* During initialization, the driver allocates a static pool of 1024 such
- * descriptors, and shares them with the GuC.
- * Currently, there exists a 1:1 mapping between a intel_guc_client and a
- * guc_stage_desc (via the client's stage_id), so effectively only one
- * gets used. This stage descriptor lets the GuC know about the workqueue and
+ * descriptors, and shares them with the GuC. Currently, we only use one
+ * descriptor. This stage descriptor lets the GuC know about the workqueue and
* process descriptor. Theoretically, it also lets the GuC know about our HW
* contexts (context ID, etc...), but we actually employ a kind of submission
- * where the GuC uses the LRCA sent via the work item instead (the single
- * guc_stage_desc associated to execbuf client contains information about the
- * default kernel context only, but this is essentially unused). This is called
+ * where the GuC uses the LRCA sent via the work item instead. This is called
* a "proxy" submission.
*
* The Scratch registers:
@@ -71,33 +60,45 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb)
return rb_entry(rb, struct i915_priolist, node);
}
-static inline bool is_high_priority(struct intel_guc_client *client)
+static struct guc_stage_desc *__get_stage_desc(struct intel_guc *guc, u32 id)
{
- return (client->priority == GUC_CLIENT_PRIORITY_KMD_HIGH ||
- client->priority == GUC_CLIENT_PRIORITY_HIGH);
+ struct guc_stage_desc *base = guc->stage_desc_pool_vaddr;
+
+ return &base[id];
}
-static struct guc_stage_desc *__get_stage_desc(struct intel_guc_client *client)
+static int guc_workqueue_create(struct intel_guc *guc)
{
- struct guc_stage_desc *base = client->guc->stage_desc_pool_vaddr;
-
- return &base[client->stage_id];
+ return intel_guc_allocate_and_map_vma(guc, GUC_WQ_SIZE, &guc->workqueue,
+ &guc->workqueue_vaddr);
}
-static inline struct guc_process_desc *
-__get_process_desc(struct intel_guc_client *client)
+static void guc_workqueue_destroy(struct intel_guc *guc)
{
- return client->vaddr;
+ i915_vma_unpin_and_release(&guc->workqueue, I915_VMA_RELEASE_MAP);
}
/*
* Initialise the process descriptor shared with the GuC firmware.
*/
-static void guc_proc_desc_init(struct intel_guc_client *client)
+static int guc_proc_desc_create(struct intel_guc *guc)
+{
+ const u32 size = PAGE_ALIGN(sizeof(struct guc_process_desc));
+
+ return intel_guc_allocate_and_map_vma(guc, size, &guc->proc_desc,
+ &guc->proc_desc_vaddr);
+}
+
+static void guc_proc_desc_destroy(struct intel_guc *guc)
+{
+ i915_vma_unpin_and_release(&guc->proc_desc, I915_VMA_RELEASE_MAP);
+}
+
+static void guc_proc_desc_init(struct intel_guc *guc)
{
struct guc_process_desc *desc;
- desc = memset(__get_process_desc(client), 0, sizeof(*desc));
+ desc = memset(guc->proc_desc_vaddr, 0, sizeof(*desc));
/*
* XXX: pDoorbell and WQVBaseAddress are pointers in process address
@@ -108,39 +109,27 @@ static void guc_proc_desc_init(struct intel_guc_client *client)
desc->wq_base_addr = 0;
desc->db_base_addr = 0;
- desc->stage_id = client->stage_id;
desc->wq_size_bytes = GUC_WQ_SIZE;
desc->wq_status = WQ_STATUS_ACTIVE;
- desc->priority = client->priority;
+ desc->priority = GUC_CLIENT_PRIORITY_KMD_NORMAL;
}
-static void guc_proc_desc_fini(struct intel_guc_client *client)
+static void guc_proc_desc_fini(struct intel_guc *guc)
{
- struct guc_process_desc *desc;
-
- desc = __get_process_desc(client);
- memset(desc, 0, sizeof(*desc));
+ memset(guc->proc_desc_vaddr, 0, sizeof(struct guc_process_desc));
}
static int guc_stage_desc_pool_create(struct intel_guc *guc)
{
u32 size = PAGE_ALIGN(sizeof(struct guc_stage_desc) *
GUC_MAX_STAGE_DESCRIPTORS);
- int ret;
-
- ret = intel_guc_allocate_and_map_vma(guc, size, &guc->stage_desc_pool,
- &guc->stage_desc_pool_vaddr);
- if (ret)
- return ret;
-
- ida_init(&guc->stage_ids);
- return 0;
+ return intel_guc_allocate_and_map_vma(guc, size, &guc->stage_desc_pool,
+ &guc->stage_desc_pool_vaddr);
}
static void guc_stage_desc_pool_destroy(struct intel_guc *guc)
{
- ida_destroy(&guc->stage_ids);
i915_vma_unpin_and_release(&guc->stage_desc_pool, I915_VMA_RELEASE_MAP);
}
@@ -148,58 +137,49 @@ static void guc_stage_desc_pool_destroy(struct intel_guc *guc)
* Initialise/clear the stage descriptor shared with the GuC firmware.
*
* This descriptor tells the GuC where (in GGTT space) to find the important
- * data structures relating to this client (process descriptor, write queue,
+ * data structures related to work submission (process descriptor, write queue,
* etc).
*/
-static void guc_stage_desc_init(struct intel_guc_client *client)
+static void guc_stage_desc_init(struct intel_guc *guc)
{
- struct intel_guc *guc = client->guc;
struct guc_stage_desc *desc;
- u32 gfx_addr;
- desc = __get_stage_desc(client);
+ /* we only use 1 stage desc, so hardcode it to 0 */
+ desc = __get_stage_desc(guc, 0);
memset(desc, 0, sizeof(*desc));
desc->attribute = GUC_STAGE_DESC_ATTR_ACTIVE |
GUC_STAGE_DESC_ATTR_KERNEL;
- if (is_high_priority(client))
- desc->attribute |= GUC_STAGE_DESC_ATTR_PREEMPT;
- desc->stage_id = client->stage_id;
- desc->priority = client->priority;
- /*
- * The process descriptor and workqueue are all parts of the client
- * object, which the GuC will reference via the GGTT
- */
- gfx_addr = intel_guc_ggtt_offset(guc, client->vma);
- desc->process_desc = gfx_addr;
- desc->wq_addr = gfx_addr + GUC_PD_SIZE;
- desc->wq_size = GUC_WQ_SIZE;
+ desc->stage_id = 0;
+ desc->priority = GUC_CLIENT_PRIORITY_KMD_NORMAL;
- desc->desc_private = ptr_to_u64(client);
+ desc->process_desc = intel_guc_ggtt_offset(guc, guc->proc_desc);
+ desc->wq_addr = intel_guc_ggtt_offset(guc, guc->workqueue);
+ desc->wq_size = GUC_WQ_SIZE;
}
-static void guc_stage_desc_fini(struct intel_guc_client *client)
+static void guc_stage_desc_fini(struct intel_guc *guc)
{
struct guc_stage_desc *desc;
- desc = __get_stage_desc(client);
+ desc = __get_stage_desc(guc, 0);
memset(desc, 0, sizeof(*desc));
}
/* Construct a Work Item and append it to the GuC's Work Queue */
-static void guc_wq_item_append(struct intel_guc_client *client,
+static void guc_wq_item_append(struct intel_guc *guc,
u32 target_engine, u32 context_desc,
u32 ring_tail, u32 fence_id)
{
/* wqi_len is in DWords, and does not include the one-word header */
const size_t wqi_size = sizeof(struct guc_wq_item);
const u32 wqi_len = wqi_size / sizeof(u32) - 1;
- struct guc_process_desc *desc = __get_process_desc(client);
+ struct guc_process_desc *desc = guc->proc_desc_vaddr;
struct guc_wq_item *wqi;
u32 wq_off;
- lockdep_assert_held(&client->wq_lock);
+ lockdep_assert_held(&guc->wq_lock);
/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
* should not have the case where structure wqi is across page, neither
@@ -220,7 +200,7 @@ static void guc_wq_item_append(struct intel_guc_client *client,
GEM_BUG_ON(wq_off & (wqi_size - 1));
/* WQ starts from the page after process_desc */
- wqi = client->vaddr + wq_off + GUC_PD_SIZE;
+ wqi = guc->workqueue_vaddr + wq_off;
/* Now fill in the 4-word work queue item */
wqi->header = WQ_TYPE_INORDER |
@@ -238,12 +218,11 @@ static void guc_wq_item_append(struct intel_guc_client *client,
static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
{
- struct intel_guc_client *client = guc->execbuf_client;
struct intel_engine_cs *engine = rq->engine;
u32 ctx_desc = lower_32_bits(rq->hw_context->lrc_desc);
u32 ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
- guc_wq_item_append(client, engine->guc_id, ctx_desc,
+ guc_wq_item_append(guc, engine->guc_id, ctx_desc,
ring_tail, rq->fence.seqno);
}
@@ -267,9 +246,8 @@ static void guc_submit(struct intel_engine_cs *engine,
struct i915_request **end)
{
struct intel_guc *guc = &engine->gt->uc.guc;
- struct intel_guc_client *client = guc->execbuf_client;
- spin_lock(&client->wq_lock);
+ spin_lock(&guc->wq_lock);
do {
struct i915_request *rq = *out++;
@@ -278,7 +256,7 @@ static void guc_submit(struct intel_engine_cs *engine,
guc_add_request(guc, rq);
} while (out != end);
- spin_unlock(&client->wq_lock);
+ spin_unlock(&guc->wq_lock);
}
static inline int rq_prio(const struct i915_request *rq)
@@ -529,126 +507,6 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
* path of guc_submit() above.
*/
-/**
- * guc_client_alloc() - Allocate an intel_guc_client
- * @guc: the intel_guc structure
- * @priority: four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
- * The kernel client to replace ExecList submission is created with
- * NORMAL priority. Priority of a client for scheduler can be HIGH,
- * while a preemption context can use CRITICAL.
- *
- * Return: An intel_guc_client object if success, else NULL.
- */
-static struct intel_guc_client *
-guc_client_alloc(struct intel_guc *guc, u32 priority)
-{
- struct intel_guc_client *client;
- struct i915_vma *vma;
- void *vaddr;
- int ret;
-
- client = kzalloc(sizeof(*client), GFP_KERNEL);
- if (!client)
- return ERR_PTR(-ENOMEM);
-
- client->guc = guc;
- client->priority = priority;
- spin_lock_init(&client->wq_lock);
-
- ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
- GFP_KERNEL);
- if (ret < 0)
- goto err_client;
-
- client->stage_id = ret;
-
- /* The first page is proc_desc. Two following pages are wq. */
- vma = intel_guc_allocate_vma(guc, GUC_PD_SIZE + GUC_WQ_SIZE);
- if (IS_ERR(vma)) {
- ret = PTR_ERR(vma);
- goto err_id;
- }
-
- /* We'll keep just the first (doorbell/proc) page permanently kmap'd. */
- client->vma = vma;
-
- vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
- if (IS_ERR(vaddr)) {
- ret = PTR_ERR(vaddr);
- goto err_vma;
- }
- client->vaddr = vaddr;
-
- DRM_DEBUG_DRIVER("new priority %u client %p: stage_id %u\n",
- priority, client, client->stage_id);
-
- return client;
-
-err_vma:
- i915_vma_unpin_and_release(&client->vma, 0);
-err_id:
- ida_simple_remove(&guc->stage_ids, client->stage_id);
-err_client:
- kfree(client);
- return ERR_PTR(ret);
-}
-
-static void guc_client_free(struct intel_guc_client *client)
-{
- i915_vma_unpin_and_release(&client->vma, I915_VMA_RELEASE_MAP);
- ida_simple_remove(&client->guc->stage_ids, client->stage_id);
- kfree(client);
-}
-
-static int guc_clients_create(struct intel_guc *guc)
-{
- struct intel_guc_client *client;
-
- GEM_BUG_ON(guc->execbuf_client);
-
- client = guc_client_alloc(guc, GUC_CLIENT_PRIORITY_KMD_NORMAL);
- if (IS_ERR(client)) {
- DRM_ERROR("Failed to create GuC client for submission!\n");
- return PTR_ERR(client);
- }
- guc->execbuf_client = client;
-
- return 0;
-}
-
-static void guc_clients_destroy(struct intel_guc *guc)
-{
- struct intel_guc_client *client;
-
- client = fetch_and_zero(&guc->execbuf_client);
- if (client)
- guc_client_free(client);
-}
-
-static void __guc_client_enable(struct intel_guc_client *client)
-{
- guc_proc_desc_init(client);
- guc_stage_desc_init(client);
-}
-
-static void __guc_client_disable(struct intel_guc_client *client)
-{
- /* Note: By the time we're here, GuC may have already been reset */
- guc_stage_desc_fini(client);
- guc_proc_desc_fini(client);
-}
-
-static void guc_clients_enable(struct intel_guc *guc)
-{
- __guc_client_enable(guc->execbuf_client);
-}
-
-static void guc_clients_disable(struct intel_guc *guc)
-{
- if (guc->execbuf_client)
- __guc_client_disable(guc->execbuf_client);
-}
-
/*
* Set up the memory resources to be shared with the GuC (via the GGTT)
* at firmware loading time.
@@ -669,12 +527,20 @@ int intel_guc_submission_init(struct intel_guc *guc)
*/
GEM_BUG_ON(!guc->stage_desc_pool);
- ret = guc_clients_create(guc);
+ ret = guc_workqueue_create(guc);
if (ret)
goto err_pool;
+ ret = guc_proc_desc_create(guc);
+ if (ret)
+ goto err_workqueue;
+
+ spin_lock_init(&guc->wq_lock);
+
return 0;
+err_workqueue:
+ guc_workqueue_destroy(guc);
err_pool:
guc_stage_desc_pool_destroy(guc);
return ret;
@@ -682,10 +548,11 @@ int intel_guc_submission_init(struct intel_guc *guc)
void intel_guc_submission_fini(struct intel_guc *guc)
{
- guc_clients_destroy(guc);
-
- if (guc->stage_desc_pool)
+ if (guc->stage_desc_pool) {
+ guc_proc_desc_destroy(guc);
+ guc_workqueue_destroy(guc);
guc_stage_desc_pool_destroy(guc);
+ }
}
static void guc_interrupts_capture(struct intel_gt *gt)
@@ -771,9 +638,8 @@ void intel_guc_submission_enable(struct intel_guc *guc)
sizeof(struct guc_wq_item) *
I915_NUM_ENGINES > GUC_WQ_SIZE);
- GEM_BUG_ON(!guc->execbuf_client);
-
- guc_clients_enable(guc);
+ guc_proc_desc_init(guc);
+ guc_stage_desc_init(guc);
/* Take over from manual control of ELSP (execlists) */
guc_interrupts_capture(gt);
@@ -790,8 +656,12 @@ void intel_guc_submission_disable(struct intel_guc *guc)
GEM_BUG_ON(gt->awake); /* GT should be parked first */
+ /* Note: By the time we're here, GuC may have already been reset */
+
guc_interrupts_release(gt);
- guc_clients_disable(guc);
+
+ guc_stage_desc_fini(guc);
+ guc_proc_desc_fini(guc);
}
static bool __guc_submission_support(struct intel_guc *guc)
@@ -809,3 +679,8 @@ void intel_guc_submission_init_early(struct intel_guc *guc)
{
guc->submission_supported = __guc_submission_support(guc);
}
+
+bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine)
+{
+ return engine->set_default_submission == guc_set_default_submission;
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index e2deb4e6f42a..e402a2932592 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -6,48 +6,10 @@
#ifndef _INTEL_GUC_SUBMISSION_H_
#define _INTEL_GUC_SUBMISSION_H_
-#include <linux/spinlock.h>
+#include <linux/types.h>
-#include "gt/intel_engine_types.h"
-
-#include "i915_gem.h"
-#include "i915_selftest.h"
-
-struct drm_i915_private;
-
-/*
- * This structure primarily describes the GEM object shared with the GuC.
- * The specs sometimes refer to this object as a "GuC context", but we use
- * the term "client" to avoid confusion with hardware contexts. This
- * GEM object is held for the entire lifetime of our interaction with
- * the GuC, being allocated before the GuC is loaded with its firmware.
- * Because there's no way to update the address used by the GuC after
- * initialisation, the shared object must stay pinned into the GGTT as
- * long as the GuC is in use. We also keep the first page (only) mapped
- * into kernel address space, as it includes shared data that must be
- * updated on every request submission.
- *
- * The single GEM object described here is actually made up of several
- * separate areas, as far as the GuC is concerned. The first page (kept
- * kmap'd) includes the "process descriptor" which holds sequence data for
- * the doorbell, and one cacheline which actually *is* the doorbell; a
- * write to this will "ring the doorbell" (i.e. send an interrupt to the
- * GuC). The subsequent pages of the client object constitute the work
- * queue (a circular array of work items), again described in the process
- * descriptor. Work queue pages are mapped momentarily as required.
- */
-struct intel_guc_client {
- struct i915_vma *vma;
- void *vaddr;
- struct intel_guc *guc;
-
- /* bitmap of (host) engine ids */
- u32 priority;
- u32 stage_id;
-
- /* Protects GuC client's WQ access */
- spinlock_t wq_lock;
-};
+struct intel_guc;
+struct intel_engine_cs;
void intel_guc_submission_init_early(struct intel_guc *guc);
int intel_guc_submission_init(struct intel_guc *guc);
@@ -56,5 +18,6 @@ void intel_guc_submission_disable(struct intel_guc *guc);
void intel_guc_submission_fini(struct intel_guc *guc);
int intel_guc_preempt_work_create(struct intel_guc *guc);
void intel_guc_preempt_work_destroy(struct intel_guc *guc);
+bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine);
#endif
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5d5974e7f3ed..f32e7b016197 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1802,23 +1802,12 @@ static void i915_guc_log_info(struct seq_file *m,
static int i915_guc_info(struct seq_file *m, void *data)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
- const struct intel_guc *guc = &dev_priv->gt.uc.guc;
- struct intel_guc_client *client = guc->execbuf_client;
if (!USES_GUC(dev_priv))
return -ENODEV;
i915_guc_log_info(m, dev_priv);
- if (!USES_GUC_SUBMISSION(dev_priv))
- return 0;
-
- GEM_BUG_ON(!guc->execbuf_client);
-
- seq_printf(m, "\nGuC execbuf client @ %p:\n", client);
- seq_printf(m, "\tPriority %d, GuC stage index: %u\n",
- client->priority,
- client->stage_id);
/* Add more as required ... */
return 0;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [CI 5/5] HAX: force enable_guc=2
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
From: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_params.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 31b88f297fbc..acda9f2a1207 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -54,7 +54,7 @@ struct drm_printer;
param(int, disable_power_well, -1) \
param(int, enable_ips, 1) \
param(int, invert_brightness, 0) \
- param(int, enable_guc, 0) \
+ param(int, enable_guc, 2) \
param(int, guc_log_level, -1) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Intel-gfx] [CI 5/5] HAX: force enable_guc=2
@ 2019-11-18 23:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-18 23:55 UTC (permalink / raw)
To: intel-gfx
From: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_params.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 31b88f297fbc..acda9f2a1207 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -54,7 +54,7 @@ struct drm_printer;
param(int, disable_power_well, -1) \
param(int, enable_ips, 1) \
param(int, invert_brightness, 0) \
- param(int, enable_guc, 0) \
+ param(int, enable_guc, 2) \
param(int, guc_log_level, -1) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-19 0:14 ` Patchwork
0 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-11-19 0:14 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
URL : https://patchwork.freedesktop.org/series/69650/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
eb4028870663 drm/i915/guc: Drop leftover preemption code
194b7f455dd3 drm/i915/guc: add a helper to allocate and map guc vma
f8ada017ed27 drm/i915/guc: kill doorbell code and selftests
-:679: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#679:
deleted file mode 100644
total: 0 errors, 1 warnings, 0 checks, 615 lines checked
51032ff2cae5 drm/i915/guc: kill the GuC client
-:69: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment
#69: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc.h:52:
+ spinlock_t wq_lock;
total: 0 errors, 0 warnings, 1 checks, 564 lines checked
f3a61dfa95bc HAX: force enable_guc=2
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
total: 0 errors, 1 warnings, 0 checks, 8 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-19 0:14 ` Patchwork
0 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-11-19 0:14 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
URL : https://patchwork.freedesktop.org/series/69650/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
eb4028870663 drm/i915/guc: Drop leftover preemption code
194b7f455dd3 drm/i915/guc: add a helper to allocate and map guc vma
f8ada017ed27 drm/i915/guc: kill doorbell code and selftests
-:679: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#679:
deleted file mode 100644
total: 0 errors, 1 warnings, 0 checks, 615 lines checked
51032ff2cae5 drm/i915/guc: kill the GuC client
-:69: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment
#69: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc.h:52:
+ spinlock_t wq_lock;
total: 0 errors, 0 warnings, 1 checks, 564 lines checked
f3a61dfa95bc HAX: force enable_guc=2
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
total: 0 errors, 1 warnings, 0 checks, 8 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.DOCS: warning for series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-19 0:30 ` Patchwork
0 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-11-19 0:30 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
URL : https://patchwork.freedesktop.org/series/69650/
State : warning
== Summary ==
$ make htmldocs 2>&1 > /dev/null | grep i915
./drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:1: warning: no structured comments found
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-19 0:30 ` Patchwork
0 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-11-19 0:30 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
URL : https://patchwork.freedesktop.org/series/69650/
State : warning
== Summary ==
$ make htmldocs 2>&1 > /dev/null | grep i915
./drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:1: warning: no structured comments found
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-19 1:05 ` Patchwork
0 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-11-19 1:05 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
URL : https://patchwork.freedesktop.org/series/69650/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7367 -> Patchwork_15323
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_15323 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_15323, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_15323:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live_gt_pm:
- fi-icl-u2: [PASS][1] -> [DMESG-FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-u2/igt@i915_selftest@live_gt_pm.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-u2/igt@i915_selftest@live_gt_pm.html
- fi-icl-u3: [PASS][3] -> [DMESG-FAIL][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-u3/igt@i915_selftest@live_gt_pm.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-u3/igt@i915_selftest@live_gt_pm.html
- fi-icl-dsi: [PASS][5] -> [DMESG-FAIL][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-dsi/igt@i915_selftest@live_gt_pm.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-dsi/igt@i915_selftest@live_gt_pm.html
- fi-icl-y: [PASS][7] -> [DMESG-FAIL][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-y/igt@i915_selftest@live_gt_pm.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-y/igt@i915_selftest@live_gt_pm.html
Known issues
------------
Here are the changes found in Patchwork_15323 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_getparams_basic@basic-subslice-total:
- fi-icl-dsi: [PASS][9] -> [DMESG-WARN][10] ([fdo#106107])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-dsi/igt@i915_getparams_basic@basic-subslice-total.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-dsi/igt@i915_getparams_basic@basic-subslice-total.html
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: [PASS][11] -> [DMESG-FAIL][12] ([fdo#108511])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live_hangcheck:
- fi-hsw-4770r: [PASS][13] -> [DMESG-FAIL][14] ([fdo#111991])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
#### Possible fixes ####
* igt@i915_selftest@live_blt:
- fi-bsw-n3050: [DMESG-FAIL][15] ([fdo#112176]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-bsw-n3050/igt@i915_selftest@live_blt.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-bsw-n3050/igt@i915_selftest@live_blt.html
- fi-hsw-peppy: [DMESG-FAIL][17] ([fdo#112225]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-hsw-peppy/igt@i915_selftest@live_blt.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-hsw-peppy/igt@i915_selftest@live_blt.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
[fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
[fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
[fdo#111991]: https://bugs.freedesktop.org/show_bug.cgi?id=111991
[fdo#112176]: https://bugs.freedesktop.org/show_bug.cgi?id=112176
[fdo#112225]: https://bugs.freedesktop.org/show_bug.cgi?id=112225
[fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
Participating hosts (49 -> 44)
------------------------------
Missing (5): fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7367 -> Patchwork_15323
CI-20190529: 20190529
CI_DRM_7367: db6c0570abdebad7d56bf88eb15ad3b51dad4699 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5293: 4bb46f08f7cb6485642c4351cecdad93072d27a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15323: f3a61dfa95bc3b64e7836fc20b9afd31cf3e5eba @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
f3a61dfa95bc HAX: force enable_guc=2
51032ff2cae5 drm/i915/guc: kill the GuC client
f8ada017ed27 drm/i915/guc: kill doorbell code and selftests
194b7f455dd3 drm/i915/guc: add a helper to allocate and map guc vma
eb4028870663 drm/i915/guc: Drop leftover preemption code
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-19 1:05 ` Patchwork
0 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-11-19 1:05 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
URL : https://patchwork.freedesktop.org/series/69650/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7367 -> Patchwork_15323
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_15323 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_15323, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_15323:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live_gt_pm:
- fi-icl-u2: [PASS][1] -> [DMESG-FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-u2/igt@i915_selftest@live_gt_pm.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-u2/igt@i915_selftest@live_gt_pm.html
- fi-icl-u3: [PASS][3] -> [DMESG-FAIL][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-u3/igt@i915_selftest@live_gt_pm.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-u3/igt@i915_selftest@live_gt_pm.html
- fi-icl-dsi: [PASS][5] -> [DMESG-FAIL][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-dsi/igt@i915_selftest@live_gt_pm.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-dsi/igt@i915_selftest@live_gt_pm.html
- fi-icl-y: [PASS][7] -> [DMESG-FAIL][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-y/igt@i915_selftest@live_gt_pm.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-y/igt@i915_selftest@live_gt_pm.html
Known issues
------------
Here are the changes found in Patchwork_15323 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_getparams_basic@basic-subslice-total:
- fi-icl-dsi: [PASS][9] -> [DMESG-WARN][10] ([fdo#106107])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-dsi/igt@i915_getparams_basic@basic-subslice-total.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-dsi/igt@i915_getparams_basic@basic-subslice-total.html
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: [PASS][11] -> [DMESG-FAIL][12] ([fdo#108511])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live_hangcheck:
- fi-hsw-4770r: [PASS][13] -> [DMESG-FAIL][14] ([fdo#111991])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
#### Possible fixes ####
* igt@i915_selftest@live_blt:
- fi-bsw-n3050: [DMESG-FAIL][15] ([fdo#112176]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-bsw-n3050/igt@i915_selftest@live_blt.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-bsw-n3050/igt@i915_selftest@live_blt.html
- fi-hsw-peppy: [DMESG-FAIL][17] ([fdo#112225]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-hsw-peppy/igt@i915_selftest@live_blt.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-hsw-peppy/igt@i915_selftest@live_blt.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
[fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
[fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
[fdo#111991]: https://bugs.freedesktop.org/show_bug.cgi?id=111991
[fdo#112176]: https://bugs.freedesktop.org/show_bug.cgi?id=112176
[fdo#112225]: https://bugs.freedesktop.org/show_bug.cgi?id=112225
[fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
Participating hosts (49 -> 44)
------------------------------
Missing (5): fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7367 -> Patchwork_15323
CI-20190529: 20190529
CI_DRM_7367: db6c0570abdebad7d56bf88eb15ad3b51dad4699 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5293: 4bb46f08f7cb6485642c4351cecdad93072d27a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15323: f3a61dfa95bc3b64e7836fc20b9afd31cf3e5eba @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
f3a61dfa95bc HAX: force enable_guc=2
51032ff2cae5 drm/i915/guc: kill the GuC client
f8ada017ed27 drm/i915/guc: kill doorbell code and selftests
194b7f455dd3 drm/i915/guc: add a helper to allocate and map guc vma
eb4028870663 drm/i915/guc: Drop leftover preemption code
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: ✗ Fi.CI.BAT: failure for series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-19 1:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-19 1:55 UTC (permalink / raw)
To: intel-gfx
On 11/18/19 5:05 PM, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
> URL : https://patchwork.freedesktop.org/series/69650/
> State : failure
>
> == Summary ==
>
> CI Bug Log - changes from CI_DRM_7367 -> Patchwork_15323
> ====================================================
>
> Summary
> -------
>
> **FAILURE**
>
> Serious unknown changes coming with Patchwork_15323 absolutely need to be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in Patchwork_15323, please notify your bug team to allow them
> to document this new failure mode, which will reduce false positives in CI.
>
> External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/index.html
>
> Possible new issues
> -------------------
>
> Here are the unknown changes that may have been introduced in Patchwork_15323:
>
> ### IGT changes ###
>
> #### Possible regressions ####
>
> * igt@i915_selftest@live_gt_pm:
> - fi-icl-u2: [PASS][1] -> [DMESG-FAIL][2]
> [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-u2/igt@i915_selftest@live_gt_pm.html
> [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-u2/igt@i915_selftest@live_gt_pm.html
> - fi-icl-u3: [PASS][3] -> [DMESG-FAIL][4]
> [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-u3/igt@i915_selftest@live_gt_pm.html
> [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-u3/igt@i915_selftest@live_gt_pm.html
> - fi-icl-dsi: [PASS][5] -> [DMESG-FAIL][6]
> [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-dsi/igt@i915_selftest@live_gt_pm.html
> [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-dsi/igt@i915_selftest@live_gt_pm.html
> - fi-icl-y: [PASS][7] -> [DMESG-FAIL][8]
> [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-y/igt@i915_selftest@live_gt_pm.html
> [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-y/igt@i915_selftest@live_gt_pm.html
>
Those are all https://bugs.freedesktop.org/show_bug.cgi?id=112205
Daniele
>
> Known issues
> ------------
>
> Here are the changes found in Patchwork_15323 that come from known issues:
>
> ### IGT changes ###
>
> #### Issues hit ####
>
> * igt@i915_getparams_basic@basic-subslice-total:
> - fi-icl-dsi: [PASS][9] -> [DMESG-WARN][10] ([fdo#106107])
> [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-dsi/igt@i915_getparams_basic@basic-subslice-total.html
> [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-dsi/igt@i915_getparams_basic@basic-subslice-total.html
>
> * igt@i915_pm_rpm@module-reload:
> - fi-skl-6770hq: [PASS][11] -> [DMESG-FAIL][12] ([fdo#108511])
> [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
> [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
>
> * igt@i915_selftest@live_hangcheck:
> - fi-hsw-4770r: [PASS][13] -> [DMESG-FAIL][14] ([fdo#111991])
> [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
> [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
>
>
> #### Possible fixes ####
>
> * igt@i915_selftest@live_blt:
> - fi-bsw-n3050: [DMESG-FAIL][15] ([fdo#112176]) -> [PASS][16]
> [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-bsw-n3050/igt@i915_selftest@live_blt.html
> [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-bsw-n3050/igt@i915_selftest@live_blt.html
> - fi-hsw-peppy: [DMESG-FAIL][17] ([fdo#112225]) -> [PASS][18]
> [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-hsw-peppy/igt@i915_selftest@live_blt.html
> [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-hsw-peppy/igt@i915_selftest@live_blt.html
>
>
> {name}: This element is suppressed. This means it is ignored when computing
> the status of the difference (SUCCESS, WARNING, or FAILURE).
>
> [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
> [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
> [fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
> [fdo#111991]: https://bugs.freedesktop.org/show_bug.cgi?id=111991
> [fdo#112176]: https://bugs.freedesktop.org/show_bug.cgi?id=112176
> [fdo#112225]: https://bugs.freedesktop.org/show_bug.cgi?id=112225
> [fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
>
>
> Participating hosts (49 -> 44)
> ------------------------------
>
> Missing (5): fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
>
>
> Build changes
> -------------
>
> * CI: CI-20190529 -> None
> * Linux: CI_DRM_7367 -> Patchwork_15323
>
> CI-20190529: 20190529
> CI_DRM_7367: db6c0570abdebad7d56bf88eb15ad3b51dad4699 @ git://anongit.freedesktop.org/gfx-ci/linux
> IGT_5293: 4bb46f08f7cb6485642c4351cecdad93072d27a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
> Patchwork_15323: f3a61dfa95bc3b64e7836fc20b9afd31cf3e5eba @ git://anongit.freedesktop.org/gfx-ci/linux
>
>
> == Linux commits ==
>
> f3a61dfa95bc HAX: force enable_guc=2
> 51032ff2cae5 drm/i915/guc: kill the GuC client
> f8ada017ed27 drm/i915/guc: kill doorbell code and selftests
> 194b7f455dd3 drm/i915/guc: add a helper to allocate and map guc vma
> eb4028870663 drm/i915/guc: Drop leftover preemption code
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/index.html
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
@ 2019-11-19 1:55 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-19 1:55 UTC (permalink / raw)
To: intel-gfx
On 11/18/19 5:05 PM, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code
> URL : https://patchwork.freedesktop.org/series/69650/
> State : failure
>
> == Summary ==
>
> CI Bug Log - changes from CI_DRM_7367 -> Patchwork_15323
> ====================================================
>
> Summary
> -------
>
> **FAILURE**
>
> Serious unknown changes coming with Patchwork_15323 absolutely need to be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in Patchwork_15323, please notify your bug team to allow them
> to document this new failure mode, which will reduce false positives in CI.
>
> External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/index.html
>
> Possible new issues
> -------------------
>
> Here are the unknown changes that may have been introduced in Patchwork_15323:
>
> ### IGT changes ###
>
> #### Possible regressions ####
>
> * igt@i915_selftest@live_gt_pm:
> - fi-icl-u2: [PASS][1] -> [DMESG-FAIL][2]
> [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-u2/igt@i915_selftest@live_gt_pm.html
> [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-u2/igt@i915_selftest@live_gt_pm.html
> - fi-icl-u3: [PASS][3] -> [DMESG-FAIL][4]
> [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-u3/igt@i915_selftest@live_gt_pm.html
> [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-u3/igt@i915_selftest@live_gt_pm.html
> - fi-icl-dsi: [PASS][5] -> [DMESG-FAIL][6]
> [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-dsi/igt@i915_selftest@live_gt_pm.html
> [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-dsi/igt@i915_selftest@live_gt_pm.html
> - fi-icl-y: [PASS][7] -> [DMESG-FAIL][8]
> [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-y/igt@i915_selftest@live_gt_pm.html
> [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-y/igt@i915_selftest@live_gt_pm.html
>
Those are all https://bugs.freedesktop.org/show_bug.cgi?id=112205
Daniele
>
> Known issues
> ------------
>
> Here are the changes found in Patchwork_15323 that come from known issues:
>
> ### IGT changes ###
>
> #### Issues hit ####
>
> * igt@i915_getparams_basic@basic-subslice-total:
> - fi-icl-dsi: [PASS][9] -> [DMESG-WARN][10] ([fdo#106107])
> [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-icl-dsi/igt@i915_getparams_basic@basic-subslice-total.html
> [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-icl-dsi/igt@i915_getparams_basic@basic-subslice-total.html
>
> * igt@i915_pm_rpm@module-reload:
> - fi-skl-6770hq: [PASS][11] -> [DMESG-FAIL][12] ([fdo#108511])
> [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
> [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
>
> * igt@i915_selftest@live_hangcheck:
> - fi-hsw-4770r: [PASS][13] -> [DMESG-FAIL][14] ([fdo#111991])
> [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
> [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
>
>
> #### Possible fixes ####
>
> * igt@i915_selftest@live_blt:
> - fi-bsw-n3050: [DMESG-FAIL][15] ([fdo#112176]) -> [PASS][16]
> [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-bsw-n3050/igt@i915_selftest@live_blt.html
> [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-bsw-n3050/igt@i915_selftest@live_blt.html
> - fi-hsw-peppy: [DMESG-FAIL][17] ([fdo#112225]) -> [PASS][18]
> [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7367/fi-hsw-peppy/igt@i915_selftest@live_blt.html
> [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/fi-hsw-peppy/igt@i915_selftest@live_blt.html
>
>
> {name}: This element is suppressed. This means it is ignored when computing
> the status of the difference (SUCCESS, WARNING, or FAILURE).
>
> [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
> [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
> [fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
> [fdo#111991]: https://bugs.freedesktop.org/show_bug.cgi?id=111991
> [fdo#112176]: https://bugs.freedesktop.org/show_bug.cgi?id=112176
> [fdo#112225]: https://bugs.freedesktop.org/show_bug.cgi?id=112225
> [fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
>
>
> Participating hosts (49 -> 44)
> ------------------------------
>
> Missing (5): fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
>
>
> Build changes
> -------------
>
> * CI: CI-20190529 -> None
> * Linux: CI_DRM_7367 -> Patchwork_15323
>
> CI-20190529: 20190529
> CI_DRM_7367: db6c0570abdebad7d56bf88eb15ad3b51dad4699 @ git://anongit.freedesktop.org/gfx-ci/linux
> IGT_5293: 4bb46f08f7cb6485642c4351cecdad93072d27a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
> Patchwork_15323: f3a61dfa95bc3b64e7836fc20b9afd31cf3e5eba @ git://anongit.freedesktop.org/gfx-ci/linux
>
>
> == Linux commits ==
>
> f3a61dfa95bc HAX: force enable_guc=2
> 51032ff2cae5 drm/i915/guc: kill the GuC client
> f8ada017ed27 drm/i915/guc: kill doorbell code and selftests
> 194b7f455dd3 drm/i915/guc: add a helper to allocate and map guc vma
> eb4028870663 drm/i915/guc: Drop leftover preemption code
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15323/index.html
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [CI 4/5] drm/i915/guc: kill the GuC client
@ 2019-11-20 0:13 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-20 0:13 UTC (permalink / raw)
To: intel-gfx
<snip>
> @@ -220,7 +200,7 @@ static void guc_wq_item_append(struct intel_guc_client *client,
> GEM_BUG_ON(wq_off & (wqi_size - 1));
>
> /* WQ starts from the page after process_desc */
Just realized that I stupidly forgot to actually commit the fix...
I'll send the fixed (for real) version after the bugfix for bug 112205
is merged, so I can get a clean CI run.
Daniele
> - wqi = client->vaddr + wq_off + GUC_PD_SIZE;
> + wqi = guc->workqueue_vaddr + wq_off;
>
> /* Now fill in the 4-word work queue item */
> wqi->header = WQ_TYPE_INORDER |
> @@ -238,12 +218,11 @@ static void guc_wq_item_append(struct intel_guc_client *client,
>
> static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
> {
> - struct intel_guc_client *client = guc->execbuf_client;
> struct intel_engine_cs *engine = rq->engine;
> u32 ctx_desc = lower_32_bits(rq->hw_context->lrc_desc);
> u32 ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
>
> - guc_wq_item_append(client, engine->guc_id, ctx_desc,
> + guc_wq_item_append(guc, engine->guc_id, ctx_desc,
> ring_tail, rq->fence.seqno);
> }
>
> @@ -267,9 +246,8 @@ static void guc_submit(struct intel_engine_cs *engine,
> struct i915_request **end)
> {
> struct intel_guc *guc = &engine->gt->uc.guc;
> - struct intel_guc_client *client = guc->execbuf_client;
>
> - spin_lock(&client->wq_lock);
> + spin_lock(&guc->wq_lock);
>
> do {
> struct i915_request *rq = *out++;
> @@ -278,7 +256,7 @@ static void guc_submit(struct intel_engine_cs *engine,
> guc_add_request(guc, rq);
> } while (out != end);
>
> - spin_unlock(&client->wq_lock);
> + spin_unlock(&guc->wq_lock);
> }
>
> static inline int rq_prio(const struct i915_request *rq)
> @@ -529,126 +507,6 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
> * path of guc_submit() above.
> */
>
> -/**
> - * guc_client_alloc() - Allocate an intel_guc_client
> - * @guc: the intel_guc structure
> - * @priority: four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
> - * The kernel client to replace ExecList submission is created with
> - * NORMAL priority. Priority of a client for scheduler can be HIGH,
> - * while a preemption context can use CRITICAL.
> - *
> - * Return: An intel_guc_client object if success, else NULL.
> - */
> -static struct intel_guc_client *
> -guc_client_alloc(struct intel_guc *guc, u32 priority)
> -{
> - struct intel_guc_client *client;
> - struct i915_vma *vma;
> - void *vaddr;
> - int ret;
> -
> - client = kzalloc(sizeof(*client), GFP_KERNEL);
> - if (!client)
> - return ERR_PTR(-ENOMEM);
> -
> - client->guc = guc;
> - client->priority = priority;
> - spin_lock_init(&client->wq_lock);
> -
> - ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
> - GFP_KERNEL);
> - if (ret < 0)
> - goto err_client;
> -
> - client->stage_id = ret;
> -
> - /* The first page is proc_desc. Two following pages are wq. */
> - vma = intel_guc_allocate_vma(guc, GUC_PD_SIZE + GUC_WQ_SIZE);
> - if (IS_ERR(vma)) {
> - ret = PTR_ERR(vma);
> - goto err_id;
> - }
> -
> - /* We'll keep just the first (doorbell/proc) page permanently kmap'd. */
> - client->vma = vma;
> -
> - vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
> - if (IS_ERR(vaddr)) {
> - ret = PTR_ERR(vaddr);
> - goto err_vma;
> - }
> - client->vaddr = vaddr;
> -
> - DRM_DEBUG_DRIVER("new priority %u client %p: stage_id %u\n",
> - priority, client, client->stage_id);
> -
> - return client;
> -
> -err_vma:
> - i915_vma_unpin_and_release(&client->vma, 0);
> -err_id:
> - ida_simple_remove(&guc->stage_ids, client->stage_id);
> -err_client:
> - kfree(client);
> - return ERR_PTR(ret);
> -}
> -
> -static void guc_client_free(struct intel_guc_client *client)
> -{
> - i915_vma_unpin_and_release(&client->vma, I915_VMA_RELEASE_MAP);
> - ida_simple_remove(&client->guc->stage_ids, client->stage_id);
> - kfree(client);
> -}
> -
> -static int guc_clients_create(struct intel_guc *guc)
> -{
> - struct intel_guc_client *client;
> -
> - GEM_BUG_ON(guc->execbuf_client);
> -
> - client = guc_client_alloc(guc, GUC_CLIENT_PRIORITY_KMD_NORMAL);
> - if (IS_ERR(client)) {
> - DRM_ERROR("Failed to create GuC client for submission!\n");
> - return PTR_ERR(client);
> - }
> - guc->execbuf_client = client;
> -
> - return 0;
> -}
> -
> -static void guc_clients_destroy(struct intel_guc *guc)
> -{
> - struct intel_guc_client *client;
> -
> - client = fetch_and_zero(&guc->execbuf_client);
> - if (client)
> - guc_client_free(client);
> -}
> -
> -static void __guc_client_enable(struct intel_guc_client *client)
> -{
> - guc_proc_desc_init(client);
> - guc_stage_desc_init(client);
> -}
> -
> -static void __guc_client_disable(struct intel_guc_client *client)
> -{
> - /* Note: By the time we're here, GuC may have already been reset */
> - guc_stage_desc_fini(client);
> - guc_proc_desc_fini(client);
> -}
> -
> -static void guc_clients_enable(struct intel_guc *guc)
> -{
> - __guc_client_enable(guc->execbuf_client);
> -}
> -
> -static void guc_clients_disable(struct intel_guc *guc)
> -{
> - if (guc->execbuf_client)
> - __guc_client_disable(guc->execbuf_client);
> -}
> -
> /*
> * Set up the memory resources to be shared with the GuC (via the GGTT)
> * at firmware loading time.
> @@ -669,12 +527,20 @@ int intel_guc_submission_init(struct intel_guc *guc)
> */
> GEM_BUG_ON(!guc->stage_desc_pool);
>
> - ret = guc_clients_create(guc);
> + ret = guc_workqueue_create(guc);
> if (ret)
> goto err_pool;
>
> + ret = guc_proc_desc_create(guc);
> + if (ret)
> + goto err_workqueue;
> +
> + spin_lock_init(&guc->wq_lock);
> +
> return 0;
>
> +err_workqueue:
> + guc_workqueue_destroy(guc);
> err_pool:
> guc_stage_desc_pool_destroy(guc);
> return ret;
> @@ -682,10 +548,11 @@ int intel_guc_submission_init(struct intel_guc *guc)
>
> void intel_guc_submission_fini(struct intel_guc *guc)
> {
> - guc_clients_destroy(guc);
> -
> - if (guc->stage_desc_pool)
> + if (guc->stage_desc_pool) {
> + guc_proc_desc_destroy(guc);
> + guc_workqueue_destroy(guc);
> guc_stage_desc_pool_destroy(guc);
> + }
> }
>
> static void guc_interrupts_capture(struct intel_gt *gt)
> @@ -771,9 +638,8 @@ void intel_guc_submission_enable(struct intel_guc *guc)
> sizeof(struct guc_wq_item) *
> I915_NUM_ENGINES > GUC_WQ_SIZE);
>
> - GEM_BUG_ON(!guc->execbuf_client);
> -
> - guc_clients_enable(guc);
> + guc_proc_desc_init(guc);
> + guc_stage_desc_init(guc);
>
> /* Take over from manual control of ELSP (execlists) */
> guc_interrupts_capture(gt);
> @@ -790,8 +656,12 @@ void intel_guc_submission_disable(struct intel_guc *guc)
>
> GEM_BUG_ON(gt->awake); /* GT should be parked first */
>
> + /* Note: By the time we're here, GuC may have already been reset */
> +
> guc_interrupts_release(gt);
> - guc_clients_disable(guc);
> +
> + guc_stage_desc_fini(guc);
> + guc_proc_desc_fini(guc);
> }
>
> static bool __guc_submission_support(struct intel_guc *guc)
> @@ -809,3 +679,8 @@ void intel_guc_submission_init_early(struct intel_guc *guc)
> {
> guc->submission_supported = __guc_submission_support(guc);
> }
> +
> +bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine)
> +{
> + return engine->set_default_submission == guc_set_default_submission;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> index e2deb4e6f42a..e402a2932592 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> @@ -6,48 +6,10 @@
> #ifndef _INTEL_GUC_SUBMISSION_H_
> #define _INTEL_GUC_SUBMISSION_H_
>
> -#include <linux/spinlock.h>
> +#include <linux/types.h>
>
> -#include "gt/intel_engine_types.h"
> -
> -#include "i915_gem.h"
> -#include "i915_selftest.h"
> -
> -struct drm_i915_private;
> -
> -/*
> - * This structure primarily describes the GEM object shared with the GuC.
> - * The specs sometimes refer to this object as a "GuC context", but we use
> - * the term "client" to avoid confusion with hardware contexts. This
> - * GEM object is held for the entire lifetime of our interaction with
> - * the GuC, being allocated before the GuC is loaded with its firmware.
> - * Because there's no way to update the address used by the GuC after
> - * initialisation, the shared object must stay pinned into the GGTT as
> - * long as the GuC is in use. We also keep the first page (only) mapped
> - * into kernel address space, as it includes shared data that must be
> - * updated on every request submission.
> - *
> - * The single GEM object described here is actually made up of several
> - * separate areas, as far as the GuC is concerned. The first page (kept
> - * kmap'd) includes the "process descriptor" which holds sequence data for
> - * the doorbell, and one cacheline which actually *is* the doorbell; a
> - * write to this will "ring the doorbell" (i.e. send an interrupt to the
> - * GuC). The subsequent pages of the client object constitute the work
> - * queue (a circular array of work items), again described in the process
> - * descriptor. Work queue pages are mapped momentarily as required.
> - */
> -struct intel_guc_client {
> - struct i915_vma *vma;
> - void *vaddr;
> - struct intel_guc *guc;
> -
> - /* bitmap of (host) engine ids */
> - u32 priority;
> - u32 stage_id;
> -
> - /* Protects GuC client's WQ access */
> - spinlock_t wq_lock;
> -};
> +struct intel_guc;
> +struct intel_engine_cs;
>
> void intel_guc_submission_init_early(struct intel_guc *guc);
> int intel_guc_submission_init(struct intel_guc *guc);
> @@ -56,5 +18,6 @@ void intel_guc_submission_disable(struct intel_guc *guc);
> void intel_guc_submission_fini(struct intel_guc *guc);
> int intel_guc_preempt_work_create(struct intel_guc *guc);
> void intel_guc_preempt_work_destroy(struct intel_guc *guc);
> +bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5d5974e7f3ed..f32e7b016197 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1802,23 +1802,12 @@ static void i915_guc_log_info(struct seq_file *m,
> static int i915_guc_info(struct seq_file *m, void *data)
> {
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> - const struct intel_guc *guc = &dev_priv->gt.uc.guc;
> - struct intel_guc_client *client = guc->execbuf_client;
>
> if (!USES_GUC(dev_priv))
> return -ENODEV;
>
> i915_guc_log_info(m, dev_priv);
>
> - if (!USES_GUC_SUBMISSION(dev_priv))
> - return 0;
> -
> - GEM_BUG_ON(!guc->execbuf_client);
> -
> - seq_printf(m, "\nGuC execbuf client @ %p:\n", client);
> - seq_printf(m, "\tPriority %d, GuC stage index: %u\n",
> - client->priority,
> - client->stage_id);
> /* Add more as required ... */
>
> return 0;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [CI 4/5] drm/i915/guc: kill the GuC client
@ 2019-11-20 0:13 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-20 0:13 UTC (permalink / raw)
To: intel-gfx
<snip>
> @@ -220,7 +200,7 @@ static void guc_wq_item_append(struct intel_guc_client *client,
> GEM_BUG_ON(wq_off & (wqi_size - 1));
>
> /* WQ starts from the page after process_desc */
Just realized that I stupidly forgot to actually commit the fix...
I'll send the fixed (for real) version after the bugfix for bug 112205
is merged, so I can get a clean CI run.
Daniele
> - wqi = client->vaddr + wq_off + GUC_PD_SIZE;
> + wqi = guc->workqueue_vaddr + wq_off;
>
> /* Now fill in the 4-word work queue item */
> wqi->header = WQ_TYPE_INORDER |
> @@ -238,12 +218,11 @@ static void guc_wq_item_append(struct intel_guc_client *client,
>
> static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
> {
> - struct intel_guc_client *client = guc->execbuf_client;
> struct intel_engine_cs *engine = rq->engine;
> u32 ctx_desc = lower_32_bits(rq->hw_context->lrc_desc);
> u32 ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
>
> - guc_wq_item_append(client, engine->guc_id, ctx_desc,
> + guc_wq_item_append(guc, engine->guc_id, ctx_desc,
> ring_tail, rq->fence.seqno);
> }
>
> @@ -267,9 +246,8 @@ static void guc_submit(struct intel_engine_cs *engine,
> struct i915_request **end)
> {
> struct intel_guc *guc = &engine->gt->uc.guc;
> - struct intel_guc_client *client = guc->execbuf_client;
>
> - spin_lock(&client->wq_lock);
> + spin_lock(&guc->wq_lock);
>
> do {
> struct i915_request *rq = *out++;
> @@ -278,7 +256,7 @@ static void guc_submit(struct intel_engine_cs *engine,
> guc_add_request(guc, rq);
> } while (out != end);
>
> - spin_unlock(&client->wq_lock);
> + spin_unlock(&guc->wq_lock);
> }
>
> static inline int rq_prio(const struct i915_request *rq)
> @@ -529,126 +507,6 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
> * path of guc_submit() above.
> */
>
> -/**
> - * guc_client_alloc() - Allocate an intel_guc_client
> - * @guc: the intel_guc structure
> - * @priority: four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
> - * The kernel client to replace ExecList submission is created with
> - * NORMAL priority. Priority of a client for scheduler can be HIGH,
> - * while a preemption context can use CRITICAL.
> - *
> - * Return: An intel_guc_client object if success, else NULL.
> - */
> -static struct intel_guc_client *
> -guc_client_alloc(struct intel_guc *guc, u32 priority)
> -{
> - struct intel_guc_client *client;
> - struct i915_vma *vma;
> - void *vaddr;
> - int ret;
> -
> - client = kzalloc(sizeof(*client), GFP_KERNEL);
> - if (!client)
> - return ERR_PTR(-ENOMEM);
> -
> - client->guc = guc;
> - client->priority = priority;
> - spin_lock_init(&client->wq_lock);
> -
> - ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
> - GFP_KERNEL);
> - if (ret < 0)
> - goto err_client;
> -
> - client->stage_id = ret;
> -
> - /* The first page is proc_desc. Two following pages are wq. */
> - vma = intel_guc_allocate_vma(guc, GUC_PD_SIZE + GUC_WQ_SIZE);
> - if (IS_ERR(vma)) {
> - ret = PTR_ERR(vma);
> - goto err_id;
> - }
> -
> - /* We'll keep just the first (doorbell/proc) page permanently kmap'd. */
> - client->vma = vma;
> -
> - vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
> - if (IS_ERR(vaddr)) {
> - ret = PTR_ERR(vaddr);
> - goto err_vma;
> - }
> - client->vaddr = vaddr;
> -
> - DRM_DEBUG_DRIVER("new priority %u client %p: stage_id %u\n",
> - priority, client, client->stage_id);
> -
> - return client;
> -
> -err_vma:
> - i915_vma_unpin_and_release(&client->vma, 0);
> -err_id:
> - ida_simple_remove(&guc->stage_ids, client->stage_id);
> -err_client:
> - kfree(client);
> - return ERR_PTR(ret);
> -}
> -
> -static void guc_client_free(struct intel_guc_client *client)
> -{
> - i915_vma_unpin_and_release(&client->vma, I915_VMA_RELEASE_MAP);
> - ida_simple_remove(&client->guc->stage_ids, client->stage_id);
> - kfree(client);
> -}
> -
> -static int guc_clients_create(struct intel_guc *guc)
> -{
> - struct intel_guc_client *client;
> -
> - GEM_BUG_ON(guc->execbuf_client);
> -
> - client = guc_client_alloc(guc, GUC_CLIENT_PRIORITY_KMD_NORMAL);
> - if (IS_ERR(client)) {
> - DRM_ERROR("Failed to create GuC client for submission!\n");
> - return PTR_ERR(client);
> - }
> - guc->execbuf_client = client;
> -
> - return 0;
> -}
> -
> -static void guc_clients_destroy(struct intel_guc *guc)
> -{
> - struct intel_guc_client *client;
> -
> - client = fetch_and_zero(&guc->execbuf_client);
> - if (client)
> - guc_client_free(client);
> -}
> -
> -static void __guc_client_enable(struct intel_guc_client *client)
> -{
> - guc_proc_desc_init(client);
> - guc_stage_desc_init(client);
> -}
> -
> -static void __guc_client_disable(struct intel_guc_client *client)
> -{
> - /* Note: By the time we're here, GuC may have already been reset */
> - guc_stage_desc_fini(client);
> - guc_proc_desc_fini(client);
> -}
> -
> -static void guc_clients_enable(struct intel_guc *guc)
> -{
> - __guc_client_enable(guc->execbuf_client);
> -}
> -
> -static void guc_clients_disable(struct intel_guc *guc)
> -{
> - if (guc->execbuf_client)
> - __guc_client_disable(guc->execbuf_client);
> -}
> -
> /*
> * Set up the memory resources to be shared with the GuC (via the GGTT)
> * at firmware loading time.
> @@ -669,12 +527,20 @@ int intel_guc_submission_init(struct intel_guc *guc)
> */
> GEM_BUG_ON(!guc->stage_desc_pool);
>
> - ret = guc_clients_create(guc);
> + ret = guc_workqueue_create(guc);
> if (ret)
> goto err_pool;
>
> + ret = guc_proc_desc_create(guc);
> + if (ret)
> + goto err_workqueue;
> +
> + spin_lock_init(&guc->wq_lock);
> +
> return 0;
>
> +err_workqueue:
> + guc_workqueue_destroy(guc);
> err_pool:
> guc_stage_desc_pool_destroy(guc);
> return ret;
> @@ -682,10 +548,11 @@ int intel_guc_submission_init(struct intel_guc *guc)
>
> void intel_guc_submission_fini(struct intel_guc *guc)
> {
> - guc_clients_destroy(guc);
> -
> - if (guc->stage_desc_pool)
> + if (guc->stage_desc_pool) {
> + guc_proc_desc_destroy(guc);
> + guc_workqueue_destroy(guc);
> guc_stage_desc_pool_destroy(guc);
> + }
> }
>
> static void guc_interrupts_capture(struct intel_gt *gt)
> @@ -771,9 +638,8 @@ void intel_guc_submission_enable(struct intel_guc *guc)
> sizeof(struct guc_wq_item) *
> I915_NUM_ENGINES > GUC_WQ_SIZE);
>
> - GEM_BUG_ON(!guc->execbuf_client);
> -
> - guc_clients_enable(guc);
> + guc_proc_desc_init(guc);
> + guc_stage_desc_init(guc);
>
> /* Take over from manual control of ELSP (execlists) */
> guc_interrupts_capture(gt);
> @@ -790,8 +656,12 @@ void intel_guc_submission_disable(struct intel_guc *guc)
>
> GEM_BUG_ON(gt->awake); /* GT should be parked first */
>
> + /* Note: By the time we're here, GuC may have already been reset */
> +
> guc_interrupts_release(gt);
> - guc_clients_disable(guc);
> +
> + guc_stage_desc_fini(guc);
> + guc_proc_desc_fini(guc);
> }
>
> static bool __guc_submission_support(struct intel_guc *guc)
> @@ -809,3 +679,8 @@ void intel_guc_submission_init_early(struct intel_guc *guc)
> {
> guc->submission_supported = __guc_submission_support(guc);
> }
> +
> +bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine)
> +{
> + return engine->set_default_submission == guc_set_default_submission;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> index e2deb4e6f42a..e402a2932592 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> @@ -6,48 +6,10 @@
> #ifndef _INTEL_GUC_SUBMISSION_H_
> #define _INTEL_GUC_SUBMISSION_H_
>
> -#include <linux/spinlock.h>
> +#include <linux/types.h>
>
> -#include "gt/intel_engine_types.h"
> -
> -#include "i915_gem.h"
> -#include "i915_selftest.h"
> -
> -struct drm_i915_private;
> -
> -/*
> - * This structure primarily describes the GEM object shared with the GuC.
> - * The specs sometimes refer to this object as a "GuC context", but we use
> - * the term "client" to avoid confusion with hardware contexts. This
> - * GEM object is held for the entire lifetime of our interaction with
> - * the GuC, being allocated before the GuC is loaded with its firmware.
> - * Because there's no way to update the address used by the GuC after
> - * initialisation, the shared object must stay pinned into the GGTT as
> - * long as the GuC is in use. We also keep the first page (only) mapped
> - * into kernel address space, as it includes shared data that must be
> - * updated on every request submission.
> - *
> - * The single GEM object described here is actually made up of several
> - * separate areas, as far as the GuC is concerned. The first page (kept
> - * kmap'd) includes the "process descriptor" which holds sequence data for
> - * the doorbell, and one cacheline which actually *is* the doorbell; a
> - * write to this will "ring the doorbell" (i.e. send an interrupt to the
> - * GuC). The subsequent pages of the client object constitute the work
> - * queue (a circular array of work items), again described in the process
> - * descriptor. Work queue pages are mapped momentarily as required.
> - */
> -struct intel_guc_client {
> - struct i915_vma *vma;
> - void *vaddr;
> - struct intel_guc *guc;
> -
> - /* bitmap of (host) engine ids */
> - u32 priority;
> - u32 stage_id;
> -
> - /* Protects GuC client's WQ access */
> - spinlock_t wq_lock;
> -};
> +struct intel_guc;
> +struct intel_engine_cs;
>
> void intel_guc_submission_init_early(struct intel_guc *guc);
> int intel_guc_submission_init(struct intel_guc *guc);
> @@ -56,5 +18,6 @@ void intel_guc_submission_disable(struct intel_guc *guc);
> void intel_guc_submission_fini(struct intel_guc *guc);
> int intel_guc_preempt_work_create(struct intel_guc *guc);
> void intel_guc_preempt_work_destroy(struct intel_guc *guc);
> +bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5d5974e7f3ed..f32e7b016197 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1802,23 +1802,12 @@ static void i915_guc_log_info(struct seq_file *m,
> static int i915_guc_info(struct seq_file *m, void *data)
> {
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> - const struct intel_guc *guc = &dev_priv->gt.uc.guc;
> - struct intel_guc_client *client = guc->execbuf_client;
>
> if (!USES_GUC(dev_priv))
> return -ENODEV;
>
> i915_guc_log_info(m, dev_priv);
>
> - if (!USES_GUC_SUBMISSION(dev_priv))
> - return 0;
> -
> - GEM_BUG_ON(!guc->execbuf_client);
> -
> - seq_printf(m, "\nGuC execbuf client @ %p:\n", client);
> - seq_printf(m, "\tPriority %d, GuC stage index: %u\n",
> - client->priority,
> - client->stage_id);
> /* Add more as required ... */
>
> return 0;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-11-20 0:13 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-18 23:55 [CI 1/5] drm/i915/guc: Drop leftover preemption code Daniele Ceraolo Spurio
2019-11-18 23:55 ` [Intel-gfx] " Daniele Ceraolo Spurio
2019-11-18 23:55 ` [CI 2/5] drm/i915/guc: add a helper to allocate and map guc vma Daniele Ceraolo Spurio
2019-11-18 23:55 ` [Intel-gfx] " Daniele Ceraolo Spurio
2019-11-18 23:55 ` [CI 3/5] drm/i915/guc: kill doorbell code and selftests Daniele Ceraolo Spurio
2019-11-18 23:55 ` [Intel-gfx] " Daniele Ceraolo Spurio
2019-11-18 23:55 ` [CI 4/5] drm/i915/guc: kill the GuC client Daniele Ceraolo Spurio
2019-11-18 23:55 ` [Intel-gfx] " Daniele Ceraolo Spurio
2019-11-20 0:13 ` Daniele Ceraolo Spurio
2019-11-20 0:13 ` [Intel-gfx] " Daniele Ceraolo Spurio
2019-11-18 23:55 ` [CI 5/5] HAX: force enable_guc=2 Daniele Ceraolo Spurio
2019-11-18 23:55 ` [Intel-gfx] " Daniele Ceraolo Spurio
2019-11-19 0:14 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/5] drm/i915/guc: Drop leftover preemption code Patchwork
2019-11-19 0:14 ` [Intel-gfx] " Patchwork
2019-11-19 0:30 ` ✗ Fi.CI.DOCS: " Patchwork
2019-11-19 0:30 ` [Intel-gfx] " Patchwork
2019-11-19 1:05 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-11-19 1:05 ` [Intel-gfx] " Patchwork
2019-11-19 1:55 ` Daniele Ceraolo Spurio
2019-11-19 1:55 ` [Intel-gfx] " Daniele Ceraolo Spurio
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.