diff mbox

xhci in slof issues

Message ID 87bn4uuksj.fsf@abhimanyu.in.ibm.com
State Superseded
Headers show

Commit Message

Nikunj A Dadhania April 28, 2016, 9:47 a.m. UTC
Dinar Valeev <k0da@opensuse.org> writes:

> Hi,
>
> While trying to fix my usb over vnc automated issues, I switched to
> xhci, which helped a lot OS wise, but I see a regression in SLOF.
>
> As you can see [1] we want to type an install repo url.  But at some
> point capitalization of some characters are lost. Nope
> openSUsE-tumbleweed instead of openSUSE-Tumbleweed.
>
> I was able to reproduce it even with simpler testcase by using vncdotool
>
> qemu-system-ppc64 -machine usb=off -vga std -enable-kvm -vnc :60 -m
> 4096 -device nec-usb-xhci -device usb-tablet -device usb-kbd
>
> cat vnc.sh
> vnc="vncdotool -s shiraz.arch:60"
> $vnc type "HaHaHaHaHaHaHaHaHa"
> $vnc key enter
>
> while true;do ./vnc.sh;done
>
> Three iterations are fine, starting with fourth characters are dropped.
>
> [1] https://openqa.opensuse.org/tests/150621/modules/bootloader_ofw/steps/7

Hi Dinar,

Can you test out the below patch in your setup. I have beaten it up
enough. I have changed the way interrupt pipe is polled and found added
multiple buffers per trb (that needs further proper changes).

Regards
Nikunj


    Fix xhci keyboard handling

Comments

Dinar Valeev April 28, 2016, 10:51 a.m. UTC | #1
On Thu, Apr 28, 2016 at 11:47 AM, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
> Dinar Valeev <k0da@opensuse.org> writes:
>
>> Hi,
>>
>> While trying to fix my usb over vnc automated issues, I switched to
>> xhci, which helped a lot OS wise, but I see a regression in SLOF.
>>
>> As you can see [1] we want to type an install repo url.  But at some
>> point capitalization of some characters are lost. Nope
>> openSUsE-tumbleweed instead of openSUSE-Tumbleweed.
>>
>> I was able to reproduce it even with simpler testcase by using vncdotool
>>
>> qemu-system-ppc64 -machine usb=off -vga std -enable-kvm -vnc :60 -m
>> 4096 -device nec-usb-xhci -device usb-tablet -device usb-kbd
>>
>> cat vnc.sh
>> vnc="vncdotool -s shiraz.arch:60"
>> $vnc type "HaHaHaHaHaHaHaHaHa"
>> $vnc key enter
>>
>> while true;do ./vnc.sh;done
>>
>> Three iterations are fine, starting with fourth characters are dropped.
>>
>> [1] https://openqa.opensuse.org/tests/150621/modules/bootloader_ofw/steps/7
>
> Hi Dinar,
>
> Can you test out the below patch in your setup. I have beaten it up
> enough. I have changed the way interrupt pipe is polled and found added
> multiple buffers per trb (that needs further proper changes).
Hi,
Wow! I don't see characters dropped anymore.
>
> Regards
> Nikunj
>
>
>     Fix xhci keyboard handling
Tested-by: Dinar Valeev <dvaleev@suse.com>
>
> diff --git a/lib/libusb/usb-xhci.c b/lib/libusb/usb-xhci.c
> index 858cd12..484ed9d 100644
> --- a/lib/libusb/usb-xhci.c
> +++ b/lib/libusb/usb-xhci.c
> @@ -238,7 +238,7 @@ static uint64_t xhci_poll_event(struct xhci_hcd *xhcd,
>         flags = le32_to_cpu(event->flags);
>
>         dprintf("Reading from event ptr %p %08x\n", event, flags);
> -       time = SLOF_GetTimer() + USB_TIMEOUT;
> +       time = SLOF_GetTimer() + (event_type == 1)? 0 : USB_TIMEOUT;
>
>         while ((flags & TRB_CYCLE_STATE) != xhcd->ering.cycle_state) {
>                 mb();
> @@ -1147,11 +1147,37 @@ static inline void *xhci_get_trb(struct xhci_seg *seg)
>         return (void *)enq;
>  }
>
> +static inline void *xhci_get_trb_deq(struct xhci_seg *seg)
> +{
> +       uint64_t val, deq;
> +       int index;
> +
> +       deq = val = seg->deq;
> +       val = val + XHCI_TRB_SIZE;
> +       index = (deq - (uint64_t)seg->trbs) / XHCI_TRB_SIZE + 1;
> +       dprintf("%s: enq %llx, val %llx %x\n", __func__, enq, val, index);
> +       /* TRBs being a cyclic buffer, here we cycle back to beginning. */
> +       if (index == (seg->size - 1)) {
> +               dprintf("%s: rounding \n", __func__);
> +               seg->deq = (uint64_t)seg->trbs;
> +               mb();
> +       }
> +       else {
> +               seg->deq = val;
> +       }
> +       return (void *)deq;
> +}
> +
>  static uint64_t xhci_get_trb_phys(struct xhci_seg *seg, uint64_t trb)
>  {
>         return seg->trbs_dma + (trb - (uint64_t)seg->trbs);
>  }
>
> +static uint32_t xhci_trb_get_index(struct xhci_seg *seg, struct xhci_transfer_trb *trb)
> +{
> +       return trb - (struct xhci_transfer_trb *)seg->trbs;
> +}
> +
>  static int usb_kb = false;
>  static int xhci_transfer_bulk(struct usb_pipe *pipe, void *td, void *td_phys,
>                         void *data, int datalen)
> @@ -1328,7 +1354,7 @@ static int xhci_get_pipe_intr(struct usb_pipe *pipe,
>                         return false;
>                 }
>         } else {
> -               xhci_init_seg(seg, XHCI_EVENT_TRBS_SIZE, TYPE_BULK);
> +               xhci_init_seg(seg, XHCI_INTR_TRBS_SIZE, TYPE_BULK);
>         }
>
>         xpipe->buf = buf;
> @@ -1349,7 +1375,8 @@ static int xhci_get_pipe_intr(struct usb_pipe *pipe,
>         xpipe->seg = seg;
>
>         trb = xhci_get_trb(seg);
> -       fill_normal_trb(trb, (void *)xpipe->buf_phys, pipe->mps);
> +       buf = xpipe->buf_phys + xhci_trb_get_index(seg, trb) * 8;
> +       fill_normal_trb(trb, (void *)buf, pipe->mps);
>         return true;
>  }
>
> @@ -1450,24 +1477,23 @@ static int xhci_poll_intr(struct usb_pipe *pipe, uint8_t *data)
>                 usb_kb = false;
>                 goto skip_poll;
>         }
> -       buf = xpipe->buf;
> -       memset(buf, 0, 8);
>
> -       mb();
>         /* Ring the doorbell - x_epno */
>         dbr = xhcd->db_regs;
>         write_reg32(&dbr->db[xdev->slot_id], x_epno);
> -       if (!xhci_poll_event(xhcd, 0)) {
> -               printf("poll intr failed\n");
> +       if (!xhci_poll_event(xhcd, 1)) {
>                 return 0;
>         }
>         mb();
> +       trb = xhci_get_trb_deq(seg);
> +       buf = xpipe->buf + xhci_trb_get_index(seg, trb) * 8;
>         memcpy(data, buf, 8);
> +       memset(buf, 0, 8);
>
>  skip_poll:
>         trb = xhci_get_trb(seg);
> -       fill_normal_trb(trb, (void *)xpipe->buf_phys, pipe->mps);
> -       mb();
> +       buf = xpipe->buf_phys + xhci_trb_get_index(seg, trb) * 8;
> +       fill_normal_trb(trb, (void *)buf, pipe->mps);
>         return ret;
>  }
>
> diff --git a/lib/libusb/usb-xhci.h b/lib/libusb/usb-xhci.h
> index 3fc7e78..3d2a73c 100644
> --- a/lib/libusb/usb-xhci.h
> +++ b/lib/libusb/usb-xhci.h
> @@ -266,7 +266,7 @@ struct xhci_seg {
>  #define XHCI_EVENT_TRBS_SIZE   4096
>  #define XHCI_CONTROL_TRBS_SIZE 4096
>  #define XHCI_DATA_TRBS_SIZE    4096
> -#define XHCI_INTR_TRBS_SIZE    4096
> +#define XHCI_INTR_TRBS_SIZE    256
>  #define XHCI_ERST_NUM_SEGS     1
>
>  #define XHCI_MAX_BULK_SIZE    0xF000
>
diff mbox

Patch

diff --git a/lib/libusb/usb-xhci.c b/lib/libusb/usb-xhci.c
index 858cd12..484ed9d 100644
--- a/lib/libusb/usb-xhci.c
+++ b/lib/libusb/usb-xhci.c
@@ -238,7 +238,7 @@  static uint64_t xhci_poll_event(struct xhci_hcd *xhcd,
 	flags = le32_to_cpu(event->flags);
 
 	dprintf("Reading from event ptr %p %08x\n", event, flags);
-	time = SLOF_GetTimer() + USB_TIMEOUT;
+	time = SLOF_GetTimer() + (event_type == 1)? 0 : USB_TIMEOUT;
 
 	while ((flags & TRB_CYCLE_STATE) != xhcd->ering.cycle_state) {
 		mb();
@@ -1147,11 +1147,37 @@  static inline void *xhci_get_trb(struct xhci_seg *seg)
 	return (void *)enq;
 }
 
+static inline void *xhci_get_trb_deq(struct xhci_seg *seg)
+{
+	uint64_t val, deq;
+	int index;
+
+	deq = val = seg->deq;
+	val = val + XHCI_TRB_SIZE;
+	index = (deq - (uint64_t)seg->trbs) / XHCI_TRB_SIZE + 1;
+	dprintf("%s: enq %llx, val %llx %x\n", __func__, enq, val, index);
+	/* TRBs being a cyclic buffer, here we cycle back to beginning. */
+	if (index == (seg->size - 1)) {
+		dprintf("%s: rounding \n", __func__);
+		seg->deq = (uint64_t)seg->trbs;
+		mb();
+	}
+	else {
+		seg->deq = val;
+	}
+	return (void *)deq;
+}
+
 static uint64_t xhci_get_trb_phys(struct xhci_seg *seg, uint64_t trb)
 {
 	return seg->trbs_dma + (trb - (uint64_t)seg->trbs);
 }
 
+static uint32_t xhci_trb_get_index(struct xhci_seg *seg, struct xhci_transfer_trb *trb)
+{
+	return trb - (struct xhci_transfer_trb *)seg->trbs;
+}
+
 static int usb_kb = false;
 static int xhci_transfer_bulk(struct usb_pipe *pipe, void *td, void *td_phys,
 			void *data, int datalen)
@@ -1328,7 +1354,7 @@  static int xhci_get_pipe_intr(struct usb_pipe *pipe,
 			return false;
 		}
 	} else {
-		xhci_init_seg(seg, XHCI_EVENT_TRBS_SIZE, TYPE_BULK);
+		xhci_init_seg(seg, XHCI_INTR_TRBS_SIZE, TYPE_BULK);
 	}
 
 	xpipe->buf = buf;
@@ -1349,7 +1375,8 @@  static int xhci_get_pipe_intr(struct usb_pipe *pipe,
 	xpipe->seg = seg;
 
 	trb = xhci_get_trb(seg);
-	fill_normal_trb(trb, (void *)xpipe->buf_phys, pipe->mps);
+	buf = xpipe->buf_phys + xhci_trb_get_index(seg, trb) * 8;
+	fill_normal_trb(trb, (void *)buf, pipe->mps);
 	return true;
 }
 
@@ -1450,24 +1477,23 @@  static int xhci_poll_intr(struct usb_pipe *pipe, uint8_t *data)
 		usb_kb = false;
 		goto skip_poll;
 	}
