* [PATCH] next-kbd: convert to use qemu_input_handler_register()
@ 2024-10-30 9:18 Mark Cave-Ayland
2024-10-30 10:28 ` Alex Bennée
2024-11-01 9:26 ` Thomas Huth
0 siblings, 2 replies; 5+ messages in thread
From: Mark Cave-Ayland @ 2024-10-30 9:18 UTC (permalink / raw)
To: peter.maydell, huth, qemu-devel
Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
function to use qemu_input_handler_register().
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/m68k/next-kbd.c | 158 +++++++++++++++++++++++++++++----------------
1 file changed, 103 insertions(+), 55 deletions(-)
diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c
index bc67810f31..85ef784491 100644
--- a/hw/m68k/next-kbd.c
+++ b/hw/m68k/next-kbd.c
@@ -68,7 +68,6 @@ struct NextKBDState {
uint16_t shift;
};
-static void queue_code(void *opaque, int code);
/* lots of magic numbers here */
static uint32_t kbd_read_byte(void *opaque, hwaddr addr)
@@ -166,68 +165,79 @@ static const MemoryRegionOps kbd_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static void nextkbd_event(void *opaque, int ch)
-{
- /*
- * Will want to set vars for caps/num lock
- * if (ch & 0x80) -> key release
- * there's also e0 escaped scancodes that might need to be handled
- */
- queue_code(opaque, ch);
-}
-
-static const unsigned char next_keycodes[128] = {
- 0x00, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x50, 0x4F,
- 0x4E, 0x1E, 0x1F, 0x20, 0x1D, 0x1C, 0x1B, 0x00,
- 0x42, 0x43, 0x44, 0x45, 0x48, 0x47, 0x46, 0x06,
- 0x07, 0x08, 0x00, 0x00, 0x2A, 0x00, 0x39, 0x3A,
- 0x3B, 0x3C, 0x3D, 0x40, 0x3F, 0x3E, 0x2D, 0x2C,
- 0x2B, 0x26, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34,
- 0x35, 0x37, 0x36, 0x2e, 0x2f, 0x30, 0x00, 0x00,
- 0x00, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
+#define NEXTKDB_NO_KEY 0xff
+
+static const int qcode_to_nextkbd_keycode[] = {
+ /* Make sure future additions are automatically set to NEXTKDB_NO_KEY */
+ [0 ... 0xff] = NEXTKDB_NO_KEY,
+
+ [Q_KEY_CODE_ESC] = 0x49,
+ [Q_KEY_CODE_1] = 0x4a,
+ [Q_KEY_CODE_2] = 0x4b,
+ [Q_KEY_CODE_3] = 0x4c,
+ [Q_KEY_CODE_4] = 0x4d,
+ [Q_KEY_CODE_5] = 0x50,
+ [Q_KEY_CODE_6] = 0x4f,
+ [Q_KEY_CODE_7] = 0x4e,
+ [Q_KEY_CODE_8] = 0x1e,
+ [Q_KEY_CODE_9] = 0x1f,
+ [Q_KEY_CODE_0] = 0x20,
+ [Q_KEY_CODE_MINUS] = 0x1d,
+ [Q_KEY_CODE_EQUAL] = 0x1c,
+ [Q_KEY_CODE_BACKSPACE] = 0x1b,
+ [Q_KEY_CODE_TAB] = 0x00,
+
+ [Q_KEY_CODE_Q] = 0x42,
+ [Q_KEY_CODE_W] = 0x43,
+ [Q_KEY_CODE_E] = 0x44,
+ [Q_KEY_CODE_R] = 0x45,
+ [Q_KEY_CODE_T] = 0x48,
+ [Q_KEY_CODE_Y] = 0x47,
+ [Q_KEY_CODE_U] = 0x46,
+ [Q_KEY_CODE_I] = 0x06,
+ [Q_KEY_CODE_O] = 0x07,
+ [Q_KEY_CODE_P] = 0x08,
+ [Q_KEY_CODE_RET] = 0x2a,
+ [Q_KEY_CODE_A] = 0x39,
+ [Q_KEY_CODE_S] = 0x3a,
+
+ [Q_KEY_CODE_D] = 0x3b,
+ [Q_KEY_CODE_F] = 0x3c,
+ [Q_KEY_CODE_G] = 0x3d,
+ [Q_KEY_CODE_H] = 0x40,
+ [Q_KEY_CODE_J] = 0x3f,
+ [Q_KEY_CODE_K] = 0x3e,
+ [Q_KEY_CODE_L] = 0x2d,
+ [Q_KEY_CODE_SEMICOLON] = 0x2c,
+ [Q_KEY_CODE_APOSTROPHE] = 0x2b,
+ [Q_KEY_CODE_GRAVE_ACCENT] = 0x26,
+ [Q_KEY_CODE_SHIFT] = 0x00,
+ [Q_KEY_CODE_BACKSLASH] = 0x00,
+ [Q_KEY_CODE_Z] = 0x31,
+ [Q_KEY_CODE_X] = 0x32,
+ [Q_KEY_CODE_C] = 0x33,
+ [Q_KEY_CODE_V] = 0x34,
+
+ [Q_KEY_CODE_B] = 0x35,
+ [Q_KEY_CODE_N] = 0x37,
+ [Q_KEY_CODE_M] = 0x36,
+ [Q_KEY_CODE_COMMA] = 0x2e,
+ [Q_KEY_CODE_DOT] = 0x2f,
+ [Q_KEY_CODE_SLASH] = 0x30,
+ [Q_KEY_CODE_SHIFT_R] = 0x00,
+
+ [Q_KEY_CODE_SPC] = 0x38,
};
-static void queue_code(void *opaque, int code)
+static void nextkbd_put_keycode(NextKBDState *s, int keycode)
{
- NextKBDState *s = NEXTKBD(opaque);
KBDQueue *q = &s->queue;
- int key = code & KD_KEYMASK;
- int release = code & 0x80;
- static int ext;
-
- if (code == 0xE0) {
- ext = 1;
- }
-
- if (code == 0x2A || code == 0x1D || code == 0x36) {
- if (code == 0x2A) {
- s->shift = KD_LSHIFT;
- } else if (code == 0x36) {
- s->shift = KD_RSHIFT;
- ext = 0;
- } else if (code == 0x1D && !ext) {
- s->shift = KD_LCOMM;
- } else if (code == 0x1D && ext) {
- ext = 0;
- s->shift = KD_RCOMM;
- }
- return;
- } else if (code == (0x2A | 0x80) || code == (0x1D | 0x80) ||
- code == (0x36 | 0x80)) {
- s->shift = 0;
- return;
- }
if (q->count >= KBD_QUEUE_SIZE) {
return;
}
- q->data[q->wptr] = next_keycodes[key] | release;
-
+ q->data[q->wptr] = keycode;
if (++q->wptr == KBD_QUEUE_SIZE) {
q->wptr = 0;
}
@@ -241,6 +251,44 @@ static void queue_code(void *opaque, int code)
/* s->update_irq(s->update_arg, 1); */
}
+static void nextkbd_event(DeviceState *dev, QemuConsole *src, InputEvent *evt)
+{
+ NextKBDState *s = NEXTKBD(dev);
+ int qcode, keycode;
+ bool key_down = evt->u.key.data->down;
+
+ qcode = qemu_input_key_value_to_qcode(evt->u.key.data->key);
+ if (qcode >= ARRAY_SIZE(qcode_to_nextkbd_keycode)) {
+ return;
+ }
+
+ keycode = qcode_to_nextkbd_keycode[qcode];
+ if (keycode == NEXTKDB_NO_KEY) {
+ return;
+ }
+
+ if (qcode == Q_KEY_CODE_SHIFT) {
+ s->shift = key_down ? KD_LSHIFT : 0;
+ }
+
+ if (qcode == Q_KEY_CODE_SHIFT_R) {
+ s->shift = key_down ? KD_RSHIFT : 0;
+ }
+
+ /* If key release event, create keyboard break code */
+ if (!key_down) {
+ keycode = keycode | 0x80;
+ }
+
+ nextkbd_put_keycode(s, keycode);
+}
+
+static const QemuInputHandler nextkbd_handler = {
+ .name = "QEMU NeXT Keyboard",
+ .mask = INPUT_EVENT_MASK_KEY,
+ .event = nextkbd_event,
+};
+
static void nextkbd_reset(DeviceState *dev)
{
NextKBDState *nks = NEXTKBD(dev);
@@ -256,7 +304,7 @@ static void nextkbd_realize(DeviceState *dev, Error **errp)
memory_region_init_io(&s->mr, OBJECT(dev), &kbd_ops, s, "next.kbd", 0x1000);
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mr);
- qemu_add_kbd_event_handler(nextkbd_event, s);
+ qemu_input_handler_register(dev, &nextkbd_handler);
}
static const VMStateDescription nextkbd_vmstate = {
--
2.39.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] next-kbd: convert to use qemu_input_handler_register()
2024-10-30 9:18 [PATCH] next-kbd: convert to use qemu_input_handler_register() Mark Cave-Ayland
@ 2024-10-30 10:28 ` Alex Bennée
2024-10-31 9:33 ` Mark Cave-Ayland
2024-11-01 9:26 ` Thomas Huth
1 sibling, 1 reply; 5+ messages in thread
From: Alex Bennée @ 2024-10-30 10:28 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: peter.maydell, huth, qemu-devel
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
> function to use qemu_input_handler_register().
If that is the last user we should probably remove the function as well.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] next-kbd: convert to use qemu_input_handler_register()
2024-10-30 10:28 ` Alex Bennée
@ 2024-10-31 9:33 ` Mark Cave-Ayland
0 siblings, 0 replies; 5+ messages in thread
From: Mark Cave-Ayland @ 2024-10-31 9:33 UTC (permalink / raw)
To: Alex Bennée; +Cc: peter.maydell, huth, qemu-devel
On 30/10/2024 10:28, Alex Bennée wrote:
> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
>
>> Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
>> function to use qemu_input_handler_register().
>
> If that is the last user we should probably remove the function as well.
True. I can send a follow-up patch to do this later unless anyone beats me to it.
ATB,
Mark.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] next-kbd: convert to use qemu_input_handler_register()
2024-10-30 9:18 [PATCH] next-kbd: convert to use qemu_input_handler_register() Mark Cave-Ayland
2024-10-30 10:28 ` Alex Bennée
@ 2024-11-01 9:26 ` Thomas Huth
2024-11-01 18:59 ` Mark Cave-Ayland
1 sibling, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2024-11-01 9:26 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: peter.maydell, qemu-devel
Am Wed, 30 Oct 2024 09:18:03 +0000
schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
> Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
> function to use qemu_input_handler_register().
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/m68k/next-kbd.c | 158 +++++++++++++++++++++++++++++----------------
> 1 file changed, 103 insertions(+), 55 deletions(-)
>
> diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c
> index bc67810f31..85ef784491 100644
> --- a/hw/m68k/next-kbd.c
> +++ b/hw/m68k/next-kbd.c
> @@ -68,7 +68,6 @@ struct NextKBDState {
> uint16_t shift;
> };
>
> -static void queue_code(void *opaque, int code);
>
> /* lots of magic numbers here */
> static uint32_t kbd_read_byte(void *opaque, hwaddr addr)
> @@ -166,68 +165,79 @@ static const MemoryRegionOps kbd_ops = {
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> -static void nextkbd_event(void *opaque, int ch)
> -{
> - /*
> - * Will want to set vars for caps/num lock
> - * if (ch & 0x80) -> key release
> - * there's also e0 escaped scancodes that might need to be handled
> - */
> - queue_code(opaque, ch);
> -}
> -
> -static const unsigned char next_keycodes[128] = {
> - 0x00, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x50, 0x4F,
> - 0x4E, 0x1E, 0x1F, 0x20, 0x1D, 0x1C, 0x1B, 0x00,
> - 0x42, 0x43, 0x44, 0x45, 0x48, 0x47, 0x46, 0x06,
> - 0x07, 0x08, 0x00, 0x00, 0x2A, 0x00, 0x39, 0x3A,
> - 0x3B, 0x3C, 0x3D, 0x40, 0x3F, 0x3E, 0x2D, 0x2C,
> - 0x2B, 0x26, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34,
> - 0x35, 0x37, 0x36, 0x2e, 0x2f, 0x30, 0x00, 0x00,
> - 0x00, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
> +#define NEXTKDB_NO_KEY 0xff
> +
> +static const int qcode_to_nextkbd_keycode[] = {
> + /* Make sure future additions are automatically set to NEXTKDB_NO_KEY */
> + [0 ... 0xff] = NEXTKDB_NO_KEY,
I think it would be better to use Q_KEY_CODE__MAX here instead of 0xff ?
> + [Q_KEY_CODE_ESC] = 0x49,
> + [Q_KEY_CODE_1] = 0x4a,
> + [Q_KEY_CODE_2] = 0x4b,
> + [Q_KEY_CODE_3] = 0x4c,
> + [Q_KEY_CODE_4] = 0x4d,
> + [Q_KEY_CODE_5] = 0x50,
> + [Q_KEY_CODE_6] = 0x4f,
> + [Q_KEY_CODE_7] = 0x4e,
> + [Q_KEY_CODE_8] = 0x1e,
> + [Q_KEY_CODE_9] = 0x1f,
> + [Q_KEY_CODE_0] = 0x20,
> + [Q_KEY_CODE_MINUS] = 0x1d,
> + [Q_KEY_CODE_EQUAL] = 0x1c,
> + [Q_KEY_CODE_BACKSPACE] = 0x1b,
> + [Q_KEY_CODE_TAB] = 0x00,
> +
> + [Q_KEY_CODE_Q] = 0x42,
> + [Q_KEY_CODE_W] = 0x43,
> + [Q_KEY_CODE_E] = 0x44,
> + [Q_KEY_CODE_R] = 0x45,
> + [Q_KEY_CODE_T] = 0x48,
> + [Q_KEY_CODE_Y] = 0x47,
> + [Q_KEY_CODE_U] = 0x46,
> + [Q_KEY_CODE_I] = 0x06,
> + [Q_KEY_CODE_O] = 0x07,
> + [Q_KEY_CODE_P] = 0x08,
> + [Q_KEY_CODE_RET] = 0x2a,
> + [Q_KEY_CODE_A] = 0x39,
> + [Q_KEY_CODE_S] = 0x3a,
> +
> + [Q_KEY_CODE_D] = 0x3b,
> + [Q_KEY_CODE_F] = 0x3c,
> + [Q_KEY_CODE_G] = 0x3d,
> + [Q_KEY_CODE_H] = 0x40,
> + [Q_KEY_CODE_J] = 0x3f,
> + [Q_KEY_CODE_K] = 0x3e,
> + [Q_KEY_CODE_L] = 0x2d,
> + [Q_KEY_CODE_SEMICOLON] = 0x2c,
> + [Q_KEY_CODE_APOSTROPHE] = 0x2b,
> + [Q_KEY_CODE_GRAVE_ACCENT] = 0x26,
> + [Q_KEY_CODE_SHIFT] = 0x00,
> + [Q_KEY_CODE_BACKSLASH] = 0x00,
> + [Q_KEY_CODE_Z] = 0x31,
> + [Q_KEY_CODE_X] = 0x32,
> + [Q_KEY_CODE_C] = 0x33,
> + [Q_KEY_CODE_V] = 0x34,
> +
> + [Q_KEY_CODE_B] = 0x35,
> + [Q_KEY_CODE_N] = 0x37,
> + [Q_KEY_CODE_M] = 0x36,
> + [Q_KEY_CODE_COMMA] = 0x2e,
> + [Q_KEY_CODE_DOT] = 0x2f,
> + [Q_KEY_CODE_SLASH] = 0x30,
> + [Q_KEY_CODE_SHIFT_R] = 0x00,
> +
> + [Q_KEY_CODE_SPC] = 0x38,
> };
>
> -static void queue_code(void *opaque, int code)
> +static void nextkbd_put_keycode(NextKBDState *s, int keycode)
> {
> - NextKBDState *s = NEXTKBD(opaque);
> KBDQueue *q = &s->queue;
> - int key = code & KD_KEYMASK;
> - int release = code & 0x80;
> - static int ext;
> -
> - if (code == 0xE0) {
> - ext = 1;
> - }
> -
> - if (code == 0x2A || code == 0x1D || code == 0x36) {
> - if (code == 0x2A) {
> - s->shift = KD_LSHIFT;
> - } else if (code == 0x36) {
> - s->shift = KD_RSHIFT;
> - ext = 0;
> - } else if (code == 0x1D && !ext) {
> - s->shift = KD_LCOMM;
> - } else if (code == 0x1D && ext) {
> - ext = 0;
> - s->shift = KD_RCOMM;
> - }
> - return;
> - } else if (code == (0x2A | 0x80) || code == (0x1D | 0x80) ||
> - code == (0x36 | 0x80)) {
> - s->shift = 0;
> - return;
> - }
>
> if (q->count >= KBD_QUEUE_SIZE) {
> return;
> }
>
> - q->data[q->wptr] = next_keycodes[key] | release;
> -
> + q->data[q->wptr] = keycode;
> if (++q->wptr == KBD_QUEUE_SIZE) {
> q->wptr = 0;
> }
> @@ -241,6 +251,44 @@ static void queue_code(void *opaque, int code)
> /* s->update_irq(s->update_arg, 1); */
> }
>
> +static void nextkbd_event(DeviceState *dev, QemuConsole *src, InputEvent *evt)
> +{
> + NextKBDState *s = NEXTKBD(dev);
> + int qcode, keycode;
> + bool key_down = evt->u.key.data->down;
> +
> + qcode = qemu_input_key_value_to_qcode(evt->u.key.data->key);
> + if (qcode >= ARRAY_SIZE(qcode_to_nextkbd_keycode)) {
> + return;
> + }
> +
> + keycode = qcode_to_nextkbd_keycode[qcode];
> + if (keycode == NEXTKDB_NO_KEY) {
> + return;
> + }
> +
> + if (qcode == Q_KEY_CODE_SHIFT) {
> + s->shift = key_down ? KD_LSHIFT : 0;
> + }
> +
> + if (qcode == Q_KEY_CODE_SHIFT_R) {
> + s->shift = key_down ? KD_RSHIFT : 0;
> + }
This does not work properly when you press both shift keys together, e.g.
press the left shift key first and keep it pressed, then shortly press the
right shift key and release it, then type some letters while you still hold
down the left shift key.
I think you should rather treat the shift value like a bitfield here, e.g.:
if (qcode == Q_KEY_CODE_SHIFT) {
if (key_down) {
s->shift |= KD_LSHIFT;
} else {
s->shift &= ~KD_LSHIFT;
}
}
if (qcode == Q_KEY_CODE_SHIFT_R) {
if (key_down) {
s->shift |= KD_RSHIFT;
} else {
s->shift &= ~KD_RSHIFT;
}
}
Thomas
> + /* If key release event, create keyboard break code */
> + if (!key_down) {
> + keycode = keycode | 0x80;
> + }
> +
> + nextkbd_put_keycode(s, keycode);
> +}
> +
> +static const QemuInputHandler nextkbd_handler = {
> + .name = "QEMU NeXT Keyboard",
> + .mask = INPUT_EVENT_MASK_KEY,
> + .event = nextkbd_event,
> +};
> +
> static void nextkbd_reset(DeviceState *dev)
> {
> NextKBDState *nks = NEXTKBD(dev);
> @@ -256,7 +304,7 @@ static void nextkbd_realize(DeviceState *dev, Error **errp)
> memory_region_init_io(&s->mr, OBJECT(dev), &kbd_ops, s, "next.kbd", 0x1000);
> sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mr);
>
> - qemu_add_kbd_event_handler(nextkbd_event, s);
> + qemu_input_handler_register(dev, &nextkbd_handler);
> }
>
> static const VMStateDescription nextkbd_vmstate = {
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] next-kbd: convert to use qemu_input_handler_register()
2024-11-01 9:26 ` Thomas Huth
@ 2024-11-01 18:59 ` Mark Cave-Ayland
0 siblings, 0 replies; 5+ messages in thread
From: Mark Cave-Ayland @ 2024-11-01 18:59 UTC (permalink / raw)
To: Thomas Huth; +Cc: peter.maydell, qemu-devel
On 01/11/2024 09:26, Thomas Huth wrote:
> Am Wed, 30 Oct 2024 09:18:03 +0000
> schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>
>> Convert the next-kbd device from the legacy UI qemu_add_kbd_event_handler()
>> function to use qemu_input_handler_register().
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/m68k/next-kbd.c | 158 +++++++++++++++++++++++++++++----------------
>> 1 file changed, 103 insertions(+), 55 deletions(-)
>>
>> diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c
>> index bc67810f31..85ef784491 100644
>> --- a/hw/m68k/next-kbd.c
>> +++ b/hw/m68k/next-kbd.c
>> @@ -68,7 +68,6 @@ struct NextKBDState {
>> uint16_t shift;
>> };
>>
>> -static void queue_code(void *opaque, int code);
>>
>> /* lots of magic numbers here */
>> static uint32_t kbd_read_byte(void *opaque, hwaddr addr)
>> @@ -166,68 +165,79 @@ static const MemoryRegionOps kbd_ops = {
>> .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>>
>> -static void nextkbd_event(void *opaque, int ch)
>> -{
>> - /*
>> - * Will want to set vars for caps/num lock
>> - * if (ch & 0x80) -> key release
>> - * there's also e0 escaped scancodes that might need to be handled
>> - */
>> - queue_code(opaque, ch);
>> -}
>> -
>> -static const unsigned char next_keycodes[128] = {
>> - 0x00, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x50, 0x4F,
>> - 0x4E, 0x1E, 0x1F, 0x20, 0x1D, 0x1C, 0x1B, 0x00,
>> - 0x42, 0x43, 0x44, 0x45, 0x48, 0x47, 0x46, 0x06,
>> - 0x07, 0x08, 0x00, 0x00, 0x2A, 0x00, 0x39, 0x3A,
>> - 0x3B, 0x3C, 0x3D, 0x40, 0x3F, 0x3E, 0x2D, 0x2C,
>> - 0x2B, 0x26, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34,
>> - 0x35, 0x37, 0x36, 0x2e, 0x2f, 0x30, 0x00, 0x00,
>> - 0x00, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
>> +#define NEXTKDB_NO_KEY 0xff
>> +
>> +static const int qcode_to_nextkbd_keycode[] = {
>> + /* Make sure future additions are automatically set to NEXTKDB_NO_KEY */
>> + [0 ... 0xff] = NEXTKDB_NO_KEY,
>
> I think it would be better to use Q_KEY_CODE__MAX here instead of 0xff ?
Ah I wasn't aware of that, but it makes sense - I ended up using adb-kbd.c as a guide
which is where the (NEXTKBD_)NO_KEY constant comes from.
>> + [Q_KEY_CODE_ESC] = 0x49,
>> + [Q_KEY_CODE_1] = 0x4a,
>> + [Q_KEY_CODE_2] = 0x4b,
>> + [Q_KEY_CODE_3] = 0x4c,
>> + [Q_KEY_CODE_4] = 0x4d,
>> + [Q_KEY_CODE_5] = 0x50,
>> + [Q_KEY_CODE_6] = 0x4f,
>> + [Q_KEY_CODE_7] = 0x4e,
>> + [Q_KEY_CODE_8] = 0x1e,
>> + [Q_KEY_CODE_9] = 0x1f,
>> + [Q_KEY_CODE_0] = 0x20,
>> + [Q_KEY_CODE_MINUS] = 0x1d,
>> + [Q_KEY_CODE_EQUAL] = 0x1c,
>> + [Q_KEY_CODE_BACKSPACE] = 0x1b,
>> + [Q_KEY_CODE_TAB] = 0x00,
>> +
>> + [Q_KEY_CODE_Q] = 0x42,
>> + [Q_KEY_CODE_W] = 0x43,
>> + [Q_KEY_CODE_E] = 0x44,
>> + [Q_KEY_CODE_R] = 0x45,
>> + [Q_KEY_CODE_T] = 0x48,
>> + [Q_KEY_CODE_Y] = 0x47,
>> + [Q_KEY_CODE_U] = 0x46,
>> + [Q_KEY_CODE_I] = 0x06,
>> + [Q_KEY_CODE_O] = 0x07,
>> + [Q_KEY_CODE_P] = 0x08,
>> + [Q_KEY_CODE_RET] = 0x2a,
>> + [Q_KEY_CODE_A] = 0x39,
>> + [Q_KEY_CODE_S] = 0x3a,
>> +
>> + [Q_KEY_CODE_D] = 0x3b,
>> + [Q_KEY_CODE_F] = 0x3c,
>> + [Q_KEY_CODE_G] = 0x3d,
>> + [Q_KEY_CODE_H] = 0x40,
>> + [Q_KEY_CODE_J] = 0x3f,
>> + [Q_KEY_CODE_K] = 0x3e,
>> + [Q_KEY_CODE_L] = 0x2d,
>> + [Q_KEY_CODE_SEMICOLON] = 0x2c,
>> + [Q_KEY_CODE_APOSTROPHE] = 0x2b,
>> + [Q_KEY_CODE_GRAVE_ACCENT] = 0x26,
>> + [Q_KEY_CODE_SHIFT] = 0x00,
>> + [Q_KEY_CODE_BACKSLASH] = 0x00,
>> + [Q_KEY_CODE_Z] = 0x31,
>> + [Q_KEY_CODE_X] = 0x32,
>> + [Q_KEY_CODE_C] = 0x33,
>> + [Q_KEY_CODE_V] = 0x34,
>> +
>> + [Q_KEY_CODE_B] = 0x35,
>> + [Q_KEY_CODE_N] = 0x37,
>> + [Q_KEY_CODE_M] = 0x36,
>> + [Q_KEY_CODE_COMMA] = 0x2e,
>> + [Q_KEY_CODE_DOT] = 0x2f,
>> + [Q_KEY_CODE_SLASH] = 0x30,
>> + [Q_KEY_CODE_SHIFT_R] = 0x00,
>> +
>> + [Q_KEY_CODE_SPC] = 0x38,
>> };
>>
>> -static void queue_code(void *opaque, int code)
>> +static void nextkbd_put_keycode(NextKBDState *s, int keycode)
>> {
>> - NextKBDState *s = NEXTKBD(opaque);
>> KBDQueue *q = &s->queue;
>> - int key = code & KD_KEYMASK;
>> - int release = code & 0x80;
>> - static int ext;
>> -
>> - if (code == 0xE0) {
>> - ext = 1;
>> - }
>> -
>> - if (code == 0x2A || code == 0x1D || code == 0x36) {
>> - if (code == 0x2A) {
>> - s->shift = KD_LSHIFT;
>> - } else if (code == 0x36) {
>> - s->shift = KD_RSHIFT;
>> - ext = 0;
>> - } else if (code == 0x1D && !ext) {
>> - s->shift = KD_LCOMM;
>> - } else if (code == 0x1D && ext) {
>> - ext = 0;
>> - s->shift = KD_RCOMM;
>> - }
>> - return;
>> - } else if (code == (0x2A | 0x80) || code == (0x1D | 0x80) ||
>> - code == (0x36 | 0x80)) {
>> - s->shift = 0;
>> - return;
>> - }
>>
>> if (q->count >= KBD_QUEUE_SIZE) {
>> return;
>> }
>>
>> - q->data[q->wptr] = next_keycodes[key] | release;
>> -
>> + q->data[q->wptr] = keycode;
>> if (++q->wptr == KBD_QUEUE_SIZE) {
>> q->wptr = 0;
>> }
>> @@ -241,6 +251,44 @@ static void queue_code(void *opaque, int code)
>> /* s->update_irq(s->update_arg, 1); */
>> }
>>
>> +static void nextkbd_event(DeviceState *dev, QemuConsole *src, InputEvent *evt)
>> +{
>> + NextKBDState *s = NEXTKBD(dev);
>> + int qcode, keycode;
>> + bool key_down = evt->u.key.data->down;
>> +
>> + qcode = qemu_input_key_value_to_qcode(evt->u.key.data->key);
>> + if (qcode >= ARRAY_SIZE(qcode_to_nextkbd_keycode)) {
>> + return;
>> + }
>> +
>> + keycode = qcode_to_nextkbd_keycode[qcode];
>> + if (keycode == NEXTKDB_NO_KEY) {
>> + return;
>> + }
>> +
>> + if (qcode == Q_KEY_CODE_SHIFT) {
>> + s->shift = key_down ? KD_LSHIFT : 0;
>> + }
>> +
>> + if (qcode == Q_KEY_CODE_SHIFT_R) {
>> + s->shift = key_down ? KD_RSHIFT : 0;
>> + }
>
> This does not work properly when you press both shift keys together, e.g.
> press the left shift key first and keep it pressed, then shortly press the
> right shift key and release it, then type some letters while you still hold
> down the left shift key.
>
> I think you should rather treat the shift value like a bitfield here, e.g.:
>
> if (qcode == Q_KEY_CODE_SHIFT) {
> if (key_down) {
> s->shift |= KD_LSHIFT;
> } else {
> s->shift &= ~KD_LSHIFT;
> }
> }
>
> if (qcode == Q_KEY_CODE_SHIFT_R) {
> if (key_down) {
> s->shift |= KD_RSHIFT;
> } else {
> s->shift &= ~KD_RSHIFT;
> }
> }
Thanks for this! I mistook the checks for !ext and ext in the current version as
being part of the scancode state machine, but as you point out they are in fact
needed for the shift key logic to work correctly.
I prefer your version of the logic above using a bitfield, and making the change is
easy since the device is currently marked as non-migratable.
I'll go ahead and make both these changes for v3.
> Thomas
>
>> + /* If key release event, create keyboard break code */
>> + if (!key_down) {
>> + keycode = keycode | 0x80;
>> + }
>> +
>> + nextkbd_put_keycode(s, keycode);
>> +}
>> +
>> +static const QemuInputHandler nextkbd_handler = {
>> + .name = "QEMU NeXT Keyboard",
>> + .mask = INPUT_EVENT_MASK_KEY,
>> + .event = nextkbd_event,
>> +};
>> +
>> static void nextkbd_reset(DeviceState *dev)
>> {
>> NextKBDState *nks = NEXTKBD(dev);
>> @@ -256,7 +304,7 @@ static void nextkbd_realize(DeviceState *dev, Error **errp)
>> memory_region_init_io(&s->mr, OBJECT(dev), &kbd_ops, s, "next.kbd", 0x1000);
>> sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mr);
>>
>> - qemu_add_kbd_event_handler(nextkbd_event, s);
>> + qemu_input_handler_register(dev, &nextkbd_handler);
>> }
>>
>> static const VMStateDescription nextkbd_vmstate = {
ATB,
Mark.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-11-01 19:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-30 9:18 [PATCH] next-kbd: convert to use qemu_input_handler_register() Mark Cave-Ayland
2024-10-30 10:28 ` Alex Bennée
2024-10-31 9:33 ` Mark Cave-Ayland
2024-11-01 9:26 ` Thomas Huth
2024-11-01 18:59 ` Mark Cave-Ayland
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).