[Sound-open-firmware] [PATCH 2/5] Volume: Replace SOF_CTRL_CMD_MUTE/UNMUTE with SOF_CTRL_CMD_SWITCH

[Seppo Ingalsuo]

On 21.10.2017 00:38, Pierre-Louis Bossart wrote:
> On 10/20/17 12:15 PM, Seppo Ingalsuo wrote:
>> Due to previous patch the volume mute is updated to be controlled with
>> SOF_CTRL_CMD_SWITCH that maps to ALSA switch style control. Mute for a
>> channel is set with an (unsigned) value larger than zero. Zero value
>> unmutes the channel.
>
> Mute and switch are not synonyms - the results are inverted.
> ALSA switches set to 1 means enable audio (unmute), 0 means mute.
> we've had this confusion in the past with inverted logic for the SST 
> driver.
> We should also treat switch as a boolean, it's zero or one so we can 
> test with switch or !switch.

Oops, I'll correct this. The same issue is also in patch 4 and 5 for EQs.

Thanks,
Seppo


>
>>
>> In both mute and volume set code the if statement to compare component
>> channel map is changed to compare to chanv[j].channel instead of used
>> chanv[j].value that looks incorrect.
>>
>> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo at linux.intel.com>
>> ---
>>   src/audio/volume.c | 29 ++++++++++++++---------------
>>   1 file changed, 14 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/audio/volume.c b/src/audio/volume.c
>> index 137be3c..28d3225 100644
>> --- a/src/audio/volume.c
>> +++ b/src/audio/volume.c
>> @@ -449,36 +449,35 @@ static int volume_ctrl_set_cmd(struct comp_dev 
>> *dev, struct sof_ipc_ctrl_data *c
>>         switch (cdata->cmd) {
>>       case SOF_CTRL_CMD_VOLUME:
>> -
>> +        trace_volume("vst");
>>           for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {
>>               for (j = 0; j < cdata->num_elems; j++) {
>> -                if (cdata->chanv[j].value == cd->chan[i])
>> +                tracev_value(cdata->chanv[j].channel);
>> +                tracev_value(cdata->chanv[j].value);
>> +                if (cdata->chanv[j].channel == cd->chan[i])
>>                       volume_set_chan(dev, i, cdata->chanv[j].value);
>>               }
>>           }
>> -
>>           work_schedule_default(&cd->volwork, VOL_RAMP_US);
>>           break;
>> -    case SOF_CTRL_CMD_MUTE:
>>   +    case SOF_CTRL_CMD_SWITCH:
>> +        trace_volume("mst");
>>           for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {
>>               for (j = 0; j < cdata->num_elems; j++) {
>> -                if (cdata->chanv[j].value == cd->chan[i])
>> -                    volume_set_chan_mute(dev, i);
>> +                tracev_value(cdata->chanv[j].channel);
>> +                tracev_value(cdata->chanv[j].value);
>> +                if (cdata->chanv[j].channel == cd->chan[i]) {
>> +                    if (cdata->chanv[j].value > 0)
>> +                        volume_set_chan_mute(dev, i);
>> +                    else
>> +                        volume_set_chan_unmute(dev, i);
>> +                }
>>               }
>>           }
>>           work_schedule_default(&cd->volwork, VOL_RAMP_US);
>>           break;
>> -    case SOF_CTRL_CMD_UNMUTE:
>>   -        for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {
>> -            for (j = 0; j < cdata->num_elems; j++) {
>> -                if (cdata->chanv[j].value == cd->chan[i])
>> -                    volume_set_chan_unmute(dev, i);
>> -            }
>> -        }
>> -        work_schedule_default(&cd->volwork, VOL_RAMP_US);
>> -        break;
>>       default:
>>           trace_volume_error("gs1");
>>           return -EINVAL;
>>
>
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>