topology2: set DMIC0 pcm id to 6 by libinyang · Pull Request #6904 · thesofproject/sof

libinyang · 2023-01-03T10:15:58Z

UCM requires DMIC PCM ID to be 6.
Fixes: #4068
Signed-off-by: Libin Yang libin.yang@intel.com

plbossart · 2023-01-03T16:40:21Z

No, this needs to be a parameter, we use the same file from multiple places

cavs-nocodec.conf: "[1-4]" "platform/intel/dmic-generic.conf" cavs-rt5682.conf: "[1-4]" "platform/intel/dmic-generic.conf" cavs-sdw.conf:"[1-4]" "platform/intel/dmic-generic.conf" cavs-sdw.conf:"1" "platform/intel/sdw-dmic-generic.conf" sof-hda-generic.conf: "[1-4]" "platform/intel/dmic-generic.conf"

@plbossart Thanks for the suggestion. I will refine the patch by using a parameter based on the UCM for each mode.

libinyang · 2023-01-05T13:07:14Z

v2 update:
use DMIC0_PCM_ID for dmic0 pcm id setting.

libinyang · 2023-01-05T13:09:31Z

This PR depends on #6787. Without #6787, sof hda will fail to create the snd card because of the conflict between HDMI PCM ID and DMIC0 PCM ID, which are both 6.

lgirdwood · 2023-01-06T15:22:19Z

This PR depends on #6787. Without #6787, sof hda will fail to create the snd card because of the conflict between HDMI PCM ID and DMIC0 PCM ID, which are both 6.

#6787 now pending final approval from @ranj063

mengdonglin · 2023-01-13T06:03:17Z

@plbossart @ranj063 Can you please check if this PR can be merged? It depends on #6787 that has been merged.

libinyang · 2023-01-17T06:08:42Z

#6787 is merged, but there are some more discussions in #6953. Let's wait for the final conclusion.

lgirdwood · 2023-01-18T13:30:06Z

#6787 is merged, but there are some more discussions in #6953. Let's wait for the final conclusion.

#6953 closed. @libinyang @bardliao @kv2019i whats the next move now ?

kv2019i

Looks good. There's a conflict now though, so needs an update.

libinyang · 2023-01-30T09:43:21Z

Patches update

lgirdwood

@plbossart changes good now ?

plbossart

sounds incomplete to me. I am pretty sure the device 10 is not right for cavs-sdw topologies

ranj063 · 2023-01-30T17:20:24Z

sounds incomplete to me. I am pretty sure the device 10 is not right for cavs-sdw topologies

@plbossart this may be incomplete but it doesnt make the existing topologies any worse right? We're currently setting 10 for all PCM IDs for all topologies. At least the HDA ones are fixed with this one

plbossart · 2023-01-30T17:27:20Z

sounds incomplete to me. I am pretty sure the device 10 is not right for cavs-sdw topologies

@plbossart this may be incomplete but it doesnt make the existing topologies any worse right? We're currently setting 10 for all PCM IDs for all topologies. At least the HDA ones are fixed with this one

what the point of doing things one step at a time, apart from wasting reviewer time?
This is a shared module, so we do things right. I don't want to review this multiple times.

Edit: I provided the SAME feedback last month in #6904 (comment). Seriously?

libinyang · 2023-01-31T08:23:53Z

what the point of doing things one step at a time, apart from wasting reviewer time? This is a shared module, so we do things right. I don't want to review this multiple times.

Edit: I provided the SAME feedback last month in #6904 (comment). Seriously?

@plbossart We have a rule for the DMIC0 PCM ID described in #6783. The DMIC0 PCM ID should be 10.
For the below topologies, I checked: what PCMID is set in topology1, what UCM requires the DMIC0 PCMID to be.

cavs-nocodec-bt.conf:
There seems no counterpart in topology1 and no special requirement from UCM.
cavs-nocodec.conf
The PCMID is 10 in topology1 and no special requirement from UCM.
cavs-rt5682.conf
The PCMID is 99 in topology1 (We should not use 99 anymore, I think) and no special requirement from UCM.
cavs-sdw.conf
The PCMID is 10 and UCM2 can accept any PCMID for sof-soundwire card (no special requirement from UCM).
sof-hda-generic.conf.
The PCMID is 6 and UCM2 requires PCMID to be 6.

