Patchwork audio streaming from usb devices

login
register
mail settings
Submitter David S. Ahern
Date Feb. 3, 2010, 3:49 p.m.
Message ID <4B699B13.60802@cisco.com>
Download mbox | patch
Permalink /patch/44385/
State New
Headers show

Comments

David S. Ahern - Feb. 3, 2010, 3:49 p.m.
I have streaming audio devices working within qemu-kvm. This is a port
of the changes to qemu.

Streaming audio generates a series of isochronous requests that are
repetitive and time sensitive. The URBs need to be submitted in
consecutive USB frames and responses need to be handled in a timely manner.

Summary of the changes for isochronous requests:

1. The initial 'valid' value is increased to 32. It needs to be higher
than its current value of 10 since the host adds a 10 frame delay to the
scheduling of the first request; if valid is set to 10 the first
isochronous request times out and qemu cancels it. 32 was chosen as a
nice round number, and it is used in the path where a TD-async pairing
already exists.

2. The token field in the TD is *not* unique for isochronous requests,
so it is not a good choice for finding a matching async request. The
buffer (where to write the guest data) is unique, so use that value instead.

3. TD's for isochronous request need to be completed in the async
completion handler so that data is pushed to the guest as soon as it is
available. The uhci code currently attempts to process complete
isochronous TDs the next time the UHCI frame with the request is
processed. The results in lost data since the async requests will have
long since timed out based on the valid parameter. Increasing the valid
value is not acceptable as it introduces a 1+ second delay in the data
getting pushed to the guest.

4. The frame timer needs to be run on 1 msec intervals. Currently, the
expire time for the processing the next frame is computed after the
processing of each frame. This regularly causes the scheduling of frames
to shift in time. When this happens the periodic scheduling of the
requests is broken and the subsequent request is seen as a new request
by the host resulting in a 10 msec delay (first isochronous request is
scheduled for 10 frames from when the URB is submitted).


[ For what's worth a small change is needed to the guest driver to have
more outstanding URBs (at least 4 URBs with 5 packets per URB).]

Signed-off-by: David Ahern <daahern@cisco.com>
David S. Ahern - Feb. 9, 2010, 2:11 p.m.
I have not seen a response to this. If there are no objections please apply.

Thanks,

David Ahern


