Re: [PATCH] hw/char: suppress sunmouse events with no changes

[Richard Henderson <richard.henderson@linaro.org>]

On 8/20/24 09:18, Carl Hauser wrote:
> @@ -959,6 +960,15 @@ static void sunmouse_event(void *opaque,
>       int ch;
> 
>       trace_escc_sunmouse_event(dx, dy, buttons_state);
> +
> +    /* Don't send duplicate events without motion */
> +    if (dx == 0 &&
> +        dy == 0 &&
> +        (s->sunmouse_prev_state ^ buttons_state) == 0) {

Were you intending to mask vs MOUSE_EVENT_*BUTTON?
Otherwise this is just plain equality.

> diff --git a/include/hw/char/escc.h b/include/hw/char/escc.h
> index 5669a5b811..bc5ba4f564 100644
> --- a/include/hw/char/escc.h
> +++ b/include/hw/char/escc.h
> @@ -46,6 +46,7 @@ typedef struct ESCCChannelState {
>       uint8_t rx, tx;
>       QemuInputHandlerState *hs;
>       char *sunkbd_layout;
> +    int sunmouse_prev_state;

This adds new state that must be migrated.

While the patch is relatively simple, I do wonder if this code could be improved by 
converting away from the legacy mouse interface to qemu_input_handler_register. 
Especially if that might help avoid needing to add migration state that isn't "really" 
part of the device.

Mark?


r~