mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Curtis Vogt <curtis.vogt@gmail.com>
To: Mario Limonciello <mario.limonciello@amd.com>
Cc: Michal Pecio <michal.pecio@gmail.com>,
	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>
Subject: Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions
Date: Thu, 10 Sep 2026 23:54:22 -0500	[thread overview]
Message-ID: <aqOJfj79_NVkU6Db@dhoja> (raw)
In-Reply-To: <55918a70-72c9-4f55-934b-81a692d4619b@amd.com>

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

  parent reply	other threads:[~2026-09-11  4:54 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
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 [this message]
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=aqOJfj79_NVkU6Db@dhoja \
    --to=curtis.vogt@gmail.com \
    --cc=bentiss@kernel.org \
    --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=michal.pecio@gmail.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®