[PATCH 0/6] Fix default DMIC gain on AMD PDM drivers

[PATCH 0/6] Fix default DMIC gain on AMD PDM drivers

[Mario Limonciello]

It's been reported that a number of laptops have a low volume
level from the digital microphone compared to Windows.

AMD offers a register that can adjust the gain for PDM which is not
configured at maximum gain by default.

To fix this change the default for all 3 drivers to raise the gain
but also offer a module parameter. The module parameter can be used
for debugging if the gain is too high on a given laptop.

This is intentionally split into multiple patches for default and
parameter so that if the default really does behave better universally
we can bring it back to stable too later.

Mario Limonciello (6):
  ASoC: amd: yc: Adjust the gain for PDM DMIC
  ASoC: amd: yc: Add a module parameter to influence pdm_gain
  ASoC: amd: renoir: Adjust the gain for PDM DMIC
  ASoC: amd: renoir: Add a module parameter to influence pdm_gain
  ASoC: amd: ps: Adjust the gain for PDM DMIC
  ASoC: amd: ps: Add a module parameter to influence pdm_gain

 sound/soc/amd/ps/acp63.h             | 2 +-
 sound/soc/amd/ps/ps-pdm-dma.c        | 7 ++++++-
 sound/soc/amd/renoir/acp3x-pdm-dma.c | 7 ++++++-
 sound/soc/amd/renoir/rn_acp3x.h      | 2 +-
 sound/soc/amd/yc/acp6x-pdm-dma.c     | 7 ++++++-
 sound/soc/amd/yc/acp6x.h             | 2 +-
 6 files changed, 21 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH 1/6] ASoC: amd: yc: Adjust the gain for PDM DMIC

[Mario Limonciello]

A number of users for Lenovo Rembrandt based laptops are
reporting that the microphone is too quiet relative to
Windows with a dual boot.

Increase the PDM gain to overcome this problem.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 sound/soc/amd/yc/acp6x.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/amd/yc/acp6x.h b/sound/soc/amd/yc/acp6x.h
index 74b596e6807ad..846ca10e24d3f 100644
--- a/sound/soc/amd/yc/acp6x.h
+++ b/sound/soc/amd/yc/acp6x.h
@@ -31,7 +31,7 @@
 #define ACP_ERROR_STAT	29
 #define PDM_DECIMATION_FACTOR	2
 #define ACP_PDM_CLK_FREQ_MASK	7
-#define ACP_WOV_MISC_CTRL_MASK	0x10
+#define ACP_WOV_MISC_CTRL_MASK	0x18
 #define ACP_PDM_ENABLE		1
 #define ACP_PDM_DISABLE		0
 #define ACP_PDM_DMA_EN_STATUS	2
-- 
2.34.1

Re: [PATCH 1/6] ASoC: amd: yc: Adjust the gain for PDM DMIC

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

On Fri, Jan 27, 2023 at 10:01:28AM -0600, Mario Limonciello wrote:
> A number of users for Lenovo Rembrandt based laptops are
> reporting that the microphone is too quiet relative to
> Windows with a dual boot.
> 
> Increase the PDM gain to overcome this problem.

Why not just make this a regular control that can be adjusted as
users see fit?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH 1/6] ASoC: amd: yc: Adjust the gain for PDM DMIC

[Limonciello, Mario]

[AMD Official Use Only - General]



> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Friday, January 27, 2023 10:53
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Jaroslav Kysela <jkysela@redhat.com>; Mukunda, Vijendar
> <Vijendar.Mukunda@amd.com>; Saba Kareem, Syed
> <Syed.SabaKareem@amd.com>; linux-kernel@vger.kernel.org;
> Pananchikkal, Renjith <Renjith.Pananchikkal@amd.com>; Mark Pearson
> <mpearson@lenovo.com>; Liam Girdwood <lgirdwood@gmail.com>;
> Jaroslav Kysela <perex@perex.cz>; Takashi Iwai <tiwai@suse.com>; alsa-
> devel@alsa-project.org
> Subject: Re: [PATCH 1/6] ASoC: amd: yc: Adjust the gain for PDM DMIC
> 
> On Fri, Jan 27, 2023 at 10:01:28AM -0600, Mario Limonciello wrote:
> > A number of users for Lenovo Rembrandt based laptops are
> > reporting that the microphone is too quiet relative to
> > Windows with a dual boot.
> >
> > Increase the PDM gain to overcome this problem.
> 
> Why not just make this a regular control that can be adjusted as
> users see fit?

Maximum HW gain is what we probably should have been setting from the
beginning.  The module parameter stuff was in case we need to debug it.

My thought is if we find some reported problems with the series we ask
users to try the module parameters.  If we're finding that certain parameters
make sense for different systems then we tear out the module parameter
and instead export a control and set policy from ucm.

If nothing is reported, we can probably drop the parameters at some time
in the future.

[PATCH 2/6] ASoC: amd: yc: Add a module parameter to influence pdm_gain

[Mario Limonciello]

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.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 sound/soc/amd/yc/acp6x-pdm-dma.c | 7 ++++++-
 sound/soc/amd/yc/acp6x.h         | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/yc/acp6x-pdm-dma.c b/sound/soc/amd/yc/acp6x-pdm-dma.c
index acecd6a4ec4b1..3d6a765c5011c 100644
--- a/sound/soc/amd/yc/acp6x-pdm-dma.c
+++ b/sound/soc/amd/yc/acp6x-pdm-dma.c
@@ -7,6 +7,7 @@
 
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/bitfield.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <sound/pcm_params.h>
@@ -18,6 +19,10 @@
 
 #define DRV_NAME "acp_yc_pdm_dma"
 
+static int pdm_gain = 3;
+module_param(pdm_gain, int, 0644);
+MODULE_PARM_DESC(pdm_gain, "Gain control (0-3)");
+
 static const struct snd_pcm_hardware acp6x_pdm_hardware_capture = {
 	.info = SNDRV_PCM_INFO_INTERLEAVED |
 		SNDRV_PCM_INFO_BLOCK_TRANSFER |
@@ -55,7 +60,7 @@ static void acp6x_enable_pdm_clock(void __iomem *acp_base)
 
 	acp6x_writel(pdm_clk_enable, acp_base + ACP_WOV_CLK_CTRL);
 	pdm_ctrl = acp6x_readl(acp_base + ACP_WOV_MISC_CTRL);
-	pdm_ctrl |= ACP_WOV_MISC_CTRL_MASK;
+	pdm_ctrl |= FIELD_PREP(ACP_WOV_GAIN_CONTROL, pdm_gain);
 	acp6x_writel(pdm_ctrl, acp_base + ACP_WOV_MISC_CTRL);
 }
 
diff --git a/sound/soc/amd/yc/acp6x.h b/sound/soc/amd/yc/acp6x.h
index 846ca10e24d3f..036207568c048 100644
--- a/sound/soc/amd/yc/acp6x.h
+++ b/sound/soc/amd/yc/acp6x.h
@@ -31,7 +31,7 @@
 #define ACP_ERROR_STAT	29
 #define PDM_DECIMATION_FACTOR	2
 #define ACP_PDM_CLK_FREQ_MASK	7
-#define ACP_WOV_MISC_CTRL_MASK	0x18
+#define ACP_WOV_GAIN_CONTROL	GENMASK(4, 3)
 #define ACP_PDM_ENABLE		1
 #define ACP_PDM_DISABLE		0
 #define ACP_PDM_DMA_EN_STATUS	2
-- 
2.34.1

Re: [PATCH 2/6] ASoC: amd: yc: Add a module parameter to influence pdm_gain

[Jaroslav Kysela]

On 27. 01. 23 17:01, Mario Limonciello wrote:
> 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.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   sound/soc/amd/yc/acp6x-pdm-dma.c | 7 ++++++-
>   sound/soc/amd/yc/acp6x.h         | 2 +-
>   2 files changed, 7 insertions(+), 2 deletions(-)

...

>   	pdm_ctrl = acp6x_readl(acp_base + ACP_WOV_MISC_CTRL);
> -	pdm_ctrl |= ACP_WOV_MISC_CTRL_MASK;
> +	pdm_ctrl |= FIELD_PREP(ACP_WOV_GAIN_CONTROL, pdm_gain);

The bits should be zeroed (AND - &) before OR to make sure that the correct 
value is written to the register. More related patches are affected.

>   	acp6x_writel(pdm_ctrl, acp_base + ACP_WOV_MISC_CTRL);

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

RE: [PATCH 2/6] ASoC: amd: yc: Add a module parameter to influence pdm_gain

[Limonciello, Mario]

[AMD Official Use Only - General]



> -----Original Message-----
> From: Jaroslav Kysela <perex@perex.cz>
> Sent: Friday, January 27, 2023 10:23
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; Mukunda, Vijendar
> <Vijendar.Mukunda@amd.com>; Saba Kareem, Syed
> <Syed.SabaKareem@amd.com>; linux-kernel@vger.kernel.org
> Cc: Pananchikkal, Renjith <Renjith.Pananchikkal@amd.com>; Mark Pearson
> <mpearson@lenovo.com>; Liam Girdwood <lgirdwood@gmail.com>; Mark
> Brown <broonie@kernel.org>; Takashi Iwai <tiwai@suse.com>; alsa-
> devel@alsa-project.org
> Subject: Re: [PATCH 2/6] ASoC: amd: yc: Add a module parameter to
> influence pdm_gain
> 
> On 27. 01. 23 17:01, Mario Limonciello wrote:
> > 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.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> >   sound/soc/amd/yc/acp6x-pdm-dma.c | 7 ++++++-
> >   sound/soc/amd/yc/acp6x.h         | 2 +-
> >   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> ...
> 
> >   	pdm_ctrl = acp6x_readl(acp_base + ACP_WOV_MISC_CTRL);
> > -	pdm_ctrl |= ACP_WOV_MISC_CTRL_MASK;
> > +	pdm_ctrl |= FIELD_PREP(ACP_WOV_GAIN_CONTROL, pdm_gain);
> 
> The bits should be zeroed (AND - &) before OR to make sure that the correct
> value is written to the register. More related patches are affected.

I had consider this, but the hardware default at reset is 0x0.  Do you think it's
still necessary for posterity?

Re: [PATCH 2/6] ASoC: amd: yc: Add a module parameter to influence pdm_gain

[Jaroslav Kysela]

On 27. 01. 23 17:25, Limonciello, Mario wrote:
> [AMD Official Use Only - General]
> 
> 
> 
>> -----Original Message-----
>> From: Jaroslav Kysela <perex@perex.cz>
>> Sent: Friday, January 27, 2023 10:23
>> To: Limonciello, Mario <Mario.Limonciello@amd.com>; Mukunda, Vijendar
>> <Vijendar.Mukunda@amd.com>; Saba Kareem, Syed
>> <Syed.SabaKareem@amd.com>; linux-kernel@vger.kernel.org
>> Cc: Pananchikkal, Renjith <Renjith.Pananchikkal@amd.com>; Mark Pearson
>> <mpearson@lenovo.com>; Liam Girdwood <lgirdwood@gmail.com>; Mark
>> Brown <broonie@kernel.org>; Takashi Iwai <tiwai@suse.com>; alsa-
>> devel@alsa-project.org
>> Subject: Re: [PATCH 2/6] ASoC: amd: yc: Add a module parameter to
>> influence pdm_gain
>>
>> On 27. 01. 23 17:01, Mario Limonciello wrote:
>>> 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.
>>>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>>    sound/soc/amd/yc/acp6x-pdm-dma.c | 7 ++++++-
>>>    sound/soc/amd/yc/acp6x.h         | 2 +-
>>>    2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> ...
>>
>>>    	pdm_ctrl = acp6x_readl(acp_base + ACP_WOV_MISC_CTRL);
>>> -	pdm_ctrl |= ACP_WOV_MISC_CTRL_MASK;
>>> +	pdm_ctrl |= FIELD_PREP(ACP_WOV_GAIN_CONTROL, pdm_gain);
>>
>> The bits should be zeroed (AND - &) before OR to make sure that the correct
>> value is written to the register. More related patches are affected.
> 
> I had consider this, but the hardware default at reset is 0x0.  Do you think it's
> still necessary for posterity?

You're using 0644 permissions for the module parameter, so the value can be 
changed by root using sysfs anytime (between SNDRV_PCM_TRIGGER calls).

						Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH 2/6] ASoC: amd: yc: Add a module parameter to influence pdm_gain

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

On Fri, Jan 27, 2023 at 05:41:44PM +0100, Jaroslav Kysela wrote:
> On 27. 01. 23 17:25, Limonciello, Mario wrote:

> > > >    	pdm_ctrl = acp6x_readl(acp_base + ACP_WOV_MISC_CTRL);
> > > > -	pdm_ctrl |= ACP_WOV_MISC_CTRL_MASK;
> > > > +	pdm_ctrl |= FIELD_PREP(ACP_WOV_GAIN_CONTROL, pdm_gain);

> > > The bits should be zeroed (AND - &) before OR to make sure that the correct
> > > value is written to the register. More related patches are affected.

> > I had consider this, but the hardware default at reset is 0x0.  Do you think it's
> > still necessary for posterity?

> You're using 0644 permissions for the module parameter, so the value can be
> changed by root using sysfs anytime (between SNDRV_PCM_TRIGGER calls).

Plus even if the value were never changed it just saves anyone
having to wonder if this has been taken care of, and means that
if there's any future hardware using this driver that uses a
different default (or a stale value in the hardware somehow) then
things will just work.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 3/6] ASoC: amd: renoir: Adjust the gain for PDM DMIC

[Mario Limonciello]

A similar issue that was reported on Rembrandt based laptops with
low DMIC volume is also being reported for Barcelo based laptops
that use renoir acp3x.

Increase the PDM gain to overcome this problem.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 sound/soc/amd/renoir/rn_acp3x.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/amd/renoir/rn_acp3x.h b/sound/soc/amd/renoir/rn_acp3x.h
index ca586603d7203..c174f84650d24 100644
--- a/sound/soc/amd/renoir/rn_acp3x.h
+++ b/sound/soc/amd/renoir/rn_acp3x.h
@@ -34,7 +34,7 @@
 #define ACP_ERROR_STAT 29
 #define PDM_DECIMATION_FACTOR 0x2
 #define ACP_PDM_CLK_FREQ_MASK 0x07
-#define ACP_WOV_MISC_CTRL_MASK 0x10
+#define ACP_WOV_MISC_CTRL_MASK 0x18
 #define ACP_PDM_ENABLE 0x01
 #define ACP_PDM_DISABLE 0x00
 #define ACP_PDM_DMA_EN_STATUS 0x02
-- 
2.34.1

[PATCH 4/6] ASoC: amd: renoir: Add a module parameter to influence pdm_gain

[Mario Limonciello]

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.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 sound/soc/amd/renoir/acp3x-pdm-dma.c | 7 ++++++-
 sound/soc/amd/renoir/rn_acp3x.h      | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/renoir/acp3x-pdm-dma.c b/sound/soc/amd/renoir/acp3x-pdm-dma.c
index 7203c6488df0e..74b70bc4b52fd 100644
--- a/sound/soc/amd/renoir/acp3x-pdm-dma.c
+++ b/sound/soc/amd/renoir/acp3x-pdm-dma.c
@@ -6,6 +6,7 @@
 
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/bitfield.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/pm_runtime.h>
@@ -17,6 +18,10 @@
 
 #define DRV_NAME "acp_rn_pdm_dma"
 
+static int pdm_gain = 3;
+module_param(pdm_gain, int, 0644);
+MODULE_PARM_DESC(pdm_gain, "Gain control (0-3)");
+
 static const struct snd_pcm_hardware acp_pdm_hardware_capture = {
 	.info = SNDRV_PCM_INFO_INTERLEAVED |
 		SNDRV_PCM_INFO_BLOCK_TRANSFER |
@@ -80,7 +85,7 @@ static void enable_pdm_clock(void __iomem *acp_base)
 
 	rn_writel(pdm_clk_enable, acp_base + ACP_WOV_CLK_CTRL);
 	pdm_ctrl = rn_readl(acp_base + ACP_WOV_MISC_CTRL);
-	pdm_ctrl |= ACP_WOV_MISC_CTRL_MASK;
+	pdm_ctrl |= FIELD_PREP(ACP_WOV_GAIN_CONTROL, pdm_gain);
 	rn_writel(pdm_ctrl, acp_base + ACP_WOV_MISC_CTRL);
 }
 
diff --git a/sound/soc/amd/renoir/rn_acp3x.h b/sound/soc/amd/renoir/rn_acp3x.h
index c174f84650d24..7d0f4e6a2834d 100644
--- a/sound/soc/amd/renoir/rn_acp3x.h
+++ b/sound/soc/amd/renoir/rn_acp3x.h
@@ -34,7 +34,7 @@
 #define ACP_ERROR_STAT 29
 #define PDM_DECIMATION_FACTOR 0x2
 #define ACP_PDM_CLK_FREQ_MASK 0x07
-#define ACP_WOV_MISC_CTRL_MASK 0x18
+#define ACP_WOV_GAIN_CONTROL	GENMASK(4, 3)
 #define ACP_PDM_ENABLE 0x01
 #define ACP_PDM_DISABLE 0x00
 #define ACP_PDM_DMA_EN_STATUS 0x02
-- 
2.34.1

[PATCH 5/6] ASoC: amd: ps: Adjust the gain for PDM DMIC

[Mario Limonciello]

No issues have been reported yet for DMIC audio level on ps platforms,
but as problems were found both on YC (Rembrandt) and Renoir based
designs it's very likely they happen on ps too.

Increase the PDM gain to solve this problem.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 sound/soc/amd/ps/acp63.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h
index 5e7f9c1c1b0e8..bae8288d8231c 100644
--- a/sound/soc/amd/ps/acp63.h
+++ b/sound/soc/amd/ps/acp63.h
@@ -30,7 +30,7 @@
 #define ACP_ERROR_STAT	29
 #define PDM_DECIMATION_FACTOR	2
 #define ACP_PDM_CLK_FREQ_MASK	7
-#define ACP_WOV_MISC_CTRL_MASK	0x10
+#define ACP_WOV_MISC_CTRL_MASK	0x18
 #define ACP_PDM_ENABLE		1
 #define ACP_PDM_DISABLE		0
 #define ACP_PDM_DMA_EN_STATUS	2
-- 
2.34.1

[PATCH 6/6] ASoC: amd: ps: Add a module parameter to influence pdm_gain

[Mario Limonciello]

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.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 sound/soc/amd/ps/acp63.h      | 2 +-
 sound/soc/amd/ps/ps-pdm-dma.c | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h
index bae8288d8231c..bed4d51e226a5 100644
--- a/sound/soc/amd/ps/acp63.h
+++ b/sound/soc/amd/ps/acp63.h
@@ -30,7 +30,7 @@
 #define ACP_ERROR_STAT	29
 #define PDM_DECIMATION_FACTOR	2
 #define ACP_PDM_CLK_FREQ_MASK	7
-#define ACP_WOV_MISC_CTRL_MASK	0x18
+#define ACP_WOV_GAIN_CONTROL	GENMASK(4, 3)
 #define ACP_PDM_ENABLE		1
 #define ACP_PDM_DISABLE		0
 #define ACP_PDM_DMA_EN_STATUS	2
diff --git a/sound/soc/amd/ps/ps-pdm-dma.c b/sound/soc/amd/ps/ps-pdm-dma.c
index eea71a9d2ef1a..1e773c5496c5f 100644
--- a/sound/soc/amd/ps/ps-pdm-dma.c
+++ b/sound/soc/amd/ps/ps-pdm-dma.c
@@ -7,6 +7,7 @@
 
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/bitfield.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <sound/pcm_params.h>
@@ -18,6 +19,10 @@
 
 #define DRV_NAME "acp_ps_pdm_dma"
 
+static int pdm_gain = 3;
+module_param(pdm_gain, int, 0644);
+MODULE_PARM_DESC(pdm_gain, "Gain control (0-3)");
+
 static const struct snd_pcm_hardware acp63_pdm_hardware_capture = {
 	.info = SNDRV_PCM_INFO_INTERLEAVED |
 		SNDRV_PCM_INFO_BLOCK_TRANSFER |
@@ -55,7 +60,7 @@ static void acp63_enable_pdm_clock(void __iomem *acp_base)
 
 	acp63_writel(pdm_clk_enable, acp_base + ACP_WOV_CLK_CTRL);
 	pdm_ctrl = acp63_readl(acp_base + ACP_WOV_MISC_CTRL);
-	pdm_ctrl |= ACP_WOV_MISC_CTRL_MASK;
+	pdm_ctrl |= FIELD_PREP(ACP_WOV_GAIN_CONTROL, pdm_gain);
 	acp63_writel(pdm_ctrl, acp_base + ACP_WOV_MISC_CTRL);
 }
 
-- 
2.34.1