linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] xpad, hid-sony: Report analog buttons
@ 2023-08-26 15:21 Max Staudt
  2023-08-26 15:21 ` [PATCH 1/2] xpad: XTYPE_XBOX: " Max Staudt
  2023-08-26 15:21 ` [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis Max Staudt
  0 siblings, 2 replies; 8+ messages in thread
From: Max Staudt @ 2023-08-26 15:21 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires
  Cc: Vicki Pfau, Pavel Rojtberg, Roderick Colenbrander, linux-input,
	linux-kernel, Max Staudt

Dear input maintainers,

I would like to add support for pressure sensitive buttons on the
original Xbox gamepad, as well as the PlayStation 3 controllers.


In an attempt to maximise backwards compatibility, the attached patches
add the corresponding analog values for BTN_A..BTN_Z as ABS_MISC+0..+5,
L1/R1 as ABS_HAT1Y/HAT1X, and the D-Pad as ABS_HAT0X/Y.

All of these axes have higher indices than any axes previously exposed,
so gamepad mappings in SDL keep working. Also, where possible, I have
stuck to the Linux gamepad specification (for HAT0/HAT1).


Now, I am wondering what best to do with the action buttons, since the
Linux gamepad specification does not foresee them being analog. In the
patches, they are reported as ABS_MISC+0..+5 - do you think this is
reasonable, or would a new ABS_* range at 0x40.. be better suited to
this task?


I'd appreciate your thoughts on the patches and on how to best add
analog buttons to the drivers!


Thanks,

Max


Patches included:
  [PATCH 1/2] xpad: XTYPE_XBOX: Report analog buttons
  [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis



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

* [PATCH 1/2] xpad: XTYPE_XBOX: Report analog buttons
  2023-08-26 15:21 [PATCH 0/2] xpad, hid-sony: Report analog buttons Max Staudt
@ 2023-08-26 15:21 ` Max Staudt
  2023-08-26 16:52   ` Rahul Rameshbabu
  2023-08-26 15:21 ` [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis Max Staudt
  1 sibling, 1 reply; 8+ messages in thread
From: Max Staudt @ 2023-08-26 15:21 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires
  Cc: Vicki Pfau, Pavel Rojtberg, Roderick Colenbrander, linux-input,
	linux-kernel, Max Staudt

The original Xbox controllers (XTYPE_XBOX) report 8 buttons in an analog
fashion, in addition to the digital on/off state:

 - Action buttons A/B/X/Y/black/white
 - Triggers L/R

Up until now, only the triggers L/R 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 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.

 - BTN_A..BTN_Z mapped to ABS_MISC+0..ABS_MISC+5, 0 to 255

Signed-off-by: Max Staudt <max@enpas.org>
---
 drivers/input/joystick/xpad.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index cdb193317c3b..609c06f795de 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -420,6 +420,14 @@ static const signed short xpad_abs_triggers[] = {
 	-1
 };
 
+/* used for analog face buttons mapped to axes */
+static const signed short xpad_abs_analog_face_buttons[] = {
+	ABS_MISC + 0, ABS_MISC + 1, /* A, B */
+	ABS_MISC + 3, ABS_MISC + 4, /* X, Y */
+	ABS_MISC + 2, ABS_MISC + 5, /* C, Z */
+	-1
+};
+
 /* used when the controller has extra paddle buttons */
 static const signed short xpad_btn_paddles[] = {
 	BTN_TRIGGER_HAPPY5, BTN_TRIGGER_HAPPY6, /* paddle upper right, lower right */
@@ -784,6 +792,15 @@ static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *d
 	input_report_key(dev, BTN_C, data[8]);
 	input_report_key(dev, BTN_Z, data[9]);
 
+	/* analog buttons A, B, X, Y as axes */
+	input_report_abs(dev, xpad_abs_analog_face_buttons[0], data[4]); /* A */
+	input_report_abs(dev, xpad_abs_analog_face_buttons[1], data[5]); /* B */
+	input_report_abs(dev, xpad_abs_analog_face_buttons[2], data[6]); /* X */
+	input_report_abs(dev, xpad_abs_analog_face_buttons[3], data[7]); /* Y */
+
+	/* analog buttons black, white (C, Z) as axes */
+	input_report_abs(dev, xpad_abs_analog_face_buttons[4], data[8]); /* C */
+	input_report_abs(dev, xpad_abs_analog_face_buttons[5], data[9]); /* Z */
 
 	input_sync(dev);
 }
@@ -1827,6 +1844,14 @@ static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
 	case ABS_HAT0Y:	/* the d-pad (only if dpad is mapped to axes */
 		input_set_abs_params(input_dev, abs, -1, 1, 0, 0);
 		break;
+	case ABS_MISC + 0:
+	case ABS_MISC + 1:
+	case ABS_MISC + 2:
+	case ABS_MISC + 3:
+	case ABS_MISC + 4:
+	case ABS_MISC + 5:
+		input_set_abs_params(input_dev, abs, 0, 255, 0, 0);
+		break;
 	case ABS_PROFILE: /* 4 value profile button (such as on XAC) */
 		input_set_abs_params(input_dev, abs, 0, 4, 0, 0);
 		break;
@@ -1928,6 +1953,10 @@ static int xpad_init_input(struct usb_xpad *xpad)
 			xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
 	}
 
+	if (xpad->xtype == XTYPE_XBOX)
+		for (i = 0; xpad_abs_analog_face_buttons[i] >= 0; i++)
+			xpad_set_up_abs(input_dev, xpad_abs_analog_face_buttons[i]);
+
 	/* setup profile button as an axis with 4 possible values */
 	if (xpad->mapping & MAP_PROFILE_BUTTON)
 		xpad_set_up_abs(input_dev, ABS_PROFILE);
-- 
2.39.2


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

* [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis
  2023-08-26 15:21 [PATCH 0/2] xpad, hid-sony: Report analog buttons Max Staudt
  2023-08-26 15:21 ` [PATCH 1/2] xpad: XTYPE_XBOX: " Max Staudt
@ 2023-08-26 15:21 ` Max Staudt
  2023-08-29 15:53   ` Max Staudt
  1 sibling, 1 reply; 8+ messages in thread
From: Max Staudt @ 2023-08-26 15:21 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires
  Cc: Vicki Pfau, Pavel Rojtberg, Roderick Colenbrander, linux-input,
	linux-kernel, Max Staudt

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

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


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

* Re: [PATCH 1/2] xpad: XTYPE_XBOX: Report analog buttons
  2023-08-26 15:21 ` [PATCH 1/2] xpad: XTYPE_XBOX: " Max Staudt
@ 2023-08-26 16:52   ` Rahul Rameshbabu
  2023-08-27 15:07     ` Max Staudt
  0 siblings, 1 reply; 8+ messages in thread
From: Rahul Rameshbabu @ 2023-08-26 16:52 UTC (permalink / raw)
  To: Max Staudt
  Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, Vicki Pfau,
	Pavel Rojtberg, Roderick Colenbrander, linux-input, linux-kernel

Hi,

You will want to update the commit message subject to use the prefix
"Input: xpad -" instead of "xpad:".

  https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/drivers/input/joystick/xpad.c

On Sun, 27 Aug, 2023 00:21:10 +0900 "Max Staudt" <max@enpas.org> wrote:
> The original Xbox controllers (XTYPE_XBOX) report 8 buttons in an analog
> fashion, in addition to the digital on/off state:
>
>  - Action buttons A/B/X/Y/black/white
>  - Triggers L/R
>
> Up until now, only the triggers L/R 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 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.

FWIW, I like the way you handled adding support for the range of the
analog buttons.

>
>  - BTN_A..BTN_Z mapped to ABS_MISC+0..ABS_MISC+5, 0 to 255
>
> Signed-off-by: Max Staudt <max@enpas.org>
> ---
>  drivers/input/joystick/xpad.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index cdb193317c3b..609c06f795de 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -420,6 +420,14 @@ static const signed short xpad_abs_triggers[] = {
>  	-1
>  };
>
> +/* used for analog face buttons mapped to axes */
> +static const signed short xpad_abs_analog_face_buttons[] = {
> +	ABS_MISC + 0, ABS_MISC + 1, /* A, B */
> +	ABS_MISC + 3, ABS_MISC + 4, /* X, Y */
> +	ABS_MISC + 2, ABS_MISC + 5, /* C, Z */
> +	-1
> +};

Would it make more sense to use an enum for this?
Something like the below enum.

  enum xpad_abs_analog_face_btn {
       XPAD_ABS_ANALOG_FACE_BTN_A = ABS_MISC,
       XPAD_ABS_ANALOG_FACE_BTN_B,
       XPAD_ABS_ANALOG_FACE_BTN_C,
       XPAD_ABS_ANALOG_FACE_BTN_X,
       XPAD_ABS_ANALOG_FACE_BTN_Y,
       XPAD_ABS_ANALOG_FACE_BTN_Z,
       XPAD_ABS_ANALOG_FACE_BTN_END, /* Must remain as the last element */
  };

This would clean up both xpad_process_packet and xpad_set_up_abs a bit
in my opinion. Your loop for xpad_set_up_abs would look like the
following.

  enum xpad_abs_analog_face_btn btn;

  ...

  for (btn = XPAD_ABS_ANALOG_FACE_BTN_A; btn != XPAD_ABS_ANALOG_FACE_BTN_END; ++btn)
          xpad_set_up_abs(input_dev, btn);

> +
>  /* used when the controller has extra paddle buttons */
>  static const signed short xpad_btn_paddles[] = {
>  	BTN_TRIGGER_HAPPY5, BTN_TRIGGER_HAPPY6, /* paddle upper right, lower right */
> @@ -784,6 +792,15 @@ static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *d
>  	input_report_key(dev, BTN_C, data[8]);
>  	input_report_key(dev, BTN_Z, data[9]);
>
> +	/* analog buttons A, B, X, Y as axes */
> +	input_report_abs(dev, xpad_abs_analog_face_buttons[0], data[4]); /* A */
> +	input_report_abs(dev, xpad_abs_analog_face_buttons[1], data[5]); /* B */
> +	input_report_abs(dev, xpad_abs_analog_face_buttons[2], data[6]); /* X */
> +	input_report_abs(dev, xpad_abs_analog_face_buttons[3], data[7]); /* Y */
> +
> +	/* analog buttons black, white (C, Z) as axes */
> +	input_report_abs(dev, xpad_abs_analog_face_buttons[4], data[8]); /* C */
> +	input_report_abs(dev, xpad_abs_analog_face_buttons[5], data[9]); /* Z */
>
>  	input_sync(dev);
>  }
> @@ -1827,6 +1844,14 @@ static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
>  	case ABS_HAT0Y:	/* the d-pad (only if dpad is mapped to axes */
>  		input_set_abs_params(input_dev, abs, -1, 1, 0, 0);
>  		break;
> +	case ABS_MISC + 0:
> +	case ABS_MISC + 1:
> +	case ABS_MISC + 2:
> +	case ABS_MISC + 3:
> +	case ABS_MISC + 4:
> +	case ABS_MISC + 5:
> +		input_set_abs_params(input_dev, abs, 0, 255, 0, 0);
> +		break;
>  	case ABS_PROFILE: /* 4 value profile button (such as on XAC) */
>  		input_set_abs_params(input_dev, abs, 0, 4, 0, 0);
>  		break;
> @@ -1928,6 +1953,10 @@ static int xpad_init_input(struct usb_xpad *xpad)
>  			xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
>  	}
>
> +	if (xpad->xtype == XTYPE_XBOX)
> +		for (i = 0; xpad_abs_analog_face_buttons[i] >= 0; i++)
> +			xpad_set_up_abs(input_dev, xpad_abs_analog_face_buttons[i]);
> +
>  	/* setup profile button as an axis with 4 possible values */
>  	if (xpad->mapping & MAP_PROFILE_BUTTON)
>  		xpad_set_up_abs(input_dev, ABS_PROFILE);

--
Thanks,

Rahul Rameshbabu


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

* Re: [PATCH 1/2] xpad: XTYPE_XBOX: Report analog buttons
  2023-08-26 16:52   ` Rahul Rameshbabu
@ 2023-08-27 15:07     ` Max Staudt
  0 siblings, 0 replies; 8+ messages in thread
From: Max Staudt @ 2023-08-27 15:07 UTC (permalink / raw)
  To: Rahul Rameshbabu
  Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, Vicki Pfau,
	Pavel Rojtberg, Roderick Colenbrander, linux-input, linux-kernel

On 8/27/23 01:52, Rahul Rameshbabu wrote:
> You will want to update the commit message subject to use the prefix
> "Input: xpad -" instead of "xpad:".

Thanks, will do!


>> +/* used for analog face buttons mapped to axes */
>> +static const signed short xpad_abs_analog_face_buttons[] = {
>> +	ABS_MISC + 0, ABS_MISC + 1, /* A, B */
>> +	ABS_MISC + 3, ABS_MISC + 4, /* X, Y */
>> +	ABS_MISC + 2, ABS_MISC + 5, /* C, Z */
>> +	-1
>> +};
> 
> Would it make more sense to use an enum for this?
> Something like the below enum.
> 
>    enum xpad_abs_analog_face_btn {
>         XPAD_ABS_ANALOG_FACE_BTN_A = ABS_MISC,
>         XPAD_ABS_ANALOG_FACE_BTN_B,
>         XPAD_ABS_ANALOG_FACE_BTN_C,
>         XPAD_ABS_ANALOG_FACE_BTN_X,
>         XPAD_ABS_ANALOG_FACE_BTN_Y,
>         XPAD_ABS_ANALOG_FACE_BTN_Z,
>         XPAD_ABS_ANALOG_FACE_BTN_END, /* Must remain as the last element */
>    };
> 
> This would clean up both xpad_process_packet and xpad_set_up_abs a bit
> in my opinion. Your loop for xpad_set_up_abs would look like the
> following.
> 
>    enum xpad_abs_analog_face_btn btn;
> 
>    ...
> 
>    for (btn = XPAD_ABS_ANALOG_FACE_BTN_A; btn != XPAD_ABS_ANALOG_FACE_BTN_END; ++btn)
>            xpad_set_up_abs(input_dev, btn);

I agree, that looks cleaner.

Since it's a step closer to standardising a mapping for those analog buttons, I'd like to wait and see whether there is a consensus across drivers and maintainers. Maybe we can include something like this enum in input-event-codes.h and have a really clean solution.



Thanks!

Max


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

* Re: [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis
  2023-08-26 15:21 ` [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis Max Staudt
@ 2023-08-29 15:53   ` Max Staudt
  2023-09-06 15:45     ` Roderick Colenbrander
  0 siblings, 1 reply; 8+ messages in thread
From: Max Staudt @ 2023-08-29 15:53 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires
  Cc: Vicki Pfau, Pavel Rojtberg, Roderick Colenbrander, linux-input,
	linux-kernel

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


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

* Re: [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis
  2023-08-29 15:53   ` Max Staudt
@ 2023-09-06 15:45     ` Roderick Colenbrander
  2023-09-11 16:51       ` Max Staudt
  0 siblings, 1 reply; 8+ messages in thread
From: Roderick Colenbrander @ 2023-09-06 15:45 UTC (permalink / raw)
  To: Max Staudt
  Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, Vicki Pfau,
	Pavel Rojtberg, Roderick Colenbrander, linux-input, linux-kernel

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

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

* Re: [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis
  2023-09-06 15:45     ` Roderick Colenbrander
@ 2023-09-11 16:51       ` Max Staudt
  0 siblings, 0 replies; 8+ messages in thread
From: Max Staudt @ 2023-09-11 16:51 UTC (permalink / raw)
  To: Roderick Colenbrander
  Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, Vicki Pfau,
	Pavel Rojtberg, Roderick Colenbrander, linux-input, linux-kernel

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


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

end of thread, other threads:[~2023-09-11 21:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-26 15:21 [PATCH 0/2] xpad, hid-sony: Report analog buttons Max Staudt
2023-08-26 15:21 ` [PATCH 1/2] xpad: XTYPE_XBOX: " Max Staudt
2023-08-26 16:52   ` Rahul Rameshbabu
2023-08-27 15:07     ` Max Staudt
2023-08-26 15:21 ` [PATCH 2/2] hid-sony: DS3: Report analog buttons for Sixaxis Max Staudt
2023-08-29 15:53   ` Max Staudt
2023-09-06 15:45     ` Roderick Colenbrander
2023-09-11 16:51       ` Max Staudt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).