Re: [RFC] Add duplex sound support for USB devices using implicit feedback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On zondag 10 mei 2020 19:31:51 CEST Takashi Iwai wrote:
> On Sat, 09 May 2020 20:10:47 +0200,
> 
> Erwin Burema wrote:
> > Hi,
> > 
> > Since I had problems getting duplex audio to work on my motu m2 I created
> > the following patch to get duplex audio to work. This is based on my
> > understanding that when implicit feedback is used the size of the next
> > output is determined by the previous input, for this the content of the
> > input is ignored but in case of duplex audio the content can actually
> > contain audio information, so long as both input and output are
> > configured the same both sould be usable.
> > 
> > With the above in mind an endpoint can be opened/configured a second time
> > so long as it is an implicit sync endpoint and the config does not
> > change. So that is what this patch does, the check if the config does not
> > change might be a bit more complex then needed, but it seems to work on
> > my motu m2.
> > 
> > Fixes bug 207023 and should fix 103751 as well
> 
> The code changes look good and even if it's not perfect, it can't be
> worse than the current situation, so I'd happily take it.
> 
> But could you submit in a formal way, especially with your
> Signed-off-by line, and with the proper patch description?
> 
> 

Send as patch with signed-off-by line and hopefully a proper patch description, 
if anything is wrong please let me know (I think this is my first patch to the 
kernel to be honest)

Kind Regards

Erwin Burema

> thanks,
> 
> Takashi
> 
> > Kind Regards,
> > 
> > Erwin Burema
> > 
> > ---
> > 
> >  sound/usb/card.h     |   1 +
> >  sound/usb/endpoint.c | 195 ++++++++++++++++++++++++++++++++++++++++++-
> >  sound/usb/pcm.c      |   5 ++
> >  3 files changed, 197 insertions(+), 4 deletions(-)
> > 
> > diff --git a/sound/usb/card.h b/sound/usb/card.h
> > index 820e564656ed..d6219fba9699 100644
> > --- a/sound/usb/card.h
> > +++ b/sound/usb/card.h
> > @@ -108,6 +108,7 @@ struct snd_usb_endpoint {
> > 
> >  	int iface, altsetting;
> >  	int skip_packets;		/* quirks for devices to 
ignore the first n packets
> >  	
> >  					   in a stream */
> > 
> > +	bool is_implicit_feedback;      /* This endpoint is used as 
implicit
> > feedback */> 
> >  	spinlock_t lock;
> >  	struct list_head list;
> > 
> > diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> > index 50104f658ed4..9bea7d3f99f8 100644
> > --- a/sound/usb/endpoint.c
> > +++ b/sound/usb/endpoint.c
> > @@ -522,6 +522,8 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct
> > snd_usb_audio *chip,> 
> >  	list_add_tail(&ep->list, &chip->ep_list);
> > 
> > +	ep->is_implicit_feedback = 0;
> > +
> > 
> >  __exit_unlock:
> >  	mutex_unlock(&chip->mutex);
> > 
> > @@ -621,6 +623,178 @@ static void release_urbs(struct snd_usb_endpoint
> > *ep, int force)> 
> >  	ep->nurbs = 0;
> >  
> >  }
> > 
> > +/*
> > + * Check data endpoint for format differences
> > + */
> > +static bool check_ep_params(struct snd_usb_endpoint *ep,
> > +			      snd_pcm_format_t pcm_format,
> > +			      unsigned int channels,
> > +			      unsigned int period_bytes,
> > +			      unsigned int frames_per_period,
> > +			      unsigned int periods_per_buffer,
> > +			      struct audioformat *fmt,
> > +			      struct snd_usb_endpoint *sync_ep)
> > +{
> > +	unsigned int maxsize, minsize, packs_per_ms, max_packs_per_urb;
> > +	unsigned int max_packs_per_period, urbs_per_period, urb_packs;
> > +	unsigned int max_urbs;
> > +	int frame_bits = snd_pcm_format_physical_width(pcm_format) * 
channels;
> > +	int tx_length_quirk = (ep->chip->tx_length_quirk &&
> > +			       usb_pipeout(ep->pipe));
> > +	bool ret = 1;
> > +
> > +	if (pcm_format == SNDRV_PCM_FORMAT_DSD_U16_LE && fmt->dsd_dop) {
> > +		/*
> > +		 * When operating in DSD DOP mode, the size of a sample 
frame
> > +		 * in hardware differs from the actual physical format 
width
> > +		 * because we need to make room for the DOP markers.
> > +		 */
> > +		frame_bits += channels << 3;
> > +	}
> > +
> > +	ret = ret && (ep->datainterval == fmt->datainterval);
> > +	ret = ret && (ep->stride == frame_bits >> 3);
> > +
> > +	switch (pcm_format) {
> > +	case SNDRV_PCM_FORMAT_U8:
> > +		ret = ret && (ep->silence_value == 0x80);
> > +		break;
> > +	case SNDRV_PCM_FORMAT_DSD_U8:
> > +	case SNDRV_PCM_FORMAT_DSD_U16_LE:
> > +	case SNDRV_PCM_FORMAT_DSD_U32_LE:
> > +	case SNDRV_PCM_FORMAT_DSD_U16_BE:
> > +	case SNDRV_PCM_FORMAT_DSD_U32_BE:
> > +		ret = ret && (ep->silence_value == 0x69);
> > +		break;
> > +	default:
> > +		ret = ret && (ep->silence_value == 0);
> > +	}
> > +
> > +	/* assume max. frequency is 50% higher than nominal */
> > +	ret = ret && (ep->freqmax == ep->freqn + (ep->freqn >> 1));
> > +	/* Round up freqmax to nearest integer in order to calculate 
maximum
> > +	 * packet size, which must represent a whole number of frames.
> > +	 * This is accomplished by adding 0x0.ffff before converting the
> > +	 * Q16.16 format into integer.
> > +	 * In order to accurately calculate the maximum packet size when
> > +	 * the data interval is more than 1 (i.e. ep->datainterval > 0),
> > +	 * multiply by the data interval prior to rounding. For instance,
> > +	 * a freqmax of 41 kHz will result in a max packet size of 6 
(5.125)
> > +	 * frames with a data interval of 1, but 11 (10.25) frames with a
> > +	 * data interval of 2.
> > +	 * (ep->freqmax << ep->datainterval overflows at 8.192 MHz for the
> > +	 * maximum datainterval value of 3, at USB full speed, higher for
> > +	 * USB high speed, noting that ep->freqmax is in units of
> > +	 * frames per packet in Q16.16 format.)
> > +	 */
> > +	maxsize = (((ep->freqmax << ep->datainterval) + 0xffff) >> 16) *
> > +			 (frame_bits >> 3);
> > +	if (tx_length_quirk)
> > +		maxsize += sizeof(__le32); /* Space for length 
descriptor */
> > +	/* but wMaxPacketSize might reduce this */
> > +	if (ep->maxpacksize && ep->maxpacksize < maxsize) {
> > +		/* whatever fits into a max. size packet */
> > +		unsigned int data_maxsize = maxsize = ep->maxpacksize;
> > +
> > +		if (tx_length_quirk)
> > +			/* Need to remove the length descriptor to 
calc freq */
> > +			data_maxsize -= sizeof(__le32);
> > +		ret = ret && (ep->freqmax == (data_maxsize / 
(frame_bits >> 3))
> > +				<< (16 - ep->datainterval));
> > +	}
> > +
> > +	if (ep->fill_max)
> > +		ret = ret && (ep->curpacksize == ep->maxpacksize);
> > +	else
> > +		ret = ret && (ep->curpacksize == maxsize);
> > +
> > +	if (snd_usb_get_speed(ep->chip->dev) != USB_SPEED_FULL) {
> > +		packs_per_ms = 8 >> ep->datainterval;
> > +		max_packs_per_urb = MAX_PACKS_HS;
> > +	} else {
> > +		packs_per_ms = 1;
> > +		max_packs_per_urb = MAX_PACKS;
> > +	}
> > +	if (sync_ep && !snd_usb_endpoint_implicit_feedback_sink(ep))
> > +		max_packs_per_urb = min(max_packs_per_urb,
> > +					1U << sync_ep-
>syncinterval);
> > +	max_packs_per_urb = max(1u, max_packs_per_urb >> ep-
>datainterval);
> > +
> > +	/*
> > +	 * Capture endpoints need to use small URBs because there's no way
> > +	 * to tell in advance where the next period will end, and we don't
> > +	 * want the next URB to complete much after the period ends.
> > +	 *
> > +	 * Playback endpoints with implicit sync much use the same 
parameters
> > +	 * as their corresponding capture endpoint.
> > +	 */
> > +	if (usb_pipein(ep->pipe) ||
> > +			snd_usb_endpoint_implicit_feedback_sink(ep)) 
{
> > +
> > +		urb_packs = packs_per_ms;
> > +		/*
> > +		 * Wireless devices can poll at a max rate of once per 
4ms.
> > +		 * For dataintervals less than 5, increase the packet 
count to
> > +		 * allow the host controller to use bursting to fill in 
the
> > +		 * gaps.
> > +		 */
> > +		if (snd_usb_get_speed(ep->chip->dev) == 
USB_SPEED_WIRELESS) {
> > +			int interval = ep->datainterval;
> > +
> > +			while (interval < 5) {
> > +				urb_packs <<= 1;
> > +				++interval;
> > +			}
> > +		}
> > +		/* make capture URBs <= 1 ms and smaller than a period 
*/
> > +		urb_packs = min(max_packs_per_urb, urb_packs);
> > +		while (urb_packs > 1 && urb_packs * maxsize >= 
period_bytes)
> > +			urb_packs >>= 1;
> > +		ret = ret && (ep->nurbs == MAX_URBS);
> > +
> > +	/*
> > +	 * Playback endpoints without implicit sync are adjusted so that
> > +	 * a period fits as evenly as possible in the smallest number of
> > +	 * URBs.  The total number of URBs is adjusted to the size of the
> > +	 * ALSA buffer, subject to the MAX_URBS and MAX_QUEUE limits.
> > +	 */
> > +	} else {
> > +		/* determine how small a packet can be */
> > +		minsize = (ep->freqn >> (16 - ep->datainterval)) *
> > +				(frame_bits >> 3);
> > +		/* with sync from device, assume it can be 12% lower */
> > +		if (sync_ep)
> > +			minsize -= minsize >> 3;
> > +		minsize = max(minsize, 1u);
> > +
> > +		/* how many packets will contain an entire ALSA period? 
*/
> > +		max_packs_per_period = DIV_ROUND_UP(period_bytes, 
minsize);
> > +
> > +		/* how many URBs will contain a period? */
> > +		urbs_per_period = DIV_ROUND_UP(max_packs_per_period,
> > +				max_packs_per_urb);
> > +		/* how many packets are needed in each URB? */
> > +		urb_packs = DIV_ROUND_UP(max_packs_per_period, 
urbs_per_period);
> > +
> > +		/* limit the number of frames in a single URB */
> > +		ret = ret && (ep->max_urb_frames ==
> > +			DIV_ROUND_UP(frames_per_period, 
urbs_per_period));
> > +
> > +		/* try to use enough URBs to contain an entire ALSA 
buffer */
> > +		max_urbs = min((unsigned) MAX_URBS,
> > +				MAX_QUEUE * packs_per_ms / 
urb_packs);
> > +		ret = ret && (ep->nurbs == min(max_urbs,
> > +				urbs_per_period * 
periods_per_buffer));
> > +	}
> > +
> > +	ret = ret && (ep->datainterval == fmt->datainterval);
> > +	ret = ret && (ep->maxpacksize == fmt->maxpacksize);
> > +	ret = ret &&
> > +		(ep->fill_max == !!(fmt->attributes & 
UAC_EP_CS_ATTR_FILL_MAX));
> > +
> > +	return ret;
> > +}
> > +
> > 
> >  /*
> >  
> >   * configure a data endpoint
> >   */
> > 
> > @@ -886,10 +1060,23 @@ int snd_usb_endpoint_set_params(struct
> > snd_usb_endpoint *ep,> 
> >  	int err;
> >  	
> >  	if (ep->use_count != 0) {
> > 
> > -		usb_audio_warn(ep->chip,
> > -			 "Unable to change format on ep #%x: already 
in use\n",
> > -			 ep->ep_num);
> > -		return -EBUSY;
> > +		bool check = ep->is_implicit_feedback &&
> > +			check_ep_params(ep, pcm_format,
> > +					     channels, 
period_bytes,
> > +					     period_frames, 
buffer_periods,
> > +					     fmt, sync_ep);
> > +
> > +		if (!check) {
> > +			usb_audio_warn(ep->chip,
> > +				"Unable to change format on ep 
#%x: already in use\n",
> > +				ep->ep_num);
> > +			return -EBUSY;
> > +		}
> > +
> > +		usb_audio_dbg(ep->chip,
> > +			      "Ep #%x already in use as implicit 
feedback but format not
> > changed\n", +			      ep->ep_num);
> > +		return 0;
> > 
> >  	}
> >  	
> >  	/* release old buffers, if any */
> > 
> > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> > index b50965ab3b3a..d61c2f1095b5 100644
> > --- a/sound/usb/pcm.c
> > +++ b/sound/usb/pcm.c
> > @@ -404,6 +404,8 @@ static int set_sync_ep_implicit_fb_quirk(struct
> > snd_usb_substream *subs,> 
> >  	if (!subs->sync_endpoint)
> >  	
> >  		return -EINVAL;
> > 
> > +	subs->sync_endpoint->is_implicit_feedback = 1;
> > +
> > 
> >  	subs->data_endpoint->sync_master = subs->sync_endpoint;
> >  	
> >  	return 1;
> > 
> > @@ -502,12 +504,15 @@ static int set_sync_endpoint(struct
> > snd_usb_substream *subs,> 
> >  						   
implicit_fb ?
> >  							
> >  							
SND_USB_ENDPOINT_TYPE_DATA :
> >  							
SND_USB_ENDPOINT_TYPE_SYNC);
> > 
> > +
> > 
> >  	if (!subs->sync_endpoint) {
> >  	
> >  		if (is_playback && attr == USB_ENDPOINT_SYNC_NONE)
> >  		
> >  			return 0;
> >  		
> >  		return -EINVAL;
> >  	
> >  	}
> > 
> > +	subs->sync_endpoint->is_implicit_feedback = implicit_fb;
> > +
> > 
> >  	subs->data_endpoint->sync_master = subs->sync_endpoint;
> >  	
> >  	return 0;







[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux