Message ID | 20211103164002.1.I09b516eff75ead160a6582dd557e7e7e900c9e8e@changeid (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | drm: Support input-boosted panel self-refresh exit | expand |
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
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
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
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
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.
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
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/
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.
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
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 --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__ */
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