[PATCH v3 4/4] usb: gadget: f_uac2: send reasonably sized packets

2014-08-26 Thread Daniel Mack
The UAC2 function driver currently responds to all packets at all times
with wMaxPacketSize packets. That results in way too fast audio
playback as the function driver (which is in fact supposed to define
the audio stream pace) delivers as fast as it can.

Fix this by pre-calculating the size of each packet to meet the
requested sample rate and format. As a simple division won't allow
for exact data rates, prepare a sequence of packet lengths, and
alternate over them when sending the IN packets.

For example, with 44100 Hz and stereo, 16-bit samples, the nominal
data rate is 176400 bytes/s. With a simple division by the number
of available frames (1000), we would be off by 400 bytes, or 100
samples per second. By preparing a sequence of lengths, we can make
each 10th packet accomodate one more frame, which results in 400 more
bytes/s.

Signed-off-by: Daniel Mack zon...@gmail.com
---
 drivers/usb/gadget/function/f_uac2.c | 68 ++--
 1 file changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac2.c 
b/drivers/usb/gadget/function/f_uac2.c
index a18f147..40c501a 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -92,6 +92,10 @@ struct snd_uac2_chip {
 
struct snd_card *card;
struct snd_pcm *pcm;
+
+   u16 p_pktsize_seq[10];
+   unsigned int p_pktsize_seq_len;
+   unsigned int cur_p_pktsize;
 };
 
 #define BUFF_SIZE_MAX  (PAGE_SIZE * 16)
@@ -185,8 +189,11 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request 
*req)
 
spin_lock_irqsave(prm-lock, flags);
 
-   if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK)
-   req-actual = req-length;
+   if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   /* alternate over the package size sequence */
+   req-length = uac2-p_pktsize_seq[uac2-cur_p_pktsize++];
+   uac2-cur_p_pktsize %= uac2-p_pktsize_seq_len;
+   }
 
pending = prm-hw_ptr % prm-period_size;
pending += req-actual;
@@ -340,6 +347,7 @@ static int uac2_pcm_open(struct snd_pcm_substream 
*substream)
c_srate = opts-c_srate;
p_chmask = opts-p_chmask;
c_chmask = opts-c_chmask;
+   uac2-cur_p_pktsize = 0;
 
runtime-hw = uac2_pcm_hardware;
 
@@ -1060,6 +1068,61 @@ err:
return -EINVAL;
 }
 
+static void
+afunc_set_p_pktsize(struct usb_gadget *gadget, struct audio_dev *agdev)
+{
+   unsigned i, residue, rate, factor, interval, framesize, pktsize, len;
+   struct snd_uac2_chip *uac2 = agdev-uac2;
+   struct usb_endpoint_descriptor *ep_desc;
+   struct f_uac2_opts *opts =
+   container_of(agdev-func.fi, struct f_uac2_opts, func_inst);
+
+   if (gadget-speed == USB_SPEED_FULL) {
+   ep_desc = fs_epin_desc;
+   factor = 1000;
+   } else {
+   ep_desc = hs_epin_desc;
+   factor = 125;
+   }
+
+   /*
+* Simply dividing the desired data rate by the number of available
+* packets per seconds give rounding errors. Hence, we prepare a
+* sequence of packet sizes that will be alternated over when sending
+* the packets to the IN endpoint.
+*/
+   framesize = opts-p_ssize * num_channels(opts-p_chmask);
+   interval = (1  (ep_desc-bInterval - 1)) * factor;
+   rate = opts-p_srate * framesize;
+
+   pktsize = min_t(unsigned, rate / interval, ep_desc-wMaxPacketSize);
+
+   /*
+* If there's a residue from the modulo operation, calculate the index
+* of the first packet that could be increased by one sample frame.
+* This will be our sequence length, and the last member carries one
+* frame more than the others. Hence, the shorter the sequence is, the
+* more bytes will be transferred.
+*/
+   residue = (rate % interval) / framesize;
+   if (residue)
+   len = min_t(unsigned, interval / residue,
+   ARRAY_SIZE(uac2-p_pktsize_seq));
+   else
+   len = 1;
+
+   /* Set all lengths in the sequence to the divided value ... */
+   for (i = 0; i  len; i++)
+   uac2-p_pktsize_seq[i] = pktsize;
+
+   /* ... and add another frame to the last sequence member */
+   if (len  1)
+   uac2-p_pktsize_seq[len - 1] += framesize;
+
+   uac2-p_pktsize_seq_len = len;
+   uac2-cur_p_pktsize = 0;
+}
+
 static int
 afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
 {
@@ -1098,6 +1161,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, 
unsigned alt)
prm = uac2-p_prm;
config_ep_by_speed(gadget, fn, ep);
agdev-as_in_alt = alt;
+   afunc_set_p_pktsize(gadget, agdev);
} else {
dev_err(dev, %s:%d Error!\n, __func__, __LINE__);
return -EINVAL;
-- 
2.1.0

--
To 

Re: [PATCH v3 4/4] usb: gadget: f_uac2: send reasonably sized packets

2014-08-26 Thread Jassi Brar
On Tue, Aug 26, 2014 at 1:46 PM, Daniel Mack zon...@gmail.com wrote:

 +static void
 +afunc_set_p_pktsize(struct usb_gadget *gadget, struct audio_dev *agdev)
 +{
 +   unsigned i, residue, rate, factor, interval, framesize, pktsize, len;
 +   struct snd_uac2_chip *uac2 = agdev-uac2;
 +   struct usb_endpoint_descriptor *ep_desc;
 +   struct f_uac2_opts *opts =
 +   container_of(agdev-func.fi, struct f_uac2_opts, func_inst);
 +
 +   if (gadget-speed == USB_SPEED_FULL) {
 +   ep_desc = fs_epin_desc;
 +   factor = 1000;
 +   } else {
 +   ep_desc = hs_epin_desc;
 +   factor = 125;
 +   }
 +
 +   /*
 +* Simply dividing the desired data rate by the number of available
 +* packets per seconds give rounding errors. Hence, we prepare a
 +* sequence of packet sizes that will be alternated over when sending
 +* the packets to the IN endpoint.
 +*/
 +   framesize = opts-p_ssize * num_channels(opts-p_chmask);
 +   interval = (1  (ep_desc-bInterval - 1)) * factor;

Or simply, interval = 1000 ?

 +   rate = opts-p_srate * framesize;
 +
 +   pktsize = min_t(unsigned, rate / interval, ep_desc-wMaxPacketSize);
 +
 +   /*
 +* If there's a residue from the modulo operation, calculate the index
 +* of the first packet that could be increased by one sample frame.
 +* This will be our sequence length, and the last member carries one
 +* frame more than the others. Hence, the shorter the sequence is, the
 +* more bytes will be transferred.
 +*/
 +   residue = (rate % interval) / framesize;
 +   if (residue)
 +   len = min_t(unsigned, interval / residue,
 +   ARRAY_SIZE(uac2-p_pktsize_seq));
 +   else
 +   len = 1;
 +
 +   /* Set all lengths in the sequence to the divided value ... */
 +   for (i = 0; i  len; i++)
 +   uac2-p_pktsize_seq[i] = pktsize;
 +
 +   /* ... and add another frame to the last sequence member */
 +   if (len  1)
 +   uac2-p_pktsize_seq[len - 1] += framesize;
 +
I see this takes care of only 44100Hz case. As I said in other post,
we should take care of all standard [5512.5, 192000] rates, possibly
also optimizing poll period.

Regards,
Jassi
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html