mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Mario Limonciello <mario.limonciello@amd.com>
Cc: Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <bentiss@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Pierre-Loup Griffais <pgriffais@valvesoftware.com>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:HID CORE LAYER" <linux-input@vger.kernel.org>,
	"open list:USB SUBSYSTEM" <linux-usb@vger.kernel.org>,
	Curtis Vogt <curtis.vogt@gmail.com>
Subject: Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
Date: Thu, 10 Sep 2026 22:04:21 +0200	[thread overview]
Message-ID: <20260910220421.40356e51.michal.pecio@gmail.com> (raw)
In-Reply-To: <20260910170254.833871-3-mario.limonciello@amd.com>

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
> 

  reply	other threads:[~2026-09-10 20:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260910220421.40356e51.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=bentiss@kernel.org \
    --cc=curtis.vogt@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=pgriffais@valvesoftware.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®