Re: [PATCH V2 03/13] ASoC: amd: acp: fix PDM1 quirk and PDM controller selection for ACP7.B/7.F

[Mukunda,Vijendar]

On 22/10/26 11:32, Vijendar Mukunda wrote:
> The ASOC_SDW_ACP_DMIC1 quirk was evaluated before dmi_check_system()
> and quirk_override were applied, so the pdm_sel fallback always saw
> the compile-time default and was effectively dead code. Move it to
> after the quirk is fully resolved and restrict it to ACP7B/7F revisions
> where PDM controller selection is meaningful.
>
> Since ACP7.B/7.F exposes exactly one active PDM controller, use
> else if for the DMIC0 fallback: check ASOC_SDW_ACP_DMIC1 first; only
> fall back to ASOC_SDW_ACP_DMIC (DMIC0) when DMIC1 is not set.
Got a minor review comment by sashiko review tool. 
https://sashiko.dev/#/patchset/20261022060724.1814741-1-Vijendar.Mukunda@xxxxxxx?part=3
Will rephrase the commit message and respin the patch series.

>
> 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 6855141293e34..2fb5c7ecafd8b 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 9e4c86bed3003..86c8e38a19bb1 100644
> --- a/sound/soc/amd/acp/soc_amd_sdw_common.h
> +++ b/sound/soc/amd/acp/soc_amd_sdw_common.h
> @@ -30,6 +30,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
> @@ -83,6 +84,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);