ASoC: SOF: Intel: hda: Fix compressed stream position tracking

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@linux.intel.com>
Fixes: 288fad2f71fa ("ASoC: SOF: Intel: hda: add quirks for HDAudio DMA position information")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20220616201953.130876-1-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>

diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c
index dc1f743..6888e0a 100644 (file)
--- 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 daeb64c..d95ae17 100644 (file)
--- 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 0f57ef5..06476ff 100644 (file)
--- 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);