diff --git a/Documentation/driver-api/soundwire/stream.rst b/Documentation/driver-api/soundwire/stream.rst index 5351bd2f34a870..9b7418ff8d59aa 100644 --- a/Documentation/driver-api/soundwire/stream.rst +++ b/Documentation/driver-api/soundwire/stream.rst @@ -156,22 +156,27 @@ Below shows the SoundWire stream states and state transition diagram. :: +-----------+ +------------+ +----------+ +----------+ | ALLOCATED +---->| CONFIGURED +---->| PREPARED +---->| ENABLED | | STATE | | STATE | | STATE | | STATE | - +-----------+ +------------+ +----------+ +----+-----+ - ^ - | - | - v - +----------+ +------------+ +----+-----+ + +-----------+ +------------+ +---+--+---+ +----+-----+ + ^ ^ ^ + | | | + __| |___________ | + | | | + v | v + +----------+ +-----+------+ +-+--+-----+ | RELEASED |<----------+ DEPREPARED |<-------+ DISABLED | | STATE | | STATE | | STATE | +----------+ +------------+ +----------+ -NOTE: State transition between prepare and deprepare is supported in Spec -but not in the software (subsystem) +NOTE: State transitions between ``SDW_STREAM_ENABLED`` and +``SDW_STREAM_DISABLED`` are only relevant when then INFO_PAUSE flag is +supported at the ALSA/ASoC level. Likewise the transition between +``SDW_DISABLED_STATE`` and ``SDW_PREPARED_STATE`` depends on the +INFO_RESUME flag. -NOTE2: Stream state transition checks need to be handled by caller -framework, for example ALSA/ASoC. No checks for stream transition exist in -SoundWire subsystem. +NOTE2: The framework implements basic state transition checks, but +does not e.g. check if a transition from DISABLED to ENABLED is valid +on a specific platform. Such tests need to be added at the ALSA/ASoC +level. Stream State Operations ----------------------- @@ -246,6 +251,9 @@ SDW_STREAM_PREPARED Prepare state of stream. Operations performed before entering in this state: + (0) Steps 1 and 2 are omitted in the case of a resume operation, + where the bus bandwidth is known. + (1) Bus parameters such as bandwidth, frame shape, clock frequency, are computed based on current stream as well as already active stream(s) on Bus. Re-computation is required to accommodate current @@ -270,13 +278,15 @@ Prepare state of stream. Operations performed before entering in this state: After all above operations are successful, stream state is set to ``SDW_STREAM_PREPARED``. -Bus implements below API for PREPARE state which needs to be called once per -stream. From ASoC DPCM framework, this stream state is linked to -.prepare() operation. +Bus implements below API for PREPARE state which needs to be called +once per stream. From ASoC DPCM framework, this stream state is linked +to .prepare() operation. Since the .trigger() operations may not +follow the .prepare(), a direct transitions from +``SDW_STREAM_PREPARED`` to ``SDW_STREAM_DEPREPARED`` is allowed. .. code-block:: c - int sdw_prepare_stream(struct sdw_stream_runtime * stream); + int sdw_prepare_stream(struct sdw_stream_runtime * stream, bool resume); SDW_STREAM_ENABLED @@ -332,6 +342,14 @@ Bus implements below API for DISABLED state which needs to be called once per stream. From ASoC DPCM framework, this stream state is linked to .trigger() stop operation. +When the INFO_PAUSE flag is supported, a direct transition to +``SDW_STREAM_ENABLED`` is allowed. + +For resume operations where ASoC will use the .prepare() callback, the +stream can transition from ``SDW_STREAM_DISABLED`` to +``SDW_STREAM_PREPARED``, with all required settings restored but +without updating the bandwidth and bit allocation. + .. code-block:: c int sdw_disable_stream(struct sdw_stream_runtime * stream); @@ -353,9 +371,18 @@ state: After all above operations are successful, stream state is set to ``SDW_STREAM_DEPREPARED``. -Bus implements below API for DEPREPARED state which needs to be called once -per stream. From ASoC DPCM framework, this stream state is linked to -.trigger() stop operation. +Bus implements below API for DEPREPARED state which needs to be called +once per stream. ALSA/ASoC do not have a concept of 'deprepare', and +the mapping from this stream state to ALSA/ASoC operation may be +implementation specific. + +When the INFO_PAUSE flag is supported, the stream state is linked to +the .hw_free() operation - the stream is not deprepared on a +TRIGGER_STOP. + +Other implementations may transition to the ``SDW_STREAM_DEPREPARED`` +state on TRIGGER_STOP, should they require a transition through the +``SDW_STREAM_PREPARED`` state. .. code-block:: c diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index 0f108fd31fd943..9c0c78da6b309f 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -81,6 +81,7 @@ struct sdw_cdns_stream_config { * @bus: Bus handle * @stream_type: Stream type * @link_id: Master link id + * after a TRIGGER_SUSPEND. */ struct sdw_cdns_dma_data { char *name; diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index a61d4cbcbea687..a430c518a8746c 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -117,6 +117,8 @@ struct sdw_intel { struct sdw_cdns cdns; int instance; struct sdw_intel_link_res *link_res; + struct snd_pcm_hw_params *hw_params; + struct sdw_cdns_pdi *pdi; #ifdef CONFIG_DEBUG_FS struct dentry *debugfs; #endif @@ -760,6 +762,8 @@ static int intel_startup(struct snd_pcm_substream *substream, struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); int ret; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + if (pm_runtime_enabled(cdns->dev)) { ret = pm_runtime_get_sync(cdns->dev); if (ret < 0) { @@ -770,7 +774,11 @@ static int intel_startup(struct snd_pcm_substream *substream, } } - return sdw_stream_setup(substream, dai); + ret = sdw_stream_setup(substream, dai); + + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); + + return ret; } static int intel_hw_params(struct snd_pcm_substream *substream, @@ -787,6 +795,8 @@ static int intel_hw_params(struct snd_pcm_substream *substream, int ret; bool pcm = true; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) return -EIO; @@ -815,6 +825,8 @@ static int intel_hw_params(struct snd_pcm_substream *substream, intel_pdi_alh_configure(sdw, pdi); sdw_cdns_config_stream(cdns, ch, dir, pdi); + sdw->pdi = pdi; + sdw->hw_params = params; /* Inform DSP about PDI stream number */ ret = intel_params_stream(sdw, substream, dai, params, @@ -851,14 +863,23 @@ static int intel_hw_params(struct snd_pcm_substream *substream, dev_err(cdns->dev, "add master to stream failed:%d\n", ret); kfree(pconfig); + error: + + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); return ret; } static int intel_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { + struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); + struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_cdns_dma_data *dma; + int ch, dir; + int ret; + + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) { @@ -867,7 +888,36 @@ static int intel_prepare(struct snd_pcm_substream *substream, return -EIO; } - return sdw_prepare_stream(dma->stream); + /* + * .prepare() is called after system resume, where we need to + * reinitialize the SHIM/ALH/Cadence IP. To avoid dealing with + * complicated state machines, we just re-initialize in all + * cases since there are no side effects. + */ + + /* configure stream */ + ch = params_channels(sdw->hw_params); + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) + dir = SDW_DATA_DIR_RX; + else + dir = SDW_DATA_DIR_TX; + + intel_pdi_shim_configure(sdw, sdw->pdi); + intel_pdi_alh_configure(sdw, sdw->pdi); + sdw_cdns_config_stream(cdns, ch, dir, sdw->pdi); + + /* Inform DSP about PDI stream number */ + ret = intel_params_stream(sdw, substream, dai, sdw->hw_params, + sdw->instance, + sdw->pdi->intel_alh_id); + if (ret) + goto err; + + ret = sdw_prepare_stream(dma->stream); + + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); +err: + return ret; } static int intel_trigger(struct snd_pcm_substream *substream, int cmd, @@ -876,6 +926,8 @@ static int intel_trigger(struct snd_pcm_substream *substream, int cmd, struct sdw_cdns_dma_data *dma; int ret; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) { dev_err(dai->dev, "failed to get dma data in %s", __func__); @@ -883,14 +935,18 @@ static int intel_trigger(struct snd_pcm_substream *substream, int cmd, } switch (cmd) { + case SNDRV_PCM_TRIGGER_RESUME: + dev_err(dai->dev, "%s: %s: resume\n", __func__, dai->name); + /* fallthrough */ case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - case SNDRV_PCM_TRIGGER_RESUME: ret = sdw_enable_stream(dma->stream); break; - case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_SUSPEND: + dev_err(dai->dev, "%s: %s: suspend\n", __func__, dai->name); + /* fallthrough */ + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_STOP: ret = sdw_disable_stream(dma->stream); break; @@ -904,6 +960,9 @@ static int intel_trigger(struct snd_pcm_substream *substream, int cmd, dev_err(dai->dev, "%s trigger %d failed: %d", __func__, cmd, ret); + + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); + return ret; } @@ -915,6 +974,8 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) struct sdw_cdns_dma_data *dma; int ret; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) return -EIO; @@ -938,8 +999,12 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) return ret; } + sdw->hw_params = NULL; + sdw->pdi = NULL; sdw_release_stream(dma->stream); + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); + return 0; } @@ -950,6 +1015,8 @@ static void intel_shutdown(struct snd_pcm_substream *substream, struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); int ret; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) return; @@ -965,6 +1032,8 @@ static void intel_shutdown(struct snd_pcm_substream *substream, "pm_runtime_put_autosuspend failed in %s:, ret %d\n", __func__, ret); } + + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); } static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai, @@ -1320,6 +1389,8 @@ static int intel_suspend(struct device *dev) return 0; } + dev_err(dev, "%s start\n", __func__); + if (pm_runtime_status_suspended(dev)) { dev_dbg(dev, "%s: pm_runtime status: suspended\n", @@ -1348,6 +1419,8 @@ static int intel_suspend(struct device *dev) intel_shim_wake(sdw, false); + dev_err(dev, "%s done\n", __func__); + return 0; } @@ -1363,6 +1436,8 @@ static int intel_suspend_runtime(struct device *dev) return 0; } + dev_err(dev, "%s start\n", __func__); + ret = sdw_cdns_enable_interrupt(cdns, false); if (ret < 0) { dev_err(dev, "cannot disable interrupts on suspend\n"); @@ -1377,6 +1452,8 @@ static int intel_suspend_runtime(struct device *dev) intel_shim_wake(sdw, false); + dev_err(dev, "%s done\n", __func__); + return 0; } @@ -1393,6 +1470,8 @@ static int intel_resume(struct device *dev) return 0; } + dev_err(dev, "%s start\n", __func__); + if (md->pm_runtime_suspended) { dev_dbg(dev, "%s: pm_runtime status was suspended, forcing active\n", @@ -1425,6 +1504,8 @@ static int intel_resume(struct device *dev) return ret; } + dev_err(dev, "%s done\n", __func__); + return ret; } @@ -1440,6 +1521,8 @@ static int intel_resume_runtime(struct device *dev) return 0; } + dev_err(dev, "%s start\n", __func__); + ret = intel_init(sdw); if (ret) { dev_err(dev, "%s failed: %d", __func__, ret); @@ -1461,6 +1544,8 @@ static int intel_resume_runtime(struct device *dev) return ret; } + dev_err(dev, "%s done\n", __func__); + return ret; } diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 7f7749ff812054..3fac5a2a071c44 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1460,7 +1460,8 @@ static void sdw_release_bus_lock(struct sdw_stream_runtime *stream) } } -static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) +static int _sdw_prepare_stream(struct sdw_stream_runtime *stream, + bool update_params) { struct sdw_master_runtime *m_rt; struct sdw_bus *bus = NULL; @@ -1480,6 +1481,9 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) return -EINVAL; } + if (!update_params) + goto program_params; + /* Increment cumulative bus bandwidth */ /* TODO: Update this during Device-Device support */ bus->params.bandwidth += m_rt->stream->params.rate * @@ -1495,6 +1499,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) } } +program_params: /* Program params */ ret = sdw_program_params(bus); if (ret < 0) { @@ -1544,23 +1549,46 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) */ int sdw_prepare_stream(struct sdw_stream_runtime *stream) { + bool update_params = true; int ret = 0; + pr_err("%s: %s start\n", __func__, stream->name); + if (!stream) { pr_err("SoundWire: Handle not found for stream\n"); return -EINVAL; } - if (stream->state != SDW_STREAM_CONFIGURED) - return 0; - sdw_acquire_bus_lock(stream); - ret = _sdw_prepare_stream(stream); + if (stream->state != SDW_STREAM_CONFIGURED && + stream->state != SDW_STREAM_DEPREPARED && + stream->state != SDW_STREAM_DISABLED) { + pr_err("%s: %s: inconsistent state state %d\n", + __func__, stream->name, stream->state); + ret = -EINVAL; + goto state_err; + } + + /* + * when the stream is DISABLED, this means sdw_prepare_stream() + * is called as a result of an underflow or a resume operation. + * In this case, the bus parameters shall not be recomputed, but + * still need to be re-applied + */ + if (stream->state == SDW_STREAM_DISABLED) + update_params = false; + + ret = _sdw_prepare_stream(stream, update_params); + if (ret < 0) pr_err("Prepare for stream:%s failed: %d\n", stream->name, ret); +state_err: sdw_release_bus_lock(stream); + + pr_err("%s: %s: done\n", __func__, stream->name); + return ret; } EXPORT_SYMBOL(sdw_prepare_stream); @@ -1617,6 +1645,8 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream) { int ret; + pr_err("%s: %s: start\n", __func__, stream->name); + if (!stream) { pr_err("SoundWire: Handle not found for stream\n"); return -EINVAL; @@ -1624,11 +1654,22 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state != SDW_STREAM_PREPARED && + stream->state != SDW_STREAM_DISABLED) { + pr_err("%s: %s: inconsistent state state %d\n", + __func__, stream->name, stream->state); + ret = -EINVAL; + goto state_err; + } + ret = _sdw_enable_stream(stream); if (ret < 0) pr_err("Enable for stream:%s failed: %d\n", stream->name, ret); +state_err: sdw_release_bus_lock(stream); + + pr_err("%s: %s: done\n", __func__, stream->name); return ret; } EXPORT_SYMBOL(sdw_enable_stream); @@ -1693,6 +1734,8 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream) { int ret; + pr_err("%s: %s: start\n", __func__, stream->name); + if (!stream) { pr_err("SoundWire: Handle not found for stream\n"); return -EINVAL; @@ -1700,11 +1743,22 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state != SDW_STREAM_ENABLED) { + pr_err("%s: %s: inconsistent state state %d\n", + __func__, stream->name, stream->state); + ret = -EINVAL; + goto state_err; + } + ret = _sdw_disable_stream(stream); if (ret < 0) pr_err("Disable for stream:%s failed: %d\n", stream->name, ret); +state_err: sdw_release_bus_lock(stream); + + pr_err("%s: %s: done\n", __func__, stream->name); + return ret; } EXPORT_SYMBOL(sdw_disable_stream); @@ -1752,17 +1806,32 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream) { int ret; + pr_err("%s: %s: start\n", __func__, stream->name); + if (!stream) { pr_err("SoundWire: Handle not found for stream\n"); return -EINVAL; } sdw_acquire_bus_lock(stream); + + if (stream->state != SDW_STREAM_PREPARED && + stream->state != SDW_STREAM_DISABLED) { + pr_err("%s: %s: inconsistent state state %d\n", + __func__, stream->name, stream->state); + ret = -EINVAL; + goto state_err; + } + ret = _sdw_deprepare_stream(stream); if (ret < 0) pr_err("De-prepare for stream:%d failed: %d\n", ret, ret); +state_err: sdw_release_bus_lock(stream); + + pr_err("%s: %s: done\n", __func__, stream->name); + return ret; } EXPORT_SYMBOL(sdw_deprepare_stream); diff --git a/sound/soc/Makefile b/sound/soc/Makefile index 861a21b7948443..429add4afe3827 100644 --- a/sound/soc/Makefile +++ b/sound/soc/Makefile @@ -1,3 +1,5 @@ +ccflags-y += -DDEBUG + # SPDX-License-Identifier: GPL-2.0 snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-utils.o soc-dai.o soc-component.o snd-soc-core-objs += soc-pcm.o soc-io.o soc-devres.o soc-ops.o diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index d72798d2b3026f..47017ca56c8aed 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -346,6 +346,9 @@ static int sof_pcm_trigger(struct snd_soc_component *component, stream.hdr.cmd |= SOF_IPC_STREAM_TRIG_RELEASE; break; case SNDRV_PCM_TRIGGER_RESUME: + dev_dbg(sdev->dev, "pcm: trigger resume stream %d dir %d cmd %d\n", + spcm->pcm.pcm_id, substream->stream, cmd); + /* set up hw_params */ ret = sof_pcm_prepare(component, substream); if (ret < 0) { @@ -359,6 +362,9 @@ static int sof_pcm_trigger(struct snd_soc_component *component, stream.hdr.cmd |= SOF_IPC_STREAM_TRIG_START; break; case SNDRV_PCM_TRIGGER_SUSPEND: + dev_dbg(sdev->dev, "pcm: trigger suspend stream %d dir %d cmd %d\n", + spcm->pcm.pcm_id, substream->stream, cmd); + /* fallthrough */ case SNDRV_PCM_TRIGGER_STOP: stream.hdr.cmd |= SOF_IPC_STREAM_TRIG_STOP; ipc_first = true; diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 81e623dfc7e52b..eb9900e709cb99 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -17,6 +17,8 @@ static int sof_restore_kcontrols(struct snd_sof_dev *sdev) int ipc_cmd, ctrl_type; int ret = 0; + dev_err(sdev->dev, "%s\n", __func__); + /* restore kcontrol values */ list_for_each_entry(scontrol, &sdev->kcontrol_list, list) { /* reset readback offset for scontrol after resuming */ @@ -69,6 +71,8 @@ static int sof_restore_pipelines(struct snd_sof_dev *sdev) struct sof_ipc_cmd_hdr *hdr; int ret; + dev_err(sdev->dev, "%s\n", __func__); + /* restore pipeline components */ list_for_each_entry_reverse(swidget, &sdev->widget_list, list) { struct sof_ipc_comp_reply r; @@ -202,6 +206,8 @@ static int sof_send_pm_ipc(struct snd_sof_dev *sdev, int cmd) struct sof_ipc_pm_ctx pm_ctx; struct sof_ipc_reply reply; + dev_err(sdev->dev, "%s\n", __func__); + memset(&pm_ctx, 0, sizeof(pm_ctx)); /* configure ctx save ipc message */ @@ -220,6 +226,7 @@ static int sof_set_hw_params_upon_resume(struct snd_sof_dev *sdev) snd_pcm_state_t state; int dir; + dev_err(sdev->dev, "%s\n", __func__); /* * SOF requires hw_params to be set-up internally upon resume. * So, set the flag to indicate this for those streams that @@ -265,6 +272,8 @@ static int sof_resume(struct device *dev, bool runtime_resume) struct snd_sof_dev *sdev = dev_get_drvdata(dev); int ret; + dev_err(dev, "%s\n", __func__); + /* do nothing if dsp resume callbacks are not set */ if (!sof_ops(sdev)->resume || !sof_ops(sdev)->runtime_resume) return 0; @@ -337,6 +346,8 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) struct snd_sof_dev *sdev = dev_get_drvdata(dev); int ret; + dev_err(dev, "%s\n", __func__); + /* do nothing if dsp suspend callback is not set */ if (!sof_ops(sdev)->suspend) return 0; @@ -393,6 +404,7 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) int snd_sof_runtime_suspend(struct device *dev) { + dev_err(dev, "%s\n", __func__); return sof_suspend(dev, true); } EXPORT_SYMBOL(snd_sof_runtime_suspend); @@ -401,24 +413,28 @@ int snd_sof_runtime_idle(struct device *dev) { struct snd_sof_dev *sdev = dev_get_drvdata(dev); + dev_err(dev, "%s\n", __func__); return snd_sof_dsp_runtime_idle(sdev); } EXPORT_SYMBOL(snd_sof_runtime_idle); int snd_sof_runtime_resume(struct device *dev) { + dev_err(dev, "%s\n", __func__); return sof_resume(dev, true); } EXPORT_SYMBOL(snd_sof_runtime_resume); int snd_sof_resume(struct device *dev) { + dev_err(dev, "%s\n", __func__); return sof_resume(dev, false); } EXPORT_SYMBOL(snd_sof_resume); int snd_sof_suspend(struct device *dev) { + dev_err(dev, "%s\n", __func__); return sof_suspend(dev, false); } EXPORT_SYMBOL(snd_sof_suspend);