From 1c2d6ac10193b394e396924143a6f10d0be7a2ae Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Thu, 14 Mar 2024 19:21:54 +0200 Subject: [PATCH 1/5] Audio: Volume: Fix ongoing gain ramp stop and wrong final gain In IPC4 the individual channel gains are passed in separate messages. One channel may have started to ramp to a new gain while a new channel gain arrives for other channel. If the gain is same as the previous control value, the ongoing ramp is stopped and the gain remains at a transition value. The incorrect code is not fixed but instead the volume_set_volume() function is simplified. When a volume control is received, it is assumed that pass-through mode is disabled and ramp is prepared. If the control is received but gains are not changed, the ramp mode is finished and pass-through is restored in ramp function. The volume_set_switch() function is updated similarly to ensure similar operation. Signed-off-by: Seppo Ingalsuo --- src/audio/volume/volume_ipc4.c | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/src/audio/volume/volume_ipc4.c b/src/audio/volume/volume_ipc4.c index d12eed346d77..ddac8dac0d07 100644 --- a/src/audio/volume/volume_ipc4.c +++ b/src/audio/volume/volume_ipc4.c @@ -79,6 +79,8 @@ static uint32_t convert_volume_ipc3_to_ipc4(uint32_t volume) static void init_ramp(struct vol_data *cd, uint32_t curve_duration, uint32_t target_volume) { + cd->ramp_finished = false; + /* In IPC4 driver sends curve_duration in hundred of ns - it should be * converted into ms value required by firmware */ @@ -213,8 +215,6 @@ static int volume_set_volume(struct processing_module *mod, const uint8_t *data, } init_ramp(cd, cdata.curve_duration, cdata.target_volume); - cd->ramp_finished = true; - channels_count = mod->priv.cfg.base_cfg.audio_fmt.channels_count; if (channels_count > SOF_IPC_MAX_CHANNELS) { comp_err(dev, "Invalid channels count %u", channels_count); @@ -232,8 +232,6 @@ static int volume_set_volume(struct processing_module *mod, const uint8_t *data, volume_set_chan(mod, i, cd->tvolume[i], true); } - if (cd->volume[i] != cd->tvolume[i]) - cd->ramp_finished = false; } } else { if (cd->muted[cdata.channel_id]) { @@ -247,25 +245,12 @@ static int volume_set_volume(struct processing_module *mod, const uint8_t *data, volume_set_chan(mod, cdata.channel_id, cd->tvolume[cdata.channel_id], true); } - if (cd->volume[cdata.channel_id] != cd->tvolume[cdata.channel_id]) - cd->ramp_finished = false; - } - - cd->is_passthrough = cd->ramp_finished; - - for (i = 0; i < channels_count; i++) { - if (cd->volume[i] != VOL_ZERO_DB) { - cd->is_passthrough = false; - break; - } } + cd->is_passthrough = false; volume_set_ramp_channel_counter(cd, channels_count); - cd->scale_vol = vol_get_processing_function(dev, cd); - volume_prepare_ramp(dev, cd); - return 0; } @@ -322,8 +307,6 @@ static int volume_set_switch(struct processing_module *mod, const uint8_t *data, ctl = (struct sof_ipc4_control_msg_payload *)data; - cd->ramp_finished = true; - channels_count = mod->priv.cfg.base_cfg.audio_fmt.channels_count; if (channels_count > SOF_IPC_MAX_CHANNELS) { comp_err(dev, "Invalid channels count %u", channels_count); @@ -344,11 +327,13 @@ static int volume_set_switch(struct processing_module *mod, const uint8_t *data, volume_set_chan_unmute(mod, i); else volume_set_chan_mute(mod, i); - - if (cd->volume[i] != cd->tvolume[i]) - cd->ramp_finished = false; } + cd->ramp_finished = false; + cd->is_passthrough = false; + volume_set_ramp_channel_counter(cd, channels_count); + cd->scale_vol = vol_get_processing_function(dev, cd); + volume_prepare_ramp(dev, cd); return 0; } From 5efacbc256e718386cbf253a5ca2817b12c0a37c Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Fri, 15 Mar 2024 14:04:30 +0200 Subject: [PATCH 2/5] Audio: Volume: Add linear ramp coefficient function with fixes The linear slope coefficient calculation is moved to a separate function. Two functional changes are done. - The ramp_coef for channel is set to zero if there is no transition for the channel. The ensure of non-zero coefficient is only needed if there is a transition that is so slow that the slope coefficient would round to zero. If this function is called for equal volume and tvolume for channel, the ramp_coef remains zero, and not smallest non-zero value. - The handling of ramp disable with zero initial_ramp is changed to similar as for windows fade. The set of coefficient to volume delta (a large value for large volume jump) is not correct even if it appeared to work. It is a remain from old code where ramp was not a function of time but a constant step added. Signed-off-by: Seppo Ingalsuo --- src/audio/volume/volume.c | 108 +++++++++++++++++++++----------------- 1 file changed, 59 insertions(+), 49 deletions(-) diff --git a/src/audio/volume/volume.c b/src/audio/volume/volume.c index 2bb5c2a5842d..bf4833173002 100644 --- a/src/audio/volume/volume.c +++ b/src/audio/volume/volume.c @@ -216,6 +216,9 @@ static const struct comp_zc_func_map zc_func_map[] = { */ static inline int32_t volume_linear_ramp(struct vol_data *cd, int32_t ramp_time, int channel) { + if (!cd->initial_ramp) + return cd->tvolume[channel]; + return cd->rvolume[channel] + ramp_time * cd->ramp_coef[channel]; } #endif @@ -415,6 +418,59 @@ static int volume_free(struct processing_module *mod) return 0; } +static void set_linear_ramp_coef(struct vol_data *cd, int chan, bool constant_rate_ramp) +{ + int32_t delta; + int32_t delta_abs; + int32_t coef; + + if (!cd->initial_ramp) + return; + + /* Get volume transition delta and absolute value */ + delta = cd->tvolume[chan] - cd->volume[chan]; + if (!delta) { + cd->ramp_coef[chan] = 0; + return; + } + + delta_abs = ABS(delta); + + /* The ramp length (initial_ramp [ms]) describes time of mute + * to vol_max unmuting. Normally the volume ramp has a + * constant linear slope defined this way and variable + * completion time. However in streaming start it is feasible + * to apply the entire topology defined ramp time to unmute to + * any used volume. In this case the ramp rate is not constant. + * Note also the legacy mode without known vol_ramp_range where + * the volume transition always uses the topology defined time. + */ + if (constant_rate_ramp && cd->vol_ramp_range > 0) + coef = cd->vol_ramp_range; + else + coef = delta_abs; + + /* Divide and round to nearest. Note that there will + * be some accumulated error in ramp time the longer + * the ramp and the smaller the transition is. + */ + coef = (2 * coef / cd->initial_ramp + 1) >> 1; + + /* Scale coefficient by 1/8, round */ + coef = ((coef >> 2) + 1) >> 1; + + /* Ensure ramp coefficient is at least min. non-zero + * fractional value. + */ + coef = MAX(coef, 1); + + /* Invert sign for volume down ramp step */ + if (delta < 0) + coef = -coef; + + cd->ramp_coef[chan] = coef; +} + /** * \brief Sets channel target volume. * \param[in,out] mod Volume processing module handle @@ -429,9 +485,6 @@ int volume_set_chan(struct processing_module *mod, int chan, { struct vol_data *cd = module_get_private_data(mod); int32_t v = vol; - int32_t delta; - int32_t delta_abs; - int32_t coef; /* Limit received volume gain to MIN..MAX range before applying it. * MAX is needed for now for the generic C gain arithmetic to prevent @@ -456,52 +509,9 @@ int volume_set_chan(struct processing_module *mod, int chan, cd->rvolume[chan] = cd->volume[chan]; cd->vol_ramp_elapsed_frames = 0; - /* Check ramp type */ - if (cd->ramp_type == SOF_VOLUME_LINEAR || - cd->ramp_type == SOF_VOLUME_LINEAR_ZC) { - /* Get volume transition delta and absolute value */ - delta = cd->tvolume[chan] - cd->volume[chan]; - delta_abs = ABS(delta); - - /* The ramp length (initial_ramp [ms]) describes time of mute - * to vol_max unmuting. Normally the volume ramp has a - * constant linear slope defined this way and variable - * completion time. However in streaming start it is feasible - * to apply the entire topology defined ramp time to unmute to - * any used volume. In this case the ramp rate is not constant. - * Note also the legacy mode without known vol_ramp_range where - * the volume transition always uses the topology defined time. - */ - if (cd->initial_ramp > 0) { - if (constant_rate_ramp && cd->vol_ramp_range > 0) - coef = cd->vol_ramp_range; - else - coef = delta_abs; - - /* Divide and round to nearest. Note that there will - * be some accumulated error in ramp time the longer - * the ramp and the smaller the transition is. - */ - coef = (2 * coef / cd->initial_ramp + 1) >> 1; - } else { - coef = delta_abs; - } - - /* Scale coefficient by 1/8, round */ - coef = ((coef >> 2) + 1) >> 1; - - /* Ensure ramp coefficient is at least min. non-zero - * fractional value. - */ - coef = MAX(coef, 1); - - /* Invert sign for volume down ramp step */ - if (delta < 0) - coef = -coef; - - cd->ramp_coef[chan] = coef; - comp_dbg(mod->dev, "cd->ramp_coef[%d] = %d", chan, cd->ramp_coef[chan]); - } + /* Ramp type specific initialize */ + if (cd->ramp_type == SOF_VOLUME_LINEAR || cd->ramp_type == SOF_VOLUME_LINEAR_ZC) + set_linear_ramp_coef(cd, chan, constant_rate_ramp); return 0; } From 3bd5d02ad583c36b2f8798a111c8b4dba9bf729f Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Fri, 15 Mar 2024 16:38:11 +0200 Subject: [PATCH 3/5] Audio: Volume: Fix condition for identical ramp The "is_same_volume=true" was returned if target volumes for all channels are the same. The check omitted the fact that start volumes for ramp can be different, e.g. one channel is attenuated while others are 0 dB. This change fixes the random ignore of volume ramp and smooth transition when a volume control is changed. Signed-off-by: Seppo Ingalsuo --- src/audio/volume/volume.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/audio/volume/volume.c b/src/audio/volume/volume.c index bf4833173002..7c888212c0f3 100644 --- a/src/audio/volume/volume.c +++ b/src/audio/volume/volume.c @@ -252,7 +252,7 @@ void volume_set_ramp_channel_counter(struct vol_data *cd, uint32_t channels_coun bool is_same_volume = true; for (i = 1; i < channels_count; i++) { - if (cd->tvolume[0] != cd->tvolume[i]) { + if (cd->tvolume[0] != cd->tvolume[i] || cd->volume[0] != cd->volume[i]) { is_same_volume = false; break; } From cb9d1aeea00d60d4fd1e2d87ab97ce372a44f750 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Wed, 20 Mar 2024 17:20:03 +0200 Subject: [PATCH 4/5] Audio: Volume: Jump volume directly to target when no ramp This ensures that volume for a channel changes immediately after receiving the control if ramp duration is zero or if type is no fade. Signed-off-by: Seppo Ingalsuo --- src/audio/volume/volume.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/audio/volume/volume.c b/src/audio/volume/volume.c index 7c888212c0f3..499fd67ff767 100644 --- a/src/audio/volume/volume.c +++ b/src/audio/volume/volume.c @@ -513,6 +513,9 @@ int volume_set_chan(struct processing_module *mod, int chan, if (cd->ramp_type == SOF_VOLUME_LINEAR || cd->ramp_type == SOF_VOLUME_LINEAR_ZC) set_linear_ramp_coef(cd, chan, constant_rate_ramp); + if (!cd->initial_ramp || cd->ramp_type == SOF_VOLUME_WINDOWS_NO_FADE) + cd->volume[chan] = v; + return 0; } From ba8af5891fb22aa4cc1e1af11c69ade87c72b223 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Wed, 20 Mar 2024 17:22:10 +0200 Subject: [PATCH 5/5] Audio: Volume: No update of process function for every ramp value The volume process function for pass-through can be changed when all ramps are completed. This change that avoids processing function lookup in worst case every 128 us is measured to save in TGL platform about 1 MCPS from CPU_PEAK(MAX). Signed-off-by: Seppo Ingalsuo --- src/audio/volume/volume.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/audio/volume/volume.c b/src/audio/volume/volume.c index 499fd67ff767..a16819a985b6 100644 --- a/src/audio/volume/volume.c +++ b/src/audio/volume/volume.c @@ -347,7 +347,8 @@ static inline void volume_ramp(struct processing_module *mod) } } - set_volume_process(cd, dev, true); + if (cd->is_passthrough) + set_volume_process(cd, dev, true); } /**