[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: Mauro Carvalho Chehab <m.chehab@samsung.com>
This commit is contained in:
		
					parent
					
						
							
								439c491c4c
							
						
					
				
			
			
				commit
				
					
						1b3fd2d342
					
				
			
		
					 2 changed files with 111 additions and 38 deletions
				
			
		| 
						 | 
					@ -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)
 | 
				
			||||||
	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)
 | 
				
			||||||
	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)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	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)
 | 
				
			||||||
	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)
 | 
				
			||||||
		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)
 | 
				
			||||||
		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)
 | 
				
			||||||
		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)
 | 
							for (j = k = 0; j < npackets; j++, k += ep_size) {
 | 
				
			||||||
			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;
 | 
					 | 
				
			||||||
			     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)
 | 
				
			||||||
		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 {
 | 
				
			||||||
	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 {
 | 
				
			||||||
 | 
					
 | 
				
			||||||
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;
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue