bcm2835: access controls under the audio mutex

I don't think the ALSA framework provides any kind of automatic
synchronization within the control callbacks. We most likely need
to ensure this manually, so add locking around all access to shared
mutable data. In particular, bcm2835_audio_set_ctls() should
probably always be called under our own audio lock.
This commit is contained in:
wm4
2016-01-13 19:43:35 +01:00
committed by popcornmix
parent 8deeaf8fdb
commit 56c57c5944
2 changed files with 66 additions and 12 deletions

View File

@@ -94,6 +94,9 @@ static int snd_bcm2835_ctl_get(struct snd_kcontrol *kcontrol,
{ {
struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol); struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
if (mutex_lock_interruptible(&chip->audio_mutex))
return -EINTR;
BUG_ON(!chip && !(chip->avail_substreams & AVAIL_SUBSTREAMS_MASK)); BUG_ON(!chip && !(chip->avail_substreams & AVAIL_SUBSTREAMS_MASK));
if (kcontrol->private_value == PCM_PLAYBACK_VOLUME) if (kcontrol->private_value == PCM_PLAYBACK_VOLUME)
@@ -103,6 +106,7 @@ static int snd_bcm2835_ctl_get(struct snd_kcontrol *kcontrol,
else if (kcontrol->private_value == PCM_PLAYBACK_DEVICE) else if (kcontrol->private_value == PCM_PLAYBACK_DEVICE)
ucontrol->value.integer.value[0] = chip->dest; ucontrol->value.integer.value[0] = chip->dest;
mutex_unlock(&chip->audio_mutex);
return 0; return 0;
} }
@@ -112,11 +116,15 @@ static int snd_bcm2835_ctl_put(struct snd_kcontrol *kcontrol,
struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol); struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
int changed = 0; int changed = 0;
if (mutex_lock_interruptible(&chip->audio_mutex))
return -EINTR;
if (kcontrol->private_value == PCM_PLAYBACK_VOLUME) { if (kcontrol->private_value == PCM_PLAYBACK_VOLUME) {
audio_info("Volume change attempted.. volume = %d new_volume = %d\n", chip->volume, (int)ucontrol->value.integer.value[0]); audio_info("Volume change attempted.. volume = %d new_volume = %d\n", chip->volume, (int)ucontrol->value.integer.value[0]);
if (chip->mute == CTRL_VOL_MUTE) { if (chip->mute == CTRL_VOL_MUTE) {
/* changed = toggle_mute(chip, CTRL_VOL_UNMUTE); */ /* changed = toggle_mute(chip, CTRL_VOL_UNMUTE); */
return 1; /* should return 0 to signify no change but the mixer takes this as the opposite sign (no idea why) */ changed = 1; /* should return 0 to signify no change but the mixer takes this as the opposite sign (no idea why) */
goto unlock;
} }
if (changed if (changed
|| (ucontrol->value.integer.value[0] != chip2alsa(chip->volume))) { || (ucontrol->value.integer.value[0] != chip2alsa(chip->volume))) {
@@ -142,6 +150,8 @@ static int snd_bcm2835_ctl_put(struct snd_kcontrol *kcontrol,
printk(KERN_ERR "Failed to set ALSA controls..\n"); printk(KERN_ERR "Failed to set ALSA controls..\n");
} }
unlock:
mutex_unlock(&chip->audio_mutex);
return changed; return changed;
} }
@@ -198,10 +208,14 @@ static int snd_bcm2835_spdif_default_get(struct snd_kcontrol *kcontrol,
struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol); struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
int i; int i;
if (mutex_lock_interruptible(&chip->audio_mutex))
return -EINTR;
for (i = 0; i < 4; i++) for (i = 0; i < 4; i++)
ucontrol->value.iec958.status[i] = ucontrol->value.iec958.status[i] =
(chip->spdif_status >> (i * 8)) && 0xff; (chip->spdif_status >> (i * 8)) && 0xff;
mutex_unlock(&chip->audio_mutex);
return 0; return 0;
} }
@@ -212,12 +226,16 @@ static int snd_bcm2835_spdif_default_put(struct snd_kcontrol *kcontrol,
unsigned int val = 0; unsigned int val = 0;
int i, change; int i, change;
if (mutex_lock_interruptible(&chip->audio_mutex))
return -EINTR;
for (i = 0; i < 4; i++) for (i = 0; i < 4; i++)
val |= (unsigned int)ucontrol->value.iec958.status[i] << (i * 8); val |= (unsigned int)ucontrol->value.iec958.status[i] << (i * 8);
change = val != chip->spdif_status; change = val != chip->spdif_status;
chip->spdif_status = val; chip->spdif_status = val;
mutex_unlock(&chip->audio_mutex);
return change; return change;
} }
@@ -253,9 +271,14 @@ static int snd_bcm2835_spdif_stream_get(struct snd_kcontrol *kcontrol,
struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol); struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
int i; int i;
if (mutex_lock_interruptible(&chip->audio_mutex))
return -EINTR;
for (i = 0; i < 4; i++) for (i = 0; i < 4; i++)
ucontrol->value.iec958.status[i] = ucontrol->value.iec958.status[i] =
(chip->spdif_status >> (i * 8)) & 0xff; (chip->spdif_status >> (i * 8)) & 0xff;
mutex_unlock(&chip->audio_mutex);
return 0; return 0;
} }
@@ -266,11 +289,15 @@ static int snd_bcm2835_spdif_stream_put(struct snd_kcontrol *kcontrol,
unsigned int val = 0; unsigned int val = 0;
int i, change; int i, change;
if (mutex_lock_interruptible(&chip->audio_mutex))
return -EINTR;
for (i = 0; i < 4; i++) for (i = 0; i < 4; i++)
val |= (unsigned int)ucontrol->value.iec958.status[i] << (i * 8); val |= (unsigned int)ucontrol->value.iec958.status[i] << (i * 8);
change = val != chip->spdif_status; change = val != chip->spdif_status;
chip->spdif_status = val; chip->spdif_status = val;
mutex_unlock(&chip->audio_mutex);
return change; return change;
} }
@@ -454,11 +481,17 @@ static int snd_bcm2835_chmap_ctl_get(struct snd_kcontrol *kcontrol,
unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id); unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id);
struct snd_pcm_substream *substream = snd_pcm_chmap_substream(info, idx); struct snd_pcm_substream *substream = snd_pcm_chmap_substream(info, idx);
struct cea_channel_speaker_allocation *ch = NULL; struct cea_channel_speaker_allocation *ch = NULL;
int res = 0;
int cur = 0; int cur = 0;
int i; int i;
if (!substream || !substream->runtime) if (mutex_lock_interruptible(&chip->audio_mutex))
return -ENODEV; return -EINTR;
if (!substream || !substream->runtime) {
res = -ENODEV;
goto unlock;
}
for (i = 0; i < ARRAY_SIZE(channel_allocations); i++) { for (i = 0; i < ARRAY_SIZE(channel_allocations); i++) {
if (channel_allocations[i].ca_index == chip->cea_chmap) if (channel_allocations[i].ca_index == chip->cea_chmap)
@@ -476,7 +509,10 @@ static int snd_bcm2835_chmap_ctl_get(struct snd_kcontrol *kcontrol,
} }
while (cur < 8) while (cur < 8)
ucontrol->value.integer.value[cur++] = SNDRV_CHMAP_NA; ucontrol->value.integer.value[cur++] = SNDRV_CHMAP_NA;
return 0;
unlock:
mutex_unlock(&chip->audio_mutex);
return res;
} }
static int snd_bcm2835_chmap_ctl_put(struct snd_kcontrol *kcontrol, static int snd_bcm2835_chmap_ctl_put(struct snd_kcontrol *kcontrol,
@@ -487,10 +523,16 @@ static int snd_bcm2835_chmap_ctl_put(struct snd_kcontrol *kcontrol,
unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id); unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id);
struct snd_pcm_substream *substream = snd_pcm_chmap_substream(info, idx); struct snd_pcm_substream *substream = snd_pcm_chmap_substream(info, idx);
int i, prepared = 0, cea_chmap = -1; int i, prepared = 0, cea_chmap = -1;
int res = 0;
int remap[8]; int remap[8];
if (!substream || !substream->runtime) if (mutex_lock_interruptible(&chip->audio_mutex))
return -ENODEV; return -EINTR;
if (!substream || !substream->runtime) {
res = -ENODEV;
goto unlock;
}
switch (substream->runtime->status->state) { switch (substream->runtime->status->state) {
case SNDRV_PCM_STATE_OPEN: case SNDRV_PCM_STATE_OPEN:
@@ -500,7 +542,8 @@ static int snd_bcm2835_chmap_ctl_put(struct snd_kcontrol *kcontrol,
prepared = 1; prepared = 1;
break; break;
default: default:
return -EBUSY; res = -EBUSY;
goto unlock;
} }
for (i = 0; i < ARRAY_SIZE(channel_allocations); i++) { for (i = 0; i < ARRAY_SIZE(channel_allocations); i++) {
@@ -538,19 +581,26 @@ static int snd_bcm2835_chmap_ctl_put(struct snd_kcontrol *kcontrol,
} }
} }
if (cea_chmap < 0) if (cea_chmap < 0) {
return -EINVAL; res = -EINVAL;
goto unlock;
}
/* don't change the layout if another substream is active */ /* don't change the layout if another substream is active */
if (chip->opened != (1 << substream->number) && chip->cea_chmap != cea_chmap) if (chip->opened != (1 << substream->number) && chip->cea_chmap != cea_chmap) {
return -EBUSY; /* unsure whether this is a good error code */ res = -EBUSY; /* unsure whether this is a good error code */
goto unlock;
}
chip->cea_chmap = cea_chmap; chip->cea_chmap = cea_chmap;
for (i = 0; i < 8; i++) for (i = 0; i < 8; i++)
chip->map_channels[i] = remap[i]; chip->map_channels[i] = remap[i];
if (prepared) if (prepared)
snd_bcm2835_pcm_prepare_again(substream); snd_bcm2835_pcm_prepare_again(substream);
return 0;
unlock:
mutex_unlock(&chip->audio_mutex);
return res;
} }
static int snd_bcm2835_add_chmap_ctl(bcm2835_chip_t * chip) static int snd_bcm2835_add_chmap_ctl(bcm2835_chip_t * chip)

View File

@@ -379,6 +379,9 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream)
audio_info(" .. IN\n"); audio_info(" .. IN\n");
if (mutex_lock_interruptible(&chip->audio_mutex))
return -EINTR;
snd_bcm2835_pcm_prepare_again(substream); snd_bcm2835_pcm_prepare_again(substream);
bcm2835_audio_setup(alsa_stream); bcm2835_audio_setup(alsa_stream);
@@ -401,6 +404,7 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream)
alsa_stream->buffer_size, alsa_stream->period_size, alsa_stream->buffer_size, alsa_stream->period_size,
alsa_stream->pos, runtime->frame_bits); alsa_stream->pos, runtime->frame_bits);
mutex_unlock(&chip->audio_mutex);
audio_info(" .. OUT\n"); audio_info(" .. OUT\n");
return 0; return 0;
} }