diff mbox series

[2/2] hid-sony: DS3: Report analog buttons for Sixaxis

Message ID 20230826152111.13525-3-max@enpas.org (mailing list archive)
State New
Delegated to: Jiri Kosina
Headers show
Series xpad, hid-sony: Report analog buttons | expand

Commit Message

Max Staudt Aug. 26, 2023, 3:21 p.m. UTC
The Sixaxis and DualShock 3 controllers report 12 buttons in an analog
fashion, in addition to the digital on/off state:

 - D-Pad up/down/left/right
 - Action buttons Triangle/Circle/Cross/Square
 - L1/R1
 - Triggers L2/R2

Up until now, only the triggers L2/R2 are reported as values 0-255. The
other pressure sensitive buttons are reported as digital buttons, as
found on other controllers.

This change exposes these buttons as axes in a way that is as backwards
compatible and as close to the Linux gamepad spec as possible.
The new axes are merely added, and numbered after any existing axes.
This way, libraries like SDL which renumber axes in enumeration order,
can keep their button/axis mapping as-is. Userspace can keep working as
before, and can optionally use the new values when handling this type of
gamepad.

 - The D-Pad as ABS_HAT0X/ABS_HAT0Y, -255 to 255
 - R1 as ABS_HAT1X, 0 to 255
 - L1 as ABS_HAT1Y, 0 to 255
 - BTN_A..BTN_Z mapped to ABS_MISC+0..ABS_MISC+5, 0 to 255

Most buttons are straight HID remappings in sixaxis_mapping().
For the D-Pad, two pairs of buttons need to be merged to a single axis
each, so this is handled manually in sixaxis_parse_report().

Signed-off-by: Max Staudt <max@enpas.org>
---
 drivers/hid/hid-sony.c | 66 ++++++++++++++++++++++++++++++++----------
 1 file changed, 50 insertions(+), 16 deletions(-)

Comments

Max Staudt Aug. 29, 2023, 3:53 p.m. UTC | #1
On 8/27/23 00:21, Max Staudt wrote:
> This change exposes these buttons as axes in a way that is as backwards
> compatible and as close to the Linux gamepad spec as possible.
> 
> [...]
> 
>   - The D-Pad as ABS_HAT0X/ABS_HAT0Y, -255 to 255


One further idea:

The DualShock 3 reports all 4 D-pad buttons separately, and hid-sony currently reports them as discrete digital buttons to userspace.


Would it be better to do the same with the analog buttons, i.e. to report the 4 measurements as discrete axes, rather than the current patch's approach of merging them into two logical axes?

Of course, this would require 4 more axes, this would not fit into any existing scheme, and since we've run out of ABS_MISC+n at this point, this could be a further reason for officially reserving a range of axes for analog buttons. Something like:


#define ABS_BTN_SOUTH		0x40
#define ABS_BTN_A		ABS_BTN_SOUTH
#define ABS_BTN_EAST		0x41
#define ABS_BTN_B		ABS_BTN_EAST
#define ABS_BTN_C		0x42
#define ABS_BTN_NORTH		0x43
#define ABS_BTN_X		ABS_BTN_NORTH
#define ABS_BTN_WEST		0x44
#define ABS_BTN_Y		ABS_BTN_WEST
#define ABS_BTN_Z		0x45

#define ABS_BTN_DPAD_UP		0x46
#define ABS_BTN_DPAD_DOWN	0x47
#define ABS_BTN_DPAD_LEFT	0x48
#define ABS_BTN_DPAD_RIGHT	0x49

#define ABS_MAX			0x4f
#define ABS_CNT			(ABS_MAX+1)



Max
Roderick Colenbrander Sept. 6, 2023, 3:45 p.m. UTC | #2
On Tue, Aug 29, 2023 at 12:12 PM Max Staudt <max@enpas.org> wrote:
>
> On 8/27/23 00:21, Max Staudt wrote:
> > This change exposes these buttons as axes in a way that is as backwards
> > compatible and as close to the Linux gamepad spec as possible.
> >
> > [...]
> >
> >   - The D-Pad as ABS_HAT0X/ABS_HAT0Y, -255 to 255
>
>
> One further idea:
>
> The DualShock 3 reports all 4 D-pad buttons separately, and hid-sony currently reports them as discrete digital buttons to userspace.
>
>
> Would it be better to do the same with the analog buttons, i.e. to report the 4 measurements as discrete axes, rather than the current patch's approach of merging them into two logical axes?
>
> Of course, this would require 4 more axes, this would not fit into any existing scheme, and since we've run out of ABS_MISC+n at this point, this could be a further reason for officially reserving a range of axes for analog buttons. Something like:
>
>
> #define ABS_BTN_SOUTH           0x40
> #define ABS_BTN_A               ABS_BTN_SOUTH
> #define ABS_BTN_EAST            0x41
> #define ABS_BTN_B               ABS_BTN_EAST
> #define ABS_BTN_C               0x42
> #define ABS_BTN_NORTH           0x43
> #define ABS_BTN_X               ABS_BTN_NORTH
> #define ABS_BTN_WEST            0x44
> #define ABS_BTN_Y               ABS_BTN_WEST
> #define ABS_BTN_Z               0x45
>
> #define ABS_BTN_DPAD_UP         0x46
> #define ABS_BTN_DPAD_DOWN       0x47
> #define ABS_BTN_DPAD_LEFT       0x48
> #define ABS_BTN_DPAD_RIGHT      0x49
>
> #define ABS_MAX                 0x4f
> #define ABS_CNT                 (ABS_MAX+1)
>
>
>
> Max
>

Hi Max,

Sorry for the late response, but I have been on vacation and just got back.

Analog buttons are as you know, fairly common on game controllers. For
this reason, I was working on this about 5 years ago as my company had
a need for it, but the need died out. I did send a proposal at the
time to linux-input, which I encourage you to look at ('Proposal to
support pressure sensitive "analog" buttons in evdev' on linux-input).
There are some good comments in there.

The summary of the discussion and also my thoughts is not to simply
reuse existing axes, but think of things in a bigger picture. In
particular I brought the example of analog keyboards into the
discussion at the time (Wooting made one) in which ALL buttons are
analog. The landscape has probably changed and I haven't caught up.
Quickly looking it looks like Razor now has one too and there are
probably more.

The key question is what are the similarities between these analog
devices. It feels it are not 'just' axes. There might be some level of
configurability (though not all of that) for example some keyboards
seem to use it as a way to trigger digital key presses at configurable
thresholds among others. Please look at the old discussion as there
were some good suggestions there if I recall from Peter Hutterer among
others.

Thanks,
Roderick
Max Staudt Sept. 11, 2023, 4:51 p.m. UTC | #3
Hi Roderick,

Thanks for pointing out the 2018 thread!

Since lore.kernel.org doesn't seem to have an archive of it, I hope this one is complete:

   https://www.spinics.net/lists/linux-input/msg57662.html


It's been 5 years since the thread you mentioned. There were many outstanding ideas, and yet as of today, Linux still has no support for pressure sensitive buttons. Hence, maybe we can find a "good enough" solution that covers most or all of what we've seen so far, without future-proofing too much?


My experience with controllers comes from sniffing and emulating wire protocols on older consoles, and from looking at the reports from USB devices such as DualShock 3. I presume you have a wider overview, but maybe we can complement each other here :)


As for keyboards, I think that we could simply report a pressure value in parallel with EV_KEY events - like you originally suggested. Maybe we can bundle the two using EV_SYN, given that I see my keyboard combining EV_MSC and EV_KEY in this manner?


As for gamepads, it seems to me like the world has converged to an Xbox360/PS3 style layout for the face buttons and joysticks. SDL, which is used for a vast array of games, maintains a mapping from raw evdev events onto a virtual Xbox 360 gamepad - gamecontrollerdb.txt - which allows games to consistently map, say, the NORTH button, according to the physical geometry rather than what evdev claims. This is something that unfortunately is not unified in the kernel UAPI, and now userspace needs to have knowledge of all devices.

The original thread mentioned the Gamecube controller. I feel that designs that stray from the Xbox360/PS3 layout, such as the Gamecube's separate digital buttons that are hidden beneath the triggers (beyond the 255 point), have disappeared. Please correct me if I'm too short-sighted, but since I don't see such designs on the horizon, I wouldn't worry about describing them in the kernel. Those buttons can be exported as e.g. L3/R3 (which the GC does not have), and userspace then needs to know about the odd physical geometry anyway. The geometry is really a separate property of the controller and breaks modern games' assumptions (see SDL above), so I'd worry about this on the kernel side only once this really comes up as a kernel problem.


There was the idea of Multitouch like event slots, to allow for future expandability. I like it. But do we really need this? We could always add this as yet another event type, or maybe even in a reserved zone within EV_PSB, but for now, all devices that I've seen report a value in the exact range 0-255, with 0 meaning that the button is released. I also don't remember seeing a controller that flakes in its idle state - it has always been a solid 0 when I released the buttons. A flaking button would currently report as EV_KEY 1 anyway, since e.g. the DS3 treats an analog reading of 1 as digital down, I believe.

Hence, how about simply adding an EV_PSB report in parallel with EV_KEY, and this report works exactly the same, except that the range is not 0-1, but 0-255? This keeps backwards compatibility through EV_KEY, and is easy to handle on all ends.


There was also the idea of an expandable array of PSB properties. In the end, it is still up to userspace to make sense of anything we feed it, and there are things we can add in retrospect, and things we can't add without breaking userspace's assumptions.

For example, which value is interpreted as "down" or "up" is something I'd again leave to userspace, or even the hardware itself - after all, up until now, userspace has happily lived with the kernel's binary EV_KEY. If we really want to, we can always add a non-binding "hint" or "suggestion" later.

The only thing I can see as really important right now is pretty much the same info as in input_absinfo - namely, the minimum/maximum values. If we avoid such a structure, and simply tell userspace that values are always 0-255, then we cannot change this afterwards.

But are there any devices that report PSB in a range that is not 0-255, or at least easily scaled into this range? DS2, DS3, Xbox face buttons are all 0-255. So are all L2/R2 triggers that I've seen. Do you know about controllers or keyboards that don't fit well in this range? If there are none, then we could skip describing minimum/maximum values, and cross our fingers for the future.


As for letting userspace detect which buttons support PSB, we could keep a bitmap like for EV_KEY. Or, we could guarantee (in the kernel driver spec) to always send EV_PSB before EV_KEY, and then userspace can dynamically learn which keys are PSBs. Since EV_PSB comes first, it can then ignore any following EV_KEY for that keycode. This way, we don't need to keep a key bitmap either.


As for high-speed chatter overloading the event pipeline, I'd report only changes to a button's value (just like EV_KEY and EV_ABS), not all buttons' state all the time, like the DS3/4 do in their wire protocols.


To avoid analog keyboards overloading classic event loops, I suggest hiding EV_PSB events until they are enabled via some ioctl() or write().



So many ideas, and I hope we can pare it down to an easy to manage minimum - maybe we can get away without any ioctl(EVIOCGPSR) at all :)



Thanks for your help!

Max
diff mbox series

