Re: [PATCH v2 1/5] ps2: Initial horizontal scroll support

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Dec 22, 2021 at 02:06:43AM +0100, Dmitry Petrov wrote:
> v2:
>   - Patch is split into a sequence
>   - value is clamped to 31 for horizontal scroll
> 
> This patch introduces horizontal scroll support for the ps/2
> mouse.
> 
> The patch is based on the previous work
> by Brad Jorsch done in 2010
> but never merge, see
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=579968

The bugs attached to that ticket don't have any Signed-off-by
from Brad. Fortunately at the time he did re-submit them on
qemu-devel with Signed-off-by present.

Can you include this link to his patches in your commit
message to get the paper trail

  https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg00223.html

> This change adds support for horizontal scroll to ps/2 mouse device
> code. The code is implemented to match the logic of linux kernel
> which is used as a reference.
> 
> Signed-off-by: Dmitry Petrov <dpetroff@gmail.com>
> ---
>  hw/input/ps2.c | 57 +++++++++++++++++++++++++++++++++++++++++++-------
>  qapi/ui.json   |  2 +-
>  2 files changed, 50 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/input/ps2.c b/hw/input/ps2.c
> index 9376a8f4ce..6236711e1b 100644
> --- a/hw/input/ps2.c
> +++ b/hw/input/ps2.c
> @@ -123,6 +123,7 @@ typedef struct {
>      int mouse_dx; /* current values, needed for 'poll' mode */
>      int mouse_dy;
>      int mouse_dz;
> +    int mouse_dw;
>      uint8_t mouse_buttons;
>  } PS2MouseState;
>  
> @@ -715,7 +716,7 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
>      /* IMPS/2 and IMEX send 4 bytes, PS2 sends 3 bytes */
>      const int needed = s->mouse_type ? 4 : 3;
>      unsigned int b;
> -    int dx1, dy1, dz1;
> +    int dx1, dy1, dz1, dw1;
>  
>      if (PS2_QUEUE_SIZE - s->common.queue.count < needed) {
>          return 0;
> @@ -724,6 +725,7 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
>      dx1 = s->mouse_dx;
>      dy1 = s->mouse_dy;
>      dz1 = s->mouse_dz;
> +    dw1 = s->mouse_dw;
>      /* XXX: increase range to 8 bits ? */
>      if (dx1 > 127)
>          dx1 = 127;
> @@ -740,6 +742,9 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
>      /* extra byte for IMPS/2 or IMEX */
>      switch(s->mouse_type) {
>      default:
> +        /* Just ignore the wheels if not supported */
> +        s->mouse_dz = 0;
> +        s->mouse_dw = 0;
>          break;
>      case 3:
>          if (dz1 > 127)
> @@ -747,13 +752,41 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
>          else if (dz1 < -127)
>                  dz1 = -127;
>          ps2_queue_noirq(&s->common, dz1 & 0xff);
> +        s->mouse_dz -= dz1;
> +        s->mouse_dw = 0;
>          break;
>      case 4:
> -        if (dz1 > 7)
> -            dz1 = 7;
> -        else if (dz1 < -7)
> -            dz1 = -7;
> -        b = (dz1 & 0x0f) | ((s->mouse_buttons & 0x18) << 1);
> +        /*
> +         * This matches what the Linux kernel expects for exps/2 in
> +         * drivers/input/mouse/psmouse-base.c. Note, if you happen to
> +         * press/release the 4th or 5th buttons at the same moment as a
> +         * horizontal wheel scroll, those button presses will get lost. I'm not
> +         * sure what to do about that, since by this point we don't know
> +         * whether those buttons actually changed state.
> +         */
> +        if (dw1 != 0) {
> +            if (dw1 > 31) {
> +                dw1 = 31;
> +            } else if (dw1 < -31) {
> +                dw1 = -31;
> +            }
> +
> +            /*
> +             * linux kernel expects first 6 bits to represent the value
> +             * for horizontal scroll
> +             */
> +            b = (dw1 & 0x3f) | 0x40;
> +            s->mouse_dw -= dw1;
> +        } else {
> +            if (dz1 > 7) {
> +                dz1 = 7;
> +            } else if (dz1 < -7) {
> +                dz1 = -7;
> +            }
> +
> +            b = (dz1 & 0x0f) | ((s->mouse_buttons & 0x18) << 1);
> +            s->mouse_dz -= dz1;
> +        }
>          ps2_queue_noirq(&s->common, b);
>          break;
>      }
> @@ -764,7 +797,6 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
>      /* update deltas */
>      s->mouse_dx -= dx1;
>      s->mouse_dy -= dy1;
> -    s->mouse_dz -= dz1;
>  
>      return 1;
>  }
> @@ -806,6 +838,12 @@ static void ps2_mouse_event(DeviceState *dev, QemuConsole *src,
>              } else if (btn->button == INPUT_BUTTON_WHEEL_DOWN) {
>                  s->mouse_dz++;
>              }
> +
> +            if (btn->button == INPUT_BUTTON_WHEEL_RIGHT) {
> +                s->mouse_dw--;
> +            } else if (btn->button == INPUT_BUTTON_WHEEL_LEFT) {
> +                s->mouse_dw++;
> +            }
>          } else {
>              s->mouse_buttons &= ~bmap[btn->button];
>          }
> @@ -833,8 +871,10 @@ static void ps2_mouse_sync(DeviceState *dev)
>          /* if not remote, send event. Multiple events are sent if
>             too big deltas */
>          while (ps2_mouse_send_packet(s)) {
> -            if (s->mouse_dx == 0 && s->mouse_dy == 0 && s->mouse_dz == 0)
> +            if (s->mouse_dx == 0 && s->mouse_dy == 0
> +                    && s->mouse_dz == 0 && s->mouse_dw == 0) {
>                  break;
> +            }
>          }
>      }
>  }
> @@ -1036,6 +1076,7 @@ static void ps2_mouse_reset(void *opaque)
>      s->mouse_dx = 0;
>      s->mouse_dy = 0;
>      s->mouse_dz = 0;
> +    s->mouse_dw = 0;
>      s->mouse_buttons = 0;
>  }
>  
> diff --git a/qapi/ui.json b/qapi/ui.json
> index d7567ac866..9dac1bf548 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -905,7 +905,7 @@
>  ##
>  { 'enum'  : 'InputButton',
>    'data'  : [ 'left', 'middle', 'right', 'wheel-up', 'wheel-down', 'side',
> -  'extra' ] }
> +  'extra', 'wheel-left', 'wheel-right' ] }
>  
>  ##
>  # @InputAxis:
> -- 
> 2.32.0
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|