ipts: Let the companion decide about no_feedback

Having a DMI match for some surface devices doesn't really match with
the concept of having a device-specific companion driver, so this commit
moves the matching code to the companion.

ipts-surface is now setup in a way that allows us to add device-specific
data pretty easily. All you need is the interface to IPTS, which can
just be copied from no_feedback.

Signed-off-by: Dorian Stoll <dorian.stoll@tmsp.io>

drivers/misc/ipts/companion.c

@@ -209,3 +209,22 @@ int ipts_request_firmware_config(struct ipts_info *ipts,
  return ret;
 
 }
+
+bool ipts_needs_no_feedback(void)
+{
+ bool ret;
+
+ // Make sure that access to the companion is synchronized
+ mutex_lock(&ipts_companion_lock);
+
+ // If the companion is ignored, or doesn't exist, assume that
+ // the device doesn't need no_feedback enabled
+ if (ipts_modparams.ignore_companion || ipts_companion == NULL)
+ ret = false;
+ else
+ ret = ipts_companion->needs_no_feedback(ipts_companion);
+
+ mutex_unlock(&ipts_companion_lock);
+
+ return ret;
+}

drivers/misc/ipts/companion.h

@@ -15,6 +15,7 @@
 #include "ipts.h"
 
 bool ipts_companion_available(void);
+bool ipts_needs_no_feedback(void);
 
 int ipts_request_firmware(const struct firmware **fw, const char *name,
  struct device *device);

drivers/misc/ipts/companion/ipts-surface.c

@@ -26,19 +26,64 @@
  MODULE_FIRMWARE("intel/ipts/" X "/vendor_desc.bin"); \
  MODULE_FIRMWARE("intel/ipts/" X "/vendor_kernel.bin")
 
+struct ipts_surface_data {
+ const char *hid;
+ bool no_feedback;
+};
+
+// Surface Book 1 / Surface Studio
+static const struct ipts_surface_data ipts_surface_mshw0076 = {
+ .hid = "MSHW0076",
+ .no_feedback = true,
+};
+
+// Surface Pro 4
+static const struct ipts_surface_data ipts_surface_mshw0078 = {
+ .hid = "MSHW0078",
+ .no_feedback = true,
+};
+
+// Surface Laptop 1 / 2
+static const struct ipts_surface_data ipts_surface_mshw0079 = {
+ .hid = "MSHW0079",
+ .no_feedback = false,
+};
+
+// Surface Pro 5 / 6
+static const struct ipts_surface_data ipts_surface_mshw0101 = {
+ .hid = "MSHW0101",
+ .no_feedback = false,
+};
+
+// Surface Book 2 15"
+static const struct ipts_surface_data ipts_surface_mshw0102 = {
+ .hid = "MSHW0102",
+ .no_feedback = false,
+};
+
+// Unknown, but firmware exists
+static const struct ipts_surface_data ipts_surface_mshw0103 = {
+ .hid = "MSHW0103",
+ .no_feedback = false,
+};
+
+// Surface Book 2 13"
+static const struct ipts_surface_data ipts_surface_mshw0137 = {
+ .hid = "MSHW0137",
+ .no_feedback = false,
+};
+
+/*
+ * Checkpatch complains about the following lines because it sees them as
+ * header files mixed with .c files. However, forward declaration is perfectly
+ * fine in C, and this allows us to seperate the companion data from the
+ * functions for the companion.
+ */
 int ipts_surface_request_firmware(struct ipts_companion *companion,
  const struct firmware **fw, const char *name,
- struct device *device)
-{
- char fw_path[MAX_IOCL_FILE_PATH_LEN];
+ struct device *device);
 
- if (companion == NULL || companion->data == NULL)
- return -ENOENT;
-
- snprintf(fw_path, MAX_IOCL_FILE_PATH_LEN, IPTS_SURFACE_FW_PATH_FMT,
- (const char *)companion->data, name);
- return request_firmware(fw, fw_path, device);
-}
+bool ipts_surface_needs_no_feedback(struct ipts_companion *companion);
 
 static struct ipts_bin_fw_info ipts_surface_vendor_kernel = {
  .fw_name = "vendor_kernel.bin",
@@ -74,20 +119,53 @@ static struct ipts_bin_fw_info *ipts_surface_fw_config[] = {
 static struct ipts_companion ipts_surface_companion = {
  .firmware_request = &ipts_surface_request_firmware,
  .firmware_config = ipts_surface_fw_config,
+ .needs_no_feedback = &ipts_surface_needs_no_feedback,
  .name = "ipts_surface",
 };
 
+int ipts_surface_request_firmware(struct ipts_companion *companion,
+ const struct firmware **fw, const char *name,
+ struct device *device)
+{
+ char fw_path[MAX_IOCL_FILE_PATH_LEN];
+ struct ipts_surface_data *data;
+
+ if (companion == NULL || companion->data == NULL)
+ return -ENOENT;
+
+ data = (struct ipts_surface_data *)companion->data;
+
+ snprintf(fw_path, MAX_IOCL_FILE_PATH_LEN, IPTS_SURFACE_FW_PATH_FMT,
+ data->hid, name);
+ return request_firmware(fw, fw_path, device);
+}
+
+bool ipts_surface_needs_no_feedback(struct ipts_companion *companion)
+{
+ struct ipts_surface_data *data;
+
+ // In case something went wrong, assume that the device doesn't
+ // need the no_feedback option set
+ if (companion == NULL || companion->data == NULL)
+ return false;
+
+ data = (struct ipts_surface_data *)companion->data;
+
+ return data->no_feedback;
+}
+
 static int ipts_surface_probe(struct platform_device *pdev)
 {
  int r;
- struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+ const struct ipts_surface_data *data =
+ acpi_device_get_match_data(&pdev->dev);
 
- if (!adev) {
+ if (!data) {
  dev_err(&pdev->dev, "Unable to find ACPI info for device\n");
  return -ENODEV;
  }
 
- ipts_surface_companion.data = (void *)acpi_device_hid(adev);
+ ipts_surface_companion.data = (void *)data;
 
  r = ipts_add_companion(&ipts_surface_companion);
  if (r) {
@@ -111,13 +189,13 @@ static int ipts_surface_remove(struct platform_device *pdev)
 }
 
 static const struct acpi_device_id ipts_surface_acpi_match[] = {
- { "MSHW0076", 0 }, // Surface Book 1 / Surface Studio
- { "MSHW0078", 0 }, // Surface Pro 4
- { "MSHW0079", 0 }, // Surface Laptop 1 / 2
- { "MSHW0101", 0 }, // Surface Book 2 15"
- { "MSHW0102", 0 }, // Surface Pro 2017 / 6
- { "MSHW0103", 0 }, // unknown, but firmware exists
- { "MSHW0137", 0 }, // Surface Book 2
+ { "MSHW0076", (unsigned long)&ipts_surface_mshw0076 },
+ { "MSHW0078", (unsigned long)&ipts_surface_mshw0078 },
+ { "MSHW0079", (unsigned long)&ipts_surface_mshw0079 },
+ { "MSHW0101", (unsigned long)&ipts_surface_mshw0101 },
+ { "MSHW0102", (unsigned long)&ipts_surface_mshw0102 },
+ { "MSHW0103", (unsigned long)&ipts_surface_mshw0103 },
+ { "MSHW0137", (unsigned long)&ipts_surface_mshw0137 },
  { },
 };
 MODULE_DEVICE_TABLE(acpi, ipts_surface_acpi_match);
@@ -142,4 +220,5 @@ IPTS_SURFACE_FIRMWARE("MSHW0079");
 IPTS_SURFACE_FIRMWARE("MSHW0101");
 IPTS_SURFACE_FIRMWARE("MSHW0102");
 IPTS_SURFACE_FIRMWARE("MSHW0103");
+
 IPTS_SURFACE_FIRMWARE("MSHW0137");

drivers/misc/ipts/hid.c

@@ -47,24 +47,6 @@ struct kernel_output_payload_error {
  char string[128];
 };
 
-static const struct dmi_system_id no_feedback_dmi_table[] = {
- {
- .matches = {
- DMI_EXACT_MATCH(DMI_SYS_VENDOR,
- "Microsoft Corporation"),
- DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book"),
- },
- },
- {
- .matches = {
- DMI_EXACT_MATCH(DMI_SYS_VENDOR,
- "Microsoft Corporation"),
- DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 4"),
- },
- },
- { }
-};
-
 static int ipts_hid_get_descriptor(struct ipts_info *ipts,
  u8 **desc, int *size)
 {
@@ -448,6 +430,10 @@ static int handle_outputs(struct ipts_info *ipts, int parallel_idx)
  * The most desirable fix could be done by raising IPTS GuC priority.
  * Until we find a better solution, use this workaround.
  *
+ * The decision which devices have no_feedback enabled by default is
+ * made by the companion driver. If no companion driver was loaded,
+ * no_feedback is disabled and the default behaviour is used.
+ *
  * Link to the comment where sebanc found this workaround:
  * https://github.com/jakeday/linux-surface/issues/374#issuecomment-508234110
  * (Touch and pen issue persists · Issue #374 · jakeday/linux-surface)
@@ -458,12 +444,8 @@ static int handle_outputs(struct ipts_info *ipts, int parallel_idx)
  */
  if (fb_buf) {
  // A negative value means "decide by dmi table"
- if (ipts_modparams.no_feedback < 0) {
- if (dmi_check_system(no_feedback_dmi_table))
- ipts_modparams.no_feedback = true;
- else
- ipts_modparams.no_feedback = false;
- }
+ if (ipts_modparams.no_feedback < 0)
+ ipts_modparams.no_feedback = ipts_needs_no_feedback();
 
  if (ipts_modparams.no_feedback)
  return 0;

include/linux/ipts-companion.h

@@ -14,6 +14,7 @@
 #include <linux/ipts-binary.h>
 
 struct ipts_companion {
+ bool (*needs_no_feedback)(struct ipts_companion *companion);
  int (*firmware_request)(struct ipts_companion *companion,
  const struct firmware **fw,
  const char *name, struct device *device);