[BUG]When set capture volume to 0 DMIC 4ch still can capture voice #1799

Activity

1. mengdonglin commented on Sep 2, 2019

   

   mengdonglin

   on Sep 2, 2019

   Last edited by mengdonglin

   More actions

   @dengyangchao Could you check the volume parameter value sent by IPC for the 4 channels? It should be one IPC for all 4 channels?

   BTW, Seppo mentioned ASoC may only be able to deal with 2 channels.
2. zrombel commented on Sep 2, 2019

   

   zrombel

   on Sep 2, 2019

   Last edited by zrombel

   More actions

   Hi,
   I have tested this issue with slim driver with firmware 66da4e8 and master top 008285e and in both cases result are correct:

   
   Audio format 48kHz 16b/16b 4ch

   And this is the data that we are sending to FW via CompSetValue IPC
   "data": [
   {
   "channel": 0,
   "value": 0
   },
   {
   "channel": 1,
   "value": 0
   },
   {
   "channel": 2,
   "value": 0
   },
   {
   "channel": 3,
   "value": 0
   }
   ]
3. dengyangchao commented on Sep 2, 2019

   

   dengyangchao

   on Sep 2, 2019

   Author

   More actions

   @mengdonglin only two channel volume can be controlled by amixer/alsamixer:

   numid=8,iface=MIXER,name='PGA10.0 10 Master Capture Volume'
     ; type=INTEGER,access=rw---R--,values=2,min=0,max=70,step=0
     : values=52,51
     | dBscale-min=-50.00dB,step=1.00dB,mute=1
   

   below audio is captured after changing volume to 0 via alsamixer
   
4. keyonjie commented on Sep 9, 2019

   

   keyonjie

   on Sep 9, 2019

   More actions

   This should be a tplg bug, it is defined 2 channels for the kcontrol, please see:

   #
   # Controls
   #
   
   # Volume Mixer control with max value of 32
   SectionControlMixer."10 Master Capture Volume" {
   
           # control belongs to this index group
           index "10"
   
           #Channel register and shift for Front Left/Right
           channel."FL" {
                   reg "1"
                   shift "0"
           }
           channel."FR" {
                   reg "1"
                   shift "1"
           }
   
           # control uses bespoke driver get/put/info ID
           ops."ctl" {
                   info "volsw"
                   thesofproject/sof#256 binds the mixer control to volume get/put handlers
                   get "256"
                   put "256"
           }
   
           #
           max "70"
           invert "false"
           #TLV 80 steps from -50dB to +20dB for 1dB
           tlv "vtlv_m50s1"
   
   }
   
   
5. keyonjie commented on Sep 9, 2019

   

   keyonjie

   on Sep 9, 2019

   More actions

   @singalsu comments?
6. singalsu commented on Sep 9, 2019

   

   singalsu

   on Sep 9, 2019

   Last edited by singalsu

   Collaborator

   More actions

   I think there's problem that continuing this way the microphones > 2 should be mapped to ALSA channel maps those were designed for surround speakers system. I wonder if next two microphones could be surround-left (SL) and surround-right(SR). If it could work it could be low effort at topology level. But it could be very confusing since with microphones there's no clear "surround" usage. Also above 6 channels there's no main + surround speakers channels defined (except subwoofer and center channels).

   Alternatively every > 2ch volume controller could be exposed as multiple 2ch volume controllers. This would be cleaner but more effort in driver side. Also changing volumes exactly the same time would be challenge. But I doubt there's requirement for perfectly synchronized volume.

   @kv2019i @juimonen What do you think?

   Edit: Actually there's a lot more speaker locations defined in sound/sof/control.h (SOF_CHMAP_x). Could using those help us forward? Or could we add more like SOF_CHMAP_MICn that would be more logical with microphone arrays?

   Edit2: ALSA channel map is like https://www.kernel.org/doc/html/latest/sound/designs/channel-mapping-api.html or uapi/sound/asound.h.
7. kv2019i commented on Sep 19, 2019

   

   kv2019i

   on Sep 19, 2019

   Collaborator

   More actions

   @singalsu wrote:

   @kv2019i @juimonen What do you think?

   I think you are right. We need to expose the mixer controls as multiple stereo pair (smaller effort), or extend ALSA with the concept of >2ch mixer controls (much larger as none of the ALSA interfaces and the many tools are equipped to handle this).
8. plbossart commented on Sep 19, 2019

   

   plbossart

   on Sep 19, 2019

   Member

   More actions

   It may be worth expanding ALSA definitions, e.g. by using USB audio class 3 representations, see table 4.8
   Also for mics there are descriptions in NHLT of the position of each mic, so maybe we can tag text in the ALSA controls to provide additional information. I think in general the Microsoft definitions are used by everyone for mic arrays.

     | Channel Relationship | Acronym | Description
   -- | -- | -- | --
   Generic | RELATIONSHIP_UNDEFINED | UND | Relationship undefined or unknown
   MONO | M | A mono channel
   LEFT | L | A generic Left channel
   RIGHT | R | A generic Right channel
   ARRAY | AR | A channel that is part of an array configuration
   Input-Related | PATTERN_X | PX | The X channel of an X/Y microphone
   PATTERN_Y | PY | The Y channel of an X/Y microphone
   PATTERN_A | PA | The A channel of an A/B microphone
   PATTERN_B | PB | The B channel of an A/B microphone
   PATTERN_M | PM | The M channel of an M/S microphone
   PATTERN_S | PS | The S channel of an M/S microphone
   Output-Related | Front Left | FL | Refer to Figure 4-3
   Front Right | FR | Refer to Figure 4-3
   Front Center | FC | Refer to Figure 4-3
   Front Left of Center | FLC | Refer to Figure 4-3
   Front Right of Center | FRC | Refer to Figure 4-3
   Front Wide Left | FWL | Refer to Figure 4-3
   Front Wide Right | FWR | Refer to Figure 4-3
   Side Left | SL | Refer to Figure 4-3
   Side Right | SR | Refer to Figure 4-3
   Surround Array Left | SAL | Refer to Figure 4-3
   Surround Array Right | SAR | Refer to Figure 4-3
   Back Left | BL | Refer to Figure 4-3
   Back Right | BR | Refer to Figure 4-3
   Back Center | BC | Refer to Figure 4-3
   Back Left of Center | BLC | Refer to Figure 4-3
   Back Right of Center | BRC | Refer to Figure 4-3
   Back Wide Left | BWL | Refer to Figure 4-3
   Back Wide Right | BWR | Refer to Figure 4-3
   Top Center | TC | Refer to Figure 4-3
   Top Front Left | TFL | Refer to Figure 4-3
   Top Front Right | TFR | Refer to Figure 4-3
   Top Front Center | TFC | Refer to Figure 4-3
   Top Front Left of Center | TFLC | Refer to Figure 4-3
   Top Front Right of Center | TFRC | Refer to Figure 4-3
   Top Front Wide Left | TFWL | Refer to Figure 4-3
   Top Front Wide Right | TFWR | Refer to Figure 4-3
   Top Side Left | TSL | Refer to Figure 4-3
   Top Side Right | TSR | Refer to Figure 4-3
   Top Surround Array Left | TSAL | Refer to Figure 4-3
   Top Surround Array Right | TSAR | Refer to Figure 4-3
   Top Back Left | TBL | Refer to Figure 4-3
   Top Back Right | TBR | Refer to Figure 4-3
   Top Back Center | TBC | Refer to Figure 4-3
   Top Back Left Of Center | TBLC | Refer to Figure 4-3
   Top Back Right Of Center | TBRC | Refer to Figure 4-3
   Top Back Wide Left | TBWL | Refer to Figure 4-3
   Top Back Wide Right | TBWR | Refer to Figure 4-3
   Bottom Center | BC | Refer to Figure 4-3
   Bottom Front Left | BFL | Refer to Figure 4-3
   Bottom Front Right | BFR | Refer to Figure 4-3
   Bottom Front Center | BFC | Refer to Figure 4-3
   Bottom Front Left Of Center | BFLC | Refer to Figure 4-3
   Bottom Front Right Of Center | BFRC | Refer to Figure 4-3
   Bottom Front Wide Left | BFWL | Refer to Figure 4-3
   Bottom Front Wide Right | BFWR | Refer to Figure 4-3
   Bottom Side Left | BSL | Refer to Figure 4-3
   Bottom Side Right | BSR | Refer to Figure 4-3
   Bottom Surround Array Left | BSAL | Refer to Figure 4-3
   Bottom Surround Array Right | BSAR | Refer to Figure 4-3
   Bottom Back Left | BBL | Refer to Figure 4-3
   Bottom Back Right | BBR | Refer to Figure 4-3
   Bottom Back Center | BBC | Refer to Figure 4-3
   Bottom Back Left Of Center | BBLC | Refer to Figure 4-3
   Bottom Back Right Of Center | BBRC | Refer to Figure 4-3
   Bottom Back Wide Left | BBWL | Refer to Figure 4-3
   Bottom Back Wide Right | BBWR | Refer to Figure 4-3
   Low Frequency Effects | LFE | Refer to Figure 4-3
   Low Frequency Effects Left | LFEL | Refer to Figure 4-3
   Low Frequency Effects Right | LFER | Refer to Figure 4-3
   Headphone Left | HPL | Refer to Figure 4-3
   Headphone Right | HPR | Refer to Figure 4-3
   
9. wenqingfu

   assigned

   singalsu

   on Oct 24, 2019
10. wenqingfu commented on Oct 24, 2019

    

    wenqingfu

    on Oct 24, 2019

    More actions

    @singalsu where should the fix go? do you think you can own this?
11. singalsu commented on Jan 8, 2020

    

    singalsu

    on Jan 8, 2020

    Collaborator

    More actions

    @wenqingfu Yes I can own this. The fix would be in ALSA, I need to discuss here with @kv2019i how it could go.

    The definitions from USB audio class 3 look useful. Thanks @plbossart !
12. sinahuang commented on Jan 10, 2020

    

    sinahuang

    on Jan 10, 2020

    Last edited by sinahuang

    More actions

    Issue also can be reproduced on TGL-U RVP with soundwire-latest(commit:921ce33) when arecord with DMIC 4ch(0,3 and 0,4)
    Test recipe

    Kernel:https://github.com/thesofproject/linux/tree/integration/soundwire-latest commit: 921ce33
    FW: https://github.com/thesofproject/sof/commits/master commit: c6baa97
    Tplg: Same with TGL FW branch : sof-tgl-rt711-rt1308.tplg
    Platform:TGL-U RVP with codec ALC711 in SDW mode and RT1308 in I2S mode
13. singalsu commented on Feb 13, 2020

    

    singalsu

    on Feb 13, 2020

    Collaborator

    More actions

    P2 - I need to increase priority for this. The FW is OK, the fix will be entirely on driver side.
14. singalsu commented on Feb 18, 2020

    

    singalsu

    on Feb 18, 2020

    Last edited by singalsu

    Collaborator

    More actions

    This need to be moved to driver side. The driver sends only controls to channels 0 and 1.

        0      2          IPC          19840492.552083       208.750000    src/ipc/handler.c:1042	ipc: hdr 0x50010000 tx (108) > rx (92)
        0      2          IPC          19840496.875000         4.322917    src/ipc/handler.c:1055	ipc: comp 42 -> cmd 2
        0      2         COMP 7.42     19840499.427083         2.552083 udio/volume/volume.c:612 	volume_cmd()
        0      2         COMP 7.42     19840502.187500         2.760417 udio/volume/volume.c:526 	volume_ctrl_set_cmd(), SOF_CTRL_CMD_SWITCH, cdata->comp_id = 42
        0      2         COMP 7.42     19840504.791667         2.604167 udio/volume/volume.c:531 	volume_ctrl_set_cmd(), channel = 0, value = 1
        0      2         COMP 7.42     19840508.072917         3.281250 udio/volume/volume.c:531 	volume_ctrl_set_cmd(), channel = 1, value = 0
        0      2          IPC          19840594.010417        85.937500    src/ipc/handler.c:1042	ipc: hdr 0x50010000 tx (108) > rx (92)
        0      2          IPC          19840598.802083         4.791667    src/ipc/handler.c:1055	ipc: comp 42 -> cmd 0
        0      2         COMP 7.42     19840601.354167         2.552083 udio/volume/volume.c:612 	volume_cmd()
        0      2         COMP 7.42     19840604.010417         2.656250 udio/volume/volume.c:493 	volume_ctrl_set_cmd(), SOF_CTRL_CMD_VOLUME, cdata->comp_id = 42
        0      2         COMP 7.42     19840606.718750         2.708333 udio/volume/volume.c:498 	volume_ctrl_set_cmd(), channel = 0, value = 65536
        0      2         COMP 7.42     19840609.895833         3.177083 udio/volume/volume.c:498 	volume_ctrl_set_cmd(), channel = 1, value = 65536
    
    
15. singalsu

    transferred this issue fromthesofproject/sofon Feb 18, 2020
16. 7 remaining items

    Load more

17. plbossart commented on Mar 3, 2020

    

    plbossart

    on Mar 3, 2020

    Member

    More actions

    @singalsu it's not 'ugly' for a N>2 channel stream to have a single volume/mute, and then separate volumes if they are 'unlocked'. Even for stereo, it's not uncommon to have a single volume and keep volume changes locked.
18. singalsu commented on Mar 4, 2020

    

    singalsu

    on Mar 4, 2020

    Last edited by singalsu

    Collaborator

    More actions

    @plbossart In this case (with the FW based controls extrapolation approach) a 4ch volume would unlock into two controls only. There would be no way to control each gain separately. The use could not know which microphone remains pairs with same gain.

    Two expose control to each channel the topology should for alsamixer convert a N channel volume into N/2 volume instances those appear to be in cascade. But instead of cascade the first instance would control channels 1&2, second 3&4, ... etc.
19. plbossart commented on Mar 4, 2020

    

    plbossart

    on Mar 4, 2020

    Member

    More actions

    @singalsu if we have 4 channels, I don't understand why we would need 2 pairs of gains.
    usually there is one gain for all channels, or separate gains for each channel. pairing by 2 makes no sense to me.

    👍React with 👍1Reacted by Paul Olaru
20. juimonen commented on Mar 4, 2020

    

    juimonen

    on Mar 4, 2020

    More actions

    @plbossart I think @singalsu tried to explain it above... we don't have easy means of exposing single 4 channel volume control to user space. 2 channels are hard coded in asoc layer mixer creation, even though we would introduce 4-channel volume in topology. I don't know if anyone has seen > 2ch mixer volume control in amixer or alsamixer with asoc code related chip? (not an evil question, I'm just not sure if it is somehow possible).

    So we should change the asoc layer to have multi-channel volume, if above is correct. @singalsu was suggesting as quick remedy to couple 1 stereo volume to 4 ch volume in firmware. As the main problem is to set volume of the whole dmic array or mute it. So that 2 channel don't "leak" as in this bug... and then you could not set individual dmic channels.

    So should we then go for "mono" volume for the whole array?
21. Liviali155 commented on Apr 13, 2020

    

    Liviali155

    on Apr 13, 2020

    More actions

    This issue also can be reproduced on sof v1.5-rc1 on TGL HDA
    Kernel:4b4681eb Sof-dev
    fw:5f7d4e2 https://github.com/thesofproject/sof/releases/tag/v1.5-rc1
22. ranj063 commented on Jun 9, 2020

    

    ranj063

    on Jun 9, 2020

    Collaborator

    More actions

    @plbossart I think @singalsu tried to explain it above... we don't have easy means of exposing single 4 channel volume control to user space. 2 channels are hard coded in asoc layer mixer creation, even though we would introduce 4-channel volume in topology. I don't know if anyone has seen > 2ch mixer volume control in amixer or alsamixer with asoc code related chip? (not an evil question, I'm just not sure if it is somehow possible).

    @juimonen where is this limitation coming from? As far as I can tell the control IO functions dont restrict to just stereo and also the channel mapping in the ALSA topology document doesnt mention it. https://www.alsa-project.org/alsa-doc/alsa-lib/group__topology.html
23. singalsu commented on Jun 15, 2020

    

    singalsu

    on Jun 15, 2020

    Collaborator

    More actions

    If a quick and ugly fix is needed the missing channels 3 and 4 controls can be extrapolated from 1 and 2. It could be done on kernel or firmware side. I don't like such but this has been open soon a full year without found way to do this. Also the channel maps for ALSA are for surround playback. It would be artificial to name mic channels as FR, FL, SR, SL, etc.
24. mengdonglin

    unassigned

    singalsu

    on Sep 16, 2020
25. kv2019i

    added

    dmicIssues related to PCH connected DMICIssues related to PCH connected DMIC

    on Sep 22, 2020
26. singalsu commented on Oct 5, 2020

    

    singalsu

    on Oct 5, 2020

    Collaborator

    More actions

    Patch by @juimonen #2469 fixes this.
27. keyonjie commented on Oct 12, 2020

    

    keyonjie

    on Oct 12, 2020

    More actions

    pr is merged, can we close it?
28. juimonen commented on Oct 12, 2020

    

    juimonen

    on Oct 12, 2020

    More actions

    @keyonjie to make the use case working through the whole stack we need:
    thesofproject/sof#3470
    and
    https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/382

    but the kernel part is merged yes.
29. plbossart commented on Jan 22, 2021

    

    plbossart

    on Jan 22, 2021

    Member

    More actions

    @juimonen can we close this?
30. juimonen commented on Jan 22, 2021

    

    juimonen

    on Jan 22, 2021

    More actions

    Yes this should be fixed, we might still get new similar issues when folks have mixed versions of kernel/fw/topology, but they can be treated separately.
31. juimonen

    closed this as completedon Jan 22, 2021