On Fri, 15 Oct 2021 21:59:32 +0200, Pierre-Louis Bossart wrote:
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
When we disable rewinds, then the .ack can be used to program SPIB with the application pointer, which allows the HDaudio DMA to save power by opportunistically bursting data transfers when the path to memory is enabled (and conversely to shut it down when there are no transfer requests).
The SPIB register can only be programmed with incremental values with wrap-around after the DMA RUN bits are set. For simplicity, we set the INFO_NO_REWINDS flag in the .open callback when we already need to program the SNDRV_PCM_INFO_EXPLICIT_SYNC flag.
Using this flag itself isn't wrong, but if we need to check only appl_ptr updates, a more appropriate flag is SNDRV_PCM_INFO_SYNC_APPLPTR. This will still allow the mmap of status (i.e. hwptr update) while the mmap of control is disabled for appl_ptr. SNDRV_PCM_INFO_EXPLICIT_SYNC flag disables both, instead.
thanks,
Takashi
Rewinds are not used by many applications. One notable application using rewinds is PulseAudio. Practical experiments with Ubuntu/PulseAudio default settings did not show any audible issues, but the user may hear volume changes and notification with a delay, depending on the size of the ring buffer and latency constraints.
The choice of disabling rewinds is exposed as a kernel parameter and not a Kconfig option to avoid any undesirable side-effects.
Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Co-developed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com
sound/soc/sof/intel/apl.c | 1 + sound/soc/sof/intel/cnl.c | 1 + sound/soc/sof/intel/hda-pcm.c | 41 ++++++++++++++++++++++++++++++-- sound/soc/sof/intel/hda-stream.c | 2 ++ sound/soc/sof/intel/hda.h | 1 + sound/soc/sof/intel/tgl.c | 1 + 6 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c index 917f78cf6daf..689679014ade 100644 --- a/sound/soc/sof/intel/apl.c +++ b/sound/soc/sof/intel/apl.c @@ -78,6 +78,7 @@ const struct snd_sof_dsp_ops sof_apl_ops = { .pcm_hw_free = hda_dsp_stream_hw_free, .pcm_trigger = hda_dsp_pcm_trigger, .pcm_pointer = hda_dsp_pcm_pointer,
- .pcm_ack = hda_dsp_pcm_ack,
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES) /* probe callbacks */ diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index 3957e2b3db32..d128c08ba726 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -283,6 +283,7 @@ const struct snd_sof_dsp_ops sof_cnl_ops = { .pcm_hw_free = hda_dsp_stream_hw_free, .pcm_trigger = hda_dsp_pcm_trigger, .pcm_pointer = hda_dsp_pcm_pointer,
- .pcm_ack = hda_dsp_pcm_ack,
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES) /* probe callbacks */ diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c index cc8ddef37f37..875350283eac 100644 --- a/sound/soc/sof/intel/hda-pcm.c +++ b/sound/soc/sof/intel/hda-pcm.c @@ -32,6 +32,10 @@ static bool hda_always_enable_dmi_l1; module_param_named(always_enable_dmi_l1, hda_always_enable_dmi_l1, bool, 0444); MODULE_PARM_DESC(always_enable_dmi_l1, "SOF HDA always enable DMI l1");
+static bool hda_disable_rewinds = IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_DISABLE_REWINDS); +module_param_named(disable_rewinds, hda_disable_rewinds, bool, 0444); +MODULE_PARM_DESC(disable_rewinds, "SOF HDA disable rewinds");
u32 hda_dsp_get_mult_div(struct snd_sof_dev *sdev, int rate) { switch (rate) { @@ -120,8 +124,11 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev, return ret; }
- /* disable SPIB, to enable buffer wrap for stream */
- hda_dsp_stream_spib_config(sdev, stream, HDA_DSP_SPIB_DISABLE, 0);
/* enable SPIB when rewinds are disabled */
if (hda_disable_rewinds)
hda_dsp_stream_spib_config(sdev, stream, HDA_DSP_SPIB_ENABLE, 0);else
hda_dsp_stream_spib_config(sdev, stream, HDA_DSP_SPIB_DISABLE, 0);/* update no_stream_position flag for ipc params */ if (hda && hda->no_ipc_position) {
@@ -140,6 +147,29 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev, return 0; }
+/* update SPIB register with appl position */ +int hda_dsp_pcm_ack(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream) +{
- struct hdac_stream *hstream = substream->runtime->private_data;
- struct hdac_ext_stream *stream = stream_to_hdac_ext_stream(hstream);
- struct snd_pcm_runtime *runtime = substream->runtime;
- ssize_t appl_pos, buf_size;
- u32 spib;
- appl_pos = frames_to_bytes(runtime, runtime->control->appl_ptr);
- buf_size = frames_to_bytes(runtime, runtime->buffer_size);
- spib = appl_pos % buf_size;
- /* Allowable value for SPIB is 1 byte to max buffer size */
- if (!spib)
spib = buf_size;- sof_io_write(sdev, stream->spib_addr, spib);
- return 0;
+}
int hda_dsp_pcm_trigger(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream, int cmd) { @@ -234,6 +264,13 @@ int hda_dsp_pcm_open(struct snd_sof_dev *sdev, return -EINVAL; }
- /*
* if we want the .ack to work, we need to prevent the status and* control from being mapped*/- if (hda_disable_rewinds)
runtime->hw.info |= SNDRV_PCM_INFO_NO_REWINDS | SNDRV_PCM_INFO_EXPLICIT_SYNC;- /*
- All playback streams are DMI L1 capable, capture streams need
- pause push/release to be disabled
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 1d845c2cbc33..b6f037815344 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -655,6 +655,8 @@ int hda_dsp_stream_hw_free(struct snd_sof_dev *sdev, SOF_HDA_REG_PP_PPCTL, mask, 0); spin_unlock_irq(&bus->reg_lock);
hda_dsp_stream_spib_config(sdev, link_dev, HDA_DSP_SPIB_DISABLE, 0);
stream->substream = NULL;
return 0;
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 1195018a1f4f..6829d36fbfe9 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -533,6 +533,7 @@ int hda_dsp_pcm_trigger(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream, int cmd); snd_pcm_uframes_t hda_dsp_pcm_pointer(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream); +int hda_dsp_pcm_ack(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream);
/*
- DSP Stream Operations.
diff --git a/sound/soc/sof/intel/tgl.c b/sound/soc/sof/intel/tgl.c index 48da8e7a67bc..2a29058e0c20 100644 --- a/sound/soc/sof/intel/tgl.c +++ b/sound/soc/sof/intel/tgl.c @@ -73,6 +73,7 @@ const struct snd_sof_dsp_ops sof_tgl_ops = { .pcm_hw_free = hda_dsp_stream_hw_free, .pcm_trigger = hda_dsp_pcm_trigger, .pcm_pointer = hda_dsp_pcm_pointer,
- .pcm_ack = hda_dsp_pcm_ack,
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES) /* probe callbacks */ -- 2.25.1