[alsa-devel] [PATCH 1/2 2.6.38] ASoC: soc-cache: Introduce the cache_bypass option
This is primarily needed to avoid writing back to the cache whenever we are syncing the cache with the hardware. This gives a performance benefit especially for large register maps.
Signed-off-by: Dimitris Papastamos dp@opensource.wolfsonmicro.com Acked-by: Liam Girdwood lrg@slimlogic.co.uk Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com --- include/sound/soc.h | 1 + sound/soc/soc-cache.c | 36 ++++++++++++++++++++++++------------ 2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 74921f2..5e19644 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -463,6 +463,7 @@ struct snd_soc_codec { /* runtime */ struct snd_ac97 *ac97; /* for ad-hoc ac97 devices */ unsigned int active; + unsigned int cache_bypass:1; /* Suppress access to the cache */ unsigned int cache_only:1; /* Suppress writes to hardware */ unsigned int cache_sync:1; /* Cache needs to be synced to hardware */ unsigned int suspended:1; /* Codec is in suspend PM state */ diff --git a/sound/soc/soc-cache.c b/sound/soc/soc-cache.c index 8c2a21a..799e376 100644 --- a/sound/soc/soc-cache.c +++ b/sound/soc/soc-cache.c @@ -25,7 +25,8 @@ static unsigned int snd_soc_4_12_read(struct snd_soc_codec *codec, unsigned int val;
if (reg >= codec->driver->reg_cache_size || - snd_soc_codec_volatile_register(codec, reg)) { + snd_soc_codec_volatile_register(codec, reg) || + codec->cache_bypass) { if (codec->cache_only) return -1;
@@ -49,7 +50,8 @@ static int snd_soc_4_12_write(struct snd_soc_codec *codec, unsigned int reg, data[1] = value & 0x00ff;
if (!snd_soc_codec_volatile_register(codec, reg) && - reg < codec->driver->reg_cache_size) { + reg < codec->driver->reg_cache_size && + !codec->cache_bypass) { ret = snd_soc_cache_write(codec, reg, value); if (ret < 0) return -1; @@ -106,7 +108,8 @@ static unsigned int snd_soc_7_9_read(struct snd_soc_codec *codec, unsigned int val;
if (reg >= codec->driver->reg_cache_size || - snd_soc_codec_volatile_register(codec, reg)) { + snd_soc_codec_volatile_register(codec, reg) || + codec->cache_bypass) { if (codec->cache_only) return -1;
@@ -130,7 +133,8 @@ static int snd_soc_7_9_write(struct snd_soc_codec *codec, unsigned int reg, data[1] = value & 0x00ff;
if (!snd_soc_codec_volatile_register(codec, reg) && - reg < codec->driver->reg_cache_size) { + reg < codec->driver->reg_cache_size && + !codec->cache_bypass) { ret = snd_soc_cache_write(codec, reg, value); if (ret < 0) return -1; @@ -191,7 +195,8 @@ static int snd_soc_8_8_write(struct snd_soc_codec *codec, unsigned int reg, data[1] = value & 0xff;
if (!snd_soc_codec_volatile_register(codec, reg) && - reg < codec->driver->reg_cache_size) { + reg < codec->driver->reg_cache_size && + !codec->cache_bypass) { ret = snd_soc_cache_write(codec, reg, value); if (ret < 0) return -1; @@ -216,7 +221,8 @@ static unsigned int snd_soc_8_8_read(struct snd_soc_codec *codec,
reg &= 0xff; if (reg >= codec->driver->reg_cache_size || - snd_soc_codec_volatile_register(codec, reg)) { + snd_soc_codec_volatile_register(codec, reg) || + codec->cache_bypass) { if (codec->cache_only) return -1;
@@ -271,7 +277,8 @@ static int snd_soc_8_16_write(struct snd_soc_codec *codec, unsigned int reg, data[2] = value & 0xff;
if (!snd_soc_codec_volatile_register(codec, reg) && - reg < codec->driver->reg_cache_size) { + reg < codec->driver->reg_cache_size && + !codec->cache_bypass) { ret = snd_soc_cache_write(codec, reg, value); if (ret < 0) return -1; @@ -295,7 +302,8 @@ static unsigned int snd_soc_8_16_read(struct snd_soc_codec *codec, unsigned int val;
if (reg >= codec->driver->reg_cache_size || - snd_soc_codec_volatile_register(codec, reg)) { + snd_soc_codec_volatile_register(codec, reg) || + codec->cache_bypass) { if (codec->cache_only) return -1;
@@ -450,7 +458,8 @@ static unsigned int snd_soc_16_8_read(struct snd_soc_codec *codec,
reg &= 0xff; if (reg >= codec->driver->reg_cache_size || - snd_soc_codec_volatile_register(codec, reg)) { + snd_soc_codec_volatile_register(codec, reg) || + codec->cache_bypass) { if (codec->cache_only) return -1;
@@ -476,7 +485,8 @@ static int snd_soc_16_8_write(struct snd_soc_codec *codec, unsigned int reg,
reg &= 0xff; if (!snd_soc_codec_volatile_register(codec, reg) && - reg < codec->driver->reg_cache_size) { + reg < codec->driver->reg_cache_size && + !codec->cache_bypass) { ret = snd_soc_cache_write(codec, reg, value); if (ret < 0) return -1; @@ -568,7 +578,8 @@ static unsigned int snd_soc_16_16_read(struct snd_soc_codec *codec, unsigned int val;
if (reg >= codec->driver->reg_cache_size || - snd_soc_codec_volatile_register(codec, reg)) { + snd_soc_codec_volatile_register(codec, reg) || + codec->cache_bypass) { if (codec->cache_only) return -1;
@@ -595,7 +606,8 @@ static int snd_soc_16_16_write(struct snd_soc_codec *codec, unsigned int reg, data[3] = value & 0xff;
if (!snd_soc_codec_volatile_register(codec, reg) && - reg < codec->driver->reg_cache_size) { + reg < codec->driver->reg_cache_size && + !codec->cache_bypass) { ret = snd_soc_cache_write(codec, reg, value); if (ret < 0) return -1;
Incorporate the use of the cache_bypass functionality in the syncing functions. The snd_soc_flat_cache_sync() need not be hooked as there is no performance benefit from using the cache_bypass option.
Signed-off-by: Dimitris Papastamos dp@opensource.wolfsonmicro.com Acked-by: Liam Girdwood lrg@slimlogic.co.uk Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com --- sound/soc/soc-cache.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-cache.c b/sound/soc/soc-cache.c index 799e376..9d43bee 100644 --- a/sound/soc/soc-cache.c +++ b/sound/soc/soc-cache.c @@ -847,7 +847,9 @@ static int snd_soc_rbtree_cache_sync(struct snd_soc_codec *codec) ret = snd_soc_cache_read(codec, rbnode->reg, &val); if (ret) return ret; + codec->cache_bypass = 1; ret = snd_soc_write(codec, rbnode->reg, val); + codec->cache_bypass = 0; if (ret) return ret; dev_dbg(codec->dev, "Synced register %#x, value = %#x\n", @@ -1134,7 +1136,9 @@ static int snd_soc_lzo_cache_sync(struct snd_soc_codec *codec) ret = snd_soc_cache_read(codec, i, &val); if (ret) return ret; + codec->cache_bypass = 1; ret = snd_soc_write(codec, i, val); + codec->cache_bypass = 0; if (ret) return ret; dev_dbg(codec->dev, "Synced register %#x, value = %#x\n",
On Fri, Feb 11, 2011 at 02:54:01PM +0000, Dimitris Papastamos wrote:
This is primarily needed to avoid writing back to the cache whenever we are syncing the cache with the hardware. This gives a performance benefit especially for large register maps.
Hrm, this is a rather large and invasive patch to be applying as part of a bug fix for a specific driver this late in the release process. Can we do something driver specific for the 2.6.38 release?
On Fri, Feb 11, 2011 at 03:18:26PM +0000, Mark Brown wrote:
On Fri, Feb 11, 2011 at 02:54:01PM +0000, Dimitris Papastamos wrote:
This is primarily needed to avoid writing back to the cache whenever we are syncing the cache with the hardware. This gives a performance benefit especially for large register maps.
Hrm, this is a rather large and invasive patch to be applying as part of a bug fix for a specific driver this late in the release process. Can we do something driver specific for the 2.6.38 release?
Em, make WM8994_POWER_MANAGEMENT_5 a volatile register? That is sort of a hack but I can't think of anything else.
Thanks, Dimitris
On Fri, Feb 11, 2011 at 03:21:03PM +0000, Dimitris Papastamos wrote:
On Fri, Feb 11, 2011 at 03:18:26PM +0000, Mark Brown wrote:
On Fri, Feb 11, 2011 at 02:54:01PM +0000, Dimitris Papastamos wrote:
This is primarily needed to avoid writing back to the cache whenever we are syncing the cache with the hardware. This gives a performance benefit especially for large register maps.
Hrm, this is a rather large and invasive patch to be applying as part of a bug fix for a specific driver this late in the release process. Can we do something driver specific for the 2.6.38 release?
Em, make WM8994_POWER_MANAGEMENT_5 a volatile register? That is sort of a hack but I can't think of anything else.
Now that I think about it, that won't work. We still need to be able to sync the other bitfields in that register on resume.
Thanks, Dimitris
On Fri, Feb 11, 2011 at 03:18:26PM +0000, Mark Brown wrote:
On Fri, Feb 11, 2011 at 02:54:01PM +0000, Dimitris Papastamos wrote:
This is primarily needed to avoid writing back to the cache whenever we are syncing the cache with the hardware. This gives a performance benefit especially for large register maps.
Hrm, this is a rather large and invasive patch to be applying as part of a bug fix for a specific driver this late in the release process. Can we do something driver specific for the 2.6.38 release?
Would this be acceptable? In wm8994_resume():
/* force a HW read */ val = wm8994_reg_read(codec->control_data, WM8994_POWER_MANAGEMENT_5);
instead of the call to snd_soc_read().
Thanks, Dimitris
On Fri, Feb 11, 2011 at 03:46:54PM +0000, Dimitris Papastamos wrote:
Would this be acceptable? In wm8994_resume():
/* force a HW read */ val = wm8994_reg_read(codec->control_data, WM8994_POWER_MANAGEMENT_5);
instead of the call to snd_soc_read().
Yes, that looks good. We can then update to use the APIs more normally in 2.6.39.
participants (2)
-
Dimitris Papastamos -
Mark Brown