diff mbox

[RFC] em28xx-audio: don't wait for lock in non-block mode

Message ID 1389529505-32508-1-git-send-email-m.chehab@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mauro Carvalho Chehab Jan. 12, 2014, 12:25 p.m. UTC 
Pulseaudio has the bad habit of stopping a streaming audio if
a device, opened in non-block mode, waits.

It is impossible to avoid em28xx to wait, as it will send commands
via I2C, and other I2C operations may be happening (firmware
transfers, Remote Controller polling, etc). Yet, as each em28xx
subdriver locks em28xx-dev to protect the access to the hardware,
it is possible to minimize the audio glitches by returning -EAGAIN
to pulseaudio, if the lock is already taken by another subdriver.

Reported-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-audio.c | 48 +++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)

Comments

Antti Palosaari Jan. 12, 2014, 3:45 p.m. UTC | #1 
That fixes the audio bug :) I can listen netradio using Rhythmbox and it 
does not lose audio anymore when em28xx device is plugged.

Tested-by: Antti Palosaari <crope@iki.fi>


regards
Antti

On 12.01.2014 14:25, Mauro Carvalho Chehab wrote:
> Pulseaudio has the bad habit of stopping a streaming audio if
> a device, opened in non-block mode, waits.
>
> It is impossible to avoid em28xx to wait, as it will send commands
> via I2C, and other I2C operations may be happening (firmware
> transfers, Remote Controller polling, etc). Yet, as each em28xx
> subdriver locks em28xx-dev to protect the access to the hardware,
> it is possible to minimize the audio glitches by returning -EAGAIN
> to pulseaudio, if the lock is already taken by another subdriver.
>
> Reported-by: Antti Palosaari <crope@iki.fi>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
>   drivers/media/usb/em28xx/em28xx-audio.c | 48 +++++++++++++++++++++++++++++----
>   1 file changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c
> index f6fcee3d4fb9..f004680219e7 100644
> --- a/drivers/media/usb/em28xx/em28xx-audio.c
> +++ b/drivers/media/usb/em28xx/em28xx-audio.c
> @@ -258,6 +258,13 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream)
>
>   	runtime->hw = snd_em28xx_hw_capture;
>   	if ((dev->alt == 0 || dev->audio_ifnum) && dev->adev.users == 0) {
> +		int nonblock = !!(substream->f_flags & O_NONBLOCK);
> +
> +		if (nonblock) {
> +			if (!mutex_trylock(&dev->lock))
> +				return -EAGAIN;
> +		} else
> +			mutex_lock(&dev->lock);
>   		if (dev->audio_ifnum)
>   			dev->alt = 1;
>   		else
> @@ -269,7 +276,6 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream)
>
>   		/* Sets volume, mute, etc */
>   		dev->mute = 0;
> -		mutex_lock(&dev->lock);
>   		ret = em28xx_audio_analog_set(dev);
>   		if (ret < 0)
>   			goto err;
> @@ -441,11 +447,19 @@ static int em28xx_vol_put(struct snd_kcontrol *kcontrol,
>   			       struct snd_ctl_elem_value *value)
>   {
>   	struct em28xx *dev = snd_kcontrol_chip(kcontrol);
> +	struct snd_pcm_substream *substream = dev->adev.capture_pcm_substream;
>   	u16 val = (0x1f - (value->value.integer.value[0] & 0x1f)) |
>   		  (0x1f - (value->value.integer.value[1] & 0x1f)) << 8;
> +	int nonblock = 0;
>   	int rc;
>
> -	mutex_lock(&dev->lock);
> +	if (substream)
> +		nonblock = !!(substream->f_flags & O_NONBLOCK);
> +	if (nonblock) {
> +		if (!mutex_trylock(&dev->lock))
> +			return -EAGAIN;
> +	} else
> +		mutex_lock(&dev->lock);
>   	rc = em28xx_read_ac97(dev, kcontrol->private_value);
>   	if (rc < 0)
>   		goto err;
> @@ -470,9 +484,17 @@ static int em28xx_vol_get(struct snd_kcontrol *kcontrol,
>   			       struct snd_ctl_elem_value *value)
>   {
>   	struct em28xx *dev = snd_kcontrol_chip(kcontrol);
> +	struct snd_pcm_substream *substream = dev->adev.capture_pcm_substream;
> +	int nonblock = 0;
>   	int val;
>
> -	mutex_lock(&dev->lock);
> +	if (substream)
> +		nonblock = !!(substream->f_flags & O_NONBLOCK);
> +	if (nonblock) {
> +		if (!mutex_trylock(&dev->lock))
> +			return -EAGAIN;
> +	} else
> +		mutex_lock(&dev->lock);
>   	val = em28xx_read_ac97(dev, kcontrol->private_value);
>   	mutex_unlock(&dev->lock);
>   	if (val < 0)
> @@ -494,9 +516,17 @@ static int em28xx_vol_put_mute(struct snd_kcontrol *kcontrol,
>   {
>   	struct em28xx *dev = snd_kcontrol_chip(kcontrol);
>   	u16 val = value->value.integer.value[0];
> +	struct snd_pcm_substream *substream = dev->adev.capture_pcm_substream;
> +	int nonblock = 0;
>   	int rc;
>
> -	mutex_lock(&dev->lock);
> +	if (substream)
> +		nonblock = !!(substream->f_flags & O_NONBLOCK);
> +	if (nonblock) {
> +		if (!mutex_trylock(&dev->lock))
> +			return -EAGAIN;
> +	} else
> +		mutex_lock(&dev->lock);
>   	rc = em28xx_read_ac97(dev, kcontrol->private_value);
>   	if (rc < 0)
>   		goto err;
> @@ -524,9 +554,17 @@ static int em28xx_vol_get_mute(struct snd_kcontrol *kcontrol,
>   			       struct snd_ctl_elem_value *value)
>   {
>   	struct em28xx *dev = snd_kcontrol_chip(kcontrol);
> +	struct snd_pcm_substream *substream = dev->adev.capture_pcm_substream;
> +	int nonblock = 0;
>   	int val;
>
> -	mutex_lock(&dev->lock);
> +	if (substream)
> +		nonblock = !!(substream->f_flags & O_NONBLOCK);
> +	if (nonblock) {
> +		if (!mutex_trylock(&dev->lock))
> +			return -EAGAIN;
> +	} else
> +		mutex_lock(&dev->lock);
>   	val = em28xx_read_ac97(dev, kcontrol->private_value);
>   	mutex_unlock(&dev->lock);
>   	if (val < 0)
>
diff mbox

Patch

diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c
index f6fcee3d4fb9..f004680219e7 100644
--- a/drivers/media/usb/em28xx/em28xx-audio.c
+++ b/drivers/media/usb/em28xx/em28xx-audio.c
@@ -258,6 +258,13 @@  static int snd_em28xx_capture_open(struct snd_pcm_substream *substream)
 
 	runtime->hw = snd_em28xx_hw_capture;
 	if ((dev->alt == 0 || dev->audio_ifnum) && dev->adev.users == 0) {
+		int nonblock = !!(substream->f_flags & O_NONBLOCK);
+
+		if (nonblock) {
+			if (!mutex_trylock(&dev->lock))
+				return -EAGAIN;
+		} else
+			mutex_lock(&dev->lock);
 		if (dev->audio_ifnum)
 			dev->alt = 1;
 		else
@@ -269,7 +276,6 @@  static int snd_em28xx_capture_open(struct snd_pcm_substream *substream)
 
 		/* Sets volume, mute, etc */
 		dev->mute = 0;
-		mutex_lock(&dev->lock);
 		ret = em28xx_audio_analog_set(dev);
 		if (ret < 0)
 			goto err;
@@ -441,11 +447,19 @@  static int em28xx_vol_put(struct snd_kcontrol *kcontrol,
 			       struct snd_ctl_elem_value *value)
 {
 	struct em28xx *dev = snd_kcontrol_chip(kcontrol);
+	struct snd_pcm_substream *substream = dev->adev.capture_pcm_substream;
 	u16 val = (0x1f - (value->value.integer.value[0] & 0x1f)) |
 		  (0x1f - (value->value.integer.value[1] & 0x1f)) << 8;
+	int nonblock = 0;
 	int rc;
 
-	mutex_lock(&dev->lock);
+	if (substream)
+		nonblock = !!(substream->f_flags & O_NONBLOCK);
+	if (nonblock) {
+		if (!mutex_trylock(&dev->lock))
+			return -EAGAIN;
+	} else
+		mutex_lock(&dev->lock);
 	rc = em28xx_read_ac97(dev, kcontrol->private_value);
 	if (rc < 0)
 		goto err;
@@ -470,9 +484,17 @@  static int em28xx_vol_get(struct snd_kcontrol *kcontrol,
 			       struct snd_ctl_elem_value *value)
 {
 	struct em28xx *dev = snd_kcontrol_chip(kcontrol);
+	struct snd_pcm_substream *substream = dev->adev.capture_pcm_substream;
+	int nonblock = 0;
 	int val;
 
-	mutex_lock(&dev->lock);
+	if (substream)
+		nonblock = !!(substream->f_flags & O_NONBLOCK);
+	if (nonblock) {
+		if (!mutex_trylock(&dev->lock))
+			return -EAGAIN;
+	} else
+		mutex_lock(&dev->lock);
 	val = em28xx_read_ac97(dev, kcontrol->private_value);
 	mutex_unlock(&dev->lock);
 	if (val < 0)
@@ -494,9 +516,17 @@  static int em28xx_vol_put_mute(struct snd_kcontrol *kcontrol,
 {
 	struct em28xx *dev = snd_kcontrol_chip(kcontrol);
 	u16 val = value->value.integer.value[0];
+	struct snd_pcm_substream *substream = dev->adev.capture_pcm_substream;
+	int nonblock = 0;
 	int rc;
 
-	mutex_lock(&dev->lock);
+	if (substream)
+		nonblock = !!(substream->f_flags & O_NONBLOCK);
+	if (nonblock) {
+		if (!mutex_trylock(&dev->lock))
+			return -EAGAIN;
+	} else
+		mutex_lock(&dev->lock);
 	rc = em28xx_read_ac97(dev, kcontrol->private_value);
 	if (rc < 0)
 		goto err;
@@ -524,9 +554,17 @@  static int em28xx_vol_get_mute(struct snd_kcontrol *kcontrol,
 			       struct snd_ctl_elem_value *value)
 {
 	struct em28xx *dev = snd_kcontrol_chip(kcontrol);
+	struct snd_pcm_substream *substream = dev->adev.capture_pcm_substream;
+	int nonblock = 0;
 	int val;
 
-	mutex_lock(&dev->lock);
+	if (substream)
+		nonblock = !!(substream->f_flags & O_NONBLOCK);
+	if (nonblock) {
+		if (!mutex_trylock(&dev->lock))
+			return -EAGAIN;
+	} else
+		mutex_lock(&dev->lock);
 	val = em28xx_read_ac97(dev, kcontrol->private_value);
 	mutex_unlock(&dev->lock);
 	if (val < 0)