[alsa-devel] [RFC 0/2] ASoC: remove io_mutex
From: Eero Nurkkala ext-eero.nurkkala@nokia.com
This RFC set addresses the existence of io_mutex. A safer method is introduced - but it changes the semantics quite a bit, as now all drivers calling snd_soc_update_bits() will need to make sure the codec mutex is taken.
snd_soc_test_bits() - function does not need to take the io_mutex at all - all calls to it have the codec mutex already taken.
Warning: Not even compile tested!
Possible drawback:
dapm_power_widgets() may hold the codec mutex for a few ms, which may be too long; meanwhile, calls to snd_soc_update_bits() are stalled.
Of course we wish optimal performance and maybe the codec mutex should only be taken for such registers that contain both volume and dapm bits...
Ideas, comments, all welcome.
From: Eero Nurkkala ext-eero.nurkkala@nokia.com
Remove the io_mutex. It has a drawback of serializing all accesses to snd_soc_update_bits() even when multiple codecs are in use. In addition, it fails to actually do its task - during snd_soc_update_bits(), dapm_update_bits() may also be accessing the same register which may result in an outdated register value.
Signed-off-by: Eero Nurkkala ext-eero.nurkkala@nokia.com --- sound/soc/soc-core.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 7ff04ad..1f42d46 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -37,7 +37,6 @@ #include <sound/initval.h>
static DEFINE_MUTEX(pcm_mutex); -static DEFINE_MUTEX(io_mutex); static DECLARE_WAIT_QUEUE_HEAD(soc_pm_waitq);
#ifdef CONFIG_DEBUG_FS @@ -1361,14 +1360,12 @@ int snd_soc_update_bits(struct snd_soc_codec *codec, unsigned short reg, int change; unsigned int old, new;
- mutex_lock(&io_mutex); old = snd_soc_read(codec, reg); new = (old & ~mask) | value; change = old != new; if (change) snd_soc_write(codec, reg, new);
- mutex_unlock(&io_mutex); return change; } EXPORT_SYMBOL_GPL(snd_soc_update_bits); @@ -1391,11 +1388,9 @@ int snd_soc_test_bits(struct snd_soc_codec *codec, unsigned short reg, int change; unsigned int old, new;
- mutex_lock(&io_mutex); old = snd_soc_read(codec, reg); new = (old & ~mask) | value; change = old != new; - mutex_unlock(&io_mutex);
return change; }
From: Eero Nurkkala ext-eero.nurkkala@nokia.com
Introduce a wrapper call snd_soc_update_bits_locked() that will take the codec mutex. This call is used when the codec mutex is not already taken.
Drivers calling snd_soc_update_bits() may wish to make sure the codec mutex is taken from the driver.
Signed-off-by: Eero Nurkkala ext-eero.nurkkala@nokia.com --- sound/soc/soc-core.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 1f42d46..abeb5fb 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1371,6 +1371,30 @@ int snd_soc_update_bits(struct snd_soc_codec *codec, unsigned short reg, EXPORT_SYMBOL_GPL(snd_soc_update_bits);
/** + * snd_soc_update_bits_locked - update codec register bits + * @codec: audio codec + * @reg: codec register + * @mask: register mask + * @value: new value + * + * Writes new register value, and takes the codec mutex. + * + * Returns 1 for change else 0. + */ +static int snd_soc_update_bits_locked(struct snd_soc_codec *codec, + unsigned short reg, unsigned int mask, + unsigned int value) +{ + int change; + + mutex_lock(&codec->mutex); + change = snd_soc_update_bits(codec, reg, mask, value); + mutex_unlock(&codec->mutex); + + return change; +} + +/** * snd_soc_test_bits - test register for change * @codec: audio codec * @reg: codec register @@ -1721,7 +1745,7 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol, mask |= (bitmask - 1) << e->shift_r; }
- return snd_soc_update_bits(codec, e->reg, mask, val); + return snd_soc_update_bits_locked(codec, e->reg, mask, val); } EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
@@ -1795,7 +1819,7 @@ int snd_soc_put_value_enum_double(struct snd_kcontrol *kcontrol, mask |= e->mask << e->shift_r; }
- return snd_soc_update_bits(codec, e->reg, mask, val); + return snd_soc_update_bits_locked(codec, e->reg, mask, val); } EXPORT_SYMBOL_GPL(snd_soc_put_value_enum_double);
@@ -1956,7 +1980,7 @@ int snd_soc_put_volsw(struct snd_kcontrol *kcontrol, val_mask |= mask << rshift; val |= val2 << rshift; } - return snd_soc_update_bits(codec, reg, val_mask, val); + return snd_soc_update_bits_locked(codec, reg, val_mask, val); } EXPORT_SYMBOL_GPL(snd_soc_put_volsw);
@@ -2062,11 +2086,11 @@ int snd_soc_put_volsw_2r(struct snd_kcontrol *kcontrol, val = val << shift; val2 = val2 << shift;
- err = snd_soc_update_bits(codec, reg, val_mask, val); + err = snd_soc_update_bits_locked(codec, reg, val_mask, val); if (err < 0) return err;
- err = snd_soc_update_bits(codec, reg2, val_mask, val2); + err = snd_soc_update_bits_locked(codec, reg2, val_mask, val2); return err; } EXPORT_SYMBOL_GPL(snd_soc_put_volsw_2r); @@ -2145,7 +2169,7 @@ int snd_soc_put_volsw_s8(struct snd_kcontrol *kcontrol, val = (ucontrol->value.integer.value[0]+min) & 0xff; val |= ((ucontrol->value.integer.value[1]+min) & 0xff) << 8;
- return snd_soc_update_bits(codec, reg, 0xffff, val); + return snd_soc_update_bits_locked(codec, reg, 0xffff, val); } EXPORT_SYMBOL_GPL(snd_soc_put_volsw_s8);
On Fri, Oct 30, 2009 at 01:34:01PM +0200, ext-eero.nurkkala@nokia.com wrote:
From: Eero Nurkkala ext-eero.nurkkala@nokia.com
This RFC set addresses the existence of io_mutex. A safer method is introduced - but it changes the semantics quite a bit, as now all drivers calling
As discussed previously this is probably sensible; it's certainly more sensible than the current code if nothing else. I've applied them, if there's problems we can fix them as they're noticed but this is a better place to start from. Thanks!
Please do always remember to CC maintainers on patches - it helps avoid your patches getting lost.
participants (2)
-
ext-eero.nurkkala＠nokia.com -
Mark Brown