Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Nikula <jarkko.nikula@linux.intel.com>
To: alsa-devel@alsa-project.org
Cc: Liam Girdwood <liam.r.girdwood@linux.intel.com>,
	Mark Brown <broonie@kernel.org>,
	Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	Liam Girdwood <lgirdwood@gmail.com>
Subject: [PATCH 1/7] ASoC: Intel: Sample Baytrail DSP DMA pointer only after each period
Date: Thu,  8 May 2014 16:07:21 +0300	[thread overview]
Message-ID: <1399554447-16297-2-git-send-email-jarkko.nikula@linux.intel.com> (raw)
In-Reply-To: <1399554447-16297-1-git-send-email-jarkko.nikula@linux.intel.com>

This is for preparing suspend/resume support but can give also more
safeguard against concurrent timestamp structure access between DSP firmware
and host.

Now DSP DMA pointer is sampled in each pcm pointer callback in
sst_byt_pcm_pointer() but that is unneeded since DSP updates the timestamp
period basis and can potentially be racy if sst_byt_pcm_pointer() is called
when DSP is updating the timestamp.

By taking DSP DMA pointer only after period elapsed IPC messages in
byt_notify_pointer() and returning stored hw pointer in
sst_byt_pcm_pointer() there is less risk for concurrent access.

The same stored hw pointer can be also used in suspend/resume code for
restarting the stream at the same position.

Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
 sound/soc/intel/sst-baytrail-pcm.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/sound/soc/intel/sst-baytrail-pcm.c b/sound/soc/intel/sst-baytrail-pcm.c
index 6d101f3813b4..b8a89e9f753d 100644
--- a/sound/soc/intel/sst-baytrail-pcm.c
+++ b/sound/soc/intel/sst-baytrail-pcm.c
@@ -45,6 +45,9 @@ struct sst_byt_pcm_data {
 	struct sst_byt_stream *stream;
 	struct snd_pcm_substream *substream;
 	struct mutex mutex;
+
+	/* latest DSP DMA hw pointer */
+	u32 hw_ptr;
 };
 
 /* private data for the driver */
@@ -168,13 +171,19 @@ static u32 byt_notify_pointer(struct sst_byt_stream *stream, void *data)
 	struct snd_pcm_substream *substream = pcm_data->substream;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	u32 pos;
+	struct sst_byt_priv_data *pdata =
+		snd_soc_platform_get_drvdata(rtd->platform);
+	struct sst_byt *byt = pdata->byt;
+	u32 pos, hw_pos;
 
+	hw_pos = sst_byt_get_dsp_position(byt, pcm_data->stream,
+					  snd_pcm_lib_buffer_bytes(substream));
+	pcm_data->hw_ptr = hw_pos;
 	pos = frames_to_bytes(runtime,
 			      (runtime->control->appl_ptr %
 			       runtime->buffer_size));
 
-	dev_dbg(rtd->dev, "PCM: App pointer %d bytes\n", pos);
+	dev_dbg(rtd->dev, "PCM: App/DMA pointer %u/%u bytes\n", pos, hw_pos);
 
 	snd_pcm_period_elapsed(substream);
 	return pos;
@@ -184,20 +193,11 @@ static snd_pcm_uframes_t sst_byt_pcm_pointer(struct snd_pcm_substream *substream
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct sst_byt_priv_data *pdata =
-		snd_soc_platform_get_drvdata(rtd->platform);
 	struct sst_byt_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(rtd);
-	struct sst_byt *byt = pdata->byt;
-	snd_pcm_uframes_t offset;
-	int pos;
 
-	pos = sst_byt_get_dsp_position(byt, pcm_data->stream,
-				       snd_pcm_lib_buffer_bytes(substream));
-	offset = bytes_to_frames(runtime, pos);
+	dev_dbg(rtd->dev, "PCM: DMA pointer %u bytes\n", pcm_data->hw_ptr);
 
-	dev_dbg(rtd->dev, "PCM: DMA pointer %zu bytes\n",
-		frames_to_bytes(runtime, (u32)offset));
-	return offset;
+	return bytes_to_frames(runtime, pcm_data->hw_ptr);
 }
 
 static int sst_byt_pcm_open(struct snd_pcm_substream *substream)
-- 
2.0.0.rc0

  reply	other threads:[~2014-05-08 13:08 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-08 13:07 [PATCH 0/7] ASoC: Intel: Baytrail SST suspend/resume support Jarkko Nikula
2014-05-08 13:07 ` Jarkko Nikula [this message]
2014-05-08 13:07 ` [PATCH 2/7] ASoC: Intel: Simplify Baytrail stream control IPC construction Jarkko Nikula
2014-05-08 13:07 ` [PATCH 3/7] ASoC: Intel: Pass stream start position to sst_byt_stream_start() Jarkko Nikula
2014-05-08 13:07 ` [PATCH 4/7] ASoC: Intel: Move Baytrail extended fw address saving to sst_byt_boot() Jarkko Nikula
2014-05-08 13:07 ` [PATCH 5/7] ASoC: Intel: Allow Rx/Tx message list can be cleared prior to suspend Jarkko Nikula
2014-05-08 13:07 ` [PATCH 6/7] ASoC: Intel: Add Baytrail suspend/resume support Jarkko Nikula
2014-05-08 13:07 ` [PATCH 7/7] ASoC: Intel: Allow byt-5640 machine driver and SST core go to suspend Jarkko Nikula
2014-05-12 21:02 ` [PATCH 0/7] ASoC: Intel: Baytrail SST suspend/resume support Mark Brown

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=1399554447-16297-2-git-send-email-jarkko.nikula@linux.intel.com \
    --to=jarkko.nikula@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=liam.r.girdwood@linux.intel.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