[PATCH] ASoC: SOF: Intel: hda: Fix compressed stream position tracking
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Thu Jun 16 22:19:53 CEST 2022
From: Peter Ujfalusi <peter.ujfalusi at linux.intel.com>
Commit 288fad2f71fa ("ASoC: SOF: Intel: hda: add quirks for HDAudio DMA position information")
modified the PCM path only, but left the compressed data patch using an
obsolete option.
Move the functionality in a helper that can be called for both PCM and
compressed data.
Reviewed-by: Ranjani Sridharan <ranjani.sridharan at linux.intel.com>
Fixes: 288fad2f71fa ("ASoC: SOF: Intel: hda: add quirks for HDAudio DMA position information")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi at linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
---
sound/soc/sof/intel/hda-pcm.c | 74 +------------------------
sound/soc/sof/intel/hda-stream.c | 94 ++++++++++++++++++++++++++++++--
sound/soc/sof/intel/hda.h | 3 +
3 files changed, 94 insertions(+), 77 deletions(-)
diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c
index dc1f743730c0d..6888e0a4665d2 100644
--- a/sound/soc/sof/intel/hda-pcm.c
+++ b/sound/soc/sof/intel/hda-pcm.c
@@ -192,79 +192,7 @@ snd_pcm_uframes_t hda_dsp_pcm_pointer(struct snd_sof_dev *sdev,
goto found;
}
- switch (sof_hda_position_quirk) {
- case SOF_HDA_POSITION_QUIRK_USE_SKYLAKE_LEGACY:
- /*
- * This legacy code, inherited from the Skylake driver,
- * mixes DPIB registers and DPIB DDR updates and
- * does not seem to follow any known hardware recommendations.
- * It's not clear e.g. why there is a different flow
- * for capture and playback, the only information that matters is
- * what traffic class is used, and on all SOF-enabled platforms
- * only VC0 is supported so the work-around was likely not necessary
- * and quite possibly wrong.
- */
-
- /* DPIB/posbuf position mode:
- * For Playback, Use DPIB register from HDA space which
- * reflects the actual data transferred.
- * For Capture, Use the position buffer for pointer, as DPIB
- * is not accurate enough, its update may be completed
- * earlier than the data written to DDR.
- */
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
- pos = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR,
- AZX_REG_VS_SDXDPIB_XBASE +
- (AZX_REG_VS_SDXDPIB_XINTERVAL *
- hstream->index));
- } else {
- /*
- * For capture stream, we need more workaround to fix the
- * position incorrect issue:
- *
- * 1. Wait at least 20us before reading position buffer after
- * the interrupt generated(IOC), to make sure position update
- * happens on frame boundary i.e. 20.833uSec for 48KHz.
- * 2. Perform a dummy Read to DPIB register to flush DMA
- * position value.
- * 3. Read the DMA Position from posbuf. Now the readback
- * value should be >= period boundary.
- */
- usleep_range(20, 21);
- snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR,
- AZX_REG_VS_SDXDPIB_XBASE +
- (AZX_REG_VS_SDXDPIB_XINTERVAL *
- hstream->index));
- pos = snd_hdac_stream_get_pos_posbuf(hstream);
- }
- break;
- case SOF_HDA_POSITION_QUIRK_USE_DPIB_REGISTERS:
- /*
- * In case VC1 traffic is disabled this is the recommended option
- */
- pos = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR,
- AZX_REG_VS_SDXDPIB_XBASE +
- (AZX_REG_VS_SDXDPIB_XINTERVAL *
- hstream->index));
- break;
- case SOF_HDA_POSITION_QUIRK_USE_DPIB_DDR_UPDATE:
- /*
- * This is the recommended option when VC1 is enabled.
- * While this isn't needed for SOF platforms it's added for
- * consistency and debug.
- */
- pos = snd_hdac_stream_get_pos_posbuf(hstream);
- break;
- default:
- dev_err_once(sdev->dev, "hda_position_quirk value %d not supported\n",
- sof_hda_position_quirk);
- pos = 0;
- break;
- }
-
- if (pos >= hstream->bufsize)
- pos = 0;
-
+ pos = hda_dsp_stream_get_position(hstream, substream->stream, true);
found:
pos = bytes_to_frames(substream->runtime, pos);
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c
index daeb64c495e40..d95ae17e81cc4 100644
--- a/sound/soc/sof/intel/hda-stream.c
+++ b/sound/soc/sof/intel/hda-stream.c
@@ -707,12 +707,13 @@ bool hda_dsp_check_stream_irq(struct snd_sof_dev *sdev)
}
static void
-hda_dsp_set_bytes_transferred(struct hdac_stream *hstream, u64 buffer_size)
+hda_dsp_compr_bytes_transferred(struct hdac_stream *hstream, int direction)
{
+ u64 buffer_size = hstream->bufsize;
u64 prev_pos, pos, num_bytes;
div64_u64_rem(hstream->curr_pos, buffer_size, &prev_pos);
- pos = snd_hdac_stream_get_pos_posbuf(hstream);
+ pos = hda_dsp_stream_get_position(hstream, direction, false);
if (pos < prev_pos)
num_bytes = (buffer_size - prev_pos) + pos;
@@ -748,8 +749,7 @@ static bool hda_dsp_stream_check(struct hdac_bus *bus, u32 status)
if (s->substream && sof_hda->no_ipc_position) {
snd_sof_pcm_period_elapsed(s->substream);
} else if (s->cstream) {
- hda_dsp_set_bytes_transferred(s,
- s->cstream->runtime->buffer_size);
+ hda_dsp_compr_bytes_transferred(s, s->cstream->direction);
snd_compr_fragment_elapsed(s->cstream);
}
}
@@ -1009,3 +1009,89 @@ void hda_dsp_stream_free(struct snd_sof_dev *sdev)
devm_kfree(sdev->dev, hda_stream);
}
}
+
+snd_pcm_uframes_t hda_dsp_stream_get_position(struct hdac_stream *hstream,
+ int direction, bool can_sleep)
+{
+ struct hdac_ext_stream *hext_stream = stream_to_hdac_ext_stream(hstream);
+ struct sof_intel_hda_stream *hda_stream = hstream_to_sof_hda_stream(hext_stream);
+ struct snd_sof_dev *sdev = hda_stream->sdev;
+ snd_pcm_uframes_t pos;
+
+ switch (sof_hda_position_quirk) {
+ case SOF_HDA_POSITION_QUIRK_USE_SKYLAKE_LEGACY:
+ /*
+ * This legacy code, inherited from the Skylake driver,
+ * mixes DPIB registers and DPIB DDR updates and
+ * does not seem to follow any known hardware recommendations.
+ * It's not clear e.g. why there is a different flow
+ * for capture and playback, the only information that matters is
+ * what traffic class is used, and on all SOF-enabled platforms
+ * only VC0 is supported so the work-around was likely not necessary
+ * and quite possibly wrong.
+ */
+
+ /* DPIB/posbuf position mode:
+ * For Playback, Use DPIB register from HDA space which
+ * reflects the actual data transferred.
+ * For Capture, Use the position buffer for pointer, as DPIB
+ * is not accurate enough, its update may be completed
+ * earlier than the data written to DDR.
+ */
+ if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
+ pos = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR,
+ AZX_REG_VS_SDXDPIB_XBASE +
+ (AZX_REG_VS_SDXDPIB_XINTERVAL *
+ hstream->index));
+ } else {
+ /*
+ * For capture stream, we need more workaround to fix the
+ * position incorrect issue:
+ *
+ * 1. Wait at least 20us before reading position buffer after
+ * the interrupt generated(IOC), to make sure position update
+ * happens on frame boundary i.e. 20.833uSec for 48KHz.
+ * 2. Perform a dummy Read to DPIB register to flush DMA
+ * position value.
+ * 3. Read the DMA Position from posbuf. Now the readback
+ * value should be >= period boundary.
+ */
+ if (can_sleep)
+ usleep_range(20, 21);
+
+ snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR,
+ AZX_REG_VS_SDXDPIB_XBASE +
+ (AZX_REG_VS_SDXDPIB_XINTERVAL *
+ hstream->index));
+ pos = snd_hdac_stream_get_pos_posbuf(hstream);
+ }
+ break;
+ case SOF_HDA_POSITION_QUIRK_USE_DPIB_REGISTERS:
+ /*
+ * In case VC1 traffic is disabled this is the recommended option
+ */
+ pos = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR,
+ AZX_REG_VS_SDXDPIB_XBASE +
+ (AZX_REG_VS_SDXDPIB_XINTERVAL *
+ hstream->index));
+ break;
+ case SOF_HDA_POSITION_QUIRK_USE_DPIB_DDR_UPDATE:
+ /*
+ * This is the recommended option when VC1 is enabled.
+ * While this isn't needed for SOF platforms it's added for
+ * consistency and debug.
+ */
+ pos = snd_hdac_stream_get_pos_posbuf(hstream);
+ break;
+ default:
+ dev_err_once(sdev->dev, "hda_position_quirk value %d not supported\n",
+ sof_hda_position_quirk);
+ pos = 0;
+ break;
+ }
+
+ if (pos >= hstream->bufsize)
+ pos = 0;
+
+ return pos;
+}
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index a3118499e34fc..dc713c20ba1d9 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -565,6 +565,9 @@ int hda_dsp_stream_setup_bdl(struct snd_sof_dev *sdev,
bool hda_dsp_check_ipc_irq(struct snd_sof_dev *sdev);
bool hda_dsp_check_stream_irq(struct snd_sof_dev *sdev);
+snd_pcm_uframes_t hda_dsp_stream_get_position(struct hdac_stream *hstream,
+ int direction, bool can_sleep);
+
struct hdac_ext_stream *
hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction, u32 flags);
int hda_dsp_stream_put(struct snd_sof_dev *sdev, int direction, int stream_tag);
--
2.34.1
More information about the Alsa-devel
mailing list