Bug 199715

Summary: hp_accel: probe of HPQ6007:00 failed with error -22 (HP Envy x360)
Product: Drivers Reporter: Lukas Kahnert (openproggerfreak)
Component: Platform_x86Assignee: drivers_platform_x86 (drivers_platform_x86)
Status: NEW ---    
Severity: normal CC: alexdeucher, bugzilla, dmaroulidis, hel, jwrdegoede, kernel, kernelbugzilla, luya, mail, marco.rodolfi, maxime, russianneuromancer, sbergmans87, stuart, superm1, sweetsound
Priority: P1    
Hardware: x86-64   
OS: Linux   
Kernel Version: 4.16 Subsystem:
Regression: No Bisected commit-id:
Attachments: dmesg 4.16.7 (Ignore ACPI Errors, its a different bug)
Disassembled ACPI table(DSDT)
EC Register Dump
[PATCH 1/2] AMD_SFH: Removed unused activecontrolstatus member from the amd_mp2_dev struct
[PATCH 2/2] AMD_SFH: Add sensor_mask module parameter

Description Lukas Kahnert 2018-05-14 12:35:03 UTC
Created attachment 275967 [details]
dmesg 4.16.7 (Ignore ACPI Errors, its a different bug)

The HP Envy x360(Ryzen) uses an integrated accelerometer which get detected but probe fails with:

    lis3lv02d: unknown sensor type 0x0
    hp_accel: probe of HPQ6007:00 failed with error -22

I tried to search in the ACPI table and find out that the invalid return-value occurs cause of a "timeout". It looks like the EC needs some activation code to work, otherwise all querys will be returned with a "timeout" return-value. I have currently no Windows installation to compare the EC Register Dumps to find out why the EC is not responding for the accelerator.
Or am I absolutely wrong and the error is somewhere else?
I'm ready to help where I can.
Comment 1 Lukas Kahnert 2018-05-14 12:39:12 UTC
Created attachment 275969 [details]
Disassembled ACPI table(DSDT)
Comment 2 Lukas Kahnert 2018-05-14 12:42:10 UTC
Created attachment 275971 [details]
EC Register Dump
Comment 3 Luya Tshimbalanga 2018-11-21 04:43:41 UTC
I confirm this bug as I have similar model as list on journalctl boot report:
Nov 20 12:21:52 kernel: DMI: HP HP ENVY x360 Convertible 15-cp0xxx/8497, BIOS F.21 10/19/2018

The report suggest that hp_accel needs an update to allow the sensors functioning properly. It appears affecting all HP laptop powered wit AMD processors as Intel based have their functions working:
https://h30434.www3.hp.com/t5/Notebook-Video-Display-and-Touch/HP-Zbook-15-external-monitor-connected-to-docking-station/td-p/4985667
Comment 4 Luya Tshimbalanga 2018-11-21 23:02:36 UTC
After some investigation with Hans, the cause of the issue is the missing driver for AMD Sensor Fusion Hub (on some AMD laptop)  which does not exist in Linux. 

If you type lspci -nn, you should see one of message:

03:00.7 Non-VGA unclassified device [0000]: Advanced Micro Devices, Inc. [AMD] Device [1022:15e4]
Comment 5 Luya Tshimbalanga 2018-11-29 05:31:21 UTC
For better clarification, it would be better to rename the report "AMD Non-VGA unclassified device lacks non-existent AMD Sensor Fusion Hub for Linux".
Comment 6 P.Erbin 2019-01-26 13:07:25 UTC
Identical for me on my hp x360 envy 13-ag00xxx
As Luya did, ask for AMD support on this thread :
https://community.amd.com/thread/235302.
Comment 7 Luya Tshimbalanga 2019-03-16 21:03:21 UTC
The Sensor Fusion Hub is correctly identified on  kernel 4.20.14 displayed a more detailed information:

03:00.7 Non-VGA unclassified device [0000]: Advanced Micro Devices, Inc. [AMD] Raven/Raven2/Renoir Sensor Fusion Hub [1022:15e4]

It looks like the info is from the linux-firmware. Still no driver.
Comment 9 Luya Tshimbalanga 2019-05-23 05:32:28 UTC
(In reply to Luya Tshimbalanga from comment #8)
> It looks like the driver has finally arrived:
> https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git/commit/?h=i2c/
> for-next&id=529766e0a0114438887382a68d97341fbf8349fb

Nevermind.
Comment 10 Luya Tshimbalanga 2019-07-16 15:21:04 UTC
According to AMD, the driver will be ready somewhere on August:
https://lists.freedesktop.org/archives/amd-gfx/2019-May/034431.html
Comment 11 JSaw 2019-09-17 17:01:26 UTC
Hi, it's September now, might there be any news on this? The screen rotation is not yet working for me. I am on Arch Linux and latest kernel. So I guess the driver isn't released yet?
Comment 12 Luya Tshimbalanga 2019-10-09 05:25:11 UTC
(In reply to JSaw from comment #11)
> Hi, it's September now, might there be any news on this? The screen rotation
> is not yet working for me. I am on Arch Linux and latest kernel. So I guess
> the driver isn't released yet?

Not yet at the time of writing. According to Alex from AMD, the FCH team clean up the initial implementation based on the community feedback. The patch should be available on the hid subsystem repository of the Linux kernel.


https://lists.freedesktop.org/archives/amd-gfx/2019-September/040553.html
Comment 13 Luya Tshimbalanga 2020-01-11 06:27:25 UTC
It looks like the patches for AMD Sensor Fusion HUB driver are finally available. 
https://patchwork.kernel.org/project/linux-iio/list/?submitter=175589
Comment 14 Jacob Michalskie 2020-01-12 14:30:47 UTC
Hm, this is interesting, I compiled the driver, and was going over the documentation. Intel ish comparison isn't wrong, however Intel implements a mechanism to register the sensors on the hub as devices with the kernel, for use with the user space applications. AMD seems to expect "hid user space" to implement the drivers based on hid input reports instead, which is an interesting approach. Maybe I'm missing something, but I don't really see a reason not to register devices based on those reports, instead of expecting user space to take care of this. I will keep poking at this though.
Comment 15 Luya Tshimbalanga 2020-01-31 20:56:47 UTC
New revised patches available:
https://patchwork.kernel.org/project/linux-iio/list/?submitter=175589
Comment 16 Luya Tshimbalanga 2020-02-05 05:03:10 UTC
I built a test kernel with enabled amd_sfh_hid as modules (amd_mp2_pcie and amd_sfhtp_hid) using the latest patches from comment #15 running on HP Envy x360 Ryzen 2500u

lspci -vv

03:00.7 Non-VGA unclassified device: Advanced Micro Devices, Inc. [AMD] Raven/Raven2/Renoir Sensor Fusion Hub
	Subsystem: Hewlett-Packard Company Device 8497
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin D routed to IRQ 33
	Region 2: Memory at fc900000 (32-bit, non-prefetchable) [size=1M]
	Region 5: Memory at fcd8c000 (32-bit, non-prefetchable) [size=8K]
	Capabilities: [48] Vendor Specific Information: Len=08 <?>
	Capabilities: [50] Power Management version 3
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [64] Express (v2) Endpoint, MSI 00
		DevCap:	MaxPayload 256 bytes, PhantFunc 0, Latency L0s <4us, L1 unlimited
			ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset- SlotPowerLimit 0.000W
		DevCtl:	CorrErr- NonFatalErr- FatalErr- UnsupReq-
			RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
			MaxPayload 256 bytes, MaxReadReq 512 bytes
		DevSta:	CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr- TransPend-
		LnkCap:	Port #0, Speed 8GT/s, Width x16, ASPM L0s L1, Exit Latency L0s <64ns, L1 <1us
			ClockPM- Surprise- LLActRep- BwNot- ASPMOptComp+
		LnkCtl:	ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk+
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 8GT/s (ok), Width x16 (ok)
			TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
		DevCap2: Completion Timeout: Not Supported, TimeoutDis-, LTR+, OBFF Not Supported
			 AtomicOpsCap: 32bit- 64bit- 128bitCAS-
		DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
			 AtomicOpsCtl: ReqEn-
		LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1-
			 EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
	Capabilities: [a0] MSI: Enable- Count=1/2 Maskable- 64bit+
		Address: 0000000000000000  Data: 0000
	Capabilities: [c0] MSI-X: Enable- Count=2 Masked-
		Vector table: BAR=5 offset=00000000
		PBA: BAR=5 offset=00001000
	Capabilities: [100 v1] Vendor Specific Information: ID=0001 Rev=1 Len=010 <?>
	Kernel driver in use: pcie_mp2_amd
	Kernel modules: amd_mp2_pcie


lsmod | grep amd
edac_mce_amd           32768  0
kvm_amd               110592  0
kvm                   802816  1 kvm_amd
ccp                   106496  1 kvm_amd
amdgpu               5308416  6
amd_iommu_v2           20480  1 amdgpu
gpu_sched              40960  1 amdgpu
i2c_algo_bit           16384  1 amdgpu
ttm                   122880  1 amdgpu
drm_kms_helper        233472  1 amdgpu
drm                   585728  9 gpu_sched,drm_kms_helper,amdgpu,ttm
amd_sfhtp_hid          24576  0
pinctrl_amd            32768  1
amd_mp2_pcie           20480  1 amd_sfhtp_hid


modinfo amd_mp2_pcie
filename:       /lib/modules/5.6.0-0.rc0.git1.9.amdsfh.fc31.x86_64/kernel/drivers/hid/amd-sfh-hid/amd-mp2-pcie.ko.xz
author:         Nehal Bakulchandra Shah <Nehal-bakulchandra.Shah@amd.com>
author:         Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
license:        Dual BSD/GPL
version:        1.0
description:    AMD(R) PCIe MP2 Communication Driver
srcversion:     320368C1AFB52D740BAD58E
alias:          pci:v00001022d000015E4sv*sd*bc*sc*i*
depends:        
retpoline:      Y
intree:         Y
name:           amd_mp2_pcie
vermagic:       5.6.0-0.rc0.git1.9.amdsfh.fc31.x86_64 SMP mod_unload 
sig_id:         PKCS#7
signer:         Fedora kernel signing key
sig_key:        0E:F7:1B:36:D8:00:09:AE:EB:27:DB:41:61:61:A1:4C:A0:2B:B8:43
sig_hashalgo:   sha256
signature:      4F:8B:A9:7C:00:F6:6B:83:7D:5E:CC:5A:87:3A:95:85:5B:D7:08:C0:
		57:F5:AC:61:56:FF:E7:55:7A:67:68:AE:29:DA:57:1A:98:7C:6B:B6:
		2B:C7:E1:DC:FF:DB:CA:69:EA:CB:FC:C8:53:43:55:B4:60:88:DC:B3:
		FF:69:CC:19:C0:B9:7E:EA:D0:89:29:F6:AC:61:C1:52:F8:5E:CF:E3:
		3D:A8:1B:21:A9:A4:7D:F4:81:CF:81:A2:A4:8B:5B:75:59:7F:34:3A:
		CA:F7:06:7E:EC:5A:33:B3:EC:D8:3A:D3:C1:D2:49:D2:76:31:D0:A0:
		A2:9E:A5:ED:41:FB:22:EC:53:CD:CC:A5:8C:2A:2C:F2:E0:90:87:9F:
		B2:DD:2E:E5:CB:40:10:52:D9:84:DE:8C:CF:21:D7:6C:6D:45:7F:85:
		B8:94:3E:46:9B:46:37:BA:F5:93:A8:75:BD:E2:A2:DE:A9:66:A6:11:
		95:99:6C:D2:9D:07:80:B6:16:31:8B:64:24:01:38:E3:F8:F1:7A:04:
		6A:B2:55:77:CD:89:95:9D:5A:99:2C:08:71:B3:83:2C:B1:12:4A:7C:
		A4:2F:31:9D:75:FC:C7:3E:51:B0:60:E4:69:61:DE:4F:92:7F:50:4F:
		9D:69:28:53:CF:E3:AF:78:EF:FE:27:F8:0E:CD:97:B1:13:03:79:66:
		2F:22:18:AC:86:F5:9D:AA:3B:D2:05:06:C6:14:BE:AA:C7:0D:1B:8E:
		50:E5:E3:74:F8:27:8A:3A:81:45:C4:65:BE:72:BA:75:37:05:F6:64:
		BD:0A:1D:67:1B:7D:7D:D2:DB:7B:D8:BA:08:AD:E4:3D:C0:2B:E7:C1:
		35:86:3C:B5:A7:95:32:C7:B3:11:8F:31:BE:D5:D6:ED:8E:66:73:C6:
		4B:76:26:A1:28:AF:C5:53:FA:52:BC:F4:B3:31:91:D1:0D:18:BF:44:
		32:FE:2E:E7:51:60:CF:7E:A5:F3:55:F6:0E:AF:A3:F1:77:C5:6F:9F:
		3C:F8:4C:E0:C9:68:AC:23:1B:1D:4E:9F:C6:EE:A8:AC:36:BE:7D:8E:
		FA:77:9B:01:5B:E8:4E:5E:74:B5:C9:53:56:B4:E4:5A:D9:85:F4:BE:
		6E:1A:03:D1:60:4F:14:71:F8:46:DC:02:DD:57:35:92:4B:C3:12:04:
		A3:AA:A8:98:F4:40:F4:9A:E7:EE:C7:05:7B:EC:9C:29:5C:97:AC:2A:
		46:D0:69:0A:D8:18:10:F7:51:E9:5B:49:13:69:5C:20:5C:4E:77:86:
		93:3B:DF:BE:B2:8A:AC:9A:C0:63:F1:5B:B0:F7:16:A9:9E:B7:1F:4E:
		59:93:04:3A:AE:B4:06:FA:0D:02:A0:D7

modinfo amd_sfhtp_hid
filename:       /lib/modules/5.6.0-0.rc0.git1.9.amdsfh.fc31.x86_64/kernel/drivers/hid/amd-sfh-hid/amd-sfhtp-hid.ko.xz
license:        Dual BSD/GPL
author:         Nehal Shah <nehal-bakulchandra.shah@amd.com>
description:    AMD(R) SFH Client Driver
alias:          acpi*:AMDI0080:*
depends:        amd-mp2-pcie
retpoline:      Y
intree:         Y
name:           amd_sfhtp_hid
vermagic:       5.6.0-0.rc0.git1.9.amdsfh.fc31.x86_64 SMP mod_unload 
sig_id:         PKCS#7
signer:         Fedora kernel signing key
sig_key:        0E:F7:1B:36:D8:00:09:AE:EB:27:DB:41:61:61:A1:4C:A0:2B:B8:43
sig_hashalgo:   sha256
signature:      57:57:03:D5:EB:7E:8F:9F:8B:5A:10:43:B1:93:B8:BF:74:7F:D5:05:
		31:24:32:FF:E5:AE:00:E1:9F:48:17:E5:80:CA:C0:FD:CF:21:C9:BC:
		A7:F7:E5:AA:F4:67:F3:AA:6C:31:3F:4B:08:82:1F:4A:C1:2F:3F:9D:
		FD:41:E7:5F:AD:5B:12:EC:9A:6A:F0:8F:42:3A:9A:67:01:B1:66:29:
		45:4D:46:C6:31:2B:EE:AE:4E:28:FA:30:8E:28:58:7C:CA:5E:F8:A6:
		A8:12:F3:0F:87:B1:2F:B2:F6:48:E3:AE:CE:B0:AD:06:15:C5:3A:F9:
		54:C3:2C:39:49:00:A0:C8:2D:64:B4:37:58:34:06:44:AB:90:11:BB:
		D2:EC:C3:32:6A:29:58:D7:38:10:D6:D1:12:61:8A:0F:B1:BE:FA:BD:
		DF:4F:44:4E:F2:23:2C:73:7D:4E:EE:66:6D:F4:E6:6B:50:B3:E2:01:
		0D:6B:6F:46:D6:13:73:E6:ED:A9:6A:48:05:25:5E:08:E1:1E:0F:CA:
		E2:DF:71:DC:04:B4:ED:8E:ED:DC:53:1E:BB:7B:91:AB:E1:50:B4:C7:
		5F:B9:C6:D7:00:58:2C:46:D5:C8:AC:85:34:7C:9F:A2:E2:D9:84:37:
		BC:C8:32:EE:70:4E:71:DD:59:0E:51:D2:EC:E6:19:61:50:03:8C:DB:
		59:BE:8E:A5:8E:CB:3B:3C:DA:F6:9F:00:1E:8C:91:F8:EA:0D:C9:46:
		75:B1:01:D0:E7:93:2E:D6:15:D5:C3:CB:71:AF:FF:B7:43:FB:BB:DC:
		B0:3F:EB:9D:60:C0:7F:52:E0:7A:65:9B:25:94:12:04:3F:83:B9:4B:
		45:00:23:9E:46:AB:04:7F:A9:D5:EF:08:3C:A0:62:1C:78:22:F5:50:
		28:95:49:90:03:10:5A:56:DA:A5:DD:5F:18:77:85:33:E9:F7:B9:DC:
		54:C2:0D:AB:D8:B8:33:94:53:00:6C:6A:8A:85:23:15:56:63:82:48:
		90:47:D3:FC:4B:6C:9C:B1:24:87:50:15:48:2D:D0:B6:99:43:A8:E7:
		0E:3A:51:E7:1E:A1:D2:97:E5:BD:D3:97:8F:82:23:59:8D:3F:12:B9:
		D3:28:FD:6E:BB:1E:0C:ED:43:98:68:E9:F9:79:B7:51:48:3A:78:8A:
		12:01:57:89:BF:2C:BF:E5:D3:D0:F8:39:02:E1:19:85:13:D8:F0:9B:
		E7:EF:48:43:3C:84:7F:FC:CD:B7:7A:BD:76:A6:7B:9E:CC:75:A4:5D:
		BE:3B:85:F6:A2:79:2C:1C:C3:25:C5:EB:5B:DD:FF:FE:1B:D3:09:19:
		1A:FC:BE:12:61:99:53:17:4B:4C:03:84


So far, the sensors are not working yet and I don't know how to properly enable them.
Comment 17 Luya Tshimbalanga 2020-02-05 05:05:15 UTC
Built kernel based on 5.6.0 rc0 git1.9
https://copr.fedorainfracloud.org/coprs/luya/kernel-amdsfh/
Comment 18 Luya Tshimbalanga 2020-03-18 23:46:48 UTC
Updated patches available:

https://patchwork.kernel.org/project/linux-iio/list/?submitter=175589

One problem is the driver needed to read the HID part did not build.

Could someone update the title by mentioning "Enable AMD Sensor Fusion HUB support" instead. Thanks
Comment 19 Richard Neumann 2020-03-25 13:39:30 UTC
With the 4th revision of AMD's patch series and kernel 5.5.11 the driver builds successfully. I put a patched version into the AUR [1].
However the driver seems to work as-is only on certain HP convertibles, reportedly the HP Envy 13-ar000nn [2] and Envy X360 15-ds0013nr [3] series.
For my convertible, an HP ENVY x360 13-ag0005ng, I needed to statically enable all available HID devices [4],[5] to make the driver work.


[1] https://aur.archlinux.org/packages/linux-sfh/
[2] https://bbs.archlinux.org/viewtopic.php?pid=1893646#p1893646
[3] https://bbs.archlinux.org/viewtopic.php?pid=1888580#p1888580
[4] https://bbs.archlinux.org/viewtopic.php?pid=1893924#p1893924
[5] https://gist.github.com/conqp/e8a0793406fbe7c9714f01f3078ea33a
Comment 20 Richard Neumann 2020-03-25 13:48:51 UTC
By "to make the driver work" I meant "to make the driver detect the HID devices".
It is also currently necessary to boot the kernel with "amd_iommu=off".
Comment 21 Alex Deucher 2020-03-30 20:16:13 UTC
(In reply to Richard Neumann from comment #20)
> By "to make the driver work" I meant "to make the driver detect the HID
> devices".
> It is also currently necessary to boot the kernel with "amd_iommu=off".

Does reverting:
be62dbf554c5 ("iommu/amd: Convert AMD iommu driver to the dma-iommu api")
fix the IOMMU issues?
Comment 22 Richard Neumann 2020-03-31 07:23:02 UTC
Unfortunately I cannot test this, since reverting the commit results in a merge conflict in drivers/iommu/amd_iommu.c that I am unable to resolve due to my limited knowledge of this file.
Comment 23 Luya Tshimbalanga 2020-04-01 02:54:03 UTC
(In reply to Richard Neumann from comment #19)
> 
> 
> [1] https://aur.archlinux.org/packages/linux-sfh/

Would you mind fixing the patch for the latest git snapshot. It failed when attempt to build the kernel. Thanks.
Comment 24 Luya Tshimbalanga 2021-01-16 20:14:25 UTC
Testing the latest kernel 5.11.0-0.rc3.20210114git65f0d2414b70 snapshot with amd_sfh module enabled:

lspci -knn
03:00.7 Non-VGA unclassified device [0000]: Advanced Micro Devices, Inc. [AMD] Raven/Raven2/Renoir Sensor Fusion Hub [1022:15e4]
	Subsystem: Hewlett-Packard Company Device [103c:8497]
	Kernel driver in use: pcie_mp2_amd
	Kernel modules: amd_sfh

lsmod | grep amd
edac_mce_amd           90112  0
kvm_amd               380928  0
kvm                  2207744  1 kvm_amd
amdgpu              19922944  16
drm_ttm_helper         16384  1 amdgpu
ttm                   188416  2 amdgpu,drm_ttm_helper
iommu_v2               36864  1 amdgpu
gpu_sched              86016  1 amdgpu
i2c_algo_bit           28672  1 amdgpu
drm_kms_helper        638976  1 amdgpu
drm                  1359872  19 gpu_sched,drm_kms_helper,amdgpu,drm_ttm_helper,ttm
ccp                   274432  1 kvm_amd
amd_sfh                36864  0
pinctrl_amd            73728  1

modinfo amd_sfh
filename:       /lib/modules/5.11.0-0.rc3.20210114git65f0d2414b70.125.amdsfh.fc33.x86_64/kernel/drivers/hid/amd-sfh-hid/amd_sfh.ko.xz
author:         Sandeep Singh <Sandeep.singh@amd.com>
author:         Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
license:        Dual BSD/GPL
description:    AMD(R) PCIe MP2 Communication Driver
rhelversion:    8.99
alias:          pci:v00001022d000015E4sv*sd*bc*sc*i*
depends:        
retpoline:      Y
intree:         Y
name:           amd_sfh
vermagic:       5.11.0-0.rc3.20210114git65f0d2414b70.125.amdsfh.fc33.x86_64 SMP mod_unload 


monitor-sensors failed to detect sensor fusion
Comment 25 Richard Neumann 2021-01-17 09:02:44 UTC
@Luya This is due to the fact that you, like me, use a laptop (HP Enxy x360 13-ag00xxx) which ships with an AMD SFH that has no sensor mask written to its P2C registers. I discussed this on the mailing lists and proposed and provided a solution by introducing a kernel module parameter [1]. However this was not adopted in the upstream version for kernel 5.11. So I submitted my refactored version, which I am using for nearly a year now, which incorporates such a parameter [2]. I have not yet received any feedback on this one or the first submission, which I sent two weeks ago and which seemingly directly went into /dev/null. Anybody interested to review and possibly use my refactored version, can obtain it from my github repo [3].

[1] https://patchwork.kernel.org/project/linux-iio/patch/1582779537-25662-3-git-send-email-Sandeep.Singh@amd.com/#23253155
[2]https://patchwork.kernel.org/project/linux-iio/patch/efa4524cd07abe0a7773b24d33b64f09e0bf1f82.camel@richard-neumann.de/
[3] https://github.com/conqp/linux/tree/amd-sfh-hid/drivers/hid/amd-sfh-hid
Comment 26 Hans de Goede 2021-01-17 10:57:04 UTC
Hi Richard,

Thank you for your work on this.

I took a look at your patch-series from 2:
https://patchwork.kernel.org/project/linux-iio/patch/efa4524cd07abe0a7773b24d33b64f09e0bf1f82.camel@richard-neumann.de/

And there are several big issues with it:

1) It seems to have only been send to the linux-iio mailing-list, while it mostly touches files under drivers/hid. So the HID maintainers did not see it and the iio maintainers are likely ignore it because it is not an IIO patch

2) You seem to have build an top of an older version of the amd-sfh driver and instead of porting your changes to the new version, your series seems to completely replace the currently upstream version with your version. A patch series like this will never be accepted upstream. You need to submit a patch upstream with a small incremental change on top of the upstream version of the driver. E.g. submit a patch just adding the module parameter.

I've prepared a patch-set which adds a module parameter for the sensor-mask on top of the current upstream version of the amd-sfh code, without making any further changes. I'll attach that here, please test.

2 questions:

1. may I ask what sensor_mask_override value you are using? That will be useful for Luya to test this on his device, as well as for adding a DMI quirk to automatically do the right thing in the future.

2. Can you please run (as normal user):

grep . /sys/class/dmi/id/* 2> /dev/null

and copy and paste the output here?  We will need to come up with some way to make this "just work" for end-users without needing to set the module-parameter and it is likely that this will be based on a DMI quirk table, so it would be good to have the DMI strings for your model.

Luya, I will build a Fedora kernel for you with the patches added. I will post a link to that in the Fedora bug you filed for this; and Luya, can you also provide the output of 'grep . /sys/class/dmi/id/* 2> /dev/null' please?

Regards,

Hans
Comment 27 Hans de Goede 2021-01-17 10:57:37 UTC
Created attachment 294691 [details]
[PATCH 1/2] AMD_SFH: Removed unused activecontrolstatus member from the amd_mp2_dev struct
Comment 28 Hans de Goede 2021-01-17 10:58:10 UTC
Created attachment 294693 [details]
[PATCH 2/2] AMD_SFH: Add sensor_mask module parameter
Comment 29 Luya Tshimbalanga 2021-01-17 11:09:33 UTC
(In reply to Hans de Goede from comment #26)
... 
> Luya, I will build a Fedora kernel for you with the patches added. I will
> post a link to that in the Fedora bug you filed for this; and Luya, can you
> also provide the output of 'grep . /sys/class/dmi/id/* 2> /dev/null' please?
> 

Certainly. Here is the requested output:

grep . /sys/class/dmi/id/* 2> /dev/null

/sys/class/dmi/id/bios_date:07/03/2020
/sys/class/dmi/id/bios_release:15.47
/sys/class/dmi/id/bios_vendor:AMI
/sys/class/dmi/id/bios_version:F.47
/sys/class/dmi/id/board_asset_tag:Base Board Asset Tag
/sys/class/dmi/id/board_name:8497
/sys/class/dmi/id/board_vendor:HP
/sys/class/dmi/id/board_version:92.48
/sys/class/dmi/id/chassis_type:31
/sys/class/dmi/id/chassis_vendor:HP
/sys/class/dmi/id/chassis_version:Chassis Version
/sys/class/dmi/id/ec_firmware_release:92.48
/sys/class/dmi/id/modalias:dmi:bvnAMI:bvrF.47:bd07/03/2020:br15.47:efr92.48:svnHP:pnHPENVYx360Convertible15-cp0xxx:pvr:rvnHP:rn8497:rvr92.48:cvnHP:ct31:cvrChassisVersion:
/sys/class/dmi/id/product_family:103C_5335KV HP Envy
/sys/class/dmi/id/product_name:HP ENVY x360 Convertible 15-cp0xxx
/sys/class/dmi/id/product_sku:4BQ00UA#ABL
/sys/class/dmi/id/sys_vendor:HP
/sys/class/dmi/id/uevent:MODALIAS=dmi:bvnAMI:bvrF.47:bd07/03/2020:br15.47:efr92.48:svnHP:pnHPENVYx360Convertible15-cp0xxx:pvr:rvnHP:rn8497:rvr92.48:cvnHP:ct31:cvrChassisVersion:

Thank you for addressing the bug report, Hans!
Comment 30 Richard Neumann 2021-01-17 11:13:10 UTC
@Hans Thank you for your input.

First, here is the requested output:

$ cat /etc/modprobe.d/amd_sfh.conf 
options amd_sfh_hid sensor_mask=524295

$ grep . /sys/class/dmi/id/* 2> /dev/null
/sys/class/dmi/id/bios_date:11/15/2019
/sys/class/dmi/id/bios_release:15.46
/sys/class/dmi/id/bios_vendor:AMI
/sys/class/dmi/id/bios_version:F.46
/sys/class/dmi/id/board_asset_tag:Base Board Asset Tag
/sys/class/dmi/id/board_name:8496
/sys/class/dmi/id/board_vendor:HP
/sys/class/dmi/id/board_version:92.48
/sys/class/dmi/id/chassis_type:31
/sys/class/dmi/id/chassis_vendor:HP
/sys/class/dmi/id/chassis_version:Chassis Version
/sys/class/dmi/id/ec_firmware_release:92.48
/sys/class/dmi/id/modalias:dmi:bvnAMI:bvrF.46:bd11/15/2019:br15.46:efr92.48:svnHP:pnHPENVYx360Convertible13-ag0xxx:pvr:rvnHP:rn8496:rvr92.48:cvnHP:ct31:cvrChassisVersion:
/sys/class/dmi/id/product_family:103C_5335KV HP Envy
/sys/class/dmi/id/product_name:HP ENVY x360 Convertible 13-ag0xxx
/sys/class/dmi/id/product_sku:4JS64EA#ABD
/sys/class/dmi/id/sys_vendor:HP
/sys/class/dmi/id/uevent:MODALIAS=dmi:bvnAMI:bvrF.46:bd11/15/2019:br15.46:efr92.48:svnHP:pnHPENVYx360Convertible13-ag0xxx:pvr:rvnHP:rn8496:rvr92.48:cvnHP:ct31:cvrChassisVersion:

Regarding the driver. Yes, it is completely refactored version that is close to a re-write. I posted it to the IIO mailing list, since this is where the original driver was also posted and discussed.
I realize that I made a lot of changes, but I am confident, that my code base has a cleaner approach, since it utilizes the respective start, stop, open and close functions of the HID LL API and contains less (if even any) unused code and data structures. It is up to the maintainers whether they accept it, request changes or whatnot. I've done my part and am happy using my clean and stable implementation, that btw. is also used by other Arch Linux users with my DKMS package in the AUR. I unsuccessfully tried to make changes to the upstream driver during its development, which were ignored by both the devs and the maintainers. Refactoring the driver to clean it up, make it integrate seamlessly with the kernel APIs and providing the params to make it configurable on devices where the manufacturers screwed up was challenging, interesting and mostly a fun experience. Getting my work upstream, frankly, not so much.
Comment 31 Hans de Goede 2021-01-17 11:34:25 UTC
(In reply to Richard Neumann from comment #30)
> $ cat /etc/modprobe.d/amd_sfh.conf 
> options amd_sfh_hid sensor_mask=524295

Ok, so that translates to:

options amd_sfh_hid sensor_mask=0x80007

So you are enabling all 4 sensors:

#define ACEL_EN         BIT(0)
#define GYRO_EN         BIT(1)
#define MAGNO_EN        BIT(2)
#define ALS_EN          BIT(19)

I assume that you have verified that all 4 work?

Note that you can actually use hex-values (prefixed with 0x) when setting integer module parameters, that typically make things easier to parse in cases where the integer is a bitmask like this case.

> Refactoring the driver to clean it up, make it integrate seamlessly with the
> kernel APIs and providing the params to make it configurable on devices where
> the manufacturers screwed up was challenging, interesting and mostly a fun
> experience. Getting my work upstream, frankly, not so much.

May I ask you to at least consider re-posting the series as is with the email going to the right people:

[hans@x1 linux]$ scripts/get_maintainer.pl -f drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
Nehal Shah <nehal-bakulchandra.shah@amd.com> (maintainer:AMD SENSOR FUSION HUB DRIVER)
Sandeep Singh <sandeep.singh@amd.com> (maintainer:AMD SENSOR FUSION HUB DRIVER)
Jiri Kosina <jikos@kernel.org> (maintainer:HID CORE LAYER)
Benjamin Tissoires <benjamin.tissoires@redhat.com> (maintainer:HID CORE LAYER)
linux-input@vger.kernel.org (open list:AMD SENSOR FUSION HUB DRIVER)

With a cover letter why you believe that your version is better / cleaner ?

Note you will very probably still be asked to refactor your work so that it will consist of incremental patches which slowly moves the existing driver to your version. But this way at least the maintainers will be aware that there is room for improvement, because I'm pretty sure that they will not have seen the posting to the linux-iio list.
Comment 32 Hans de Goede 2021-01-17 11:36:50 UTC
p.s.

Richard any tips for Luya to test the GYRO and MAGNO sensors ?
Comment 33 Richard Neumann 2021-01-17 12:15:14 UTC
@Hans "work" is a relative term here. They are all recognized and started. Going into details would go beyond the scope of this thread, I think.
dmesg excerpt:

[   10.855774] amd-sfh-pci 0000:04:00.7: Sensor mask: 0x80007
[   10.856054] hid-generic 0018:03FE:0001.0002: hidraw1: I2C HID v0.01 Device [accelerometer] on AMD Sensor Fusion Hub (PCIe)
[   10.856325] hid-generic 0018:03FE:0001.0003: hidraw2: I2C HID v0.01 Device [gyroscope] on AMD Sensor Fusion Hub (PCIe)
[   10.856614] hid-generic 0018:03FE:0001.0004: hidraw3: I2C HID v0.01 Device [magnetometer] on AMD Sensor Fusion Hub (PCIe)
[   10.856893] hid-generic 0018:03FE:0001.0005: hidraw4: I2C HID v0.01 Device [ambient light sensor] on AMD Sensor Fusion Hub (PCIe)

I am only *using* the accelerometer, so a mask of "1" or "0x1" respectively would suffice.

Testing the other sensors is non-trivial. I branched my refactored version of the driver to extend it with an API to allow for reverse-engineering the PCI device to detect other sensors. But again, this maybe something to discuss in another place (or write a paper about :p).
Comment 34 Hans de Goede 2021-01-17 14:23:38 UTC
(In reply to Richard Neumann from comment #33)
> They are all recognized

That does not mean anything since the HID descriptors are provided by the amd_fsh driver itself, so of course they describe a valid gyroscope and magnetometer.

> Testing the other sensors is non-trivial.

Testing the ALS sensor actually should be easy, I assume that you have iio-sensor-proxy installed (that is what is typically used to do accelerometer based auto-display-rotation) ? Then you can just run monitor-sensor and it will print the ALS readings and then moving the laptop from a dark environment (say inside in a room with the blinds closed) to a light environment (say outside) should lead to significant different readings, with the outside readings being significantly higher.

As you indicate there are not really any userspace consumers of the gyroscope and 
magnetometer so testing those might be harder, although I think that monitor-sensor probably also prints magnetometer readings. And that should behave as a compass so if you stand upright, holding the laptop and then spin around the axis from your head to your feet, then those should change.
Comment 35 Richard Neumann 2021-01-17 14:46:14 UTC
Okay, given this context, I can say, that the ALS does *not* work, as it always returns 0.0 on my machine. That is with my refactored version and all driver versions up until v8 from the upstream version. Maybe Luya can confirm that. The magnetometer and gyroscope produce varying input data with my debug version, but as you correctly stated, there is nothing in the userspace that I am aware of, that can make use of this data.
Comment 36 Dimitris 2021-01-17 14:49:43 UTC
I've just tested Richard's Arch Linux DKMS package on my Envy x360 13-ag0xxx, with the module options in comment #31 and monitor-sensor displays only accelerometer data correctly. It states that the ALS is recognized but reports 0 lux ambience in a sun-lit room, and that value doesn't change when I change rooms. 

I'm available to test sensors and modules on my laptop.
Comment 37 Richard Neumann 2021-01-17 14:51:41 UTC
@Dimitris, @Luya can you test the ALS with the latest upstream version stages for kernel 5.11, so we can rule out that this is a bug in my refactored version? I currently have no time to build and test the upstream driver.
Comment 38 Dimitris 2021-01-17 14:57:35 UTC
@Richard I'm currently quite short on time. I can try it later this week. Maybe @Luya has better luck.
Comment 39 Richard Neumann 2021-01-17 16:01:01 UTC
I just tried to test the latest staged upstream version, but it does not even build:

make: Entering directory '/usr/lib/modules/5.10.7-zen1-1-zen/build'
  CC [M]  /var/lib/dkms/amd-sfh-hid/1.0.3/build/amd_sfh_hid.o
  CC [M]  /var/lib/dkms/amd-sfh-hid/1.0.3/build/amd_sfh_client.o
  CC [M]  /var/lib/dkms/amd-sfh-hid/1.0.3/build/amd_sfh_pcie.o
  CC [M]  /var/lib/dkms/amd-sfh-hid/1.0.3/build/hid_descriptor/amd_sfh_hid_desc.o
/var/lib/dkms/amd-sfh-hid/1.0.3/build/hid_descriptor/amd_sfh_hid_desc.c:12:10: fatal error: amd_sfh_pcie.h: No such file or directory
   12 | #include "amd_sfh_pcie.h"
      |          ^~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.build:279: /var/lib/dkms/amd-sfh-hid/1.0.3/build/hid_descriptor/amd_sfh_hid_desc.o] Error 1
make: *** [Makefile:1805: /var/lib/dkms/amd-sfh-hid/1.0.3/build] Error 2
make: Leaving directory '/usr/lib/modules/5.10.7-zen1-1-zen/build'

I never understood why they put the hid descriptors in a subdir anyway.
I already spent far too much time on this driver. I have my working refactored version. I'll leave the rest for somebody else.
Comment 40 Luya Tshimbalanga 2021-01-17 21:11:07 UTC
@Richard ALS failed on my side. I'll wait for more details from Hans.
Comment 41 Luya Tshimbalanga 2021-01-19 03:58:35 UTC
@Richand and @Hans,
Testing the ALS with the custom kernel build including patches provided by Hans always returns the value 0 despite switching different room light.
Comment 42 Dimitris 2021-01-19 13:38:48 UTC
After updating to kernel v5.10.8.arch1-1 (on Archlinux), and building the latest module from AUR, I got an error during module loading on startup that the bitmask was out of bounds. So I set the bitmask to 0x8007 instead of 0x80007 as in comment #31.
Comment 43 Richard Neumann 2021-01-19 18:56:36 UTC
@Luya: That would suggest that this is either a problem with the upstream code, or that our models simply do not have an ALS on the hub.
@Dimitris: Sorry about that. I refactored the package yesterday and introduced a bug. I migrated the sensor_mask to an usigned int and thought that a short would suffice, which it didn't. v. 2.8.7 should have this fixed.
Comment 44 Dimitris 2021-01-19 20:20:34 UTC
@Richard I just upgraded to v2.8.7, corrected the module's options and now the accelerometer functions correctly, like before. 

Today I noticed that monitor-sensors stated that no ambient light sensor was present. If I understand correctly a bitmask of 0x80007 activates all sensors on the hub. So that means that the light sensor would be active too. That would suggest the ag0xxx doesn't have an ALS. I couldn't find any information about whether that line has an ALS on HP support or PartSurfer.
Comment 45 Hans de Goede 2021-01-19 20:24:54 UTC
I believe that your models simply do not have an ALS sensor.

Luya did also test the magnetometer and gyro bits and reported his results in the Fedora bug about this:
https://bugzilla.redhat.com/show_bug.cgi?id=1651886

"""
Gyroscope
---------
ls /sys/bus/iio/devices/iio\:device0/
buffer/                        in_anglvel_z_raw
dev                            name
in_anglvel_hysteresis          power/
in_anglvel_offset              scan_elements/
in_anglvel_sampling_frequency  subsystem/
in_anglvel_scale               trigger/
in_anglvel_x_raw               uevent
in_anglvel_y_raw    

cat /sys/bus/iio/devices/iio\:device0/name 
gyro_3d

cat /sys/bus/iio/devices/trigger0/name 
gyro_3d-dev0

Gyroscope value returned 0.0.0 (x.y.z)


Magnetometer
------------
ls /sys/bus/iio/devices/iio\:device3/
buffer              in_magn_sampling_frequency  in_magn_z_raw  subsystem
dev                 in_magn_scale               name           trigger
in_magn_hysteresis  in_magn_x_raw               power          uevent
in_magn_offset      in_magn_y_raw               scan_elements

cat /sys/bus/iio/devices/iio\:device3/name 
magn_3d

cat /sys/bus/iio/devices/trigger2/name 
magn_3d-dev3

Test with cat /sys/bus/iio/devices/iio:device3/in_magn_x_raw successful.
Test with cat /sys/bus/iio/devices/iio:device3/in_magn_y_raw successful.
Test with cat /sys/bus/iio/devices/iio:device3/in_magn_z_raw successful.
"""

Where I assume successful for the magn device means that the values changed when doing the following: "For the magneto values (compass) you expect the x and y values to change if you sit the laptop on the desk and then rotate it while keeping it sitting on the desk, while the values should stat more or less the same when you do not move the laptop."

@Richard, can you confirm that your model behaves the same, so working accelerometer and magnetometer, with the gyro and als always reading all 0 ?

I would like to know, so that I can use the right sensor_mask for the DMI quirk for the upstream driver for this.
Comment 46 Dimitris 2021-01-19 20:36:50 UTC
@Hans I get 0 on the gyro, the magneto functions correctly (tested with Luya method in comment #45) and the accelerometer also. I have no ALS detected.

My laptop model is Envy x360 13-ag0011nv (P/N: 5VZ54EA).
Comment 47 Hans de Goede 2021-01-19 20:45:58 UTC
(In reply to Dimitris from comment #46)
> @Hans I get 0 on the gyro, the magneto functions correctly (tested with Luya
> method in comment #45) and the accelerometer also. I have no ALS detected.
> 
> My laptop model is Envy x360 13-ag0011nv (P/N: 5VZ54EA).

Thanks, so that would be a "HP ENVY x360 Convertible 13-ag0xxx" like the one from Richard. Can you confirm that:

cat /sys/class/dmi/id/product_name

Outputs "HP ENVY x360 Convertible 13-ag0xxx" ?



@Richard, no need for you to also test this then.
Comment 48 Richard Neumann 2021-01-19 22:25:59 UTC
$ cat /sys/class/dmi/id/product_name
HP ENVY x360 Convertible 13-ag0xxx

$ for file in /sys/bus/iio/devices/iio\:device0/in_anglvel_*; do echo "$(basename "${file}"): $(cat "${file}")"; done
in_anglvel_hysteresis: 1.270000
in_anglvel_offset: 0
in_anglvel_sampling_frequency: 12.500000
in_anglvel_scale: 0.000174532
in_anglvel_x_raw: 0
in_anglvel_y_raw: 0
in_anglvel_z_raw: 0

$ for file in /sys/bus/iio/devices/iio\:device1/in_magn_*; do echo "$(basename "${file}"): $(cat "${file}")"; done
in_magn_hysteresis: 1.270000
in_magn_offset: 0
in_magn_sampling_frequency: 12.500000
in_magn_scale: 0.000001000
in_magn_x_raw: 40
in_magn_y_raw: -14
in_magn_z_raw: -16

$ for file in /sys/bus/iio/devices/iio\:device2/in_accel_*; do echo "$(basename "${file}"): $(cat "${file}")"; done
in_accel_hysteresis: 1.270000
in_accel_offset: 0
in_accel_sampling_frequency: 12.500000
in_accel_scale: 0.098066500
in_accel_x_raw: 0
in_accel_y_raw: -9
in_accel_z_raw: -5

$ for file in /sys/bus/iio/devices/iio\:device3/in_i*; do echo "$(basename "${file}"): $(cat "${file}")"; done
cat: '/sys/bus/iio/devices/iio:device3/in_illuminance_hysteresis': Invalid argument
in_illuminance_hysteresis: 
in_illuminance_offset: 0
in_illuminance_raw: 0
in_illuminance_sampling_frequency: 12.500000
in_illuminance_scale: 0.100000000
in_intensity_both_raw: 0
cat: '/sys/bus/iio/devices/iio:device3/in_intensity_hysteresis': Invalid argument
in_intensity_hysteresis: 
in_intensity_offset: 0
in_intensity_sampling_frequency: 12.500000
in_intensity_scale: 0.100000000
Comment 49 Bastien Nocera 2021-01-20 09:24:52 UTC
You might want to use "iio_generic_buffer -A -N 0" for testing instead.

This makes sure that all the triggers are enabled, so that there's hopefully some data coming out of the devices. Substitute the "0" with the IIO device number, the command live in kernel-tools in Fedora.

If you get all zeroes out of the light sensor, it might also be the same problem I've recently reported to the IIO list, which would point to a problem in the common IIO code, rather than a problem with the light sensor in the hid-sensor-hub code.
Comment 50 Dimitris 2021-01-20 11:09:34 UTC
@Hans yes it's the same product_name.

$ cat /sys/class/dmi/id/product_name 
HP ENVY x360 Convertible 13-ag0xxx
Comment 51 Dimitris 2021-01-20 11:45:58 UTC
One thing I'm noticing, is that after waking up from sleep the accelerometer disappears.
Comment 52 Andy Shevchenko 2021-01-21 16:58:07 UTC
(In reply to Richard Neumann from comment #48)

> $ for file in /sys/bus/iio/devices/iio\:device0/in_anglvel_*; do echo
> "$(basename "${file}"): $(cat "${file}")"; done

Just a side note: The above can be much simplified, compare:

  grep -H . /sys/bus/iio/devices/iio\:device0/in_anglvel_*
Comment 53 Richard Neumann 2021-01-23 14:15:16 UTC
@Dimitris: The latest version (2.9.0-1) should have this resolved.
I tested it successfully on my laptop and all workds as expected.
I submitted my refactored version upstream - this time using the correct process:
https://patchwork.kernel.org/project/linux-input/list/?series=420493
Thanks you Any for the hints about the correct submission process.
Comment 54 Dimitris 2021-01-23 20:58:41 UTC
(In reply to Richard Neumann from comment #53)

So if I understand correctly, and excuse me but I'm not very familiar with the submission/acceptance of patches from upstream, if your patches get accepted and integrated into the kernel tree, we will be able to get the amd_sfh_hid module on downstream distributions of the kernel? What I mean is, as an end-user, will we be able to just modprobe that module to our running kernel as-is (without rebuilding the kernel) p.ex. in Archlinux, or will we need to recompile the kernel with the relevant driver selected in menuconfig to be built into the kernel?

I think the handling of the issue in the last question lies with the distributions.
Comment 55 Richard Neumann 2021-01-23 22:08:40 UTC
@Dimitris. The submitted v8 Version of AMD is already merged into the mainline kernel for v5.11, afaik. So we will definitely get a driver with kernel 5.11.
The only problem is, that it does not include an option to override the sensor mask if the manufacturer of the system did not write it into the P2C register as is the case on our HP Envy 13-ag0xxx models.
So the upstream driver is pretty useless to us.

My refactored patch series is a side-project I developed after the initial driver version of AMD was submitted upstream. I repeatedly asked for adding a kernel parameter to be able to override the sensor mask, which was confirmed as a good idea, but was never implemented. So I took a look at the code myself and added it. While I was at it, I encountered some more problems regarding the code structure and DMA handling, that I also reported upstream [1]. I also saw some unused code. So, to challenge myself to improve the driver, I read into the Linux kernel API documentation for the HID subsystem, PCI drivers and platform drivers and re-wrote the patch to what eventually became the patch series that I now submitted upstream.

I understand Hans' concern about the rewrite, but I think that it would benefit the Kernel to share my work on the driver. If the maintainers are reluctant to replace the driver with my changes, that's okay. They can still cherry pick what they regard as improvements on the code.

[1] https://lkml.org/lkml/2020/3/30/157
Comment 56 Richard Neumann 2021-01-24 12:04:41 UTC
@Dimitris, @Luya
I have added DMI-based model detection to my driver [1] (v. 2.9.8 in the AUR).
I successfully tested it on my system.
Please make sure that you disable i.e. comment out any sensor override if you want to test whether it works on your systems. The quirk should enable only the accelerometer and magnetometer for the devices that Luya and I posted above.

[1] https://github.com/conqp/amd-sfh-hid-dkms/blob/master/amd-sfh-quirks.c
Comment 57 Dimitris 2021-01-24 16:11:29 UTC
(In reply to Richard Neumann from comment #56)
> @Dimitris, @Luya
> I have added DMI-based model detection to my driver [1] (v. 2.9.8 in the
> AUR).

@Richard works beautifully (I removed the bitmask override)!
Comment 58 Luya Tshimbalanga 2021-01-27 01:57:40 UTC
(In reply to Richard Neumann from comment #56)
> @Dimitris, @Luya
> I have added DMI-based model detection to my driver [1] (v. 2.9.8 in the
> AUR).
> I successfully tested it on my system.
> Please make sure that you disable i.e. comment out any sensor override if
> you want to test whether it works on your systems. The quirk should enable
> only the accelerometer and magnetometer for the devices that Luya and I
> posted above.
> 
> [1] https://github.com/conqp/amd-sfh-hid-dkms/blob/master/amd-sfh-quirks.c

Thanks, I will try it on my Fedora system.
Comment 59 kernelbugzilla 2021-02-07 11:28:05 UTC
FYI, I unsuccessfully tested both the upstream driver and Richard's version on a Renoir device (Ryzen 3 4300U on a Asus Vivobook Flip 14 TM420IA). An accelerometer device is detected, but it returns no data.

Apparently, both drivers are missing a report descriptor for the newer Renoir hardware.

Reference: https://github.com/conqp/amd-sfh-hid-dkms/issues/1 (thanks Richard for the prompt reply there)
Comment 60 Marco 2021-10-12 10:44:49 UTC
(In reply to kernelbugzilla from comment #59)
> FYI, I unsuccessfully tested both the upstream driver and Richard's version
> on a Renoir device (Ryzen 3 4300U on a Asus Vivobook Flip 14 TM420IA). An
> accelerometer device is detected, but it returns no data.
> 
> Apparently, both drivers are missing a report descriptor for the newer
> Renoir hardware.
> 
> Reference: https://github.com/conqp/amd-sfh-hid-dkms/issues/1 (thanks
> Richard for the prompt reply there)

On the exact same device, but with a 4500U as of 5.14.11, I do get accelerometer data. 

The issues with sensors however is that I do not have an event for the lid flip on this device in order to disable automatically keyboard and trackpad in tablet mode and no way to use an apparently present ALS for automatic brightness (I'm not sure even if this device has one, but I've seen quite a lot of reference to a Capella Micro cm32181 in the DSDT under i2c, and the kernel even load the mentioned driver. No iio data anywhere, though). 

If the position of the hinges are derived from two accelerometers (one in the base, the other in the display) I would like to write an implementation around that, but the sensor reports only the one in the display panel and nothing else, so that's really not useful anyway.

However at least I can use display rotation with that.

I really hope that AMD will provide documentation around that or at least will properly support his sensor hub to Ryzen 4/5 devices themselves.
Comment 61 Stuart Morgan 2021-12-04 11:00:36 UTC
@Richard - it's been nearly a year since any updates to this ticket and zero activity on your github repo since Feb. Are you still fighting to get your fixes upstream, or is this effectively a dead effort?

Even with AMD's own official updates to 5.14 mid-year, the official SFH driver still doesn't work with the AMD 4000 series HP x360 laptops. I'm currently on 5.15.

I personally have an AY007NA (note AY not AG). I'm assuming this to be similar if not identical in behaviour to the AGxxx models and would therefore benefit from the same fixes.
Comment 62 Andy Shevchenko 2021-12-04 11:06:34 UTC
Add Mario. Mario, can you pull the strings to unstale this efforts?
Comment 63 Richard Neumann 2021-12-04 14:48:10 UTC
@Stuart
I am no longer interested in getting my refactored version upstream.
I tried to follow the upstream development of the driver by AMD to my best abilities, but I've not yet seen further development on "interesting" parts, such as the accelerometer support for the newest-generation sensor hubs or a lid switch driver. So yes, this is basically a dead effort.
If I should have overlooked upstream commits regarding above features, please let me know. I'm still interested in incorporating those into my refactored driver version.
Comment 64 Nikos Fytilis 2021-12-04 16:12:55 UTC
Hi, i stumbled across this report through the dkms sfh github issues.

I have a flex 5 with a 4500U and have not installed the dkms module. I am getting in_accel_[xyz]_raw in the sysfs directory of the iio device and they work (i can move the laptop and they change consistantly)

however the buffer0/* files do not change at all. is this the same issue with all of you?

if so, a userland solution could be very easy - basically a edited iio proxy capable of using the raw values.
Comment 65 Marco 2022-02-02 19:04:31 UTC
(In reply to Richard Neumann from comment #63)
> @Stuart
> I am no longer interested in getting my refactored version upstream.
> I tried to follow the upstream development of the driver by AMD to my best
> abilities, but I've not yet seen further development on "interesting" parts,
> such as the accelerometer support for the newest-generation sensor hubs or a
> lid switch driver. So yes, this is basically a dead effort.
> If I should have overlooked upstream commits regarding above features,
> please let me know. I'm still interested in incorporating those into my
> refactored driver version.

When I saw recently that you added lid switch support and I've got my hopes up in believing that at least that piece was fixed. Then I remembered that it unfortunately doesn't work under Ryzen 4000+ devices. I thought that the issue was on asus_nb_wmi driver, but there is no data for the lid sensor, I've just recently confirmed when I had a copy of Windows and removed the AMD UMDF Sensor driver. The automatic keyboard disabler was gone.

Essentially with the upstream kernel driver I have accelerometer data, but no lid switch. With yours unfortunately I haven't both.

I don't really get why they can't release documentation on this device if they do not want to support in Linux properly, frankly.

Hope that someone will be able to fix it,

Marco.
Comment 67 Alex Deucher 2022-02-23 20:30:42 UTC
Support for newer platforms was added last year:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0aad9c95eb9a2b086322e28ae2e58ad25598604e
Comment 68 Marco 2022-02-24 06:22:47 UTC
(In reply to Alex Deucher from comment #67)
> Support for newer platforms was added last year:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/
> ?id=0aad9c95eb9a2b086322e28ae2e58ad25598604e

As said above, this is correct. The issue is that the LID sensor coming from the SFH (the way that Windows tell if the system has switched into tablet mode, verified by myself after disabling the driver and losing that functionality) is still missing, and the older driver forked above had added support recently. Unfortunately that driver doesn't support Ryzen APU platforms after 3000 series.

Both options for me aren't unfortunately usable, and I still hope that someone from AMD will fix this issue.

Marco.