On 27/04/16 19:03, Dave Gordon wrote:
Rather than wait to see whether more space becomes available in the GuC
submission workqueue, we can just return -EAGAIN and let the caller try
again in a little while. This gets rid of an uninterruptable sleep in
the polling code :)
We'll also add a counter to the GuC client statistics, to see how often
we find the WQ full.
Signed-off-by: Dave Gordon <[email protected]>
---
drivers/gpu/drm/i915/i915_debugfs.c | 1 +
drivers/gpu/drm/i915/i915_guc_submission.c | 16 +++++-----------
drivers/gpu/drm/i915/intel_guc.h | 8 ++++----
3 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 8b8d6f0..1024947 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2509,6 +2509,7 @@ static void i915_guc_client_info(struct seq_file *m,
seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
client->wq_size, client->wq_offset, client->wq_tail);
+ seq_printf(m, "\tWork queue full: %u\n", client->no_wq_space);
seq_printf(m, "\tFailed to queue: %u\n", client->q_fail);
seq_printf(m, "\tFailed doorbell: %u\n", client->b_fail);
seq_printf(m, "\tLast submission result: %d\n", client->retcode);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 66af5ce..6626eff 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -453,27 +453,21 @@ 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 size = sizeof(struct guc_wq_item);
+ const size_t wqi_size = sizeof(struct guc_wq_item);
struct i915_guc_client *gc = request->i915->guc.execbuf_client;
struct guc_process_desc *desc;
- int ret = -ETIMEDOUT, timeout_counter = 200;
if (!gc)
return 0;
Not part of this patch but spotted it - can this really happen, and if
so, does it warrant a WARN_ONCE, GEM_BUG_ON or something?
desc = gc->client_base + gc->proc_desc_offset;
- while (timeout_counter-- > 0) {
- if (CIRC_SPACE(gc->wq_tail, desc->head, gc->wq_size) >= size) {
- ret = 0;
- break;
- }
+ if (CIRC_SPACE(gc->wq_tail, desc->head, gc->wq_size) >= wqi_size)
+ return 0;
Could stick a likely here to lay out the code for the expected case but
it could be just OCD. :)
- if (timeout_counter)
- usleep_range(1000, 2000);
- };
+ gc->no_wq_space += 1;
- return ret;
+ return -EAGAIN;
I suppose it is OK to make userspace pay the cost since this should be
extremely rare, correct?
}
static int guc_add_workqueue_item(struct i915_guc_client *gc,
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index b37c731..436f2d6 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -73,10 +73,10 @@ struct i915_guc_client {
/* GuC submission statistics & status */
uint64_t submissions[GUC_MAX_ENGINES_NUM];
- uint32_t q_fail;
- uint32_t b_fail;
- int retcode;
- int spare; /* pad to 32 DWords */
+ uint32_t no_wq_space; /* Space pre-check failed */
+ uint32_t q_fail; /* Failed to queue (MBZ) */
+ uint32_t b_fail; /* Doorbell failure (MBZ) */
+ int retcode; /* Result of last guc_submit() */
};
enum intel_guc_fw_status {
Reviewed-by: Tvrtko Ursulin <[email protected]>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
