On 05/10/17 09:00, Michał Winiarski wrote:
On Thu, Oct 05, 2017 at 03:22:59PM +0000, Daniele Ceraolo Spurio wrote:
On 05/10/17 02:13, Michał Winiarski wrote:
From: Dave Gordon <david.s.gordon@xxxxxxxxx>
This second client is created with priority KMD_HIGH, and marked
as preemptive. This will allow us to request preemption using GuC actions.
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Jeff Mcgee <jeff.mcgee@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 ++++
drivers/gpu/drm/i915/i915_guc_submission.c | 32 ++++++++++++++++++++++++++----
drivers/gpu/drm/i915/intel_uc.h | 1 +
3 files changed, 33 insertions(+), 4 deletions(-)
[SNIP]
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 10e8f0ed02e4..c6c6f8513bbf 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -107,6 +107,7 @@ struct intel_guc {
struct ida stage_ids;
struct i915_guc_client *execbuf_client;
+ struct i915_guc_client *preempt_client;
DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
uint32_t db_cacheline; /* Cyclic counter mod pagesize */
I think you also need to update guc_init_doorbell_hw() to handle the new
client. The comment in there says:
/* Now for every client (and not only execbuf_client) make sure their
* doorbells are known by the GuC */
But I don't want the doorbell for preempt_client...
I'm not submitting anything 'directly' using this client (just indirectly -
through preempt action).
Are you sure we need the doorbell?
Last time I tried to refactor GuC submission to use doorbell per engine, I got
the info that I should *avoid* allocating multiple doorbells.
-Michał
Don't you still get a doorbell from guc_client_alloc()? or am I missing
something?
One of the issues that guc_init_doorbell_hw() tries to solve is that
after a GuC reset the doorbell HW is not cleaned up, so you can
potentially have an active doorbell that GuC doesn't know about, which
could lead to a failure if we try to release that doorbell later on. By
doing the H2G we are sure that HW and GuC FW are in sync and I think
this should apply to the doorbell in the preempt client as well.
The problem with having many doorbells is that they add a slight delay
when waking the power well (not sure about the exact details) and AFAIK
this happens even if you don't ring them. For this reason it is true
that we want to keep the number of doorbells limited, but having 2 in
total shouldn't be an issue. However, if we really don't need the
doorbell then we should probably modify guc_client_alloc() to skip the
doorbell allocation for the preempt client, but that could lead to a
bigger rework to remove the assumption that each client has a doorbell.
Daniele
Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
