mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] Add 'hid-valve-index' reset driver
@ 2026-09-10 17:02 Mario Limonciello
  2026-09-10 17:02 ` [PATCH 1/3] HID: Add shutdown callback for device drivers Mario Limonciello
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Mario Limonciello @ 2026-09-10 17:02 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Greg Kroah-Hartman
  Cc: Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM, Curtis Vogt, Mario Limonciello

The Valve Index has a problem that when the DP link is torn down
(such as a power state transition) the device stops responding to EDID
requests the next time it goes up.  Either manually resetting the device
or hotplugging it brings it back to normal behavior.

Introduce a driver that during a power state transition will reset the
device.  This helps most cases, but if the system crashes the device
can still be in a bad state.  So also export a sysfs file that userspace
could potentially use to trigger a reset on demand while in this circumstance.
Curtis Vogt (1):
  USB: quirks: Ignore remote wakeup from the Valve Index breakout box
    hub

Mario Limonciello (2):
  HID: Add shutdown callback for device drivers
  HID: valve-index: Reboot headset on system power transitions

 .../ABI/testing/sysfs-driver-hid-valve-index  |  12 ++
 drivers/hid/Kconfig                           |  11 ++
 drivers/hid/Makefile                          |   1 +
 drivers/hid/hid-core.c                        |  13 ++
 drivers/hid/hid-ids.h                         |   1 +
 drivers/hid/hid-valve-index.c                 | 142 ++++++++++++++++++
 drivers/usb/core/quirks.c                     |   4 +
 include/linux/hid.h                           |   2 +
 8 files changed, 186 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-valve-index
 create mode 100644 drivers/hid/hid-valve-index.c

-- 
2.43.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] HID: Add shutdown callback for device drivers
  2026-09-10 17:02 [PATCH 0/3] Add 'hid-valve-index' reset driver Mario Limonciello
@ 2026-09-10 17:02 ` Mario Limonciello
  2026-09-10 17:02 ` [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions Mario Limonciello
  2026-09-10 17:02 ` [PATCH 3/3] USB: quirks: Ignore remote wakeup from the Valve Index breakout box hub Mario Limonciello
  2 siblings, 0 replies; 17+ messages in thread
From: Mario Limonciello @ 2026-09-10 17:02 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Greg Kroah-Hartman
  Cc: Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM, Curtis Vogt, Mario Limonciello

HID device drivers can receive suspend and resume notifications through
callbacks forwarded by their transport driver.  There is no corresponding
way to perform device-specific work during an orderly system shutdown.

Add a shutdown callback to struct hid_driver and dispatch it from the HID
bus shutdown operation.  This allows a HID device driver to communicate
with hardware before its transport is shut down.

Assisted-by: LLM
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/hid/hid-core.c | 13 +++++++++++++
 include/linux/hid.h    |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a3ff0514f9cdf..7c8c1aa1f09c3 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2738,6 +2738,18 @@ static int hid_bus_match(struct device *dev, const struct device_driver *drv)
 	return hid_match_device(hdev, hdrv) != NULL;
 }
 
+static void hid_bus_shutdown(struct device *dev)
+{
+	struct hid_driver *hdrv;
+
+	if (!dev->driver)
+		return;
+
+	hdrv = to_hid_driver(dev->driver);
+	if (hdrv->shutdown)
+		hdrv->shutdown(to_hid_device(dev));
+}
+
 /**
  * hid_compare_device_paths - check if both devices share the same path
  * @hdev_a: hid device
@@ -3012,6 +3024,7 @@ const struct bus_type hid_bus_type = {
 	.match		= hid_bus_match,
 	.probe		= hid_device_probe,
 	.remove		= hid_device_remove,
+	.shutdown	= hid_bus_shutdown,
 	.uevent		= hid_uevent,
 };
 EXPORT_SYMBOL(hid_bus_type);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 8d17b741638c9..ddf57779bb3ef 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -861,6 +861,7 @@ struct hid_usage_id {
  * @suspend: invoked on suspend (NULL means nop)
  * @resume: invoked on resume if device was not reset (NULL means nop)
  * @reset_resume: invoked on resume if device was reset (NULL means nop)
+ * @shutdown: invoked on system shutdown (NULL means nop)
  * @on_hid_hw_open: invoked when hid core opens first instance (NULL means nop)
  * @on_hid_hw_close: invoked when hid core closes last instance (NULL means nop)
  *
@@ -924,6 +925,7 @@ struct hid_driver {
 	int (*suspend)(struct hid_device *hdev, pm_message_t message);
 	int (*resume)(struct hid_device *hdev);
 	int (*reset_resume)(struct hid_device *hdev);
+	void (*shutdown)(struct hid_device *hdev);
 	void (*on_hid_hw_open)(struct hid_device *hdev);
 	void (*on_hid_hw_close)(struct hid_device *hdev);
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-10 17:02 [PATCH 0/3] Add 'hid-valve-index' reset driver Mario Limonciello
  2026-09-10 17:02 ` [PATCH 1/3] HID: Add shutdown callback for device drivers Mario Limonciello
@ 2026-09-10 17:02 ` Mario Limonciello
  2026-09-10 20:04   ` Michal Pecio
  2026-09-11  5:54   ` Greg Kroah-Hartman
  2026-09-10 17:02 ` [PATCH 3/3] USB: quirks: Ignore remote wakeup from the Valve Index breakout box hub Mario Limonciello
  2 siblings, 2 replies; 17+ messages in thread
From: Mario Limonciello @ 2026-09-10 17:02 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Greg Kroah-Hartman
  Cc: Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM, Curtis Vogt, Mario Limonciello

The Valve Index HMD stops serving its EDID after the host disables the
DisplayPort PHY.  The headset remains powered by its breakout box across
suspend and shutdown, so the bad state survives and the next connector
detection reports "No EDID read".  The HMD then appears as a synthesized
640x480 display until it is power-cycled.

The 64-byte HID output report 0x16 with command 0x01 reboots the headset
and restores its EDID service.  Add a device-specific driver which sends
this report for system sleep transitions and orderly shutdown while leaving
runtime autosuspend alone.

Resume a runtime-suspended interface for a shutdown request and restrict
the command to the composite interface which declares report 0x16.

Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/4333
Link: https://github.com/ValveSoftware/SteamVR-for-Linux/issues/939
Assisted-by: LLM
Co-developed-by: Curtis Vogt <curtis.vogt@gmail.com>
Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 .../ABI/testing/sysfs-driver-hid-valve-index  |  12 ++
 drivers/hid/Kconfig                           |  11 ++
 drivers/hid/Makefile                          |   1 +
 drivers/hid/hid-ids.h                         |   1 +
 drivers/hid/hid-valve-index.c                 | 142 ++++++++++++++++++
 5 files changed, 167 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-valve-index
 create mode 100644 drivers/hid/hid-valve-index.c

diff --git a/Documentation/ABI/testing/sysfs-driver-hid-valve-index b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
new file mode 100644
index 0000000000000..47d8c26b1eace
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
@@ -0,0 +1,12 @@
+What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/reboot
+Date:		October 2026
+Contact:	linux-input@vger.kernel.org
+Description:
+		Writing a boolean true value reboots the Valve Index headset to
+		recover its EDID service. Writing a boolean false value has no
+		effect. This file is write-only.
+
+		The Valve Index is a composite HID device. The reboot command is
+		only supported by the interface that provides the headset's 64-byte
+		output report. Writing true to this file on another interface fails
+		with -ENODEV.
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a81bf51cbcf10..8ea2dd570058f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -547,6 +547,17 @@ config HID_WALTOP
 	help
 	Support for Waltop tablets.
 
+config HID_VALVE_INDEX
+	tristate "Valve Index headset"
+	depends on USB_HID
+	help
+	  Support for the Valve Index headset. This driver works around the
+	  headset failing to provide its EDID after a DisplayPort link shutdown
+	  by rebooting the headset on resume from system suspend and at shutdown.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called hid-valve-index.
+
 config HID_VIEWSONIC
 	tristate "ViewSonic/Signotec"
 	help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 48a863b245eed..21f512cab6250 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -156,6 +156,7 @@ obj-$(CONFIG_HID_XIAOMI)	+= hid-xiaomi.o
 obj-$(CONFIG_HID_XINMO)		+= hid-xinmo.o
 obj-$(CONFIG_HID_ZEROPLUS)	+= hid-zpff.o
 obj-$(CONFIG_HID_ZYDACRON)	+= hid-zydacron.o
+obj-$(CONFIG_HID_VALVE_INDEX)	+= hid-valve-index.o
 obj-$(CONFIG_HID_VIEWSONIC)	+= hid-viewsonic.o
 obj-$(CONFIG_HID_VRC2)		+= hid-vrc2.o
 obj-$(CONFIG_HID_HUAWEI)	+= hid-huawei.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b3aca5aa91767..15cd29a338a8d 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1391,6 +1391,7 @@
 #define USB_DEVICE_ID_STEAM_CONTROLLER_IBEX_BLE	0x1303
 #define USB_DEVICE_ID_STEAM_CONTROLLER_PROTEUS	0x1304
 #define USB_DEVICE_ID_STEAM_CONTROLLER_NEREID	0x1305
+#define USB_DEVICE_ID_VALVE_INDEX_HEADSET	0x2300
 
 #define USB_VENDOR_ID_STEELSERIES	0x1038
 #define USB_DEVICE_ID_STEELSERIES_SRWS1	0x1410
diff --git a/drivers/hid/hid-valve-index.c b/drivers/hid/hid-valve-index.c
new file mode 100644
index 0000000000000..43c1142b7215b
--- /dev/null
+++ b/drivers/hid/hid-valve-index.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for the Valve Index headset
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+#define VALVE_INDEX_REBOOT_REPORT_ID	0x16
+#define VALVE_INDEX_REBOOT_CMD		0x01
+#define VALVE_INDEX_REPORT_SIZE		64
+
+static bool valve_index_has_reboot_report(struct hid_device *hdev)
+{
+	struct hid_report *report;
+
+	/*
+	 * The reboot command is a vendor protocol carried in the unnumbered
+	 * 64-byte output report of the headset's third interface; the first
+	 * data byte is the command id.  Report 0x16 is only declared as a
+	 * feature report and is not what the command is sent as.
+	 */
+	report = hdev->report_enum[HID_OUTPUT_REPORT].report_id_hash[0];
+
+	return report && hid_report_len(report) == VALVE_INDEX_REPORT_SIZE;
+}
+
+static void valve_index_reboot(struct hid_device *hdev, bool wake)
+{
+	u8 *report;
+	int ret;
+
+	if (!valve_index_has_reboot_report(hdev))
+		return;
+
+	/* USB transfer buffers must be DMA-able, so not on the stack. */
+	report = kzalloc(VALVE_INDEX_REPORT_SIZE, GFP_KERNEL);
+	if (!report)
+		return;
+	report[0] = VALVE_INDEX_REBOOT_REPORT_ID;
+	report[1] = VALVE_INDEX_REBOOT_CMD;
+
+	if (wake) {
+		ret = hid_hw_power(hdev, PM_HINT_FULLON);
+		if (ret < 0) {
+			hid_warn(hdev, "failed to resume headset for reboot: %d\n",
+				 ret);
+			goto out;
+		}
+	}
+
+	/* Use the same interrupt-out then SET_REPORT fallback as hidraw. */
+	ret = hid_hw_output_report(hdev, report, VALVE_INDEX_REPORT_SIZE);
+	if (ret == -ENOSYS)
+		ret = hid_hw_raw_request(hdev, report[0], report,
+					 VALVE_INDEX_REPORT_SIZE,
+					 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+	if (ret < 0)
+		hid_warn(hdev, "failed to reboot headset: %d\n", ret);
+	else if (ret != VALVE_INDEX_REPORT_SIZE)
+		hid_warn(hdev, "short headset reboot report: %d\n", ret);
+
+	if (wake)
+		hid_hw_power(hdev, PM_HINT_NORMAL);
+out:
+	kfree(report);
+}
+
+/*
+ * The suspend and shutdown hooks only cover orderly power transitions.  After
+ * a crash, a hard reset or a power cut the headset is left in the state where
+ * its EDID no longer reads, and nothing recovers it until the next orderly
+ * transition.  Expose the reboot command as a write-only "reboot" attribute
+ * on the HID device so userspace can recover it, for instance from a udev
+ * rule that fires only when the connector reports no EDID.  Writing to an
+ * interface that does not carry the reboot report returns -ENODEV.
+ */
+static ssize_t reboot_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct hid_device *hdev = to_hid_device(dev);
+	bool val;
+
+	if (kstrtobool(buf, &val))
+		return -EINVAL;
+	if (!val)
+		return count;
+	if (!valve_index_has_reboot_report(hdev))
+		return -ENODEV;
+
+	valve_index_reboot(hdev, true);
+
+	return count;
+}
+static DEVICE_ATTR_WO(reboot);
+
+static struct attribute *valve_index_attrs[] = {
+	&dev_attr_reboot.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(valve_index);
+
+/*
+ * The headset's EDID service is lost when the host disables the DisplayPort
+ * PHY during system suspend, so it needs the reboot on the way out of
+ * suspend.  Doing it on the way in does not work: the headset dropping off
+ * USB is a remote-wakeup event from its hub and aborts the suspend.
+ */
+static int valve_index_resume(struct hid_device *hdev)
+{
+	valve_index_reboot(hdev, false);
+
+	return 0;
+}
+
+static void valve_index_shutdown(struct hid_device *hdev)
+{
+	valve_index_reboot(hdev, true);
+}
+
+static const struct hid_device_id valve_index_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
+			 USB_DEVICE_ID_VALVE_INDEX_HEADSET) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, valve_index_devices);
+
+static struct hid_driver valve_index_driver = {
+	.name = "valve-index",
+	.id_table = valve_index_devices,
+	.resume = valve_index_resume,
+	.reset_resume = valve_index_resume,
+	.shutdown = valve_index_shutdown,
+	.driver.dev_groups = valve_index_groups,
+};
+module_hid_driver(valve_index_driver);
+
+MODULE_AUTHOR("Mario Limonciello <mario.limonciello@amd.com>");
+MODULE_DESCRIPTION("HID driver for Valve Index headset");
+MODULE_LICENSE("GPL");
-- 
2.43.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/3] USB: quirks: Ignore remote wakeup from the Valve Index breakout box hub
  2026-09-10 17:02 [PATCH 0/3] Add 'hid-valve-index' reset driver Mario Limonciello
  2026-09-10 17:02 ` [PATCH 1/3] HID: Add shutdown callback for device drivers Mario Limonciello
  2026-09-10 17:02 ` [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions Mario Limonciello
@ 2026-09-10 17:02 ` Mario Limonciello
  2 siblings, 0 replies; 17+ messages in thread
From: Mario Limonciello @ 2026-09-10 17:02 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Greg Kroah-Hartman
  Cc: Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM, Curtis Vogt, Mario Limonciello

From: Curtis Vogt <curtis.vogt@gmail.com>

The hub in the Valve Index breakout box (28de:2613, bcdDevice 1.82) asserts
remote wakeup a few seconds after the host enters S3, so a system with the
headset attached does not stay suspended.  Observed on 7.3-rc2 with an
amdgpu host: every suspend attempt returned after 4-8 seconds with wakeup
events recorded on the hub and its xHCI controller, with no HID driver
bound to the headset.  Disabling wakeup on the hub through sysfs lets the
same system stay asleep until woken from the front panel.  The headset
most likely reacts to the DisplayPort link being taken down as the host
suspends.

Nothing behind this hub is a device that should be able to wake the host
(the headset, its radio and microphone, and a further hub), so ignore its
remote wakeup capability, as is already done for the ASUS T100 base
station's hub.

Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/usb/core/quirks.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index b5b577f0b931c..219e1cf5ebf00 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -596,6 +596,10 @@ static const struct usb_device_id usb_quirk_list[] = {
 	/* ShanWan Wireless Gamepad */
 	{ USB_DEVICE(0x2563, 0x0575), .driver_info = USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE },
 
+	/* Valve Index breakout box hub */
+	{ USB_DEVICE(0x28de, 0x2613), .driver_info =
+			USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
 	/* UGREEN 35871 - BOS descriptor fetch hangs at SuperSpeed Plus */
 	{ USB_DEVICE(0x2b89, 0x5871), .driver_info = USB_QUIRK_NO_BOS },
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-10 17:02 ` [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions Mario Limonciello
@ 2026-09-10 20:04   ` Michal Pecio
  2026-09-10 20:43     ` Mario Limonciello
  2026-09-11  5:54   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 17+ messages in thread
From: Michal Pecio @ 2026-09-10 20:04 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Jiri Kosina, Benjamin Tissoires, Greg Kroah-Hartman,
	Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM, Curtis Vogt

On Thu, 10 Sep 2026 12:02:53 -0500, Mario Limonciello wrote:
> The Valve Index HMD stops serving its EDID after the host disables the
> DisplayPort PHY.  The headset remains powered by its breakout box across
> suspend and shutdown, so the bad state survives and the next connector
> detection reports "No EDID read".  The HMD then appears as a synthesized
> 640x480 display until it is power-cycled.
> 
> The 64-byte HID output report 0x16 with command 0x01 reboots the headset
> and restores its EDID service.  Add a device-specific driver which sends
> this report for system sleep transitions and orderly shutdown while leaving
> runtime autosuspend alone.
> 
> Resume a runtime-suspended interface for a shutdown request and restrict
> the command to the composite interface which declares report 0x16.
> 
> Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/4333

Hmm, people say it's a regression, so it looks like at least one
alternative solution should, in theory, exist...

Obligatory question: does it work any better with Windows? :)

> Link: https://github.com/ValveSoftware/SteamVR-for-Linux/issues/939
> Assisted-by: LLM
> Co-developed-by: Curtis Vogt <curtis.vogt@gmail.com>
> Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  .../ABI/testing/sysfs-driver-hid-valve-index  |  12 ++
>  drivers/hid/Kconfig                           |  11 ++
>  drivers/hid/Makefile                          |   1 +
>  drivers/hid/hid-ids.h                         |   1 +
>  drivers/hid/hid-valve-index.c                 | 142 ++++++++++++++++++
>  5 files changed, 167 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-valve-index
>  create mode 100644 drivers/hid/hid-valve-index.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-hid-valve-index b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
> new file mode 100644
> index 0000000000000..47d8c26b1eace
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
> @@ -0,0 +1,12 @@
> +What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/reboot
> +Date:		October 2026
> +Contact:	linux-input@vger.kernel.org
> +Description:
> +		Writing a boolean true value reboots the Valve Index headset to
> +		recover its EDID service. Writing a boolean false value has no
> +		effect. This file is write-only.
> +
> +		The Valve Index is a composite HID device. The reboot command is
> +		only supported by the interface that provides the headset's 64-byte
> +		output report. Writing true to this file on another interface fails
> +		with -ENODEV.
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index a81bf51cbcf10..8ea2dd570058f 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -547,6 +547,17 @@ config HID_WALTOP
>  	help
>  	Support for Waltop tablets.
>  
> +config HID_VALVE_INDEX
> +	tristate "Valve Index headset"
> +	depends on USB_HID
> +	help
> +	  Support for the Valve Index headset. This driver works around the
> +	  headset failing to provide its EDID after a DisplayPort link shutdown
> +	  by rebooting the headset on resume from system suspend and at shutdown.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called hid-valve-index.
> +
>  config HID_VIEWSONIC
>  	tristate "ViewSonic/Signotec"
>  	help
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 48a863b245eed..21f512cab6250 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -156,6 +156,7 @@ obj-$(CONFIG_HID_XIAOMI)	+= hid-xiaomi.o
>  obj-$(CONFIG_HID_XINMO)		+= hid-xinmo.o
>  obj-$(CONFIG_HID_ZEROPLUS)	+= hid-zpff.o
>  obj-$(CONFIG_HID_ZYDACRON)	+= hid-zydacron.o
> +obj-$(CONFIG_HID_VALVE_INDEX)	+= hid-valve-index.o
>  obj-$(CONFIG_HID_VIEWSONIC)	+= hid-viewsonic.o
>  obj-$(CONFIG_HID_VRC2)		+= hid-vrc2.o
>  obj-$(CONFIG_HID_HUAWEI)	+= hid-huawei.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b3aca5aa91767..15cd29a338a8d 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -1391,6 +1391,7 @@
>  #define USB_DEVICE_ID_STEAM_CONTROLLER_IBEX_BLE	0x1303
>  #define USB_DEVICE_ID_STEAM_CONTROLLER_PROTEUS	0x1304
>  #define USB_DEVICE_ID_STEAM_CONTROLLER_NEREID	0x1305
> +#define USB_DEVICE_ID_VALVE_INDEX_HEADSET	0x2300
>  
>  #define USB_VENDOR_ID_STEELSERIES	0x1038
>  #define USB_DEVICE_ID_STEELSERIES_SRWS1	0x1410
> diff --git a/drivers/hid/hid-valve-index.c b/drivers/hid/hid-valve-index.c
> new file mode 100644
> index 0000000000000..43c1142b7215b
> --- /dev/null
> +++ b/drivers/hid/hid-valve-index.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * HID driver for the Valve Index headset
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +#define VALVE_INDEX_REBOOT_REPORT_ID	0x16
> +#define VALVE_INDEX_REBOOT_CMD		0x01
> +#define VALVE_INDEX_REPORT_SIZE		64
> +
> +static bool valve_index_has_reboot_report(struct hid_device *hdev)
> +{
> +	struct hid_report *report;
> +
> +	/*
> +	 * The reboot command is a vendor protocol carried in the unnumbered
> +	 * 64-byte output report of the headset's third interface; the first
> +	 * data byte is the command id.  Report 0x16 is only declared as a
> +	 * feature report and is not what the command is sent as.
> +	 */
> +	report = hdev->report_enum[HID_OUTPUT_REPORT].report_id_hash[0];
> +
> +	return report && hid_report_len(report) == VALVE_INDEX_REPORT_SIZE;
> +}
> +
> +static void valve_index_reboot(struct hid_device *hdev, bool wake)
> +{
> +	u8 *report;
> +	int ret;
> +
> +	if (!valve_index_has_reboot_report(hdev))
> +		return;
> +
> +	/* USB transfer buffers must be DMA-able, so not on the stack. */
> +	report = kzalloc(VALVE_INDEX_REPORT_SIZE, GFP_KERNEL);
> +	if (!report)
> +		return;
> +	report[0] = VALVE_INDEX_REBOOT_REPORT_ID;
> +	report[1] = VALVE_INDEX_REBOOT_CMD;
> +
> +	if (wake) {
> +		ret = hid_hw_power(hdev, PM_HINT_FULLON);
> +		if (ret < 0) {
> +			hid_warn(hdev, "failed to resume headset for reboot: %d\n",
> +				 ret);
> +			goto out;
> +		}
> +	}
> +
> +	/* Use the same interrupt-out then SET_REPORT fallback as hidraw. */
> +	ret = hid_hw_output_report(hdev, report, VALVE_INDEX_REPORT_SIZE);
> +	if (ret == -ENOSYS)
> +		ret = hid_hw_raw_request(hdev, report[0], report,
> +					 VALVE_INDEX_REPORT_SIZE,
> +					 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> +	if (ret < 0)
> +		hid_warn(hdev, "failed to reboot headset: %d\n", ret);
> +	else if (ret != VALVE_INDEX_REPORT_SIZE)
> +		hid_warn(hdev, "short headset reboot report: %d\n", ret);
> +
> +	if (wake)
> +		hid_hw_power(hdev, PM_HINT_NORMAL);
> +out:
> +	kfree(report);
> +}
> +
> +/*
> + * The suspend and shutdown hooks only cover orderly power transitions.  After
> + * a crash, a hard reset or a power cut the headset is left in the state where
> + * its EDID no longer reads, and nothing recovers it until the next orderly
> + * transition.  Expose the reboot command as a write-only "reboot" attribute
> + * on the HID device so userspace can recover it, for instance from a udev
> + * rule that fires only when the connector reports no EDID.  Writing to an
> + * interface that does not carry the reboot report returns -ENODEV.
> + */
> +static ssize_t reboot_store(struct device *dev, struct device_attribute *attr,
> +			    const char *buf, size_t count)
> +{
> +	struct hid_device *hdev = to_hid_device(dev);
> +	bool val;
> +
> +	if (kstrtobool(buf, &val))
> +		return -EINVAL;
> +	if (!val)
> +		return count;
> +	if (!valve_index_has_reboot_report(hdev))
> +		return -ENODEV;
> +
> +	valve_index_reboot(hdev, true);
> +
> +	return count;
> +}
> +static DEVICE_ATTR_WO(reboot);
> +
> +static struct attribute *valve_index_attrs[] = {
> +	&dev_attr_reboot.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(valve_index);
> +
> +/*
> + * The headset's EDID service is lost when the host disables the DisplayPort
> + * PHY during system suspend, so it needs the reboot on the way out of
> + * suspend.  Doing it on the way in does not work: the headset dropping off
> + * USB is a remote-wakeup event from its hub and aborts the suspend.
> + */

The internal hub which will be quirked by the next patch, or its parent?

> +static int valve_index_resume(struct hid_device *hdev)
> +{
> +	valve_index_reboot(hdev, false);
> +
> +	return 0;
> +}
> +
> +static void valve_index_shutdown(struct hid_device *hdev)
> +{
> +	valve_index_reboot(hdev, true);
> +}
> +
> +static const struct hid_device_id valve_index_devices[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
> +			 USB_DEVICE_ID_VALVE_INDEX_HEADSET) },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(hid, valve_index_devices);
> +
> +static struct hid_driver valve_index_driver = {
> +	.name = "valve-index",
> +	.id_table = valve_index_devices,
> +	.resume = valve_index_resume,
> +	.reset_resume = valve_index_resume,
> +	.shutdown = valve_index_shutdown,
> +	.driver.dev_groups = valve_index_groups,
> +};
> +module_hid_driver(valve_index_driver);
> +
> +MODULE_AUTHOR("Mario Limonciello <mario.limonciello@amd.com>");
> +MODULE_DESCRIPTION("HID driver for Valve Index headset");
> +MODULE_LICENSE("GPL");
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-10 20:04   ` Michal Pecio
@ 2026-09-10 20:43     ` Mario Limonciello
  2026-09-10 20:52       ` Michal Pecio
  2026-09-11  4:54       ` Curtis Vogt
  0 siblings, 2 replies; 17+ messages in thread
From: Mario Limonciello @ 2026-09-10 20:43 UTC (permalink / raw)
  To: Michal Pecio, Curtis Vogt
  Cc: Jiri Kosina, Benjamin Tissoires, Greg Kroah-Hartman,
	Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM



On 9/10/26 15:04, Michal Pecio wrote:
> On Thu, 10 Sep 2026 12:02:53 -0500, Mario Limonciello wrote:
>> The Valve Index HMD stops serving its EDID after the host disables the
>> DisplayPort PHY.  The headset remains powered by its breakout box across
>> suspend and shutdown, so the bad state survives and the next connector
>> detection reports "No EDID read".  The HMD then appears as a synthesized
>> 640x480 display until it is power-cycled.
>>
>> The 64-byte HID output report 0x16 with command 0x01 reboots the headset
>> and restores its EDID service.  Add a device-specific driver which sends
>> this report for system sleep transitions and orderly shutdown while leaving
>> runtime autosuspend alone.
>>
>> Resume a runtime-suspended interface for a shutdown request and restrict
>> the command to the composite interface which declares report 0x16.
>>
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/4333
> 
> Hmm, people say it's a regression, so it looks like at least one
> alternative solution should, in theory, exist...

Right.  This bug sat for a very long time hoping someone with the 
hardware would bisect and we could explain what changed.

My initial suspicion is timing.  But scouring the web you can see it 
happens on NVIDIA hardware too.

https://forums.developer.nvidia.com/t/valve-index-initialized-in-unusable-state-on-boot/324710

So 'unlikely' that a DRM change caused it.  Maybe tied to the F/W 
version on the Index and it got updated from initial report to failure?

I have no idea.  I don't have this hardware so I'm just trying to help 
these people how I can :)

> 
> Obligatory question: does it work any better with Windows? :)

Curtis?

> 
>> Link: https://github.com/ValveSoftware/SteamVR-for-Linux/issues/939
>> Assisted-by: LLM
>> Co-developed-by: Curtis Vogt <curtis.vogt@gmail.com>
>> Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   .../ABI/testing/sysfs-driver-hid-valve-index  |  12 ++
>>   drivers/hid/Kconfig                           |  11 ++
>>   drivers/hid/Makefile                          |   1 +
>>   drivers/hid/hid-ids.h                         |   1 +
>>   drivers/hid/hid-valve-index.c                 | 142 ++++++++++++++++++
>>   5 files changed, 167 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-valve-index
>>   create mode 100644 drivers/hid/hid-valve-index.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-hid-valve-index b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
>> new file mode 100644
>> index 0000000000000..47d8c26b1eace
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
>> @@ -0,0 +1,12 @@
>> +What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/reboot
>> +Date:		October 2026
>> +Contact:	linux-input@vger.kernel.org
>> +Description:
>> +		Writing a boolean true value reboots the Valve Index headset to
>> +		recover its EDID service. Writing a boolean false value has no
>> +		effect. This file is write-only.
>> +
>> +		The Valve Index is a composite HID device. The reboot command is
>> +		only supported by the interface that provides the headset's 64-byte
>> +		output report. Writing true to this file on another interface fails
>> +		with -ENODEV.
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index a81bf51cbcf10..8ea2dd570058f 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -547,6 +547,17 @@ config HID_WALTOP
>>   	help
>>   	Support for Waltop tablets.
>>   
>> +config HID_VALVE_INDEX
>> +	tristate "Valve Index headset"
>> +	depends on USB_HID
>> +	help
>> +	  Support for the Valve Index headset. This driver works around the
>> +	  headset failing to provide its EDID after a DisplayPort link shutdown
>> +	  by rebooting the headset on resume from system suspend and at shutdown.
>> +
>> +	  To compile this driver as a module, choose M here: the module will be
>> +	  called hid-valve-index.
>> +
>>   config HID_VIEWSONIC
>>   	tristate "ViewSonic/Signotec"
>>   	help
>> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
>> index 48a863b245eed..21f512cab6250 100644
>> --- a/drivers/hid/Makefile
>> +++ b/drivers/hid/Makefile
>> @@ -156,6 +156,7 @@ obj-$(CONFIG_HID_XIAOMI)	+= hid-xiaomi.o
>>   obj-$(CONFIG_HID_XINMO)		+= hid-xinmo.o
>>   obj-$(CONFIG_HID_ZEROPLUS)	+= hid-zpff.o
>>   obj-$(CONFIG_HID_ZYDACRON)	+= hid-zydacron.o
>> +obj-$(CONFIG_HID_VALVE_INDEX)	+= hid-valve-index.o
>>   obj-$(CONFIG_HID_VIEWSONIC)	+= hid-viewsonic.o
>>   obj-$(CONFIG_HID_VRC2)		+= hid-vrc2.o
>>   obj-$(CONFIG_HID_HUAWEI)	+= hid-huawei.o
>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> index b3aca5aa91767..15cd29a338a8d 100644
>> --- a/drivers/hid/hid-ids.h
>> +++ b/drivers/hid/hid-ids.h
>> @@ -1391,6 +1391,7 @@
>>   #define USB_DEVICE_ID_STEAM_CONTROLLER_IBEX_BLE	0x1303
>>   #define USB_DEVICE_ID_STEAM_CONTROLLER_PROTEUS	0x1304
>>   #define USB_DEVICE_ID_STEAM_CONTROLLER_NEREID	0x1305
>> +#define USB_DEVICE_ID_VALVE_INDEX_HEADSET	0x2300
>>   
>>   #define USB_VENDOR_ID_STEELSERIES	0x1038
>>   #define USB_DEVICE_ID_STEELSERIES_SRWS1	0x1410
>> diff --git a/drivers/hid/hid-valve-index.c b/drivers/hid/hid-valve-index.c
>> new file mode 100644
>> index 0000000000000..43c1142b7215b
>> --- /dev/null
>> +++ b/drivers/hid/hid-valve-index.c
>> @@ -0,0 +1,142 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * HID driver for the Valve Index headset
>> + */
>> +
>> +#include <linux/hid.h>
>> +#include <linux/module.h>
>> +
>> +#include "hid-ids.h"
>> +
>> +#define VALVE_INDEX_REBOOT_REPORT_ID	0x16
>> +#define VALVE_INDEX_REBOOT_CMD		0x01
>> +#define VALVE_INDEX_REPORT_SIZE		64
>> +
>> +static bool valve_index_has_reboot_report(struct hid_device *hdev)
>> +{
>> +	struct hid_report *report;
>> +
>> +	/*
>> +	 * The reboot command is a vendor protocol carried in the unnumbered
>> +	 * 64-byte output report of the headset's third interface; the first
>> +	 * data byte is the command id.  Report 0x16 is only declared as a
>> +	 * feature report and is not what the command is sent as.
>> +	 */
>> +	report = hdev->report_enum[HID_OUTPUT_REPORT].report_id_hash[0];
>> +
>> +	return report && hid_report_len(report) == VALVE_INDEX_REPORT_SIZE;
>> +}
>> +
>> +static void valve_index_reboot(struct hid_device *hdev, bool wake)
>> +{
>> +	u8 *report;
>> +	int ret;
>> +
>> +	if (!valve_index_has_reboot_report(hdev))
>> +		return;
>> +
>> +	/* USB transfer buffers must be DMA-able, so not on the stack. */
>> +	report = kzalloc(VALVE_INDEX_REPORT_SIZE, GFP_KERNEL);
>> +	if (!report)
>> +		return;
>> +	report[0] = VALVE_INDEX_REBOOT_REPORT_ID;
>> +	report[1] = VALVE_INDEX_REBOOT_CMD;
>> +
>> +	if (wake) {
>> +		ret = hid_hw_power(hdev, PM_HINT_FULLON);
>> +		if (ret < 0) {
>> +			hid_warn(hdev, "failed to resume headset for reboot: %d\n",
>> +				 ret);
>> +			goto out;
>> +		}
>> +	}
>> +
>> +	/* Use the same interrupt-out then SET_REPORT fallback as hidraw. */
>> +	ret = hid_hw_output_report(hdev, report, VALVE_INDEX_REPORT_SIZE);
>> +	if (ret == -ENOSYS)
>> +		ret = hid_hw_raw_request(hdev, report[0], report,
>> +					 VALVE_INDEX_REPORT_SIZE,
>> +					 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
>> +	if (ret < 0)
>> +		hid_warn(hdev, "failed to reboot headset: %d\n", ret);
>> +	else if (ret != VALVE_INDEX_REPORT_SIZE)
>> +		hid_warn(hdev, "short headset reboot report: %d\n", ret);
>> +
>> +	if (wake)
>> +		hid_hw_power(hdev, PM_HINT_NORMAL);
>> +out:
>> +	kfree(report);
>> +}
>> +
>> +/*
>> + * The suspend and shutdown hooks only cover orderly power transitions.  After
>> + * a crash, a hard reset or a power cut the headset is left in the state where
>> + * its EDID no longer reads, and nothing recovers it until the next orderly
>> + * transition.  Expose the reboot command as a write-only "reboot" attribute
>> + * on the HID device so userspace can recover it, for instance from a udev
>> + * rule that fires only when the connector reports no EDID.  Writing to an
>> + * interface that does not carry the reboot report returns -ENODEV.
>> + */
>> +static ssize_t reboot_store(struct device *dev, struct device_attribute *attr,
>> +			    const char *buf, size_t count)
>> +{
>> +	struct hid_device *hdev = to_hid_device(dev);
>> +	bool val;
>> +
>> +	if (kstrtobool(buf, &val))
>> +		return -EINVAL;
>> +	if (!val)
>> +		return count;
>> +	if (!valve_index_has_reboot_report(hdev))
>> +		return -ENODEV;
>> +
>> +	valve_index_reboot(hdev, true);
>> +
>> +	return count;
>> +}
>> +static DEVICE_ATTR_WO(reboot);
>> +
>> +static struct attribute *valve_index_attrs[] = {
>> +	&dev_attr_reboot.attr,
>> +	NULL
>> +};
>> +ATTRIBUTE_GROUPS(valve_index);
>> +
>> +/*
>> + * The headset's EDID service is lost when the host disables the DisplayPort
>> + * PHY during system suspend, so it needs the reboot on the way out of
>> + * suspend.  Doing it on the way in does not work: the headset dropping off
>> + * USB is a remote-wakeup event from its hub and aborts the suspend.
>> + */
> 
> The internal hub which will be quirked by the next patch, or its parent?

It has to be the internal hub if quirking it works, no?

I guess it's easy to check this by looking up wakeup count from all the 
applicable devices in sysfs while toggling the sysfs file introduced by 
this patch?

> 
>> +static int valve_index_resume(struct hid_device *hdev)
>> +{
>> +	valve_index_reboot(hdev, false);
>> +
>> +	return 0;
>> +}
>> +
>> +static void valve_index_shutdown(struct hid_device *hdev)
>> +{
>> +	valve_index_reboot(hdev, true);
>> +}
>> +
>> +static const struct hid_device_id valve_index_devices[] = {
>> +	{ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
>> +			 USB_DEVICE_ID_VALVE_INDEX_HEADSET) },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(hid, valve_index_devices);
>> +
>> +static struct hid_driver valve_index_driver = {
>> +	.name = "valve-index",
>> +	.id_table = valve_index_devices,
>> +	.resume = valve_index_resume,
>> +	.reset_resume = valve_index_resume,
>> +	.shutdown = valve_index_shutdown,
>> +	.driver.dev_groups = valve_index_groups,
>> +};
>> +module_hid_driver(valve_index_driver);
>> +
>> +MODULE_AUTHOR("Mario Limonciello <mario.limonciello@amd.com>");
>> +MODULE_DESCRIPTION("HID driver for Valve Index headset");
>> +MODULE_LICENSE("GPL");
>> -- 
>> 2.43.0
>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-10 20:43     ` Mario Limonciello
@ 2026-09-10 20:52       ` Michal Pecio
  2026-09-10 20:58         ` Mario Limonciello
  2026-09-11  4:54       ` Curtis Vogt
  1 sibling, 1 reply; 17+ messages in thread
From: Michal Pecio @ 2026-09-10 20:52 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Curtis Vogt, Jiri Kosina, Benjamin Tissoires, Greg Kroah-Hartman,
	Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM

On Thu, 10 Sep 2026 15:43:18 -0500, Mario Limonciello wrote:
> >> +/*
> >> + * The headset's EDID service is lost when the host disables the DisplayPort
> >> + * PHY during system suspend, so it needs the reboot on the way out of
> >> + * suspend.  Doing it on the way in does not work: the headset dropping off
> >> + * USB is a remote-wakeup event from its hub and aborts the suspend.
> >> + */  
> > 
> > The internal hub which will be quirked by the next patch, or its parent?  
> 
> It has to be the internal hub if quirking it works, no?

I believe there are two separate problems here:

1. resetting the device at suspend causes instant wakeup
2. a few seconds later the system wakes up anyway

1. is solved by resetting on resume rather than suspend
2. is solved by the quirk

Questions:

Any chance that 2 also solves 1?
Would resetting on suspend be preferable, as the comment suggests?
Maybe it would, if the reset can race with DP seeing empty EDID? 

Regards,
Michal

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-10 20:52       ` Michal Pecio
@ 2026-09-10 20:58         ` Mario Limonciello
  2026-09-12 14:07           ` Curtis Vogt
  0 siblings, 1 reply; 17+ messages in thread
From: Mario Limonciello @ 2026-09-10 20:58 UTC (permalink / raw)
  To: Michal Pecio
  Cc: Curtis Vogt, Jiri Kosina, Benjamin Tissoires, Greg Kroah-Hartman,
	Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM



On 9/10/26 15:52, Michal Pecio wrote:
> On Thu, 10 Sep 2026 15:43:18 -0500, Mario Limonciello wrote:
>>>> +/*
>>>> + * The headset's EDID service is lost when the host disables the DisplayPort
>>>> + * PHY during system suspend, so it needs the reboot on the way out of
>>>> + * suspend.  Doing it on the way in does not work: the headset dropping off
>>>> + * USB is a remote-wakeup event from its hub and aborts the suspend.
>>>> + */
>>>
>>> The internal hub which will be quirked by the next patch, or its parent?
>>
>> It has to be the internal hub if quirking it works, no?
> 
> I believe there are two separate problems here:
> 
> 1. resetting the device at suspend causes instant wakeup
> 2. a few seconds later the system wakes up anyway
> 
> 1. is solved by resetting on resume rather than suspend
> 2. is solved by the quirk
> 
> Questions:
> 
> Any chance that 2 also solves 1?
> Would resetting on suspend be preferable, as the comment suggests?
> Maybe it would, if the reset can race with DP seeing empty EDID?
> 
I do think that resetting on suspend makes a lot more sense for that 
exact reason.  That's why my original PoC did it that way.

That's a very good idea to see if the quirk + moving it back to suspend 
works.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-10 20:43     ` Mario Limonciello
  2026-09-10 20:52       ` Michal Pecio
@ 2026-09-11  4:54       ` Curtis Vogt
  2026-09-11  9:08         ` Michal Pecio
  1 sibling, 1 reply; 17+ messages in thread
From: Curtis Vogt @ 2026-09-11  4:54 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Michal Pecio, Jiri Kosina, Benjamin Tissoires,
	Greg Kroah-Hartman, Pierre-Loup Griffais, open list,
	open list:HID CORE LAYER, open list:USB SUBSYSTEM

On Thu, Sep 10, 2026 at 03:43:18PM -0500, Mario Limonciello wrote:
> 
> 
> On 9/10/26 15:04, Michal Pecio wrote:
> > On Thu, 10 Sep 2026 12:02:53 -0500, Mario Limonciello wrote:
> > > The Valve Index HMD stops serving its EDID after the host disables the
> > > DisplayPort PHY.  The headset remains powered by its breakout box across
> > > suspend and shutdown, so the bad state survives and the next connector
> > > detection reports "No EDID read".  The HMD then appears as a synthesized
> > > 640x480 display until it is power-cycled.
> > > 
> > > The 64-byte HID output report 0x16 with command 0x01 reboots the headset
> > > and restores its EDID service.  Add a device-specific driver which sends
> > > this report for system sleep transitions and orderly shutdown while leaving
> > > runtime autosuspend alone.
> > > 
> > > Resume a runtime-suspended interface for a shutdown request and restrict
> > > the command to the composite interface which declares report 0x16.
> > > 
> > > Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/4333
> > 
> > Hmm, people say it's a regression, so it looks like at least one
> > alternative solution should, in theory, exist...
> 
> Right.  This bug sat for a very long time hoping someone with the hardware
> would bisect and we could explain what changed.
> 
> My initial suspicion is timing.  But scouring the web you can see it happens
> on NVIDIA hardware too.
> 
> https://forums.developer.nvidia.com/t/valve-index-initialized-in-unusable-state-on-boot/324710
> 
> So 'unlikely' that a DRM change caused it.  Maybe tied to the F/W version on
> the Index and it got updated from initial report to failure?
> 
> I have no idea.  I don't have this hardware so I'm just trying to help these
> people how I can :)
> 
> > 
> > Obligatory question: does it work any better with Windows? :)
> 
> Curtis?
>

When I was using Windows I believe I observed a similiar behavior of the
system booting into a black screen and hanging. I remember this
occurring as far back as 2020 when I was using a 5700XT (RDNA1).

I'm willing to test on Windows again but I'll need to setup a Windows
system since I don't currently have one available.

> > 
> > > Link: https://github.com/ValveSoftware/SteamVR-for-Linux/issues/939
> > > Assisted-by: LLM
> > > Co-developed-by: Curtis Vogt <curtis.vogt@gmail.com>
> > > Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > ---
> > >   .../ABI/testing/sysfs-driver-hid-valve-index  |  12 ++
> > >   drivers/hid/Kconfig                           |  11 ++
> > >   drivers/hid/Makefile                          |   1 +
> > >   drivers/hid/hid-ids.h                         |   1 +
> > >   drivers/hid/hid-valve-index.c                 | 142 ++++++++++++++++++
> > >   5 files changed, 167 insertions(+)
> > >   create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-valve-index
> > >   create mode 100644 drivers/hid/hid-valve-index.c
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-driver-hid-valve-index b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
> > > new file mode 100644
> > > index 0000000000000..47d8c26b1eace
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
> > > @@ -0,0 +1,12 @@
> > > +What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/reboot
> > > +Date:		October 2026
> > > +Contact:	linux-input@vger.kernel.org
> > > +Description:
> > > +		Writing a boolean true value reboots the Valve Index headset to
> > > +		recover its EDID service. Writing a boolean false value has no
> > > +		effect. This file is write-only.
> > > +
> > > +		The Valve Index is a composite HID device. The reboot command is
> > > +		only supported by the interface that provides the headset's 64-byte
> > > +		output report. Writing true to this file on another interface fails
> > > +		with -ENODEV.
> > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > > index a81bf51cbcf10..8ea2dd570058f 100644
> > > --- a/drivers/hid/Kconfig
> > > +++ b/drivers/hid/Kconfig
> > > @@ -547,6 +547,17 @@ config HID_WALTOP
> > >   	help
> > >   	Support for Waltop tablets.
> > > +config HID_VALVE_INDEX
> > > +	tristate "Valve Index headset"
> > > +	depends on USB_HID
> > > +	help
> > > +	  Support for the Valve Index headset. This driver works around the
> > > +	  headset failing to provide its EDID after a DisplayPort link shutdown
> > > +	  by rebooting the headset on resume from system suspend and at shutdown.
> > > +
> > > +	  To compile this driver as a module, choose M here: the module will be
> > > +	  called hid-valve-index.
> > > +
> > >   config HID_VIEWSONIC
> > >   	tristate "ViewSonic/Signotec"
> > >   	help
> > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > > index 48a863b245eed..21f512cab6250 100644
> > > --- a/drivers/hid/Makefile
> > > +++ b/drivers/hid/Makefile
> > > @@ -156,6 +156,7 @@ obj-$(CONFIG_HID_XIAOMI)	+= hid-xiaomi.o
> > >   obj-$(CONFIG_HID_XINMO)		+= hid-xinmo.o
> > >   obj-$(CONFIG_HID_ZEROPLUS)	+= hid-zpff.o
> > >   obj-$(CONFIG_HID_ZYDACRON)	+= hid-zydacron.o
> > > +obj-$(CONFIG_HID_VALVE_INDEX)	+= hid-valve-index.o
> > >   obj-$(CONFIG_HID_VIEWSONIC)	+= hid-viewsonic.o
> > >   obj-$(CONFIG_HID_VRC2)		+= hid-vrc2.o
> > >   obj-$(CONFIG_HID_HUAWEI)	+= hid-huawei.o
> > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > index b3aca5aa91767..15cd29a338a8d 100644
> > > --- a/drivers/hid/hid-ids.h
> > > +++ b/drivers/hid/hid-ids.h
> > > @@ -1391,6 +1391,7 @@
> > >   #define USB_DEVICE_ID_STEAM_CONTROLLER_IBEX_BLE	0x1303
> > >   #define USB_DEVICE_ID_STEAM_CONTROLLER_PROTEUS	0x1304
> > >   #define USB_DEVICE_ID_STEAM_CONTROLLER_NEREID	0x1305
> > > +#define USB_DEVICE_ID_VALVE_INDEX_HEADSET	0x2300
> > >   #define USB_VENDOR_ID_STEELSERIES	0x1038
> > >   #define USB_DEVICE_ID_STEELSERIES_SRWS1	0x1410
> > > diff --git a/drivers/hid/hid-valve-index.c b/drivers/hid/hid-valve-index.c
> > > new file mode 100644
> > > index 0000000000000..43c1142b7215b
> > > --- /dev/null
> > > +++ b/drivers/hid/hid-valve-index.c
> > > @@ -0,0 +1,142 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * HID driver for the Valve Index headset
> > > + */
> > > +
> > > +#include <linux/hid.h>
> > > +#include <linux/module.h>
> > > +
> > > +#include "hid-ids.h"
> > > +
> > > +#define VALVE_INDEX_REBOOT_REPORT_ID	0x16
> > > +#define VALVE_INDEX_REBOOT_CMD		0x01
> > > +#define VALVE_INDEX_REPORT_SIZE		64
> > > +
> > > +static bool valve_index_has_reboot_report(struct hid_device *hdev)
> > > +{
> > > +	struct hid_report *report;
> > > +
> > > +	/*
> > > +	 * The reboot command is a vendor protocol carried in the unnumbered
> > > +	 * 64-byte output report of the headset's third interface; the first
> > > +	 * data byte is the command id.  Report 0x16 is only declared as a
> > > +	 * feature report and is not what the command is sent as.
> > > +	 */
> > > +	report = hdev->report_enum[HID_OUTPUT_REPORT].report_id_hash[0];
> > > +
> > > +	return report && hid_report_len(report) == VALVE_INDEX_REPORT_SIZE;
> > > +}
> > > +
> > > +static void valve_index_reboot(struct hid_device *hdev, bool wake)
> > > +{
> > > +	u8 *report;
> > > +	int ret;
> > > +
> > > +	if (!valve_index_has_reboot_report(hdev))
> > > +		return;
> > > +
> > > +	/* USB transfer buffers must be DMA-able, so not on the stack. */
> > > +	report = kzalloc(VALVE_INDEX_REPORT_SIZE, GFP_KERNEL);
> > > +	if (!report)
> > > +		return;
> > > +	report[0] = VALVE_INDEX_REBOOT_REPORT_ID;
> > > +	report[1] = VALVE_INDEX_REBOOT_CMD;
> > > +
> > > +	if (wake) {
> > > +		ret = hid_hw_power(hdev, PM_HINT_FULLON);
> > > +		if (ret < 0) {
> > > +			hid_warn(hdev, "failed to resume headset for reboot: %d\n",
> > > +				 ret);
> > > +			goto out;
> > > +		}
> > > +	}
> > > +
> > > +	/* Use the same interrupt-out then SET_REPORT fallback as hidraw. */
> > > +	ret = hid_hw_output_report(hdev, report, VALVE_INDEX_REPORT_SIZE);
> > > +	if (ret == -ENOSYS)
> > > +		ret = hid_hw_raw_request(hdev, report[0], report,
> > > +					 VALVE_INDEX_REPORT_SIZE,
> > > +					 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> > > +	if (ret < 0)
> > > +		hid_warn(hdev, "failed to reboot headset: %d\n", ret);
> > > +	else if (ret != VALVE_INDEX_REPORT_SIZE)
> > > +		hid_warn(hdev, "short headset reboot report: %d\n", ret);
> > > +
> > > +	if (wake)
> > > +		hid_hw_power(hdev, PM_HINT_NORMAL);
> > > +out:
> > > +	kfree(report);
> > > +}
> > > +
> > > +/*
> > > + * The suspend and shutdown hooks only cover orderly power transitions.  After
> > > + * a crash, a hard reset or a power cut the headset is left in the state where
> > > + * its EDID no longer reads, and nothing recovers it until the next orderly
> > > + * transition.  Expose the reboot command as a write-only "reboot" attribute
> > > + * on the HID device so userspace can recover it, for instance from a udev
> > > + * rule that fires only when the connector reports no EDID.  Writing to an
> > > + * interface that does not carry the reboot report returns -ENODEV.
> > > + */
> > > +static ssize_t reboot_store(struct device *dev, struct device_attribute *attr,
> > > +			    const char *buf, size_t count)
> > > +{
> > > +	struct hid_device *hdev = to_hid_device(dev);
> > > +	bool val;
> > > +
> > > +	if (kstrtobool(buf, &val))
> > > +		return -EINVAL;
> > > +	if (!val)
> > > +		return count;
> > > +	if (!valve_index_has_reboot_report(hdev))
> > > +		return -ENODEV;
> > > +
> > > +	valve_index_reboot(hdev, true);
> > > +
> > > +	return count;
> > > +}
> > > +static DEVICE_ATTR_WO(reboot);
> > > +
> > > +static struct attribute *valve_index_attrs[] = {
> > > +	&dev_attr_reboot.attr,
> > > +	NULL
> > > +};
> > > +ATTRIBUTE_GROUPS(valve_index);
> > > +
> > > +/*
> > > + * The headset's EDID service is lost when the host disables the DisplayPort
> > > + * PHY during system suspend, so it needs the reboot on the way out of
> > > + * suspend.  Doing it on the way in does not work: the headset dropping off
> > > + * USB is a remote-wakeup event from its hub and aborts the suspend.
> > > + */
> > 
> > The internal hub which will be quirked by the next patch, or its parent?
> 
> It has to be the internal hub if quirking it works, no?
> 
> I guess it's easy to check this by looking up wakeup count from all the
> applicable devices in sysfs while toggling the sysfs file introduced by this
> patch?
> 

I can try to look into this.

> > 
> > > +static int valve_index_resume(struct hid_device *hdev)
> > > +{
> > > +	valve_index_reboot(hdev, false);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static void valve_index_shutdown(struct hid_device *hdev)
> > > +{
> > > +	valve_index_reboot(hdev, true);
> > > +}
> > > +
> > > +static const struct hid_device_id valve_index_devices[] = {
> > > +	{ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
> > > +			 USB_DEVICE_ID_VALVE_INDEX_HEADSET) },
> > > +	{ }
> > > +};
> > > +MODULE_DEVICE_TABLE(hid, valve_index_devices);
> > > +
> > > +static struct hid_driver valve_index_driver = {
> > > +	.name = "valve-index",
> > > +	.id_table = valve_index_devices,
> > > +	.resume = valve_index_resume,
> > > +	.reset_resume = valve_index_resume,
> > > +	.shutdown = valve_index_shutdown,
> > > +	.driver.dev_groups = valve_index_groups,
> > > +};
> > > +module_hid_driver(valve_index_driver);
> > > +
> > > +MODULE_AUTHOR("Mario Limonciello <mario.limonciello@amd.com>");
> > > +MODULE_DESCRIPTION("HID driver for Valve Index headset");
> > > +MODULE_LICENSE("GPL");
> > > -- 
> > > 2.43.0
> > > 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-10 17:02 ` [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions Mario Limonciello
  2026-09-10 20:04   ` Michal Pecio
@ 2026-09-11  5:54   ` Greg Kroah-Hartman
  2026-09-11  6:01     ` Mario Limonciello
  2026-09-11  9:16     ` Michal Pecio
  1 sibling, 2 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2026-09-11  5:54 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Jiri Kosina, Benjamin Tissoires, Pierre-Loup Griffais, open list,
	open list:HID CORE LAYER, open list:USB SUBSYSTEM, Curtis Vogt

On Thu, Sep 10, 2026 at 12:02:53PM -0500, Mario Limonciello wrote:
> The Valve Index HMD stops serving its EDID after the host disables the
> DisplayPort PHY.  The headset remains powered by its breakout box across
> suspend and shutdown, so the bad state survives and the next connector
> detection reports "No EDID read".  The HMD then appears as a synthesized
> 640x480 display until it is power-cycled.
> 
> The 64-byte HID output report 0x16 with command 0x01 reboots the headset
> and restores its EDID service.  Add a device-specific driver which sends
> this report for system sleep transitions and orderly shutdown while leaving
> runtime autosuspend alone.
> 
> Resume a runtime-suspended interface for a shutdown request and restrict
> the command to the composite interface which declares report 0x16.
> 
> Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/4333
> Link: https://github.com/ValveSoftware/SteamVR-for-Linux/issues/939
> Assisted-by: LLM
> Co-developed-by: Curtis Vogt <curtis.vogt@gmail.com>
> Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  .../ABI/testing/sysfs-driver-hid-valve-index  |  12 ++
>  drivers/hid/Kconfig                           |  11 ++
>  drivers/hid/Makefile                          |   1 +
>  drivers/hid/hid-ids.h                         |   1 +
>  drivers/hid/hid-valve-index.c                 | 142 ++++++++++++++++++
>  5 files changed, 167 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-valve-index
>  create mode 100644 drivers/hid/hid-valve-index.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-hid-valve-index b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
> new file mode 100644
> index 0000000000000..47d8c26b1eace
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
> @@ -0,0 +1,12 @@
> +What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/reboot
> +Date:		October 2026
> +Contact:	linux-input@vger.kernel.org
> +Description:
> +		Writing a boolean true value reboots the Valve Index headset to
> +		recover its EDID service. Writing a boolean false value has no
> +		effect. This file is write-only.

Shouldn't this just be a debugfs file?  Making it a sysfs file seems
"risky" as it's not a normal operation.

> +		The Valve Index is a composite HID device. The reboot command is
> +		only supported by the interface that provides the headset's 64-byte
> +		output report. Writing true to this file on another interface fails
> +		with -ENODEV.

Why would it be present on "another interface'?  That feels wrong.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-11  5:54   ` Greg Kroah-Hartman
@ 2026-09-11  6:01     ` Mario Limonciello
  2026-09-11  9:16     ` Michal Pecio
  1 sibling, 0 replies; 17+ messages in thread
From: Mario Limonciello @ 2026-09-11  6:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Kosina, Benjamin Tissoires, Pierre-Loup Griffais, open list,
	open list:HID CORE LAYER, open list:USB SUBSYSTEM, Curtis Vogt



On 9/11/26 00:54, Greg Kroah-Hartman wrote:
> On Thu, Sep 10, 2026 at 12:02:53PM -0500, Mario Limonciello wrote:
>> The Valve Index HMD stops serving its EDID after the host disables the
>> DisplayPort PHY.  The headset remains powered by its breakout box across
>> suspend and shutdown, so the bad state survives and the next connector
>> detection reports "No EDID read".  The HMD then appears as a synthesized
>> 640x480 display until it is power-cycled.
>>
>> The 64-byte HID output report 0x16 with command 0x01 reboots the headset
>> and restores its EDID service.  Add a device-specific driver which sends
>> this report for system sleep transitions and orderly shutdown while leaving
>> runtime autosuspend alone.
>>
>> Resume a runtime-suspended interface for a shutdown request and restrict
>> the command to the composite interface which declares report 0x16.
>>
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/4333
>> Link: https://github.com/ValveSoftware/SteamVR-for-Linux/issues/939
>> Assisted-by: LLM
>> Co-developed-by: Curtis Vogt <curtis.vogt@gmail.com>
>> Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   .../ABI/testing/sysfs-driver-hid-valve-index  |  12 ++
>>   drivers/hid/Kconfig                           |  11 ++
>>   drivers/hid/Makefile                          |   1 +
>>   drivers/hid/hid-ids.h                         |   1 +
>>   drivers/hid/hid-valve-index.c                 | 142 ++++++++++++++++++
>>   5 files changed, 167 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-valve-index
>>   create mode 100644 drivers/hid/hid-valve-index.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-hid-valve-index b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
>> new file mode 100644
>> index 0000000000000..47d8c26b1eace
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-hid-valve-index
>> @@ -0,0 +1,12 @@
>> +What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/reboot
>> +Date:		October 2026
>> +Contact:	linux-input@vger.kernel.org
>> +Description:
>> +		Writing a boolean true value reboots the Valve Index headset to
>> +		recover its EDID service. Writing a boolean false value has no
>> +		effect. This file is write-only.
> 
> Shouldn't this just be a debugfs file?  Making it a sysfs file seems
> "risky" as it's not a normal operation.

Yeah I guess that makes sense.

> 
>> +		The Valve Index is a composite HID device. The reboot command is
>> +		only supported by the interface that provides the headset's 64-byte
>> +		output report. Writing true to this file on another interface fails
>> +		with -ENODEV.
> 
> Why would it be present on "another interface'?  That feels wrong.
> 
> thanks,
> 
> greg k-h


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-11  4:54       ` Curtis Vogt
@ 2026-09-11  9:08         ` Michal Pecio
  0 siblings, 0 replies; 17+ messages in thread
From: Michal Pecio @ 2026-09-11  9:08 UTC (permalink / raw)
  To: Curtis Vogt
  Cc: Mario Limonciello, Jiri Kosina, Benjamin Tissoires,
	Greg Kroah-Hartman, Pierre-Loup Griffais, open list,
	open list:HID CORE LAYER, open list:USB SUBSYSTEM

On Thu, 10 Sep 2026 23:54:22 -0500, Curtis Vogt wrote:
> When I was using Windows I believe I observed a similiar behavior of
> the system booting into a black screen and hanging. I remember this
> occurring as far back as 2020 when I was using a 5700XT (RDNA1).
> 
> I'm willing to test on Windows again but I'll need to setup a Windows
> system since I don't currently have one available.

It's not extremely important, I frankly asked this question because
I didn't expect problems on Windows. I assumed it would be either:

1. Obviously, Windows resets this device every time when XYZ, therefore
   Linux can just reset it the same, problem solved.
2. Windows doesn't reset anything and doesn't have any issues, clearly
   Linux is broken, maybe somebody will finally bother to bisect it.

IDK if anything useful can be learned from seeing that Windows fails.

Regards,
Michal

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-11  5:54   ` Greg Kroah-Hartman
  2026-09-11  6:01     ` Mario Limonciello
@ 2026-09-11  9:16     ` Michal Pecio
  2026-09-11  9:23       ` Greg Kroah-Hartman
  1 sibling, 1 reply; 17+ messages in thread
From: Michal Pecio @ 2026-09-11  9:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Mario Limonciello, Jiri Kosina, Benjamin Tissoires,
	Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM, Curtis Vogt

On Fri, 11 Sep 2026 07:54:12 +0200, Greg Kroah-Hartman wrote:
> > +What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/reboot
> > +Date:		October 2026
> > +Contact:	linux-input@vger.kernel.org
> > +Description:
> > +		Writing a boolean true value reboots the Valve Index headset to
> > +		recover its EDID service. Writing a boolean false value has no
> > +		effect. This file is write-only.  
> 
> Shouldn't this just be a debugfs file?  Making it a sysfs file seems
> "risky" as it's not a normal operation.

Not sure what's "risky" about it, it's hopefully a root-only thing?

And it's the only supported way for userspace to recover buggy devices
from certain failure condition, it's been suggested that this could be
run by udev scripts. Isn't it more "risky" to ask userspace to mount
and tinker with debugfs as a matter of routine?

Regards,
Michal

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-11  9:16     ` Michal Pecio
@ 2026-09-11  9:23       ` Greg Kroah-Hartman
  2026-09-11 15:12         ` Mario Limonciello
  0 siblings, 1 reply; 17+ messages in thread
From: Greg Kroah-Hartman @ 2026-09-11  9:23 UTC (permalink / raw)
  To: Michal Pecio
  Cc: Mario Limonciello, Jiri Kosina, Benjamin Tissoires,
	Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM, Curtis Vogt

On Fri, Sep 11, 2026 at 11:16:52AM +0200, Michal Pecio wrote:
> On Fri, 11 Sep 2026 07:54:12 +0200, Greg Kroah-Hartman wrote:
> > > +What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/reboot
> > > +Date:		October 2026
> > > +Contact:	linux-input@vger.kernel.org
> > > +Description:
> > > +		Writing a boolean true value reboots the Valve Index headset to
> > > +		recover its EDID service. Writing a boolean false value has no
> > > +		effect. This file is write-only.  
> > 
> > Shouldn't this just be a debugfs file?  Making it a sysfs file seems
> > "risky" as it's not a normal operation.
> 
> Not sure what's "risky" about it, it's hopefully a root-only thing?
> 
> And it's the only supported way for userspace to recover buggy devices
> from certain failure condition, it's been suggested that this could be
> run by udev scripts. Isn't it more "risky" to ask userspace to mount
> and tinker with debugfs as a matter of routine?

You are creating an "odd" user/kernel api that is used for debugging,
that's not what sysfs is for.  sysfs is to show attributes that a device
has NOT to cause the device to go off and do some random thing (yes,
there are exceptions, but generally that's the rule.)

debugfs is "do whatever you want", so that's a better place for this.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-11  9:23       ` Greg Kroah-Hartman
@ 2026-09-11 15:12         ` Mario Limonciello
  2026-09-11 18:20           ` Michal Pecio
  0 siblings, 1 reply; 17+ messages in thread
From: Mario Limonciello @ 2026-09-11 15:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Michal Pecio
  Cc: Jiri Kosina, Benjamin Tissoires, Pierre-Loup Griffais, open list,
	open list:HID CORE LAYER, open list:USB SUBSYSTEM, Curtis Vogt



On 9/11/26 04:23, Greg Kroah-Hartman wrote:
> On Fri, Sep 11, 2026 at 11:16:52AM +0200, Michal Pecio wrote:
>> On Fri, 11 Sep 2026 07:54:12 +0200, Greg Kroah-Hartman wrote:
>>>> +What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/reboot
>>>> +Date:		October 2026
>>>> +Contact:	linux-input@vger.kernel.org
>>>> +Description:
>>>> +		Writing a boolean true value reboots the Valve Index headset to
>>>> +		recover its EDID service. Writing a boolean false value has no
>>>> +		effect. This file is write-only.
>>>
>>> Shouldn't this just be a debugfs file?  Making it a sysfs file seems
>>> "risky" as it's not a normal operation.
>>
>> Not sure what's "risky" about it, it's hopefully a root-only thing?
>>
>> And it's the only supported way for userspace to recover buggy devices
>> from certain failure condition, it's been suggested that this could be
>> run by udev scripts. Isn't it more "risky" to ask userspace to mount
>> and tinker with debugfs as a matter of routine?
> 
> You are creating an "odd" user/kernel api that is used for debugging,
> that's not what sysfs is for.  sysfs is to show attributes that a device
> has NOT to cause the device to go off and do some random thing (yes,
> there are exceptions, but generally that's the rule.)
> 
> debugfs is "do whatever you want", so that's a better place for this.
> 
> thanks,
> 
> greg k-h

I suppose an alternative path we can build to all of this reset handling 
at power state transitions and sysfs/debugfs is a callback system for a 
failed EDID read.

Something like DRM drivers can send a notify to a global queue when an 
EDID read failed.  Other drivers could subscribe to that notify and 
react.  I guess this would only work if the USB reset sequence to the 
HMD looks like an HPD event to DRM though.

Also it wouldn't be perfect.  If the HMD was connected and the read 
failed on an unrelated monitor on the system the HMD would reset needlessly.

Eh, I think I'm talking myself back into this approach the patch does 
now.  I have no qualms moving the on demand knob to debugfs.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-11 15:12         ` Mario Limonciello
@ 2026-09-11 18:20           ` Michal Pecio
  0 siblings, 0 replies; 17+ messages in thread
From: Michal Pecio @ 2026-09-11 18:20 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Greg Kroah-Hartman, Jiri Kosina, Benjamin Tissoires,
	Pierre-Loup Griffais, open list, open list:HID CORE LAYER,
	open list:USB SUBSYSTEM, Curtis Vogt

On Fri, 11 Sep 2026 10:12:34 -0500, Mario Limonciello wrote:
> On 9/11/26 04:23, Greg Kroah-Hartman wrote:
> > On Fri, Sep 11, 2026 at 11:16:52AM +0200, Michal Pecio wrote:  
> >> On Fri, 11 Sep 2026 07:54:12 +0200, Greg Kroah-Hartman wrote:
> > You are creating an "odd" user/kernel api that is used for
> > debugging, that's not what sysfs is for.  sysfs is to show
> > attributes that a device has NOT to cause the device to go off and
> > do some random thing (yes, there are exceptions, but generally
> > that's the rule.)
> > 
> > debugfs is "do whatever you want", so that's a better place for
> > this.
> > 
> > thanks,
> > 
> > greg k-h  
> 
> I suppose an alternative path we can build to all of this reset
> handling at power state transitions and sysfs/debugfs is a callback
> system for a failed EDID read.
> 
> Something like DRM drivers can send a notify to a global queue when
> an EDID read failed.  Other drivers could subscribe to that notify
> and react.  I guess this would only work if the USB reset sequence to
> the HMD looks like an HPD event to DRM though.
> 
> Also it wouldn't be perfect.  If the HMD was connected and the read 
> failed on an unrelated monitor on the system the HMD would reset
> needlessly.
> 
> Eh, I think I'm talking myself back into this approach the patch does 
> now.  I have no qualms moving the on demand knob to debugfs.

Perhaps one more option is to put this in userspace, with some libusb
or HIDRAW hacking. And if it could be scripted to run on shutdown, no
kernel driver even required at all.

Regards,
Michal

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
  2026-09-10 20:58         ` Mario Limonciello
@ 2026-09-12 14:07           ` Curtis Vogt
  0 siblings, 0 replies; 17+ messages in thread
From: Curtis Vogt @ 2026-09-12 14:07 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Michal Pecio, Jiri Kosina, Benjamin Tissoires,
	Greg Kroah-Hartman, Pierre-Loup Griffais, open list,
	open list:HID CORE LAYER, open list:USB SUBSYSTEM

On Thu, Sep 10, 2026 at 03:58:27PM -0500, Mario Limonciello wrote:
> 
> 
> On 9/10/26 15:52, Michal Pecio wrote:
> > On Thu, 10 Sep 2026 15:43:18 -0500, Mario Limonciello wrote:
> > > > > +/*
> > > > > + * The headset's EDID service is lost when the host disables the DisplayPort
> > > > > + * PHY during system suspend, so it needs the reboot on the way out of
> > > > > + * suspend.  Doing it on the way in does not work: the headset dropping off
> > > > > + * USB is a remote-wakeup event from its hub and aborts the suspend.
> > > > > + */
> > > > 
> > > > The internal hub which will be quirked by the next patch, or its parent?
> > > 
> > > It has to be the internal hub if quirking it works, no?

The parent needs to be quirked: the breakout box's own hub, 28de:2613,
sits above Microchip USB2744 (0424:2744). Only the 28de:2613 hub ever
registered wakeup events, and quirking it alone is enough.

I also tested the quirk using a stock kernel (7.2.3) using the kernel
param `usbcore.quirks=28de:2613:j` which stopped the headset from waking
the host. Without that param I found that the headset would wake the
host when healthy but not when wedged.

> > I believe there are two separate problems here:
> > 
> > 1. resetting the device at suspend causes instant wakeup
> > 2. a few seconds later the system wakes up anyway
> > 
> > 1. is solved by resetting on resume rather than suspend
> > 2. is solved by the quirk
> > 
> > Questions:
> > 
> > Any chance that 2 also solves 1?

It does. With the quirk from 3/3 and the driver rebooting from the suspend
hook instead of resume the headset reboots during suspend entry and the
host stays asleep.

> > Would resetting on suspend be preferable, as the comment suggests?
> > Maybe it would, if the reset can race with DP seeing empty EDID?

Yes, and the log bears out the ordering concern. With the reboot on
suspend the headset is already back and so the EDID reads cleanly the
first time. With the headset reboot on resume a re-enumeration occurs.
There was no "EDID err" upon resume with either variant. We can go back to
Mario's implementation of headset reboot on suspend.

> I do think that resetting on suspend makes a lot more sense for that exact
> reason.  That's why my original PoC did it that way.
> 
> That's a very good idea to see if the quirk + moving it back to suspend
> works.

I've validated this approach works. Happy to return to it.

Diff against 2/3 moving the headset reboot back to the suspend hook:

---8<---
 drivers/hid/hid-valve-index.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-valve-index.c b/drivers/hid/hid-valve-index.c
index 43c1142b7215..f6941dd04910 100644
--- a/drivers/hid/hid-valve-index.c
+++ b/drivers/hid/hid-valve-index.c
@@ -103,14 +103,18 @@ static struct attribute *valve_index_attrs[] = {
 ATTRIBUTE_GROUPS(valve_index);
 
 /*
- * The headset's EDID service is lost when the host disables the DisplayPort
- * PHY during system suspend, so it needs the reboot on the way out of
- * suspend.  Doing it on the way in does not work: the headset dropping off
- * USB is a remote-wakeup event from its hub and aborts the suspend.
+ * Disabling the DisplayPort PHY during system suspend leaves the headset
+ * unable to serve its EDID until it is rebooted, so send the reboot from
+ * the suspend hook.  The headset drops off USB about a second later; that
+ * does not abort the suspend because its breakout box hub is quirked to
+ * not be a wakeup source.  On resume the hub finds the rebooted headset on
+ * the same port and resets it in place, and the connector detection reads
+ * a fresh EDID.  Runtime autosuspend is left alone.
  */
-static int valve_index_resume(struct hid_device *hdev)
+static int valve_index_suspend(struct hid_device *hdev, pm_message_t message)
 {
-	valve_index_reboot(hdev, false);
+	if (!PMSG_IS_AUTO(message))
+		valve_index_reboot(hdev, false);
 
 	return 0;
 }
@@ -130,8 +134,7 @@ MODULE_DEVICE_TABLE(hid, valve_index_devices);
 static struct hid_driver valve_index_driver = {
 	.name = "valve-index",
 	.id_table = valve_index_devices,
-	.resume = valve_index_resume,
-	.reset_resume = valve_index_resume,
+	.suspend = valve_index_suspend,
 	.shutdown = valve_index_shutdown,
 	.driver.dev_groups = valve_index_groups,
 };

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-09-12 14:07 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-10 17:02 [PATCH 0/3] Add 'hid-valve-index' reset driver Mario Limonciello
2026-09-10 17:02 ` [PATCH 1/3] HID: Add shutdown callback for device drivers Mario Limonciello
2026-09-10 17:02 ` [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions Mario Limonciello
2026-09-10 20:04   ` Michal Pecio
2026-09-10 20:43     ` Mario Limonciello
2026-09-10 20:52       ` Michal Pecio
2026-09-10 20:58         ` Mario Limonciello
2026-09-12 14:07           ` Curtis Vogt
2026-09-11  4:54       ` Curtis Vogt
2026-09-11  9:08         ` Michal Pecio
2026-09-11  5:54   ` Greg Kroah-Hartman
2026-09-11  6:01     ` Mario Limonciello
2026-09-11  9:16     ` Michal Pecio
2026-09-11  9:23       ` Greg Kroah-Hartman
2026-09-11 15:12         ` Mario Limonciello
2026-09-11 18:20           ` Michal Pecio
2026-09-10 17:02 ` [PATCH 3/3] USB: quirks: Ignore remote wakeup from the Valve Index breakout box hub Mario Limonciello

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®