Re: [PATCH v4 06/16] ASoC: amd: acp: add PDM controller selection for ACP7.B/7.F SoundWire machine driver

[Mukunda,Vijendar]

On 01/11/26 23:58, Pierre-Louis Bossart wrote:
> On 10/30/26 10:45, Vijendar Mukunda wrote:
>> Add ASOC_SDW_ACP_DMIC1 quirk bit (BIT(6)) and pdm_sel field to
>> amd_mc_ctx to support per-controller DMIC DAI selection on ACP7.B/7.F
>> platforms.
>>
>> In mc_probe(), read the active PDM controller from mach->pdata
>> (populated via ACPI acp-audio-ep-port _DSD). When ACPI does not set
>> pdm_sel, derive it from the quirk bits after dmi_check_system() and
>> quirk_override are applied: ASOC_SDW_ACP_DMIC1 selects DMIC1;
>> ASOC_SDW_ACP_DMIC selects DMIC0. Since only one PDM controller is
>> active at a time, DMIC1 is checked first.
> nit-pick: it'd be useful to explain the mapping between PDM controllers
> and PDM microphones. Is this one controller:2mics?

>
> Also does anything prevent DMIC and DMIC1 from being active simultaneously?
Yes, each PDM controller supports two microphones, i.e. a 2-channel
DMIC configuration. PDM0 and PDM1 are alternative controllers, with
the OEM/platform configuration determining which controller is used.
Only one PDM controller is selected for a given platform, so DMIC and
DMIC1 are not expected to be active simultaneously.

Will add a comment in the code to clarify this and post a supplemental
patch.
>
>> In create_dmic_dailinks(), select the SOF CPU DAI name based on
>> pdm_sel: "acp-sof-dmic1" for ACP7X_PDM_DMIC1, "acp-sof-dmic0"
>> otherwise on ACP7.B/7.F; all other platforms use "acp-sof-dmic".
>>
>> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
>> ---
>>  sound/soc/amd/acp/acp-sdw-sof-mach.c   | 44 +++++++++++++++++++++++++-
>>  sound/soc/amd/acp/soc_amd_sdw_common.h |  3 ++
>>  2 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/amd/acp/acp-sdw-sof-mach.c b/sound/soc/amd/acp/acp-sdw-sof-mach.c
>> index 97b2e224492f..34a71f7a8c54 100644
>> --- a/sound/soc/amd/acp/acp-sdw-sof-mach.c
>> +++ b/sound/soc/amd/acp/acp-sdw-sof-mach.c
>> @@ -265,11 +265,31 @@ static int create_dmic_dailinks(struct snd_soc_card *card,
>>  				struct snd_soc_dai_link **dai_links, int *be_id, int no_pcm)
>>  {
>>  	struct device *dev = card->dev;
>> +	struct asoc_sdw_mc_private *ctx = snd_soc_card_get_drvdata(card);
>> +	struct amd_mc_ctx *amd_ctx = (struct amd_mc_ctx *)ctx->private;
>> +	const char *cpu_dai_name;
>>  	int ret;
>>  
>> +	/*
>> +	 * For ACP7.B/7.F platforms the PDM controller is selected from ACPI
>> +	 * acp-audio-ep-port and stored in amd_ctx->pdm_sel. Use the
>> +	 * matching SOF DAI name ("acp-sof-dmic0" or "acp-sof-dmic1").
>> +	 * All other platforms use the generic "acp-sof-dmic" DAI.
>> +	 */
>> +	switch (amd_ctx->acp_rev) {
>> +	case ACP7B_PCI_REV:
>> +	case ACP7F_PCI_REV:
>> +		cpu_dai_name = (amd_ctx->pdm_sel == ACP7X_PDM_DMIC1) ?
>> +				"acp-sof-dmic1" : "acp-sof-dmic0";
>> +		break;
>> +	default:
>> +		cpu_dai_name = "acp-sof-dmic";
>> +		break;
>> +	}
>> +
>>  	ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, "acp-dmic-codec",
>>  					    0, 1, // DMIC only supports capture
>> -					    "acp-sof-dmic", platform_component->name,
>> +					    cpu_dai_name, platform_component->name,
>>  					    "dmic-codec", "dmic-hifi", no_pcm,
>>  					    asoc_sdw_dmic_init, NULL);
>>  	if (ret)
>> @@ -378,6 +398,7 @@ static int sof_card_dai_links_create(struct snd_soc_card *card)
>>  static int mc_probe(struct platform_device *pdev)
>>  {
>>  	struct snd_soc_acpi_mach *mach = dev_get_platdata(&pdev->dev);
>> +	struct amd_pdm_pdata *pdm_pdata;
>>  	struct snd_soc_card *card;
>>  	struct amd_mc_ctx *amd_ctx;
>>  	struct asoc_sdw_mc_private *ctx;
>> @@ -398,6 +419,10 @@ static int mc_probe(struct platform_device *pdev)
>>  		amd_ctx->max_sdw_links = ACP63_SDW_MAX_LINKS;
>>  		break;
>>  	}
>> +	if (mach->pdata) {
>> +		pdm_pdata = mach->pdata;
>> +		amd_ctx->pdm_sel = pdm_pdata->pdm_sel;
>> +	}
>>  	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
>>  	if (!ctx)
>>  		return -ENOMEM;
>> @@ -419,6 +444,23 @@ static int mc_probe(struct platform_device *pdev)
>>  		sof_sdw_quirk = quirk_override;
>>  	}
>>  
>> +	/*
>> +	 * If ACPI did not set pdm_sel (no acp-audio-ep-port in _DSD), derive
>> +	 * it from the quirk bits. ACPI takes priority: quirks are only
>> +	 * consulted when pdm_sel is still zero. PDM controller selection is
>> +	 * only meaningful on ACP7B/7F platforms.
>> +	 * ASOC_SDW_ACP_DMIC selects DMIC0 (default); ASOC_SDW_ACP_DMIC1
>> +	 * overrides to DMIC1.
>> +	 */
>> +	if (!amd_ctx->pdm_sel &&
>> +	    (amd_ctx->acp_rev == ACP7B_PCI_REV ||
>> +	     amd_ctx->acp_rev == ACP7F_PCI_REV)) {
>> +		if (sof_sdw_quirk & ASOC_SDW_ACP_DMIC1)
>> +			amd_ctx->pdm_sel = ACP7X_PDM_DMIC1;
>> +		else if (sof_sdw_quirk & ASOC_SDW_ACP_DMIC)
>> +			amd_ctx->pdm_sel = ACP7X_PDM_DMIC0;
>> +	}
>> +
>>  	log_quirks(&pdev->dev);
>>  
>>  	ctx->mc_quirk = sof_sdw_quirk;
>> diff --git a/sound/soc/amd/acp/soc_amd_sdw_common.h b/sound/soc/amd/acp/soc_amd_sdw_common.h
>> index c979adf4e978..07837a2cfa54 100644
>> --- a/sound/soc/amd/acp/soc_amd_sdw_common.h
>> +++ b/sound/soc/amd/acp/soc_amd_sdw_common.h
>> @@ -29,6 +29,7 @@
>>  #define SOC_JACK_JDSRC(quirk)		((quirk) & GENMASK(3, 0))
>>  #define ASOC_SDW_FOUR_SPK		BIT(4)
>>  #define ASOC_SDW_ACP_DMIC		BIT(5)
>> +#define ASOC_SDW_ACP_DMIC1		BIT(6)	/* select DMIC1 controller for ACP7.B/7.F */
>>  #define ASOC_SDW_CODEC_SPKR		BIT(15)
>>  
>>  #define AMD_SDW0	0
>> @@ -94,6 +95,8 @@
>>  struct amd_mc_ctx {
>>  	unsigned int acp_rev;
>>  	unsigned int max_sdw_links;
>> +	/* PDM controller selection, copied from amd_pdm_pdata at probe time */
>> +	unsigned int pdm_sel;
>>  };
>>  
>>  int get_acp63_cpu_pin_id(u32 sdw_link_id, int be_id, int *cpu_pin_id, struct device *dev);