Re: [PATCH 1/2] drm/input_helper: Add new input-handling helper

[Rob Clark <robdclark@gmail.com>]

On Fri, Nov 12, 2021 at 4:52 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Nov 3, 2021 at 4:40 PM 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>
> > ---
> >
> >  drivers/gpu/drm/Makefile           |   3 +-
> >  drivers/gpu/drm/drm_input_helper.c | 143 +++++++++++++++++++++++++++++
> >  include/drm/drm_input_helper.h     |  22 +++++
> >  3 files changed, 167 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/drm_input_helper.c
> >  create mode 100644 include/drm/drm_input_helper.h
> >
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 0dff40bb863c..378761685b47 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -49,7 +49,8 @@ drm_kms_helper-y := drm_bridge_connector.o drm_crtc_helper.o drm_dp_helper.o \
> >                 drm_scdc_helper.o drm_gem_atomic_helper.o \
> >                 drm_gem_framebuffer_helper.o \
> >                 drm_atomic_state_helper.o drm_damage_helper.o \
> > -               drm_format_helper.o drm_self_refresh_helper.o
> > +               drm_format_helper.o drm_self_refresh_helper.o \
> > +               drm_input_helper.o
>
> Note that the Makefile part conflicts with drm-misc-next right now.
> It's not very hard to resolve, but since this would likely land there
> maybe it'd be nice to resolve?
>
> Other than that, this seems sane to me and much better than copying
> this between places, so assuming that the build problems get resolved
> then I'm happy.
>
> I guess one random thought I had is whether there would be an
> appropriate place to put this that _wasn't_ in DRM. I still wonder
> whether we'll ever try to upstream something like the cpufreq boost
> driver that we're carrying around and using in Chrome OS. If so, it
> would want to use these same helpers and it'd be pretty awkward for it
> to have to reach into DRM. ...any chance we could just land these
> helpers somewhere more generic?
>

One of the problems with cpu input boost is the cooldown period
(anti-spacebar-heater) aspect vs relatively short boost period.. in
that it is short enough that you need to consider that certain keys
(ie. modifier keys) should boost on key-release instead of key-press.

That isn't really an issue for triggers to exit PSR, or what we use
input-boost for downstream in drm/msm (where it isn't really boost,
just an early trigger to start powering up the GPU if it is suspended)

BR,
-R