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

[Hans de Goede]

Hi,

On 05-09-19 13:28, Bastien Nocera wrote:
> On Thu, 2019-09-05 at 13:13 +0200, Hans de Goede wrote:
>> 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.
> 
> That works for me, although the mount matrix would now be wrong and
> should probably be removed (or moved).

True, I will remove it.

> I'd check the existing entries for tablets and 2-in-1 at the same time
> though, and those ones would use the mount matrix, right?

Yes those should use the mount matrix, but there are no existing
model specific entries for the lis3lv02d other then 2 which set

      ACCEL_LOCATION=base

Which are rendered obsolete by us now setting this for the generic
quirk.

Regards,

Hans