-	buf = xpipe->buf;
-	memset(buf, 0, 8);
 
-	mb();
 	/* Ring the doorbell - x_epno */
 	dbr = xhcd->db_regs;
 	write_reg32(&dbr->db[xdev->slot_id], x_epno);
-	if (!xhci_poll_event(xhcd, 0)) {
-		printf("poll intr failed\n");
+	if (!xhci_poll_event(xhcd, 1)) {
 		return 0;
 	}
 	mb();
+	trb = xhci_get_trb_deq(seg);
+	buf = xpipe->buf + xhci_trb_get_index(seg, trb) * 8;
 	memcpy(data, buf, 8);
+	memset(buf, 0, 8);
 
 skip_poll:
 	trb = xhci_get_trb(seg);
-	fill_normal_trb(trb, (void *)xpipe->buf_phys, pipe->mps);
-	mb();
+	buf = xpipe->buf_phys + xhci_trb_get_index(seg, trb) * 8;
+	fill_normal_trb(trb, (void *)buf, pipe->mps);
 	return ret;
 }
 
diff --git a/lib/libusb/usb-xhci.h b/lib/libusb/usb-xhci.h
index 3fc7e78..3d2a73c 100644
--- a/lib/libusb/usb-xhci.h
+++ b/lib/libusb/usb-xhci.h
@@ -266,7 +266,7 @@  struct xhci_seg {
 #define XHCI_EVENT_TRBS_SIZE   4096
 #define XHCI_CONTROL_TRBS_SIZE 4096
 #define XHCI_DATA_TRBS_SIZE    4096
-#define XHCI_INTR_TRBS_SIZE    4096
+#define XHCI_INTR_TRBS_SIZE    256
 #define XHCI_ERST_NUM_SEGS     1
 
 #define XHCI_MAX_BULK_SIZE    0xF000