diff mbox

[7/7] drm/i915: Mark elsps submitted when they are pushed to hw

Message ID 1436170165-3100-1-git-send-email-mika.kuoppala@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kuoppala July 6, 2015, 8:09 a.m. UTC
Now when we have requests this deep on call chain, we can mark
the elsp being submitted when it actually is. Remove temp variable
and readjust commenting to more closely fit to the code.

v2: Avoid tmp variable and reduce number of writes (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

Comments

Chris Wilson July 6, 2015, 9:25 a.m. UTC | #1
On Mon, Jul 06, 2015 at 11:09:25AM +0300, Mika Kuoppala wrote:
> Now when we have requests this deep on call chain, we can mark
> the elsp being submitted when it actually is. Remove temp variable
> and readjust commenting to more closely fit to the code.
> 
> v2: Avoid tmp variable and reduce number of writes (Chris)
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Looks cleaner to me,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Chris Wilson July 6, 2015, 9:32 a.m. UTC | #2
On Mon, Jul 06, 2015 at 10:25:17AM +0100, Chris Wilson wrote:
> On Mon, Jul 06, 2015 at 11:09:25AM +0300, Mika Kuoppala wrote:
> > Now when we have requests this deep on call chain, we can mark
> > the elsp being submitted when it actually is. Remove temp variable
> > and readjust commenting to more closely fit to the code.
> > 
> > v2: Avoid tmp variable and reduce number of writes (Chris)
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Looks cleaner to me,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

In fact, nothing in the series scared me and with the two minor changes,
might as well apply that r-b to the whole series. For the benefit of
others, can you repost the contracted series with my r-b applied?
-Chris
Daniel Vetter July 6, 2015, 2:52 p.m. UTC | #3
On Mon, Jul 06, 2015 at 10:32:11AM +0100, Chris Wilson wrote:
> On Mon, Jul 06, 2015 at 10:25:17AM +0100, Chris Wilson wrote:
> > On Mon, Jul 06, 2015 at 11:09:25AM +0300, Mika Kuoppala wrote:
> > > Now when we have requests this deep on call chain, we can mark
> > > the elsp being submitted when it actually is. Remove temp variable
> > > and readjust commenting to more closely fit to the code.
> > > 
> > > v2: Avoid tmp variable and reduce number of writes (Chris)
> > > 
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > 
> > Looks cleaner to me,
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> In fact, nothing in the series scared me and with the two minor changes,
> might as well apply that r-b to the whole series. For the benefit of
> others, can you repost the contracted series with my r-b applied?

Yeah maybe this was a bit too much splitting but I didn't find it too much
so just went ahead and applied it all.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 468f569..de35db6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -300,31 +300,29 @@  static void execlists_elsp_write(struct drm_i915_gem_request *rq0,
 	struct intel_engine_cs *ring = rq0->ring;
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint64_t temp = 0;
-	uint32_t desc[4];
+	uint64_t desc[2];
 
-	/* XXX: You must always write both descriptors in the order below. */
-	if (rq1)
-		temp = execlists_ctx_descriptor(rq1);
-	else
-		temp = 0;
-	desc[1] = (u32)(temp >> 32);
-	desc[0] = (u32)temp;
+	if (rq1) {
+		desc[1] = execlists_ctx_descriptor(rq1);
+		rq1->elsp_submitted++;
+	} else {
+		desc[1] = 0;
+	}
 
-	temp = execlists_ctx_descriptor(rq0);
-	desc[3] = (u32)(temp >> 32);
-	desc[2] = (u32)temp;
+	desc[0] = execlists_ctx_descriptor(rq0);
+	rq0->elsp_submitted++;
 
+	/* You must always write both descriptors in the order below. */
 	spin_lock(&dev_priv->uncore.lock);
 	intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
-	I915_WRITE_FW(RING_ELSP(ring), desc[1]);
-	I915_WRITE_FW(RING_ELSP(ring), desc[0]);
-	I915_WRITE_FW(RING_ELSP(ring), desc[3]);
+	I915_WRITE_FW(RING_ELSP(ring), upper_32_bits(desc[1]));
+	I915_WRITE_FW(RING_ELSP(ring), lower_32_bits(desc[1]));
 
+	I915_WRITE_FW(RING_ELSP(ring), upper_32_bits(desc[0]));
 	/* The context is automatically loaded after the following */
-	I915_WRITE_FW(RING_ELSP(ring), desc[2]);
+	I915_WRITE_FW(RING_ELSP(ring), lower_32_bits(desc[0]));
 
-	/* ELSP is a wo register, so use another nearby reg for posting instead */
+	/* ELSP is a wo register, use another nearby reg for posting */
 	POSTING_READ_FW(RING_EXECLIST_STATUS(ring));
 	intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
 	spin_unlock(&dev_priv->uncore.lock);
@@ -433,10 +431,6 @@  static void execlists_context_unqueue(struct intel_engine_cs *ring)
 	WARN_ON(req1 && req1->elsp_submitted);
 
 	execlists_submit_requests(req0, req1);
-
-	req0->elsp_submitted++;
-	if (req1)
-		req1->elsp_submitted++;
 }
 
 static bool execlists_check_remove_request(struct intel_engine_cs *ring,