[PATCH] mei: me: add ice lake point LP iTouch DID

[PATCH] mei: me: add ice lake point LP iTouch DID

[Antonio Vázquez Blanco]

Add the device ID for the Intel Precise Touch & Stylus (iTouch) function
on Ice Lake Point LP. It appears as PCI device 8086:34e4, next to the
regular MEI function at 8086:34e0, on Microsoft Surface Pro 7 and
Surface Laptop 3.

Without this entry mei_me does not bind, the touch controller is
unreachable, and neither the touchscreen nor the pen work. Users
currently work around this by binding the device by hand at boot:

  echo "8086 34e4 ffffffff ffffffff 0 0 9" > \
          /sys/bus/pci/drivers/mei_me/new_id

where the trailing 9 is the mei_me_cfgs[] index of MEI_ME_PCH12_CFG.
That is fragile by construction, since the index is positional and any
reordering of the config array silently changes which config the device
gets.

This ID was first reported in June 2020, together with Sunrise Point's
0x9d3e, in "mei: me: Add itouch device IDs for SPT / ICP" [1]. That
submission was declined over the server platform ID conflict, and the
Sunrise Point half was subsequently added in commit 372a829862e5 ("mei:
me: add MEI device for SPT with ITPS capability"). Ice Lake Point was
never added.

Use MEI_ME_PCH12_CFG, matching the adjacent MEI_ICP_LP and MEI_ICP_N
entries.

Link: https://lore.kernel.org/all/20200601155954.764558-1-dorian.stoll@tmsp.io/ [1]
Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco@gmail.com>
---
Notes for review:

Built on char-misc-next (x86_64 defconfig + CONFIG_INTEL_MEI_ME=m); mei/mei-me
compile clean with no new warnings, and checkpatch --strict is clean.

I verified the resulting table entry in pci-me.o rather than just trusting the
macro: vendor=0x8086 device=0x34e4 subvendor/subdevice=PCI_ANY_ID
driver_data=9, i.e. byte-identical to what the hand-written new_id workaround
above produces. So this should be a no-op for anyone already using that, other
than making it unnecessary.

I do not have Ice Lake hardware, so this is NOT tested on a real device.

On the config choice: I used MEI_ME_PCH12_CFG to match the adjacent MEI_ICP_LP
and MEI_ICP_N entries, and because it is what users are already selecting by
hand. It does mean the device reports kind "mei" rather than "itouch" in sysfs.
The only existing Gen12 iTouch config, MEI_ME_PCH12_SPS_ITOUCH_CFG, also
carries MEI_CFG_FW_SPS_IGN and drops MEI_CFG_DMA_128, and I did not want to
assume either is correct for an LP client part, particularly as the DMA ring
is negotiated at runtime via hbm_f_dr_supported anyway. If you would prefer the
device kind reported correctly I can add a plain Gen12 iTouch config instead;
please say which you want.

 drivers/misc/mei/hw-me-regs.h | 1 +
 drivers/misc/mei/pci-me.c     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/misc/mei/hw-me-regs.h b/drivers/misc/mei/hw-me-regs.h
index b9a67629496b..3972f9504f12 100644
--- a/drivers/misc/mei/hw-me-regs.h
+++ b/drivers/misc/mei/hw-me-regs.h
@@ -94,6 +94,7 @@
 #define PCI_DEVICE_ID_INTEL_MEI_CDF        0x18D3  /* Cedar Fork */
 
 #define PCI_DEVICE_ID_INTEL_MEI_ICP_LP     0x34E0  /* Ice Lake Point LP */
+#define PCI_DEVICE_ID_INTEL_MEI_ICP_LP_3   0x34E4  /* Ice Lake Point LP 3 (iTouch) */
 #define PCI_DEVICE_ID_INTEL_MEI_ICP_N      0x38E0  /* Ice Lake Point N */
 
 #define PCI_DEVICE_ID_INTEL_MEI_JSP_N      0x4DE0  /* Jasper Lake Point N */
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 790a4647811b..e8805bcd3695 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -98,6 +98,7 @@ static const struct pci_device_id mei_me_pci_tbl[] = {
 	{ PCI_DEVICE_DATA(INTEL, MEI_CMP_H_3, MEI_ME_PCH8_ITOUCH_CFG) },
 
 	{ PCI_DEVICE_DATA(INTEL, MEI_ICP_LP, MEI_ME_PCH12_CFG) },
+	{ PCI_DEVICE_DATA(INTEL, MEI_ICP_LP_3, MEI_ME_PCH12_CFG) },
 	{ PCI_DEVICE_DATA(INTEL, MEI_ICP_N, MEI_ME_PCH12_CFG) },
 
 	{ PCI_DEVICE_DATA(INTEL, MEI_TGP_LP, MEI_ME_PCH15_CFG) },
-- 
2.56.0

RE: [PATCH] mei: me: add ice lake point LP iTouch DID

[Usyskin, Alexander]

> Subject: [PATCH] mei: me: add ice lake point LP iTouch DID
> 
> Add the device ID for the Intel Precise Touch & Stylus (iTouch) function
> on Ice Lake Point LP. It appears as PCI device 8086:34e4, next to the
> regular MEI function at 8086:34e0, on Microsoft Surface Pro 7 and
> Surface Laptop 3.
> 
> Without this entry mei_me does not bind, the touch controller is
> unreachable, and neither the touchscreen nor the pen work. Users
> currently work around this by binding the device by hand at boot:
> 
>   echo "8086 34e4 ffffffff ffffffff 0 0 9" > \
>           /sys/bus/pci/drivers/mei_me/new_id
> 
> where the trailing 9 is the mei_me_cfgs[] index of MEI_ME_PCH12_CFG.
> That is fragile by construction, since the index is positional and any
> reordering of the config array silently changes which config the device
> gets.
> 
> This ID was first reported in June 2020, together with Sunrise Point's
> 0x9d3e, in "mei: me: Add itouch device IDs for SPT / ICP" [1]. That
> submission was declined over the server platform ID conflict, and the
> Sunrise Point half was subsequently added in commit 372a829862e5 ("mei:
> me: add MEI device for SPT with ITPS capability"). Ice Lake Point was
> never added.
> 
> Use MEI_ME_PCH12_CFG, matching the adjacent MEI_ICP_LP and MEI_ICP_N
> entries.
> 
> Link: https://lore.kernel.org/all/20200601155954.764558-1-
> dorian.stoll@tmsp.io/ [1]
> Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco@gmail.com>
> ---
> Notes for review:
> 
> Built on char-misc-next (x86_64 defconfig + CONFIG_INTEL_MEI_ME=m);
> mei/mei-me
> compile clean with no new warnings, and checkpatch --strict is clean.
> 
> I verified the resulting table entry in pci-me.o rather than just trusting the
> macro: vendor=0x8086 device=0x34e4 subvendor/subdevice=PCI_ANY_ID
> driver_data=9, i.e. byte-identical to what the hand-written new_id
> workaround
> above produces. So this should be a no-op for anyone already using that,
> other
> than making it unnecessary.
> 
> I do not have Ice Lake hardware, so this is NOT tested on a real device.
> 
> On the config choice: I used MEI_ME_PCH12_CFG to match the adjacent
> MEI_ICP_LP
> and MEI_ICP_N entries, and because it is what users are already selecting by
> hand. It does mean the device reports kind "mei" rather than "itouch" in sysfs.
> The only existing Gen12 iTouch config, MEI_ME_PCH12_SPS_ITOUCH_CFG,
> also
> carries MEI_CFG_FW_SPS_IGN and drops MEI_CFG_DMA_128, and I did not
> want to
> assume either is correct for an LP client part, particularly as the DMA ring
> is negotiated at runtime via hbm_f_dr_supported anyway. If you would prefer
> the
> device kind reported correctly I can add a plain Gen12 iTouch config instead;
> please say which you want.
> 

Please use MEI_ME_PCH12_SPS_ITOUCH_CFG as right kind report is important.
And about servers' exclusion - better be safe then sorry here.

- - 
Thanks,
Sasha



>  drivers/misc/mei/hw-me-regs.h | 1 +
>  drivers/misc/mei/pci-me.c     | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/misc/mei/hw-me-regs.h b/drivers/misc/mei/hw-me-regs.h
> index b9a67629496b..3972f9504f12 100644
> --- a/drivers/misc/mei/hw-me-regs.h
> +++ b/drivers/misc/mei/hw-me-regs.h
> @@ -94,6 +94,7 @@
>  #define PCI_DEVICE_ID_INTEL_MEI_CDF        0x18D3  /* Cedar Fork */
> 
>  #define PCI_DEVICE_ID_INTEL_MEI_ICP_LP     0x34E0  /* Ice Lake Point LP */
> +#define PCI_DEVICE_ID_INTEL_MEI_ICP_LP_3   0x34E4  /* Ice Lake Point LP 3
> (iTouch) */
>  #define PCI_DEVICE_ID_INTEL_MEI_ICP_N      0x38E0  /* Ice Lake Point N */
> 
>  #define PCI_DEVICE_ID_INTEL_MEI_JSP_N      0x4DE0  /* Jasper Lake Point N
> */
> diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
> index 790a4647811b..e8805bcd3695 100644
> --- a/drivers/misc/mei/pci-me.c
> +++ b/drivers/misc/mei/pci-me.c
> @@ -98,6 +98,7 @@ static const struct pci_device_id mei_me_pci_tbl[] = {
>  	{ PCI_DEVICE_DATA(INTEL, MEI_CMP_H_3,
> MEI_ME_PCH8_ITOUCH_CFG) },
> 
>  	{ PCI_DEVICE_DATA(INTEL, MEI_ICP_LP, MEI_ME_PCH12_CFG) },
> +	{ PCI_DEVICE_DATA(INTEL, MEI_ICP_LP_3, MEI_ME_PCH12_CFG) },
>  	{ PCI_DEVICE_DATA(INTEL, MEI_ICP_N, MEI_ME_PCH12_CFG) },
> 
>  	{ PCI_DEVICE_DATA(INTEL, MEI_TGP_LP, MEI_ME_PCH15_CFG) },
> --
> 2.56.0

[PATCH v2] mei: me: add ice lake point LP iTouch DID

[Antonio Vázquez Blanco]

Add the device ID for the Intel Precise Touch & Stylus (iTouch) function
on Ice Lake Point LP. It appears as PCI device 8086:34e4, next to the
regular MEI function at 8086:34e0, on Microsoft Surface Pro 7 and
Surface Laptop 3.

Without this entry mei_me does not bind, the touch controller is
unreachable, and neither the touchscreen nor the pen work. Users
currently work around this by binding the device by hand at boot:

  echo "8086 34e4 ffffffff ffffffff 0 0 9" > \
          /sys/bus/pci/drivers/mei_me/new_id

where the trailing 9 is the mei_me_cfgs[] index of MEI_ME_PCH12_CFG.
That is fragile by construction, since the index is positional and any
reordering of the config array silently changes which config the device
gets.

This ID was first reported in June 2020, together with Sunrise Point's
0x9d3e, in "mei: me: Add itouch device IDs for SPT / ICP" [1]. That
submission was declined over the server platform ID conflict, and the
Sunrise Point half was subsequently added in commit 372a829862e5 ("mei:
me: add MEI device for SPT with ITPS capability"). Ice Lake Point was
never added.

Use MEI_ME_PCH12_SPS_ITOUCH_CFG, so that the device is reported with the
correct kind and the SPS firmware exclusion applies. Note this is not the
config the manual new_id workaround above selects: it reports kind
"itouch" rather than "mei", and it does not set up a DMA ring.

Link: https://lore.kernel.org/all/20200601155954.764558-1-dorian.stoll@tmsp.io/ [1]
Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco@gmail.com>
---
v2: use MEI_ME_PCH12_SPS_ITOUCH_CFG instead of MEI_ME_PCH12_CFG, as
    requested by Alexander Usyskin. Commit message updated to say why, and
    to spell out that this is deliberately not the config the new_id
    workaround selects.

v1: https://lore.kernel.org/all/20261028221017.54426-1-antoniovazquezblanco@gmail.com/

Notes for review:

Built on char-misc-next (x86_64 defconfig + CONFIG_INTEL_MEI_ME=m); mei and
mei-me compile clean with no new warnings, and checkpatch --strict is clean.
The emitted table entry in pci-me.o is vendor=0x8086 device=0x34e4
subvendor/subdevice=PCI_ANY_ID driver_data=12, which is
MEI_ME_PCH12_SPS_ITOUCH_CFG as intended.

I still do not have Ice Lake hardware, so this is NOT tested on a real device.

One thing worth flagging explicitly: unlike v1, this is a behavioural change
for people currently using the new_id workaround, since that selects
MEI_ME_PCH12_CFG and so gives them a DMA ring where this patch will not. I am
following your call on the hardware, but it is a real difference rather than a
relabelling, so a Tested-by on a Surface Pro 7 or Laptop 3 would be more useful
here than it was for v1.

 drivers/misc/mei/hw-me-regs.h | 1 +
 drivers/misc/mei/pci-me.c     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/misc/mei/hw-me-regs.h b/drivers/misc/mei/hw-me-regs.h
index b9a67629496b..3972f9504f12 100644
--- a/drivers/misc/mei/hw-me-regs.h
+++ b/drivers/misc/mei/hw-me-regs.h
@@ -94,6 +94,7 @@
 #define PCI_DEVICE_ID_INTEL_MEI_CDF        0x18D3  /* Cedar Fork */
 
 #define PCI_DEVICE_ID_INTEL_MEI_ICP_LP     0x34E0  /* Ice Lake Point LP */
+#define PCI_DEVICE_ID_INTEL_MEI_ICP_LP_3   0x34E4  /* Ice Lake Point LP 3 (iTouch) */
 #define PCI_DEVICE_ID_INTEL_MEI_ICP_N      0x38E0  /* Ice Lake Point N */
 
 #define PCI_DEVICE_ID_INTEL_MEI_JSP_N      0x4DE0  /* Jasper Lake Point N */
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 790a4647811b..cfdef9531ab1 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -98,6 +98,7 @@ static const struct pci_device_id mei_me_pci_tbl[] = {
 	{ PCI_DEVICE_DATA(INTEL, MEI_CMP_H_3, MEI_ME_PCH8_ITOUCH_CFG) },
 
 	{ PCI_DEVICE_DATA(INTEL, MEI_ICP_LP, MEI_ME_PCH12_CFG) },
+	{ PCI_DEVICE_DATA(INTEL, MEI_ICP_LP_3, MEI_ME_PCH12_SPS_ITOUCH_CFG) },
 	{ PCI_DEVICE_DATA(INTEL, MEI_ICP_N, MEI_ME_PCH12_CFG) },
 
 	{ PCI_DEVICE_DATA(INTEL, MEI_TGP_LP, MEI_ME_PCH15_CFG) },
-- 
2.56.0