Re: [Qemu-devel] [PATCH] ps2: add support of auto-repeat

[Amos Kong]

On Thu, May 16, 2013 at 07:50:35AM +0100, Peter Maydell wrote:
> On 16 May 2013 05:30, Amos Kong <address@hidden> wrote:
> > Guest driver sets repeat rate and delay time by KBD_CMD_SET_RATE,
> > but ps2 backend doesn't process it and no auto-repeat implementation.
> > This patch adds support of auto-repeat feature.
> 
> > diff --git a/hw/input/ps2.c b/hw/input/ps2.c
> > index 3412079..1cfe055 100644
> > --- a/hw/input/ps2.c
> > +++ b/hw/input/ps2.c
> > @@ -94,6 +94,9 @@ typedef struct {
> >      int translate;
> >      int scancode_set; /* 1=XT, 2=AT, 3=PS/2 */
> >      int ledstate;
> > +    int repeat_period; /* typematic period, ms */
> > +    int repeat_delay; /* typematic delay, ms */
> > +    int repeat_key; /* keycode to repeat */
> >  } PS2KbdState;
> >
> >  typedef struct {
> > @@ -146,6 +149,22 @@ void ps2_queue(void *opaque, int b)
> >      s->update_irq(s->update_arg, 1);
> >  }
> >
> > +static QEMUTimer *repeat_timer;
> > +static bool auto_repeat;
> 
> These shouldn't be static -- what would happen
> on a system with two ps2 keyboard models in it?

I will move them to PS2KbdState.


For the migrate issue mentioned by jason, the keyboard state
(repeat_period/repeat_delay/etc) are configured by guest, it
should be saved to vmstate and migrated to dest vm.

If we want the unfinished repeat continue, repeat-timer also
should be migrated, but another key_timer in ui/input.c for
send-key could not be migrated, it means the release event
will be lost.

Do we need to migrate the keyboard state?
 
> You need to reset your qemu_timer in the ps2 reset
> handler, as well; otherwise it could go off unexpectedly
> after a reset. (Though perhaps not if we're simulating
> a human with their finger held down on the key...)

Ok
 
> thanks
> -- PMM

-- 
                        Amos.