[PATCH v5 1/3] HAX enable guc submission for CI

[PATCH v5 1/3] HAX enable guc submission for CI

[Chris Wilson]

---
 drivers/gpu/drm/i915/i915_params.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 2e9645e6555a..8fa96edddf9f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 1,
+	.enable_guc_submission = 1,
 	.guc_log_level = -1,
 	.enable_dp_mst = true,
 	.inject_load_failure = 0,
-- 
2.11.0

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

[PATCH v5 2/3] drm/i915/guc: Make wq_lock irq-safe

[Chris Wilson]

Following the use of dma_fence_signal() from within our interrupt
handler, we need to make guc->wq_lock also irq-safe. This was done
previously as part of the guc scheduler patch (which also started
mixing our fences with the interrupt handler), but is now required to
fix the current guc submission backend.

v4: Document that __i915_guc_submit is always under an irq disabled
section
v5: Move wq_rsvd adjustment to its own function

Fixes: 67b807a89230 ("drm/i915: Delay disabling the user interrupt for breadcrumbs")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index beec88a30347..d6a6cf2540a1 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -348,7 +348,7 @@ int i915_guc_wq_reserve(struct drm_i915_gem_request *request)
 	u32 freespace;
 	int ret;
 
-	spin_lock(&client->wq_lock);
+	spin_lock_irq(&client->wq_lock);
 	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
 	freespace -= client->wq_rsvd;
 	if (likely(freespace >= wqi_size)) {
@@ -358,21 +358,27 @@ int i915_guc_wq_reserve(struct drm_i915_gem_request *request)
 		client->no_wq_space++;
 		ret = -EAGAIN;
 	}
-	spin_unlock(&client->wq_lock);
+	spin_unlock_irq(&client->wq_lock);
 
 	return ret;
 }
 
+static void guc_client_update_wq_rsvd(struct i915_guc_client *client, int size)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&client->wq_lock, flags);
+	client->wq_rsvd += size;
+	spin_unlock_irqrestore(&client->wq_lock, flags);
+}
+
 void i915_guc_wq_unreserve(struct drm_i915_gem_request *request)
 {
-	const size_t wqi_size = sizeof(struct guc_wq_item);
+	const int wqi_size = sizeof(struct guc_wq_item);
 	struct i915_guc_client *client = request->i915->guc.execbuf_client;
 
 	GEM_BUG_ON(READ_ONCE(client->wq_rsvd) < wqi_size);
-
-	spin_lock(&client->wq_lock);
-	client->wq_rsvd -= wqi_size;
-	spin_unlock(&client->wq_lock);
+	guc_client_update_wq_rsvd(client, -wqi_size);
 }
 
 /* Construct a Work Item and append it to the GuC's Work Queue */
@@ -511,6 +517,9 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	struct i915_guc_client *client = guc->execbuf_client;
 	int b_ret;
 
+	/* We are always called with irqs disabled */
+	GEM_BUG_ON(!irqs_disabled());
+
 	spin_lock(&client->wq_lock);
 	guc_wq_item_append(client, rq);
 
@@ -945,16 +954,19 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 
 	/* Take over from manual control of ELSP (execlists) */
 	for_each_engine(engine, dev_priv, id) {
+		const int wqi_size = sizeof(struct guc_wq_item);
 		struct drm_i915_gem_request *rq;
 
 		engine->submit_request = i915_guc_submit;
 		engine->schedule = NULL;
 
 		/* Replay the current set of previously submitted requests */
+		spin_lock_irq(&engine->timeline->lock);
 		list_for_each_entry(rq, &engine->timeline->requests, link) {
-			client->wq_rsvd += sizeof(struct guc_wq_item);
+			guc_client_update_wq_rsvd(client, wqi_size);
 			__i915_guc_submit(rq);
 		}
+		spin_unlock_irq(&engine->timeline->lock);
 	}
 
 	return 0;
-- 
2.11.0

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

[PATCH v5 3/3] drm/i915/guc: Reorder __i915_guc_submit to reduce spinlock holdtime

