On 7/28/2025 4:22 PM, Greg KH wrote:

On Mon, Jul 28, 2025 at 03:12:27PM +0530, Venkata Prasad Potturu wrote:

...

Adjust pdm dimc gain value using module param. In case of regressions for any users that the new pdm_gain value is too high and for additional debugging, introduce a module parameter that would let them configure it.

This parameter should be removed in the future:

• If it's determined that the parameter is not needed, just hardcode the correct value as before
• If users do end up using it to debug and report different values we should introduce a config knob that can have policy set by ucm.

Signed-off-by: Venkata Prasad Potturu venkataprasad.potturu@amd.com

sound/soc/amd/acp/acp-legacy-common.c | 3 ++- sound/soc/amd/acp/acp-pdm.c | 3 ++- sound/soc/amd/acp/amd.h | 6 +++++- 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/sound/soc/amd/acp/acp-legacy-common.c b/sound/soc/amd/acp/acp-legacy-common.c index 57982d057c3a..dd804fb95790 100644 --- a/sound/soc/amd/acp/acp-legacy-common.c +++ b/sound/soc/amd/acp/acp-legacy-common.c @@ -173,7 +173,8 @@ static void set_acp_pdm_clk(struct snd_pcm_substream *substream, /* Enable default ACP PDM clk */ writel(PDM_CLK_FREQ_MASK, chip->base + ACP_WOV_CLK_CTRL); pdm_ctrl = readl(chip->base + ACP_WOV_MISC_CTRL);

• pdm_ctrl |= PDM_MISC_CTRL_MASK;

• pdm_ctrl &= ~ACP_WOV_GAIN_CONTROL;
• pdm_ctrl |= FIELD_PREP(ACP_WOV_GAIN_CONTROL, clamp(pdm_gain, 0, 3)); writel(pdm_ctrl, chip->base + ACP_WOV_MISC_CTRL); set_acp_pdm_ring_buffer(substream, dai); }

diff --git a/sound/soc/amd/acp/acp-pdm.c b/sound/soc/amd/acp/acp-pdm.c index 1bfc34c2aa53..ffb622a7a69a 100644 --- a/sound/soc/amd/acp/acp-pdm.c +++ b/sound/soc/amd/acp/acp-pdm.c @@ -38,7 +38,8 @@ static int acp_dmic_prepare(struct snd_pcm_substream *substream, /* Enable default DMIC clk */ writel(PDM_CLK_FREQ_MASK, chip->base + ACP_WOV_CLK_CTRL); dmic_ctrl = readl(chip->base + ACP_WOV_MISC_CTRL);

• dmic_ctrl |= PDM_MISC_CTRL_MASK;

• dmic_ctrl &= ~ACP_WOV_GAIN_CONTROL;

• dmic_ctrl |= FIELD_PREP(ACP_WOV_GAIN_CONTROL, clamp(pdm_gain, 0, 3)); writel(dmic_ctrl, chip->base + ACP_WOV_MISC_CTRL);

  period_bytes = frames_to_bytes(substream->runtime,

diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h index cb8d97122f95..f2567e06ccd3 100644 --- a/sound/soc/amd/acp/amd.h +++ b/sound/soc/amd/acp/amd.h @@ -130,7 +130,7 @@ #define PDM_DMA_INTR_MASK 0x10000 #define PDM_DEC_64 0x2 #define PDM_CLK_FREQ_MASK 0x07 -#define PDM_MISC_CTRL_MASK 0x10 +#define ACP_WOV_GAIN_CONTROL GENMASK(4, 3) #define PDM_ENABLE 0x01 #define PDM_DISABLE 0x00 #define DMA_EN_MASK 0x02 @@ -138,6 +138,10 @@ #define PDM_TIMEOUT 1000 #define ACP_REGION2_OFFSET 0x02000000

+static int pdm_gain = 3; +module_param(pdm_gain, int, 0644); +MODULE_PARM_DESC(pdm_gain, "Gain control (0-3)");

This is not the 1990's, please do not add new module parameters, it will not work for modern systems and is a horrible thing to attempt to support over time :(

Please do this properly, with a per-device setting.

thanks,

greg k-h

As the main purpose for this parameter is for being able to tune whether the property is correct, how about adding a debugfs file instead?

AFAICT it should just be a single register write, so debugfs to read current value and write the debugging value seems pretty straightforward.