[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] ps2: add support of auto-repeat
From: |
li guang |
Subject: |
Re: [Qemu-devel] [PATCH] ps2: add support of auto-repeat |
Date: |
Thu, 16 May 2013 15:13:10 +0800 |
在 2013-05-16四的 14:58 +0800,Amos Kong写道:
> On Thu, May 16, 2013 at 01:30:28PM +0800, li guang wrote:
> > 在 2013-05-16四的 12:30 +0800,Amos Kong写道:
> > > 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.
> > >
> > > Guest ps2 driver sets autorepeat to fastest possible in reset,
> > > period: 250ms, delay: 33ms
> > >
> > > Tested by 'sendkey' monitor command.
> > >
> > > referenced: http://www.computer-engineering.org/ps2keyboard/
> > >
> > > Signed-off-by: Amos Kong <address@hidden>
> > > ---
> > > hw/input/ps2.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 42 insertions(+)
> > >
> > > 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;
> > > +
> > > +static void repeat_ps2_queue(void *opaque)
> > > +{
> > > + PS2KbdState *s = opaque;
> > > +
>
> Hi, Li guang
>
> > theoretically, we have to check if the typematic key is in break
> > now, if so, we will not do repeat anymore.
>
> You mean key is released? I checked by '~keycode & 0x80', stop repeat
> when key is release.
>
> > don't you think we have a chance that new key can come in during
> > waiting?
>
> When the new key (press) comes, repeat_timer will be modified by
> qemu_mod_timer(), original repate will be end.
>
> > > + if (auto_repeat) {
> > > + qemu_mod_timer(repeat_timer, qemu_get_clock_ns(vm_clock) +
> > > + muldiv64(get_ticks_per_sec(), s->repeat_period,
> > > + 1000));
> > > + ps2_queue(&s->common, s->repeat_key);
> > > + }
> > > +}
> > > +
> > > +
> > > /*
> > > keycode is expressed as follow:
> > > bit 7 - 0 key pressed, 1 = key released
> > > @@ -167,7 +186,17 @@ static void ps2_put_keycode(void *opaque, int
> > > keycode)
> > > keycode = ps2_raw_keycode_set3[keycode & 0x7f];
> > > }
> > > }
> > > +
> > > + /* only auto-repeat press event */
> > > + auto_repeat = ~keycode & 0x80;
>
> ^^^
case:
1. new key pressed
2. enter ps2_put_keycode
3. previous timer timeout
4. enter repeat_ps2_queue
5. put previous keycode in queue
6. back to ps2_put_keycode
7. check auto_repeat
so, an obsolete key comes up.
can timer interrupt ps2_put_keycode?
>
> > > ps2_queue(&s->common, keycode);
> > > +
> > > + if (auto_repeat) {
> > > + s->repeat_key = keycode;
> > > + /* delay a while before first repeat */
> > > + qemu_mod_timer(repeat_timer, qemu_get_clock_ns(vm_clock) +
> > > + muldiv64(get_ticks_per_sec(), s->repeat_delay,
> > > 1000));
> > > + }
> > > }
> > >
> > > uint32_t ps2_read_data(void *opaque)
> > > @@ -213,6 +242,11 @@ static void ps2_reset_keyboard(PS2KbdState *s)
> > >
> > > void ps2_write_keyboard(void *opaque, int val)
> > > {
> > > + /* repeat period/delay table from kernel
> > > (drivers/input/keyboard/atkbd.c) */
> > > + const short period[32] = { 33, 37, 42, 46, 50, 54, 58, 63,
> > > 67, 75,
> > > + 83, 92, 100, 109, 116, 125, 133, 149, 167, 182, 200,
> > > 217, 232,
> > > + 250, 270, 303, 333, 370, 400, 435, 470, 500 };
> > > + const short delay[4] = { 250, 500, 750, 1000 };
> > > PS2KbdState *s = (PS2KbdState *)opaque;
> > >
> > > switch(s->common.write_cmd) {
> > > @@ -288,6 +322,11 @@ void ps2_write_keyboard(void *opaque, int val)
> > > s->common.write_cmd = -1;
> > > break;
> > > case KBD_CMD_SET_RATE:
> > > + /* Bit0-4 specifies the repeat rate */
> > > + s->repeat_period = period[val & 0x1f];
> > > + /* Bit5-6 bit specifies the delay time */
> > > + s->repeat_delay = delay[(val & 0x60) >> 5];
> >
> > s/(val & 0x60) >> 5/(val & 0x60) >> 5 & 0x4/
> ^^ 0x3 ?
>
> val >> 5 & 0x3
Oh, yes should be '& 0x3'