Re: [PATCH] escc: convert Sun mouse to use QemuInputHandler

[Carl Hauser via <qemu-devel@nongnu.org>]

[-- Attachment #1: Type: text/plain, Size: 6359 bytes --]

This still, but less frequently, shows the behavior of having the cursor
leap downwards occasionally. I may not be able to work on debugging it
until next week, but I'll try to see if I can figure it out sooner. The
hypothesis with the old code was that it was sending floods of mouse
messages and the Sun driver was dropping a byte at some point. So that's
the first thing I'll look into with this new code, but it could be
something different. This is definitely better than the old code -- just
not sending anything in response to mouse wheel movement helps a lot.

On Mon, Sep 2, 2024 at 2:38 PM Mark Cave-Ayland <
mark.cave-ayland@ilande.co.uk> 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
> ---
>  hw/char/escc.c         | 79 ++++++++++++++++++++++++++++++------------
>  include/hw/char/escc.h |  3 ++
>  roms/seabios           |  2 +-
>  3 files changed, 60 insertions(+), 24 deletions(-)
>
> diff --git a/hw/char/escc.c b/hw/char/escc.c
> index d450d70eda..6d4e3e3350 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,85 @@ 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,
> +        [INPUT_BUTTON_SIDE]   = 0x0,
> +        [INPUT_BUTTON_EXTRA]  = 0x0,
> +    };
> +
> +    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 (btn->down) {
> +            s->sunmouse_buttons |= bmap[btn->button];
> +        } else {
> +            s->sunmouse_buttons &= ~bmap[btn->button];
> +        }
> +        break;
>
> -    if (buttons_state & MOUSE_EVENT_LBUTTON) {
> -        ch ^= 0x4;
> -    }
> -    if (buttons_state & MOUSE_EVENT_MBUTTON) {
> -        ch ^= 0x2;
> -    }
> -    if (buttons_state & MOUSE_EVENT_RBUTTON) {
> -        ch ^= 0x1;
> +    default:
> +        /* keep gcc happy */
> +        break;
>      }
> +}
>
> -    put_queue(s, ch);
> +static void sunmouse_sync(DeviceState *dev)
> +{
> +    ESCCChannelState *s = (ESCCChannelState *)dev;
> +    int ch;
>
> -    ch = dx;
> +    trace_escc_sunmouse_event(s->sunmouse_dx, s->sunmouse_dy, 0);
> +    ch = 0x80 | 0x7; /* protocol start byte, no buttons pressed */
> +    ch ^= s->sunmouse_buttons;
> +    put_queue(s, ch);
>
> +    ch = s->sunmouse_dx;
>      if (ch > 127) {
>          ch = 127;
>      } else if (ch < -127) {
>          ch = -127;
>      }
> -
>      put_queue(s, ch & 0xff);
> +    s->sunmouse_dx = 0;
>
> -    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 = 0;
>
>      /* 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 +1069,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 {
> diff --git a/roms/seabios b/roms/seabios
> index a6ed6b701f..7d0c606870 160000
> --- a/roms/seabios
> +++ b/roms/seabios
> @@ -1 +1 @@
> -Subproject commit a6ed6b701f0a57db0569ab98b0661c12a6ec3ff8
> +Subproject commit 7d0c6068703eae9f2498be0c900ab95b25b4f07a
> --
> 2.39.2
>
>
>

[-- Attachment #2: Type: text/html, Size: 7758 bytes --]