On Fri, 19 Nov 2021 16:56:01 +0100 Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Nov 19, 2021 at 12:38:41PM +0200, Pekka Paalanen wrote:
On Thu, 18 Nov 2021 17:46:10 -0800 Brian Norris briannorris@chromium.org wrote:
Hi Pekka,
Thanks for the thoughts and review. I've tried to respond below:
On Thu, Nov 18, 2021 at 12:39:28PM +0200, Pekka Paalanen wrote:
On Wed, 17 Nov 2021 14:48:40 -0800 Brian Norris briannorris@chromium.org wrote:
A variety of applications have found it useful to listen to user-initiated input events to make decisions within a DRM driver, given that input events are often the first sign that we're going to start doing latency-sensitive activities:
Panel self-refresh: software-directed self-refresh (e.g., with Rockchip eDP) is especially latency sensitive. In some cases, it can take 10s of milliseconds for a panel to exit self-refresh, which can be noticeable. Rockchip RK3399 Chrome OS systems have always shipped with an input_handler boost, that preemptively exits self-refresh whenever there is input activity.
GPU drivers: on GPU-accelerated desktop systems, we may need to render new frames immediately after user activity. Powering up the GPU can take enough time that it is worthwhile to start this process as soon as there is input activity. Many Chrome OS systems also ship with an input_handler boost that powers up the GPU.
This patch provides a small helper library that abstracts some of the input-subsystem details around picking which devices to listen to, and some other boilerplate. This will be used in the next patch to implement the first bullet: preemptive exit for panel self-refresh.
Bits of this are adapted from code the Android and/or Chrome OS kernels have been carrying for a while.
Signed-off-by: Brian Norris briannorris@chromium.org
Thanks Simon for the CC.
Hi Brian,
while this feature in general makes sense and sounds good, to start warming up display hardware early when something might start to happen, this particular proposal has many problems from UAPI perspective (as it has none). Comments below.
Btw. if PSR is that slow to wake up from, how much do you actually gain from this input event watching? I would imagine the improvement to not be noticeable.
Patch 2 has details. It's not really about precisely how slow PSR is, but how much foresight we can gain: in patch 2, I note that with my particular user space and system, I can start PSR-exit 50ms earlier than I would otherweise. (FWIW, this measurement is exactly the same it was with the original version written 4 years ago.)
For how long PSR-exit takes: the measurements I'm able to do (via ftrace) show that drm_self_refresh_transition() takes between 35 and 55 ms. That's noticeable at 60 fps. And quite conveniently, the input-boost manages to hide nearly 100% of that latency.
Typical use cases where one notices PSR latency (and where this 35-55ms matters) involve simply moving a cursor; it's very noticeable when you have more than a few frames of latency to "get started".
Hi Brian,
that is very interesting, thanks.
I would never have expected to have userspace take *that* long to react. But, that sounds like it could be just your userspace software stack.
In the other subthread we're talking about making this more explicit. Maybe we need to combine this with a "I expect to take this many milliseconds to get the first frame out" value.
That way compositors which take 50ms (which frankly is shocking slow) can set that, and kms can enable sr exit (since sr exit will actually help here). But other compositors which expect to get the first frame out in maybe 20 can spec that, and then the driver will not sr exit (because too high chances we'll just make shit slower), and instead will only boost render clocks.
Thoughts?
I wonder if the compositor or the userspace stack can know how long it usually takes to prepare the first KMS submission after a pause. I guess it would need to measure that at runtime. Hmm, doable I guess, sure. Input to output latency in general is interesting.
However, that sounds like a pretty vague API with the delay value. I think it has a high risk of regressing into a boolean toggle by userspace choosing an arbitrary number and then assuming the threshold in the driver is always the same.
Thanks, pq