[Chris Wilson]

A couple of operations, the flushes and the tracepoint, do not require
serialisation by client->wq_lock, so move them before we take it.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index d6a6cf2540a1..7b535a32fc27 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -517,18 +517,18 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	struct i915_guc_client *client = guc->execbuf_client;
 	int b_ret;
 
-	/* We are always called with irqs disabled */
-	GEM_BUG_ON(!irqs_disabled());
-
-	spin_lock(&client->wq_lock);
-	guc_wq_item_append(client, rq);
-
 	/* WA to flush out the pending GMADR writes to ring buffer. */
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
 		POSTING_READ_FW(GUC_STATUS);
 
 	trace_i915_gem_request_in(rq, 0);
 
+	/* We are always called with irqs disabled */
+	GEM_BUG_ON(!irqs_disabled());
+
+	spin_lock(&client->wq_lock);
+
+	guc_wq_item_append(client, rq);
 	b_ret = guc_ring_doorbell(client);
 
 	client->submissions[engine_id] += 1;
@@ -538,6 +538,7 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 
 	guc->submissions[engine_id] += 1;
 	guc->last_seqno[engine_id] = rq->global_seqno;
+
 	spin_unlock(&client->wq_lock);
 }
 
-- 
2.11.0

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

Re: [PATCH v5 2/3] drm/i915/guc: Make wq_lock irq-safe

[Tvrtko Ursulin]

On 28/02/2017 11:28, Chris Wilson wrote:
> Following the use of dma_fence_signal() from within our interrupt
> handler, we need to make guc->wq_lock also irq-safe. This was done
> previously as part of the guc scheduler patch (which also started
> mixing our fences with the interrupt handler), but is now required to
> fix the current guc submission backend.
>
> v4: Document that __i915_guc_submit is always under an irq disabled
> section
> v5: Move wq_rsvd adjustment to its own function
>
> Fixes: 67b807a89230 ("drm/i915: Delay disabling the user interrupt for breadcrumbs")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index beec88a30347..d6a6cf2540a1 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -348,7 +348,7 @@ int i915_guc_wq_reserve(struct drm_i915_gem_request *request)
>  	u32 freespace;
>  	int ret;
>
> -	spin_lock(&client->wq_lock);
> +	spin_lock_irq(&client->wq_lock);
>  	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
>  	freespace -= client->wq_rsvd;
>  	if (likely(freespace >= wqi_size)) {
> @@ -358,21 +358,27 @@ int i915_guc_wq_reserve(struct drm_i915_gem_request *request)
>  		client->no_wq_space++;
>  		ret = -EAGAIN;
>  	}
> -	spin_unlock(&client->wq_lock);
> +	spin_unlock_irq(&client->wq_lock);
>
>  	return ret;
>  }
>
> +static void guc_client_update_wq_rsvd(struct i915_guc_client *client, int size)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&client->wq_lock, flags);
> +	client->wq_rsvd += size;
> +	spin_unlock_irqrestore(&client->wq_lock, flags);
> +}
> +
>  void i915_guc_wq_unreserve(struct drm_i915_gem_request *request)
>  {
> -	const size_t wqi_size = sizeof(struct guc_wq_item);
> +	const int wqi_size = sizeof(struct guc_wq_item);
>  	struct i915_guc_client *client = request->i915->guc.execbuf_client;
>
>  	GEM_BUG_ON(READ_ONCE(client->wq_rsvd) < wqi_size);
> -
> -	spin_lock(&client->wq_lock);
> -	client->wq_rsvd -= wqi_size;
> -	spin_unlock(&client->wq_lock);
> +	guc_client_update_wq_rsvd(client, -wqi_size);
>  }
>
>  /* Construct a Work Item and append it to the GuC's Work Queue */
> @@ -511,6 +517,9 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
>  	struct i915_guc_client *client = guc->execbuf_client;
>  	int b_ret;
>
> +	/* We are always called with irqs disabled */
> +	GEM_BUG_ON(!irqs_disabled());
> +
>  	spin_lock(&client->wq_lock);
>  	guc_wq_item_append(client, rq);
>
> @@ -945,16 +954,19 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
>
>  	/* Take over from manual control of ELSP (execlists) */
>  	for_each_engine(engine, dev_priv, id) {
> +		const int wqi_size = sizeof(struct guc_wq_item);
>  		struct drm_i915_gem_request *rq;
>
>  		engine->submit_request = i915_guc_submit;
>  		engine->schedule = NULL;
>
>  		/* Replay the current set of previously submitted requests */
> +		spin_lock_irq(&engine->timeline->lock);
>  		list_for_each_entry(rq, &engine->timeline->requests, link) {
> -			client->wq_rsvd += sizeof(struct guc_wq_item);
> +			guc_client_update_wq_rsvd(client, wqi_size);
>  			__i915_guc_submit(rq);
>  		}
> +		spin_unlock_irq(&engine->timeline->lock);
>  	}
>
>  	return 0;
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH v5 3/3] drm/i915/guc: Reorder __i915_guc_submit to reduce spinlock holdtime

