Re: [RFC 10/13] ASoC: Intel: avs: Path state management

On 2/7/22 07:25, Cezary Rojewski wrote:

Add functions to ease with state changing of all objects found in the path. Each represents either a BIND/UNBIND or SET_PIPEPILE_STATE IPC.

SET_PIPELINE?

DSP pipelines follow simple state machine scheme: CREATE -> RESET -> PAUSE -> RUNNING -> PAUSE -> RESET -> DELETE> There is no STOP, PAUSE serves that purpose instead.

that's not fully correct, the STOP can be handled in two steps due to DMA programming flows.

Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com

---

sound/soc/intel/avs/path.c | 130 +++++++++++++++++++++++++++++++++++++ sound/soc/intel/avs/path.h | 5 ++ 2 files changed, 135 insertions(+)

diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index 272d868bedc9..c6db3f091e66 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -285,3 +285,133 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id,

return path; }

+int avs_path_bind(struct avs_path *path) +{

• struct avs_path_pipeline *ppl;
• struct avs_dev *adev = path->owner;
• int ret;
• list_for_each_entry(ppl, &path->ppl_list, node) {

• struct avs_path_binding *binding;
  

• list_for_each_entry(binding, &ppl->binding_list, node) {
  

• 	struct avs_path_module *source, *sink;
  

• 	source = binding->source;
  

• 	sink = binding->sink;
  

• 	ret = avs_ipc_bind(adev, source->module_id,
  

• 			   source->instance_id, sink->module_id,
  

• 			   sink->instance_id, binding->sink_pin,
  

• 			   binding->source_pin);
  

• 	if (ret) {
  

• 		dev_err(adev->dev, "bind path failed: %d\n", ret);
  

• 		return AVS_IPC_RET(ret);
  

so what happens for all the previously bound path?

Is there an error-unwinding loop somewhere?

• 	}
  

• }
  

• }
• return 0;

+}

+int avs_path_unbind(struct avs_path *path) +{

• struct avs_path_pipeline *ppl;
• struct avs_dev *adev = path->owner;
• int ret;
• list_for_each_entry(ppl, &path->ppl_list, node) {

• struct avs_path_binding *binding;
  

• list_for_each_entry(binding, &ppl->binding_list, node) {
  

• 	struct avs_path_module *source, *sink;
  

• 	source = binding->source;
  

• 	sink = binding->sink;
  

• 	ret = avs_ipc_unbind(adev, source->module_id,
  

• 			     source->instance_id, sink->module_id,
  

• 			     sink->instance_id, binding->sink_pin,
  

• 			     binding->source_pin);
  

• 	if (ret) {
  

• 		dev_err(adev->dev, "unbind path failed: %d\n", ret);
  

• 		return AVS_IPC_RET(ret);
  

what happens then? reboot?

• 	}
  

• }
  

• }
• return 0;

+}

+int avs_path_reset(struct avs_path *path) +{

• struct avs_path_pipeline *ppl;
• struct avs_dev *adev = path->owner;
• int ret;
• if (path->state == AVS_PPL_STATE_RESET)

• return 0;
  

• list_for_each_entry(ppl, &path->ppl_list, node) {

• ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id,
  

• 				 AVS_PPL_STATE_RESET);
  

• if (ret) {
  

• 	dev_err(adev->dev, "reset path failed: %d\n", ret);
  

• 	path->state = AVS_PPL_STATE_INVALID;
  

• 	return AVS_IPC_RET(ret);
  

• }
  

• }
• path->state = AVS_PPL_STATE_RESET;
• return 0;

+}

+int avs_path_pause(struct avs_path *path) +{

• struct avs_path_pipeline *ppl;
• struct avs_dev *adev = path->owner;
• int ret;
• if (path->state == AVS_PPL_STATE_PAUSED)

• return 0;
  

• list_for_each_entry_reverse(ppl, &path->ppl_list, node) {

does the order actually matter?

• ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id,
  

• 				 AVS_PPL_STATE_PAUSED);
  

• if (ret) {
  

• 	dev_err(adev->dev, "pause path failed: %d\n", ret);
  

• 	path->state = AVS_PPL_STATE_INVALID;
  

• 	return AVS_IPC_RET(ret);
  

• }
  

• }
• path->state = AVS_PPL_STATE_PAUSED;
• return 0;

+}

it looks like you could use a helper since the flows are identical.

+int avs_path_run(struct avs_path *path, int trigger) +{

• struct avs_path_pipeline *ppl;
• struct avs_dev *adev = path->owner;
• int ret;
• if (path->state == AVS_PPL_STATE_RUNNING && trigger == AVS_TPLG_TRIGGER_AUTO)

• return 0;
  

• list_for_each_entry(ppl, &path->ppl_list, node) {

• if (ppl->template->cfg->trigger != trigger)
  

• 	continue;
  

• ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id,
  

• 				 AVS_PPL_STATE_RUNNING);
  

• if (ret) {
  

• 	dev_err(adev->dev, "run path failed: %d\n", ret);
  

• 	path->state = AVS_PPL_STATE_INVALID;
  

• 	return AVS_IPC_RET(ret);
  

• }
  

• }
• path->state = AVS_PPL_STATE_RUNNING;
• return 0;

+} diff --git a/sound/soc/intel/avs/path.h b/sound/soc/intel/avs/path.h index 3843e5a062a1..04a06473f04b 100644 --- a/sound/soc/intel/avs/path.h +++ b/sound/soc/intel/avs/path.h @@ -62,5 +62,10 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id, struct avs_tplg_path_template *template, struct snd_pcm_hw_params *fe_params, struct snd_pcm_hw_params *be_params); +int avs_path_bind(struct avs_path *path); +int avs_path_unbind(struct avs_path *path); +int avs_path_reset(struct avs_path *path); +int avs_path_pause(struct avs_path *path); +int avs_path_run(struct avs_path *path, int trigger);

#endif