Patch

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index dd942061fd77..642fd715ba39 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -484,6 +484,7 @@  struct sony_sc {
 	spinlock_t lock;
 	struct list_head list_node;
 	struct hid_device *hdev;
+	struct input_dev *gamepad;
 	struct input_dev *touchpad;
 	struct input_dev *sensor_dev;
 	struct led_classdev *leds[MAX_LEDS];
@@ -711,22 +712,37 @@  static int sixaxis_mapping(struct hid_device *hdev, struct hid_input *hi,
 	} else if (usage->hid == HID_GD_POINTER) {
 		/* The DS3 provides analog values for most buttons and even
 		 * for HAT axes through GD Pointer. L2 and R2 are reported
-		 * among these as well instead of as GD Z / RZ. Remap L2
-		 * and R2 and ignore other analog 'button axes' as there is
-		 * no good way for reporting them.
+		 * among these as well instead of as GD Z / RZ.
 		 */
+		__u16 c;
+
 		switch (usage->usage_index) {
+		case 10: /* L1 */
+			c = ABS_HAT1Y;
+			break;
+		case 11: /* R1 */
+			c = ABS_HAT1X;
+			break;
+		case 12: /* NORTH */
+		case 13: /* EAST */
+		case 14: /* SOUTH */
+		case 15: /* WEST */
+			c = sixaxis_keymap[usage->usage_index + 1]
+				- BTN_GAMEPAD + ABS_MISC;
+			break;
 		case 8: /* L2 */
 			usage->hid = HID_GD_Z;
+			c = ABS_Z;
 			break;
 		case 9: /* R2 */
 			usage->hid = HID_GD_RZ;
+			c = ABS_RZ;
 			break;
 		default:
 			return -1;
 		}
 
-		hid_map_usage_clear(hi, usage, bit, max, EV_ABS, usage->hid & 0xf);
+		hid_map_usage_clear(hi, usage, bit, max, EV_ABS, c);
 		return 1;
 	} else if ((usage->hid & HID_USAGE_PAGE) == HID_UP_GENDESK) {
 		unsigned int abs = usage->hid & HID_USAGE;
@@ -837,6 +853,17 @@  static void sixaxis_parse_report(struct sony_sc *sc, u8 *rd, int size)
 		val = 511 - ((rd[offset+3] << 8) | rd[offset+2]);
 		input_report_abs(sc->sensor_dev, ABS_Z, val);
 
+		/* Report analog D-pad */
+		if (rd[17] > rd[15])  /* left */
+			input_report_abs(sc->gamepad, ABS_HAT0X, 0 - rd[17]);
+		else  /* right */
+			input_report_abs(sc->gamepad, ABS_HAT0X, rd[15]);
+
+		if (rd[14] > rd[16]) /* up */
+			input_report_abs(sc->gamepad, ABS_HAT0Y, 0 - rd[14]);
+		else /* down */
+			input_report_abs(sc->gamepad, ABS_HAT0Y, rd[16]);
+
 		input_sync(sc->sensor_dev);
 	}
 }
@@ -1597,18 +1624,8 @@  static int sony_play_effect(struct input_dev *dev, void *data,
 
 static int sony_init_ff(struct sony_sc *sc)
 {
-	struct hid_input *hidinput;
-	struct input_dev *input_dev;
-
-	if (list_empty(&sc->hdev->inputs)) {
-		hid_err(sc->hdev, "no inputs found\n");
-		return -ENODEV;
-	}
-	hidinput = list_entry(sc->hdev->inputs.next, struct hid_input, list);
-	input_dev = hidinput->input;
-
-	input_set_capability(input_dev, EV_FF, FF_RUMBLE);
-	return input_ff_create_memless(input_dev, NULL, sony_play_effect);
+	input_set_capability(sc->gamepad, EV_FF, FF_RUMBLE);
+	return input_ff_create_memless(sc->gamepad, NULL, sony_play_effect);
 }
 
 #else
@@ -2039,6 +2056,23 @@  static int sony_input_configured(struct hid_device *hdev,
 		}
 	}
 
+	if (sc->quirks & (SONY_FF_SUPPORT | SIXAXIS_CONTROLLER)) {
+		struct hid_input *hidinput;
+
+		if (list_empty(&sc->hdev->inputs)) {
+			hid_err(sc->hdev, "no inputs found\n");
+			return -ENODEV;
+		}
+		hidinput = list_entry(sc->hdev->inputs.next, struct hid_input, list);
+		sc->gamepad = hidinput->input;
+	}
+
+	if (sc->quirks & SIXAXIS_CONTROLLER) {
+		/* Register axes for analog buttons */
+		input_set_abs_params(sc->gamepad, ABS_HAT0X, -255, 255, 0, 0);
+		input_set_abs_params(sc->gamepad, ABS_HAT0Y, -255, 255, 0, 0);
+	}
+
 	if (sc->quirks & SONY_FF_SUPPORT) {
 		ret = sony_init_ff(sc);
 		if (ret < 0)