[systemd-devel] hwdb: inconsistency in 60-sensor.hwdb accel_mount_matrix

[Hans de Goede]

Hi,

On 6/27/22 21:50, Gwendal Grignou wrote:
> In 60-sensor.hwdb, we can specify a matrix to rotate sensor output to
> match the expected 'natural' orientation [1].
> 
> When behind a screen, accelerometers are usually soldered upside down,
> so a rotation matrix is needed to rotate their raw output into the
> 'natural' orientation when read directly.
> 
> When flat on a table, standalone accelerometers/IMUs report the force
> exercised by the table to prevent the sensor from falling down
> further. In the 'natural' orientation, this is [0, 0, 1g]. See for
> instance the BMI160 specification[2], page 107).
> 
> However, Gnome expects the sensor to return the gravity force itself,
> as specified by windows and the HID specification:  When flat on a
> table, the expected value is [0, 0 , -1g].
> 
> Therefore, the matrix, in addition to rotating the sensors, should
> also invert the axes.

That depends on the handedness of the sensor itself though, if the
sensor already is using Windows style handedness then the matrix
would be a pure rotation matrix and different sensors may have
a different handedness at the raw hardware level.

Also currently userspace is not really using the Z-axis.
Most quirks are user-submitted based on fixing auto-rotation which
only uses the X and Y axis, so the Z-axis sign might very well be
wrong in some cases.

> When a matrix inverts the axes, its determinant is -1 [3].
> Out of the 160 matrices defined, 136 invert the axes, 29 does not and
> one as a determinant equals to 0.
> 
> That matrix, for Hometech device is obviously not correct: the sensor
> input vector X component is is used for both Y and Z axes of the
> output vector:
> 
> #########################################
> # Hometech
> ########################################
> 
> # Nobody bothered to use Linux on any device of this manufacturer
> # so current marks might be too general and need fixes.
> # These values are based on Wi101 model.
> sensor:modalias:acpi:BMA250E*:dmi:*:svnInsyde*:pni101c:*
>  ACCEL_MOUNT_MATRIX=0,1,0;-1,0,0;-1,0,0

Yes that is obviously wrong, as said above current userspace
doesn't really use the Z-axis much, which is why this is still
working fine.

> 
> Remains 29 matrices where axes are not inverted:
> 9 (':acpi:BOSC0200*', 1.0)
> 3 (':acpi:INVN6500*', 1.0)
> 2 (':acpi:KIOX010A*', 1.0)
> 2 (':acpi:KIOX020A*', 1.0)
> 1 (':acpi:KIOX0009*', 1.0)
> 1 (':acpi:SMO8500:*', 1.0)
> 1 (':acpi:MXC6655*', 1.0)
> 1 (':acpi:ACCE0001*', 1.0)
> 1 (':acpi:KIOX000A*', 1.0)
> 1 (':i2c:bmc150_accel', 1.0)
> 1 (':acpi:NCPE0388*', 1.0)
> 1 (':acpi:SMO8500*', 1.0)
> 
> Interestingly, some matrices for the same sensors are inverting axes:
>  KIOX000A* sensors axes are inverted 36 times.
> 
> The reason is the expectation that in convertible devices the
> accelerometers return the same data after rotation when the lid angle
> is 0:

I don't think so the KIOX000A sensor hw-id is not used in
devices with 2 sensors, those use KIOX001A and KIOX002A as
hw-ids for the 2 sensors IIRC.

> # A note about setting ACCEL_MOUNT_MATRIX for ACCEL_LOCATION=base sensors,
> # [...] This means that the
> # base-accelerometer's mount-matrix must be such, that after applying
> # the mount-matrices to both sensors, the base-accelerometer's readings must
> # be identical to the display-accelerometer's readings (when the lid is
> # closed).
> 
> It means that one of 2 accelerometers is expected to not return data
> in the 'normal' orientation.

*No*, both sensors are expected to return data, but when the lid is closed
(or the display fully folded against the back of the keyboard) then the
readings of the 2 sensors are expected to be the same, IOW substracting
the 2 vectors given by the 2 sensors is expected to result in a 0, 0, 0
vector, or something close to it.

> W3C device orientation specs [4] implies the sensors use the same
> device coordinate frame, and therefore should return the same data
> when the lid angle is 180 degree.

W3C is for browsers Linux is an OS *not* a browser! So W3C has *zero*
relevance for Linux/systemd.

We don't care about W3C. What we do instead is try to do is follow what
the factory installed OS on the same hw is doing, so that we can re-use
the existing firmware provided info for the factory installed OS.

Specifically the reason why the base mount-matrix is defined in hwdb
as it is is because that is how the base mount-matrix looks like in
ACPI on x86 Windows devices which use 2 accel sensors and which
have their mount-matrices available in ACPI.

Also note that all the mount-matrices available in ACPI (which Linux
supports and adds as a kernel provided mount-matrix) are using the
Windows/HID handedness!

I must say that I'm getting very tired of this whole discussion,
please just accept that the Linux/systemd mount-matrices do not
follow the W3C axis definitions and adjust the read data accordingly
inside chrome-the-browser and/or inside the android-sensor-hal.

What is next? Complaining that USB-devicefs:
include/uapi/linux/usbdevice_fs.h does not follow the W3C specification
for USB device access?  No of course chrome-the-browser does not
expect this, instead it translates between the W3C USB API and
the kernel's USB-devicefs API.

> The convention of aligning returned data to 180 degree lid angle can
> be fixed completely in user space as long as the algorithm for
> calculating lid angle and the udev tables are changed at the same
> time.

Changing these at the same time is *impossible*, for one we would
need to fix matrices inside ACPI tables, which we don't control at
all; and we would need to update the kernel + userspace consumers
in lock step which we don't do either. We do the opposite, we promise
to *never* break the userspace API and again your proposal is
breaking the userspace API.

I really don't understand why Chrome can do the much more complex
USB API translation, but for some reason you believe that the accel
axis definition must be fixed at the kernel level ?  Translating
from the OS-definition of the accel axis to the W3C one literary
is the browsers job. So please just do the translation there and
stop this fruitless discussion.

Regards,

Hans