* [PATCH 2/5] drm/i915/guc: refactor guc_init_doorbell_hw()
2016-07-19 12:59 [PATCH 1/5] drm/i915/guc: doorbell reset should avoid used doorbells Dave Gordon
@ 2016-07-19 12:59 ` Dave Gordon
2016-07-19 14:51 ` Tvrtko Ursulin
2016-07-19 12:59 ` [PATCH 3/5] drm/i915/guc: use a separate GuC client for each engine Dave Gordon
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Dave Gordon @ 2016-07-19 12:59 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 54 +++++++++++++++++-------------
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index d8402e4..dc5f485 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -698,32 +698,47 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
kfree(client);
}
+/* Check that a doorbell register is in the expected state */
+static bool guc_doorbell_check(struct intel_guc *guc, uint16_t db_id)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ i915_reg_t drbreg = GEN8_DRBREGL(db_id);
+ uint32_t value = I915_READ(drbreg);
+ bool enabled = (value & GUC_DOORBELL_ENABLED) != 0;
+ bool expected = test_bit(db_id, guc->doorbell_bitmap);
+
+ if (enabled == expected)
+ return true;
+
+ DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) 0x%x, should be %s\n",
+ db_id, drbreg.reg, value,
+ expected ? "active" : "inactive");
+
+ return false;
+}
+
/*
* Borrow the first client to set up & tear down each unused doorbell
* in turn, to ensure that all doorbell h/w is (re)initialised.
*/
static void guc_init_doorbell_hw(struct intel_guc *guc)
{
- struct drm_i915_private *dev_priv = guc_to_i915(guc);
struct i915_guc_client *client = guc->execbuf_client;
- uint16_t db_id, i;
- int err;
+ uint16_t db_id;
+ int i, err;
+ /* Save client's original doorbell selection */
db_id = client->doorbell_id;
for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
- i915_reg_t drbreg = GEN8_DRBREGL(i);
- u32 value = I915_READ(drbreg);
-
- if (test_bit(i, guc->doorbell_bitmap))
+ /* Skip if doorbell is OK */
+ if (guc_doorbell_check(guc, i))
continue;
err = guc_update_doorbell_id(guc, client, i);
-
- /* Report update failure or unexpectedly active doorbell */
- if (err || (i != db_id && (value & GUC_DOORBELL_ENABLED)))
- DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) was 0x%x, err %d\n",
- i, drbreg.reg, value, err);
+ if (err)
+ DRM_DEBUG_DRIVER("Doorbell %d update failed, err %d\n",
+ i, err);
}
/* Restore to original value */
@@ -732,18 +747,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
DRM_ERROR("Failed to restore doorbell to %d, err %d\n",
db_id, err);
- for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
- i915_reg_t drbreg = GEN8_DRBREGL(i);
- u32 value = I915_READ(drbreg);
-
- if (test_bit(i, guc->doorbell_bitmap))
- continue;
-
- if (i != db_id && (value & GUC_DOORBELL_ENABLED))
- DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) finally 0x%x\n",
- i, drbreg.reg, value);
-
- }
+ /* Read back & verify all doorbell registers */
+ for (i = 0; i < GUC_MAX_DOORBELLS; ++i)
+ (void)guc_doorbell_check(guc, i);
}
/**
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/5] drm/i915/guc: refactor guc_init_doorbell_hw()
2016-07-19 12:59 ` [PATCH 2/5] drm/i915/guc: refactor guc_init_doorbell_hw() Dave Gordon
@ 2016-07-19 14:51 ` Tvrtko Ursulin
0 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2016-07-19 14:51 UTC (permalink / raw)
To: Dave Gordon, intel-gfx
On 19/07/16 13:59, Dave Gordon wrote:
Commit message missing! :)
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 54 +++++++++++++++++-------------
> 1 file changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index d8402e4..dc5f485 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -698,32 +698,47 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
> kfree(client);
> }
>
> +/* Check that a doorbell register is in the expected state */
> +static bool guc_doorbell_check(struct intel_guc *guc, uint16_t db_id)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + i915_reg_t drbreg = GEN8_DRBREGL(db_id);
> + uint32_t value = I915_READ(drbreg);
> + bool enabled = (value & GUC_DOORBELL_ENABLED) != 0;
> + bool expected = test_bit(db_id, guc->doorbell_bitmap);
> +
> + if (enabled == expected)
> + return true;
> +
> + DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) 0x%x, should be %s\n",
> + db_id, drbreg.reg, value,
> + expected ? "active" : "inactive");
> +
> + return false;
> +}
> +
> /*
> * Borrow the first client to set up & tear down each unused doorbell
> * in turn, to ensure that all doorbell h/w is (re)initialised.
> */
> static void guc_init_doorbell_hw(struct intel_guc *guc)
> {
> - struct drm_i915_private *dev_priv = guc_to_i915(guc);
> struct i915_guc_client *client = guc->execbuf_client;
> - uint16_t db_id, i;
> - int err;
> + uint16_t db_id;
> + int i, err;
>
> + /* Save client's original doorbell selection */
> db_id = client->doorbell_id;
>
> for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
> - i915_reg_t drbreg = GEN8_DRBREGL(i);
> - u32 value = I915_READ(drbreg);
> -
> - if (test_bit(i, guc->doorbell_bitmap))
> + /* Skip if doorbell is OK */
> + if (guc_doorbell_check(guc, i))
> continue;
>
> err = guc_update_doorbell_id(guc, client, i);
> -
> - /* Report update failure or unexpectedly active doorbell */
> - if (err || (i != db_id && (value & GUC_DOORBELL_ENABLED)))
> - DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) was 0x%x, err %d\n",
> - i, drbreg.reg, value, err);
> + if (err)
> + DRM_DEBUG_DRIVER("Doorbell %d update failed, err %d\n",
> + i, err);
> }
>
> /* Restore to original value */
> @@ -732,18 +747,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
> DRM_ERROR("Failed to restore doorbell to %d, err %d\n",
> db_id, err);
>
> - for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
> - i915_reg_t drbreg = GEN8_DRBREGL(i);
> - u32 value = I915_READ(drbreg);
> -
> - if (test_bit(i, guc->doorbell_bitmap))
> - continue;
> -
> - if (i != db_id && (value & GUC_DOORBELL_ENABLED))
> - DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) finally 0x%x\n",
> - i, drbreg.reg, value);
> -
> - }
> + /* Read back & verify all doorbell registers */
> + for (i = 0; i < GUC_MAX_DOORBELLS; ++i)
> + (void)guc_doorbell_check(guc, i);
> }
>
> /**
>
Otherwise looks fine.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/5] drm/i915/guc: use a separate GuC client for each engine
2016-07-19 12:59 [PATCH 1/5] drm/i915/guc: doorbell reset should avoid used doorbells Dave Gordon
2016-07-19 12:59 ` [PATCH 2/5] drm/i915/guc: refactor guc_init_doorbell_hw() Dave Gordon
@ 2016-07-19 12:59 ` Dave Gordon
2016-07-19 15:02 ` Tvrtko Ursulin
2016-07-19 12:59 ` [PATCH 4/5] drm/i915/guc: add engine mask to GuC client & pass to GuC Dave Gordon
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Dave Gordon @ 2016-07-19 12:59 UTC (permalink / raw)
To: intel-gfx
When using a single GuC client for multiple engines, the i915 driver has
to merge all work items into a single work queue, which the GuC firmware
then demultiplexes into separate submission queues per engine. In
theory, this could lead to the single queue becoming a bottleneck in
which an excess of outstanding work for one or more engines might
prevent work for an idle engine reaching the hardware.
To reduce this risk, we can create one GuC client per engine. Each will
have its own workqueue, to be used only for work targeting a single
engine, so there will be no cross-engine contention for workqueue slots.
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 25 ++++++++++++++++-----
drivers/gpu/drm/i915/i915_guc_submission.c | 35 +++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_guc.h | 2 +-
3 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 90aef45..5cbb8ef 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2570,20 +2570,26 @@ static int i915_guc_info(struct seq_file *m, void *data)
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_guc guc;
- struct i915_guc_client client = {};
+ struct i915_guc_client *clients;
struct intel_engine_cs *engine;
+ enum intel_engine_id id;
u64 total = 0;
if (!HAS_GUC_SCHED(dev_priv))
return 0;
+ clients = kcalloc(I915_NUM_ENGINES, sizeof(*clients), GFP_KERNEL);
+ if (clients == NULL)
+ return -ENOMEM;
+
if (mutex_lock_interruptible(&dev->struct_mutex))
- return 0;
+ goto done;
/* Take a local copy of the GuC data, so we can dump it at leisure */
guc = dev_priv->guc;
- if (guc.execbuf_client)
- client = *guc.execbuf_client;
+ for_each_engine_id(engine, dev_priv, id)
+ if (guc.exec_clients[id])
+ clients[id] = *guc.exec_clients[id];
mutex_unlock(&dev->struct_mutex);
@@ -2606,11 +2612,18 @@ static int i915_guc_info(struct seq_file *m, void *data)
}
seq_printf(m, "\t%s: %llu\n", "Total", total);
- seq_printf(m, "\nGuC execbuf client @ %p:\n", guc.execbuf_client);
- i915_guc_client_info(m, dev_priv, &client);
+ for_each_engine_id(engine, dev_priv, id) {
+ seq_printf(m, "\nGuC exec_client[%d] @ %p:\n",
+ id, guc.exec_clients[id]);
+ if (guc.exec_clients[id])
+ i915_guc_client_info(m, dev_priv, &clients[id]);
+ }
/* Add more as required ... */
+done:
+ kfree(clients);
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index dc5f485..b0f9945 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -434,7 +434,9 @@ static void guc_fini_ctx_desc(struct intel_guc *guc,
int i915_guc_wq_check_space(struct drm_i915_gem_request *request)
{
const size_t wqi_size = sizeof(struct guc_wq_item);
- struct i915_guc_client *gc = request->i915->guc.execbuf_client;
+ enum intel_engine_id engine_id = request->engine->id;
+ struct intel_guc *guc = &request->i915->guc;
+ struct i915_guc_client *gc = guc->exec_clients[engine_id];
struct guc_process_desc *desc;
u32 freespace;
@@ -589,7 +591,7 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
{
unsigned int engine_id = rq->engine->id;
struct intel_guc *guc = &rq->i915->guc;
- struct i915_guc_client *client = guc->execbuf_client;
+ struct i915_guc_client *client = guc->exec_clients[engine_id];
int b_ret;
guc_add_workqueue_item(client, rq);
@@ -723,7 +725,7 @@ static bool guc_doorbell_check(struct intel_guc *guc, uint16_t db_id)
*/
static void guc_init_doorbell_hw(struct intel_guc *guc)
{
- struct i915_guc_client *client = guc->execbuf_client;
+ struct i915_guc_client *client = guc->exec_clients[RCS];
uint16_t db_id;
int i, err;
@@ -1004,17 +1006,21 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
struct i915_guc_client *client;
+ struct intel_engine_cs *engine;
- /* client for execbuf submission */
- client = guc_client_alloc(dev_priv,
- GUC_CTX_PRIORITY_KMD_NORMAL,
- dev_priv->kernel_context);
- if (!client) {
- DRM_ERROR("Failed to create execbuf guc_client\n");
- return -ENOMEM;
+ for_each_engine(engine, dev_priv) {
+ /* client for execbuf submission */
+ client = guc_client_alloc(dev_priv,
+ GUC_CTX_PRIORITY_KMD_NORMAL,
+ dev_priv->kernel_context);
+ if (!client) {
+ DRM_ERROR("Failed to create GuC client(s)\n");
+ return -ENOMEM;
+ }
+
+ guc->exec_clients[engine->id] = client;
}
- guc->execbuf_client = client;
host2guc_sample_forcewake(guc, client);
guc_init_doorbell_hw(guc);
@@ -1024,9 +1030,12 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
+ struct intel_engine_cs *engine;
- guc_client_free(dev_priv, guc->execbuf_client);
- guc->execbuf_client = NULL;
+ for_each_engine(engine, dev_priv) {
+ guc_client_free(dev_priv, guc->exec_clients[engine->id]);
+ guc->exec_clients[engine->id] = NULL;
+ }
}
void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 3e3e743..7b4cc4d 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -132,7 +132,7 @@ struct intel_guc {
struct drm_i915_gem_object *ctx_pool_obj;
struct ida ctx_ids;
- struct i915_guc_client *execbuf_client;
+ struct i915_guc_client *exec_clients[I915_NUM_ENGINES];
DECLARE_BITMAP(doorbell_bitmap, GUC_MAX_DOORBELLS);
uint32_t db_cacheline; /* Cyclic counter mod pagesize */
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/5] drm/i915/guc: use a separate GuC client for each engine
2016-07-19 12:59 ` [PATCH 3/5] drm/i915/guc: use a separate GuC client for each engine Dave Gordon
@ 2016-07-19 15:02 ` Tvrtko Ursulin
2016-07-19 15:13 ` Dave Gordon
0 siblings, 1 reply; 14+ messages in thread
From: Tvrtko Ursulin @ 2016-07-19 15:02 UTC (permalink / raw)
To: Dave Gordon, intel-gfx
On 19/07/16 13:59, Dave Gordon wrote:
> When using a single GuC client for multiple engines, the i915 driver has
> to merge all work items into a single work queue, which the GuC firmware
> then demultiplexes into separate submission queues per engine. In
> theory, this could lead to the single queue becoming a bottleneck in
> which an excess of outstanding work for one or more engines might
> prevent work for an idle engine reaching the hardware.
>
> To reduce this risk, we can create one GuC client per engine. Each will
> have its own workqueue, to be used only for work targeting a single
> engine, so there will be no cross-engine contention for workqueue slots.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 25 ++++++++++++++++-----
> drivers/gpu/drm/i915/i915_guc_submission.c | 35 +++++++++++++++++++-----------
> drivers/gpu/drm/i915/intel_guc.h | 2 +-
> 3 files changed, 42 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 90aef45..5cbb8ef 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2570,20 +2570,26 @@ static int i915_guc_info(struct seq_file *m, void *data)
> struct drm_device *dev = node->minor->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_guc guc;
> - struct i915_guc_client client = {};
> + struct i915_guc_client *clients;
> struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> u64 total = 0;
>
> if (!HAS_GUC_SCHED(dev_priv))
> return 0;
>
> + clients = kcalloc(I915_NUM_ENGINES, sizeof(*clients), GFP_KERNEL);
> + if (clients == NULL)
> + return -ENOMEM;
> +
> if (mutex_lock_interruptible(&dev->struct_mutex))
> - return 0;
> + goto done;
>
> /* Take a local copy of the GuC data, so we can dump it at leisure */
> guc = dev_priv->guc;
> - if (guc.execbuf_client)
> - client = *guc.execbuf_client;
> + for_each_engine_id(engine, dev_priv, id)
> + if (guc.exec_clients[id])
> + clients[id] = *guc.exec_clients[id];
>
> mutex_unlock(&dev->struct_mutex);
>
> @@ -2606,11 +2612,18 @@ static int i915_guc_info(struct seq_file *m, void *data)
> }
> seq_printf(m, "\t%s: %llu\n", "Total", total);
>
> - seq_printf(m, "\nGuC execbuf client @ %p:\n", guc.execbuf_client);
> - i915_guc_client_info(m, dev_priv, &client);
> + for_each_engine_id(engine, dev_priv, id) {
> + seq_printf(m, "\nGuC exec_client[%d] @ %p:\n",
> + id, guc.exec_clients[id]);
Minor and not a blocker for this patch, but I would potentially
re-consider if printing out the client pointer is useful.
> + if (guc.exec_clients[id])
> + i915_guc_client_info(m, dev_priv, &clients[id]);
> + }
>
> /* Add more as required ... */
>
> +done:
> + kfree(clients);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index dc5f485..b0f9945 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -434,7 +434,9 @@ static void guc_fini_ctx_desc(struct intel_guc *guc,
> int i915_guc_wq_check_space(struct drm_i915_gem_request *request)
> {
> const size_t wqi_size = sizeof(struct guc_wq_item);
> - struct i915_guc_client *gc = request->i915->guc.execbuf_client;
> + enum intel_engine_id engine_id = request->engine->id;
> + struct intel_guc *guc = &request->i915->guc;
> + struct i915_guc_client *gc = guc->exec_clients[engine_id];
> struct guc_process_desc *desc;
> u32 freespace;
>
> @@ -589,7 +591,7 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
> {
> unsigned int engine_id = rq->engine->id;
> struct intel_guc *guc = &rq->i915->guc;
> - struct i915_guc_client *client = guc->execbuf_client;
> + struct i915_guc_client *client = guc->exec_clients[engine_id];
> int b_ret;
>
> guc_add_workqueue_item(client, rq);
> @@ -723,7 +725,7 @@ static bool guc_doorbell_check(struct intel_guc *guc, uint16_t db_id)
> */
> static void guc_init_doorbell_hw(struct intel_guc *guc)
> {
> - struct i915_guc_client *client = guc->execbuf_client;
> + struct i915_guc_client *client = guc->exec_clients[RCS];
> uint16_t db_id;
> int i, err;
>
> @@ -1004,17 +1006,21 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
> {
> struct intel_guc *guc = &dev_priv->guc;
> struct i915_guc_client *client;
> + struct intel_engine_cs *engine;
>
> - /* client for execbuf submission */
> - client = guc_client_alloc(dev_priv,
> - GUC_CTX_PRIORITY_KMD_NORMAL,
> - dev_priv->kernel_context);
> - if (!client) {
> - DRM_ERROR("Failed to create execbuf guc_client\n");
> - return -ENOMEM;
> + for_each_engine(engine, dev_priv) {
> + /* client for execbuf submission */
> + client = guc_client_alloc(dev_priv,
> + GUC_CTX_PRIORITY_KMD_NORMAL,
> + dev_priv->kernel_context);
> + if (!client) {
> + DRM_ERROR("Failed to create GuC client(s)\n");
> + return -ENOMEM;
> + }
> +
> + guc->exec_clients[engine->id] = client;
> }
>
> - guc->execbuf_client = client;
> host2guc_sample_forcewake(guc, client);
> guc_init_doorbell_hw(guc);
>
> @@ -1024,9 +1030,12 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
> void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
> {
> struct intel_guc *guc = &dev_priv->guc;
> + struct intel_engine_cs *engine;
>
> - guc_client_free(dev_priv, guc->execbuf_client);
> - guc->execbuf_client = NULL;
> + for_each_engine(engine, dev_priv) {
> + guc_client_free(dev_priv, guc->exec_clients[engine->id]);
> + guc->exec_clients[engine->id] = NULL;
> + }
> }
>
> void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 3e3e743..7b4cc4d 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -132,7 +132,7 @@ struct intel_guc {
> struct drm_i915_gem_object *ctx_pool_obj;
> struct ida ctx_ids;
>
> - struct i915_guc_client *execbuf_client;
> + struct i915_guc_client *exec_clients[I915_NUM_ENGINES];
>
> DECLARE_BITMAP(doorbell_bitmap, GUC_MAX_DOORBELLS);
> uint32_t db_cacheline; /* Cyclic counter mod pagesize */
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/5] drm/i915/guc: use a separate GuC client for each engine
2016-07-19 15:02 ` Tvrtko Ursulin
@ 2016-07-19 15:13 ` Dave Gordon
0 siblings, 0 replies; 14+ messages in thread
From: Dave Gordon @ 2016-07-19 15:13 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
On 19/07/16 16:02, Tvrtko Ursulin wrote:
>
> On 19/07/16 13:59, Dave Gordon wrote:
>> When using a single GuC client for multiple engines, the i915 driver has
>> to merge all work items into a single work queue, which the GuC firmware
>> then demultiplexes into separate submission queues per engine. In
>> theory, this could lead to the single queue becoming a bottleneck in
>> which an excess of outstanding work for one or more engines might
>> prevent work for an idle engine reaching the hardware.
>>
>> To reduce this risk, we can create one GuC client per engine. Each will
>> have its own workqueue, to be used only for work targeting a single
>> engine, so there will be no cross-engine contention for workqueue slots.
>>
>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 25 ++++++++++++++++-----
>> drivers/gpu/drm/i915/i915_guc_submission.c | 35
>> +++++++++++++++++++-----------
>> drivers/gpu/drm/i915/intel_guc.h | 2 +-
>> 3 files changed, 42 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 90aef45..5cbb8ef 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2570,20 +2570,26 @@ static int i915_guc_info(struct seq_file *m,
>> void *data)
>> struct drm_device *dev = node->minor->dev;
>> struct drm_i915_private *dev_priv = to_i915(dev);
>> struct intel_guc guc;
>> - struct i915_guc_client client = {};
>> + struct i915_guc_client *clients;
>> struct intel_engine_cs *engine;
>> + enum intel_engine_id id;
>> u64 total = 0;
>>
>> if (!HAS_GUC_SCHED(dev_priv))
>> return 0;
>>
>> + clients = kcalloc(I915_NUM_ENGINES, sizeof(*clients), GFP_KERNEL);
>> + if (clients == NULL)
>> + return -ENOMEM;
>> +
>> if (mutex_lock_interruptible(&dev->struct_mutex))
>> - return 0;
>> + goto done;
>>
>> /* Take a local copy of the GuC data, so we can dump it at
>> leisure */
>> guc = dev_priv->guc;
>> - if (guc.execbuf_client)
>> - client = *guc.execbuf_client;
>> + for_each_engine_id(engine, dev_priv, id)
>> + if (guc.exec_clients[id])
>> + clients[id] = *guc.exec_clients[id];
>>
>> mutex_unlock(&dev->struct_mutex);
>>
>> @@ -2606,11 +2612,18 @@ static int i915_guc_info(struct seq_file *m,
>> void *data)
>> }
>> seq_printf(m, "\t%s: %llu\n", "Total", total);
>>
>> - seq_printf(m, "\nGuC execbuf client @ %p:\n", guc.execbuf_client);
>> - i915_guc_client_info(m, dev_priv, &client);
>> + for_each_engine_id(engine, dev_priv, id) {
>> + seq_printf(m, "\nGuC exec_client[%d] @ %p:\n",
>> + id, guc.exec_clients[id]);
>
> Minor and not a blocker for this patch, but I would potentially
> re-consider if printing out the client pointer is useful.
It's really only useful to know whether it's NULL or not; but printing
the pointer itself is simpler than printing a message saying that. This
is a debugfs interface, so the content is pretty much ad-hoc.
.Dave.
>> + if (guc.exec_clients[id])
>> + i915_guc_client_info(m, dev_priv, &clients[id]);
>> + }
>>
>> /* Add more as required ... */
>>
>> +done:
>> + kfree(clients);
>> +
>> return 0;
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index dc5f485..b0f9945 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -434,7 +434,9 @@ static void guc_fini_ctx_desc(struct intel_guc *guc,
>> int i915_guc_wq_check_space(struct drm_i915_gem_request *request)
>> {
>> const size_t wqi_size = sizeof(struct guc_wq_item);
>> - struct i915_guc_client *gc = request->i915->guc.execbuf_client;
>> + enum intel_engine_id engine_id = request->engine->id;
>> + struct intel_guc *guc = &request->i915->guc;
>> + struct i915_guc_client *gc = guc->exec_clients[engine_id];
>> struct guc_process_desc *desc;
>> u32 freespace;
>>
>> @@ -589,7 +591,7 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
>> {
>> unsigned int engine_id = rq->engine->id;
>> struct intel_guc *guc = &rq->i915->guc;
>> - struct i915_guc_client *client = guc->execbuf_client;
>> + struct i915_guc_client *client = guc->exec_clients[engine_id];
>> int b_ret;
>>
>> guc_add_workqueue_item(client, rq);
>> @@ -723,7 +725,7 @@ static bool guc_doorbell_check(struct intel_guc
>> *guc, uint16_t db_id)
>> */
>> static void guc_init_doorbell_hw(struct intel_guc *guc)
>> {
>> - struct i915_guc_client *client = guc->execbuf_client;
>> + struct i915_guc_client *client = guc->exec_clients[RCS];
>> uint16_t db_id;
>> int i, err;
>>
>> @@ -1004,17 +1006,21 @@ int i915_guc_submission_enable(struct
>> drm_i915_private *dev_priv)
>> {
>> struct intel_guc *guc = &dev_priv->guc;
>> struct i915_guc_client *client;
>> + struct intel_engine_cs *engine;
>>
>> - /* client for execbuf submission */
>> - client = guc_client_alloc(dev_priv,
>> - GUC_CTX_PRIORITY_KMD_NORMAL,
>> - dev_priv->kernel_context);
>> - if (!client) {
>> - DRM_ERROR("Failed to create execbuf guc_client\n");
>> - return -ENOMEM;
>> + for_each_engine(engine, dev_priv) {
>> + /* client for execbuf submission */
>> + client = guc_client_alloc(dev_priv,
>> + GUC_CTX_PRIORITY_KMD_NORMAL,
>> + dev_priv->kernel_context);
>> + if (!client) {
>> + DRM_ERROR("Failed to create GuC client(s)\n");
>> + return -ENOMEM;
>> + }
>> +
>> + guc->exec_clients[engine->id] = client;
>> }
>>
>> - guc->execbuf_client = client;
>> host2guc_sample_forcewake(guc, client);
>> guc_init_doorbell_hw(guc);
>>
>> @@ -1024,9 +1030,12 @@ int i915_guc_submission_enable(struct
>> drm_i915_private *dev_priv)
>> void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
>> {
>> struct intel_guc *guc = &dev_priv->guc;
>> + struct intel_engine_cs *engine;
>>
>> - guc_client_free(dev_priv, guc->execbuf_client);
>> - guc->execbuf_client = NULL;
>> + for_each_engine(engine, dev_priv) {
>> + guc_client_free(dev_priv, guc->exec_clients[engine->id]);
>> + guc->exec_clients[engine->id] = NULL;
>> + }
>> }
>>
>> void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h
>> b/drivers/gpu/drm/i915/intel_guc.h
>> index 3e3e743..7b4cc4d 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -132,7 +132,7 @@ struct intel_guc {
>> struct drm_i915_gem_object *ctx_pool_obj;
>> struct ida ctx_ids;
>>
>> - struct i915_guc_client *execbuf_client;
>> + struct i915_guc_client *exec_clients[I915_NUM_ENGINES];
>>
>> DECLARE_BITMAP(doorbell_bitmap, GUC_MAX_DOORBELLS);
>> uint32_t db_cacheline; /* Cyclic counter mod pagesize */
>>
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Regards,
> Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/5] drm/i915/guc: add engine mask to GuC client & pass to GuC
2016-07-19 12:59 [PATCH 1/5] drm/i915/guc: doorbell reset should avoid used doorbells Dave Gordon
2016-07-19 12:59 ` [PATCH 2/5] drm/i915/guc: refactor guc_init_doorbell_hw() Dave Gordon
2016-07-19 12:59 ` [PATCH 3/5] drm/i915/guc: use a separate GuC client for each engine Dave Gordon
@ 2016-07-19 12:59 ` Dave Gordon
2016-07-19 15:06 ` Tvrtko Ursulin
2016-07-27 20:06 ` kbuild test robot
2016-07-19 12:59 ` [PATCH 5/5] drm/i915/guc: use for_each_engine_id() where appropriate Dave Gordon
` (2 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Dave Gordon @ 2016-07-19 12:59 UTC (permalink / raw)
To: intel-gfx
The Context Descriptor passed by the kernel to the GuC contains a field
specifying which engine(s) the context will use. Historically, this was
always set to "all of them", but now that we have one client per engine,
we can be more precise, and set only the single bit for the engine that
the client is associated with.
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++++++++++-----
drivers/gpu/drm/i915/intel_guc.h | 3 ++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b0f9945..4daba77 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -340,7 +340,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
desc.priority = client->priority;
desc.db_id = client->doorbell_id;
- for_each_engine(engine, dev_priv) {
+ for_each_engine_masked(engine, dev_priv, client->engines) {
struct intel_context *ce = &ctx->engine[engine->id];
struct guc_execlist_context *lrc = &desc.lrc[engine->guc_id];
struct drm_i915_gem_object *obj;
@@ -374,6 +374,8 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
desc.engines_used |= (1 << engine->guc_id);
}
+ DRM_DEBUG_DRIVER("Host engines 0x%x => GuC engines used 0x%x\n",
+ client->engines, desc.engines_used);
WARN_ON(desc.engines_used == 0);
/*
@@ -768,6 +770,7 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
*/
static struct i915_guc_client *
guc_client_alloc(struct drm_i915_private *dev_priv,
+ uint32_t engines,
uint32_t priority,
struct i915_gem_context *ctx)
{
@@ -780,10 +783,11 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
if (!client)
return NULL;
- client->doorbell_id = GUC_INVALID_DOORBELL_ID;
- client->priority = priority;
client->owner = ctx;
client->guc = guc;
+ client->engines = engines;
+ client->priority = priority;
+ client->doorbell_id = GUC_INVALID_DOORBELL_ID;
client->ctx_index = (uint32_t)ida_simple_get(&guc->ctx_ids, 0,
GUC_MAX_GPU_CONTEXTS, GFP_KERNEL);
@@ -825,8 +829,8 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
if (guc_init_doorbell(guc, client, db_id))
goto err;
- DRM_DEBUG_DRIVER("new priority %u client %p: ctx_index %u\n",
- priority, client, client->ctx_index);
+ DRM_DEBUG_DRIVER("new priority %u client %p for engine(s) 0x%x: ctx_index %u\n",
+ priority, client, client->engines, client->ctx_index);
DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%x\n",
client->doorbell_id, client->doorbell_offset);
@@ -1011,6 +1015,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
for_each_engine(engine, dev_priv) {
/* client for execbuf submission */
client = guc_client_alloc(dev_priv,
+ intel_engine_flag(engine),
GUC_CTX_PRIORITY_KMD_NORMAL,
dev_priv->kernel_context);
if (!client) {
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 7b4cc4d..53d41b5 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -67,6 +67,8 @@ struct i915_guc_client {
void *client_base; /* first page (only) of above */
struct i915_gem_context *owner;
struct intel_guc *guc;
+
+ uint32_t engines; /* bitmap of (host) engine ids */
uint32_t priority;
uint32_t ctx_index;
@@ -79,7 +81,6 @@ struct i915_guc_client {
uint32_t wq_offset;
uint32_t wq_size;
uint32_t wq_tail;
- uint32_t unused; /* Was 'wq_head' */
uint32_t no_wq_space;
uint32_t q_fail; /* No longer used */
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 4/5] drm/i915/guc: add engine mask to GuC client & pass to GuC
2016-07-19 12:59 ` [PATCH 4/5] drm/i915/guc: add engine mask to GuC client & pass to GuC Dave Gordon
@ 2016-07-19 15:06 ` Tvrtko Ursulin
2016-07-27 20:06 ` kbuild test robot
1 sibling, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2016-07-19 15:06 UTC (permalink / raw)
To: Dave Gordon, intel-gfx
On 19/07/16 13:59, Dave Gordon wrote:
> The Context Descriptor passed by the kernel to the GuC contains a field
> specifying which engine(s) the context will use. Historically, this was
> always set to "all of them", but now that we have one client per engine,
> we can be more precise, and set only the single bit for the engine that
> the client is associated with.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++++++++++-----
> drivers/gpu/drm/i915/intel_guc.h | 3 ++-
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index b0f9945..4daba77 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -340,7 +340,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
> desc.priority = client->priority;
> desc.db_id = client->doorbell_id;
>
> - for_each_engine(engine, dev_priv) {
> + for_each_engine_masked(engine, dev_priv, client->engines) {
> struct intel_context *ce = &ctx->engine[engine->id];
> struct guc_execlist_context *lrc = &desc.lrc[engine->guc_id];
> struct drm_i915_gem_object *obj;
> @@ -374,6 +374,8 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
> desc.engines_used |= (1 << engine->guc_id);
> }
>
> + DRM_DEBUG_DRIVER("Host engines 0x%x => GuC engines used 0x%x\n",
> + client->engines, desc.engines_used);
> WARN_ON(desc.engines_used == 0);
>
> /*
> @@ -768,6 +770,7 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
> */
> static struct i915_guc_client *
> guc_client_alloc(struct drm_i915_private *dev_priv,
> + uint32_t engines,
> uint32_t priority,
> struct i915_gem_context *ctx)
> {
> @@ -780,10 +783,11 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
> if (!client)
> return NULL;
>
> - client->doorbell_id = GUC_INVALID_DOORBELL_ID;
> - client->priority = priority;
> client->owner = ctx;
> client->guc = guc;
> + client->engines = engines;
> + client->priority = priority;
> + client->doorbell_id = GUC_INVALID_DOORBELL_ID;
>
> client->ctx_index = (uint32_t)ida_simple_get(&guc->ctx_ids, 0,
> GUC_MAX_GPU_CONTEXTS, GFP_KERNEL);
> @@ -825,8 +829,8 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
> if (guc_init_doorbell(guc, client, db_id))
> goto err;
>
> - DRM_DEBUG_DRIVER("new priority %u client %p: ctx_index %u\n",
> - priority, client, client->ctx_index);
> + DRM_DEBUG_DRIVER("new priority %u client %p for engine(s) 0x%x: ctx_index %u\n",
> + priority, client, client->engines, client->ctx_index);
> DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%x\n",
> client->doorbell_id, client->doorbell_offset);
>
> @@ -1011,6 +1015,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
> for_each_engine(engine, dev_priv) {
> /* client for execbuf submission */
> client = guc_client_alloc(dev_priv,
> + intel_engine_flag(engine),
> GUC_CTX_PRIORITY_KMD_NORMAL,
> dev_priv->kernel_context);
> if (!client) {
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 7b4cc4d..53d41b5 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -67,6 +67,8 @@ struct i915_guc_client {
> void *client_base; /* first page (only) of above */
> struct i915_gem_context *owner;
> struct intel_guc *guc;
> +
> + uint32_t engines; /* bitmap of (host) engine ids */
> uint32_t priority;
> uint32_t ctx_index;
>
> @@ -79,7 +81,6 @@ struct i915_guc_client {
> uint32_t wq_offset;
> uint32_t wq_size;
> uint32_t wq_tail;
> - uint32_t unused; /* Was 'wq_head' */
>
> uint32_t no_wq_space;
> uint32_t q_fail; /* No longer used */
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/5] drm/i915/guc: add engine mask to GuC client & pass to GuC
2016-07-19 12:59 ` [PATCH 4/5] drm/i915/guc: add engine mask to GuC client & pass to GuC Dave Gordon
2016-07-19 15:06 ` Tvrtko Ursulin
@ 2016-07-27 20:06 ` kbuild test robot
1 sibling, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2016-07-27 20:06 UTC (permalink / raw)
To: Dave Gordon; +Cc: intel-gfx, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 15044 bytes --]
Hi,
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20160727]
[cannot apply to v4.7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dave-Gordon/drm-i915-guc-doorbell-reset-should-avoid-used-doorbells/20160725-094456
base: git://anongit.freedesktop.org/drm-intel for-linux-next
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
Warning: The Sphinx 'sphinx_rtd_theme' HTML theme was not found. Make sure you have the theme installed to produce pretty HTML output. Falling back to the default theme.
include/drm/drm_fourcc.h:1: warning: no structured comments found
include/drm/drm_crtc.h:848: warning: No description found for parameter 'index'
include/drm/drm_crtc.h:1223: warning: No description found for parameter 'index'
include/drm/drm_crtc.h:1720: warning: No description found for parameter 'index'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'connector_ida'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'edid_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'dpms_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'path_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tile_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'plane_type_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'rotation_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_src_x'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_src_y'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_src_w'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_src_h'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_crtc_x'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_crtc_y'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_crtc_w'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_crtc_h'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_fb_id'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_crtc_id'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_active'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'prop_mode_id'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'dvi_i_subconnector_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'dvi_i_select_subconnector_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_subconnector_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_select_subconnector_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_mode_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_left_margin_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_right_margin_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_top_margin_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_bottom_margin_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_brightness_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_contrast_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_flicker_reduction_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_overscan_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_saturation_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'tv_hue_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'scaling_mode_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'aspect_ratio_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'dirty_info_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'suggested_x_property'
include/drm/drm_crtc.h:2505: warning: No description found for parameter 'suggested_y_property'
include/drm/drmP.h:172: warning: No description found for parameter 'fmt'
include/drm/drmP.h:188: warning: No description found for parameter 'fmt'
include/drm/drmP.h:206: warning: No description found for parameter 'fmt'
include/drm/drmP.h:251: warning: No description found for parameter 'dev'
include/drm/drmP.h:251: warning: No description found for parameter 'data'
include/drm/drmP.h:251: warning: No description found for parameter 'file_priv'
include/drm/drmP.h:284: warning: No description found for parameter 'ioctl'
include/drm/drmP.h:284: warning: No description found for parameter '_func'
include/drm/drmP.h:284: warning: No description found for parameter '_flags'
include/drm/drmP.h:365: warning: cannot understand function prototype: 'struct drm_lock_data '
include/drm/drmP.h:392: warning: cannot understand function prototype: 'struct drm_driver '
include/drm/drmP.h:655: warning: cannot understand function prototype: 'struct drm_info_list '
include/drm/drmP.h:665: warning: cannot understand function prototype: 'struct drm_info_node '
include/drm/drmP.h:675: warning: cannot understand function prototype: 'struct drm_minor '
include/drm/drmP.h:720: warning: cannot understand function prototype: 'struct drm_device '
include/drm/drm_dp_helper.h:752: warning: No description found for parameter 'i2c_nack_count'
include/drm/drm_dp_helper.h:752: warning: No description found for parameter 'i2c_defer_count'
drivers/gpu/drm/drm_dp_mst_topology.c:2383: warning: No description found for parameter 'connector'
include/drm/drm_dp_mst_helper.h:93: warning: No description found for parameter 'cached_edid'
include/drm/drm_dp_mst_helper.h:93: warning: No description found for parameter 'has_audio'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'max_dpcd_transaction_bytes'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'sink_count'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'total_slots'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'avail_slots'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'total_pbn'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'qlock'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'tx_msg_downq'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'tx_down_in_progress'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'payload_lock'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'proposed_vcpis'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'payloads'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'payload_mask'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'vcpi_mask'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'tx_waitq'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'work'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'tx_work'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'destroy_connector_list'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'destroy_connector_lock'
include/drm/drm_dp_mst_helper.h:467: warning: No description found for parameter 'destroy_connector_work'
drivers/gpu/drm/drm_dp_mst_topology.c:2384: warning: No description found for parameter 'connector'
drivers/gpu/drm/i915/i915_vgpu.c:105: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/i915_vgpu.c:184: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/i915_vgpu.c:184: warning: Excess function parameter 'dev' description in 'intel_vgt_balloon'
drivers/gpu/drm/i915/i915_vgpu.c:106: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/i915_vgpu.c:185: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/i915_vgpu.c:185: warning: Excess function parameter 'dev' description in 'intel_vgt_balloon'
drivers/gpu/drm/i915/i915_gem.c:929: warning: No description found for parameter 'i915'
drivers/gpu/drm/i915/i915_gem.c:929: warning: Excess function parameter 'dev' description in 'i915_gem_gtt_pwrite_fast'
drivers/gpu/drm/i915/intel_hotplug.c:543: warning: Excess function parameter 'enabled' description in 'intel_hpd_poll_init'
drivers/gpu/drm/i915/intel_hotplug.c:544: warning: Excess function parameter 'enabled' description in 'intel_hpd_poll_init'
drivers/gpu/drm/i915/intel_fbc.c:1087: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_fbc.c:1087: warning: No description found for parameter 'plane_state'
drivers/gpu/drm/i915/intel_fbc.c:1088: warning: No description found for parameter 'crtc_state'
drivers/gpu/drm/i915/intel_fbc.c:1088: warning: No description found for parameter 'plane_state'
>> drivers/gpu/drm/i915/i915_guc_submission.c:776: warning: No description found for parameter 'engines'
drivers/gpu/drm/i915/i915_guc_submission.c:777: warning: No description found for parameter 'engines'
Documentation/gpu/drm-internals.rst:249: ERROR: Unexpected indentation.
Documentation/gpu/drm-internals.rst:188: ERROR: Unknown target name: "devm".
Documentation/gpu/drm-kms.rst:929: WARNING: Inline literal start-string without end-string.
Documentation/gpu/drm-kms.rst:971: WARNING: Inline literal start-string without end-string.
Documentation/gpu/drm-kms.rst:1451: WARNING: Inline literal start-string without end-string.
Documentation/gpu/drm-kms.rst:1565: WARNING: Inline literal start-string without end-string.
Documentation/gpu/drm-kms.rst:1615: WARNING: Inline literal start-string without end-string.
Documentation/gpu/drm-kms.rst:1627: WARNING: Inline literal start-string without end-string.
Documentation/gpu/drm-kms.rst:836: WARNING: Option list ends without a blank line; unexpected unindent.
Documentation/gpu/drm-kms-helpers.rst:647: WARNING: Inline emphasis start-string without end-string.
Documentation/gpu/drm-kms-helpers.rst:275: WARNING: Inline literal start-string without end-string.
Documentation/gpu/drm-mm.rst:505: WARNING: Inline emphasis start-string without end-string.
drivers/gpu/drm/i915/intel_uncore.c:1622: ERROR: Unexpected indentation.
Documentation/gpu/i915.rst:156: WARNING: Block quote ends without a blank line; unexpected unindent.
drivers/gpu/drm/i915/intel_uncore.c:1656: ERROR: Unexpected indentation.
Documentation/gpu/i915.rst:192: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/gpu/i915.rst:129: WARNING: Literal block ends without a blank line; unexpected unindent.
Documentation/gpu/i915.rst:148: WARNING: Inline emphasis start-string without end-string.
Documentation/gpu/i915.rst:148: WARNING: Inline emphasis start-string without end-string.
drivers/gpu/drm/i915/intel_lrc.c:1165: ERROR: Unexpected indentation.
Documentation/gpu/i915.rst:339: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/gpu/i915.rst:329: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/gpu/i915.rst:348: WARNING: Enumerated list ends without a blank line; unexpected unindent.
Documentation/gpu/drm-kms.rst:499: WARNING: Could not lex literal_block as "C". Highlighting skipped.
Documentation/gpu/drm-kms-helpers.rst:125: WARNING: Could not lex literal_block as "C". Highlighting skipped.
Documentation/gpu/i915.rst:109: WARNING: Could not lex literal_block as "C". Highlighting skipped.
vim +/engines +776 drivers/gpu/drm/i915/i915_guc_submission.c
44a28b1d Dave Gordon 2015-08-12 760 * guc_client_alloc() - Allocate an i915_guc_client
0daf556c Dave Gordon 2016-06-10 761 * @dev_priv: driver private data structure
44a28b1d Dave Gordon 2015-08-12 762 * @priority: four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
44a28b1d Dave Gordon 2015-08-12 763 * The kernel client to replace ExecList submission is created with
44a28b1d Dave Gordon 2015-08-12 764 * NORMAL priority. Priority of a client for scheduler can be HIGH,
44a28b1d Dave Gordon 2015-08-12 765 * while a preemption context can use CRITICAL.
feda33ef Alex Dai 2015-10-19 766 * @ctx: the context that owns the client (we use the default render
feda33ef Alex Dai 2015-10-19 767 * context)
44a28b1d Dave Gordon 2015-08-12 768 *
0d92a6a4 Dave Gordon 2016-04-19 769 * Return: An i915_guc_client object if success, else NULL.
44a28b1d Dave Gordon 2015-08-12 770 */
0daf556c Dave Gordon 2016-06-10 771 static struct i915_guc_client *
0daf556c Dave Gordon 2016-06-10 772 guc_client_alloc(struct drm_i915_private *dev_priv,
e5601222 Dave Gordon 2016-07-19 773 uint32_t engines,
d1675198 Alex Dai 2015-08-12 774 uint32_t priority,
e2efd130 Chris Wilson 2016-05-24 775 struct i915_gem_context *ctx)
44a28b1d Dave Gordon 2015-08-12 @776 {
44a28b1d Dave Gordon 2015-08-12 777 struct i915_guc_client *client;
44a28b1d Dave Gordon 2015-08-12 778 struct intel_guc *guc = &dev_priv->guc;
44a28b1d Dave Gordon 2015-08-12 779 struct drm_i915_gem_object *obj;
a667429b Dave Gordon 2016-06-13 780 uint16_t db_id;
44a28b1d Dave Gordon 2015-08-12 781
44a28b1d Dave Gordon 2015-08-12 782 client = kzalloc(sizeof(*client), GFP_KERNEL);
44a28b1d Dave Gordon 2015-08-12 783 if (!client)
44a28b1d Dave Gordon 2015-08-12 784 return NULL;
:::::: The code at line 776 was first introduced by commit
:::::: 44a28b1d36762499de6fd701fcce6814eefe31d7 drm/i915: Implementation of GuC submission client
:::::: TO: Dave Gordon <david.s.gordon@intel.com>
:::::: CC: Daniel Vetter <daniel.vetter@ffwll.ch>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6370 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/5] drm/i915/guc: use for_each_engine_id() where appropriate
2016-07-19 12:59 [PATCH 1/5] drm/i915/guc: doorbell reset should avoid used doorbells Dave Gordon
` (2 preceding siblings ...)
2016-07-19 12:59 ` [PATCH 4/5] drm/i915/guc: add engine mask to GuC client & pass to GuC Dave Gordon
@ 2016-07-19 12:59 ` Dave Gordon
2016-07-19 15:09 ` Tvrtko Ursulin
2016-07-19 14:16 ` ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915/guc: doorbell reset should avoid used doorbells Patchwork
2016-07-19 14:43 ` [PATCH 1/5] " Tvrtko Ursulin
5 siblings, 1 reply; 14+ messages in thread
From: Dave Gordon @ 2016-07-19 12:59 UTC (permalink / raw)
To: intel-gfx
Now that host structures are indexed by host engine-id rather than
guc_id, we can usefully convert some for_each_engine() loops to use
for_each_engine_id() and avoid multiple dereferences of engine->id.
Also a few related tweaks to cache structure members locally wherever
they're used more than once or twice, hopefully eliminating memory
references.
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++--------
drivers/gpu/drm/i915/i915_guc_submission.c | 22 +++++++++++++---------
2 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5cbb8ef..76918ab 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2541,6 +2541,7 @@ static void i915_guc_client_info(struct seq_file *m,
struct i915_guc_client *client)
{
struct intel_engine_cs *engine;
+ enum intel_engine_id id;
uint64_t tot = 0;
seq_printf(m, "\tPriority %d, GuC ctx index: %u, PD offset 0x%x\n",
@@ -2555,11 +2556,11 @@ static void i915_guc_client_info(struct seq_file *m,
seq_printf(m, "\tFailed doorbell: %u\n", client->b_fail);
seq_printf(m, "\tLast submission result: %d\n", client->retcode);
- for_each_engine(engine, dev_priv) {
+ for_each_engine_id(engine, dev_priv, id) {
+ u64 submissions = client->submissions[id];
+ tot += submissions;
seq_printf(m, "\tSubmissions: %llu %s\n",
- client->submissions[engine->id],
- engine->name);
- tot += client->submissions[engine->id];
+ submissions, engine->name);
}
seq_printf(m, "\tTotal: %llu\n", tot);
}
@@ -2604,11 +2605,11 @@ static int i915_guc_info(struct seq_file *m, void *data)
seq_printf(m, "GuC last action error code: %d\n", guc.action_err);
seq_printf(m, "\nGuC submissions:\n");
- for_each_engine(engine, dev_priv) {
+ for_each_engine_id(engine, dev_priv, id) {
+ u64 submissions = guc.submissions[id];
+ total += submissions;
seq_printf(m, "\t%-24s: %10llu, last seqno 0x%08x\n",
- engine->name, guc.submissions[engine->id],
- guc.last_seqno[engine->id]);
- total += guc.submissions[engine->id];
+ engine->name, submissions, guc.last_seqno[id]);
}
seq_printf(m, "\t%s: %llu\n", "Total", total);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 4daba77..5beed1b 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -342,7 +342,8 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
for_each_engine_masked(engine, dev_priv, client->engines) {
struct intel_context *ce = &ctx->engine[engine->id];
- struct guc_execlist_context *lrc = &desc.lrc[engine->guc_id];
+ uint32_t guc_engine_id = engine->guc_id;
+ struct guc_execlist_context *lrc = &desc.lrc[guc_engine_id];
struct drm_i915_gem_object *obj;
/* TODO: We have a design issue to be solved here. Only when we
@@ -361,7 +362,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
gfx_addr = i915_gem_obj_ggtt_offset(ce->state);
lrc->ring_lcra = gfx_addr + LRC_STATE_PN * PAGE_SIZE;
lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) |
- (engine->guc_id << GUC_ELC_ENGINE_OFFSET);
+ (guc_engine_id << GUC_ELC_ENGINE_OFFSET);
obj = ce->ringbuf->obj;
gfx_addr = i915_gem_obj_ggtt_offset(obj);
@@ -371,7 +372,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
lrc->ring_next_free_location = gfx_addr;
lrc->ring_current_tail_pointer_value = 0;
- desc.engines_used |= (1 << engine->guc_id);
+ desc.engines_used |= (1 << guc_engine_id);
}
DRM_DEBUG_DRIVER("Host engines 0x%x => GuC engines used 0x%x\n",
@@ -461,6 +462,7 @@ static void guc_add_workqueue_item(struct i915_guc_client *gc,
/* 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 intel_engine_cs *engine = rq->engine;
struct guc_process_desc *desc;
struct guc_wq_item *wqi;
void *base;
@@ -502,12 +504,11 @@ static void guc_add_workqueue_item(struct i915_guc_client *gc,
/* Now fill in the 4-word work queue item */
wqi->header = WQ_TYPE_INORDER |
(wqi_len << WQ_LEN_SHIFT) |
- (rq->engine->guc_id << WQ_TARGET_SHIFT) |
+ (engine->guc_id << WQ_TARGET_SHIFT) |
WQ_NO_WCFLUSH_WAIT;
/* The GuC wants only the low-order word of the context descriptor */
- wqi->context_desc = (u32)intel_lr_context_descriptor(rq->ctx,
- rq->engine);
+ wqi->context_desc = (u32)intel_lr_context_descriptor(rq->ctx, engine);
wqi->ring_tail = tail << WQ_RING_TAIL_SHIFT;
wqi->fence_id = rq->seqno;
@@ -1035,11 +1036,14 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
+ struct i915_guc_client **gc_ptr;
struct intel_engine_cs *engine;
+ enum intel_engine_id engine_id;
- for_each_engine(engine, dev_priv) {
- guc_client_free(dev_priv, guc->exec_clients[engine->id]);
- guc->exec_clients[engine->id] = NULL;
+ for_each_engine_id(engine, dev_priv, engine_id) {
+ gc_ptr = &guc->exec_clients[engine_id];
+ guc_client_free(dev_priv, *gc_ptr);
+ *gc_ptr = NULL;
}
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 5/5] drm/i915/guc: use for_each_engine_id() where appropriate
2016-07-19 12:59 ` [PATCH 5/5] drm/i915/guc: use for_each_engine_id() where appropriate Dave Gordon
@ 2016-07-19 15:09 ` Tvrtko Ursulin
0 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2016-07-19 15:09 UTC (permalink / raw)
To: Dave Gordon, intel-gfx
On 19/07/16 13:59, Dave Gordon wrote:
> Now that host structures are indexed by host engine-id rather than
> guc_id, we can usefully convert some for_each_engine() loops to use
> for_each_engine_id() and avoid multiple dereferences of engine->id.
>
> Also a few related tweaks to cache structure members locally wherever
> they're used more than once or twice, hopefully eliminating memory
> references.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++++--------
> drivers/gpu/drm/i915/i915_guc_submission.c | 22 +++++++++++++---------
> 2 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5cbb8ef..76918ab 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2541,6 +2541,7 @@ static void i915_guc_client_info(struct seq_file *m,
> struct i915_guc_client *client)
> {
> struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> uint64_t tot = 0;
>
> seq_printf(m, "\tPriority %d, GuC ctx index: %u, PD offset 0x%x\n",
> @@ -2555,11 +2556,11 @@ static void i915_guc_client_info(struct seq_file *m,
> seq_printf(m, "\tFailed doorbell: %u\n", client->b_fail);
> seq_printf(m, "\tLast submission result: %d\n", client->retcode);
>
> - for_each_engine(engine, dev_priv) {
> + for_each_engine_id(engine, dev_priv, id) {
> + u64 submissions = client->submissions[id];
> + tot += submissions;
> seq_printf(m, "\tSubmissions: %llu %s\n",
> - client->submissions[engine->id],
> - engine->name);
> - tot += client->submissions[engine->id];
> + submissions, engine->name);
> }
> seq_printf(m, "\tTotal: %llu\n", tot);
> }
> @@ -2604,11 +2605,11 @@ static int i915_guc_info(struct seq_file *m, void *data)
> seq_printf(m, "GuC last action error code: %d\n", guc.action_err);
>
> seq_printf(m, "\nGuC submissions:\n");
> - for_each_engine(engine, dev_priv) {
> + for_each_engine_id(engine, dev_priv, id) {
> + u64 submissions = guc.submissions[id];
> + total += submissions;
> seq_printf(m, "\t%-24s: %10llu, last seqno 0x%08x\n",
> - engine->name, guc.submissions[engine->id],
> - guc.last_seqno[engine->id]);
> - total += guc.submissions[engine->id];
> + engine->name, submissions, guc.last_seqno[id]);
> }
> seq_printf(m, "\t%s: %llu\n", "Total", total);
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 4daba77..5beed1b 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -342,7 +342,8 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
>
> for_each_engine_masked(engine, dev_priv, client->engines) {
> struct intel_context *ce = &ctx->engine[engine->id];
> - struct guc_execlist_context *lrc = &desc.lrc[engine->guc_id];
> + uint32_t guc_engine_id = engine->guc_id;
> + struct guc_execlist_context *lrc = &desc.lrc[guc_engine_id];
> struct drm_i915_gem_object *obj;
>
> /* TODO: We have a design issue to be solved here. Only when we
> @@ -361,7 +362,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
> gfx_addr = i915_gem_obj_ggtt_offset(ce->state);
> lrc->ring_lcra = gfx_addr + LRC_STATE_PN * PAGE_SIZE;
> lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) |
> - (engine->guc_id << GUC_ELC_ENGINE_OFFSET);
> + (guc_engine_id << GUC_ELC_ENGINE_OFFSET);
>
> obj = ce->ringbuf->obj;
> gfx_addr = i915_gem_obj_ggtt_offset(obj);
> @@ -371,7 +372,7 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
> lrc->ring_next_free_location = gfx_addr;
> lrc->ring_current_tail_pointer_value = 0;
>
> - desc.engines_used |= (1 << engine->guc_id);
> + desc.engines_used |= (1 << guc_engine_id);
> }
>
> DRM_DEBUG_DRIVER("Host engines 0x%x => GuC engines used 0x%x\n",
> @@ -461,6 +462,7 @@ static void guc_add_workqueue_item(struct i915_guc_client *gc,
> /* 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 intel_engine_cs *engine = rq->engine;
> struct guc_process_desc *desc;
> struct guc_wq_item *wqi;
> void *base;
> @@ -502,12 +504,11 @@ static void guc_add_workqueue_item(struct i915_guc_client *gc,
> /* Now fill in the 4-word work queue item */
> wqi->header = WQ_TYPE_INORDER |
> (wqi_len << WQ_LEN_SHIFT) |
> - (rq->engine->guc_id << WQ_TARGET_SHIFT) |
> + (engine->guc_id << WQ_TARGET_SHIFT) |
> WQ_NO_WCFLUSH_WAIT;
>
> /* The GuC wants only the low-order word of the context descriptor */
> - wqi->context_desc = (u32)intel_lr_context_descriptor(rq->ctx,
> - rq->engine);
> + wqi->context_desc = (u32)intel_lr_context_descriptor(rq->ctx, engine);
>
> wqi->ring_tail = tail << WQ_RING_TAIL_SHIFT;
> wqi->fence_id = rq->seqno;
> @@ -1035,11 +1036,14 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
> void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
> {
> struct intel_guc *guc = &dev_priv->guc;
> + struct i915_guc_client **gc_ptr;
> struct intel_engine_cs *engine;
> + enum intel_engine_id engine_id;
>
> - for_each_engine(engine, dev_priv) {
> - guc_client_free(dev_priv, guc->exec_clients[engine->id]);
> - guc->exec_clients[engine->id] = NULL;
> + for_each_engine_id(engine, dev_priv, engine_id) {
> + gc_ptr = &guc->exec_clients[engine_id];
> + guc_client_free(dev_priv, *gc_ptr);
> + *gc_ptr = NULL;
> }
> }
>
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915/guc: doorbell reset should avoid used doorbells
2016-07-19 12:59 [PATCH 1/5] drm/i915/guc: doorbell reset should avoid used doorbells Dave Gordon
` (3 preceding siblings ...)
2016-07-19 12:59 ` [PATCH 5/5] drm/i915/guc: use for_each_engine_id() where appropriate Dave Gordon
@ 2016-07-19 14:16 ` Patchwork
2016-07-19 15:07 ` Dave Gordon
2016-07-19 14:43 ` [PATCH 1/5] " Tvrtko Ursulin
5 siblings, 1 reply; 14+ messages in thread
From: Patchwork @ 2016-07-19 14:16 UTC (permalink / raw)
To: Dave Gordon; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/guc: doorbell reset should avoid used doorbells
URL : https://patchwork.freedesktop.org/series/10040/
State : failure
== Summary ==
Series 10040v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/10040/revisions/1/mbox
Test drv_module_reload_basic:
pass -> SKIP (ro-hsw-i3-4010u)
Test gem_sync:
Subgroup basic-store-each:
fail -> PASS (ro-bdw-i7-5600u)
Test kms_cursor_legacy:
Subgroup basic-cursor-vs-flip:
pass -> FAIL (ro-ilk1-i5-650)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-c-frame-sequence:
pass -> DMESG-WARN (fi-hsw-i7-4770k)
Subgroup suspend-read-crc-pipe-c:
pass -> SKIP (fi-hsw-i7-4770k)
Test prime_vgem:
Subgroup basic-fence-read:
fail -> PASS (ro-byt-n2820)
fi-hsw-i7-4770k total:243 pass:211 dwarn:1 dfail:0 fail:10 skip:21
fi-kbl-qkkr total:243 pass:177 dwarn:26 dfail:1 fail:10 skip:29
fi-skl-i7-6700k total:243 pass:208 dwarn:0 dfail:0 fail:9 skip:26
fi-snb-i7-2600 total:243 pass:193 dwarn:0 dfail:0 fail:10 skip:40
ro-bdw-i5-5250u total:244 pass:217 dwarn:4 dfail:0 fail:10 skip:13
ro-bdw-i7-5557U total:244 pass:219 dwarn:1 dfail:0 fail:9 skip:15
ro-bdw-i7-5600u total:244 pass:202 dwarn:0 dfail:0 fail:10 skip:32
ro-bsw-n3050 total:218 pass:173 dwarn:0 dfail:0 fail:2 skip:42
ro-byt-n2820 total:244 pass:195 dwarn:0 dfail:0 fail:11 skip:38
ro-hsw-i3-4010u total:244 pass:208 dwarn:0 dfail:0 fail:11 skip:25
ro-hsw-i7-4770r total:244 pass:209 dwarn:0 dfail:0 fail:11 skip:24
ro-ilk1-i5-650 total:239 pass:169 dwarn:0 dfail:0 fail:12 skip:58
ro-ivb-i7-3770 total:244 pass:200 dwarn:0 dfail:0 fail:11 skip:33
ro-skl3-i5-6260u total:244 pass:221 dwarn:1 dfail:0 fail:10 skip:12
ro-snb-i7-2620M total:244 pass:190 dwarn:0 dfail:0 fail:12 skip:42
fi-skl-i5-6260u failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_1532/
635555a drm-intel-nightly: 2016y-07m-19d-13h-02m-39s UTC integration manifest
207ec9d drm/i915/guc: use for_each_engine_id() where appropriate
5982c52 drm/i915/guc: add engine mask to GuC client & pass to GuC
b1f5991 drm/i915/guc: use a separate GuC client for each engine
89623f4 drm/i915/guc: refactor guc_init_doorbell_hw()
b9c0a41 drm/i915/guc: doorbell reset should avoid used doorbells
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915/guc: doorbell reset should avoid used doorbells
2016-07-19 14:16 ` ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915/guc: doorbell reset should avoid used doorbells Patchwork
@ 2016-07-19 15:07 ` Dave Gordon
0 siblings, 0 replies; 14+ messages in thread
From: Dave Gordon @ 2016-07-19 15:07 UTC (permalink / raw)
To: intel-gfx
On 19/07/16 15:16, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [1/5] drm/i915/guc: doorbell reset should avoid used doorbells
> URL : https://patchwork.freedesktop.org/series/10040/
> State : failure
>
> == Summary ==
>
> Series 10040v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/10040/revisions/1/mbox
>
> Test drv_module_reload_basic:
> pass -> SKIP (ro-hsw-i3-4010u)
> Test gem_sync:
> Subgroup basic-store-each:
> fail -> PASS (ro-bdw-i7-5600u)
> Test kms_cursor_legacy:
> Subgroup basic-cursor-vs-flip:
> pass -> FAIL (ro-ilk1-i5-650)
Wibble? The test log for this says:
+ Results for igt@kms_cursor_legacy@basic-flip-vs-cursor
+ Result: pass
+
+ IGT-Version: 1.15-g4d03467 (x86_64) (Linux:
4.7.0-rc7-gfxbench-RO_Patchwork_1532+ x86_64)
+ Test requirement not met in function __real_main427, file
kms_cursor_legacy.c:448:
+ Test requirement: !(n >= data.resources->count_crtcs)
+ Subtest basic-flip-vs-cursor: SUCCESS (1.156s)
+
+ Command /opt/igt/tests/kms_cursor_legacy --run-subtest
basic-flip-vs-cursor
So whatever that unfulfilled test requirement is, the result should be PASS.
> Test kms_pipe_crc_basic:
> Subgroup read-crc-pipe-c-frame-sequence:
> pass -> DMESG-WARN (fi-hsw-i7-4770k)
[ 436.687908] [drm:drm_edid_block_valid] *ERROR* EDID checksum is
invalid, remainder is 122
[ 436.688345] [drm:drm_edid_block_valid] *ERROR* EDID checksum is
invalid, remainder is 122
which looks like
https://bugzilla.kernel.org/show_bug.cgi?id=85951
[hsw] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid,
remainder is 46
(which was closed as unreproducible)
.Dave.
> Subgroup suspend-read-crc-pipe-c:
> pass -> SKIP (fi-hsw-i7-4770k)
> Test prime_vgem:
> Subgroup basic-fence-read:
> fail -> PASS (ro-byt-n2820)
>
> fi-hsw-i7-4770k total:243 pass:211 dwarn:1 dfail:0 fail:10 skip:21
> fi-kbl-qkkr total:243 pass:177 dwarn:26 dfail:1 fail:10 skip:29
> fi-skl-i7-6700k total:243 pass:208 dwarn:0 dfail:0 fail:9 skip:26
> fi-snb-i7-2600 total:243 pass:193 dwarn:0 dfail:0 fail:10 skip:40
> ro-bdw-i5-5250u total:244 pass:217 dwarn:4 dfail:0 fail:10 skip:13
> ro-bdw-i7-5557U total:244 pass:219 dwarn:1 dfail:0 fail:9 skip:15
> ro-bdw-i7-5600u total:244 pass:202 dwarn:0 dfail:0 fail:10 skip:32
> ro-bsw-n3050 total:218 pass:173 dwarn:0 dfail:0 fail:2 skip:42
> ro-byt-n2820 total:244 pass:195 dwarn:0 dfail:0 fail:11 skip:38
> ro-hsw-i3-4010u total:244 pass:208 dwarn:0 dfail:0 fail:11 skip:25
> ro-hsw-i7-4770r total:244 pass:209 dwarn:0 dfail:0 fail:11 skip:24
> ro-ilk1-i5-650 total:239 pass:169 dwarn:0 dfail:0 fail:12 skip:58
> ro-ivb-i7-3770 total:244 pass:200 dwarn:0 dfail:0 fail:11 skip:33
> ro-skl3-i5-6260u total:244 pass:221 dwarn:1 dfail:0 fail:10 skip:12
> ro-snb-i7-2620M total:244 pass:190 dwarn:0 dfail:0 fail:12 skip:42
> fi-skl-i5-6260u failed to connect after reboot
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1532/
>
> 635555a drm-intel-nightly: 2016y-07m-19d-13h-02m-39s UTC integration manifest
> 207ec9d drm/i915/guc: use for_each_engine_id() where appropriate
> 5982c52 drm/i915/guc: add engine mask to GuC client & pass to GuC
> b1f5991 drm/i915/guc: use a separate GuC client for each engine
> 89623f4 drm/i915/guc: refactor guc_init_doorbell_hw()
> b9c0a41 drm/i915/guc: doorbell reset should avoid used doorbells
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] drm/i915/guc: doorbell reset should avoid used doorbells
2016-07-19 12:59 [PATCH 1/5] drm/i915/guc: doorbell reset should avoid used doorbells Dave Gordon
` (4 preceding siblings ...)
2016-07-19 14:16 ` ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915/guc: doorbell reset should avoid used doorbells Patchwork
@ 2016-07-19 14:43 ` Tvrtko Ursulin
5 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2016-07-19 14:43 UTC (permalink / raw)
To: Dave Gordon, intel-gfx
On 19/07/16 13:59, Dave Gordon wrote:
> guc_init_doorbell_hw() borrows the (currently single) GuC client to use
> in reinitialising ALL the doorbell registers (as the hardware doesn't
> reset them when the GuC is reset). As a prerequisite for accommodating
> multiple clients, it should only reset doorbells that are supposed to be
> disabled, avoiding those that are marked as in use by any client.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 2112e02..d8402e4 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -699,7 +699,7 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
> }
>
> /*
> - * Borrow the first client to set up & tear down every doorbell
> + * Borrow the first client to set up & tear down each unused doorbell
> * in turn, to ensure that all doorbell h/w is (re)initialised.
> */
> static void guc_init_doorbell_hw(struct intel_guc *guc)
> @@ -715,6 +715,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
> i915_reg_t drbreg = GEN8_DRBREGL(i);
> u32 value = I915_READ(drbreg);
>
> + if (test_bit(i, guc->doorbell_bitmap))
> + continue;
> +
> err = guc_update_doorbell_id(guc, client, i);
>
> /* Report update failure or unexpectedly active doorbell */
> @@ -733,6 +736,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
> i915_reg_t drbreg = GEN8_DRBREGL(i);
> u32 value = I915_READ(drbreg);
>
> + if (test_bit(i, guc->doorbell_bitmap))
> + continue;
> +
> if (i != db_id && (value & GUC_DOORBELL_ENABLED))
> DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) finally 0x%x\n",
> i, drbreg.reg, value);
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread