Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions

[Mario Limonciello <mario.limonciello@amd.com>]

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
>>