Re: [PATCHv2 0/7] Support inhibiting input devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans,

W dniu 18.05.2020 o 14:24, Hans de Goede pisze:
Hi,

On 5/18/20 12:48 PM, Andrzej Pietrasiewicz wrote:
Hi Hans,

W dniu 15.05.2020 o 20:19, Hans de Goede pisze:
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?

I also noticed that you keep the device open (do not call the
input_device's close callback) when inhibited and just throw away

I'm not sure if I understand you correctly, it is called:

+static inline void input_stop(struct input_dev *dev)
+{
+    if (dev->poller)
+        input_dev_poller_stop(dev->poller);
+    if (dev->close)
+        dev->close(dev);
                 ^^^^^^^^^^^^^^^^
+static int input_inhibit(struct input_dev *dev)
+{
+    int ret = 0;
+
+    mutex_lock(&dev->mutex);
+
+    if (dev->inhibited)
+        goto out;
+
+    if (dev->users) {
+        if (dev->inhibit) {
+            ret = dev->inhibit(dev);
+            if (ret)
+                goto out;
+        }
+        input_stop(dev);
                 ^^^^^^^^^^^^^^^^

It will not be called when dev->users is zero, but if it is zero,
then nobody has opened the device yet so there is nothing to close.

Ah, I missed that.

So if the device implements the inhibit call back then on
inhibit it will get both the inhibit and close callback called?


That's right. And conversely, upon uninhibit open() and uninhibit()
callbacks will be invoked. Please note that just as with open()/close(),
providing inhibit()/uninhibit() is optional.

And what happens if the last user goes away and the device
is not inhibited?

close() is called as usually.


I'm trying to understand here what the difference between the 2
is / what the goal of having a separate inhibit callback ?


Drivers have very different ideas about what it means to suspend/resume
and open/close. The optional inhibit/uninhibit callbacks are meant for
the drivers to know that it is this particular action going on.

For inhibit() there's one more argument: close() does not return a value,
so its meaning is "do some last cleanup" and as such it is not allowed
to fail - whatever its effect is, we must deem it successful. inhibit()
does return a value and so it is allowed to fail.

All in all, it is up to the drivers to decide which callback they
provide. Based on my work so far I would say that there are tens
of simple cases where open() and close() are sufficient, out of total
~400 users of input_allocate_device():

$ git grep "input_allocate_device(" | grep -v ^Documentation | \
cut -f1 -d: | sort | uniq | wc
    390     390   13496

Andrzej



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux