[PATCH 1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command

[PATCH 1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command

[Mastan Katragadda]

ACP driver should send SHA DMA completion command to PSP module for RN
platform only.
Add a revision check for RN platform.

Signed-off-by: Mastan Katragadda <Mastan.Katragadda@amd.com>
---
 sound/soc/sof/amd/acp.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c
index 2ae76bcd3590..afb505461ea1 100644
--- a/sound/soc/sof/amd/acp.c
+++ b/sound/soc/sof/amd/acp.c
@@ -217,6 +217,7 @@ int configure_and_run_sha_dma(struct acp_dev_data *adata, void *image_addr,
 			      unsigned int image_length)
 {
 	struct snd_sof_dev *sdev = adata->dev;
+	const struct sof_amd_acp_desc *desc = get_chip_info(sdev->pdata);
 	unsigned int tx_count, fw_qualifier, val;
 	int ret;
 
@@ -251,9 +252,12 @@ int configure_and_run_sha_dma(struct acp_dev_data *adata, void *image_addr,
 		return ret;
 	}
 
-	ret = psp_send_cmd(adata, MBOX_ACP_SHA_DMA_COMMAND);
-	if (ret)
-		return ret;
+	/* psp_send_cmd only required for renoir platform (rev - 3) */
+	if (desc->rev == 3) {
+		ret = psp_send_cmd(adata, MBOX_ACP_SHA_DMA_COMMAND);
+		if (ret)
+			return ret;
+	}
 
 	ret = snd_sof_dsp_read_poll_timeout(sdev, ACP_DSP_BAR, ACP_SHA_DSP_FW_QUALIFIER,
 					    fw_qualifier, fw_qualifier & DSP_FW_RUN_ENABLE,
-- 
2.25.1

[PATCH 2/2] ASoC: SOF: amd: refactor PSP smn_read

[Mastan Katragadda]

Use the read_poll_timeout marco for PSP smn_read calls.

Signed-off-by: Mastan Katragadda <Mastan.Katragadda@amd.com>
---
 sound/soc/sof/amd/acp.c | 34 +++++++++++++++-------------------
 sound/soc/sof/amd/acp.h |  4 ++--
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c
index afb505461ea1..c450931ae77e 100644
--- a/sound/soc/sof/amd/acp.c
+++ b/sound/soc/sof/amd/acp.c
@@ -28,12 +28,14 @@ static int smn_write(struct pci_dev *dev, u32 smn_addr, u32 data)
 	return 0;
 }
 
-static int smn_read(struct pci_dev *dev, u32 smn_addr, u32 *data)
+static int smn_read(struct pci_dev *dev, u32 smn_addr)
 {
+	u32 data = 0;
+
 	pci_write_config_dword(dev, 0x60, smn_addr);
-	pci_read_config_dword(dev, 0x64, data);
+	pci_read_config_dword(dev, 0x64, &data);
 
-	return 0;
+	return data;
 }
 
 static void init_dma_descriptor(struct acp_dev_data *adata)
@@ -150,15 +152,13 @@ int configure_and_run_dma(struct acp_dev_data *adata, unsigned int src_addr,
 static int psp_mbox_ready(struct acp_dev_data *adata, bool ack)
 {
 	struct snd_sof_dev *sdev = adata->dev;
-	int timeout;
+	int ret;
 	u32 data;
 
-	for (timeout = ACP_PSP_TIMEOUT_COUNTER; timeout > 0; timeout--) {
-		msleep(20);
-		smn_read(adata->smn_dev, MP0_C2PMSG_114_REG, &data);
-		if (data & MBOX_READY_MASK)
-			return 0;
-	}
+	ret = read_poll_timeout(smn_read, data, data & MBOX_READY_MASK, MBOX_DELAY_US,
+				ACP_PSP_TIMEOUT_US, false, adata->smn_dev, MP0_C2PMSG_114_REG);
+	if (!ret)
+		return 0;
 
 	dev_err(sdev->dev, "PSP error status %x\n", data & MBOX_STATUS_MASK);
 
@@ -177,23 +177,19 @@ static int psp_mbox_ready(struct acp_dev_data *adata, bool ack)
 static int psp_send_cmd(struct acp_dev_data *adata, int cmd)
 {
 	struct snd_sof_dev *sdev = adata->dev;
-	int ret, timeout;
+	int ret;
 	u32 data;
 
 	if (!cmd)
 		return -EINVAL;
 
 	/* Get a non-zero Doorbell value from PSP */
-	for (timeout = ACP_PSP_TIMEOUT_COUNTER; timeout > 0; timeout--) {
-		msleep(MBOX_DELAY);
-		smn_read(adata->smn_dev, MP0_C2PMSG_73_REG, &data);
-		if (data)
-			break;
-	}
+	ret = read_poll_timeout(smn_read, data, data, MBOX_DELAY_US, ACP_PSP_TIMEOUT_US, false,
+				adata->smn_dev, MP0_C2PMSG_73_REG);
 
-	if (!timeout) {
+	if (ret) {
 		dev_err(sdev->dev, "Failed to get Doorbell from MBOX %x\n", MP0_C2PMSG_73_REG);
-		return -EINVAL;
+		return ret;
 	}
 
 	/* Check if PSP is ready for new command */
diff --git a/sound/soc/sof/amd/acp.h b/sound/soc/sof/amd/acp.h
index dc624f727aa3..c3659dbc3745 100644
--- a/sound/soc/sof/amd/acp.h
+++ b/sound/soc/sof/amd/acp.h
@@ -61,12 +61,12 @@
 #define HOST_BRIDGE_CZN				0x1630
 #define HOST_BRIDGE_RMB				0x14B5
 #define ACP_SHA_STAT				0x8000
-#define ACP_PSP_TIMEOUT_COUNTER			5
+#define ACP_PSP_TIMEOUT_US			1000000
 #define ACP_EXT_INTR_ERROR_STAT			0x20000000
 #define MP0_C2PMSG_114_REG			0x3810AC8
 #define MP0_C2PMSG_73_REG			0x3810A24
 #define MBOX_ACP_SHA_DMA_COMMAND		0x70000
-#define MBOX_DELAY				1000
+#define MBOX_DELAY_US				1000
 #define MBOX_READY_MASK				0x80000000
 #define MBOX_STATUS_MASK			0xFFFF
 
-- 
2.25.1

Re: (subset) [PATCH 1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command

[Mark Brown]

On Fri, 30 Jun 2023 12:35:42 +0530, Mastan Katragadda wrote:
> ACP driver should send SHA DMA completion command to PSP module for RN
> platform only.
> Add a revision check for RN platform.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command
      commit: b1db244c716fe300e36428aeb0874913d2e0a91d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: (subset) [PATCH 1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command

[Mukunda,Vijendar]

On 30/06/23 22:37, Mark Brown wrote:
> On Fri, 30 Jun 2023 12:35:42 +0530, Mastan Katragadda wrote:
>> ACP driver should send SHA DMA completion command to PSP module for RN
>> platform only.
>> Add a revision check for RN platform.
>>
>>
> Applied to
>
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
>
> Thanks!
>
> [1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command
>       commit: b1db244c716fe300e36428aeb0874913d2e0a91d
It looks like second patch didn't get merged in this series.
Do we need to resend the second patch?

> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
>
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
>
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
>
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
>
> Thanks,
> Mark
>

Re: (subset) [PATCH 1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command

[Mark Brown]

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

On Mon, Jul 03, 2023 at 08:58:17PM +0530, Mukunda,Vijendar wrote:
> On 30/06/23 22:37, Mark Brown wrote:

> > Thanks!
> >
> > [1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command
> >       commit: b1db244c716fe300e36428aeb0874913d2e0a91d

> It looks like second patch didn't get merged in this series.
> Do we need to resend the second patch?

I don't know off hand what that second patch was.  If it wasn't a bug
fix it'll just be waiting till after the merge window.

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

Re: (subset) [PATCH 1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command

[Mastan Katragadda]

On 03/07/23 21:40, Mark Brown wrote:
> On Mon, Jul 03, 2023 at 08:58:17PM +0530, Mukunda,Vijendar wrote:
>> On 30/06/23 22:37, Mark Brown wrote:
>>> Thanks!
>>>
>>> [1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command
>>>        commit: b1db244c716fe300e36428aeb0874913d2e0a91d
>> It looks like second patch didn't get merged in this series.
>> Do we need to resend the second patch?
> I don't know off hand what that second patch was.  If it wasn't a bug
> fix it'll just be waiting till after the merge window.
  Do I need to resend the second patch?

Re: (subset) [PATCH 1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command

[Mark Brown]

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

On Wed, Jul 12, 2023 at 09:03:30AM +0530, Mastan Katragadda wrote:
> On 03/07/23 21:40, Mark Brown wrote:

> > I don't know off hand what that second patch was.  If it wasn't a bug
> > fix it'll just be waiting till after the merge window.

>  Do I need to resend the second patch?

I still don't know what the patch was - if it's not applied probably.

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

Re: [PATCH 2/2] ASoC: SOF: amd: refactor PSP smn_read

[Mastan Katragadda]

On 30/06/23 12:35, Mastan Katragadda wrote:
> Use the read_poll_timeout marco for PSP smn_read calls.
This patch has not been merged.
>
> Signed-off-by: Mastan Katragadda <Mastan.Katragadda@amd.com>
> ---
>   sound/soc/sof/amd/acp.c | 34 +++++++++++++++-------------------
>   sound/soc/sof/amd/acp.h |  4 ++--
>   2 files changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c
> index afb505461ea1..c450931ae77e 100644
> --- a/sound/soc/sof/amd/acp.c
> +++ b/sound/soc/sof/amd/acp.c
> @@ -28,12 +28,14 @@ static int smn_write(struct pci_dev *dev, u32 smn_addr, u32 data)
>   	return 0;
>   }
>   
> -static int smn_read(struct pci_dev *dev, u32 smn_addr, u32 *data)
> +static int smn_read(struct pci_dev *dev, u32 smn_addr)
>   {
> +	u32 data = 0;
> +
>   	pci_write_config_dword(dev, 0x60, smn_addr);
> -	pci_read_config_dword(dev, 0x64, data);
> +	pci_read_config_dword(dev, 0x64, &data);
>   
> -	return 0;
> +	return data;
>   }
>   
>   static void init_dma_descriptor(struct acp_dev_data *adata)
> @@ -150,15 +152,13 @@ int configure_and_run_dma(struct acp_dev_data *adata, unsigned int src_addr,
>   static int psp_mbox_ready(struct acp_dev_data *adata, bool ack)
>   {
>   	struct snd_sof_dev *sdev = adata->dev;
> -	int timeout;
> +	int ret;
>   	u32 data;
>   
> -	for (timeout = ACP_PSP_TIMEOUT_COUNTER; timeout > 0; timeout--) {
> -		msleep(20);
> -		smn_read(adata->smn_dev, MP0_C2PMSG_114_REG, &data);
> -		if (data & MBOX_READY_MASK)
> -			return 0;
> -	}
> +	ret = read_poll_timeout(smn_read, data, data & MBOX_READY_MASK, MBOX_DELAY_US,
> +				ACP_PSP_TIMEOUT_US, false, adata->smn_dev, MP0_C2PMSG_114_REG);
> +	if (!ret)
> +		return 0;
>   
>   	dev_err(sdev->dev, "PSP error status %x\n", data & MBOX_STATUS_MASK);
>   
> @@ -177,23 +177,19 @@ static int psp_mbox_ready(struct acp_dev_data *adata, bool ack)
>   static int psp_send_cmd(struct acp_dev_data *adata, int cmd)
>   {
>   	struct snd_sof_dev *sdev = adata->dev;
> -	int ret, timeout;
> +	int ret;
>   	u32 data;
>   
>   	if (!cmd)
>   		return -EINVAL;
>   
>   	/* Get a non-zero Doorbell value from PSP */
> -	for (timeout = ACP_PSP_TIMEOUT_COUNTER; timeout > 0; timeout--) {
> -		msleep(MBOX_DELAY);
> -		smn_read(adata->smn_dev, MP0_C2PMSG_73_REG, &data);
> -		if (data)
> -			break;
> -	}
> +	ret = read_poll_timeout(smn_read, data, data, MBOX_DELAY_US, ACP_PSP_TIMEOUT_US, false,
> +				adata->smn_dev, MP0_C2PMSG_73_REG);
>   
> -	if (!timeout) {
> +	if (ret) {
>   		dev_err(sdev->dev, "Failed to get Doorbell from MBOX %x\n", MP0_C2PMSG_73_REG);
> -		return -EINVAL;
> +		return ret;
>   	}
>   
>   	/* Check if PSP is ready for new command */
> diff --git a/sound/soc/sof/amd/acp.h b/sound/soc/sof/amd/acp.h
> index dc624f727aa3..c3659dbc3745 100644
> --- a/sound/soc/sof/amd/acp.h
> +++ b/sound/soc/sof/amd/acp.h
> @@ -61,12 +61,12 @@
>   #define HOST_BRIDGE_CZN				0x1630
>   #define HOST_BRIDGE_RMB				0x14B5
>   #define ACP_SHA_STAT				0x8000
> -#define ACP_PSP_TIMEOUT_COUNTER			5
> +#define ACP_PSP_TIMEOUT_US			1000000
>   #define ACP_EXT_INTR_ERROR_STAT			0x20000000
>   #define MP0_C2PMSG_114_REG			0x3810AC8
>   #define MP0_C2PMSG_73_REG			0x3810A24
>   #define MBOX_ACP_SHA_DMA_COMMAND		0x70000
> -#define MBOX_DELAY				1000
> +#define MBOX_DELAY_US				1000
>   #define MBOX_READY_MASK				0x80000000
>   #define MBOX_STATUS_MASK			0xFFFF
>   

Re: [PATCH 2/2] ASoC: SOF: amd: refactor PSP smn_read

[Mark Brown]

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

On Wed, Jul 12, 2023 at 04:38:46PM +0530, Mastan Katragadda wrote:
> 
> On 30/06/23 12:35, Mastan Katragadda wrote:
> > Use the read_poll_timeout marco for PSP smn_read calls.
> This patch has not been merged.

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

> > Signed-off-by: Mastan Katragadda <Mastan.Katragadda@amd.com>

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

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

Re: (subset) [PATCH 1/2] ASoC: SOF: amd: add revision check for sending sha dma completion command

[Mark Brown]

On Fri, 30 Jun 2023 12:35:42 +0530, Mastan Katragadda wrote:
> ACP driver should send SHA DMA completion command to PSP module for RN
> platform only.
> Add a revision check for RN platform.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[2/2] ASoC: SOF: amd: refactor PSP smn_read
      commit: 2b48d170fb9965dda9d41edcb0bbfc9ee4c6584f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark