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

[Pierre-Louis Bossart]

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 at linux.intel.com>
> Signed-off-by: Cezary Rojewski <cezary.rojewski at 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