Re: [Intel-gfx] [PATCH v2 02/11] drm/i915/execlists: Cache the last priolist lookup

[Chris Wilson]

Quoting Joonas Lahtinen (2017-09-28 12:59:01)
> On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> > From: Michał Winiarski 
> > 
> > Avoid the repeated rbtree lookup for each request as we unwind them by
> > tracking the last priolist.
> > 
> > v2: Fix up my unhelpful suggestion of using default_priolist.
> > 
> > Signed-off-by: Michał Winiarski 
> > Signed-off-by: Chris Wilson 
> 
> 
> 
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -358,25 +358,31 @@ static void unwind_wa_tail(struct 
> > drm_i915_gem_request *rq)
> >  static void unwind_incomplete_requests(struct intel_engine_cs *engine)
> >  {
> >   struct drm_i915_gem_request *rq, *rn;
> > + struct i915_priolist *uninitialized_var(p);
> > + int last_prio = INT_MAX;
> >  
> >   lockdep_assert_held(>timeline->lock);
> >  
> >   list_for_each_entry_safe_reverse(rq, rn,
> >>timeline->requests,
> >link) {
> > - struct i915_priolist *p;
> > -
> >   if (i915_gem_request_completed(rq))
> >   return;
> >  
> >   __i915_gem_request_unsubmit(rq);
> >   unwind_wa_tail(rq);
> >  
> > - p = lookup_priolist(engine,
> > - >priotree,
> > - rq->priotree.priority);
> > - list_add(>priotree.link,
> > -  _mask_bits(p, 1)->requests);
> > + GEM_BUG_ON(rq->priotree.priority == INT_MAX);
> 
> This doesn't read aloud too logically when coming from the ring reset
> codepath, at first like would seem like we're not allowing maximum
> priority tasks to be unwinded (which only makes sense on the pre-empt
> odepath). #define INVALID_PRIORITY INT_MAX might help

And adding the GEM_BUG_ON() to execlists_schedule() to make sure we
never try to set it as well.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 02/11] drm/i915/execlists: Cache the last priolist lookup

[Joonas Lahtinen]

On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> From: Michał Winiarski 
> 
> Avoid the repeated rbtree lookup for each request as we unwind them by
> tracking the last priolist.
> 
> v2: Fix up my unhelpful suggestion of using default_priolist.
> 
> Signed-off-by: Michał Winiarski 
> Signed-off-by: Chris Wilson 



> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -358,25 +358,31 @@ static void unwind_wa_tail(struct drm_i915_gem_request 
> *rq)
>  static void unwind_incomplete_requests(struct intel_engine_cs *engine)
>  {
>   struct drm_i915_gem_request *rq, *rn;
> + struct i915_priolist *uninitialized_var(p);
> + int last_prio = INT_MAX;
>  
>   lockdep_assert_held(>timeline->lock);
>  
>   list_for_each_entry_safe_reverse(rq, rn,
>>timeline->requests,
>link) {
> - struct i915_priolist *p;
> -
>   if (i915_gem_request_completed(rq))
>   return;
>  
>   __i915_gem_request_unsubmit(rq);
>   unwind_wa_tail(rq);
>  
> - p = lookup_priolist(engine,
> - >priotree,
> - rq->priotree.priority);
> - list_add(>priotree.link,
> -  _mask_bits(p, 1)->requests);
> + GEM_BUG_ON(rq->priotree.priority == INT_MAX);

This doesn't read aloud too logically when coming from the ring reset
codepath, at first like would seem like we're not allowing maximum
priority tasks to be unwinded (which only makes sense on the pre-empt
odepath). #define INVALID_PRIORITY INT_MAX might help

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 02/11] drm/i915/execlists: Cache the last priolist lookup

[Chris Wilson]

From: Michał Winiarski 

Avoid the repeated rbtree lookup for each request as we unwind them by
tracking the last priolist.

v2: Fix up my unhelpful suggestion of using default_priolist.

Signed-off-by: Michał Winiarski 
Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_lrc.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d67907a7e8e6..3998f359d4f0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -358,25 +358,31 @@ static void unwind_wa_tail(struct drm_i915_gem_request 
*rq)
 static void unwind_incomplete_requests(struct intel_engine_cs *engine)
 {
struct drm_i915_gem_request *rq, *rn;
+   struct i915_priolist *uninitialized_var(p);
+   int last_prio = INT_MAX;
 
lockdep_assert_held(>timeline->lock);
 
list_for_each_entry_safe_reverse(rq, rn,
 >timeline->requests,
 link) {
-   struct i915_priolist *p;
-
if (i915_gem_request_completed(rq))
return;
 
__i915_gem_request_unsubmit(rq);
unwind_wa_tail(rq);
 
-   p = lookup_priolist(engine,
-   >priotree,
-   rq->priotree.priority);
-   list_add(>priotree.link,
-_mask_bits(p, 1)->requests);
+   GEM_BUG_ON(rq->priotree.priority == INT_MAX);
+   if (rq->priotree.priority != last_prio) {
+   p = lookup_priolist(engine,
+   >priotree,
+   rq->priotree.priority);
+   p = ptr_mask_bits(p, 1);
+
+   last_prio = rq->priotree.priority;
+   }
+
+   list_add(>priotree.link, >requests);
}
 }
 
-- 
2.14.1

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