Skip to content

Commit

Permalink
ASoC: SOF: Intel: hda: Fix compressed stream position tracking
Browse files Browse the repository at this point in the history 
Commit 288fad2 ("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: 288fad2 ("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>
  • Loading branch information
Peter Ujfalusi authored and Mark Brown committed Jun 24, 2022
1 parent 427eb3e commit ca7ab1d
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 77 deletions.
74 changes: 1 addition & 73 deletions sound/soc/sof/intel/hda-pcm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
94 changes: 90 additions & 4 deletions sound/soc/sof/intel/hda-stream.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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;
}
3 changes: 3 additions & 0 deletions sound/soc/sof/intel/hda.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit ca7ab1d

Please sign in to comment.