[PATCH 1/2] drm/i915/execlists: Drop unused parameter to lookup_priolist()

[PATCH 1/2] drm/i915/execlists: Drop unused parameter to lookup_priolist()

[Chris Wilson]

lookup_priolist() no longer attaches the request into the priolist, it
just returns the priolist for the given priority instead. Drop the
unused parameter.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f9f4064dec0e..a9d211f28ab8 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -258,9 +258,7 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
 }
 
 static struct i915_priolist *
-lookup_priolist(struct intel_engine_cs *engine,
-		struct i915_sched_node *node,
-		int prio)
+lookup_priolist(struct intel_engine_cs *engine, int prio)
 {
 	struct intel_engine_execlists * const execlists = &engine->execlists;
 	struct i915_priolist *p;
@@ -345,7 +343,7 @@ static void __unwind_incomplete_requests(struct intel_engine_cs *engine)
 		GEM_BUG_ON(rq_prio(rq) == I915_PRIORITY_INVALID);
 		if (rq_prio(rq) != last_prio) {
 			last_prio = rq_prio(rq);
-			p = lookup_priolist(engine, &rq->sched, last_prio);
+			p = lookup_priolist(engine, last_prio);
 		}
 
 		list_add(&rq->sched.link, &p->requests);
@@ -1144,7 +1142,7 @@ static void queue_request(struct intel_engine_cs *engine,
 			  int prio)
 {
 	list_add_tail(&node->link,
-		      &lookup_priolist(engine, node, prio)->requests);
+		      &lookup_priolist(engine, prio)->requests);
 }
 
 static void __submit_queue(struct intel_engine_cs *engine, int prio)
-- 
2.17.0

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

[PATCH 2/2] drm/i915/execlists: Cache the priolist when rescheduling

[Chris Wilson]

