diff mbox series

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

Message ID 20211103164002.1.I09b516eff75ead160a6582dd557e7e7e900c9e8e@changeid (mailing list archive)
State Superseded
Headers show
Series drm: Support input-boosted panel self-refresh exit | expand

Commit Message

Brian Norris Nov. 3, 2021, 11:40 p.m. UTC
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

Comments

kernel test robot Nov. 4, 2021, 10:47 a.m. UTC | #1
Hi Brian,

I love your patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v5.15 next-20211104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Brian-Norris/drm-Support-input-boosted-panel-self-refresh-exit/20211104-074200
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: openrisc-randconfig-c004-20211104 (attached as .config)
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/57dffacf2caa97c6d4c205ff6d43ffaa200ab54a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Brian-Norris/drm-Support-input-boosted-panel-self-refresh-exit/20211104-074200
        git checkout 57dffacf2caa97c6d4c205ff6d43ffaa200ab54a
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=openrisc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   or1k-linux-ld: drivers/gpu/drm/drm_input_helper.o: in function `drm_input_disconnect':
>> drm_input_helper.c:(.text+0x48): undefined reference to `input_close_device'
   drm_input_helper.c:(.text+0x48): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `input_close_device'
>> or1k-linux-ld: drm_input_helper.c:(.text+0x50): undefined reference to `input_unregister_handle'
   drm_input_helper.c:(.text+0x50): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `input_unregister_handle'
   or1k-linux-ld: drivers/gpu/drm/drm_input_helper.o: in function `drm_input_connect':
>> drm_input_helper.c:(.text+0xd0): undefined reference to `input_register_handle'
   drm_input_helper.c:(.text+0xd0): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `input_register_handle'
>> or1k-linux-ld: drm_input_helper.c:(.text+0xe4): undefined reference to `input_open_device'
   drm_input_helper.c:(.text+0xe4): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `input_open_device'
   or1k-linux-ld: drm_input_helper.c:(.text+0xf8): undefined reference to `input_unregister_handle'
   drm_input_helper.c:(.text+0xf8): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `input_unregister_handle'
   or1k-linux-ld: drivers/gpu/drm/drm_input_helper.o: in function `drm_input_handle_register':
>> drm_input_helper.c:(.text+0x1d4): undefined reference to `input_register_handler'
   drm_input_helper.c:(.text+0x1d4): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `input_register_handler'
   or1k-linux-ld: drivers/gpu/drm/drm_input_helper.o: in function `drm_input_handle_unregister':
>> drm_input_helper.c:(.text+0x234): undefined reference to `input_unregister_handler'
   drm_input_helper.c:(.text+0x234): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `input_unregister_handler'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Nov. 4, 2021, 1:20 p.m. UTC | #2
Hi Brian,

I love your patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v5.15 next-20211104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Brian-Norris/drm-Support-input-boosted-panel-self-refresh-exit/20211104-074200
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: arm-randconfig-r001-20211104 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 847a6807332b13f43704327c2d30103ec0347c77)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/57dffacf2caa97c6d4c205ff6d43ffaa200ab54a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Brian-Norris/drm-Support-input-boosted-panel-self-refresh-exit/20211104-074200
        git checkout 57dffacf2caa97c6d4c205ff6d43ffaa200ab54a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "input_unregister_handler" [drivers/gpu/drm/drm_kms_helper.ko] undefined!
>> ERROR: modpost: "input_close_device" [drivers/gpu/drm/drm_kms_helper.ko] undefined!
>> ERROR: modpost: "input_unregister_handle" [drivers/gpu/drm/drm_kms_helper.ko] undefined!
>> ERROR: modpost: "input_open_device" [drivers/gpu/drm/drm_kms_helper.ko] undefined!
>> ERROR: modpost: "input_register_handle" [drivers/gpu/drm/drm_kms_helper.ko] undefined!
>> ERROR: modpost: "input_register_handler" [drivers/gpu/drm/drm_kms_helper.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Nov. 4, 2021, 1:46 p.m. UTC | #3
Hi Brian,

I love your patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v5.15 next-20211104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Brian-Norris/drm-Support-input-boosted-panel-self-refresh-exit/20211104-074200
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: nios2-randconfig-r012-20211104 (attached as .config)
compiler: nios2-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/57dffacf2caa97c6d4c205ff6d43ffaa200ab54a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Brian-Norris/drm-Support-input-boosted-panel-self-refresh-exit/20211104-074200
        git checkout 57dffacf2caa97c6d4c205ff6d43ffaa200ab54a
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   nios2-linux-ld: drivers/gpu/drm/drm_input_helper.o: in function `drm_input_handle_register':
   drm_input_helper.c:(.text+0xa4): undefined reference to `input_register_handler'
   drm_input_helper.c:(.text+0xa4): relocation truncated to fit: R_NIOS2_CALL26 against `input_register_handler'
   nios2-linux-ld: drivers/gpu/drm/drm_input_helper.o: in function `drm_input_disconnect':
   drm_input_helper.c:(.text+0xf0): undefined reference to `input_close_device'
   drm_input_helper.c:(.text+0xf0): relocation truncated to fit: R_NIOS2_CALL26 against `input_close_device'
>> nios2-linux-ld: drm_input_helper.c:(.text+0xf8): undefined reference to `input_unregister_handle'
   drm_input_helper.c:(.text+0xf8): relocation truncated to fit: R_NIOS2_CALL26 against `input_unregister_handle'
   nios2-linux-ld: drivers/gpu/drm/drm_input_helper.o: in function `drm_input_handle_unregister':
   drm_input_helper.c:(.text+0x124): undefined reference to `input_unregister_handler'
   drm_input_helper.c:(.text+0x124): relocation truncated to fit: R_NIOS2_CALL26 against `input_unregister_handler'
   nios2-linux-ld: drivers/gpu/drm/drm_input_helper.o: in function `drm_input_connect':
   drm_input_helper.c:(.text+0x18c): undefined reference to `input_register_handle'
   drm_input_helper.c:(.text+0x18c): relocation truncated to fit: R_NIOS2_CALL26 against `input_register_handle'
>> nios2-linux-ld: drm_input_helper.c:(.text+0x19c): undefined reference to `input_open_device'
   drm_input_helper.c:(.text+0x19c): relocation truncated to fit: R_NIOS2_CALL26 against `input_open_device'
   nios2-linux-ld: drm_input_helper.c:(.text+0x1ac): undefined reference to `input_unregister_handle'
   drm_input_helper.c:(.text+0x1ac): relocation truncated to fit: R_NIOS2_CALL26 against `input_unregister_handle'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Brian Norris Nov. 4, 2021, 9:45 p.m. UTC | #4
On Thu, Nov 04, 2021 at 06:47:34PM +0800, kernel test robot wrote:
> Hi Brian,

Hi robot!

> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>

I'm not sure how to do that in a v2 patch -- one doesn't normally credit
"Reported-by" for every testing/review improvement on an in-development
patch series.

> All errors (new ones prefixed by >>):
> 
>    or1k-linux-ld: drivers/gpu/drm/drm_input_helper.o: in function `drm_input_disconnect':
> >> drm_input_helper.c:(.text+0x48): undefined reference to `input_close_device'

I've cooked a local change to separate the CONFIG_INPUT dependencies
under another Kconfig symbol which optionally builds this stuff into the
drm helper module, only when CONFIG_INPUT is present. That should fix
the build issues while still leaving DRM_KMS_HELPER workable with or
without CONFIG_INPUT.

I'll wait to see if there's any other feedback before spinning though.

Brian
Dmitry Torokhov Nov. 5, 2021, 11:11 p.m. UTC | #5
On Wed, Nov 03, 2021 at 04:40:17PM -0700, Brian Norris 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.

If this is acceptable to DRM folks I am OK with this from input POV. The
only think I would call out that the callback is executed in atomic
context with interrupts disabled.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.
Doug Anderson Nov. 13, 2021, 12:52 a.m. UTC | #6
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?


> +struct drm_input_handler {
> +       struct input_handler handler;
> +       void *priv;
> +       void (*callback)(void *priv);
> +};

Super bike-sheddy comment is that "void *" data arguments are not
super common in the kernel. I would have expected the callback
function to just be passed a pointer to the "struct drm_input_handler"
and it could find any other data it needed via "container_of". I won't
insist though. ;-)


-Doug
Brian Norris Nov. 17, 2021, 6:38 p.m. UTC | #7
On Fri, Nov 12, 2021 at 4:52 PM Doug Anderson <dianders@chromium.org> wrote:

(Snip other comments; they seem reasonable, and I'll factor them into
the next version)

> 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?

Yeah, I was torn on what to do here as well. I'd rather land something
than nothing, and when reading past conversations, it sounded like
Dmitry didn't want this kind of thing in drivers/input/ [1]. I'd love
to be wrong here though.

I'm not sure where else this would belong though -- either in the
producing subsystem (input) or the consuming one(s) (drm, cpufreq). We
could make up some odd middle ground I suppose (lib/?), but that seems
pretty artificial.

I guess one question is, what is this abstracting, and is that
abstraction actually a shared need for multiple subsystems? I think
the abstraction is, "impending user activity; <component X> should
prepare itself". That general need is exactly the same for the cases
I'm aware of. And if there is any tuning needed (e.g., ignore input
device Y; or turn the whole thing off, because we're ignoring input
for now), that would also seem to be a shared need.

Anyway, back to my first paragraph: I'll plan on keeping this as-is
(as a DRM helper) unless I hear otherwise from input folks.

Brian

[1] https://lore.kernel.org/all/20180416174117.GA77055@dtor-ws/
Dmitry Torokhov Nov. 17, 2021, 8:47 p.m. UTC | #8
On Wed, Nov 17, 2021 at 10:38:58AM -0800, Brian Norris wrote:
> On Fri, Nov 12, 2021 at 4:52 PM Doug Anderson <dianders@chromium.org> wrote:
> 
> (Snip other comments; they seem reasonable, and I'll factor them into
> the next version)
> 
> > 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?
> 
> Yeah, I was torn on what to do here as well. I'd rather land something
> than nothing, and when reading past conversations, it sounded like
> Dmitry didn't want this kind of thing in drivers/input/ [1]. I'd love
> to be wrong here though.

I simply feel that input_handler is already a very simple abstraction
and trying to specialize it to simplify users further is not productive.

> 
> I'm not sure where else this would belong though -- either in the
> producing subsystem (input) or the consuming one(s) (drm, cpufreq). We
> could make up some odd middle ground I suppose (lib/?), but that seems
> pretty artificial.
> 
> I guess one question is, what is this abstracting, and is that
> abstraction actually a shared need for multiple subsystems? I think
> the abstraction is, "impending user activity; <component X> should
> prepare itself". That general need is exactly the same for the cases
> I'm aware of. And if there is any tuning needed (e.g., ignore input
> device Y; or turn the whole thing off, because we're ignoring input
> for now), that would also seem to be a shared need.
> 
> Anyway, back to my first paragraph: I'll plan on keeping this as-is
> (as a DRM helper) unless I hear otherwise from input folks.
> 
> Brian
> 
> [1] https://lore.kernel.org/all/20180416174117.GA77055@dtor-ws/

Thanks.
Doug Anderson Nov. 17, 2021, 8:56 p.m. UTC | #9
Hi,

On Wed, Nov 17, 2021 at 12:47 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Wed, Nov 17, 2021 at 10:38:58AM -0800, Brian Norris wrote:
> > On Fri, Nov 12, 2021 at 4:52 PM Doug Anderson <dianders@chromium.org> wrote:
> >
> > (Snip other comments; they seem reasonable, and I'll factor them into
> > the next version)
> >
> > > 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?
> >
> > Yeah, I was torn on what to do here as well. I'd rather land something
> > than nothing, and when reading past conversations, it sounded like
> > Dmitry didn't want this kind of thing in drivers/input/ [1]. I'd love
> > to be wrong here though.
>
> I simply feel that input_handler is already a very simple abstraction
> and trying to specialize it to simplify users further is not productive.

I guess, if nothing else, it would be nice to avoid the tables that
we'd have to copy between DRM and cpufreq: the set of input devices
that are likely a sign that the user is interacting with the device.
It always seemed weird to copy that from place to place and if there's
ever a new input device to add it would be annoying to have to update
it everywhere.

It would be nice to avoid some of the other boilerplate code here
connecting things together when all we need is a callback, but I agree
that if those were copied it wouldn't be the end of the world.

-Doug
Rob Clark Nov. 17, 2021, 10:27 p.m. UTC | #10
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
diff mbox series

Patch

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
 
 drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
 drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_input_helper.c b/drivers/gpu/drm/drm_input_helper.c
new file mode 100644
index 000000000000..a31efc0d3030
--- /dev/null
+++ b/drivers/gpu/drm/drm_input_helper.c
@@ -0,0 +1,143 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Google, Inc.
+ */
+#include <linux/input.h>
+#include <linux/slab.h>
+
+#include <drm/drm_device.h>
+#include <drm/drm_input_helper.h>
+
+/**
+ * DOC: overview
+ *
+ * This helper library provides a thin wrapper around input handles, so that
+ * DRM drivers can easily perform domain-specific actions in response to user
+ * activity. e.g., if someone is moving a mouse, we're likely to want to
+ * display something soon, and we should exit panel self-refresh.
+ */
+
+static void drm_input_event(struct input_handle *handle, unsigned int type,
+			    unsigned int code, int value)
+{
+	struct drm_input_handler *handler = handle->handler->private;
+
+	handler->callback(handler->priv);
+}
+
+static int drm_input_connect(struct input_handler *handler,
+			     struct input_dev *dev,
+			     const struct input_device_id *id)
+{
+	struct input_handle *handle;
+	int error;
+
+	handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
+	if (!handle)
+		return -ENOMEM;
+
+	handle->dev = dev;
+	handle->handler = handler;
+	handle->name = "drm-input-helper";
+
+	error = input_register_handle(handle);
+	if (error)
+		goto err2;
+
+	error = input_open_device(handle);
+	if (error)
+		goto err1;
+
+	return 0;
+
+err1:
+	input_unregister_handle(handle);
+err2:
+	kfree(handle);
+	return error;
+}
+
+static void drm_input_disconnect(struct input_handle *handle)
+{
+	input_close_device(handle);
+	input_unregister_handle(handle);
+	kfree(handle);
+}
+
+static const struct input_device_id drm_input_ids[] = {
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+			 INPUT_DEVICE_ID_MATCH_ABSBIT,
+		.evbit = { BIT_MASK(EV_ABS) },
+		.absbit = { [BIT_WORD(ABS_MT_POSITION_X)] =
+			    BIT_MASK(ABS_MT_POSITION_X) |
+			    BIT_MASK(ABS_MT_POSITION_Y) },
+	}, /* multi-touch touchscreen */
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT,
+		.evbit = { BIT_MASK(EV_ABS) },
+		.absbit = { [BIT_WORD(ABS_X)] = BIT_MASK(ABS_X) }
+
+	}, /* stylus or joystick device */
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT,
+		.evbit = { BIT_MASK(EV_KEY) },
+		.keybit = { [BIT_WORD(BTN_LEFT)] = BIT_MASK(BTN_LEFT) },
+	}, /* pointer (e.g. trackpad, mouse) */
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT,
+		.evbit = { BIT_MASK(EV_KEY) },
+		.keybit = { [BIT_WORD(KEY_ESC)] = BIT_MASK(KEY_ESC) },
+	}, /* keyboard */
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+			 INPUT_DEVICE_ID_MATCH_KEYBIT,
+		.evbit = { BIT_MASK(EV_KEY) },
+		.keybit = {[BIT_WORD(BTN_JOYSTICK)] = BIT_MASK(BTN_JOYSTICK) },
+	}, /* joysticks not caught by ABS_X above */
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT |
+			 INPUT_DEVICE_ID_MATCH_KEYBIT,
+		.evbit = { BIT_MASK(EV_KEY) },
+		.keybit = { [BIT_WORD(BTN_GAMEPAD)] = BIT_MASK(BTN_GAMEPAD) },
+	}, /* gamepad */
+	{ },
+};
+
+int drm_input_handle_register(struct drm_device *dev,
+			      struct drm_input_handler *handler)
+{
+	int ret;
+
+	if (!handler->callback)
+		return -EINVAL;
+
+	handler->handler.event = drm_input_event;
+	handler->handler.connect = drm_input_connect;
+	handler->handler.disconnect = drm_input_disconnect;
+	handler->handler.name = kasprintf(GFP_KERNEL, "drm-input-helper-%s",
+					  dev_name(dev->dev));
+	if (!handler->handler.name)
+		return -ENOMEM;
+
+	handler->handler.id_table = drm_input_ids;
+	handler->handler.private = handler;
+
+	ret = input_register_handler(&handler->handler);
+	if (ret)
+		goto err;
+
+	return 0;
+
+err:
+	kfree(handler->handler.name);
+	return ret;
+}
+EXPORT_SYMBOL(drm_input_handle_register);
+
+void drm_input_handle_unregister(struct drm_input_handler *handler)
+{
+	input_unregister_handler(&handler->handler);
+	kfree(handler->handler.name);
+}
+EXPORT_SYMBOL(drm_input_handle_unregister);
diff --git a/include/drm/drm_input_helper.h b/include/drm/drm_input_helper.h
new file mode 100644
index 000000000000..b4d68896830c
--- /dev/null
+++ b/include/drm/drm_input_helper.h
@@ -0,0 +1,22 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2021 Google, Inc.
+ */
+#ifndef __DRM_INPUT_HELPER_H__
+#define __DRM_INPUT_HELPER_H__
+
+#include <linux/input.h>
+
+struct drm_device;
+
+struct drm_input_handler {
+	struct input_handler handler;
+	void *priv;
+	void (*callback)(void *priv);
+};
+
+int drm_input_handle_register(struct drm_device *dev,
+			      struct drm_input_handler *handler);
+void drm_input_handle_unregister(struct drm_input_handler *handler);
+
+#endif /* __DRM_INPUT_HELPER_H__ */