From 2cc128387bec1c063156252229543cd6535ecdd7 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 22 Oct 2021 13:40:50 -0500 Subject: [PATCH 1/2] Revert "ASoC: SOF: Intel: add .ack support for HDaudio platforms" This reverts commit 5c1ba8cac306ff51514a258b9c407da7da526684. Mechanical revert before new submission and better commit message Signed-off-by: Pierre-Louis Bossart --- 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, 2 insertions(+), 45 deletions(-) diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c index 1b87c9016081aa..4f85a96bade498 100644 --- a/sound/soc/sof/intel/apl.c +++ b/sound/soc/sof/intel/apl.c @@ -78,7 +78,6 @@ 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 643506fa7c572a..6b8aa2570b947b 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -283,7 +283,6 @@ 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 b3f78639c30238..efc2a283616fb0 100644 --- a/sound/soc/sof/intel/hda-pcm.c +++ b/sound/soc/sof/intel/hda-pcm.c @@ -32,10 +32,6 @@ 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) { @@ -124,11 +120,8 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev, return ret; } - /* 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); + /* disable SPIB, to enable buffer wrap for stream */ + 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) { @@ -147,29 +140,6 @@ 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) { @@ -299,13 +269,6 @@ 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 4502a6133735ca..10d966024be583 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -662,8 +662,6 @@ 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 b60c77cd0d8f62..0063c81642db88 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -535,7 +535,6 @@ 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 7f7929c5cb88fd..237e92e790b724 100644 --- a/sound/soc/sof/intel/tgl.c +++ b/sound/soc/sof/intel/tgl.c @@ -113,7 +113,6 @@ 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 */ From d0351fbd7b6933acb616a4c070e7586eb970cda9 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Fri, 2 Apr 2021 16:40:03 -0500 Subject: [PATCH 2/2] ASoC: SOF: Intel: add .ack support for HDaudio platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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_SYNC_APPLPTR flag. 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 Reviewed-by: Kai Vehmanen Reviewed-by: Ranjani Sridharan Co-developed-by: Pierre-Louis Bossart Signed-off-by: Pierre-Louis Bossart Signed-off-by: Ranjani Sridharan --- 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 4f85a96bade498..1b87c9016081aa 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 6b8aa2570b947b..643506fa7c572a 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 efc2a283616fb0..431ad0f7d2daad 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) { @@ -269,6 +299,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 control from being mapped. + * The status can still be mapped. + */ + if (hda_disable_rewinds) + runtime->hw.info |= SNDRV_PCM_INFO_NO_REWINDS | SNDRV_PCM_INFO_SYNC_APPLPTR; + /* * 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 10d966024be583..4502a6133735ca 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -662,6 +662,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 0063c81642db88..b60c77cd0d8f62 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -535,6 +535,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 237e92e790b724..7f7929c5cb88fd 100644 --- a/sound/soc/sof/intel/tgl.c +++ b/sound/soc/sof/intel/tgl.c @@ -113,6 +113,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 */