linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Darrell Kavanagh <darrell.kavanagh@gmail.com>
Cc: Bastien Nocera <hadess@hadess.net>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Jonathan Cameron <jic23@kernel.org>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: Bug#1029850: linux: Driver not loaded for ST Microelectronics LSM6DS3TR-C accelerometer (acpi:SMO8B30:SMO8B30:)
Date: Sat, 4 Feb 2023 22:31:10 +0100	[thread overview]
Message-ID: <f256b437-7495-1115-7eea-619b6e241e02@redhat.com> (raw)
In-Reply-To: <CAMxBKG1EUgJqMtrxBVOWP9GnZkc518HU2xUnLW4UUWH+8_u=cQ@mail.gmail.com>

Hi,

On 2/4/23 18:09, Darrell Kavanagh wrote:
> I've just noticed that the working mount matrix that I added to my
> hwdb is the matrix retrieved from the ACPI ROTM call in the amended
> driver, transposed.

An other word for the mount matrix would be a rotation matrix,
since it defines how the physical sensor is mounted on the PCB
in a rotated fashion compared to its standard orientation.

The x, y, z axis relationship underling of course does
not change by the rotation, so yes all mount matrices
are a transposition of the standard:

1, 0, 0 : 0, 1, 0 : 0, 0, 1

matrix, that is expected. Where that to not be the case
then there would be a bug in the accelerometer driver itself
where the driver itself is swapping or inverting axis.

Regards,

Hans




> On Fri, 3 Feb 2023 at 18:23, Darrell Kavanagh
> <darrell.kavanagh@gmail.com> wrote:
>>
>> Finally got a 6.2.0-rc6 kernel built and installed, with the following
>> patch, and everything is working as expected.
>>
>> Moving on now to look at Bastien's suggestion.
>>
>> Thanks,
>> Darrell
>>
>> diff --git a/kernel/drm_panel_orientation_quirks.c
>> b/kernel/linux-6.2-rc6/drivers/gpu/drm/drm_panel_orientation_quirks.c
>> index 3659f04..590bb7b 100644
>> --- a/kernel/drm_panel_orientation_quirks.c
>> +++ b/kernel/linux-6.2-rc6/drivers/gpu/drm/drm_panel_orientation_quirks.c
>> @@ -304,6 +304,12 @@ static const struct dmi_system_id orientation_data[] = {
>>                   DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad
>> D330-10IGM"),
>>                 },
>>                 .driver_data = (void *)&lcd1200x1920_rightside_up,
>> +       }, {    /* Lenovo IdeaPad Duet 3 10IGL5 */
>> +               .matches = {
>> +                 DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> +                 DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "IdeaPad Duet 3 10IGL5"),
>> +               },
>> +               .driver_data = (void *)&lcd1200x1920_rightside_up,
>>         }, {    /* Lenovo Ideapad D330-10IGL (HD) */
>>                 .matches = {
>>                   DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>>
>> On Wed, 1 Feb 2023 at 17:55, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 2/1/23 18:50, Darrell Kavanagh wrote:
>>>> Thank you. I don't have anything that could be called a big machine.
>>>> The fastest processor I have access to is a Core m3-8100Y - that's in
>>>> a Chromebook with 4GB memory - it can run Linux in a chroot or
>>>> officially in Google's VM. I also have an ancient gen 2 core i5-2410M
>>>> machine which is slower than the m3 in theory, but that has 6GB of
>>>> memory.
>>>>
>>>> Is the kernel build more processor or memory bound?
>>>
>>> It is mostly processor bound, esp. wtih something like make -j4,
>>> make -j16 will start taking some RAM, but with make -j4 I expect you
>>> to be fully CPU bound.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>> On Wed, 1 Feb 2023 at 16:12, Bastien Nocera <hadess@hadess.net> wrote:
>>>>>
>>>>> On Wed, 2023-02-01 at 12:00 +0100, Hans de Goede wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 2/1/23 11:28, Jonathan Cameron wrote:
>>>>>>> On Wed, 1 Feb 2023 01:40:49 +0000
>>>>>>> Darrell Kavanagh <darrell.kavanagh@gmail.com> wrote:
>>>>>>>
>>>>>>>> Hello, all.
>>>>>>>>
>>>>>>>> I've finally reached a conclusion on this, after testing all the
>>>>>>>> combinations of the patches (with and without reading the acpi
>>>>>>>> mounting matrix), window managers (wayland, xorg) and the
>>>>>>>> presence or
>>>>>>>> not of my custom kernel parms.
>>>>>>>>
>>>>>>>> What works well is the full set of patches with the custom kernel
>>>>>>>> parms and a new hwdb entry for the sensor:
>>>>>>>>
>>>>>>>> sensor:modalias:acpi:SMO8B30*:dmi:*:svnLENOVO*:pn82AT:*
>>>>>>>>  ACCEL_MOUNT_MATRIX=0, 1, 0; -1, 0, 0; 0, 0, 1
>>>>>>>>
>>>>>>>> The autorotate then works correctly in wayland and xorg, but for
>>>>>>>> xorg,
>>>>>>>> the settings say the screen is "portrait left" when in actual
>>>>>>>> fact it
>>>>>>>> is in standard laptop landscape orientation. Wayland does not
>>>>>>>> have
>>>>>>>> this problem (I guess because wayland's view of the screen is
>>>>>>>> straight
>>>>>>>> from the kernel).
>>>>>>>>
>>>>>>>> Without the hwdb entry, the orientation is 90 degrees out without
>>>>>>>> using the acpi matrix and 180 degrees out when using it. I could
>>>>>>>> have
>>>>>>>> gone either way here with appropriate hwdb entries, but my view
>>>>>>>> is
>>>>>>>> that we *should* be using the matrix.
>>>>>>>
>>>>>>> Added Hans de Goede as he has probably run into more of this mess
>>>>>>> than anyone else.  Hans, any thoughts on if we are doing something
>>>>>>> wrong on kernel side?  Or is the matrix just wrong *sigh*
>>>>>>
>>>>>> I see below that this laptop has a panel which is mounted 90 degrees
>>>>>> rotated, that likely explains why the ACPI matrix does not work.
>>>>>> So the best thing to do here is to just override it with a hwdb
>>>>>> entries.
>>>>>>
>>>>>> IIRC there are already 1 or 2 other hwdb entries which actually
>>>>>> override the ACPI provided matrix because of similar issues.
>>>>>>
>>>>>> Linux userspace expects the matrix in this case to be set so that
>>>>>> it causes e.g. gnome's auto-rotation to put the image upright
>>>>>> even with older gnome versions / mate / xfce which don't know about
>>>>>> the panel being mounted 90 degrees.
>>>>>>
>>>>>> So e.g. "monitor-sensor" will report left-side-up or right-side-up
>>>>>> while the device is actually in normal clamshell mode with the
>>>>>> display up-right.
>>>>>>
>>>>>> This reporting of left-side-up or right-side-up is actually "correct"
>>>>>> looking from the native LCD panel orientation and as mentioned is
>>>>>> done for backward compatibility. This is documented here:
>>>>>>
>>>>>> https://github.com/systemd/systemd/blob/main/hwdb.d/60-sensor.hwdb#L54
>>>>>>
>>>>>> The way we are handling this is likely incompatible with how Windows
>>>>>> handles this special case of 90° rotated screen + ROTM. Or the
>>>>>> matrix in the ACPI tables could be just wrong...
>>>>>>
>>>>>>> I think 'ROTM' is defined by MS.
>>>>>>> https://learn.microsoft.com/en-us/windows-hardware/drivers/sensors/sensors-acpi-entries
>>>>>>
>>>>>> Right and as such it would be good if we can still add support to
>>>>>> it to the sensor driver in question. Because the ROTM info usually
>>>>>> is correct and avoids the need for adding more and more hwdb entries.
>>>>>>
>>>>>> Note there already is existing support in some other sensor drivers.
>>>>>>
>>>>>> So we probably need to factor out some helper code for this and share
>>>>>> that between sensor drivers.
>>>>>>
>>>>>>
>>>>>>>> The only thing that concerns me is the need for custom kernel
>>>>>>>> parms.
>>>>>>>> It would be better if there was a way to avoid this, so that the
>>>>>>>> user
>>>>>>>> didn't have to mess around with their grub config. Though having
>>>>>>>> said
>>>>>>>> that, the sensors fix as we have it doesn't make things worse -
>>>>>>>> under
>>>>>>>> currently released kernels the screen always starts up sideways
>>>>>>>> unless
>>>>>>>> custom parms are added in grub.
>>>>>>
>>>>>> We actually have a quirk mechanism in the kernel for specifying
>>>>>> the need for: video=DSI-1:panel_orientation=right_side_up  and this
>>>>>> will also automatically fix the fbcon orientation, see:
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_panel_orientation_quirks.c
>>>>>>
>>>>>> If you submit a patch for this upstream please Cc me.
>>>>>
>>>>> And if after that change, and copy/pasting the orientation from the
>>>>> DSDT into hwdb the sensor and screen move in the expected ways, then
>>>>> maybe stealing the BMC150 driver's
>>>>> bmc150_apply_bosc0200_acpi_orientation() might be a good idea.
>>>>>
>>>>> Once exported through "mount_matrix", iio-sensor-proxy should see it
>>>>> and read it without the need for a hwdb entry.
>>>>>
>>>>> Cheers
>>>>
>>>
> 


  reply	other threads:[~2023-02-04 21:32 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <167493679618.4533.12181720504943588640.reportbug@debian-duet>
     [not found] ` <Y9WGmBc9HG4Tx9gf@eldamar.lan>
     [not found]   ` <CAMxBKG1670TFuV3nHP7Yk8s6H+oBF7iiyiB-b=PvKv9hcH22xQ@mail.gmail.com>
2023-01-29 18:24     ` Bug#1029850: linux: Driver not loaded for ST Microelectronics LSM6DS3TR-C accelerometer (acpi:SMO8B30:SMO8B30:) Jonathan Cameron
     [not found]       ` <CAMxBKG0tyLSpaDPGBXsJbqgHSG9rH6owtSJsLw_ekmTA3Kyvdw@mail.gmail.com>
2023-01-30  3:37         ` Fwd: " Darrell Kavanagh
2023-01-30 12:31           ` Jonathan Cameron
2023-01-30 13:08             ` Darrell Kavanagh
2023-01-30 17:35               ` Jonathan Cameron
2023-01-30 18:32                 ` Darrell Kavanagh
2023-01-30 19:41                   ` Jonathan Cameron
2023-01-30 20:02                     ` Darrell Kavanagh
2023-01-30 20:31                       ` Jonathan Cameron
2023-01-31 13:34                         ` Darrell Kavanagh
2023-02-01  1:40                           ` Darrell Kavanagh
2023-02-01  1:46                             ` Darrell Kavanagh
2023-02-01 10:29                               ` Jonathan Cameron
2023-02-01 10:28                             ` Jonathan Cameron
2023-02-01 11:00                               ` Hans de Goede
2023-02-01 15:03                                 ` Darrell Kavanagh
2023-02-01 15:46                                   ` Hans de Goede
2023-02-01 16:12                                 ` Bastien Nocera
2023-02-01 17:50                                   ` Darrell Kavanagh
2023-02-01 17:55                                     ` Hans de Goede
2023-02-03 18:23                                       ` Darrell Kavanagh
2023-02-04 17:09                                         ` Darrell Kavanagh
2023-02-04 21:31                                           ` Hans de Goede [this message]
2023-02-04 22:15                                             ` Darrell Kavanagh
2023-02-05  8:50                                               ` Hans de Goede
2023-02-05 14:36                                                 ` Jonathan Cameron
2023-02-05 16:06                                                   ` Darrell Kavanagh
2023-02-05 18:06                                                     ` Hans de Goede
2023-02-05 22:05                                                       ` Darrell Kavanagh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f256b437-7495-1115-7eea-619b6e241e02@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=darrell.kavanagh@gmail.com \
    --cc=hadess@hadess.net \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).