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.winiar...@intel.com>
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiar...@intel.com>
>
> [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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
