On 16/08/16 17:18, Chris Wilson wrote:
On Tue, Aug 16, 2016 at 05:04:21PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Build the legacy semaphore initialisation array using the engine
hardware ids instead of driver internal ones. This makes the
static array size dependent only on the number of gen6 semaphore
engines.
Also makes the per-engine semaphore wait and signal tables
hardware id indexed saving some more space.
v2: Refactor I915_GEN6_NUM_ENGINES to GEN6_SEMAPHORE_LAST. (Chris Wilson)
v3: More polish. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 55 +++++++++++++++++----------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 7 +++--
2 files changed, 34 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index fa22bd87bab0..12703ea27259 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1337,8 +1337,7 @@ static int gen6_signal(struct drm_i915_gem_request *req)
{
struct intel_ring *ring = req->ring;
struct drm_i915_private *dev_priv = req->i915;
- struct intel_engine_cs *useless;
- enum intel_engine_id id;
+ struct intel_engine_cs *engine;
int ret, num_rings;
num_rings = INTEL_INFO(dev_priv)->num_rings;
@@ -1346,9 +1345,13 @@ static int gen6_signal(struct drm_i915_gem_request *req)
if (ret)
return ret;
- for_each_engine_id(useless, dev_priv, id) {
- i915_reg_t mbox_reg = req->engine->semaphore.mbox.signal[id];
+ for_each_engine(engine, dev_priv) {
+ i915_reg_t mbox_reg;
+
+ if (!(BIT(engine->hw_id) & GEN6_SEMAPHORES_MASK))
+ continue;
Later on, it would be nice if this used
for_each_engine_masked()
instead (presupposing we have an efficient iterator for a sparse mask).
The issue is in defining GEN6_SEMAPHORES_MASK
#define GEN6_SEMAPHORES_MASK \
(RENDER_RING |
BSD_RING |
BSD2_RING |
BLT_RING |
VECS_RING)
Defnitely pencil that in for when otherwise we would iterate over 10
empty slots in dev_priv->engines[].
I'm thinking that we should make for_each_engine_mask() the default, say
for_each_engine(engine, dev_priv, ALL_ENGINES, id)
Or add an initialized engine array to dev_priv, in addition to the
existing map for best of both worlds.
That would prevent churn in for_each_engine users.
Or just give up and add something smarter, like a typdef
intel_engine_it, to each of them and be done with churn once for all.
But we said there aren't any iterators on fast paths anyway so I would
rather we chose to do nothing. :)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
