Re: [Qemu-devel] [PATCH] uhci: improved TD matching, working ISOC transfers

[Aurelien Jarno <aurelien@aurel32.net>]

On Fri, Aug 22, 2008 at 05:54:53AM +0000, Max Krasnyansky wrote:
> While trying to make VX-3000 camera work on XP under KVM I realized that
> we do not necessarily have to find original TD address. All we care about
> is the token which identifies the transfer rather well (direction, endpoint,
> size, etc).
> This is especially important for the isochronous transfers because otherwise
> they are being canceled left and right and we do not make much progress.
> 
> With this patch all devices that used bulk transfers that I've tried so
> far continue to work just as well. And now my USB web cammera (isoc transfers)
> is working well tool. It's not as smooth as native Windows but it's pretty
> darn smooth.
> 
> The cool thing is that new USB code (both usb-uhci and usb-linux) is totaly
> generic and does not need any special logic for ISOC.
> 
> Signed-off-by: Max Krasnyansky <maxk@kernel.org>

Thanks, applied.

> ---
>  hw/usb-uhci.c |   40 ++++++++++++++++++++++++++++------------
>  1 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
> index 1b15074..0714520 100644
> --- a/hw/usb-uhci.c
> +++ b/hw/usb-uhci.c
> @@ -265,25 +265,41 @@ static void uhci_async_cancel_all(UHCIState *s)
>  static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t addr, uint32_t token)
>  {
>      UHCIAsync *async = s->async_pending;
> +    UHCIAsync *match = NULL;
> +    int count = 0;
> +
> +    /* 
> +     * We're looking for the best match here. ie both td addr and token.
> +     * Otherwise we return last good match. ie just token.
> +     * It's ok to match just token because it identifies the transaction
> +     * rather well, token includes: device addr, endpoint, size, etc.
> +     *
> +     * Also since we queue async transactions in reverse order by returning 
> +     * last good match we restores the order.
> +     *
> +     * It's expected that we wont have a ton of outstanding transactions.
> +     * If we ever do we'd want to optimize this algorithm. 
> +     */
>  
>      while (async) {
> -        if (async->td == addr) {
> -            if (async->token == token)
> -                return async;
> -
> -            /*
> -             * TD was reused for a different transfer.
> -             * Invalidate the original one asap.
> -             */
> -            if (async->valid > 0) {
> -                async->valid = 0;
> -                dprintf("husb: bad reuse. td 0x%x\n", async->td);
> +        if (async->token == token) {
> +            /* Good match */
> +            match = async;
> +
> +            if (async->td == addr) {
> +                /* Best match */
> +                break;
>              }
>          }
>  
>          async = async->next;
> +        count++;
>      }
> -    return NULL;
> +
> +    if (count > 64)
> +	fprintf(stderr, "uhci: warning lots of async transactions\n");
> +
> +    return match;
>  }
>  
>  static void uhci_attach(USBPort *port1, USBDevice *dev);
> -- 
> 1.5.5.1
> 
> 
> 
> 

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net