[systemd-devel] Bad accelerometer values cause incorrect screen rotation

[Hans de Goede]

Hi,

On 05-09-19 12:49, Bastien Nocera wrote:
> On Thu, 2019-09-05 at 18:38 +0800, Daniel Drake wrote:
>> On Thu, Sep 5, 2019 at 6:07 PM Bastien Nocera <hadess at hadess.net>
>> wrote:
>>> I've read through this, and I'm happy blacklisting the hp_accel
>>> driver
>>> in code. For the other devices, I'd rather leave it as-is.
>>
>> That would indeed avoid most problem cases that I've seen, and the
>> current case, probably enough to stop me grumbling for another year
>> or
>> so until this happens again in some other context :)
>> So I support that idea. Do you have any preference on where we
>> blacklist it?
>>
>> In the hwdb it's quite easy to match DMI vendor HP & driver
>> lis3lv02d.
>> But we'd really want a new way of saying "ignore the accelerometer"
>> as
>> ACCEL_POSITION=base doesn't seem like the right way to express that.
>>
>> Or we could blacklist it in iio-sensor-proxy but since there's no
>> mention of hp_accel in the udev properties for the device (you just
>> get the driver as li3lv02d) then you'd need to grab the DMI vendor
>> name from /sys/class/dmi/id or something like that.
>> Maybe making this driver export enough data so we can blacklist it
>> would be the best way to go about it, along with a new udev property.
> 
> We should make this driver export enough data so we can differentiate
> it, then we can install a udev property private to iio-sensor-proxy
> about ignoring specific accelerometers such as this one. This way, the
> sensor hwdb only contains quirks, not policy decisions about whether
> the hp_accel driver is worthy.

I think a good question to ask here is, why do we want to blacklist
the lis3lv02d when used in HP laptops and I think the answer is
because it usually sits in the base of the device. So a simpler answer
here might be to just do this:

diff --git a/hwdb/60-sensor.hwdb b/hwdb/60-sensor.hwdb
index 3976d9a76a..de5e1b95a2 100644
--- a/hwdb/60-sensor.hwdb
+++ b/hwdb/60-sensor.hwdb
@@ -288,13 +288,6 @@ sensor:modalias:acpi:KIOX000A*:dmi:bvnAmericanMegatrendsInc.:bvr5.11:bd05/25/201
  # is then applied below.
  sensor:modalias:platform:lis3lv02d:dmi:*svn*Hewlett-Packard*:*
   ACCEL_MOUNT_MATRIX=1, 0, 0; 0, 0, -1; 0, 1, 0
-
-# HP laptops which have the lis3lv02d device in the base, tell iio-sensor-proxy
-# about this so that the sensor is not used for display orientation
-sensor:modalias:platform:lis3lv02d:dmi:*svn*Hewlett-Packard*:*pnHPProBook4535s*
- ACCEL_LOCATION=base
-
-sensor:modalias:platform:lis3lv02d:dmi:*:svnHewlett-Packard:pnHPENVY17NotebookPC:*
   ACCEL_LOCATION=base

  sensor:modalias:acpi:SMO8500*:dmi:*:svnHewlett-Packard:pnHPStream7Tablet:*


So in the default hwdb rule for HP laptops with the lis3lv02d sensor mark
the sensor as being in the base.

This will have the same result as marking it as hp_accel + blacklisting
hp_accel in iio-sensor-proxy. With the added advantage that we can
whitelist it if we encounter a HP tablet or 2-in-1 where we do actually
want to use it for auto-rotation.

And this would not require adding any extra code anywhere, so I believe
this would be a nice KISS solution.

Regards,

Hans