mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Agrawal, Akshu" <Akshu.Agrawal@amd.com>
To: unlisted-recipients:; (no To-header on input)
Cc: "Agrawal, Akshu" <Akshu.Agrawal@amd.com>,
	"djkurtz@chromium.org" <djkurtz@chromium.org>,
	"Deucher, Alexander" <Alexander.Deucher@amd.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	"Mukunda, Vijendar" <Vijendar.Mukunda@amd.com>,
	"Deucher, Alexander" <Alexander.Deucher@amd.com>,
	Guenter Roeck <linux@roeck-us.net>,
	"moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER
	MANAGEM..."  <alsa-devel@alsa-project.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: [PATCH] ASoC: AMD: Fix race condition between register access
Date: Mon, 29 Oct 2018 07:38:58 +0000	[thread overview]
Message-ID: <1540798621-32627-1-git-send-email-akshu.agrawal@amd.com> (raw)

During simultaneous running of playback and capture, we
got hit by incorrect value write on common register. This was due
to race condition between 2 streams.
Fixing this by locking the common register access.

Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
---
 sound/soc/amd/acp-pcm-dma.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 0ac4b5b..993a7db 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -121,6 +121,9 @@
 	.periods_max = CAPTURE_MAX_NUM_PERIODS,
 };
 
+/* Lock to protect access to registers */
+static DEFINE_SPINLOCK(lock);
+
 static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg)
 {
 	return readl(acp_mmio + (reg * 4));
@@ -168,9 +171,12 @@ static void config_dma_descriptor_in_sram(void __iomem *acp_mmio,
 					  acp_dma_dscr_transfer_t *descr_info)
 {
 	u32 sram_offset;
+	unsigned long flags;
 
 	sram_offset = (descr_idx * sizeof(acp_dma_dscr_transfer_t));
 
+	spin_lock_irqsave(&lock, flags);
+
 	/* program the source base address. */
 	acp_reg_write(sram_offset, acp_mmio, mmACP_SRBM_Targ_Idx_Addr);
 	acp_reg_write(descr_info->src,	acp_mmio, mmACP_SRBM_Targ_Idx_Data);
@@ -181,6 +187,8 @@ static void config_dma_descriptor_in_sram(void __iomem *acp_mmio,
 	/* program the number of bytes to be transferred for this descriptor. */
 	acp_reg_write(sram_offset + 8,	acp_mmio, mmACP_SRBM_Targ_Idx_Addr);
 	acp_reg_write(descr_info->xfer_val, acp_mmio, mmACP_SRBM_Targ_Idx_Data);
+
+	spin_unlock_irqrestore(&lock, flags);
 }
 
 static void pre_config_reset(void __iomem *acp_mmio, u16 ch_num)
@@ -309,8 +317,12 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 	u32 low;
 	u32 high;
 	u32 offset;
+	unsigned long flags;
 
 	offset	= ACP_DAGB_GRP_SRBM_SRAM_BASE_OFFSET + (pte_offset * 8);
+
+	spin_lock_irqsave(&lock, flags);
+
 	for (page_idx = 0; page_idx < (num_of_pages); page_idx++) {
 		/* Load the low address of page int ACP SRAM through SRBM */
 		acp_reg_write((offset + (page_idx * 8)),
@@ -333,6 +345,8 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 		/* Move to next physically contiguos page */
 		pg++;
 	}
+
+	spin_unlock_irqrestore(&lock, flags);
 }
 
 static void config_acp_dma(void __iomem *acp_mmio,
@@ -367,6 +381,7 @@ static void acp_dma_cap_channel_enable(void __iomem *acp_mmio,
 				       u16 cap_channel)
 {
 	u32 val, ch_reg, imr_reg, res_reg;
+	unsigned long flags;
 
 	switch (cap_channel) {
 	case CAP_CHANNEL1:
@@ -381,6 +396,8 @@ static void acp_dma_cap_channel_enable(void __iomem *acp_mmio,
 		imr_reg = mmACP_I2SMICSP_IMR0;
 		break;
 	}
+	spin_lock_irqsave(&lock, flags);
+
 	val = acp_reg_read(acp_mmio,
 			   mmACP_I2S_16BIT_RESOLUTION_EN);
 	if (val & ACP_I2S_MIC_16BIT_RESOLUTION_EN) {
@@ -393,12 +410,15 @@ static void acp_dma_cap_channel_enable(void __iomem *acp_mmio,
 	val &= ~ACP_I2SMICSP_IMR1__I2SMICSP_RXFOM_MASK;
 	acp_reg_write(val, acp_mmio, imr_reg);
 	acp_reg_write(0x1, acp_mmio, ch_reg);
+
+	spin_unlock_irqrestore(&lock, flags);
 }
 
 static void acp_dma_cap_channel_disable(void __iomem *acp_mmio,
 					u16 cap_channel)
 {
 	u32 val, ch_reg, imr_reg;
+	unsigned long flags;
 
 	switch (cap_channel) {
 	case CAP_CHANNEL1:
@@ -411,11 +431,15 @@ static void acp_dma_cap_channel_disable(void __iomem *acp_mmio,
 		ch_reg = mmACP_I2SMICSP_RER0;
 		break;
 	}
+	spin_lock_irqsave(&lock, flags);
+
 	val = acp_reg_read(acp_mmio, imr_reg);
 	val |= ACP_I2SMICSP_IMR1__I2SMICSP_RXDAM_MASK;
 	val |= ACP_I2SMICSP_IMR1__I2SMICSP_RXFOM_MASK;
 	acp_reg_write(val, acp_mmio, imr_reg);
 	acp_reg_write(0x0, acp_mmio, ch_reg);
+
+	spin_unlock_irqrestore(&lock, flags);
 }
 
 /* Start a given DMA channel transfer */
@@ -847,6 +871,7 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 	int status;
 	uint64_t size;
 	u32 val = 0;
+	unsigned long flags;
 	struct page *pg;
 	struct snd_pcm_runtime *runtime;
 	struct audio_substream_data *rtd;
@@ -870,6 +895,8 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 		}
 	}
 	if (adata->asic_type == CHIP_STONEY) {
+		spin_lock_irqsave(&lock, flags);
+
 		val = acp_reg_read(adata->acp_mmio,
 				   mmACP_I2S_16BIT_RESOLUTION_EN);
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -893,6 +920,8 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 		}
 		acp_reg_write(val, adata->acp_mmio,
 			      mmACP_I2S_16BIT_RESOLUTION_EN);
+
+		spin_unlock_irqrestore(&lock, flags);
 	}
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-- 
1.9.1


             reply	other threads:[~2018-10-29  7:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-29  7:38 Agrawal, Akshu [this message]
2018-10-30  1:32 ` Daniel Kurtz
2018-10-30 15:48   ` Agrawal, Akshu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1540798621-32627-1-git-send-email-akshu.agrawal@amd.com \
    --to=akshu.agrawal@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Vijendar.Mukunda@amd.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=djkurtz@chromium.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®