Hi,
Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx> writes:
> Immediate data transfers (IDT) allow the HCD to copy small chunks of
> data (up to 8bits) directly into its output transfer TRBs. This avoids
^^^^^
8 bytes
> the somewhat expensive DMA mappings that are performed by default on
> most URBs submissions.
>
> In the case an URB was suitable for IDT. The data is directly copied
> into the "Data Buffer Pointer" region of the TRB and the IDT flag is
> set. Instead of triggering memory accesses the HC will use the data
> directly.
>
> An additional URB flag was created to mark whenever the URB doesn't need
> any DMA mapping. Ideally it would have been nice to use a private flag
> as this is specific to XHCI. Yet it's not possible as the URB private
> area is allocated only after the DMA mapping is done.
>
> Isochronous transfers are not implemented so far as it's hard to find a
> device that will send such small packets.
>
> This was tested using USB/serial adapter and by controlling the leds on
> an XBOX controller. There where no disruptions on the rest of USB
> devices attached on the system.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
> ---
> drivers/usb/host/xhci-ring.c | 6 ++++++
> drivers/usb/host/xhci.c | 37 ++++++++++++++++++++++++++++++++++++
> drivers/usb/host/xhci.h | 2 ++
> include/linux/usb.h | 2 ++
> 4 files changed, 47 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 40fa25c4d041..dd9805fb0566 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3272,6 +3272,12 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> field |= TRB_IOC;
> more_trbs_coming = false;
> td->last_trb = ring->enqueue;
> +
> + if (urb->transfer_flags & URB_NO_DMA_MAP) {
do you really need the flag? Why don't you do this unconditionally as
long as urb->transfer_length is <= 8?
> + memcpy(&send_addr, urb->transfer_buffer,
> + full_len);
> + field |= TRB_IDT;
> + }
> }
>
> /* Only set interrupt on short packet for IN endpoints */
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 005e65922608..ce3b6663f940 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1238,6 +1238,41 @@ EXPORT_SYMBOL_GPL(xhci_resume);
>
> /*-------------------------------------------------------------------------*/
>
> +static void xhci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
> +{
> + if (urb->transfer_flags & URB_NO_DMA_MAP)
> + urb->transfer_flags &= ~URB_NO_DMA_MAP;
> + else
> + usb_hcd_unmap_urb_for_dma(hcd, urb);
> +}
> +
> +static int xhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
> + gfp_t mem_flags)
> +{
> + int maxp = usb_endpoint_maxp(&urb->ep->desc);
> + int len = urb->transfer_buffer_length;
> + int ret = 0;
> +
> + /*
> + * Checks if URB is suitable for Immediate Transfer (IDT): instead of
> + * mapping the buffer for DMA and passing the address to the host
> + * controller, we copy the actual data into the TRB address register.
> + * This is limited to transfers up to 8 bytes.
> + *
> + * IDT is only supported for Bulk and Interrupt endpoints. Apart from
> + * the size constraints special care is taken to avoid cases where
> + * wMaxPacketSize is smaller than 8 bytes as it's not supported.
> + */
> + if ((usb_endpoint_is_int_out(&urb->ep->desc) ||
> + usb_endpoint_is_bulk_out(&urb->ep->desc)) &&
I don't understand the check for endpoint type. IDT is, actually,
already used for control endpoints because setup packets are composed of
8 bytes. You're also showing that this works for INT and BULK types. It
would be a surprise if it doesn't work for ISOC.
> + maxp >= TRB_IDT_MAX_SIZE && len <= TRB_IDT_MAX_SIZE)
why the maxp check? What if I'm an interrupt endpoint with maxp of 2?
Is there really a limitation that you couldn't use IDT for those?
> + urb->transfer_flags |= URB_NO_DMA_MAP;
do we really need this? I wonder if returning zero here would be enough,
then you could:
...map_urb_for_dma(...)
{
ret = 0;
if (urb->transfer_length > 8)
ret = usb_hcd_map_urb_for_dma(hcd, urb, flags);
return ret;
}
...unmap_urb_for_dma(...)
{
if urb->transfer_length > 8)
usb_hcd_unmap_urb_for_dma(hcd, urb);
}
...xhci_queue_bulk_tx(...)
{
[...]
if (urb->transfer_length <= 8) {
memcpy(&addr, urb->transfer_buffer, 8)l
field |= TRB_IDT;
}
[...]
}
--
balbi
Attachment:
signature.asc
Description: PGP signature
