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

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> From: Michał Winiarski <michal.winiarski@intel.com>
> 
> 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 <michal.winiarski@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> +++ 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(&engine->timeline->lock);
>  
>  	list_for_each_entry_safe_reverse(rq, rn,
>  					 &engine->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,
> -				    &rq->priotree,
> -				    rq->priotree.priority);
> -		list_add(&rq->priotree.link,
> -			 &ptr_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 <joonas.lahtinen@linux.intel.com>

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