OK, pull request has been submitted. https://github.com/systemd/systemd/pull/26317 On Sun, 5 Feb 2023 at 18:06, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi, > > On 2/5/23 17:06, Darrell Kavanagh wrote: > > So does this mean that the least worst (only?) option is to get my > > hwdb mount matrix entry added to systemd? I can raise a bug as > > suggested in hwdb.d/60-sensor.hwdb if so. > > Yes you should add a hwdb entry for this, note just submitting > a pull-req with the fix is better then filing an issue for this. > > Regards, > > Hans > > > > > On Sun, 5 Feb 2023 at 14:22, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > >> > >> On Sun, 5 Feb 2023 09:50:51 +0100 > >> Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > >> > >>> Hi, > >>> > >>> On 2/4/23 23:15, Darrell Kavanagh wrote: > >>>> Yes, I understand that. > >>>> > >>>> What I mean is that the matrix read from the DSDT by Jonathan's > >>>> amended driver is > >>>> > >>>> 0 -1 0 > >>>> 1 0 0 > >>>> 0 0 1 > >>>> > >>>> and the (correct) matrix created with my new hwdb entry is > >>>> > >>>> 0 1 0 > >>>> -1 0 0 > >>>> 0 0 1 > >> > >> May be concidence, but I think that's the inverse of the one we are reading > >> from ROTM - so represents the transform in the other direction. > >> > >> The way ROTM is defined is that first row represents the direction of > >> the x axis in device coordinates - so it's the transform from sensor > >> to device space. > >> > >> I wonder if the hwdb matrix is defined from world space to sensor? Seems > >> unlikely. > >> > >> The IIO ABI docs describe mount matrix as being what you apply to data to > >> tranform into device space (oh for a diagram in the docs). Anyhow my reading > >> is that matches with ROTM definition but maybe I'm reading that wrong... > >> > >> For extra annoyance, the ROTM matrix on this device isn't a rotation matrix. > >> It's flipping the handedness of the sensor. Determinant isn't -1 which it > >> should be. I guess the sensor itself might have an axis backwards from > >> windows convention though *sigh* I think windows uses left handed convention > >> and looks like sensor is using right handed (which I think is what Android and > >> similar use). > >> > >>>> > >>>> which is the algebraic transposition (ie reflection in the diagonal) > >>>> of the DSDT one. > >>>> > >>>> In other words, though the DST matrix is wrong, it is wrong in a > >>>> specific way - the rows should be the columns, and vv. I was just > >>>> wondering if this was a DSDT bug that might have been seen elsewhere > >>>> before. > >>> > >>> No this does not ring a bell, but the x and y axis being swapped > >>> does seem related to the LCD panel being 90° rotated. > >>> > >>>> BTW, there is another matrix in the DSTD, but I can't find the > >>>> associated HID (10EC5280) anywhere (Linux sysfs or Windows Powershell > >>>> system data extract). It's not a correct matrix, though - could it be > >>>> just a bit of redundant code in the DST? > >>> > >>> Yes that is likely there often is a bunch of dead stuff DSDT leftover > >>> from other device models. > >>> > >>> Regards, > >>> > >>> Hans > >>> > >>> > >>> > >>> > >>> > >>>> > >>>> Darrell > >>>> > >>>> On Sat, 4 Feb 2023 at 21:31, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > >>>>> > >>>>> 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@xxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxx> 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 > >>>>>>>>> > >>>>>>>> > >>>>>> > >>>>> > >>>> > >>> > >> > > >
