[alsa-devel] [PATCH v2 00/11] ASoC: Intel: Skylake: More driver updates
From: Jeeja KP jeeja.kp@intel.com
This patch series provides updates on the following: o Fix LLCH register read. o Add 16-bit constraint to FE bxt_rt298 machine. o Fix DMA position reporting for capture stream. o Use the sig_bits to define dai bps caps. o Update sig_bits based on converter capability. o Rearrangement of code to cleanup SKL SST library. o Add support for deferred DSP module bind.
Changes in v2: - Addressed Pierre's comments for "Fix DMA position reporting for capture stream" patch.
B, Jayachandran (1): ALSA: hda: Fix LLCH register read
G Kranthi (1): ASoC: Intel: Skylake: Add 16-bit constraint to FE bxt_rt298 machine
Hardik T Shah (1): ASoC: Intel: Skylake: Fix DMA position reporting for capture stream
Jeeja KP (5): ASoC: Intel: Skylake: Use the sig_bits to define dai bps capability ASoC: hdac_hdmi: Update sig_bits based on converter capability ASoC: Intel: Skylake: Rearrangement of code to cleanup SKL SST library ASoC: Intel: Skylake: Fix module state after unbind and delete ASoC: Intel: Skylake: Add support for deferred DSP module bind
Vinod Koul (3): ASoC: Intel: Skylake: Don't unload module when in use ASoC: Intel: Skylake: Remove redundant vmixer handler ASoC: Intel: Skylake: remove hard coded ACPI path
sound/hda/hdac_controller.c | 2 +- sound/soc/codecs/hdac_hdmi.c | 4 +- sound/soc/intel/boards/bxt_rt298.c | 3 + sound/soc/intel/skylake/skl-messages.c | 2 +- sound/soc/intel/skylake/skl-nhlt.c | 7 +- sound/soc/intel/skylake/skl-pcm.c | 95 ++++++++++++++++--- sound/soc/intel/skylake/skl-sst-dsp.h | 24 +++-- sound/soc/intel/skylake/skl-sst-ipc.h | 8 ++ sound/soc/intel/skylake/skl-sst-utils.c | 60 ++---------- sound/soc/intel/skylake/skl-sst.c | 5 + sound/soc/intel/skylake/skl-topology.c | 159 +++++++++++++++++++++++++------- sound/soc/intel/skylake/skl-topology.h | 17 ++-- sound/soc/intel/skylake/skl.h | 1 + 13 files changed, 263 insertions(+), 124 deletions(-)
From: "B, Jayachandran" jayachandran.b@intel.com
LLCH is a 16 bit register. Use readw instead of readl API.
Signed-off-by: B, Jayachandran jayachandran.b@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/hda/hdac_controller.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c index 0430658..6f1e99c 100644 --- a/sound/hda/hdac_controller.c +++ b/sound/hda/hdac_controller.c @@ -268,7 +268,7 @@ int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus) unsigned int offset; unsigned int counter = 0;
- offset = snd_hdac_chip_readl(bus, LLCH); + offset = snd_hdac_chip_readw(bus, LLCH);
/* Lets walk the linked capabilities list */ do {
On Fri, 24 Mar 2017 18:40:24 +0100, jeeja.kp@intel.com wrote:
From: "B, Jayachandran" jayachandran.b@intel.com
LLCH is a 16 bit register. Use readw instead of readl API.
Signed-off-by: B, Jayachandran jayachandran.b@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com
Acked-by: Takashi Iwai tiwai@suse.de
Takashi
sound/hda/hdac_controller.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c index 0430658..6f1e99c 100644 --- a/sound/hda/hdac_controller.c +++ b/sound/hda/hdac_controller.c @@ -268,7 +268,7 @@ int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus) unsigned int offset; unsigned int counter = 0;
- offset = snd_hdac_chip_readl(bus, LLCH);
offset = snd_hdac_chip_readw(bus, LLCH);
/* Lets walk the linked capabilities list */ do {
-- 2.5.0
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
From: Jeeja KP jeeja.kp@intel.com
For calculating the HDA DMA format, use the max_bps supported by the DAI caps instead of fixing it to 32/24. For host DMA the Max bps support is 32, but in case of link DMA, this depends on the codec capability. So use the sig_bits to define the bps supported by dai.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/skl-pcm.c | 25 ++++++++++++++++++++++--- sound/soc/intel/skylake/skl-topology.c | 2 ++ sound/soc/intel/skylake/skl-topology.h | 2 ++ 3 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index 2f90bc4..3c61dba 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -155,7 +155,7 @@ int skl_pcm_host_dma_prepare(struct device *dev, struct skl_pipe_params *params) snd_hdac_ext_stream_decouple(ebus, stream, true);
format_val = snd_hdac_calc_stream_format(params->s_freq, - params->ch, params->format, 32, 0); + params->ch, params->format, params->host_bps, 0);
dev_dbg(dev, "format_val=%d, rate=%d, ch=%d, format=%d\n", format_val, params->s_freq, params->ch, params->format); @@ -190,8 +190,8 @@ int skl_pcm_link_dma_prepare(struct device *dev, struct skl_pipe_params *params)
stream = stream_to_hdac_ext_stream(hstream); snd_hdac_ext_stream_decouple(ebus, stream, true); - format_val = snd_hdac_calc_stream_format(params->s_freq, - params->ch, params->format, 24, 0); + format_val = snd_hdac_calc_stream_format(params->s_freq, params->ch, + params->format, params->link_bps, 0);
dev_dbg(dev, "format_val=%d, rate=%d, ch=%d, format=%d\n", format_val, params->s_freq, params->ch, params->format); @@ -309,6 +309,11 @@ static int skl_pcm_hw_params(struct snd_pcm_substream *substream, p_params.host_dma_id = dma_id; p_params.stream = substream->stream; p_params.format = params_format(params); + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + p_params.host_bps = dai->driver->playback.sig_bits; + else + p_params.host_bps = dai->driver->capture.sig_bits; +
m_cfg = skl_tplg_fe_get_cpr_module(dai, p_params.stream); if (m_cfg) @@ -547,6 +552,11 @@ static int skl_link_hw_params(struct snd_pcm_substream *substream, p_params.link_index = link->index; p_params.format = params_format(params);
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + p_params.link_bps = codec_dai->driver->playback.sig_bits; + else + p_params.link_bps = codec_dai->driver->capture.sig_bits; + return skl_tplg_be_update_params(dai, &p_params); }
@@ -652,6 +662,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = { .rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_8000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE, + .sig_bits = 32, }, .capture = { .stream_name = "System Capture", @@ -659,6 +670,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = { .channels_max = HDA_STEREO, .rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, + .sig_bits = 32, }, }, { @@ -670,6 +682,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = { .channels_max = HDA_QUAD, .rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, + .sig_bits = 32, }, }, { @@ -681,6 +694,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = { .channels_max = HDA_STEREO, .rates = SNDRV_PCM_RATE_48000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, + .sig_bits = 32, }, }, { @@ -692,6 +706,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = { .channels_max = HDA_STEREO, .rates = SNDRV_PCM_RATE_48000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, + .sig_bits = 32, }, }, { @@ -703,6 +718,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = { .channels_max = HDA_QUAD, .rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, + .sig_bits = 32, }, }, { @@ -718,6 +734,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = { SNDRV_PCM_RATE_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE, + .sig_bits = 32, }, }, { @@ -733,6 +750,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = { SNDRV_PCM_RATE_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE, + .sig_bits = 32, }, }, { @@ -748,6 +766,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = { SNDRV_PCM_RATE_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE, + .sig_bits = 32, }, },
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index c6bd4bb..43f9cb3 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -1242,10 +1242,12 @@ static void skl_tplg_fill_dma_id(struct skl_module_cfg *mcfg, case SKL_DEVICE_HDALINK: pipe->p_params->link_dma_id = params->link_dma_id; pipe->p_params->link_index = params->link_index; + pipe->p_params->link_bps = params->link_bps; break;
case SKL_DEVICE_HDAHOST: pipe->p_params->host_dma_id = params->host_dma_id; + pipe->p_params->host_bps = params->host_bps; break;
default: diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h index fefab0e..bf2c63b 100644 --- a/sound/soc/intel/skylake/skl-topology.h +++ b/sound/soc/intel/skylake/skl-topology.h @@ -257,6 +257,8 @@ struct skl_pipe_params { snd_pcm_format_t format; int link_index; int stream; + unsigned int host_bps; + unsigned int link_bps; };
struct skl_pipe {
From: Jeeja KP jeeja.kp@intel.com
When creating the codec dai, use sig_bits to update the max bps based on the codec capability. So both the link DMA and codec format will be calculated based on DAI sig_bits.
So update the sig_bits with converter capability and use the sig_bits for HDA format calculation.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/codecs/hdac_hdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c index 78fca8a..5b76947 100644 --- a/sound/soc/codecs/hdac_hdmi.c +++ b/sound/soc/codecs/hdac_hdmi.c @@ -469,7 +469,7 @@ static int hdac_hdmi_set_hw_params(struct snd_pcm_substream *substream,
format = snd_hdac_calc_stream_format(params_rate(hparams), params_channels(hparams), params_format(hparams), - 24, 0); + dai->driver->playback.sig_bits, 0);
pcm = hdac_hdmi_get_pcm_from_cvt(hdmi, dai_map->cvt); if (!pcm) @@ -1419,8 +1419,8 @@ static int hdac_hdmi_create_dais(struct hdac_device *hdac, hdmi_dais[i].playback.rate_min = rate_min; hdmi_dais[i].playback.channels_min = 2; hdmi_dais[i].playback.channels_max = 2; + hdmi_dais[i].playback.sig_bits = bps; hdmi_dais[i].ops = &hdmi_dai_ops; - i++; }
From: G Kranthi gudishax.kranthikumar@intel.com
Add constraint to FE to restrict sample format to 16-bit for bxt_rt298 machine
Signed-off-by: G Kranthi gudishax.kranthikumar@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/boards/bxt_rt298.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/intel/boards/bxt_rt298.c b/sound/soc/intel/boards/bxt_rt298.c index 176c080..1a68d04 100644 --- a/sound/soc/intel/boards/bxt_rt298.c +++ b/sound/soc/intel/boards/bxt_rt298.c @@ -274,12 +274,15 @@ static int bxt_fe_startup(struct snd_pcm_substream *substream) * on this platform for PCM device we support: * 48Khz * stereo + * 16-bit audio */
runtime->hw.channels_max = 2; snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, &constraints_channels);
+ runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE; + snd_pcm_hw_constraint_msbits(runtime, 0, 16, 16); snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &constraints_rates);
From: Vinod Koul vinod.koul@intel.com
A module may have multiple instances in DSP, so unload only when usage count is zero.
Signed-off-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/skl-sst.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c index 39d4aaa..5395297 100644 --- a/sound/soc/intel/skylake/skl-sst.c +++ b/sound/soc/intel/skylake/skl-sst.c @@ -417,6 +417,11 @@ static int skl_unload_module(struct sst_dsp *ctx, u16 mod_id) dev_err(ctx->dev, "Module bad usage cnt!:%d\n", usage_cnt); return -EIO; } + + /* if module is used by others return, no need to unload */ + if (usage_cnt > 0) + return 0; + ret = skl_ipc_unload_modules(&skl->ipc, SKL_NUM_MODULES, &mod_id); if (ret < 0) {
From: Vinod Koul vinod.koul@intel.com
Initially vmixer and mixer widget handlers were bit different, but over time they became same so remove the duplicate code.
Signed-off-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/skl-topology.c | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index 43f9cb3..35d9d7b 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -1073,36 +1073,6 @@ static int skl_tplg_pga_dapm_post_pmd_event(struct snd_soc_dapm_widget *w, }
/* - * In modelling, we assume there will be ONLY one mixer in a pipeline. If - * mixer is not required then it is treated as static mixer aka vmixer with - * a hard path to source module - * So we don't need to check if source is started or not as hard path puts - * dependency on each other - */ -static int skl_tplg_vmixer_event(struct snd_soc_dapm_widget *w, - struct snd_kcontrol *k, int event) -{ - struct snd_soc_dapm_context *dapm = w->dapm; - struct skl *skl = get_skl_ctx(dapm->dev); - - switch (event) { - case SND_SOC_DAPM_PRE_PMU: - return skl_tplg_mixer_dapm_pre_pmu_event(w, skl); - - case SND_SOC_DAPM_POST_PMU: - return skl_tplg_mixer_dapm_post_pmu_event(w, skl); - - case SND_SOC_DAPM_PRE_PMD: - return skl_tplg_mixer_dapm_pre_pmd_event(w, skl); - - case SND_SOC_DAPM_POST_PMD: - return skl_tplg_mixer_dapm_post_pmd_event(w, skl); - } - - return 0; -} - -/* * In modelling, we assume there will be ONLY one mixer in a pipeline. If a * second one is required that is created as another pipe entity. * The mixer is responsible for pipe management and represent a pipeline @@ -1570,7 +1540,7 @@ int skl_tplg_be_update_params(struct snd_soc_dai *dai,
static const struct snd_soc_tplg_widget_events skl_tplg_widget_ops[] = { {SKL_MIXER_EVENT, skl_tplg_mixer_event}, - {SKL_VMIXER_EVENT, skl_tplg_vmixer_event}, + {SKL_VMIXER_EVENT, skl_tplg_mixer_event}, {SKL_PGA_EVENT, skl_tplg_pga_event}, };
From: Vinod Koul vinod.koul@intel.com
We should not hard code the ACPI path to get acpi_handle. Instead use ACPI_HANDLE macro to do the job.
Signed-off-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/skl-nhlt.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c index 7eb9c41..e3f0667 100644 --- a/sound/soc/intel/skylake/skl-nhlt.c +++ b/sound/soc/intel/skylake/skl-nhlt.c @@ -24,8 +24,6 @@ static u8 OSC_UUID[16] = {0x6E, 0x88, 0x9F, 0xA6, 0xEB, 0x6C, 0x94, 0x45, 0xA4, 0x1F, 0x7B, 0x5D, 0xCE, 0x24, 0xC5, 0x53};
-#define DSDT_NHLT_PATH "\_SB.PCI0.HDAS" - struct nhlt_acpi_table *skl_nhlt_init(struct device *dev) { acpi_handle handle; @@ -33,8 +31,9 @@ struct nhlt_acpi_table *skl_nhlt_init(struct device *dev) struct nhlt_resource_desc *nhlt_ptr = NULL; struct nhlt_acpi_table *nhlt_table = NULL;
- if (ACPI_FAILURE(acpi_get_handle(NULL, DSDT_NHLT_PATH, &handle))) { - dev_err(dev, "Requested NHLT device not found\n"); + handle = ACPI_HANDLE(dev); + if (!handle) { + dev_err(dev, "Didn't find ACPI_HANDLE\n"); return NULL; }
From: Jeeja KP jeeja.kp@intel.com
Skylake driver topology header/driver structure is referenced and used in SST library which creates circular dependency. Hence the rearrangement.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/skl-pcm.c | 35 +++++++++++++++---- sound/soc/intel/skylake/skl-sst-dsp.h | 24 +++++++++---- sound/soc/intel/skylake/skl-sst-ipc.h | 8 +++++ sound/soc/intel/skylake/skl-sst-utils.c | 60 +++++---------------------------- sound/soc/intel/skylake/skl-topology.c | 11 ++++-- sound/soc/intel/skylake/skl-topology.h | 13 ------- 6 files changed, 70 insertions(+), 81 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index 3c61dba..ef440d8 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -1174,29 +1174,52 @@ static int skl_pcm_new(struct snd_soc_pcm_runtime *rtd) return retval; }
+static int skl_get_module_info(struct skl *skl, struct skl_module_cfg *mconfig) +{ + struct skl_sst *ctx = skl->skl_sst; + struct uuid_module *module; + uuid_le *uuid_mod; + + uuid_mod = (uuid_le *)mconfig->guid; + + if (list_empty(&ctx->uuid_list)) { + dev_err(ctx->dev, "Module list is empty\n"); + return -EIO; + } + + list_for_each_entry(module, &ctx->uuid_list, list) { + if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) { + mconfig->id.module_id = module->id; + mconfig->is_loadable = module->is_loadable; + return 0; + } + } + + return -EIO; +} + static int skl_populate_modules(struct skl *skl) { struct skl_pipeline *p; struct skl_pipe_module *m; struct snd_soc_dapm_widget *w; struct skl_module_cfg *mconfig; - int ret; + int ret = 0;
list_for_each_entry(p, &skl->ppl_list, node) { list_for_each_entry(m, &p->pipe->w_list, node) { - w = m->w; mconfig = w->priv;
- ret = snd_skl_get_module_info(skl->skl_sst, mconfig); + ret = skl_get_module_info(skl, mconfig); if (ret < 0) { dev_err(skl->skl_sst->dev, - "query module info failed:%d\n", ret); - goto err; + "query module info failed\n"); + return ret; } } } -err: + return ret; }
diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h b/sound/soc/intel/skylake/skl-sst-dsp.h index 5d7a93a..7229a12 100644 --- a/sound/soc/intel/skylake/skl-sst-dsp.h +++ b/sound/soc/intel/skylake/skl-sst-dsp.h @@ -17,13 +17,14 @@ #define __SKL_SST_DSP_H__
#include <linux/interrupt.h> +#include <linux/uuid.h> #include <sound/memalloc.h> #include "skl-sst-cldma.h" -#include "skl-topology.h"
struct sst_dsp; struct skl_sst; struct sst_dsp_device; +struct skl_lib_info;
/* Intel HD Audio General DSP Registers */ #define SKL_ADSP_GEN_BASE 0x0 @@ -172,6 +173,19 @@ struct skl_dsp_loader_ops { int stream_tag); };
+#define MAX_INSTANCE_BUFF 2 + +struct uuid_module { + uuid_le uuid; + int id; + int is_loadable; + int max_instance; + u64 pvt_id[MAX_INSTANCE_BUFF]; + int *instance_id; + + struct list_head list; +}; + struct skl_load_module_info { u16 mod_id; const struct firmware *fw; @@ -223,14 +237,10 @@ int bxt_sst_init_fw(struct device *dev, struct skl_sst *ctx); void skl_sst_dsp_cleanup(struct device *dev, struct skl_sst *ctx); void bxt_sst_dsp_cleanup(struct device *dev, struct skl_sst *ctx);
-int snd_skl_get_module_info(struct skl_sst *ctx, - struct skl_module_cfg *mconfig); int snd_skl_parse_uuids(struct sst_dsp *ctx, const struct firmware *fw, unsigned int offset, int index); -int skl_get_pvt_id(struct skl_sst *ctx, - struct skl_module_cfg *mconfig); -int skl_put_pvt_id(struct skl_sst *ctx, - struct skl_module_cfg *mconfig); +int skl_get_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int instance_id); +int skl_put_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int *pvt_id); int skl_get_pvt_instance_id_map(struct skl_sst *ctx, int module_id, int instance_id); void skl_freeup_uuid_list(struct skl_sst *ctx); diff --git a/sound/soc/intel/skylake/skl-sst-ipc.h b/sound/soc/intel/skylake/skl-sst-ipc.h index fc07c39..4abf98c 100644 --- a/sound/soc/intel/skylake/skl-sst-ipc.h +++ b/sound/soc/intel/skylake/skl-sst-ipc.h @@ -69,6 +69,14 @@ struct skl_d0i3_data { struct delayed_work work; };
+#define SKL_LIB_NAME_LENGTH 128 +#define SKL_MAX_LIB 16 + +struct skl_lib_info { + char name[SKL_LIB_NAME_LENGTH]; + const struct firmware *fw; +}; + struct skl_sst { struct device *dev; struct sst_dsp *dsp; diff --git a/sound/soc/intel/skylake/skl-sst-utils.c b/sound/soc/intel/skylake/skl-sst-utils.c index ea162fb..6d5bff0 100644 --- a/sound/soc/intel/skylake/skl-sst-utils.c +++ b/sound/soc/intel/skylake/skl-sst-utils.c @@ -94,19 +94,6 @@ struct adsp_fw_hdr { u32 load_offset; } __packed;
-#define MAX_INSTANCE_BUFF 2 - -struct uuid_module { - uuid_le uuid; - int id; - int is_loadable; - int max_instance; - u64 pvt_id[MAX_INSTANCE_BUFF]; - int *instance_id; - - struct list_head list; -}; - struct skl_ext_manifest_hdr { u32 id; u32 len; @@ -115,32 +102,6 @@ struct skl_ext_manifest_hdr { u32 entries; };
-int snd_skl_get_module_info(struct skl_sst *ctx, - struct skl_module_cfg *mconfig) -{ - struct uuid_module *module; - uuid_le *uuid_mod; - - uuid_mod = (uuid_le *)mconfig->guid; - - if (list_empty(&ctx->uuid_list)) { - dev_err(ctx->dev, "Module list is empty\n"); - return -EINVAL; - } - - list_for_each_entry(module, &ctx->uuid_list, list) { - if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) { - mconfig->id.module_id = module->id; - mconfig->is_loadable = module->is_loadable; - - return 0; - } - } - - return -EINVAL; -} -EXPORT_SYMBOL_GPL(snd_skl_get_module_info); - static int skl_get_pvtid_map(struct uuid_module *module, int instance_id) { int pvt_id; @@ -222,21 +183,18 @@ static inline int skl_pvtid_128(struct uuid_module *module) * This generates a 128 bit private unique id for a module TYPE so that * module instance is unique */ -int skl_get_pvt_id(struct skl_sst *ctx, struct skl_module_cfg *mconfig) +int skl_get_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int instance_id) { struct uuid_module *module; - uuid_le *uuid_mod; int pvt_id;
- uuid_mod = (uuid_le *)mconfig->guid; - list_for_each_entry(module, &ctx->uuid_list, list) { if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
pvt_id = skl_pvtid_128(module); if (pvt_id >= 0) { - module->instance_id[pvt_id] = - mconfig->id.instance_id; + module->instance_id[pvt_id] = instance_id; + return pvt_id; } } @@ -254,23 +212,21 @@ EXPORT_SYMBOL_GPL(skl_get_pvt_id); * * This frees a 128 bit private unique id previously generated */ -int skl_put_pvt_id(struct skl_sst *ctx, struct skl_module_cfg *mconfig) +int skl_put_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int *pvt_id) { int i; - uuid_le *uuid_mod; struct uuid_module *module;
- uuid_mod = (uuid_le *)mconfig->guid; list_for_each_entry(module, &ctx->uuid_list, list) { if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
- if (mconfig->id.pvt_id != 0) - i = (mconfig->id.pvt_id) / 64; + if (*pvt_id != 0) + i = (*pvt_id) / 64; else i = 0;
- module->pvt_id[i] &= ~(1 << (mconfig->id.pvt_id)); - mconfig->id.pvt_id = -1; + module->pvt_id[i] &= ~(1 << (*pvt_id)); + *pvt_id = -1; return 0; } } diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index 35d9d7b..8bd5ded 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -539,6 +539,7 @@ skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe) int ret = 0;
list_for_each_entry(w_module, &pipe->w_list, node) { + uuid_le *uuid_mod; w = w_module->w; mconfig = w->priv;
@@ -576,13 +577,15 @@ skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe) * FE/BE params */ skl_tplg_update_module_params(w, ctx); - mconfig->id.pvt_id = skl_get_pvt_id(ctx, mconfig); + uuid_mod = (uuid_le *)mconfig->guid; + mconfig->id.pvt_id = skl_get_pvt_id(ctx, uuid_mod, + mconfig->id.instance_id); if (mconfig->id.pvt_id < 0) return ret; skl_tplg_set_module_init_data(w); ret = skl_init_module(ctx, mconfig); if (ret < 0) { - skl_put_pvt_id(ctx, mconfig); + skl_put_pvt_id(ctx, uuid_mod, &mconfig->id.pvt_id); return ret; } skl_tplg_alloc_pipe_mcps(skl, mconfig); @@ -602,7 +605,9 @@ static int skl_tplg_unload_pipe_modules(struct skl_sst *ctx, struct skl_module_cfg *mconfig = NULL;
list_for_each_entry(w_module, &pipe->w_list, node) { + uuid_le *uuid_mod; mconfig = w_module->w->priv; + uuid_mod = (uuid_le *)mconfig->guid;
if (mconfig->is_loadable && ctx->dsp->fw_ops.unload_mod && mconfig->m_state > SKL_MODULE_UNINIT) { @@ -611,7 +616,7 @@ static int skl_tplg_unload_pipe_modules(struct skl_sst *ctx, if (ret < 0) return -EIO; } - skl_put_pvt_id(ctx, mconfig); + skl_put_pvt_id(ctx, uuid_mod, &mconfig->id.pvt_id); }
/* no modules to unload in this path, so return */ diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h index bf2c63b..8536d03 100644 --- a/sound/soc/intel/skylake/skl-topology.h +++ b/sound/soc/intel/skylake/skl-topology.h @@ -336,19 +336,6 @@ struct skl_pipeline { struct list_head node; };
-#define SKL_LIB_NAME_LENGTH 128 -#define SKL_MAX_LIB 16 - -struct skl_lib_info { - char name[SKL_LIB_NAME_LENGTH]; - const struct firmware *fw; -}; - -struct skl_manifest { - u32 lib_count; - struct skl_lib_info lib[SKL_MAX_LIB]; -}; - static inline struct skl *get_skl_ctx(struct device *dev) { struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
From: Hardik T Shah hardik.t.shah@intel.com
As per hardware recommendation, for every capture stream completion following operations need to be done in order to reflect the actual data that is received in position buffer.
1. Wait for 20us before reading the DMA position in buffer once the interrupt is generated for stream completion. 2. Read any of the register to flush the DMA position value. This is dummy read operation.
Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Hardik T Shah hardik.t.shah@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index ef440d8..1823197 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -21,6 +21,7 @@
#include <linux/pci.h> #include <linux/pm_runtime.h> +#include <linux/delay.h> #include <sound/pcm_params.h> #include <sound/soc.h> #include "skl.h" @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t skl_platform_pcm_pointer * HAD space reflects the actual data that is transferred. * Use the position buffer for capture, as DPIB write gets * completed earlier than the actual data written to the DDR. + * + * For capture stream following workaround is required to fix the + * incorrect position reporting. + * + * 1. Wait for 20us before reading the DMA position in buffer once + * the interrupt is generated for stream completion as update happens + * on the HDA frame boundary i.e. 20.833uSec. + * 2. Read DPIB register to flush the DMA position value. This dummy + * read is required to flush DMA position value. + * 3. Read the DMA Position-in-Buffer. This value now will be equal to + * or greater than period boundary. */ - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { pos = readl(ebus->bus.remap_addr + AZX_REG_VS_SDXDPIB_XBASE + (AZX_REG_VS_SDXDPIB_XINTERVAL * hdac_stream(hstream)->index)); - else + } else { + udelay(20); + readl(ebus->bus.remap_addr + + AZX_REG_VS_SDXDPIB_XBASE + + (AZX_REG_VS_SDXDPIB_XINTERVAL * + hdac_stream(hstream)->index)); pos = snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream)); + }
if (pos >= hdac_stream(hstream)->bufsize) pos = 0;
On Fri, 24 Mar 2017 18:40:32 +0100, jeeja.kp@intel.com wrote:
From: Hardik T Shah hardik.t.shah@intel.com
As per hardware recommendation, for every capture stream completion following operations need to be done in order to reflect the actual data that is received in position buffer.
- Wait for 20us before reading the DMA position in buffer once the
interrupt is generated for stream completion. 2. Read any of the register to flush the DMA position value. This is dummy read operation.
Are these workarounds needed for the legacy driver? If yes, which chips require it?
thanks,
Takashi
Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Hardik T Shah hardik.t.shah@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com
sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index ef440d8..1823197 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -21,6 +21,7 @@
#include <linux/pci.h> #include <linux/pm_runtime.h> +#include <linux/delay.h> #include <sound/pcm_params.h> #include <sound/soc.h> #include "skl.h" @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t skl_platform_pcm_pointer * HAD space reflects the actual data that is transferred. * Use the position buffer for capture, as DPIB write gets * completed earlier than the actual data written to the DDR.
** For capture stream following workaround is required to fix the* incorrect position reporting.** 1. Wait for 20us before reading the DMA position in buffer once* the interrupt is generated for stream completion as update happens* on the HDA frame boundary i.e. 20.833uSec.* 2. Read DPIB register to flush the DMA position value. This dummy* read is required to flush DMA position value.* 3. Read the DMA Position-in-Buffer. This value now will be equal to* or greater than period boundary.
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { pos = readl(ebus->bus.remap_addr + AZX_REG_VS_SDXDPIB_XBASE + (AZX_REG_VS_SDXDPIB_XINTERVAL * hdac_stream(hstream)->index));
- else
} else {
udelay(20);readl(ebus->bus.remap_addr +AZX_REG_VS_SDXDPIB_XBASE +(AZX_REG_VS_SDXDPIB_XINTERVAL *hdac_stream(hstream)->index));pos = snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
}
if (pos >= hdac_stream(hstream)->bufsize) pos = 0;
-- 2.5.0
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa- project.org] On Behalf Of Takashi Iwai Sent: Friday, March 24, 2017 11:38 AM To: Kp, Jeeja jeeja.kp@intel.com Cc: alsa-devel@alsa-project.org; R, Dharageswari dharageswari.r@intel.com; Patches Audio patches.audio@intel.com; Shah, Hardik T hardik.t.shah@intel.com; broonie@kernel.org; Girdwood, Liam R liam.r.girdwood@intel.com Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream
On Fri, 24 Mar 2017 18:40:32 +0100, jeeja.kp@intel.com wrote:
From: Hardik T Shah hardik.t.shah@intel.com
As per hardware recommendation, for every capture stream completion following operations need to be done in order to reflect the actual data that is received in position buffer.
- Wait for 20us before reading the DMA position in buffer once the
interrupt is generated for stream completion. 2. Read any of the register to flush the DMA position value. This is dummy read operation.
Are these workarounds needed for the legacy driver? If yes, which chips require it?
Yes, these are needed in legacy driver as well.
From SKL and BXT onwards, it is needed.
thanks,
Takashi
Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Hardik T Shah hardik.t.shah@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com
sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c
b/sound/soc/intel/skylake/skl-pcm.c
index ef440d8..1823197 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -21,6 +21,7 @@
#include <linux/pci.h> #include <linux/pm_runtime.h> +#include <linux/delay.h> #include <sound/pcm_params.h> #include <sound/soc.h> #include "skl.h" @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t
skl_platform_pcm_pointer
* HAD space reflects the actual data that is transferred. * Use the position buffer for capture, as DPIB write gets * completed earlier than the actual data written to the DDR.
** For capture stream following workaround is required to fix the* incorrect position reporting.** 1. Wait for 20us before reading the DMA position in buffer once* the interrupt is generated for stream completion as update happens* on the HDA frame boundary i.e. 20.833uSec.* 2. Read DPIB register to flush the DMA position value. This dummy* read is required to flush DMA position value.* 3. Read the DMA Position-in-Buffer. This value now will be equal to* or greater than period boundary.
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { pos = readl(ebus->bus.remap_addr +
AZX_REG_VS_SDXDPIB_XBASE +
(AZX_REG_VS_SDXDPIB_XINTERVAL * hdac_stream(hstream)->index));
- else
- } else {
udelay(20);readl(ebus->bus.remap_addr +AZX_REG_VS_SDXDPIB_XBASE +(AZX_REG_VS_SDXDPIB_XINTERVAL *hdac_stream(hstream)->index));snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
}
if (pos >= hdac_stream(hstream)->bufsize) pos = 0;
-- 2.5.0
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Fri, 24 Mar 2017 19:43:52 +0100, Ughreja, Rakesh A wrote:
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa- project.org] On Behalf Of Takashi Iwai Sent: Friday, March 24, 2017 11:38 AM To: Kp, Jeeja jeeja.kp@intel.com Cc: alsa-devel@alsa-project.org; R, Dharageswari dharageswari.r@intel.com; Patches Audio patches.audio@intel.com; Shah, Hardik T hardik.t.shah@intel.com; broonie@kernel.org; Girdwood, Liam R liam.r.girdwood@intel.com Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream
On Fri, 24 Mar 2017 18:40:32 +0100, jeeja.kp@intel.com wrote:
From: Hardik T Shah hardik.t.shah@intel.com
As per hardware recommendation, for every capture stream completion following operations need to be done in order to reflect the actual data that is received in position buffer.
- Wait for 20us before reading the DMA position in buffer once the
interrupt is generated for stream completion. 2. Read any of the register to flush the DMA position value. This is dummy read operation.
Are these workarounds needed for the legacy driver? If yes, which chips require it?
Yes, these are needed in legacy driver as well. From SKL and BXT onwards, it is needed.
OK, thanks for confirmation.
Now, from what I read in the above, is the workaround required *only* after the interrupt is generated? 20us delay isn't so cheap, and we tend to inquire PCM positions often. If the workaround is needed only after the PCM period elapse, we can set some flag in the irq handler and apply the workaround only when necessary.
Takashi
thanks,
Takashi
Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Hardik T Shah hardik.t.shah@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com
sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c
b/sound/soc/intel/skylake/skl-pcm.c
index ef440d8..1823197 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -21,6 +21,7 @@
#include <linux/pci.h> #include <linux/pm_runtime.h> +#include <linux/delay.h> #include <sound/pcm_params.h> #include <sound/soc.h> #include "skl.h" @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t
skl_platform_pcm_pointer
* HAD space reflects the actual data that is transferred. * Use the position buffer for capture, as DPIB write gets * completed earlier than the actual data written to the DDR.
** For capture stream following workaround is required to fix the* incorrect position reporting.** 1. Wait for 20us before reading the DMA position in buffer once* the interrupt is generated for stream completion as update happens* on the HDA frame boundary i.e. 20.833uSec.* 2. Read DPIB register to flush the DMA position value. This dummy* read is required to flush DMA position value.* 3. Read the DMA Position-in-Buffer. This value now will be equal to* or greater than period boundary.
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { pos = readl(ebus->bus.remap_addr +
AZX_REG_VS_SDXDPIB_XBASE +
(AZX_REG_VS_SDXDPIB_XINTERVAL * hdac_stream(hstream)->index));
- else
- } else {
udelay(20);readl(ebus->bus.remap_addr +AZX_REG_VS_SDXDPIB_XBASE +(AZX_REG_VS_SDXDPIB_XINTERVAL *hdac_stream(hstream)->index));snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
}
if (pos >= hdac_stream(hstream)->bufsize) pos = 0;
-- 2.5.0
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa- project.org] On Behalf Of Takashi Iwai Sent: Friday, March 24, 2017 11:50 AM To: Ughreja, Rakesh A rakesh.a.ughreja@intel.com Cc: alsa-devel@alsa-project.org; R, Dharageswari dharageswari.r@intel.com; Patches Audio patches.audio@intel.com; Shah, Hardik T hardik.t.shah@intel.com; broonie@kernel.org; Girdwood, Liam R liam.r.girdwood@intel.com; Kp, Jeeja jeeja.kp@intel.com Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream
On Fri, 24 Mar 2017 19:43:52 +0100, Ughreja, Rakesh A wrote:
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-
bounces@alsa-
project.org] On Behalf Of Takashi Iwai Sent: Friday, March 24, 2017 11:38 AM To: Kp, Jeeja jeeja.kp@intel.com Cc: alsa-devel@alsa-project.org; R, Dharageswari
Patches Audio patches.audio@intel.com; Shah, Hardik T hardik.t.shah@intel.com; broonie@kernel.org; Girdwood, Liam R liam.r.girdwood@intel.com Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream
On Fri, 24 Mar 2017 18:40:32 +0100, jeeja.kp@intel.com wrote:
From: Hardik T Shah hardik.t.shah@intel.com
As per hardware recommendation, for every capture stream completion following operations need to be done in order to reflect the actual data that is received in position buffer.
- Wait for 20us before reading the DMA position in buffer once the
interrupt is generated for stream completion. 2. Read any of the register to flush the DMA position value. This is dummy read operation.
Are these workarounds needed for the legacy driver? If yes, which chips require it?
Yes, these are needed in legacy driver as well. From SKL and BXT onwards, it is needed.
OK, thanks for confirmation.
Now, from what I read in the above, is the workaround required *only* after the interrupt is generated? 20us delay isn't so cheap, and we tend to inquire PCM positions often. If the workaround is needed only after the PCM period elapse, we can set some flag in the irq handler and apply the workaround only when necessary.
Yes, Takashi the workaround is required only in the period elapsed interrupt. In some cases the DMA Position updates are delayed and so when the period elapsed interrupt occurs the wait_for_avail thinks that one period worth of data is not available and so returns only on the next period elapsed interrupt. This creates problem for 2 periods playback/capture streams.
So even in the period elapsed interrupt the wait is required only if the position is less than the period boundary.
Takashi
thanks,
Takashi
Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Hardik T Shah hardik.t.shah@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com
sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c
b/sound/soc/intel/skylake/skl-pcm.c
index ef440d8..1823197 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -21,6 +21,7 @@
#include <linux/pci.h> #include <linux/pm_runtime.h> +#include <linux/delay.h> #include <sound/pcm_params.h> #include <sound/soc.h> #include "skl.h" @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t
skl_platform_pcm_pointer
* HAD space reflects the actual data that is transferred. * Use the position buffer for capture, as DPIB write gets * completed earlier than the actual data written to the DDR.
** For capture stream following workaround is required to fix the* incorrect position reporting.** 1. Wait for 20us before reading the DMA position in buffer once* the interrupt is generated for stream completion as update happens* on the HDA frame boundary i.e. 20.833uSec.* 2. Read DPIB register to flush the DMA position value. This dummy* read is required to flush DMA position value.* 3. Read the DMA Position-in-Buffer. This value now will be equal to* or greater than period boundary.
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { pos = readl(ebus->bus.remap_addr +
AZX_REG_VS_SDXDPIB_XBASE +
(AZX_REG_VS_SDXDPIB_XINTERVAL * hdac_stream(hstream)->index));
- else
- } else {
udelay(20);readl(ebus->bus.remap_addr +AZX_REG_VS_SDXDPIB_XBASE +(AZX_REG_VS_SDXDPIB_XINTERVAL *hdac_stream(hstream)->index));snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
}
if (pos >= hdac_stream(hstream)->bufsize) pos = 0;
-- 2.5.0
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Sat, Mar 25, 2017 at 02:21:13AM +0530, Ughreja, Rakesh A wrote:
Are these workarounds needed for the legacy driver? If yes, which chips require it?
Yes, these are needed in legacy driver as well. From SKL and BXT onwards, it is needed.
OK, thanks for confirmation.
Now, from what I read in the above, is the workaround required *only* after the interrupt is generated? 20us delay isn't so cheap, and we tend to inquire PCM positions often. If the workaround is needed only after the PCM period elapse, we can set some flag in the irq handler and apply the workaround only when necessary.
Yes, Takashi the workaround is required only in the period elapsed interrupt. In some cases the DMA Position updates are delayed and so when the period elapsed interrupt occurs the wait_for_avail thinks that one period worth of data is not available and so returns only on the next period elapsed interrupt. This creates problem for 2 periods playback/capture streams.
So even in the period elapsed interrupt the wait is required only if the position is less than the period boundary.
Hi Takashi,
So we need this for 2 periods when the device is in irq mode. Not for other cases. ie SKL_PLUS..
But have you seen any user reports on this till now.
On Mon, 27 Mar 2017 12:29:54 +0200, Vinod Koul wrote:
On Sat, Mar 25, 2017 at 02:21:13AM +0530, Ughreja, Rakesh A wrote:
Are these workarounds needed for the legacy driver? If yes, which chips require it?
Yes, these are needed in legacy driver as well. From SKL and BXT onwards, it is needed.
OK, thanks for confirmation.
Now, from what I read in the above, is the workaround required *only* after the interrupt is generated? 20us delay isn't so cheap, and we tend to inquire PCM positions often. If the workaround is needed only after the PCM period elapse, we can set some flag in the irq handler and apply the workaround only when necessary.
Yes, Takashi the workaround is required only in the period elapsed interrupt. In some cases the DMA Position updates are delayed and so when the period elapsed interrupt occurs the wait_for_avail thinks that one period worth of data is not available and so returns only on the next period elapsed interrupt. This creates problem for 2 periods playback/capture streams.
So even in the period elapsed interrupt the wait is required only if the position is less than the period boundary.
Hi Takashi,
So we need this for 2 periods when the device is in irq mode. Not for other cases. ie SKL_PLUS..
Yeah, thanks. I'd cook a couple of patches to do that for the legacy driver. But I still wonder whether the wait is always needed.
Judging from the description, does the discrepancy of posbuf read happen *only* when the DMA position goes across the BD boundary? Or does it happen at any time?
When you trace, you can see that the apps frequently inquires the position. So, an unconditional wait should be really avoided.
But have you seen any user reports on this till now.
I've seen some bug reports mentioning about crackling audio capture on SKL (I forgot URLs). It might be triggered by that.
thanks,
Takashi
On Mon, Mar 27, 2017 at 03:12:02PM +0200, Takashi Iwai wrote:
On Mon, 27 Mar 2017 12:29:54 +0200, Vinod Koul wrote:
On Sat, Mar 25, 2017 at 02:21:13AM +0530, Ughreja, Rakesh A wrote:
Are these workarounds needed for the legacy driver? If yes, which chips require it?
Yes, these are needed in legacy driver as well. From SKL and BXT onwards, it is needed.
OK, thanks for confirmation.
Now, from what I read in the above, is the workaround required *only* after the interrupt is generated? 20us delay isn't so cheap, and we tend to inquire PCM positions often. If the workaround is needed only after the PCM period elapse, we can set some flag in the irq handler and apply the workaround only when necessary.
Yes, Takashi the workaround is required only in the period elapsed interrupt. In some cases the DMA Position updates are delayed and so when the period elapsed interrupt occurs the wait_for_avail thinks that one period worth of data is not available and so returns only on the next period elapsed interrupt. This creates problem for 2 periods playback/capture streams.
So even in the period elapsed interrupt the wait is required only if the position is less than the period boundary.
Hi Takashi,
So we need this for 2 periods when the device is in irq mode. Not for other cases. ie SKL_PLUS..
Yeah, thanks. I'd cook a couple of patches to do that for the legacy driver. But I still wonder whether the wait is always needed.
Judging from the description, does the discrepancy of posbuf read happen *only* when the DMA position goes across the BD boundary? Or does it happen at any time?
I think it can happen anytime, but the impact is not felt unless we have a 2 period case. The update is in-flight, so read returns a value lesser than period boundary. We will sleep till next period ie next wake, which results in overrun. For more than 2 periods it doesn't impact much as the overrun case should not happen
When you trace, you can see that the apps frequently inquires the position. So, an unconditional wait should be really avoided.
If they enquire after a while then we should be okay, but if they are written nicely with good power behaviour then we may have issue, as writes are typically done after period boundaries.
But have you seen any user reports on this till now.
I've seen some bug reports mentioning about crackling audio capture on SKL (I forgot URLs). It might be triggered by that.
Quiet possible..
The patch
ASoC: Intel: Skylake: Fix DMA position reporting for capture stream
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From fdd85a054b850db43c6abe39c1da28b581be5e93 Mon Sep 17 00:00:00 2001
From: Hardik T Shah hardik.t.shah@intel.com Date: Fri, 24 Mar 2017 23:10:32 +0530 Subject: [PATCH] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream
As per hardware recommendation, for every capture stream completion following operations need to be done in order to reflect the actual data that is received in position buffer.
1. Wait for 20us before reading the DMA position in buffer once the interrupt is generated for stream completion. 2. Read any of the register to flush the DMA position value. This is dummy read operation.
Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Hardik T Shah hardik.t.shah@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com Acked-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index ef440d8629e8..1823197c15c8 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -21,6 +21,7 @@
#include <linux/pci.h> #include <linux/pm_runtime.h> +#include <linux/delay.h> #include <sound/pcm_params.h> #include <sound/soc.h> #include "skl.h" @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t skl_platform_pcm_pointer * HAD space reflects the actual data that is transferred. * Use the position buffer for capture, as DPIB write gets * completed earlier than the actual data written to the DDR. + * + * For capture stream following workaround is required to fix the + * incorrect position reporting. + * + * 1. Wait for 20us before reading the DMA position in buffer once + * the interrupt is generated for stream completion as update happens + * on the HDA frame boundary i.e. 20.833uSec. + * 2. Read DPIB register to flush the DMA position value. This dummy + * read is required to flush DMA position value. + * 3. Read the DMA Position-in-Buffer. This value now will be equal to + * or greater than period boundary. */ - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { pos = readl(ebus->bus.remap_addr + AZX_REG_VS_SDXDPIB_XBASE + (AZX_REG_VS_SDXDPIB_XINTERVAL * hdac_stream(hstream)->index)); - else + } else { + udelay(20); + readl(ebus->bus.remap_addr + + AZX_REG_VS_SDXDPIB_XBASE + + (AZX_REG_VS_SDXDPIB_XINTERVAL * + hdac_stream(hstream)->index)); pos = snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream)); + }
if (pos >= hdac_stream(hstream)->bufsize) pos = 0;
From: Jeeja KP jeeja.kp@intel.com
When DSP module is unbound, the module state needs to be in INIT_DONE state instead of UNINT. Also the state needs to be set to UNINIT after module is deleted from DSP pipeline.
So, set the module state to INIT_DONE after unbind and then UNINIT after module is deleted.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/skl-messages.c | 2 +- sound/soc/intel/skylake/skl-topology.c | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index ba1ec97..09730dd 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -862,7 +862,7 @@ static void skl_clear_module_state(struct skl_module_pin *mpin, int max, }
if (!found) - mcfg->m_state = SKL_MODULE_UNINIT; + mcfg->m_state = SKL_MODULE_INIT_DONE; return; }
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index 8bd5ded..e960d9f 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -1037,6 +1037,11 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
skl_delete_pipe(ctx, mconfig->pipe);
+ list_for_each_entry(w_module, &s_pipe->w_list, node) { + src_module = w_module->w->priv; + src_module->m_state = SKL_MODULE_UNINIT; + } + return skl_tplg_unload_pipe_modules(ctx, s_pipe); }
From: Jeeja KP jeeja.kp@intel.com
Module at the end of DSP pipeline that needs to be connected to a module in another pipeline are represented as a PGA(leaf node) and in PGA event handler these modules are bound/unbounded. Modules other than PGA leaf can be connected directly or via switch to a module in another pipeline. Example: reference path.
To support the deferred DSP module bind, following changes are done: o When the path is enabled, the destination module that needs to be bound may not be initialized. If the module is not initialized, add these modules in a deferred bind list. o When the destination module is initialized, check for these modules in deferred bind list. If found, bind them. o When the destination module is deleted, Unbind the modules. o When the source module is deleted, remove the entry from the deferred bind list.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/skl-pcm.c | 12 ++++ sound/soc/intel/skylake/skl-topology.c | 109 ++++++++++++++++++++++++++++++++- sound/soc/intel/skylake/skl-topology.h | 6 ++ sound/soc/intel/skylake/skl.h | 1 + 4 files changed, 127 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index 1823197..600faad 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -1300,6 +1300,7 @@ int skl_platform_register(struct device *dev) struct skl *skl = ebus_to_skl(ebus);
INIT_LIST_HEAD(&skl->ppl_list); + INIT_LIST_HEAD(&skl->bind_list);
ret = snd_soc_register_platform(dev, &skl_platform_drv); if (ret) { @@ -1320,6 +1321,17 @@ int skl_platform_register(struct device *dev)
int skl_platform_unregister(struct device *dev) { + struct hdac_ext_bus *ebus = dev_get_drvdata(dev); + struct skl *skl = ebus_to_skl(ebus); + struct skl_module_deferred_bind *modules; + + if (!list_empty(&skl->bind_list)) { + list_for_each_entry(modules, &skl->bind_list, node) { + list_del(&modules->node); + kfree(modules); + } + } + snd_soc_unregister_component(dev); snd_soc_unregister_platform(dev); return 0; diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index e960d9f..7f28517 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -638,8 +638,9 @@ static int skl_tplg_mixer_dapm_pre_pmu_event(struct snd_soc_dapm_widget *w, struct skl_module_cfg *mconfig = w->priv; struct skl_pipe_module *w_module; struct skl_pipe *s_pipe = mconfig->pipe; - struct skl_module_cfg *src_module = NULL, *dst_module; + struct skl_module_cfg *src_module = NULL, *dst_module, *module; struct skl_sst *ctx = skl->skl_sst; + struct skl_module_deferred_bind *modules;
/* check resource available */ if (!skl_is_pipe_mcps_avail(skl, mconfig)) @@ -680,6 +681,22 @@ static int skl_tplg_mixer_dapm_pre_pmu_event(struct snd_soc_dapm_widget *w, src_module = dst_module; }
+ /* + * When the destination module is initialized, check for these modules + * in deferred bind list. If found, bind them. + */ + list_for_each_entry(w_module, &s_pipe->w_list, node) { + if (list_empty(&skl->bind_list)) + break; + + list_for_each_entry(modules, &skl->bind_list, node) { + module = w_module->w->priv; + if (modules->dst == module) + skl_bind_modules(ctx, modules->src, + modules->dst); + } + } + return 0; }
@@ -776,6 +793,44 @@ static int skl_tplg_set_module_bind_params(struct snd_soc_dapm_widget *w, return 0; }
+ +static int skl_tplg_module_add_deferred_bind(struct skl *skl, + struct skl_module_cfg *src, struct skl_module_cfg *dst) +{ + struct skl_module_deferred_bind *m_list, *modules; + int i; + + /* only supported for module with static pin connection */ + for (i = 0; i < dst->max_in_queue; i++) { + struct skl_module_pin *pin = &dst->m_in_pin[i]; + + if (pin->is_dynamic) + continue; + + if ((pin->id.module_id == src->id.module_id) && + (pin->id.instance_id == src->id.instance_id)) { + + if (!list_empty(&skl->bind_list)) { + list_for_each_entry(modules, &skl->bind_list, node) { + if (modules->src == src && modules->dst == dst) + return 0; + } + } + + m_list = kzalloc(sizeof(*m_list), GFP_KERNEL); + if (!m_list) + return -ENOMEM; + + m_list->src = src; + m_list->dst = dst; + + list_add(&m_list->node, &skl->bind_list); + } + } + + return 0; +} + static int skl_tplg_bind_sinks(struct snd_soc_dapm_widget *w, struct skl *skl, struct snd_soc_dapm_widget *src_w, @@ -810,6 +865,28 @@ static int skl_tplg_bind_sinks(struct snd_soc_dapm_widget *w, sink = p->sink; sink_mconfig = sink->priv;
+ /* + * Modules other than PGA leaf can be connected + * directly or via switch to a module in another + * pipeline. EX: reference path + * when the path is enabled, the dst module that needs + * to be bound may not be initialized. if the module is + * not initialized, add these modules in the deferred + * bind list and when the dst module is initialised, + * bind this module to the dst_module in deferred list. + */ + if (((src_mconfig->m_state == SKL_MODULE_INIT_DONE) + && (sink_mconfig->m_state == SKL_MODULE_UNINIT))) { + + ret = skl_tplg_module_add_deferred_bind(skl, + src_mconfig, sink_mconfig); + + if (ret < 0) + return ret; + + } + + if (src_mconfig->m_state == SKL_MODULE_UNINIT || sink_mconfig->m_state == SKL_MODULE_UNINIT) continue; @@ -1014,6 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w, struct skl_module_cfg *src_module = NULL, *dst_module; struct skl_sst *ctx = skl->skl_sst; struct skl_pipe *s_pipe = mconfig->pipe; + struct skl_module_deferred_bind *modules;
if (s_pipe->state == SKL_PIPE_INVALID) return -EINVAL; @@ -1022,6 +1100,35 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w, skl_tplg_free_pipe_mem(skl, mconfig);
list_for_each_entry(w_module, &s_pipe->w_list, node) { + if (list_empty(&skl->bind_list)) + break; + + src_module = w_module->w->priv; + + list_for_each_entry(modules, &skl->bind_list, node) { + /* + * When the destination module is deleted, Unbind the + * modules from deferred bind list. + */ + if (modules->dst == src_module) { + skl_unbind_modules(ctx, modules->src, + modules->dst); + } + + /* + * When the source module is deleted, remove this entry + * from the deferred bind list. + */ + if (modules->src == src_module) { + list_del(&modules->node); + modules->src = NULL; + modules->dst = NULL; + kfree(modules); + } + } + } + + list_for_each_entry(w_module, &s_pipe->w_list, node) { dst_module = w_module->w->priv;
if (mconfig->m_state >= SKL_MODULE_INIT_DONE) diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h index 8536d03..cc64d6b 100644 --- a/sound/soc/intel/skylake/skl-topology.h +++ b/sound/soc/intel/skylake/skl-topology.h @@ -336,6 +336,12 @@ struct skl_pipeline { struct list_head node; };
+struct skl_module_deferred_bind { + struct skl_module_cfg *src; + struct skl_module_cfg *dst; + struct list_head node; +}; + static inline struct skl *get_skl_ctx(struct device *dev) { struct hdac_ext_bus *ebus = dev_get_drvdata(dev); diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h index bbef77d2b..5b3fa4b 100644 --- a/sound/soc/intel/skylake/skl.h +++ b/sound/soc/intel/skylake/skl.h @@ -77,6 +77,7 @@ struct skl {
struct skl_dsp_resource resource; struct list_head ppl_list; + struct list_head bind_list;
const char *fw_name; char tplg_name[64];
On Fri, Mar 24, 2017 at 11:10:23PM +0530, jeeja.kp@intel.com wrote:
From: Jeeja KP jeeja.kp@intel.com
This patch series provides updates on the following: o Fix LLCH register read. o Add 16-bit constraint to FE bxt_rt298 machine. o Fix DMA position reporting for capture stream. o Use the sig_bits to define dai bps caps. o Update sig_bits based on converter capability. o Rearrangement of code to cleanup SKL SST library. o Add support for deferred DSP module bind.
Changes in v2:
- Addressed Pierre's comments for "Fix DMA position reporting for capture stream" patch.
B, Jayachandran (1): ALSA: hda: Fix LLCH register read
G Kranthi (1): ASoC: Intel: Skylake: Add 16-bit constraint to FE bxt_rt298 machine
Hardik T Shah (1): ASoC: Intel: Skylake: Fix DMA position reporting for capture stream
Jeeja KP (5): ASoC: Intel: Skylake: Use the sig_bits to define dai bps capability ASoC: hdac_hdmi: Update sig_bits based on converter capability ASoC: Intel: Skylake: Rearrangement of code to cleanup SKL SST library ASoC: Intel: Skylake: Fix module state after unbind and delete ASoC: Intel: Skylake: Add support for deferred DSP module bind
All these:
Acked-by: Vinod Koul vinod.koul@intel.com
participants (5)
-
jeeja.kp＠intel.com -
Mark Brown -
Takashi Iwai -
Ughreja, Rakesh A -
Vinod Koul