[PATCH] ASoC: codecs: lpass-wsa-macro: rewrite the interpolator volume after enabling clocks

[PATCH] ASoC: codecs: lpass-wsa-macro: rewrite the interpolator volume after enabling clocks

[Liviu Nicoara]

Commit 902f497a1ff5 ("ASoC: codecs: lpass-wsa-macro: remove useless
gain read/write sequence") removed the read and write of the digital
volume register in wsa_macro_enable_interpolator(), on the grounds that
writing back the value just read does nothing. The comment above it,
"apply gain after int clk is enabled", was left in place.

On the Dell XPS 13 9345 (X1E80100, four WSA8845 amplifiers on two WSA
macros) the write does something: a volume change made while the path
is idle does not take effect when playback starts. Lowering the digital
volume from 81 to 63 with nothing playing, then playing a test tone,
gave about the same level as before the change. With the rewrite
restored, lowering it from 81 to 69 while idle played audibly quieter,
and restoring 81 while idle brought the level back. Changes made during
playback take effect with or without the rewrite.

This is the behaviour described in commit 46188db080bd ("ASoC: codecs:
lpass-wsa-macro: fix compander volume hack"): "the volume registers
still need to be written after enabling clocks in order for any prior
updates to take effect." The value read comes from the register cache,
so the write pushes the last requested volume to the hardware once its
clock runs.

Restore the rewrite in the interpolator's POST_PMU event only. The mix
path event removed later in the same series is not brought back.

Fixes: 902f497a1ff5 ("ASoC: codecs: lpass-wsa-macro: remove useless gain read/write sequence")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Liviu Nicoara <lnicoara@thinkoid.org>
---

Notes:
    Tested on v7.2.6 on the machine above, by listening. This function is
    unchanged between v7.2 and for-next; the driver's clocks moved to the
    PM clock framework in cd054a6e272c after v7.2, which was not tested
    here. Build-tested on broonie/for-next (arm64 defconfig).

 sound/soc/codecs/lpass-wsa-macro.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index 4242366601a5..899c8bee4645 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -1849,6 +1849,7 @@ static int wsa_macro_enable_interpolator(struct snd_soc_dapm_widget *w,
 					 int event)
 {
 	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+	unsigned int gain;
 	u16 gain_reg;
 	u16 reg;
 	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
@@ -1890,6 +1891,8 @@ static int wsa_macro_enable_interpolator(struct snd_soc_dapm_widget *w,
 					CDC_WSA_RX_PGA_HALF_DB_MASK,
 					CDC_WSA_RX_PGA_HALF_DB_ENABLE);
 		}
+		gain = snd_soc_component_read(component, gain_reg);
+		snd_soc_component_write(component, gain_reg, gain);
 		wsa_macro_config_ear_spkr_gain(component, wsa,
 						event, gain_reg);
 		break;
-- 
2.55.0

Re: [PATCH] ASoC: codecs: lpass-wsa-macro: rewrite the interpolator volume after enabling clocks

[Jonathan Marek]

this is just papering over the real problem, which would be the regmap 
cache being out of sync with the HW registers. there's no requirement 
that the volume register be written after the int/path clock enable

(fwiw, this problem doesn't reproduce on my setup, where this driver 
does not autosuspend and the regmap doesn't go into cache-only mode)

On 9/24/26 8:59 AM, Liviu Nicoara wrote:
> Commit 902f497a1ff5 ("ASoC: codecs: lpass-wsa-macro: remove useless
> gain read/write sequence") removed the read and write of the digital
> volume register in wsa_macro_enable_interpolator(), on the grounds that
> writing back the value just read does nothing. The comment above it,
> "apply gain after int clk is enabled", was left in place.
> 
> On the Dell XPS 13 9345 (X1E80100, four WSA8845 amplifiers on two WSA
> macros) the write does something: a volume change made while the path
> is idle does not take effect when playback starts. Lowering the digital
> volume from 81 to 63 with nothing playing, then playing a test tone,
> gave about the same level as before the change. With the rewrite
> restored, lowering it from 81 to 69 while idle played audibly quieter,
> and restoring 81 while idle brought the level back. Changes made during
> playback take effect with or without the rewrite.
> 
> This is the behaviour described in commit 46188db080bd ("ASoC: codecs:
> lpass-wsa-macro: fix compander volume hack"): "the volume registers
> still need to be written after enabling clocks in order for any prior
> updates to take effect." The value read comes from the register cache,
> so the write pushes the last requested volume to the hardware once its
> clock runs.
> 
> Restore the rewrite in the interpolator's POST_PMU event only. The mix
> path event removed later in the same series is not brought back.
> 
> Fixes: 902f497a1ff5 ("ASoC: codecs: lpass-wsa-macro: remove useless gain read/write sequence")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Liviu Nicoara <lnicoara@thinkoid.org>
> ---
> 
> Notes:
>      Tested on v7.2.6 on the machine above, by listening. This function is
>      unchanged between v7.2 and for-next; the driver's clocks moved to the
>      PM clock framework in cd054a6e272c after v7.2, which was not tested
>      here. Build-tested on broonie/for-next (arm64 defconfig).
> 
>   sound/soc/codecs/lpass-wsa-macro.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
> index 4242366601a5..899c8bee4645 100644
> --- a/sound/soc/codecs/lpass-wsa-macro.c
> +++ b/sound/soc/codecs/lpass-wsa-macro.c
> @@ -1849,6 +1849,7 @@ static int wsa_macro_enable_interpolator(struct snd_soc_dapm_widget *w,
>   					 int event)
>   {
>   	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
> +	unsigned int gain;
>   	u16 gain_reg;
>   	u16 reg;
>   	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
> @@ -1890,6 +1891,8 @@ static int wsa_macro_enable_interpolator(struct snd_soc_dapm_widget *w,
>   					CDC_WSA_RX_PGA_HALF_DB_MASK,
>   					CDC_WSA_RX_PGA_HALF_DB_ENABLE);
>   		}
> +		gain = snd_soc_component_read(component, gain_reg);
> +		snd_soc_component_write(component, gain_reg, gain);
>   		wsa_macro_config_ear_spkr_gain(component, wsa,
>   						event, gain_reg);
>   		break;
> 

Re: [PATCH] ASoC: codecs: lpass-wsa-macro: rewrite the interpolator volume after enabling clocks

[Liviu Nicoara]

On Thu, Sep 24, 2026 at 3:02 PM Jonathan Marek <jonathan@marek.ca> wrote:
>
> this is just papering over the real problem, which would be the regmap
> cache being out of sync with the HW registers. there's no requirement
> that the volume register be written after the int/path clock enable
>
> (fwiw, this problem doesn't reproduce on my setup, where this driver
> does not autosuspend and the regmap doesn't go into cache-only mode)
>
>

Thanks for looking. I checked whether the cache and the hardware
disagree on this machine (v7.2.6, where the macro runtime-suspends after
3 s). With the macro suspended, I changed the digital volume from 81 to
69, which updates only the cache.  I then forced a runtime resume, which
runs regcache_sync(), and read the volume register straight from the
hardware with the regmap debugfs cache_bypass: it held the new value
(0xf1).

However, on my unpatched driver an idle volume change did not take place
when playback starts (changed it from 69 to 81 and it played at 69). A
change done during playback though, takes effect at once. I don't think
anything writes the register in between.

The register holds the new value, but the path does not use it until it
is written with the interpolator clock running. The read/write-back that
902f497a1ff5 removed did exactly that; Johan kept it deliberately in
46188db080bd ("Note that the volume registers still need to be written
after enabling clocks in order for any prior updates to take
effect"). Unless I am reading it wrong.

On your setup, does a volume change made with no stream running take
effect when the next stream starts?  If your changes land while the path
is active, that could explain why it does not reproduce there.

Thanks again.

Re: [PATCH] ASoC: codecs: lpass-wsa-macro: rewrite the interpolator volume after enabling clocks

[Jonathan Marek]

On 9/24/26 3:52 PM, Liviu Nicoara wrote:> However, on my unpatched 
driver an idle volume change did not take place
> when playback starts (changed it from 69 to 81 and it played at 69). A
> change done during playback though, takes effect at once. I don't think
> anything writes the register in between.

in this case, while its playing with the wrong volume, use /dev/mem to 
directly read the value of the volume register and then write the same 
value, if that fixes the volume from 69 to 81, then this patch might be 
necessary, otherwise its papering over the issue like I wrote

Re: [PATCH] ASoC: codecs: lpass-wsa-macro: rewrite the interpolator volume after enabling clocks

[Liviu Nicoara]

> in this case, while its playing with the wrong volume, use /dev/mem to
> directly read the value of the volume register and then write the same
> value, if that fixes the volume from 69 to 81, then this patch might be
> necessary, otherwise its papering over the issue like I wrote

Done, on the unpatched driver (v7.2.6, stock lpass-wsa-macro, booted
with iomem=relaxed so /dev/mem can reach the macro at 0x6b00000):

I played back a tone that I changed from 81 to 69 mid-play and the
volume went down audibly. It read back 0xf1 through /dev/mem. Then:

1. With the macro runtime-suspended, I changed WSA_RX0 Digital Volume
   from 69 to 81.
2. I started playback: it played at the 69 level, quieter.
3. While it was playing, a /dev/mem read of 0x6b00414 returned 0xfd
   (81).
4. I wrote 0xfd back to 0x6b00414 through /dev/mem: the volume went up
   to 81 at once.

So the hardware register already held the new value, and rewriting it
with the path running, without regmap involved, is what made it take
effect.