So I think this PR can cover all the scenarios in topology2: set DMIC0 PCMID to 6 only on HDA platform and let other platforms to be 10.

plbossart · 2023-01-31T17:17:50Z

cavs-rt5682.conf
The PCMID is 99 in topology1 (We should not use 99 anymore, I think) and no special requirement from UCM.

have you asked the Google/Chrome folks? They use UCM, don't they? @cujomalainey @sathya-nujella
It's not our decision to change the device numbers, and there is nothing in the card name that identifies MTL/topology2/

cavs-sdw.conf
The PCMID is 10 and UCM2 can accept any PCMID for sof-soundwire card (no special requirement from UCM).

the requirement is this:
CapturePCM "hw:${CardId},${find-device:type=pcm,stream=capture,field=id,regex='DMIC '}"

is this met?

sathya-nujella · 2023-01-31T17:53:42Z

cavs-rt5682.conf
The PCMID is 99 in topology1 (We should not use 99 anymore, I think) and no special requirement from UCM.

have you asked the Google/Chrome folks? They use UCM, don't they? @cujomalainey @sathya-nujella It's not our decision to change the device numbers, and there is nothing in the card name that identifies MTL/topology2/

Topology1 based Chrome systems are using 99 for DMIC capture. Can you please limit the change to topology2 only if possible?
If it is affecting topology2 only, then from MTL onwards we can update UCM's in Chromium gits accordingly.

plbossart · 2023-01-31T18:14:32Z

cavs-rt5682.conf
The PCMID is 99 in topology1 (We should not use 99 anymore, I think) and no special requirement from UCM.

have you asked the Google/Chrome folks? They use UCM, don't they? @cujomalainey @sathya-nujella It's not our decision to change the device numbers, and there is nothing in the card name that identifies MTL/topology2/

Topology1 based Chrome systems are using 99 for DMIC capture. Can you please limit the change to topology2 only if possible? If it is affecting topology2 only, then from MTL onwards we can update UCM's in Chromium gits accordingly.

if we change only for topology2, you won't be able to test this topology on existing TGL chromebooks. I see no value in changing the device number and make integration/testing more complicated. Yes 99 is not a very good number, but it's not worse than 10. It's easier to stick with legacy.

libinyang · 2023-02-01T06:23:28Z

Topology1 based Chrome systems are using 99 for DMIC capture. Can you please limit the change to topology2 only if possible? If it is affecting topology2 only, then from MTL onwards we can update UCM's in Chromium gits accordingly.

if we change only for topology2, you won't be able to test this topology on existing TGL chromebooks. I see no value in changing the device number and make integration/testing more complicated. Yes 99 is not a very good number, but it's not worse than 10. It's easier to stick with legacy.

@plbossart @sathya-nujella Thanks for the feedback. I will follow @plbossart 's suggestion and make the DMIC0 PCMID to be 99 for sof-mtl-max98357a-rt5682 platforms on topology2, and topology1 will not be impacted. BTW: Our current rt5682 in tplg2 is used for sof-mtl-max98357a-rt5682 platforms. However, rt5682 can also be used in other combinations, for example, in tplg1, it can be used in sof-cml-rt1011-rt5682 and sof-cml-rt5682-max98357a, and the dmic0 PCMID is 1 on such platforms. To make it more flexible, I will set the DMIC0_PCM_ID=99 in the CMakefile.

the requirement is this:
CapturePCM "hw:${CardId},${find-device:type=pcm,stream=capture,field=id,regex='DMIC '}"

is this met?

Currently, it meets the requirement on our supported tplg2 platforms. But this may be conflicted with DMIC1, for example WoV PCM name. I will have a check and test. If there is conflict, I will fix it in another PR as this is a PCM name issue, not PCM ID.

libinyang · 2023-02-01T07:01:17Z

patch updated:

set sof-mtl-max98357a-rt5682 DMIC0 pcm id to 99

mengdonglin · 2023-02-02T05:57:50Z

@plbossart @kv2019i @lgirdwood Are we good to merge this PR?

ranj063 · 2023-02-02T14:59:01Z

@plbossart @kv2019i @lgirdwood Are we good to merge this PR?

@libinyang @mengdonglin there are conflicts

Different platforms are using the same dmic-generic.conf. But the DMIC0 PCM ID are different for different platforms. So let's use parameter DMIC0_PCM_ID for the DMIC0 pcm id setting. Signed-off-by: Libin Yang <libin.yang@intel.com>

UCM requires sof-hda-generic DMIC0 PCM ID to be 6. Signed-off-by: Libin Yang <libin.yang@intel.com>

Set sof-mtl-max98357a-rt5682 pcm id to 99 to make it the same in topology1. Signed-off-by: Libin Yang <libin.yang@intel.com>

libinyang · 2023-02-03T08:15:25Z

Patches are updated based on the latest code.

libinyang requested review from dbaluta, lbetlej, lgirdwood, mmaka1 and plbossart as code owners January 3, 2023 10:15

libinyang mentioned this pull request Jan 3, 2023

pulse audio not detecting sound card (MTL,HDA) thesofproject/linux#4068

Closed

plbossart requested changes Jan 3, 2023

View reviewed changes

libinyang force-pushed the DMIC_PCMID branch from 5fc0a86 to 3067ef4 Compare January 5, 2023 13:06

bardliao approved these changes Jan 19, 2023

View reviewed changes

ranj063 approved these changes Jan 19, 2023

View reviewed changes

kv2019i approved these changes Jan 19, 2023

View reviewed changes

libinyang force-pushed the DMIC_PCMID branch from 3067ef4 to ec1369d Compare January 30, 2023 08:51

lgirdwood approved these changes Jan 30, 2023

View reviewed changes

plbossart requested changes Jan 30, 2023

View reviewed changes

Comment thread tools/topology/topology2/platform/intel/dmic-default.conf Outdated

libinyang requested review from aborisovich and marc-hb as code owners February 1, 2023 06:59

libinyang requested a review from plbossart February 1, 2023 07:01

plbossart approved these changes Feb 1, 2023

View reviewed changes

libinyang added 3 commits February 3, 2023 16:12

topology2: set sof-hda-generic DMIC0 pcm id to 6

7610c93

UCM requires sof-hda-generic DMIC0 PCM ID to be 6. Signed-off-by: Libin Yang <libin.yang@intel.com>

topology2: set sof-mtl-max98357a-rt5682 DMIC0 pcm id to 99

d10231a

Set sof-mtl-max98357a-rt5682 pcm id to 99 to make it the same in topology1. Signed-off-by: Libin Yang <libin.yang@intel.com>

libinyang force-pushed the DMIC_PCMID branch from b1ecf3b to d10231a Compare February 3, 2023 08:14

kv2019i merged commit 9a6b50c into thesofproject:main Feb 3, 2023

Conversation

libinyang commented Jan 3, 2023 • edited by keqiaozhang Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plbossart Jan 3, 2023

Choose a reason for hiding this comment

Uh oh!

libinyang Jan 4, 2023

Choose a reason for hiding this comment

Uh oh!

libinyang commented Jan 5, 2023

Uh oh!

libinyang commented Jan 5, 2023

Uh oh!

lgirdwood commented Jan 6, 2023

Uh oh!

mengdonglin commented Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

libinyang commented Jan 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood commented Jan 18, 2023

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

libinyang commented Jan 30, 2023

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ranj063 commented Jan 30, 2023

Uh oh!

plbossart commented Jan 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

libinyang commented Jan 31, 2023

Uh oh!

plbossart commented Jan 31, 2023

Uh oh!

sathya-nujella commented Jan 31, 2023

Uh oh!

plbossart commented Jan 31, 2023

Uh oh!

libinyang commented Feb 1, 2023

Uh oh!

libinyang commented Feb 1, 2023

Uh oh!

mengdonglin commented Feb 2, 2023

Uh oh!

ranj063 commented Feb 2, 2023

Uh oh!

libinyang commented Feb 3, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

libinyang commented Jan 3, 2023 •

edited by keqiaozhang

Loading

mengdonglin commented Jan 13, 2023 •

edited

Loading

libinyang commented Jan 17, 2023 •

edited

Loading

plbossart commented Jan 30, 2023 •

edited

Loading