[Intel-gfx] [PATCH 09/11] drm/i915/execlists: Refactor out can_merge_rq()

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-01-31 09:19:18)
> 
> On 30/01/2019 18:14, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-01-30 18:05:42)
> >>
> >> On 30/01/2019 02:19, Chris Wilson wrote:
> >>> @@ -827,8 +836,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> >>>         * request triggering preemption on the next dequeue (or subsequent
> >>>         * interrupt for secondary ports).
> >>>         */
> >>> -     execlists->queue_priority_hint =
> >>> -             port != execlists->port ? rq_prio(last) : INT_MIN;
> >>> +     execlists->queue_priority_hint = queue_prio(execlists);
> >>
> >> This shouldn't be in this patch.
> > 
> > If we terminate the loop early, we need to look at the head of the
> > queue.
> 
> Why it is different for ending early for any other (existing) reason? 
> Although I concede better management of queue_priority_hint is exactly 
> what I was suggesting. Oops. Consequences are not entirely straight 
> forward though.. if we decide not to submit all of a single context, or 
> leave port1 empty, currently we would hint scheduling the tasklet for 
> any new submission. With this change only after a CS or if a higher ctx 
> is submitted. Which is what makes me feel it should be a separate patch 
> for a behaviour change (since a high prio, higher than INT_MIN, is 
> potentially head of the queue).

Not quite. Previously if we saw port1 was empty it meant that last was
invalid and so the right choice was INT_MIN as the queue was empty. In
all other cases last is the first request in the priority list.

After this patch, we cannot draw the same conclusions from port1 being
empty, and nor can we directly inspect last. So to get the same result
as before the patch, we must actually look at the priority queue.
-Chris