[Tvrtko Ursulin]

On 28/02/2017 11:28, Chris Wilson wrote:
> A couple of operations, the flushes and the tracepoint, do not require
> serialisation by client->wq_lock, so move them before we take it.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index d6a6cf2540a1..7b535a32fc27 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -517,18 +517,18 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
>  	struct i915_guc_client *client = guc->execbuf_client;
>  	int b_ret;
>
> -	/* We are always called with irqs disabled */
> -	GEM_BUG_ON(!irqs_disabled());
> -
> -	spin_lock(&client->wq_lock);
> -	guc_wq_item_append(client, rq);
> -
>  	/* WA to flush out the pending GMADR writes to ring buffer. */
>  	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
>  		POSTING_READ_FW(GUC_STATUS);
>
>  	trace_i915_gem_request_in(rq, 0);
>
> +	/* We are always called with irqs disabled */
> +	GEM_BUG_ON(!irqs_disabled());
> +
> +	spin_lock(&client->wq_lock);
> +
> +	guc_wq_item_append(client, rq);
>  	b_ret = guc_ring_doorbell(client);
>
>  	client->submissions[engine_id] += 1;
> @@ -538,6 +538,7 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
>
>  	guc->submissions[engine_id] += 1;
>  	guc->last_seqno[engine_id] = rq->global_seqno;
> +
>  	spin_unlock(&client->wq_lock);
>  }
>
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

✓ Fi.CI.BAT: success for series starting with [v5,1/3] HAX enable guc submission for CI

[Patchwork]

== Series Details ==

Series: series starting with [v5,1/3] HAX enable guc submission for CI
URL   : https://patchwork.freedesktop.org/series/20375/
State : success

== Summary ==

Series 20375v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/20375/revisions/1/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 

cbeef675410c1628969ac5396d751ff44a541a81 drm-tip: 2017y-02m-28d-13h-53m-02s UTC integration manifest
1485f0f drm/i915/guc: Reorder __i915_guc_submit to reduce spinlock holdtime
46e04ea drm/i915/guc: Make wq_lock irq-safe
5c33c9f HAX enable guc submission for CI

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4000/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for series starting with [v5,1/3] HAX enable guc submission for CI

[Chris Wilson]

On Tue, Feb 28, 2017 at 02:48:25PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [v5,1/3] HAX enable guc submission for CI
> URL   : https://patchwork.freedesktop.org/series/20375/
> State : success
> 
> == Summary ==
> 
> Series 20375v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/20375/revisions/1/mbox/
> 
> fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
> fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
> fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
> fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
> fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
> fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
> fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
> fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
> fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
> fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
> fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 
> 
> cbeef675410c1628969ac5396d751ff44a541a81 drm-tip: 2017y-02m-28d-13h-53m-02s UTC integration manifest
> 1485f0f drm/i915/guc: Reorder __i915_guc_submit to reduce spinlock holdtime
> 46e04ea drm/i915/guc: Make wq_lock irq-safe

Applied this pair and now back to wondering about bxt.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx