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.