Commit 1b3fd2d3 authored by Mauro Carvalho Chehab's avatar Mauro Carvalho Chehab

[media] em28xx-audio: don't hardcode audio URB calculus

The current code hardcodes the number of audio URBs, the number
of packets per URB and the maximum URB size.

This is not a good idea, as it:

- wastes more bandwidth than necessary, by using a very
  large number of packets;

- those constants are bound to an specific scenario, with
  a bandwidth of 48 kHz;

- don't take the maximum endpoint size into account;

- with urb->interval = 1 on xHCI, those constraints cause a "funny"
  setup: URBs with 64 packets inside, with only 24 bytes total. E. g.
  a complete waste of space.

Change the code to do dynamic URB audio calculus and allocation.

For now, use the same constraints as used before this patch, to
avoid regressions.

A good scenario (tested) seems to use those defines, instead:

	#define EM28XX_MAX_AUDIO_BUFS          8
	#define EM28XX_MIN_AUDIO_PACKETS       2

But let's not do such change here, letting the optimization to
happen on latter patches, after more tests.
Signed-off-by: default avatarMauro Carvalho Chehab <m.chehab@samsung.com>
parent 439c491c
...@@ -50,6 +50,8 @@ static int debug; ...@@ -50,6 +50,8 @@ static int debug;
module_param(debug, int, 0644); module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "activates debug info"); MODULE_PARM_DESC(debug, "activates debug info");
#define EM28XX_MAX_AUDIO_BUFS 5
#define EM28XX_MIN_AUDIO_PACKETS 64
#define dprintk(fmt, arg...) do { \ #define dprintk(fmt, arg...) do { \
if (debug) \ if (debug) \
printk(KERN_INFO "em28xx-audio %s: " fmt, \ printk(KERN_INFO "em28xx-audio %s: " fmt, \
...@@ -63,7 +65,7 @@ static int em28xx_deinit_isoc_audio(struct em28xx *dev) ...@@ -63,7 +65,7 @@ static int em28xx_deinit_isoc_audio(struct em28xx *dev)
int i; int i;
dprintk("Stopping isoc\n"); dprintk("Stopping isoc\n");
for (i = 0; i < EM28XX_AUDIO_BUFS; i++) { for (i = 0; i < dev->adev.num_urb; i++) {
struct urb *urb = dev->adev.urb[i]; struct urb *urb = dev->adev.urb[i];
if (!irqs_disabled()) if (!irqs_disabled())
...@@ -168,7 +170,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev) ...@@ -168,7 +170,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev)
dprintk("Starting isoc transfers\n"); dprintk("Starting isoc transfers\n");
/* Start streaming */ /* Start streaming */
for (i = 0; i < EM28XX_AUDIO_BUFS; i++) { for (i = 0; i < dev->adev.num_urb; i++) {
memset(dev->adev.transfer_buffer[i], 0x80, memset(dev->adev.transfer_buffer[i], 0x80,
dev->adev.urb[i]->transfer_buffer_length); dev->adev.urb[i]->transfer_buffer_length);
...@@ -598,21 +600,35 @@ static void em28xx_audio_free_urb(struct em28xx *dev) ...@@ -598,21 +600,35 @@ static void em28xx_audio_free_urb(struct em28xx *dev)
{ {
int i; int i;
for (i = 0; i < EM28XX_AUDIO_BUFS; i++) { for (i = 0; i < dev->adev.num_urb; i++) {
struct urb *urb = dev->adev.urb[i]; struct urb *urb = dev->adev.urb[i];
if (!dev->adev.urb[i]) if (!urb)
continue; continue;
usb_free_coherent(dev->udev, if (dev->adev.transfer_buffer[i])
urb->transfer_buffer_length, usb_free_coherent(dev->udev,
dev->adev.transfer_buffer[i], urb->transfer_buffer_length,
urb->transfer_dma); dev->adev.transfer_buffer[i],
urb->transfer_dma);
usb_free_urb(urb); usb_free_urb(urb);
dev->adev.urb[i] = NULL;
dev->adev.transfer_buffer[i] = NULL;
} }
kfree(dev->adev.urb);
kfree(dev->adev.transfer_buffer);
dev->adev.num_urb = 0;
}
/* high bandwidth multiplier, as encoded in highspeed endpoint descriptors */
static int em28xx_audio_ep_packet_size(struct usb_device *udev,
struct usb_endpoint_descriptor *e)
{
int size = le16_to_cpu(e->wMaxPacketSize);
if (udev->speed == USB_SPEED_HIGH)
return (size & 0x7ff) * (1 + (((size) >> 11) & 0x03));
return size & 0x7ff;
} }
static int em28xx_audio_init(struct em28xx *dev) static int em28xx_audio_init(struct em28xx *dev)
...@@ -623,9 +639,8 @@ static int em28xx_audio_init(struct em28xx *dev) ...@@ -623,9 +639,8 @@ static int em28xx_audio_init(struct em28xx *dev)
struct usb_interface *intf; struct usb_interface *intf;
struct usb_endpoint_descriptor *e, *ep = NULL; struct usb_endpoint_descriptor *e, *ep = NULL;
static int devnr; static int devnr;
int err, i; int err, i, ep_size, interval, num_urb, npackets;
const int sb_size = EM28XX_NUM_AUDIO_PACKETS * int urb_size, bytes_per_transfer;
EM28XX_AUDIO_MAX_PACKET_SIZE;
u8 alt; u8 alt;
if (!dev->has_alsa_audio || dev->audio_ifnum < 0) { if (!dev->has_alsa_audio || dev->audio_ifnum < 0) {
...@@ -648,6 +663,9 @@ static int em28xx_audio_init(struct em28xx *dev) ...@@ -648,6 +663,9 @@ static int em28xx_audio_init(struct em28xx *dev)
return err; return err;
spin_lock_init(&adev->slock); spin_lock_init(&adev->slock);
adev->sndcard = card;
adev->udev = dev->udev;
err = snd_pcm_new(card, "Em28xx Audio", 0, 0, 1, &pcm); err = snd_pcm_new(card, "Em28xx Audio", 0, 0, 1, &pcm);
if (err < 0) { if (err < 0) {
snd_card_free(card); snd_card_free(card);
...@@ -712,25 +730,92 @@ static int em28xx_audio_init(struct em28xx *dev) ...@@ -712,25 +730,92 @@ static int em28xx_audio_init(struct em28xx *dev)
return -ENODEV; return -ENODEV;
} }
/* Alloc URB and transfer buffers */ ep_size = em28xx_audio_ep_packet_size(dev->udev, ep);
for (i = 0; i < EM28XX_AUDIO_BUFS; i++) { interval = 1 << (ep->bInterval - 1);
em28xx_info("Endpoint 0x%02x %s on intf %d alt %d interval = %d, size %d\n",
EM28XX_EP_AUDIO, usb_speed_string(dev->udev->speed),
dev->audio_ifnum, alt,
interval,
ep_size);
/* Calculate the number and size of URBs to better fit the audio samples */
/*
* Estimate the number of bytes per DMA transfer.
*
* This is given by the bit rate (for now, only 48000 Hz) multiplied
* by 2 channels and 2 bytes/sample divided by the number of microframe
* intervals and by the microframe rate (125 us)
*/
bytes_per_transfer = DIV_ROUND_UP(48000 * 2 * 2, 125 * interval);
/*
* Estimate the number of transfer URBs. Don't let it go past the
* maximum number of URBs that is known to be supported by the device.
*/
num_urb = DIV_ROUND_UP(bytes_per_transfer, ep_size);
if (num_urb > EM28XX_MAX_AUDIO_BUFS)
num_urb = EM28XX_MAX_AUDIO_BUFS;
/*
* Now that we know the number of bytes per transfer and the number of
* URBs, estimate the typical size of an URB, in order to adjust the
* minimal number of packets.
*/
urb_size = bytes_per_transfer / num_urb;
/*
* Now, calculate the amount of audio packets to be filled on each
* URB. In order to preserve the old behaviour, use a minimal
* threshold for this value.
*/
npackets = EM28XX_MIN_AUDIO_PACKETS;
if (urb_size > ep_size * npackets)
npackets = DIV_ROUND_UP(urb_size, ep_size);
em28xx_info("Number of URBs: %d, with %d packets and %d size",
num_urb, npackets, urb_size);
/* Allocate space to store the number of URBs to be used */
dev->adev.transfer_buffer = kcalloc(num_urb,
sizeof(*dev->adev.transfer_buffer),
GFP_ATOMIC);
if (!dev->adev.transfer_buffer) {
snd_card_free(card);
return -ENOMEM;
}
dev->adev.urb = kcalloc(num_urb, sizeof(*dev->adev.urb), GFP_ATOMIC);
if (!dev->adev.urb) {
snd_card_free(card);
kfree(dev->adev.transfer_buffer);
return -ENOMEM;
}
/* Alloc memory for each URB and for each transfer buffer */
dev->adev.num_urb = num_urb;
for (i = 0; i < num_urb; i++) {
struct urb *urb; struct urb *urb;
int j, k; int j, k;
void *buf; void *buf;
urb = usb_alloc_urb(EM28XX_NUM_AUDIO_PACKETS, GFP_ATOMIC); urb = usb_alloc_urb(npackets, GFP_ATOMIC);
if (!urb) { if (!urb) {
em28xx_errdev("usb_alloc_urb failed!\n"); em28xx_errdev("usb_alloc_urb failed!\n");
em28xx_audio_free_urb(dev); em28xx_audio_free_urb(dev);
snd_card_free(card);
return -ENOMEM; return -ENOMEM;
} }
dev->adev.urb[i] = urb; dev->adev.urb[i] = urb;
buf = usb_alloc_coherent(dev->udev, sb_size, GFP_ATOMIC, buf = usb_alloc_coherent(dev->udev, npackets * ep_size, GFP_ATOMIC,
&urb->transfer_dma); &urb->transfer_dma);
if (!buf) { if (!buf) {
em28xx_errdev("usb_alloc_coherent failed!\n"); em28xx_errdev("usb_alloc_coherent failed!\n");
em28xx_audio_free_urb(dev); em28xx_audio_free_urb(dev);
snd_card_free(card);
return -ENOMEM; return -ENOMEM;
} }
dev->adev.transfer_buffer[i] = buf; dev->adev.transfer_buffer[i] = buf;
...@@ -739,23 +824,15 @@ static int em28xx_audio_init(struct em28xx *dev) ...@@ -739,23 +824,15 @@ static int em28xx_audio_init(struct em28xx *dev)
urb->context = dev; urb->context = dev;
urb->pipe = usb_rcvisocpipe(dev->udev, EM28XX_EP_AUDIO); urb->pipe = usb_rcvisocpipe(dev->udev, EM28XX_EP_AUDIO);
urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP; urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
urb->transfer_buffer = dev->adev.transfer_buffer[i]; urb->transfer_buffer = buf;
urb->interval = 1 << (ep->bInterval - 1); urb->interval = interval;
urb->complete = em28xx_audio_isocirq; urb->complete = em28xx_audio_isocirq;
urb->number_of_packets = EM28XX_NUM_AUDIO_PACKETS; urb->number_of_packets = npackets;
urb->transfer_buffer_length = sb_size; urb->transfer_buffer_length = ep_size * npackets;
if (!i)
dprintk("Will use ep 0x%02x on intf %d alt %d interval = %d (rcv isoc pipe: 0x%08x)\n",
EM28XX_EP_AUDIO, dev->audio_ifnum, alt,
urb->interval,
urb->pipe);
for (j = k = 0; j < EM28XX_NUM_AUDIO_PACKETS; for (j = k = 0; j < npackets; j++, k += ep_size) {
j++, k += EM28XX_AUDIO_MAX_PACKET_SIZE) {
urb->iso_frame_desc[j].offset = k; urb->iso_frame_desc[j].offset = k;
urb->iso_frame_desc[j].length = urb->iso_frame_desc[j].length = ep_size;
EM28XX_AUDIO_MAX_PACKET_SIZE;
} }
} }
...@@ -765,8 +842,6 @@ static int em28xx_audio_init(struct em28xx *dev) ...@@ -765,8 +842,6 @@ static int em28xx_audio_init(struct em28xx *dev)
snd_card_free(card); snd_card_free(card);
return err; return err;
} }
adev->sndcard = card;
adev->udev = dev->udev;
em28xx_info("Audio extension successfully initialized\n"); em28xx_info("Audio extension successfully initialized\n");
return 0; return 0;
......
...@@ -481,9 +481,6 @@ struct em28xx_eeprom { ...@@ -481,9 +481,6 @@ struct em28xx_eeprom {
u8 string_idx_table; u8 string_idx_table;
}; };
#define EM28XX_AUDIO_BUFS 5
#define EM28XX_NUM_AUDIO_PACKETS 64
#define EM28XX_AUDIO_MAX_PACKET_SIZE 196 /* static value */
#define EM28XX_CAPTURE_STREAM_EN 1 #define EM28XX_CAPTURE_STREAM_EN 1
/* em28xx extensions */ /* em28xx extensions */
...@@ -498,8 +495,9 @@ struct em28xx_eeprom { ...@@ -498,8 +495,9 @@ struct em28xx_eeprom {
struct em28xx_audio { struct em28xx_audio {
char name[50]; char name[50];
char *transfer_buffer[EM28XX_AUDIO_BUFS]; unsigned num_urb;
struct urb *urb[EM28XX_AUDIO_BUFS]; char **transfer_buffer;
struct urb **urb;
struct usb_device *udev; struct usb_device *udev;
unsigned int capture_transfer_done; unsigned int capture_transfer_done;
struct snd_pcm_substream *capture_pcm_substream; struct snd_pcm_substream *capture_pcm_substream;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment