[Intel-gfx] [PATCH 01/18] drm/i915/execlists: Set queue priority from secondary port

[Chris Wilson]

Quoting Chris Wilson (2018-04-10 15:56:03)
> Quoting Tvrtko Ursulin (2018-04-10 15:42:07)
> > 
> > On 10/04/2018 15:24, Chris Wilson wrote:
> > > Quoting Tvrtko Ursulin (2018-04-10 15:05:33)
> > >> Although I failed to understand what do we do in both cases if a new
> > >> request arrives of higher prio than the one in ELSP[1]. Looks like
> > >> nothing? Wait until GPU moves it to ELSP[0] and preempt then? Is this so
> > >> because we can't safely or I misread something?
> > > 
> > > This is covered by igt/gem_exec_schedule/preempt-queue*. If we receive a
> > > request higher than ELSP[1], we start a preemption as
> > > 
> > >       if (need_preempt(engine, last, execlists->queue_priority)) {
> > > 
> > > will evaluate to true. It's either the lowest executing priority (new
> > > code), or lowest pending priority (old code). In either case, if the new
> > > request is more important than the queue_priority, we preempt.
> > 
> > How when "last" is request from ELSP[0]? And also 
> > execlists->queue_priority has not yet been updated to reflect the new 
> > priority?
> 
> When we start executing last on ELSP[0] there will have been another
> execlists_dequeue() where we see an empty slot (or fill it) and update
> queue_priority. If we are down to the last request, it will be set to
> INT_MIN. Upon its completion, it will remain INT_MIN.
> 
> > Then there is also "if (port_count(port0)) goto unlock;" suggesting that 
> > if there were any appends to ELSP[0] we will also fail to act in this 
> > situation?
> 
> If we only write into ELSP[0], then ELSP[1] remains empty and the
> queue_priority still needs to INT_MIN so that we service any new
> i915_request_add immediately.
>  
> > > We won't evaluate preemption if we are still awaiting the HWACK from the
> > > last ELSP write, or if we are still preempting. In both of those cases,
> > > we expect to receive an interrupt promptly, upon which we then redo our
> > > evaluations.
> > > 
> > >> Also, don't you need to manage execlists->queue_priority after CSB
> > >> processing now? So that it correctly reflects the priority of request in
> > >> ELSP[1] after ELSP[0] gets completed? It seems that without it would get
> > >> stuck at the previous value and then submission could decide to skip
> > >> scheduling the tasklet if new priority is lower than what was in ELSP[1]
> > >> before, and so would fail to fill ELSP[1].
> > > 
> > > Yes, that is also done here. Since we are always looking one request
> > > ahead, we either update the priority based on the queue following the
> > > resubmission on interrupt, or it is left as INT_MIN on completion.
> > > Indeed, making sure we reset back to INT_MIN is essential so that we
> > > don't any future submissions from idle.
> > 
> > Okay I see it - because execlists_dequeue is called and runs to the 
> > queue_priority update bit even when there is nothing in the queue.
> 
> Phew, I can get away from having to draw ascii diagrams. I'll leave that
> to Mika as he figures out how to hook up N ports ;)

        /*
         * Here be a bit of magic! Or sleight-of-hand, whichever you prefer.
         *
         * We choose queue_priority such that if we add a request of greater
         * priority than this, we kick the submission tasklet to decide on
         * the right order of submitting the requests to hardware. We must
         * also be prepared to reorder requests as they are in-flight on the
         * HW. We derive the queue_priority then as the first "hole" in
         * the HW submission ports and if there are no available slots,
         * it the priority of the lowest executing request, i.e. the last one.
         */
        execlists->queue_priority =
                port != execlists->port ? rq_prio(last) : INT_MIN;

Does that help, or do I need ASCII art?
-Chris