Quoting Michał Winiarski (2017-06-23 11:35:06) > On Thu, Jun 22, 2017 at 11:55:51AM +0100, Chris Wilson wrote: > > Once a client has requested a waitboost, we keep that waitboost active > > until all clients are no longer waiting. This is because we don't > > distinguish which waiter deserves the boost. However, with the advent of > > fence signaling, the signaler threads appear as waiters to the RPS > > interrupt handler. So instead of using a single boolean to track when to > > keep the waitboost active, use a counter of all outstanding waitboosted > > requests. > > > > At this point, I have removed all vestiges of the rate limiting on > > clients. Whilst this means that compositors should remain more fluid, > > it also means that boosts are more prevalent. > > > > A drawback of this implementation is that it requires constant request > > submission to keep the waitboost trimmed (as it is now cancelled when the > > request is completed). This will be fine for a busy system, but near > > idle the boosts may be kept for longer than desired (effectively tens of > > vblanks worstcase) and there is a reliance on rc6 instead. > > In other words, now we're disabling boost when all requests that required boost > are retired. > We may need to tweak igt pm_rps boost scenarios to account for that extra time. It should be less time boosted than before, if the requests are retired promptly. But that's a big if. > > Reported-by: Michał Winiarski <michal.winiarski@xxxxxxxxx> > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx> > > [SNIP] > > > @@ -2347,11 +2349,10 @@ static int i915_rps_boost_info(struct seq_file *m, void *data) > > > > rcu_read_lock(); > > task = pid_task(file->pid, PIDTYPE_PID); > > - seq_printf(m, "%s [%d]: %d boosts%s\n", > > + seq_printf(m, "%s [%d]: %d boosts\n", > > task ? task->comm : "<unknown>", > > task ? task->pid : -1, > > - file_priv->rps.boosts, > > - list_empty(&file_priv->rps.link) ? "" : ", active"); > > + atomic_read(&file_priv->rps.boosts)); > > rcu_read_unlock(); > > } > > seq_printf(m, "Kernel (anonymous) boosts: %d\n", dev_priv->rps.boosts); > > dev_priv->rps.boosts seems to be unused at this point, could you remove it while > you're here? Oh, I should just hook up the other side of > > + if (rps != NULL) > > + atomic_inc(&rps->boosts); else atomic_inc(&dev_priv->rps.boosts); The only user is dead-code atm, but it may come in useful again for atomic modesetting. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
