> Subject: Re: [PATCH] ps2: set ps/2 output buffer size as the same as kernel
>
> > @@ -137,7 +139,7 @@ void ps2_queue(void *opaque, int b)
> > PS2State *s = (PS2State *)opaque;
> > PS2Queue *q = &s->queue;
> >
> > - if (q->count >= PS2_QUEUE_SIZE)
> > + if (q->count >= PS2_QUEUE_SIZE - 1)
>
> Why?
>
Because when the queue buffer is 16 bytes (full), the i8042_controller_check()
will report the error in linux kernel code. And the testing results show
decreasing 1 is necessary.
> > if (!(s->mouse_status & MOUSE_STATUS_REMOTE) &&
> > - (s->common.queue.count < (PS2_QUEUE_SIZE - 16))) {
> > + (s->common.queue.count < PS2_QUEUE_SIZE)) {
>
> To me this looks like an attempt to make sure the queue has enough space
> for the whole mouse message. Message size is 3 or 4 bytes (depending on
> mode), so I think we should make that "... < (PS2_QUEUE_SIZE-4)".
>
TBH, I don't understand the reason, and I haven't found any patches
about " PS2_QUEUE_SIZE - 16". But I quite agree with you, and I will test it.
> > for(;;) {
> > /* if not remote, send event. Multiple events are sent if
> > too big deltas */
>
> ... and move the check into the loop. Or, maybe even better, into the
> ps2_mouse_send_packet() function.
>
OK.
> > + for (i = 0; i < size; i++) {
> > + /* move the queue elements to the temporary buffer */
> > + tmp_data[i] = q->data[q->rptr];
> > + if (++q->rptr == 256) {
> > + q->rptr = 0;
> > + }
> > + }
> > + /* move the queue elements to the start of data array */
> > + if (size > 0) {
> > + memcpy(q->data, tmp_data, size);
> > + }
>
> You can move the loop into the "if (size > 0) { ... }" section.
>
Agreed.
> > static const VMStateDescription vmstate_ps2_mouse = {
> > .name = "ps2mouse",
> > .version_id = 2,
> > .minimum_version_id = 2,
> > .minimum_version_id_old = 2,
> > + .post_load = ps2_mouse_post_load,
>
> You have to call ps2_common_post_load in pre_save too.
>
OK. I will rework the patch and test it. Thanks, Gerd.
Best regards,
-Gonglei
