All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Remove local timeline var from submit/unsubmit
@ 2018-03-22 12:48 Chris Wilson
  2018-03-22 12:55 ` [PATCH v2] " Chris Wilson
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Chris Wilson @ 2018-03-22 12:48 UTC (permalink / raw)
  To: intel-gfx

Both request_submit and request_unsubmit deal with transferring the
request from the client's timeline onto the execution timeline and back
again. As both functions deal with a pair of timeline's, using a
shorthand for just one of them is slightly confusing, especially as the
different functions use the shorthand for the alternate timeline.
Instead, use the full version of each timeline so it should be easier to
keep track of the transfer between the request/client and the engine.

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

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 2325886d1d55..024169383d6a 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -495,7 +495,6 @@ static u32 timeline_get_seqno(struct intel_timeline *tl)
 void __i915_request_submit(struct i915_request *request)
 {
 	struct intel_engine_cs *engine = request->engine;
-	struct intel_timeline *timeline;
 	u32 seqno;
 
 	GEM_TRACE("%s fence %llx:%d -> global_seqno %d\n",
@@ -506,12 +505,9 @@ void __i915_request_submit(struct i915_request *request)
 	GEM_BUG_ON(!irqs_disabled());
 	lockdep_assert_held(&engine->timeline->lock);
 
-	/* Transfer from per-context onto the global per-engine timeline */
-	timeline = engine->timeline;
-	GEM_BUG_ON(timeline == request->timeline);
 	GEM_BUG_ON(request->global_seqno);
 
-	seqno = timeline_get_seqno(timeline);
+	seqno = timeline_get_seqno(engine->timeline);
 	GEM_BUG_ON(!seqno);
 	GEM_BUG_ON(i915_seqno_passed(intel_engine_get_seqno(engine), seqno));
 
@@ -525,8 +521,10 @@ void __i915_request_submit(struct i915_request *request)
 	engine->emit_breadcrumb(request,
 				request->ring->vaddr + request->postfix);
 
+	/* Transfer from per-context onto the global per-engine timeline */
+	GEM_BUG_ON(engine->timeline == request->timeline);
 	spin_lock(&request->timeline->lock);
-	list_move_tail(&request->link, &timeline->requests);
+	list_move_tail(&request->link, &engine->timeline->requests);
 	spin_unlock(&request->timeline->lock);
 
 	trace_i915_request_execute(request);
@@ -550,7 +548,6 @@ void i915_request_submit(struct i915_request *request)
 void __i915_request_unsubmit(struct i915_request *request)
 {
 	struct intel_engine_cs *engine = request->engine;
-	struct intel_timeline *timeline;
 
 	GEM_TRACE("%s fence %llx:%d <- global_seqno %d\n",
 		  request->engine->name,
@@ -578,12 +575,10 @@ void __i915_request_unsubmit(struct i915_request *request)
 	spin_unlock(&request->lock);
 
 	/* Transfer back from the global per-engine timeline to per-context */
-	timeline = request->timeline;
-	GEM_BUG_ON(timeline == engine->timeline);
-
-	spin_lock(&timeline->lock);
-	list_move(&request->link, &timeline->requests);
-	spin_unlock(&timeline->lock);
+	GEM_BUG_ON(request->timeline == engine->timeline);
+	spin_lock(&request->timeline->lock);
+	list_move(&request->link, &request->timeline->requests);
+	spin_unlock(&request->timeline->lock);
 
 	/*
 	 * We don't need to wake_up any waiters on request->execute, they
-- 
2.16.2

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] drm/i915: Remove local timeline var from submit/unsubmit
  2018-03-22 12:48 [PATCH] drm/i915: Remove local timeline var from submit/unsubmit Chris Wilson
@ 2018-03-22 12:55 ` Chris Wilson
  2018-03-22 13:01   ` Chris Wilson
  2018-03-22 13:10 ` [PATCH v3] " Chris Wilson
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2018-03-22 12:55 UTC (permalink / raw)
  To: intel-gfx

