Unverified Commit a37a9224 authored by Peter Ujfalusi's avatar Peter Ujfalusi Committed by Mark Brown
Browse files

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



Commit 288fad2f ("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: default avatarRanjani Sridharan <ranjani.sridharan@linux.intel.com>
Fixes: 288fad2f ("ASoC: SOF: Intel: hda: add quirks for HDAudio DMA position information")
Signed-off-by: default avatarPeter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: default avatarPierre-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: default avatarMark Brown <broonie@kernel.org>
parent 62257638
Loading
Loading
Loading
Loading
+1 −73
Original line number Diff line number Diff line
@@ -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);

+90 −4
Original line number Diff line number Diff line
@@ -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;
}
+3 −0
Original line number Diff line number Diff line
@@ -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);