When rescheduling a change of dependencies, they all need to be added to
the same priolist (at least the ones on the same engine!). Since we
likely want to move a batch of requests, keep the priolist around.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index a9d211f28ab8..28a02d3b53a4 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1197,7 +1197,8 @@ sched_lock_engine(struct i915_sched_node *node, struct intel_engine_cs *locked)
 static void execlists_schedule(struct i915_request *request,
 			       const struct i915_sched_attr *attr)
 {
-	struct intel_engine_cs *engine;
+	struct i915_priolist *uninitialized_var(pl);
+	struct intel_engine_cs *engine, *last;
 	struct i915_dependency *dep, *p;
 	struct i915_dependency stack;
 	const int prio = attr->priority;
@@ -1270,6 +1271,7 @@ static void execlists_schedule(struct i915_request *request,
 		__list_del_entry(&stack.dfs_link);
 	}
 
+	last = NULL;
 	engine = request->engine;
 	spin_lock_irq(&engine->timeline.lock);
 
@@ -1286,8 +1288,11 @@ static void execlists_schedule(struct i915_request *request,
 
 		node->attr.priority = prio;
 		if (!list_empty(&node->link)) {
-			__list_del_entry(&node->link);
-			queue_request(engine, node, prio);
+			if (last != engine) {
+				pl = lookup_priolist(engine, prio);
+				last = engine;
+			}
+			list_move_tail(&node->link, &pl->requests);
 		}
 
 		if (prio > engine->execlists.queue_priority &&
-- 
2.17.0

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/execlists: Drop unused parameter to lookup_priolist()

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/execlists: Drop unused parameter to lookup_priolist()
URL   : https://patchwork.freedesktop.org/series/42836/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
fbc6c33847ee drm/i915/execlists: Drop unused parameter to lookup_priolist()
b3992f1e5c08 drm/i915/execlists: Cache the priolist when rescheduling
-:21: WARNING:FUNCTION_ARGUMENTS: function definition argument 'pl' should also have an identifier name
#21: FILE: drivers/gpu/drm/i915/intel_lrc.c:1200:
+	struct i915_priolist *uninitialized_var(pl);

total: 0 errors, 1 warnings, 0 checks, 29 lines checked

_______________________________________________
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 [1/2] drm/i915/execlists: Drop unused parameter to lookup_priolist()

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/execlists: Drop unused parameter to lookup_priolist()
URL   : https://patchwork.freedesktop.org/series/42836/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4152 -> Patchwork_8930 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42836/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8930 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-reload:
      fi-bsw-n3050:       PASS -> DMESG-FAIL (fdo#106373)

    
    ==== Possible fixes ====

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-skl-6770hq:      FAIL (fdo#103928, fdo#100368) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#106373 https://bugs.freedesktop.org/show_bug.cgi?id=106373


== Participating hosts (41 -> 36) ==

  Missing    (5): fi-ctg-p8600 fi-byt-squawks fi-ilk-m540 fi-glk-j4005 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4152 -> Patchwork_8930

  CI_DRM_4152: 06e15c0f055b8b8e326bb65fa83f69b1b7391e51 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4464: 1bb318b32db003a377da14715c7b80675a712b6b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8930: b3992f1e5c088e330d75c3fe8c99835d4328673a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4464: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit


== Linux commits ==

b3992f1e5c08 drm/i915/execlists: Cache the priolist when rescheduling
fbc6c33847ee drm/i915/execlists: Drop unused parameter to lookup_priolist()

== Logs ==

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

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/execlists: Drop unused parameter to lookup_priolist()

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/execlists: Drop unused parameter to lookup_priolist()
URL   : https://patchwork.freedesktop.org/series/42836/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4152_full -> Patchwork_8930_full =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42836/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8930_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          PASS -> FAIL (fdo#103355)

    igt@kms_flip@2x-dpms-vs-vblank-race-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103060)

    igt@kms_flip@flip-vs-wf_vblank-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103928)
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@plain-flip-ts-check:
      shard-hsw:          PASS -> FAIL (fdo#100368) +1

    igt@kms_rotation_crc@primary-rotation-270:
      shard-apl:          PASS -> FAIL (fdo#103925, fdo#104724) +1

    
    ==== Possible fixes ====

    igt@drv_selftest@live_contexts:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_flip@2x-plain-flip-fb-recreate:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@dpms-vs-vblank-race-interruptible:
      shard-hsw:          FAIL (fdo#103060) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4152 -> Patchwork_8930

  CI_DRM_4152: 06e15c0f055b8b8e326bb65fa83f69b1b7391e51 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4464: 1bb318b32db003a377da14715c7b80675a712b6b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8930: b3992f1e5c088e330d75c3fe8c99835d4328673a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4464: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH 1/2] drm/i915/execlists: Drop unused parameter to lookup_priolist()

[Joonas Lahtinen]

Quoting Chris Wilson (2018-05-08 03:30:45)
> lookup_priolist() no longer attaches the request into the priolist, it
> just returns the priolist for the given priority instead. Drop the
> unused parameter.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

Re: [PATCH 2/2] drm/i915/execlists: Cache the priolist when rescheduling

[Joonas Lahtinen]

Quoting Chris Wilson (2018-05-08 03:30:46)
> When rescheduling a change of dependencies, they all need to be added to
> the same priolist (at least the ones on the same engine!). Since we
> likely want to move a batch of requests, keep the priolist around.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Comment below.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index a9d211f28ab8..28a02d3b53a4 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1197,7 +1197,8 @@ sched_lock_engine(struct i915_sched_node *node, struct intel_engine_cs *locked)
>  static void execlists_schedule(struct i915_request *request,
>                                const struct i915_sched_attr *attr)
>  {
> -       struct intel_engine_cs *engine;
> +       struct i915_priolist *uninitialized_var(pl);
> +       struct intel_engine_cs *engine, *last;
>         struct i915_dependency *dep, *p;
>         struct i915_dependency stack;
>         const int prio = attr->priority;
> @@ -1270,6 +1271,7 @@ static void execlists_schedule(struct i915_request *request,
>                 __list_del_entry(&stack.dfs_link);
>         }
>  
> +       last = NULL;
>         engine = request->engine;
>         spin_lock_irq(&engine->timeline.lock);
>  
> @@ -1286,8 +1288,11 @@ static void execlists_schedule(struct i915_request *request,
>  
>                 node->attr.priority = prio;
>                 if (!list_empty(&node->link)) {
> -                       __list_del_entry(&node->link);
> -                       queue_request(engine, node, prio);
> +                       if (last != engine) {
> +                               pl = lookup_priolist(engine, prio);
> +                               last = engine;
> +                       }

As you suggested, GEM_BUG_ON(pl->prio != prio) might make sense.

Regards, Joonas

> +                       list_move_tail(&node->link, &pl->requests);
>                 }
>  
>                 if (prio > engine->execlists.queue_priority &&
> -- 
> 2.17.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/execlists: Cache the priolist when rescheduling

[Chris Wilson]

Quoting Joonas Lahtinen (2018-05-08 09:18:03)
> Quoting Chris Wilson (2018-05-08 03:30:46)
> > When rescheduling a change of dependencies, they all need to be added to
> > the same priolist (at least the ones on the same engine!). Since we
> > likely want to move a batch of requests, keep the priolist around.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Comment below.
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Applied the suggested assertions and pushed. Thanks for the review,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx