[RFC] Reporting "orientation changed" event

[RFC] Reporting "orientation changed" event

[Anisse Astier]

Pegatron Lucid tablet sends an ACPI hotkey event(0xEA) when the
accelerometer detects coarse orientation change. My initial thought was
to just translate this event into KEY_DIRECTION, which seems to be the
norm from what we can see in hp-wmi driver. (See patch below for an
implementation.)

But this isn't just a key per se like on the HP touchsmart tablet, this
is an event that is triggered when the device is rotated.

This could be defined as a new Misc (EV_MSC) event:
MSC_ORIENTATION_CHANGED ?

Or we could use the upcoming IIO subsystem which is supposed to be for
sensors, but then we'd have a mismatch between the device based on ACPI
with firmware in the middle (driver submitted by Andy Ross) and the
purpose of IIO ("SPI or I2C device").


Maybe I'm over-thinking this and this might be just right:

From: Anisse Astier <anisse@astier.eu>
Subject: [PATCH RFC] asus-laptop: Send input key for tablet rotation on Pegatron Lucid Tablet

Pegatron Lucid tablet sends ACPI event on coarse orientation changes.
Translate this into KEY_DIRECTION input event.

Cc: Dmitry Torokhov <dtor@mail.ru>
Cc: Matthew Garrett <mjg@redhat.com>
Cc: Andy Ross <andy.ross@windriver.com>
Cc: Corentin Chary <corentin.chary@gmail.com>
Signed-off-by: Anisse Astier <anisse@astier.eu>
---
Please note that this patch depends on Andy Ross's first patch in order for
asus-laptop to bind with the tablet.

Regards,

Anisse
---
 drivers/platform/x86/asus-laptop.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 5c23b20..decb958 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -312,6 +312,8 @@ static const struct key_entry asus_keymap[] = {
 	{KE_KEY, 0xc4, { KEY_KBDILLUMUP } },
 	{KE_KEY, 0xc5, { KEY_KBDILLUMDOWN } },
 	{KE_KEY, 0xb5, { KEY_CALC } },
+	/* Pegatron Lucid tablet specific */
+	{KE_KEY, 0xEA, { KEY_DIRECTION } }, /* Orientation changed */
 	{KE_END, 0},
 };
 
-- 
1.7.3.2

Re: [RFC] Reporting "orientation changed" event

[Bastien Nocera]

On Thu, 2011-04-07 at 18:18 +0200, Anisse Astier wrote:
> Pegatron Lucid tablet sends an ACPI hotkey event(0xEA) when the
> accelerometer detects coarse orientation change. My initial thought was
> to just translate this event into KEY_DIRECTION, which seems to be the
> norm from what we can see in hp-wmi driver. (See patch below for an
> implementation.)
> 
> But this isn't just a key per se like on the HP touchsmart tablet, this
> is an event that is triggered when the device is rotated.
> 
> This could be defined as a new Misc (EV_MSC) event:
> MSC_ORIENTATION_CHANGED ?
> 
> Or we could use the upcoming IIO subsystem which is supposed to be for
> sensors, but then we'd have a mismatch between the device based on ACPI
> with firmware in the middle (driver submitted by Andy Ross) and the
> purpose of IIO ("SPI or I2C device").

How would one know which orientation the device is now in, either from
the event, or when the orientation already changed? How would this be
"key" to user-space in general, and X in particular?

Re: [RFC] Reporting "orientation changed" event

[Anisse Astier]

On Thu, 07 Apr 2011 17:37:35 +0100, Bastien Nocera <hadess@hadess.net> wrote :

> On Thu, 2011-04-07 at 18:18 +0200, Anisse Astier wrote:
> > Pegatron Lucid tablet sends an ACPI hotkey event(0xEA) when the
> > accelerometer detects coarse orientation change. My initial thought was
> > to just translate this event into KEY_DIRECTION, which seems to be the
> > norm from what we can see in hp-wmi driver. (See patch below for an
> > implementation.)
> > 
> > But this isn't just a key per se like on the HP touchsmart tablet, this
> > is an event that is triggered when the device is rotated.
> > 
> > This could be defined as a new Misc (EV_MSC) event:
> > MSC_ORIENTATION_CHANGED ?
> > 
> > Or we could use the upcoming IIO subsystem which is supposed to be for
> > sensors, but then we'd have a mismatch between the device based on ACPI
> > with firmware in the middle (driver submitted by Andy Ross) and the
> > purpose of IIO ("SPI or I2C device").
> 
> How would one know which orientation the device is now in, either from
> the event, or when the orientation already changed? How would this be
> "key" to user-space in general, and X in particular?
> 

There's an accelerometer driver (the one I referenced as submitted by
Andy Ross), but it only works in polling mode (input-polled). With this
event, userspace doesn't have to use this polled device until orientation
changes, hence saving CPU, power, etc.

See http://www.spinics.net/lists/platform-driver-x86/msg01682.html for
the patches.

Regards,

Anisse

Re: [RFC] Reporting "orientation changed" event

[Jonathan Cameron]

On 04/07/11 17:18, Anisse Astier wrote:
> 
> Pegatron Lucid tablet sends an ACPI hotkey event(0xEA) when the
> accelerometer detects coarse orientation change. My initial thought was
> to just translate this event into KEY_DIRECTION, which seems to be the
> norm from what we can see in hp-wmi driver. (See patch below for an
> implementation.)
> 
> But this isn't just a key per se like on the HP touchsmart tablet, this
> is an event that is triggered when the device is rotated.
> 
> This could be defined as a new Misc (EV_MSC) event:
> MSC_ORIENTATION_CHANGED ?
> 
> Or we could use the upcoming IIO subsystem which is supposed to be for
> sensors, but then we'd have a mismatch between the device based on ACPI
> with firmware in the middle (driver submitted by Andy Ross) and the
> purpose of IIO ("SPI or I2C device").
Don't take that too literally. It's examples of types of bus IIO sensors use.
Having said this, if your main use is user input then it should probably be
handled through input rather than IIO anyway.

> 
> Maybe I'm over-thinking this and this might be just right:
> 
> From: Anisse Astier <anisse@astier.eu>
> Subject: [PATCH RFC] asus-laptop: Send input key for tablet rotation on Pegatron Lucid Tablet
> 
> Pegatron Lucid tablet sends ACPI event on coarse orientation changes.
> Translate this into KEY_DIRECTION input event.
> 
> Cc: Dmitry Torokhov <dtor@mail.ru>
> Cc: Matthew Garrett <mjg@redhat.com>
> Cc: Andy Ross <andy.ross@windriver.com>
> Cc: Corentin Chary <corentin.chary@gmail.com>
> Signed-off-by: Anisse Astier <anisse@astier.eu>
> ---
> Please note that this patch depends on Andy Ross's first patch in order for
> asus-laptop to bind with the tablet.
> 
> Regards,
> 
> Anisse
> ---
>  drivers/platform/x86/asus-laptop.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index 5c23b20..decb958 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -312,6 +312,8 @@ static const struct key_entry asus_keymap[] = {
>  	{KE_KEY, 0xc4, { KEY_KBDILLUMUP } },
>  	{KE_KEY, 0xc5, { KEY_KBDILLUMDOWN } },
>  	{KE_KEY, 0xb5, { KEY_CALC } },
> +	/* Pegatron Lucid tablet specific */
> +	{KE_KEY, 0xEA, { KEY_DIRECTION } }, /* Orientation changed */
>  	{KE_END, 0},
>  };
>  

Re: [RFC] Reporting "orientation changed" event

[Anisse Astier]

On Thu, 07 Apr 2011 18:01:39 +0100, Jonathan Cameron <jic23@cam.ac.uk> wrote :

> On 04/07/11 17:18, Anisse Astier wrote:
> > 
> > Pegatron Lucid tablet sends an ACPI hotkey event(0xEA) when the
> > accelerometer detects coarse orientation change. My initial thought was
> > to just translate this event into KEY_DIRECTION, which seems to be the
> > norm from what we can see in hp-wmi driver. (See patch below for an
> > implementation.)
> > 
> > But this isn't just a key per se like on the HP touchsmart tablet, this
> > is an event that is triggered when the device is rotated.
> > 
> > This could be defined as a new Misc (EV_MSC) event:
> > MSC_ORIENTATION_CHANGED ?
> > 
> > Or we could use the upcoming IIO subsystem which is supposed to be for
> > sensors, but then we'd have a mismatch between the device based on ACPI
> > with firmware in the middle (driver submitted by Andy Ross) and the
> > purpose of IIO ("SPI or I2C device").
> Don't take that too literally. It's examples of types of bus IIO sensors use.
> Having said this, if your main use is user input then it should probably be
> handled through input rather than IIO anyway.
> 

Indeed, main use is user input. There's also the BIOS translation layer
to ACPI, so I guess it's a bit distant from IIO's purpose.


Anisse

Re: [RFC] Reporting "orientation changed" event

[Matthew Garrett]

(Sorry for taking so long to respond)

On Thu, Apr 07, 2011 at 06:18:00PM +0200, Anisse Astier wrote:

> Pegatron Lucid tablet sends an ACPI hotkey event(0xEA) when the
> accelerometer detects coarse orientation change. My initial thought was
> to just translate this event into KEY_DIRECTION, which seems to be the
> norm from what we can see in hp-wmi driver. (See patch below for an
> implementation.)
> 
> But this isn't just a key per se like on the HP touchsmart tablet, this
> is an event that is triggered when the device is rotated.
> 
> This could be defined as a new Misc (EV_MSC) event:
> MSC_ORIENTATION_CHANGED ?

I'd say that it ought to be hooked into the accelerometer driver and 
that should indicate that the orientation has changed.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC] Reporting "orientation changed" event

[Andy Ross]

On 05/09/2011 07:38 AM, Matthew Garrett wrote:
> On Thu, Apr 07, 2011 at 06:18:00PM +0200, Anisse Astier wrote:
> > Pegatron Lucid tablet sends an ACPI hotkey event(0xEA) when the
> > accelerometer detects coarse orientation change
>
> I'd say that it ought to be hooked into the accelerometer driver and
> that should indicate that the orientation has changed.

How would that work?  If it's an extra sysfs node, it would require an
extra file descriptor in the reader.  And if it's an event inside the
input stream, that would require the reader implement handling to throttle
the polling rate.  The point here was to avoid having to poll the
(noisy) accelerometer stream just for gross orientation changes.

Just to add a little iconoclasm, is this really something that needs
to be designed?  Right now, it's limited to one device.  And the only
reason we care about this event is that the accelerometer device* is
noisy and very slow.  If it worked like "regular" accelerometers we'd
just use the standard code path for polling an accelerometer device
and do it all in userspace.

The specific use case here is the MeGo integration for the
ExoPC/WeTab.  You can see the code here:

http://build.meego.com/package/files?package=sensorfw-pegatron&project=Trunk

It's just a one-liner acpid hook which uses dbus-send to tickle a
method in the sensor daemon plugin.  It's clean, works well, and owing
to acpid's configuration scheme is trivial to package.

Does it really need to be "fixed"?  If this were a whole class of
hardware, then I guess I'd agree.  But right now it's one quirky
device.

Andy

* The upstreaming of which is still waiting on me to get back to the
  panics I saw and triage them.  I haven't forgotten Corentin, I just
  haven't been able to find the time.

Re: [RFC] Reporting "orientation changed" event

[Matthew Garrett]

On Mon, May 09, 2011 at 08:37:55AM -0700, Andy Ross wrote:

> How would that work?  If it's an extra sysfs node, it would require an
> extra file descriptor in the reader.  And if it's an event inside the
> input stream, that would require the reader implement handling to throttle
> the polling rate.  The point here was to avoid having to poll the
> (noisy) accelerometer stream just for gross orientation changes.

Yes, so the accelerometer driver should (in-kernel) know that a coarse 
orientation event has occured and then send an appropriate uevent to 
userspace indiciating that it has new data.

> http://build.meego.com/package/files?package=sensorfw-pegatron&project=Trunk

Yeah, that's absolutely dreadful. Don't do that.

> Does it really need to be "fixed"?  If this were a whole class of
> hardware, then I guess I'd agree.  But right now it's one quirky
> device.

I'm going to NAK anything that reports "Coarse orientation change" to 
userspace without providing any context.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC] Reporting "orientation changed" event

[Andy Ross]

On 05/09/2011 08:47 AM, Matthew Garrett wrote:
> Yes, so the accelerometer driver should (in-kernel) know that a coarse 
> orientation event has occured and then send an appropriate uevent to 
> userspace indiciating that it has new data.

OK, so substituting udev for acpid, but otherwise leaving the
input-polldev device alone.  That certainly sounds nice to me, though
I'm not sure where the "dreadful / don't do that" advice is directed
as the handling in userspace will be virtual identical (moving the
dbus-send from the acpid event file into a udev rule).

> I'm going to NAK anything that reports "Coarse orientation change"
> to userspace without providing any context.

Just to be clear: there's no kernel code to NAK here.  The acpid hook
is raw, and in userspace.  It's not clean, but it's also a single-device
fixup: seems to me to be pretty much exactly what apcid is for, no?

Andy

Re: [RFC] Reporting "orientation changed" event

[Anisse Astier]

On Mon, 09 May 2011 08:59:01 -0700, Andy Ross <andy.ross@windriver.com> wrote :

> On 05/09/2011 08:47 AM, Matthew Garrett wrote:
> > Yes, so the accelerometer driver should (in-kernel) know that a coarse 
> > orientation event has occured and then send an appropriate uevent to 
> > userspace indiciating that it has new data.
> 
> OK, so substituting udev for acpid, but otherwise leaving the
> input-polldev device alone.  That certainly sounds nice to me, though
> I'm not sure where the "dreadful / don't do that" advice is directed
> as the handling in userspace will be virtual identical (moving the
> dbus-send from the acpid event file into a udev rule).
> 
> > I'm going to NAK anything that reports "Coarse orientation change"
> > to userspace without providing any context.
> 
> Just to be clear: there's no kernel code to NAK here.  The acpid hook
> is raw, and in userspace.  It's not clean, but it's also a single-device
> fixup: seems to me to be pretty much exactly what apcid is for, no?

I think this was aimed at my proposal to send an input event
KEY_DIRECTION from within asus_laptop driver, which wouldn't provide
context with regard to which accelerometer device triggered it (although
in our setup there's only one device, and the ACPI event doesn't provide
any context either, but at least we know it's an ACPI accelerometer).

An udev event(e.g "change") works for me, it would provide context, and
wouldn't disturb the handling of the polled-input device which we'd like
to open as little as possible.

Anyway, whatever solution we choose, the priority is to get this driver
included, even if we don't support the "Coarse Orientation Changed" event
in kernel yet.

Anisse

Re: [RFC] Reporting "orientation changed" event

[Matthew Garrett]

On Mon, May 09, 2011 at 08:59:01AM -0700, Andy Ross wrote:
> On 05/09/2011 08:47 AM, Matthew Garrett wrote:
> > Yes, so the accelerometer driver should (in-kernel) know that a coarse 
> > orientation event has occured and then send an appropriate uevent to 
> > userspace indiciating that it has new data.
> 
> OK, so substituting udev for acpid, but otherwise leaving the
> input-polldev device alone.  That certainly sounds nice to me, though
> I'm not sure where the "dreadful / don't do that" advice is directed
> as the handling in userspace will be virtual identical (moving the
> dbus-send from the acpid event file into a udev rule).

It shouldn't even be a dbus send - something in userspace should just be 
listening for event notifications on the accelerometer. Use udev 
directly.

> > I'm going to NAK anything that reports "Coarse orientation change"
> > to userspace without providing any context.
> 
> Just to be clear: there's no kernel code to NAK here.  The acpid hook
> is raw, and in userspace.  It's not clean, but it's also a single-device
> fixup: seems to me to be pretty much exactly what apcid is for, no?

acpid is for dealing with cases where the kernel doesn't provide 
functionality that the kernel should provide. Arbitrary APCI events 
shouldn't be being delivered to userspace - they should be handled 
in-kernel and delivered through a meaningful mechanism in order to avoid 
cases where userspace needs to know about a platform implementation. In 
this case the right way for the event to hit userspace is as a generic 
message from the accelerometer, not as a device-specific ACPI event 
through a deprecated interface.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC] Reporting "orientation changed" event

[Anisse Astier]

On Mon, 9 May 2011 17:40:06 +0100, Matthew Garrett <mjg59@srcf.ucam.org> wrote :

> On Mon, May 09, 2011 at 08:59:01AM -0700, Andy Ross wrote:
> > On 05/09/2011 08:47 AM, Matthew Garrett wrote:
> > > Yes, so the accelerometer driver should (in-kernel) know that a coarse 
> > > orientation event has occured and then send an appropriate uevent to 
> > > userspace indiciating that it has new data.
> > 
> > OK, so substituting udev for acpid, but otherwise leaving the
> > input-polldev device alone.  That certainly sounds nice to me, though
> > I'm not sure where the "dreadful / don't do that" advice is directed
> > as the handling in userspace will be virtual identical (moving the
> > dbus-send from the acpid event file into a udev rule).
> 
> It shouldn't even be a dbus send - something in userspace should just be 
> listening for event notifications on the accelerometer. Use udev 
> directly.
> 
> > > I'm going to NAK anything that reports "Coarse orientation change"
> > > to userspace without providing any context.
> > 
> > Just to be clear: there's no kernel code to NAK here.  The acpid hook
> > is raw, and in userspace.  It's not clean, but it's also a single-device
> > fixup: seems to me to be pretty much exactly what apcid is for, no?
> 
> acpid is for dealing with cases where the kernel doesn't provide 
> functionality that the kernel should provide. Arbitrary APCI events 
> shouldn't be being delivered to userspace - they should be handled 
> in-kernel and delivered through a meaningful mechanism in order to avoid 
> cases where userspace needs to know about a platform implementation. In 
> this case the right way for the event to hit userspace is as a generic 
> message from the accelerometer, not as a device-specific ACPI event 
> through a deprecated interface.
> 

Implementation RFC :

From: Anisse Astier <anisse@astier.eu>
Date: Mon, 9 May 2011 19:51:40 +0200
Subject: [PATCH RFC] asus-laptop: pega_accel - Report accelerometer orientation change through udev

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 drivers/platform/x86/asus-laptop.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 2105b42..e23d555 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -1337,6 +1337,13 @@ static void asus_acpi_notify(struct acpi_device *device, u32 event)
 		}
 		return ;
 	}
+
+	/* Accelerometer "coarse orientation change" event */
+	if (asus->pega_accel && event == 0xEA) {
+		kobject_uevent(&asus->pega_accel->dev.kobj, KOBJ_CHANGE);
+		return ;
+	}
+
 	asus_input_notify(asus, event);
 }
 
-- 
1.7.4.3

Re: [RFC] Reporting "orientation changed" event

[Matthew Garrett]

On Mon, May 09, 2011 at 07:58:02PM +0200, Anisse Astier wrote:

> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index 2105b42..e23d555 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -1337,6 +1337,13 @@ static void asus_acpi_notify(struct acpi_device *device, u32 event)
>  		}
>  		return ;
>  	}
> +
> +	/* Accelerometer "coarse orientation change" event */
> +	if (asus->pega_accel && event == 0xEA) {
> +		kobject_uevent(&asus->pega_accel->dev.kobj, KOBJ_CHANGE);
> +		return ;
> +	}
> +
>  	asus_input_notify(asus, event);
>  }

Precisely that.

-- 
Matthew Garrett | mjg59@srcf.ucam.org