Update the Sun mouse implementation to use QemuInputHandler instead of the
legacy qemu_add_mouse_event_handler() function.

Note that this conversion adds extra sunmouse_* members to ESCCChannelState
but they are not added to the migration stream (similar to the Sun keyboard
members). If this were desired in future, the Sun devices should be split
into separate devices and added to the migration stream there instead.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2518
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/char/escc.c         | 88 +++++++++++++++++++++++++++++++-----------
 include/hw/char/escc.h |  3 ++
 2 files changed, 69 insertions(+), 22 deletions(-)

v3:
- Subtract the deltas in sunmouse_sync() instead of resetting them to zero
  which provides better tracking if the mouse movement exceeds the 8-bit
  delta limit of the MSC protocol 

- Add R-B tag from Richard

v2:
- Only allow left, middle and right button events (use bit 7 which is always
  set in the first byte to indicate a valid event)

- Remove zero entries from the bmap table as static entries should be
  zero anyway


diff --git a/hw/char/escc.c b/hw/char/escc.c
index d450d70eda..245a7b19d3 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -287,6 +287,7 @@ static void escc_reset_chn(ESCCChannelState *s)
     s->rxint = s->txint = 0;
     s->rxint_under_svc = s->txint_under_svc = 0;
     s->e0_mode = s->led_mode = s->caps_lock_mode = s->num_lock_mode = 0;
+    s->sunmouse_dx = s->sunmouse_dy = s->sunmouse_buttons = 0;
     clear_queue(s);
 }
 
@@ -952,53 +953,96 @@ static void handle_kbd_command(ESCCChannelState *s, int val)
     }
 }
 
-static void sunmouse_event(void *opaque,
-                               int dx, int dy, int dz, int buttons_state)
+static void sunmouse_handle_event(DeviceState *dev, QemuConsole *src,
+                                  InputEvent *evt)
 {
-    ESCCChannelState *s = opaque;
-    int ch;
+    ESCCChannelState *s = (ESCCChannelState *)dev;
+    InputMoveEvent *move;
+    InputBtnEvent *btn;
+    static const int bmap[INPUT_BUTTON__MAX] = {
+        [INPUT_BUTTON_LEFT]   = 0x4,
+        [INPUT_BUTTON_MIDDLE] = 0x2,
+        [INPUT_BUTTON_RIGHT]  = 0x1,
+    };
+
+    switch (evt->type) {
+    case INPUT_EVENT_KIND_REL:
+        move = evt->u.rel.data;
+        if (move->axis == INPUT_AXIS_X) {
+            s->sunmouse_dx += move->value;
+        } else if (move->axis == INPUT_AXIS_Y) {
+            s->sunmouse_dy -= move->value;
+        }
+        break;
 
-    trace_escc_sunmouse_event(dx, dy, buttons_state);
-    ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
+    case INPUT_EVENT_KIND_BTN:
+        btn = evt->u.btn.data;
+        if (bmap[btn->button]) {
+            if (btn->down) {
+                s->sunmouse_buttons |= bmap[btn->button];
+            } else {
+                s->sunmouse_buttons &= ~bmap[btn->button];
+            }
+            /* Indicate we have a supported button event */
+            s->sunmouse_buttons |= 0x80;
+        }
+        break;
 
-    if (buttons_state & MOUSE_EVENT_LBUTTON) {
-        ch ^= 0x4;
-    }
-    if (buttons_state & MOUSE_EVENT_MBUTTON) {
-        ch ^= 0x2;
+    default:
+        /* keep gcc happy */
+        break;
     }
-    if (buttons_state & MOUSE_EVENT_RBUTTON) {
-        ch ^= 0x1;
+}
+
+static void sunmouse_sync(DeviceState *dev)
+{
+    ESCCChannelState *s = (ESCCChannelState *)dev;
+    int ch;
+
+    if (s->sunmouse_dx == 0 && s->sunmouse_dy == 0 &&
+        (s->sunmouse_buttons & 0x80) == 0) {
+            /* Nothing to do after button event filter */
+            return;
     }
 
+    /* Clear our button event flag */
+    s->sunmouse_buttons &= ~0x80;
+    trace_escc_sunmouse_event(s->sunmouse_dx, s->sunmouse_dy,
+                              s->sunmouse_buttons);
+    ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
+    ch ^= s->sunmouse_buttons;
     put_queue(s, ch);
 
-    ch = dx;
-
+    ch = s->sunmouse_dx;
     if (ch > 127) {
         ch = 127;
     } else if (ch < -127) {
         ch = -127;
     }
-
     put_queue(s, ch & 0xff);
+    s->sunmouse_dx -= ch;
 
-    ch = -dy;
-
+    ch = s->sunmouse_dy;
     if (ch > 127) {
         ch = 127;
     } else if (ch < -127) {
         ch = -127;
     }
-
     put_queue(s, ch & 0xff);
+    s->sunmouse_dy -= ch;
 
     /* MSC protocol specifies two extra motion bytes */
-
     put_queue(s, 0);
     put_queue(s, 0);
 }
 
+static const QemuInputHandler sunmouse_handler = {
+    .name  = "QEMU Sun Mouse",
+    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
+    .event = sunmouse_handle_event,
+    .sync  = sunmouse_sync,
+};
+
 static void escc_init1(Object *obj)
 {
     ESCCState *s = ESCC(obj);
@@ -1036,8 +1080,8 @@ static void escc_realize(DeviceState *dev, Error **errp)
     }
 
     if (s->chn[0].type == escc_mouse) {
-        qemu_add_mouse_event_handler(sunmouse_event, &s->chn[0], 0,
-                                     "QEMU Sun Mouse");
+        s->chn[0].hs = qemu_input_handler_register((DeviceState *)(&s->chn[0]),
+                                                   &sunmouse_handler);
     }
     if (s->chn[1].type == escc_kbd) {
         s->chn[1].hs = qemu_input_handler_register((DeviceState *)(&s->chn[1]),
diff --git a/include/hw/char/escc.h b/include/hw/char/escc.h
index 5669a5b811..8c4c6a7730 100644
--- a/include/hw/char/escc.h
+++ b/include/hw/char/escc.h
@@ -46,6 +46,9 @@ typedef struct ESCCChannelState {
     uint8_t rx, tx;
     QemuInputHandlerState *hs;
     char *sunkbd_layout;
+    int sunmouse_dx;
+    int sunmouse_dy;
+    int sunmouse_buttons;
 } ESCCChannelState;
 
 struct ESCCState {
-- 
2.39.2

On 4/9/24 12:23, Mark Cave-Ayland wrote:
> Update the Sun mouse implementation to use QemuInputHandler instead of the
> legacy qemu_add_mouse_event_handler() function.
> 
> Note that this conversion adds extra sunmouse_* members to ESCCChannelState
> but they are not added to the migration stream (similar to the Sun keyboard
> members). If this were desired in future, the Sun devices should be split
> into separate devices and added to the migration stream there instead.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2518
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

On v3 there is also an implicit:
Tested-by: Carl Hauser <chauser@pullman.com>

> ---
>   hw/char/escc.c         | 88 +++++++++++++++++++++++++++++++-----------
>   include/hw/char/escc.h |  3 ++
>   2 files changed, 69 insertions(+), 22 deletions(-)
> 
> v3:
> - Subtract the deltas in sunmouse_sync() instead of resetting them to zero
>    which provides better tracking if the mouse movement exceeds the 8-bit
>    delta limit of the MSC protocol
> 
> - Add R-B tag from Richard
> 
> v2:
> - Only allow left, middle and right button events (use bit 7 which is always
>    set in the first byte to indicate a valid event)
> 
> - Remove zero entries from the bmap table as static entries should be
>    zero anyway
> 
> 
> diff --git a/hw/char/escc.c b/hw/char/escc.c
> index d450d70eda..245a7b19d3 100644
> --- a/hw/char/escc.c
> +++ b/hw/char/escc.c
> @@ -287,6 +287,7 @@ static void escc_reset_chn(ESCCChannelState *s)
>       s->rxint = s->txint = 0;
>       s->rxint_under_svc = s->txint_under_svc = 0;
>       s->e0_mode = s->led_mode = s->caps_lock_mode = s->num_lock_mode = 0;
> +    s->sunmouse_dx = s->sunmouse_dy = s->sunmouse_buttons = 0;
>       clear_queue(s);
>   }
>   
> @@ -952,53 +953,96 @@ static void handle_kbd_command(ESCCChannelState *s, int val)
>       }
>   }
>   
> -static void sunmouse_event(void *opaque,
> -                               int dx, int dy, int dz, int buttons_state)
> +static void sunmouse_handle_event(DeviceState *dev, QemuConsole *src,
> +                                  InputEvent *evt)
>   {
> -    ESCCChannelState *s = opaque;
> -    int ch;
> +    ESCCChannelState *s = (ESCCChannelState *)dev;
> +    InputMoveEvent *move;
> +    InputBtnEvent *btn;
> +    static const int bmap[INPUT_BUTTON__MAX] = {
> +        [INPUT_BUTTON_LEFT]   = 0x4,
> +        [INPUT_BUTTON_MIDDLE] = 0x2,
> +        [INPUT_BUTTON_RIGHT]  = 0x1,
> +    };
> +
> +    switch (evt->type) {
> +    case INPUT_EVENT_KIND_REL:
> +        move = evt->u.rel.data;
> +        if (move->axis == INPUT_AXIS_X) {
> +            s->sunmouse_dx += move->value;
> +        } else if (move->axis == INPUT_AXIS_Y) {
> +            s->sunmouse_dy -= move->value;
> +        }
> +        break;
>   
> -    trace_escc_sunmouse_event(dx, dy, buttons_state);
> -    ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
> +    case INPUT_EVENT_KIND_BTN:
> +        btn = evt->u.btn.data;
> +        if (bmap[btn->button]) {
> +            if (btn->down) {
> +                s->sunmouse_buttons |= bmap[btn->button];
> +            } else {
> +                s->sunmouse_buttons &= ~bmap[btn->button];
> +            }
> +            /* Indicate we have a supported button event */
> +            s->sunmouse_buttons |= 0x80;
> +        }
> +        break;
>   
> -    if (buttons_state & MOUSE_EVENT_LBUTTON) {
> -        ch ^= 0x4;
> -    }
> -    if (buttons_state & MOUSE_EVENT_MBUTTON) {
> -        ch ^= 0x2;
> +    default:
> +        /* keep gcc happy */
> +        break;
>       }
> -    if (buttons_state & MOUSE_EVENT_RBUTTON) {
> -        ch ^= 0x1;
> +}
> +
> +static void sunmouse_sync(DeviceState *dev)
> +{
> +    ESCCChannelState *s = (ESCCChannelState *)dev;
> +    int ch;
> +
> +    if (s->sunmouse_dx == 0 && s->sunmouse_dy == 0 &&
> +        (s->sunmouse_buttons & 0x80) == 0) {
> +            /* Nothing to do after button event filter */
> +            return;
>       }
>   
> +    /* Clear our button event flag */
> +    s->sunmouse_buttons &= ~0x80;
> +    trace_escc_sunmouse_event(s->sunmouse_dx, s->sunmouse_dy,
> +                              s->sunmouse_buttons);
> +    ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
> +    ch ^= s->sunmouse_buttons;
>       put_queue(s, ch);
>   
> -    ch = dx;
> -
> +    ch = s->sunmouse_dx;
>       if (ch > 127) {
>           ch = 127;
>       } else if (ch < -127) {
>           ch = -127;
>       }
> -
>       put_queue(s, ch & 0xff);
> +    s->sunmouse_dx -= ch;
>   
> -    ch = -dy;
> -
> +    ch = s->sunmouse_dy;
>       if (ch > 127) {
>           ch = 127;
>       } else if (ch < -127) {
>           ch = -127;
>       }
> -
>       put_queue(s, ch & 0xff);
> +    s->sunmouse_dy -= ch;
>   
>       /* MSC protocol specifies two extra motion bytes */
> -
>       put_queue(s, 0);
>       put_queue(s, 0);
>   }
>   
> +static const QemuInputHandler sunmouse_handler = {
> +    .name  = "QEMU Sun Mouse",
> +    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
> +    .event = sunmouse_handle_event,
> +    .sync  = sunmouse_sync,
> +};
> +
>   static void escc_init1(Object *obj)
>   {
>       ESCCState *s = ESCC(obj);
> @@ -1036,8 +1080,8 @@ static void escc_realize(DeviceState *dev, Error **errp)
>       }
>   
>       if (s->chn[0].type == escc_mouse) {
> -        qemu_add_mouse_event_handler(sunmouse_event, &s->chn[0], 0,
> -                                     "QEMU Sun Mouse");
> +        s->chn[0].hs = qemu_input_handler_register((DeviceState *)(&s->chn[0]),
> +                                                   &sunmouse_handler);
>       }
>       if (s->chn[1].type == escc_kbd) {
>           s->chn[1].hs = qemu_input_handler_register((DeviceState *)(&s->chn[1]),
> diff --git a/include/hw/char/escc.h b/include/hw/char/escc.h
> index 5669a5b811..8c4c6a7730 100644
> --- a/include/hw/char/escc.h
> +++ b/include/hw/char/escc.h
> @@ -46,6 +46,9 @@ typedef struct ESCCChannelState {
>       uint8_t rx, tx;
>       QemuInputHandlerState *hs;
>       char *sunkbd_layout;
> +    int sunmouse_dx;
> +    int sunmouse_dy;
> +    int sunmouse_buttons;
>   } ESCCChannelState;
>   
>   struct ESCCState {

On 04/09/2024 11:53, Philippe Mathieu-Daudé wrote:

> On 4/9/24 12:23, Mark Cave-Ayland wrote:
>> Update the Sun mouse implementation to use QemuInputHandler instead of the
>> legacy qemu_add_mouse_event_handler() function.
>>
>> Note that this conversion adds extra sunmouse_* members to ESCCChannelState
>> but they are not added to the migration stream (similar to the Sun keyboard
>> members). If this were desired in future, the Sun devices should be split
>> into separate devices and added to the migration stream there instead.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2518
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> On v3 there is also an implicit:
> Tested-by: Carl Hauser <chauser@pullman.com>

That's true, although I'm hesitant to add such a tag without a nod from the tester. 
Carl, are you happy for me to add your Tested-by tag upon merge?

>> ---
>>   hw/char/escc.c         | 88 +++++++++++++++++++++++++++++++-----------
>>   include/hw/char/escc.h |  3 ++
>>   2 files changed, 69 insertions(+), 22 deletions(-)
>>
>> v3:
>> - Subtract the deltas in sunmouse_sync() instead of resetting them to zero
>>    which provides better tracking if the mouse movement exceeds the 8-bit
>>    delta limit of the MSC protocol
>>
>> - Add R-B tag from Richard
>>
>> v2:
>> - Only allow left, middle and right button events (use bit 7 which is always
>>    set in the first byte to indicate a valid event)
>>
>> - Remove zero entries from the bmap table as static entries should be
>>    zero anyway
>>
>>
>> diff --git a/hw/char/escc.c b/hw/char/escc.c
>> index d450d70eda..245a7b19d3 100644
>> --- a/hw/char/escc.c
>> +++ b/hw/char/escc.c
>> @@ -287,6 +287,7 @@ static void escc_reset_chn(ESCCChannelState *s)
>>       s->rxint = s->txint = 0;
>>       s->rxint_under_svc = s->txint_under_svc = 0;
>>       s->e0_mode = s->led_mode = s->caps_lock_mode = s->num_lock_mode = 0;
>> +    s->sunmouse_dx = s->sunmouse_dy = s->sunmouse_buttons = 0;
>>       clear_queue(s);
>>   }
>> @@ -952,53 +953,96 @@ static void handle_kbd_command(ESCCChannelState *s, int val)
>>       }
>>   }
>> -static void sunmouse_event(void *opaque,
>> -                               int dx, int dy, int dz, int buttons_state)
>> +static void sunmouse_handle_event(DeviceState *dev, QemuConsole *src,
>> +                                  InputEvent *evt)
>>   {
>> -    ESCCChannelState *s = opaque;
>> -    int ch;
>> +    ESCCChannelState *s = (ESCCChannelState *)dev;
>> +    InputMoveEvent *move;
>> +    InputBtnEvent *btn;
>> +    static const int bmap[INPUT_BUTTON__MAX] = {
>> +        [INPUT_BUTTON_LEFT]   = 0x4,
>> +        [INPUT_BUTTON_MIDDLE] = 0x2,
>> +        [INPUT_BUTTON_RIGHT]  = 0x1,
>> +    };
>> +
>> +    switch (evt->type) {
>> +    case INPUT_EVENT_KIND_REL:
>> +        move = evt->u.rel.data;
>> +        if (move->axis == INPUT_AXIS_X) {
>> +            s->sunmouse_dx += move->value;
>> +        } else if (move->axis == INPUT_AXIS_Y) {
>> +            s->sunmouse_dy -= move->value;
>> +        }
>> +        break;
>> -    trace_escc_sunmouse_event(dx, dy, buttons_state);
>> -    ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
>> +    case INPUT_EVENT_KIND_BTN:
>> +        btn = evt->u.btn.data;
>> +        if (bmap[btn->button]) {
>> +            if (btn->down) {
>> +                s->sunmouse_buttons |= bmap[btn->button];
>> +            } else {
>> +                s->sunmouse_buttons &= ~bmap[btn->button];
>> +            }
>> +            /* Indicate we have a supported button event */
>> +            s->sunmouse_buttons |= 0x80;
>> +        }
>> +        break;
>> -    if (buttons_state & MOUSE_EVENT_LBUTTON) {
>> -        ch ^= 0x4;
>> -    }
>> -    if (buttons_state & MOUSE_EVENT_MBUTTON) {
>> -        ch ^= 0x2;
>> +    default:
>> +        /* keep gcc happy */
>> +        break;
>>       }
>> -    if (buttons_state & MOUSE_EVENT_RBUTTON) {
>> -        ch ^= 0x1;
>> +}
>> +
>> +static void sunmouse_sync(DeviceState *dev)
>> +{
>> +    ESCCChannelState *s = (ESCCChannelState *)dev;
>> +    int ch;
>> +
>> +    if (s->sunmouse_dx == 0 && s->sunmouse_dy == 0 &&
>> +        (s->sunmouse_buttons & 0x80) == 0) {
>> +            /* Nothing to do after button event filter */
>> +            return;
>>       }
>> +    /* Clear our button event flag */
>> +    s->sunmouse_buttons &= ~0x80;
>> +    trace_escc_sunmouse_event(s->sunmouse_dx, s->sunmouse_dy,
>> +                              s->sunmouse_buttons);
>> +    ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
>> +    ch ^= s->sunmouse_buttons;
>>       put_queue(s, ch);
>> -    ch = dx;
>> -
>> +    ch = s->sunmouse_dx;
>>       if (ch > 127) {
>>           ch = 127;
>>       } else if (ch < -127) {
>>           ch = -127;
>>       }
>> -
>>       put_queue(s, ch & 0xff);
>> +    s->sunmouse_dx -= ch;
>> -    ch = -dy;
>> -
>> +    ch = s->sunmouse_dy;
>>       if (ch > 127) {
>>           ch = 127;
>>       } else if (ch < -127) {
>>           ch = -127;
>>       }
>> -
>>       put_queue(s, ch & 0xff);
>> +    s->sunmouse_dy -= ch;
>>       /* MSC protocol specifies two extra motion bytes */
>> -
>>       put_queue(s, 0);
>>       put_queue(s, 0);
>>   }
>> +static const QemuInputHandler sunmouse_handler = {
>> +    .name  = "QEMU Sun Mouse",
>> +    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
>> +    .event = sunmouse_handle_event,
>> +    .sync  = sunmouse_sync,
>> +};
>> +
>>   static void escc_init1(Object *obj)
>>   {
>>       ESCCState *s = ESCC(obj);
>> @@ -1036,8 +1080,8 @@ static void escc_realize(DeviceState *dev, Error **errp)
>>       }
>>       if (s->chn[0].type == escc_mouse) {
>> -        qemu_add_mouse_event_handler(sunmouse_event, &s->chn[0], 0,
>> -                                     "QEMU Sun Mouse");
>> +        s->chn[0].hs = qemu_input_handler_register((DeviceState *)(&s->chn[0]),
>> +                                                   &sunmouse_handler);
>>       }
>>       if (s->chn[1].type == escc_kbd) {
>>           s->chn[1].hs = qemu_input_handler_register((DeviceState *)(&s->chn[1]),
>> diff --git a/include/hw/char/escc.h b/include/hw/char/escc.h
>> index 5669a5b811..8c4c6a7730 100644
>> --- a/include/hw/char/escc.h
>> +++ b/include/hw/char/escc.h
>> @@ -46,6 +46,9 @@ typedef struct ESCCChannelState {
>>       uint8_t rx, tx;
>>       QemuInputHandlerState *hs;
>>       char *sunkbd_layout;
>> +    int sunmouse_dx;
>> +    int sunmouse_dy;
>> +    int sunmouse_buttons;
>>   } ESCCChannelState;
>>   struct ESCCState {


ATB,

Mark.

On 04/09/2024 12:19, Mark Cave-Ayland wrote:

> On 04/09/2024 11:53, Philippe Mathieu-Daudé wrote:
> 
>> On 4/9/24 12:23, Mark Cave-Ayland wrote:
>>> Update the Sun mouse implementation to use QemuInputHandler instead of the
>>> legacy qemu_add_mouse_event_handler() function.
>>>
>>> Note that this conversion adds extra sunmouse_* members to ESCCChannelState
>>> but they are not added to the migration stream (similar to the Sun keyboard
>>> members). If this were desired in future, the Sun devices should be split
>>> into separate devices and added to the migration stream there instead.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2518
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> On v3 there is also an implicit:
>> Tested-by: Carl Hauser <chauser@pullman.com>
> 
> That's true, although I'm hesitant to add such a tag without a nod from the tester. 
> Carl, are you happy for me to add your Tested-by tag upon merge?

I've received confirmation off-list that Carl is happy with the above tag, so we're 
good to add it to the final merged version. Phil, are you able to queue this in your 
next PR or would you prefer me to send a separate PR instead?

>>> ---
>>>   hw/char/escc.c         | 88 +++++++++++++++++++++++++++++++-----------
>>>   include/hw/char/escc.h |  3 ++
>>>   2 files changed, 69 insertions(+), 22 deletions(-)
>>>
>>> v3:
>>> - Subtract the deltas in sunmouse_sync() instead of resetting them to zero
>>>    which provides better tracking if the mouse movement exceeds the 8-bit
>>>    delta limit of the MSC protocol
>>>
>>> - Add R-B tag from Richard
>>>
>>> v2:
>>> - Only allow left, middle and right button events (use bit 7 which is always
>>>    set in the first byte to indicate a valid event)
>>>
>>> - Remove zero entries from the bmap table as static entries should be
>>>    zero anyway
>>>
>>>
>>> diff --git a/hw/char/escc.c b/hw/char/escc.c
>>> index d450d70eda..245a7b19d3 100644
>>> --- a/hw/char/escc.c
>>> +++ b/hw/char/escc.c
>>> @@ -287,6 +287,7 @@ static void escc_reset_chn(ESCCChannelState *s)
>>>       s->rxint = s->txint = 0;
>>>       s->rxint_under_svc = s->txint_under_svc = 0;
>>>       s->e0_mode = s->led_mode = s->caps_lock_mode = s->num_lock_mode = 0;
>>> +    s->sunmouse_dx = s->sunmouse_dy = s->sunmouse_buttons = 0;
>>>       clear_queue(s);
>>>   }
>>> @@ -952,53 +953,96 @@ static void handle_kbd_command(ESCCChannelState *s, int val)
>>>       }
>>>   }
>>> -static void sunmouse_event(void *opaque,
>>> -                               int dx, int dy, int dz, int buttons_state)
>>> +static void sunmouse_handle_event(DeviceState *dev, QemuConsole *src,
>>> +                                  InputEvent *evt)
>>>   {
>>> -    ESCCChannelState *s = opaque;
>>> -    int ch;
>>> +    ESCCChannelState *s = (ESCCChannelState *)dev;
>>> +    InputMoveEvent *move;
>>> +    InputBtnEvent *btn;
>>> +    static const int bmap[INPUT_BUTTON__MAX] = {
>>> +        [INPUT_BUTTON_LEFT]   = 0x4,
>>> +        [INPUT_BUTTON_MIDDLE] = 0x2,
>>> +        [INPUT_BUTTON_RIGHT]  = 0x1,
>>> +    };
>>> +
>>> +    switch (evt->type) {
>>> +    case INPUT_EVENT_KIND_REL:
>>> +        move = evt->u.rel.data;
>>> +        if (move->axis == INPUT_AXIS_X) {
>>> +            s->sunmouse_dx += move->value;
>>> +        } else if (move->axis == INPUT_AXIS_Y) {
>>> +            s->sunmouse_dy -= move->value;
>>> +        }
>>> +        break;
>>> -    trace_escc_sunmouse_event(dx, dy, buttons_state);
>>> -    ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
>>> +    case INPUT_EVENT_KIND_BTN:
>>> +        btn = evt->u.btn.data;
>>> +        if (bmap[btn->button]) {
>>> +            if (btn->down) {
>>> +                s->sunmouse_buttons |= bmap[btn->button];
>>> +            } else {
>>> +                s->sunmouse_buttons &= ~bmap[btn->button];
>>> +            }
>>> +            /* Indicate we have a supported button event */
>>> +            s->sunmouse_buttons |= 0x80;
>>> +        }
>>> +        break;
>>> -    if (buttons_state & MOUSE_EVENT_LBUTTON) {
>>> -        ch ^= 0x4;
>>> -    }
>>> -    if (buttons_state & MOUSE_EVENT_MBUTTON) {
>>> -        ch ^= 0x2;
>>> +    default:
>>> +        /* keep gcc happy */
>>> +        break;
>>>       }
>>> -    if (buttons_state & MOUSE_EVENT_RBUTTON) {
>>> -        ch ^= 0x1;
>>> +}
>>> +
>>> +static void sunmouse_sync(DeviceState *dev)
>>> +{
>>> +    ESCCChannelState *s = (ESCCChannelState *)dev;
>>> +    int ch;
>>> +
>>> +    if (s->sunmouse_dx == 0 && s->sunmouse_dy == 0 &&
>>> +        (s->sunmouse_buttons & 0x80) == 0) {
>>> +            /* Nothing to do after button event filter */
>>> +            return;
>>>       }
>>> +    /* Clear our button event flag */
>>> +    s->sunmouse_buttons &= ~0x80;
>>> +    trace_escc_sunmouse_event(s->sunmouse_dx, s->sunmouse_dy,
>>> +                              s->sunmouse_buttons);
>>> +    ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
>>> +    ch ^= s->sunmouse_buttons;
>>>       put_queue(s, ch);
>>> -    ch = dx;
>>> -
>>> +    ch = s->sunmouse_dx;
>>>       if (ch > 127) {
>>>           ch = 127;
>>>       } else if (ch < -127) {
>>>           ch = -127;
>>>       }
>>> -
>>>       put_queue(s, ch & 0xff);
>>> +    s->sunmouse_dx -= ch;
>>> -    ch = -dy;
>>> -
>>> +    ch = s->sunmouse_dy;
>>>       if (ch > 127) {
>>>           ch = 127;
>>>       } else if (ch < -127) {
>>>           ch = -127;
>>>       }
>>> -
>>>       put_queue(s, ch & 0xff);
>>> +    s->sunmouse_dy -= ch;
>>>       /* MSC protocol specifies two extra motion bytes */
>>> -
>>>       put_queue(s, 0);
>>>       put_queue(s, 0);
>>>   }
>>> +static const QemuInputHandler sunmouse_handler = {
>>> +    .name  = "QEMU Sun Mouse",
>>> +    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
>>> +    .event = sunmouse_handle_event,
>>> +    .sync  = sunmouse_sync,
>>> +};
>>> +
>>>   static void escc_init1(Object *obj)
>>>   {
>>>       ESCCState *s = ESCC(obj);
>>> @@ -1036,8 +1080,8 @@ static void escc_realize(DeviceState *dev, Error **errp)
>>>       }
>>>       if (s->chn[0].type == escc_mouse) {
>>> -        qemu_add_mouse_event_handler(sunmouse_event, &s->chn[0], 0,
>>> -                                     "QEMU Sun Mouse");
>>> +        s->chn[0].hs = qemu_input_handler_register((DeviceState *)(&s->chn[0]),
>>> +                                                   &sunmouse_handler);
>>>       }
>>>       if (s->chn[1].type == escc_kbd) {
>>>           s->chn[1].hs = qemu_input_handler_register((DeviceState *)(&s->chn[1]),
>>> diff --git a/include/hw/char/escc.h b/include/hw/char/escc.h
>>> index 5669a5b811..8c4c6a7730 100644
>>> --- a/include/hw/char/escc.h
>>> +++ b/include/hw/char/escc.h
>>> @@ -46,6 +46,9 @@ typedef struct ESCCChannelState {
>>>       uint8_t rx, tx;
>>>       QemuInputHandlerState *hs;
>>>       char *sunkbd_layout;
>>> +    int sunmouse_dx;
>>> +    int sunmouse_dy;
>>> +    int sunmouse_buttons;
>>>   } ESCCChannelState;
>>>   struct ESCCState {


ATB,

Mark.

On 6/9/24 22:55, Mark Cave-Ayland wrote:
> On 04/09/2024 12:19, Mark Cave-Ayland wrote:
> 
>> On 04/09/2024 11:53, Philippe Mathieu-Daudé wrote:
>>
>>> On 4/9/24 12:23, Mark Cave-Ayland wrote:
>>>> Update the Sun mouse implementation to use QemuInputHandler instead 
>>>> of the
>>>> legacy qemu_add_mouse_event_handler() function.
>>>>
>>>> Note that this conversion adds extra sunmouse_* members to 
>>>> ESCCChannelState
>>>> but they are not added to the migration stream (similar to the Sun 
>>>> keyboard
>>>> members). If this were desired in future, the Sun devices should be 
>>>> split
>>>> into separate devices and added to the migration stream there instead.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2518
>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>
>>> On v3 there is also an implicit:
>>> Tested-by: Carl Hauser <chauser@pullman.com>
>>
>> That's true, although I'm hesitant to add such a tag without a nod 
>> from the tester. Carl, are you happy for me to add your Tested-by tag 
>> upon merge?
> 
> I've received confirmation off-list that Carl is happy with the above 
> tag, so we're good to add it to the final merged version. Phil, are you 
> able to queue this in your next PR or would you prefer me to send a 
> separate PR instead?

Patch queued, thanks!

>>>> ---
>>>>   hw/char/escc.c         | 88 
>>>> +++++++++++++++++++++++++++++++-----------
>>>>   include/hw/char/escc.h |  3 ++
>>>>   2 files changed, 69 insertions(+), 22 deletions(-)