Both request_submit and request_unsubmit deal with transferring the
request from the client's timeline onto the execution timeline and back
again. As both functions deal with a pair of timeline's, using a
shorthand for just one of them is slightly confusing, especially as the
different functions use the shorthand for the alternate timeline.
Instead, use the full version of each timeline so it should be easier to
keep track of the transfer between the request/client and the engine.

v2: Refactor the common lock+list_move

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

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 2325886d1d55..cc4f01ea2a03 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -492,10 +492,19 @@ static u32 timeline_get_seqno(struct intel_timeline *tl)
 	return ++tl->seqno;
 }
 
+static void move_to_timeline(struct i915_request *request,
+			     struct intel_timeline *timeline)
+{
+	GEM_BUG_ON(request->timeline == request->engine->timeline);
+
+	spin_lock(&request->timeline->lock);
+	list_move_tail(&request->link, &timeline->requests);
+	spin_unlock(&request->timeline->lock);
+}
+
 void __i915_request_submit(struct i915_request *request)
 {
 	struct intel_engine_cs *engine = request->engine;
-	struct intel_timeline *timeline;
 	u32 seqno;
 
 	GEM_TRACE("%s fence %llx:%d -> global_seqno %d\n",
@@ -506,12 +515,9 @@ void __i915_request_submit(struct i915_request *request)
 	GEM_BUG_ON(!irqs_disabled());
 	lockdep_assert_held(&engine->timeline->lock);
 
-	/* Transfer from per-context onto the global per-engine timeline */
-	timeline = engine->timeline;
-	GEM_BUG_ON(timeline == request->timeline);
 	GEM_BUG_ON(request->global_seqno);
 
-	seqno = timeline_get_seqno(timeline);
+	seqno = timeline_get_seqno(engine->timeline);
 	GEM_BUG_ON(!seqno);
 	GEM_BUG_ON(i915_seqno_passed(intel_engine_get_seqno(engine), seqno));
 
@@ -525,9 +531,8 @@ void __i915_request_submit(struct i915_request *request)
 	engine->emit_breadcrumb(request,
 				request->ring->vaddr + request->postfix);
 
-	spin_lock(&request->timeline->lock);
-	list_move_tail(&request->link, &timeline->requests);
-	spin_unlock(&request->timeline->lock);
+	/* Transfer from per-context onto the global per-engine timeline */
+	move_to_timeline(request, engine->timeline);
 
 	trace_i915_request_execute(request);
 
@@ -550,7 +555,6 @@ void i915_request_submit(struct i915_request *request)
 void __i915_request_unsubmit(struct i915_request *request)
 {
 	struct intel_engine_cs *engine = request->engine;
-	struct intel_timeline *timeline;
 
 	GEM_TRACE("%s fence %llx:%d <- global_seqno %d\n",
 		  request->engine->name,
@@ -578,12 +582,7 @@ void __i915_request_unsubmit(struct i915_request *request)
 	spin_unlock(&request->lock);
 
 	/* Transfer back from the global per-engine timeline to per-context */
-	timeline = request->timeline;
-	GEM_BUG_ON(timeline == engine->timeline);
-
-	spin_lock(&timeline->lock);
-	list_move(&request->link, &timeline->requests);
-	spin_unlock(&timeline->lock);
+	move_to_timeline(request, request->timeline);
 
 	/*
 	 * We don't need to wake_up any waiters on request->execute, they
-- 
2.16.2

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/i915: Remove local timeline var from submit/unsubmit
  2018-03-22 12:55 ` [PATCH v2] " Chris Wilson
@ 2018-03-22 13:01   ` Chris Wilson
  0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-03-22 13:01 UTC (permalink / raw)
  To: intel-gfx

Quoting Chris Wilson (2018-03-22 12:55:52)
> Both request_submit and request_unsubmit deal with transferring the
> request from the client's timeline onto the execution timeline and back
> again. As both functions deal with a pair of timeline's, using a
> shorthand for just one of them is slightly confusing, especially as the
> different functions use the shorthand for the alternate timeline.
> Instead, use the full version of each timeline so it should be easier to
> keep track of the transfer between the request/client and the engine.
> 
> v2: Refactor the common lock+list_move
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_request.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 2325886d1d55..cc4f01ea2a03 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -492,10 +492,19 @@ static u32 timeline_get_seqno(struct intel_timeline *tl)
>         return ++tl->seqno;
>  }
>  
> +static void move_to_timeline(struct i915_request *request,
> +                            struct intel_timeline *timeline)
> +{
> +       GEM_BUG_ON(request->timeline == request->engine->timeline);
lockdep_assert_held(&request->engine->timeline->lock);

for clarity.

> +
> +       spin_lock(&request->timeline->lock);
> +       list_move_tail(&request->link, &timeline->requests);
> +       spin_unlock(&request->timeline->lock);
> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] drm/i915: Remove local timeline var from submit/unsubmit
  2018-03-22 12:48 [PATCH] drm/i915: Remove local timeline var from submit/unsubmit Chris Wilson
  2018-03-22 12:55 ` [PATCH v2] " Chris Wilson
@ 2018-03-22 13:10 ` Chris Wilson
  2018-03-22 13:24   ` Mika Kuoppala
  2018-03-22 13:17 ` ✓ Fi.CI.BAT: success for drm/i915: Remove local timeline var from submit/unsubmit (rev2) Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2018-03-22 13:10 UTC (permalink / raw)
  To: intel-gfx

Both request_submit and request_unsubmit deal with transferring the
request from the client's timeline onto the execution timeline and back
again. As both functions deal with a pair of timeline's, using a
shorthand for just one of them is slightly confusing, especially as the
different functions use the shorthand for the alternate timeline.
Instead, use the full version of each timeline so it should be easier to
keep track of the transfer between the request/client and the engine.

v2: Refactor the common lock+list_move
v3: Be clear we require the other timeline list to be locked as well.

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

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 2325886d1d55..854af07994f3 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -492,10 +492,20 @@ static u32 timeline_get_seqno(struct intel_timeline *tl)
 	return ++tl->seqno;
 }
 
+static void move_to_timeline(struct i915_request *request,
+			     struct intel_timeline *timeline)
+{
+	GEM_BUG_ON(request->timeline == request->engine->timeline);
+	lockdep_assert_held(&request->engine->timeline->lock);
+
+	spin_lock(&request->timeline->lock);
+	list_move_tail(&request->link, &timeline->requests);
+	spin_unlock(&request->timeline->lock);
+}
+
 void __i915_request_submit(struct i915_request *request)
 {
 	struct intel_engine_cs *engine = request->engine;
-	struct intel_timeline *timeline;
 	u32 seqno;
 
 	GEM_TRACE("%s fence %llx:%d -> global_seqno %d\n",
@@ -506,12 +516,9 @@ void __i915_request_submit(struct i915_request *request)
 	GEM_BUG_ON(!irqs_disabled());
 	lockdep_assert_held(&engine->timeline->lock);
 
-	/* Transfer from per-context onto the global per-engine timeline */
-	timeline = engine->timeline;
-	GEM_BUG_ON(timeline == request->timeline);
 	GEM_BUG_ON(request->global_seqno);
 
-	seqno = timeline_get_seqno(timeline);
+	seqno = timeline_get_seqno(engine->timeline);
 	GEM_BUG_ON(!seqno);
 	GEM_BUG_ON(i915_seqno_passed(intel_engine_get_seqno(engine), seqno));
 
@@ -525,9 +532,8 @@ void __i915_request_submit(struct i915_request *request)
 	engine->emit_breadcrumb(request,
 				request->ring->vaddr + request->postfix);
 
-	spin_lock(&request->timeline->lock);
-	list_move_tail(&request->link, &timeline->requests);
-	spin_unlock(&request->timeline->lock);
+	/* Transfer from per-context onto the global per-engine timeline */
+	move_to_timeline(request, engine->timeline);
 
 	trace_i915_request_execute(request);
 
@@ -550,7 +556,6 @@ void i915_request_submit(struct i915_request *request)
 void __i915_request_unsubmit(struct i915_request *request)
 {
 	struct intel_engine_cs *engine = request->engine;
-	struct intel_timeline *timeline;
 
 	GEM_TRACE("%s fence %llx:%d <- global_seqno %d\n",
 		  request->engine->name,
@@ -578,12 +583,7 @@ void __i915_request_unsubmit(struct i915_request *request)
 	spin_unlock(&request->lock);
 
 	/* Transfer back from the global per-engine timeline to per-context */
-	timeline = request->timeline;
-	GEM_BUG_ON(timeline == engine->timeline);
-
-	spin_lock(&timeline->lock);
-	list_move(&request->link, &timeline->requests);
-	spin_unlock(&timeline->lock);
+	move_to_timeline(request, request->timeline);
 
 	/*
 	 * We don't need to wake_up any waiters on request->execute, they
-- 
2.16.2

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Remove local timeline var from submit/unsubmit (rev2)
  2018-03-22 12:48 [PATCH] drm/i915: Remove local timeline var from submit/unsubmit Chris Wilson
  2018-03-22 12:55 ` [PATCH v2] " Chris Wilson
  2018-03-22 13:10 ` [PATCH v3] " Chris Wilson
@ 2018-03-22 13:17 ` Patchwork
  2018-03-22 13:48 ` ✓ Fi.CI.BAT: success for drm/i915: Remove local timeline var from submit/unsubmit (rev3) Patchwork
  2018-03-22 14:38 ` ✗ Fi.CI.IGT: failure " Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-03-22 13:17 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Remove local timeline var from submit/unsubmit (rev2)
URL   : https://patchwork.freedesktop.org/series/40458/
State : success

== Summary ==

Series 40458v2 drm/i915: Remove local timeline var from submit/unsubmit
https://patchwork.freedesktop.org/api/1.0/series/40458/revisions/2/mbox/

---- Known issues:

Test debugfs_test:
        Subgroup read_all_entries:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-bxt-dsi) fdo#103927

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:430s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:443s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:380s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:536s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:299s
fi-bxt-dsi       total:243  pass:216  dwarn:0   dfail:0   fail:0   skip:26 
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:514s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:514s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:502s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:409s
fi-cnl-drrs      total:285  pass:254  dwarn:3   dfail:0   fail:0   skip:28  time:520s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:588s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:425s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:318s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:538s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:401s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:423s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:487s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:435s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:474s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:466s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:519s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:649s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:438s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:529s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:505s
fi-skl-6770hq    total:285  pass:264  dwarn:0   dfail:0   fail:1   skip:20  time:505s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:427s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:446s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:568s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:399s
fi-cfl-s3 failed to connect after reboot
fi-cfl-u failed to connect after reboot

dff9ece60048108782aab6d6123822c1d34b0e5a drm-tip: 2018y-03m-21d-20h-44m-14s UTC integration manifest
6a5002ec412c drm/i915: Remove local timeline var from submit/unsubmit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8446/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] drm/i915: Remove local timeline var from submit/unsubmit
  2018-03-22 13:10 ` [PATCH v3] " Chris Wilson
@ 2018-03-22 13:24   ` Mika Kuoppala
  2018-03-22 16:00     ` Chris Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: Mika Kuoppala @ 2018-03-22 13:24 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Both request_submit and request_unsubmit deal with transferring the
> request from the client's timeline onto the execution timeline and back
> again. As both functions deal with a pair of timeline's, using a
> shorthand for just one of them is slightly confusing, especially as the

The amount confusion is correlated with readers previous knowledge about
timelines. Slightly is an understatement for the newcomers but
we can accept that as a good amount of confusion, on average.

> different functions use the shorthand for the alternate timeline.
> Instead, use the full version of each timeline so it should be easier to
> keep track of the transfer between the request/client and the engine.
>

I pondered if the comment can even be removed on this, much
easier to read, version. But as the comment gives a
proper names to timelines, it has value.

Only one thing remains is that should the 'client's timeline'
be changed to 'per context timeline' on this commit msg,
to reflect the comment in the code.

> v2: Refactor the common lock+list_move
> v3: Be clear we require the other timeline list to be locked as well.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_request.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 2325886d1d55..854af07994f3 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -492,10 +492,20 @@ static u32 timeline_get_seqno(struct intel_timeline *tl)
>  	return ++tl->seqno;
>  }
>  
> +static void move_to_timeline(struct i915_request *request,
> +			     struct intel_timeline *timeline)
> +{
> +	GEM_BUG_ON(request->timeline == request->engine->timeline);
> +	lockdep_assert_held(&request->engine->timeline->lock);
> +
> +	spin_lock(&request->timeline->lock);
> +	list_move_tail(&request->link, &timeline->requests);
> +	spin_unlock(&request->timeline->lock);
> +}
> +
>  void __i915_request_submit(struct i915_request *request)
>  {
>  	struct intel_engine_cs *engine = request->engine;
> -	struct intel_timeline *timeline;
>  	u32 seqno;
>  
>  	GEM_TRACE("%s fence %llx:%d -> global_seqno %d\n",
> @@ -506,12 +516,9 @@ void __i915_request_submit(struct i915_request *request)
>  	GEM_BUG_ON(!irqs_disabled());
>  	lockdep_assert_held(&engine->timeline->lock);
>  
> -	/* Transfer from per-context onto the global per-engine timeline */
> -	timeline = engine->timeline;
> -	GEM_BUG_ON(timeline == request->timeline);
>  	GEM_BUG_ON(request->global_seqno);
>  
> -	seqno = timeline_get_seqno(timeline);
> +	seqno = timeline_get_seqno(engine->timeline);
>  	GEM_BUG_ON(!seqno);
>  	GEM_BUG_ON(i915_seqno_passed(intel_engine_get_seqno(engine), seqno));
>  
> @@ -525,9 +532,8 @@ void __i915_request_submit(struct i915_request *request)
>  	engine->emit_breadcrumb(request,
>  				request->ring->vaddr + request->postfix);
>  
> -	spin_lock(&request->timeline->lock);
> -	list_move_tail(&request->link, &timeline->requests);
> -	spin_unlock(&request->timeline->lock);
> +	/* Transfer from per-context onto the global per-engine timeline */
> +	move_to_timeline(request, engine->timeline);
>  
>  	trace_i915_request_execute(request);
>  
> @@ -550,7 +556,6 @@ void i915_request_submit(struct i915_request *request)
>  void __i915_request_unsubmit(struct i915_request *request)
>  {
>  	struct intel_engine_cs *engine = request->engine;
> -	struct intel_timeline *timeline;
>  
>  	GEM_TRACE("%s fence %llx:%d <- global_seqno %d\n",
>  		  request->engine->name,
> @@ -578,12 +583,7 @@ void __i915_request_unsubmit(struct i915_request *request)
>  	spin_unlock(&request->lock);
>  
>  	/* Transfer back from the global per-engine timeline to per-context */
> -	timeline = request->timeline;
> -	GEM_BUG_ON(timeline == engine->timeline);
> -
> -	spin_lock(&timeline->lock);
> -	list_move(&request->link, &timeline->requests);
> -	spin_unlock(&timeline->lock);
> +	move_to_timeline(request, request->timeline);
>  
>  	/*
>  	 * We don't need to wake_up any waiters on request->execute, they
> -- 
> 2.16.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Remove local timeline var from submit/unsubmit (rev3)
  2018-03-22 12:48 [PATCH] drm/i915: Remove local timeline var from submit/unsubmit Chris Wilson
                   ` (2 preceding siblings ...)
  2018-03-22 13:17 ` ✓ Fi.CI.BAT: success for drm/i915: Remove local timeline var from submit/unsubmit (rev2) Patchwork
@ 2018-03-22 13:48 ` Patchwork
  2018-03-22 14:38 ` ✗ Fi.CI.IGT: failure " Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-03-22 13:48 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Remove local timeline var from submit/unsubmit (rev3)
URL   : https://patchwork.freedesktop.org/series/40458/
State : success

== Summary ==

Series 40458v3 drm/i915: Remove local timeline var from submit/unsubmit
https://patchwork.freedesktop.org/api/1.0/series/40458/revisions/3/mbox/

---- Known issues:

Test debugfs_test:
        Subgroup read_all_entries:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (fi-skl-6770hq) fdo#100368
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-bxt-dsi) fdo#103927

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:434s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:438s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:380s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:532s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:298s
fi-bxt-dsi       total:243  pass:216  dwarn:0   dfail:0   fail:0   skip:26 
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:515s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:512s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:503s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:410s
fi-cnl-drrs      total:285  pass:254  dwarn:3   dfail:0   fail:0   skip:28  time:526s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:584s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:425s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:315s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:404s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:419s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:479s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:441s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:475s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:470s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:511s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:653s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:442s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:537s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:501s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:492s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:430s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:450s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:572s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:403s

dff9ece60048108782aab6d6123822c1d34b0e5a drm-tip: 2018y-03m-21d-20h-44m-14s UTC integration manifest
c50b8eac365b drm/i915: Remove local timeline var from submit/unsubmit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8447/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✗ Fi.CI.IGT: failure for drm/i915: Remove local timeline var from submit/unsubmit (rev3)
  2018-03-22 12:48 [PATCH] drm/i915: Remove local timeline var from submit/unsubmit Chris Wilson
                   ` (3 preceding siblings ...)
  2018-03-22 13:48 ` ✓ Fi.CI.BAT: success for drm/i915: Remove local timeline var from submit/unsubmit (rev3) Patchwork
@ 2018-03-22 14:38 ` Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-03-22 14:38 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Remove local timeline var from submit/unsubmit (rev3)
URL   : https://patchwork.freedesktop.org/series/40458/
State : failure

== Summary ==

---- Possible new issues:

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> INCOMPLETE (shard-snb)

---- Known issues:

Test kms_flip:
        Subgroup flip-vs-absolute-wf_vblank-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368 +1
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-apl) fdo#99912
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047

fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:3478 pass:1814 dwarn:1   dfail:0   fail:7   skip:1655 time:13132s
shard-hsw        total:3478 pass:1767 dwarn:1   dfail:0   fail:2   skip:1707 time:11872s
shard-snb        total:3472 pass:1352 dwarn:1   dfail:0   fail:3   skip:2115 time:7040s
Blacklisted hosts:
shard-kbl        total:3478 pass:1938 dwarn:1   dfail:1   fail:10  skip:1528 time:9931s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8447/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] drm/i915: Remove local timeline var from submit/unsubmit
  2018-03-22 13:24   ` Mika Kuoppala
@ 2018-03-22 16:00     ` Chris Wilson
  0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-03-22 16:00 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2018-03-22 13:24:13)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Both request_submit and request_unsubmit deal with transferring the
> > request from the client's timeline onto the execution timeline and back
> > again. As both functions deal with a pair of timeline's, using a
> > shorthand for just one of them is slightly confusing, especially as the
> 
> The amount confusion is correlated with readers previous knowledge about
> timelines. Slightly is an understatement for the newcomers but
> we can accept that as a good amount of confusion, on average.
> 
> > different functions use the shorthand for the alternate timeline.
> > Instead, use the full version of each timeline so it should be easier to
> > keep track of the transfer between the request/client and the engine.
> >
> 
> I pondered if the comment can even be removed on this, much
> easier to read, version. But as the comment gives a
> proper names to timelines, it has value.
> 
> Only one thing remains is that should the 'client's timeline'
> be changed to 'per context timeline' on this commit msg,
> to reflect the comment in the code.

I use client/context quite interchangeably. Every context is a client,
but a client may have more than one context.

I should probably avoid "client" altogether since we only have fd and
contexts, and a hint of process (which is more or less limited to mmu,
more or less meaningless when we look at the GPU structs).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-03-22 16:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 12:48 [PATCH] drm/i915: Remove local timeline var from submit/unsubmit Chris Wilson
2018-03-22 12:55 ` [PATCH v2] " Chris Wilson
2018-03-22 13:01   ` Chris Wilson
2018-03-22 13:10 ` [PATCH v3] " Chris Wilson
2018-03-22 13:24   ` Mika Kuoppala
2018-03-22 16:00     ` Chris Wilson
2018-03-22 13:17 ` ✓ Fi.CI.BAT: success for drm/i915: Remove local timeline var from submit/unsubmit (rev2) Patchwork
2018-03-22 13:48 ` ✓ Fi.CI.BAT: success for drm/i915: Remove local timeline var from submit/unsubmit (rev3) Patchwork
2018-03-22 14:38 ` ✗ Fi.CI.IGT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.