On 25/07/16 08:44, Chris Wilson wrote:
If we rewrite the I915_WRITE_TAIL specialisation for the legacy
ringbuffer as submitting the request onto the ringbuffer, we can unify
the vfunc with both execlists and GuC in the next patch.

Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c |  8 ++---
 drivers/gpu/drm/i915/intel_lrc.c        |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 53 +++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  3 +-
 4 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c 
b/drivers/gpu/drm/i915/i915_gem_request.c
index 1c185e293bf0..8814e9c5266b 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -467,15 +467,13 @@ void __i915_add_request(struct drm_i915_gem_request 
*request,
         */
        request->postfix = ring->tail;

-       if (i915.enable_execlists) {
+       if (i915.enable_execlists)
                ret = engine->emit_request(request);
-       } else {
+       else
                ret = engine->add_request(request);
-
-               request->tail = ring->tail;
-       }
        /* Not allowed to fail! */
        WARN(ret, "emit|add_request failed: %d!\n", ret);
+
        /* Sanity check that the reserved size was large enough. */
        ret = ring->tail - request_start;
        if (ret < 0)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 567d94de3300..250edb2bcef7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -373,7 +373,7 @@ static void execlists_update_context(struct 
drm_i915_gem_request *rq)
        struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt;
        uint32_t *reg_state = rq->ctx->engine[engine->id].lrc_reg_state;

-       reg_state[CTX_RING_TAIL+1] = rq->tail;
+       reg_state[CTX_RING_TAIL+1] = rq->tail % (rq->ring->size - 1);

mod ringsize-1 ?

Surely tail % ringsize, or tail & (ringsize-1).

But it's redundant anyway, rq->tail cannot exceed ring->size,
so the original code was correct.

        /* True 32b PPGTT with dynamic page allocation: update PDP
         * registers and point the unallocated PDPs to scratch page.
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0a2e25e67450..3e1049c972e0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -58,14 +58,6 @@ void intel_ring_update_space(struct intel_ring *ring)
                                         ring->tail, ring->size);
 }

-static void __intel_engine_submit(struct intel_engine_cs *engine)
-{
-       struct intel_ring *ring = engine->buffer;
-
-       ring->tail &= ring->size - 1;
-       engine->write_tail(engine, ring->tail);
-}
-
 static int
 gen2_render_ring_flush(struct drm_i915_gem_request *req,
                       u32      invalidate_domains,
@@ -421,13 +413,6 @@ gen8_render_ring_flush(struct drm_i915_gem_request *req,
        return gen8_emit_pipe_control(req, flags, scratch_addr);
 }

-static void ring_write_tail(struct intel_engine_cs *engine,
-                           u32 value)
-{
-       struct drm_i915_private *dev_priv = engine->i915;
-       I915_WRITE_TAIL(engine, value);
-}
-
 u64 intel_engine_get_active_head(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
@@ -541,7 +526,7 @@ static bool stop_ring(struct intel_engine_cs *engine)

        I915_WRITE_CTL(engine, 0);
        I915_WRITE_HEAD(engine, 0);
-       engine->write_tail(engine, 0);
+       I915_WRITE_TAIL(engine, 0);

        if (!IS_GEN2(dev_priv)) {
                (void)I915_READ_CTL(engine);
@@ -1482,7 +1467,10 @@ gen6_add_request(struct drm_i915_gem_request *req)
        intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT);
        intel_ring_emit(ring, req->fence.seqno);
        intel_ring_emit(ring, MI_USER_INTERRUPT);
-       __intel_engine_submit(engine);
+       intel_ring_advance(ring);
+
+       req->tail = ring->tail;
+       engine->submit_request(req);

        return 0;
 }
@@ -1512,7 +1500,9 @@ gen8_render_add_request(struct drm_i915_gem_request *req)
        intel_ring_emit(ring, 0);
        intel_ring_emit(ring, MI_USER_INTERRUPT);
        intel_ring_emit(ring, MI_NOOP);
-       __intel_engine_submit(engine);
+
+       req->tail = ring->tail;
+       engine->submit_request(req);

        return 0;
 }
@@ -1731,11 +1721,22 @@ i9xx_add_request(struct drm_i915_gem_request *req)
        intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT);
        intel_ring_emit(ring, req->fence.seqno);
        intel_ring_emit(ring, MI_USER_INTERRUPT);
-       __intel_engine_submit(req->engine);
+       intel_ring_advance(ring);
+
+       req->tail = ring->tail;
+       req->engine->submit_request(req);

        return 0;
 }

+static void i9xx_submit_request(struct drm_i915_gem_request *request)
+{
+       struct drm_i915_private *dev_priv = request->i915;
+
+       I915_WRITE_TAIL(request->engine,
+                       request->tail % (request->ring->size - 1));
+}

Another "modulo ringsize-1" here too.

.Dave.

 static void
 gen6_irq_enable(struct intel_engine_cs *engine)
 {
@@ -2494,10 +2495,9 @@ void intel_engine_init_seqno(struct intel_engine_cs 
*engine, u32 seqno)
        rcu_read_unlock();
 }

-static void gen6_bsd_ring_write_tail(struct intel_engine_cs *engine,
-                                    u32 value)
+static void gen6_bsd_submit_request(struct drm_i915_gem_request *request)
 {
-       struct drm_i915_private *dev_priv = engine->i915;
+       struct drm_i915_private *dev_priv = request->i915;

        intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);

@@ -2521,8 +2521,9 @@ static void gen6_bsd_ring_write_tail(struct 
intel_engine_cs *engine,
                DRM_ERROR("timed out waiting for the BSD ring to wake up\n");

        /* Now that the ring is fully powered up, update the tail */
-       I915_WRITE_FW(RING_TAIL(engine->mmio_base), value);
-       POSTING_READ_FW(RING_TAIL(engine->mmio_base));
+       I915_WRITE_FW(RING_TAIL(request->engine->mmio_base),
+                     request->tail % (request->ring->size - 1));
+       POSTING_READ_FW(RING_TAIL(request->engine->mmio_base));

        /* Let the ring send IDLE messages to the GT again,
         * and so let it sleep to conserve power when idle.
@@ -2828,7 +2829,7 @@ static void intel_ring_default_vfuncs(struct 
drm_i915_private *dev_priv,
                                      struct intel_engine_cs *engine)
 {
        engine->init_hw = init_ring_common;
-       engine->write_tail = ring_write_tail;
+       engine->submit_request = i9xx_submit_request;

        engine->add_request = i9xx_add_request;
        if (INTEL_GEN(dev_priv) >= 6)
@@ -2912,7 +2913,7 @@ int intel_init_bsd_ring_buffer(struct intel_engine_cs 
*engine)
        if (INTEL_GEN(dev_priv) >= 6) {
                /* gen6 bsd needs a special wa for tail updates */
                if (IS_GEN6(dev_priv))
-                       engine->write_tail = gen6_bsd_ring_write_tail;
+                       engine->submit_request = gen6_bsd_submit_request;
                engine->emit_flush = gen6_bsd_ring_flush;
                if (INTEL_GEN(dev_priv) < 8)
                        engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 13b816f2d264..5428a3c288d5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -204,8 +204,6 @@ struct intel_engine_cs {

        int             (*init_context)(struct drm_i915_gem_request *req);

-       void            (*write_tail)(struct intel_engine_cs *engine,
-                                     u32 value);
        int             (*add_request)(struct drm_i915_gem_request *req);
        /* Some chipsets are not quite as coherent as advertised and need
         * an expensive kick to force a true read of the up-to-date seqno.
@@ -294,6 +292,7 @@ struct intel_engine_cs {
 #define I915_DISPATCH_SECURE 0x1
 #define I915_DISPATCH_PINNED 0x2
 #define I915_DISPATCH_RS     0x4
+       void            (*submit_request)(struct drm_i915_gem_request *req);

        /**
         * List of objects currently involved in rendering from the


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to