Linux-Samsung-soc Archive on lore.kernel.org
 help / color / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Peter Hutterer <peter.hutterer@who-t.net>
Cc: Hans de Goede <hdegoede@redhat.com>,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	linux-input@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-iio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-tegra@vger.kernel.org,
	patches@opensource.cirrus.com,
	ibm-acpi-devel@lists.sourceforge.net,
	platform-driver-x86@vger.kernel.org,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>, Jonathan Cameron <jic23@kernel.org>,
	Hartmut Knaack <knaack.h@gmx.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
	Kukjin Kim <kgene@kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Vladimir Zapolskiy <vz@mleia.com>,
	Sylvain Lemieux <slemieux.tyco@gmail.com>,
	Laxman Dewangan <ldewangan@nvidia.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Barry Song <baohua@kernel.org>,
	Michael Hennerich <michael.hennerich@analog.com>,
	Nick Dyer <nick@shmanahar.org>, Ferruh Yigit <fery@cypress.com>,
	Sangwon Jee <jeesw@melfas.com>,
	Henrique de Moraes Holschuh <ibm-acpi@hmh.eng.br>,
	kernel@collabora.com, Peter Hutterer <peter.hutterer@redhat.com>,
	Benjamin Tissoires <btissoir@redhat.com>
Subject: Re: [PATCHv2 0/7] Support inhibiting input devices
Date: Sun, 17 May 2020 19:40:34 -0700
Message-ID: <20200518024034.GL89269@dtor-ws> (raw)
In-Reply-To: <20200517225510.GA205823@koala>

Hi Hans, Peter,

On Mon, May 18, 2020 at 08:55:10AM +1000, Peter Hutterer wrote:
> On Fri, May 15, 2020 at 08:19:10PM +0200, Hans de Goede wrote:
> > Hi Andrezj,
> > 
> > On 5/15/20 6:49 PM, Andrzej Pietrasiewicz wrote:
> > > Userspace might want to implement a policy to temporarily disregard input
> > > from certain devices, including not treating them as wakeup sources.
> > > 
> > > An example use case is a laptop, whose keyboard can be folded under the
> > > screen to create tablet-like experience. The user then must hold the laptop
> > > in such a way that it is difficult to avoid pressing the keyboard keys. It
> > > is therefore desirable to temporarily disregard input from the keyboard,
> > > until it is folded back. This obviously is a policy which should be kept
> > > out of the kernel, but the kernel must provide suitable means to implement
> > > such a policy.
> > 
> > Actually libinput already binds together (inside libinput) SW_TABLET_MODE
> > generating evdev nodes and e.g. internal keyboards on devices with 360°
> > hinges for this reason. libinput simply closes the /dev/input/event#
> > node when folded and re-opens it when the keyboard should become active
> > again. Thus not only suppresses events but allows e.g. touchpads to
> > enter runtime suspend mode which saves power. Typically closing the
> > /dev/input/event# node will also disable the device as wakeup source.
> > 
> > So I wonder what this series actually adds for functionality for
> > userspace which can not already be achieved this way?
> 
> Thanks Hans. To expand on this:
> libinput has heuristics to guess which input devices (keyboards, touchpads)
> are built-in ones. When the tablet mode switch is on, we disable these
> devices internally (this is not visible to callers), and re-enable it again
> later when the tablet mode switch is off again.

I think that is great that libinput has tried solving this for the
tablet mode, but unfortunately libinput only works for users of
libinput, leaving cases such as:

1. In-kernel input handlers, such as SysRq, VT and others
2. Systems that do not rely on libinput for userspace handing (Android,
Chrome OS)
3. Systems with policies that are more complex than tablet mode only.

Because of libinput's inability to affect the kernel, and the presence
of "always on" input handlers (sysrq, VT keyboard, potentially others),
while libinput may control whether consumers receive events from certain
input devices, it will not allow power savings that an explicit
"inhibit" allows when coming from dedicated power policy manager.

I think pushing policy decisions into a library, and trying to have all
clients agree with it, is much harder and leaks unnecessary knowledge
into quite a few layers. A dedicated power policy manager, that is not
only responsible for input device, but power state of the system as a
whole, is a very viable architecture.

> 
> This is done for keyboards and touchpads atm (and I think pointing sticks)
> and where the heuristics fail we have extra quirks in place. For example
> the Lenovo Yogas tend to disable the keyboard mechanically in tablet mode
> but buttons (e.g. volume keys) around the screen send events through the
> same event node. So on those devices we don't disable the keyboard.
> 
> We've had this code for a few years now and the only changes to it have been
> the various device quirks for devices that must not suspend the keyboard,
> it's otherwise working as expected.
> 
> If we ever have a device where we need to disable parts of the keyboard
> only, we could address this with EVIOCSMASK but so far that hasn't been
> necessary.
> 
> I agree with Hans, right now I don't see the usefulness of this new sysfs
> toggle. For it to be really useful you'd have to guarantee that it's
> available for 100% of the devices and that's IMO unlikely to happen.

The inhibiting of the events works for 100% of input devices, the power
savings work for the ones that implement it. It is responsibility of
folds shipping the systems to make sure drivers they use support inhibit
if they believe it will help their battery life.

> 
> Cheers,
>    Peter
> 
> > I also noticed that you keep the device open (do not call the
> > input_device's close callback) when inhibited and just throw away
> > any events generated. This seems inefficient and may lead to
> > the internal state getting out of sync. What if a key is pressed
> > while inhibited and then the device is uninhibited while the key
> > is still pressed?  Now the press event is lost and userspace
> > querying the current state will see the pressed key as being
> > released.

This is a good point. We should look into signalling that some events
have been dropped (via EV_SYN/SYN_DROPPED) so that clients are aware of
it.

> > 
> > On top of this you add special inhibit and uninhibit callbacks
> > and implement those for just a few devices. How do these differ
> > from just closing the device and later opening it again ?

I believe majority will simply reuse open/close callbacks. In Chrome OS
we have dedicated inhibit/uninhibit, but I would like to allow using
open/close as alternatives.

> > 
> > Also using a sysfs property for this is very weird given that the
> > rest of the evdev interface is using ioctls for everything...

This is not evdev interface, it is at the level above evdev (so that it
can affect all handlers, not only evdev). As such it is not bound by
evdev interface.

Thanks.

-- 
Dmitry

  reply index

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200506002746.GB89269@dtor-ws>
2020-05-15 16:49 ` Andrzej Pietrasiewicz
2020-05-15 16:51   ` [PATCHv2 1/7] Input: add input_device_enabled() Andrzej Pietrasiewicz
2020-05-15 16:52   ` [PATCHv2 4/7] iio: adc: exynos: Use input_device_enabled() Andrzej Pietrasiewicz
2020-05-15 16:52   ` [PATCHv2 6/7] Input: Add "inhibited" property Andrzej Pietrasiewicz
2020-05-15 18:19   ` [PATCHv2 0/7] Support inhibiting input devices Hans de Goede
2020-05-17 22:55     ` Peter Hutterer
2020-05-18  2:40       ` Dmitry Torokhov [this message]
2020-05-18  7:36         ` Hans de Goede
2020-05-22 15:35         ` Andrzej Pietrasiewicz
2020-05-27  6:13           ` Peter Hutterer
2020-05-18 10:48     ` Andrzej Pietrasiewicz
2020-05-18 12:24       ` Hans de Goede
2020-05-18 13:49         ` Andrzej Pietrasiewicz
2020-05-18 14:23           ` Hans de Goede
2020-05-19  9:02             ` Andrzej Pietrasiewicz
2020-05-19  9:36               ` Hans de Goede
2020-05-27  6:34                 ` Dmitry Torokhov
2020-06-02 16:56                   ` Andrzej Pietrasiewicz
2020-06-02 17:52                     ` Dmitry Torokhov
2020-06-02 18:50                       ` Andrzej Pietrasiewicz
2020-06-02 20:19                         ` Hans de Goede
2020-06-03 13:07                           ` Andrzej Pietrasiewicz
2020-06-03 17:38                             ` Hans de Goede
2020-06-03 17:54                               ` Andrzej Pietrasiewicz
2020-06-03 19:37                                 ` Hans de Goede
2020-06-04  7:28                                   ` Dmitry Torokhov
2020-06-05 17:33                                     ` [PATCH v3 " Andrzej Pietrasiewicz
2020-06-05 17:33                                       ` [PATCH v3 1/7] Input: add input_device_enabled() Andrzej Pietrasiewicz
2020-06-05 17:33                                       ` [PATCH v3 2/7] Input: use input_device_enabled() Andrzej Pietrasiewicz
2020-06-05 17:33                                       ` [PATCH v3 3/7] ACPI: button: Access input device's users under appropriate mutex Andrzej Pietrasiewicz
2020-06-05 17:33                                       ` [PATCH v3 4/7] ACPI: button: Use input_device_enabled() helper Andrzej Pietrasiewicz
2020-06-05 17:33                                       ` [PATCH v3 5/7] iio: adc: exynos: Use input_device_enabled() Andrzej Pietrasiewicz
2020-06-05 19:49                                         ` Michał Mirosław
2020-06-05 17:33                                       ` [PATCH v3 6/7] platform/x86: thinkpad_acpi: " Andrzej Pietrasiewicz
2020-06-05 17:33                                       ` [PATCH v3 7/7] Input: Add "inhibited" property Andrzej Pietrasiewicz
2020-06-05 17:41                                         ` Hans de Goede
2020-06-08 11:22                                           ` [PATCH v4 0/7] Support inhibiting input devices Andrzej Pietrasiewicz
2020-06-08 11:22                                             ` [PATCH v4 1/7] Input: add input_device_enabled() Andrzej Pietrasiewicz
2020-06-08 11:22                                             ` [PATCH v4 2/7] Input: use input_device_enabled() Andrzej Pietrasiewicz
2020-06-08 11:22                                             ` [PATCH v4 3/7] ACPI: button: Access input device's users under appropriate mutex Andrzej Pietrasiewicz
2020-06-24 15:00                                               ` Rafael J. Wysocki
2020-06-25  5:23                                                 ` Dmitry Torokhov
2020-06-25 10:55                                                   ` Rafael J. Wysocki
2020-06-08 11:22                                             ` [PATCH v4 4/7] ACPI: button: Use input_device_enabled() helper Andrzej Pietrasiewicz
2020-06-25  5:24                                               ` Dmitry Torokhov
2020-06-08 11:22                                             ` [PATCH v4 5/7] iio: adc: exynos: Use input_device_enabled() Andrzej Pietrasiewicz
2020-06-10  1:28                                               ` Michał Mirosław
2020-06-10  7:52                                                 ` [FIXED PATCH " Andrzej Pietrasiewicz
2020-06-08 11:22                                             ` [PATCH v4 6/7] platform/x86: thinkpad_acpi: " Andrzej Pietrasiewicz
2020-06-08 11:22                                             ` [PATCH v4 7/7] Input: Add "inhibited" property Andrzej Pietrasiewicz
2020-06-10  9:49                                             ` [PATCH v4 0/7] Support inhibiting input devices Hans de Goede
2020-06-10 10:38                                               ` Rafael J. Wysocki
2020-06-10 13:12                                                 ` Andrzej Pietrasiewicz
2020-06-10 13:21                                                   ` Hans de Goede
2020-06-10 13:41                                                     ` Andrzej Pietrasiewicz
2020-06-12  8:30                                                       ` Hans de Goede
2020-06-12  8:47                                                         ` Andrzej Pietrasiewicz
2020-06-16 17:29                                                         ` [PATCH] Input: document inhibiting Andrzej Pietrasiewicz
2020-06-16 17:38                                                           ` Randy Dunlap
2020-06-17  7:44                                                           ` Hans de Goede
2020-06-17 10:18                                                             ` [PATCH v2] " Andrzej Pietrasiewicz
2020-06-17 10:21                                                               ` Hans de Goede
2020-06-17 16:52                                                               ` Randy Dunlap
2020-06-23 13:35                                                               ` Pavel Machek
2020-06-10 14:01                                                     ` [PATCH v4 0/7] Support inhibiting input devices Rafael J. Wysocki
2020-06-10 13:52                                                 ` Hans de Goede
2020-06-10 18:28                                                 ` Dmitry Torokhov
2020-06-12  8:14                                                   ` Hans de Goede
2020-06-12  8:17                                             ` Hans de Goede
2020-08-03 14:40                                               ` Andrzej Pietrasiewicz
2020-06-07 20:24                                       ` [PATCH v3 " Pavel Machek
2020-06-08  5:37                                         ` Dmitry Torokhov
2020-06-08  9:28                                           ` Andrzej Pietrasiewicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200518024034.GL89269@dtor-ws \
    --to=dmitry.torokhov@gmail.com \
    --cc=andrzej.p@collabora.com \
    --cc=baohua@kernel.org \
    --cc=btissoir@redhat.com \
    --cc=fery@cypress.com \
    --cc=festevam@gmail.com \
    --cc=hdegoede@redhat.com \
    --cc=ibm-acpi-devel@lists.sourceforge.net \
    --cc=ibm-acpi@hmh.eng.br \
    --cc=jeesw@melfas.com \
    --cc=jic23@kernel.org \
    --cc=jonathanh@nvidia.com \
    --cc=kernel@collabora.com \
    --cc=kernel@pengutronix.de \
    --cc=kgene@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=krzk@kernel.org \
    --cc=lars@metafoo.de \
    --cc=ldewangan@nvidia.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=michael.hennerich@analog.com \
    --cc=nick@shmanahar.org \
    --cc=patches@opensource.cirrus.com \
    --cc=peter.hutterer@redhat.com \
    --cc=peter.hutterer@who-t.net \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    --cc=rjw@rjwysocki.net \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=slemieux.tyco@gmail.com \
    --cc=thierry.reding@gmail.com \
    --cc=vz@mleia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Samsung-soc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-samsung-soc/0 linux-samsung-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-samsung-soc linux-samsung-soc/ https://lore.kernel.org/linux-samsung-soc \
		linux-samsung-soc@vger.kernel.org
	public-inbox-index linux-samsung-soc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-samsung-soc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git