On 02/03/2010 08:49 AM, David S. Ahern wrote:
> 
> I have streaming audio devices working within qemu-kvm. This is a port
> of the changes to qemu.
> 
> Streaming audio generates a series of isochronous requests that are
> repetitive and time sensitive. The URBs need to be submitted in
> consecutive USB frames and responses need to be handled in a timely manner.
> 
> Summary of the changes for isochronous requests:
> 
> 1. The initial 'valid' value is increased to 32. It needs to be higher
> than its current value of 10 since the host adds a 10 frame delay to the
> scheduling of the first request; if valid is set to 10 the first
> isochronous request times out and qemu cancels it. 32 was chosen as a
> nice round number, and it is used in the path where a TD-async pairing
> already exists.
> 
> 2. The token field in the TD is *not* unique for isochronous requests,
> so it is not a good choice for finding a matching async request. The
> buffer (where to write the guest data) is unique, so use that value instead.
> 
> 3. TD's for isochronous request need to be completed in the async
> completion handler so that data is pushed to the guest as soon as it is
> available. The uhci code currently attempts to process complete
> isochronous TDs the next time the UHCI frame with the request is
> processed. The results in lost data since the async requests will have
> long since timed out based on the valid parameter. Increasing the valid
> value is not acceptable as it introduces a 1+ second delay in the data
> getting pushed to the guest.
> 
> 4. The frame timer needs to be run on 1 msec intervals. Currently, the
> expire time for the processing the next frame is computed after the
> processing of each frame. This regularly causes the scheduling of frames
> to shift in time. When this happens the periodic scheduling of the
> requests is broken and the subsequent request is seen as a new request
> by the host resulting in a 10 msec delay (first isochronous request is
> scheduled for 10 frames from when the URB is submitted).
> 
> 
> [ For what's worth a small change is needed to the guest driver to have
> more outstanding URBs (at least 4 URBs with 5 packets per URB).]
> 
> Signed-off-by: David Ahern <daahern@cisco.com>
> 
>
Anthony Liguori - Feb. 10, 2010, 7:28 p.m.
On 02/03/2010 09:49 AM, David S. Ahern wrote:
> I have streaming audio devices working within qemu-kvm. This is a port
> of the changes to qemu.
>
> Streaming audio generates a series of isochronous requests that are
> repetitive and time sensitive. The URBs need to be submitted in
> consecutive USB frames and responses need to be handled in a timely manner.
>
> Summary of the changes for isochronous requests:
>
> 1. The initial 'valid' value is increased to 32. It needs to be higher
> than its current value of 10 since the host adds a 10 frame delay to the
> scheduling of the first request; if valid is set to 10 the first
> isochronous request times out and qemu cancels it. 32 was chosen as a
> nice round number, and it is used in the path where a TD-async pairing
> already exists.
>
> 2. The token field in the TD is *not* unique for isochronous requests,
> so it is not a good choice for finding a matching async request. The
> buffer (where to write the guest data) is unique, so use that value instead.
>
> 3. TD's for isochronous request need to be completed in the async
> completion handler so that data is pushed to the guest as soon as it is
> available. The uhci code currently attempts to process complete
> isochronous TDs the next time the UHCI frame with the request is
> processed. The results in lost data since the async requests will have
> long since timed out based on the valid parameter. Increasing the valid
> value is not acceptable as it introduces a 1+ second delay in the data
> getting pushed to the guest.
>
> 4. The frame timer needs to be run on 1 msec intervals. Currently, the
> expire time for the processing the next frame is computed after the
> processing of each frame. This regularly causes the scheduling of frames
> to shift in time. When this happens the periodic scheduling of the
> requests is broken and the subsequent request is seen as a new request
> by the host resulting in a 10 msec delay (first isochronous request is
> scheduled for 10 frames from when the URB is submitted).
>
>
> [ For what's worth a small change is needed to the guest driver to have
> more outstanding URBs (at least 4 URBs with 5 packets per URB).]
>
> Signed-off-by: David Ahern<daahern@cisco.com>
>    

Applied.  Thanks.

Regards,

Anthony Liguori

Patch

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index fdbb4d1..19b4ce6 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -112,6 +112,7 @@  typedef struct UHCIAsync {
     uint32_t  td;
     uint32_t  token;
     int8_t    valid;
+    uint8_t   isoc;
     uint8_t   done;
     uint8_t   buffer[2048];
 } UHCIAsync;
@@ -131,6 +132,7 @@  typedef struct UHCIState {
     uint32_t fl_base_addr; /* frame list base address */
     uint8_t sof_timing;
     uint8_t status2; /* bit 0 and 1 are used to generate UHCI_STS_USBINT */
+    int64_t expire_time;
     QEMUTimer *frame_timer;
     UHCIPort ports[NB_PORTS];
 
@@ -164,6 +166,7 @@  static UHCIAsync *uhci_async_alloc(UHCIState *s)
     async->td    = 0;
     async->token = 0;
     async->done  = 0;
+    async->isoc  = 0;
     async->next  = NULL;
 
     return async;
@@ -762,13 +765,25 @@  static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, uint32_t *in
 {
     UHCIAsync *async;
     int len = 0, max_len;
-    uint8_t pid;
+    uint8_t pid, isoc;
+    uint32_t token;
 
     /* Is active ? */
     if (!(td->ctrl & TD_CTRL_ACTIVE))
         return 1;
 
-    async = uhci_async_find_td(s, addr, td->token);
+    /* token field is not unique for isochronous requests,
+     * so use the destination buffer 
+     */
+    if (td->ctrl & TD_CTRL_IOS) {
+        token = td->buffer;
+        isoc = 1;
+    } else {
+        token = td->token;
+        isoc = 0;
+    }
+
+    async = uhci_async_find_td(s, addr, token);
     if (async) {
         /* Already submitted */
         async->valid = 32;
@@ -785,9 +800,13 @@  static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, uint32_t *in
     if (!async)
         return 1;
 
-    async->valid = 10;
+    /* valid needs to be large enough to handle 10 frame delay
+     * for initial isochronous requests
+     */
+    async->valid = 32;
     async->td    = addr;
-    async->token = td->token;
+    async->token = token;
+    async->isoc  = isoc;
 
     max_len = ((td->token >> 21) + 1) & 0x7ff;
     pid = td->token & 0xff;
@@ -841,9 +860,31 @@  static void uhci_async_complete(USBPacket *packet, void *opaque)
 
     dprintf("uhci: async complete. td 0x%x token 0x%x\n", async->td, async->token);
 
-    async->done = 1;
+    if (async->isoc) {
+        UHCI_TD td;
+        uint32_t link = async->td;
+        uint32_t int_mask = 0, val;
+        int len;
+ 
+        cpu_physical_memory_read(link & ~0xf, (uint8_t *) &td, sizeof(td));
+        le32_to_cpus(&td.link);
+        le32_to_cpus(&td.ctrl);
+        le32_to_cpus(&td.token);
+        le32_to_cpus(&td.buffer);
+
+        uhci_async_unlink(s, async);
+        len = uhci_complete_td(s, &td, async, &int_mask);
+        s->pending_int_mask |= int_mask;
 
-    uhci_process_frame(s);
+        /* update the status bits of the TD */
+        val = cpu_to_le32(td.ctrl);
+        cpu_physical_memory_write((link & ~0xf) + 4,
+                                  (const uint8_t *)&val, sizeof(val));
+        uhci_async_free(s, async);
+    } else {
+        async->done = 1;
+        uhci_process_frame(s);
+    }
 }
 
 static int is_valid(uint32_t link)
@@ -1005,13 +1046,15 @@  static void uhci_process_frame(UHCIState *s)
         /* go to the next entry */
     }
 
-    s->pending_int_mask = int_mask;
+    s->pending_int_mask |= int_mask;
 }
 
 static void uhci_frame_timer(void *opaque)
 {
     UHCIState *s = opaque;
-    int64_t expire_time;
+
+    /* prepare the timer for the next frame */
+    s->expire_time += (get_ticks_per_sec() / FRAME_TIMER_FREQ);
 
     if (!(s->cmd & UHCI_CMD_RS)) {
         /* Full stop */
@@ -1029,6 +1072,7 @@  static void uhci_frame_timer(void *opaque)
         s->status  |= UHCI_STS_USBINT;
         uhci_update_irq(s);
     }
+    s->pending_int_mask = 0;
 
     /* Start new frame */
     s->frnum = (s->frnum + 1) & 0x7ff;
@@ -1041,10 +1085,7 @@  static void uhci_frame_timer(void *opaque)
 
     uhci_async_validate_end(s);
 
-    /* prepare the timer for the next frame */
-    expire_time = qemu_get_clock(vm_clock) +
-        (get_ticks_per_sec() / FRAME_TIMER_FREQ);
-    qemu_mod_timer(s->frame_timer, expire_time);
+    qemu_mod_timer(s->frame_timer, s->expire_time);
 }
 
 static void uhci_map(PCIDevice *pci_dev, int region_num,
@@ -1078,6 +1119,8 @@  static int usb_uhci_common_initfn(UHCIState *s)
         usb_register_port(&s->bus, &s->ports[i].port, s, i, uhci_attach);
     }
     s->frame_timer = qemu_new_timer(vm_clock, uhci_frame_timer, s);
+    s->expire_time = qemu_get_clock(vm_clock) +
+        (get_ticks_per_sec() / FRAME_TIMER_FREQ);
     s->num_ports_vmstate = NB_PORTS;
 
     qemu_register_reset(uhci_reset, s);