[BUG] v1.5 - Digital Microphone issues

[perexg]

Issues:

• the controls were (again!) renamed from "Dmic0 Capture Volume" to "Dmic0 10 Capture Volume" ?
• the dmic volume is weaker than with 1.4.2

Control settings:

numid=49,iface=MIXER,name='Dmic0 10 Capture Switch'
  ; type=BOOLEAN,access=rw------,values=2
  : values=on,on
numid=48,iface=MIXER,name='Dmic0 10 Capture Volume'
  ; type=INTEGER,access=rw---R--,values=2,min=0,max=70,step=0
  : values=70,70
  | dBscale-min=-50.00dB,step=1.00dB,mute=1
numid=51,iface=MIXER,name='Dmic1 Capture Volume'
  ; type=INTEGER,access=rw---R--,values=2,min=0,max=70,step=0
  : values=70,70
  | dBscale-min=-50.00dB,step=1.00dB,mute=1

Testing sof-bin - v1.5 branch...

The peak is only ~1% for my voice with this setting using the arecord test:
arecord -vvv -f dat -c 4 -D hw:0,6 /tmp/a.wav

[juimonen]

@perexg, crap, this is clear bug from us... We obviously have still issues testing the full stack from kernel to user space, dmic being the hard one. It is caused by firmware commit d65495e. This needs to be fixed.

Before we might have had also amplifying eq parameters so you could have better volume then, @singalsu FYI.

[singalsu]

@perexg What device model is this and what topology name does the kernel load? We seem to have a confusion with gains, sorry for that. Also does the above indicate that you have already maximized the available user volume control for capture (70. 70)?

[kv2019i]

FYI @lgirdwood @mengdonglin @fredoh9

This seems to be a regression with this patch:
#2660

The patch changed ALSA mixer controls which our UCMs rely on:
https://github.com/alsa-project/alsa-ucm-conf/blob/master/ucm2/sof-hda-dsp/HiFi.conf

... I think this calls for a 1.5 hotfix. Can you @mengdonglin check how this passed validation?

[perexg]

HW: It's Lenovo Carbon X1 7th gen (sof-cnl.ri + sof-hda-generic-idisp-4ch.tplg).
Volumes: Yes, I tried to increase the appropriate volume controls to max - if you have another ideas, I can try them.

[juimonen]

@fredoh9 can you take a look at this? If I remove the pipeline numbering "index" values cause issues with alsatplg. Old topologies before your commit had dmic indexes close to each other, but now when the dmic0 and dmic1 index are separated far away, alsatplg can't compile same name mixer elements...

@ranj063 you have any idea how this should work? And what's the meaning of index values in topology? I understood it was grouping the widgets together, but now I'm puzzled...

[juimonen]

hmm, I think the change was made because we had 2 "Capture Volume" control definitions (name identical) for Dmic0 and Dmic1 in conf file. I think ASoC layer is combining these for alsamixer as, so we get nice "Dmic0 Capture Volume".

But as I now see the alsa-lib conf parsing code, it is actually merging the "duplicate" definitions, so we have in essence only 1 "Capture Volume" definition. It was working before, because the Dmic0 and Dmic1 "index" definition we're close to each other (something like 7 & 8) and alsa conf parsing survives this for some reason. So it finds the single "Capture Volume" for both Dmic0 and Dmic1. Now with change in 1.5 fw the indexes are far apart (like 3 vs 8) and alsa conf parser bails out. And that's probably @fredoh9 was adding the number to the control name.

I'm little bit confused how this index thing would work before and not anymore...

Anyway it seems that if we want to have 2 separate pga's with 2 separate controls, they all need to have different name. This however makes the naming little bit funny as we would get:
"Dmic0 Capture Volume 0" and "Dmic1 Capture Volume 1". You can replace the last number with foo/bar, but this kind of explains why we have pga monster names in the topologies.

If I add a ?-mark like following to the 2 section names:
SectionControlMixer.?"Capture Volume"
I get the alsatplg to create the tplg binary with 2 capture volumes and the indexing doesn't matter, but I'm not sure about the semantics of the ?-sign here...
It says "? = do not override" in alsaconf docs, but not sure it affects as we want here.

@perexg you have any insights of the above behavior, or any preference about the naming?

[perexg]

Sorry, I don't get what you're doing with the alsa-lib configs. Could you show me the pieces you refer (from the .conf files not .m4 files)?

[juimonen]

@perexg sorry for being confusing... I'm just trying to understand how the conf parser works and I need to jump between alsa-utils and alsa-lib.

Attached a conf where I removed the volume control numbering (working as the old way) and it doesn't compile with alsatplg. I renamed the conf to txt as you can't attach otherwise

• However if you change the second "Capture Volume" index from 8 to 4, it compiles(????).
• Also if you add ?-sign in front of the MixerSections like: SectionControlMixer.?"Capture Volume" {
  it compiles.
  testi.txt

[perexg]

You can imagine the configuration like a tree. You have this in your config:

SectionControlMixer."Capture Volume" {
   Index 3
}
SectionControlMixer."Capture Volume" {
   Index 8
}

The result after load is:

SectionControlMixer."Capture Volume" {
   index 8
}

If I normalize your unreadable configs using alsatplg -n t.conf -o n.conf the problem is visible:

SectionControlMixer {
  'Capture Switch' {
     index 3
  }
  `Capture Volume' {
     index 8
  }
}

There is no 'index 3' section for 'Capture Volume', because configs get merged (identical identifier "Capture Volume" in the configuration tree) - last parsed entry wins in this case.

The 'Dmic0' in SectionWidget has 'index 3', thus you need to define the SectionControlMixer controls with this 'index 3'.

The '?' or '-' operations (?='replace' or minus='remove before parse') won't help you, because you cannot link blocks from the SectionWidget to SectionControlMixer with different indexes (functional groups).

Just choose a different name for "Capture Volume" for Dmic1 like "Second Capture Volume". It's ugly, but you're in trouble because nobody in ASoC use the right ALSA control indexes (do not mix with the functional topology group). ALSA allows multiple controls with the same name with the only integer index difference like "Capture Volume,0" and "Capture Volume,1". Another way is to put Dmic0 and Dmic1 mixer controls to the one functional group (index).

I would really suggest to add the proper support for the ALSA mixer control indexes to ASoC and topology.

[perexg]

Thinking more: Perhaps, it may be sufficient to add a suffix to the control mixer name specified in SectionControlMixer and strip it in the topology library when the contents is written to the output binary file. Like "Capture Volume|Dmic0" where "|Dmic0" will be stripped. If I understand correctly, the identical mixer control names may exist if they're in different functional groups, right?

[juimonen]

@perexg, Yes the idea was exactly that we would have same named controls, but in different indexes. So the prefixing pga name would be the differentiator. But as you said, with current methods, to get this right, we need to have different control names also.

Anyway this current method of attaching pipeline id to control name is bad, because it changes all the time (or can change). So it needs to be changed now to some static identifier, but that requires changes in UCM configs also.

[juimonen]

I made now this:
#2942

So Dmic0 name remains as it was and Dmic1 is indexed. I think we don't currently reference Dmic1 in upstream UCM's so UCM's should work with this....

[perexg]

It's just a workaround. I would really prefer a fix. Some thoughts:

1. the SectionControlMixer controls are static, so the name should be unique and adding a next extra prefix in the driver seems like a big mistake (why not define directly 'Dmic0 Volume Control') ?

2. allow private instantions of the control mixer elements for DAPM widget (enhance the config syntax and improve the alsa-lib's topology code)