[Sound-open-firmware] [PATCH v1 1/3] comp: default function to set comp state

Liam Girdwood liam.r.girdwood at intel.com
Mon Sep 4 15:43:59 CEST 2017


On Mon, 2017-09-04 at 14:35 +0100, Ranjani Sridharan wrote:
> This patch adds a the comp_set_state() used for
> the mandatory pipeline commands, START, STOP, PAUSE and RELEASE. It also
> updates the cmd method in existing components to use comp_set_state().
> 
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan at linux.intel.com>
> ---
>  src/audio/component.c              | 36 ++++++++++++++++++++++++++++++++++++
>  src/audio/eq_fir.c                 | 23 ++---------------------
>  src/audio/host.c                   | 16 ++++------------
>  src/audio/src.c                    | 26 +++-----------------------
>  src/audio/tone.c                   | 26 +++-----------------------
>  src/audio/volume.c                 | 20 +++-----------------
>  src/include/reef/audio/component.h |  3 +++
>  7 files changed, 54 insertions(+), 96 deletions(-)
> 
> diff --git a/src/audio/component.c b/src/audio/component.c
> index 6369e2e..6f2e0c3 100644
> --- a/src/audio/component.c
> +++ b/src/audio/component.c
> @@ -117,6 +117,42 @@ void comp_unregister(struct comp_driver *drv)
>  	spin_unlock(&cd->lock);
>  }
>  
> +int comp_set_state(struct comp_dev *dev, int cmd)
> +{
> +	int ret = 0;
> +
> +	switch (cmd) {
> +	case COMP_CMD_START:
> +	case COMP_CMD_RELEASE:
> +		dev->state = COMP_STATE_RUNNING;
> +		break;
> +	case COMP_CMD_STOP:
> +		if (dev->state == COMP_STATE_RUNNING ||
> +		dev->state == COMP_STATE_DRAINING ||
> +		dev->state == COMP_STATE_PAUSED) {
> +			comp_buffer_reset(dev);
> +			dev->state = COMP_STATE_SETUP;
> +		} else {
> +			trace_comp_error("CEs");
> +			ret = -EINVAL;
> +		}
> +		break;
> +	case COMP_CMD_PAUSE:
> +		/* only support pausing for running */
> +		if (dev->state == COMP_STATE_RUNNING)
> +			dev->state = COMP_STATE_PAUSED;
> +		else {
> +			trace_comp_error("CEp");
> +			ret = -EINVAL;
> +		}
> +		break;
> +	default:

we should set ret to -EINVAL here and trace.

> +		break;
> +	}
> +
> +	return ret;
> +}
> +
>  void sys_comp_init(void)
>  {
>  	cd = rzalloc(RZONE_SYS, RFLAGS_NONE, sizeof(*cd));
> diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c
> index 534a4a1..6380340 100644
> --- a/src/audio/eq_fir.c
> +++ b/src/audio/eq_fir.c
> @@ -364,31 +364,12 @@ static int eq_fir_cmd(struct comp_dev *dev, int cmd, void *data)
>  
>  		break;
>  	case COMP_CMD_START:
> -		trace_src("EFs");
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
>  	case COMP_CMD_STOP:
> -		trace_src("ESp");
> -		if (dev->state == COMP_STATE_RUNNING ||
> -			dev->state == COMP_STATE_DRAINING ||
> -			dev->state == COMP_STATE_PAUSED) {
> -			comp_buffer_reset(dev);
> -			dev->state = COMP_STATE_SETUP;
> -		}
> -		break;
>  	case COMP_CMD_PAUSE:
> -		trace_src("EPe");
> -		/* only support pausing for running */
> -		if (dev->state == COMP_STATE_RUNNING)
> -			dev->state = COMP_STATE_PAUSED;
> -
> -		break;
>  	case COMP_CMD_RELEASE:
> -		trace_src("ERl");
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
>  	default:
> -		trace_src("EDf");
> +		ret = comp_set_state(dev, cmd);
> +		break;
>  	}
>  
>  	return ret;
> diff --git a/src/audio/host.c b/src/audio/host.c
> index e71e95f..f81fd66 100644
> --- a/src/audio/host.c
> +++ b/src/audio/host.c
> @@ -553,28 +553,20 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data)
>  
>  	// TODO: align cmd macros.
>  	switch (cmd) {
> -	case COMP_CMD_PAUSE:
> -		/* only support pausing for running, channel is paused by DAI */
> -		if (dev->state == COMP_STATE_RUNNING)
> -			dev->state = COMP_STATE_PAUSED;
> -		break;
>  	case COMP_CMD_STOP:
>  		if (dev->state == COMP_STATE_RUNNING ||
>  			dev->state == COMP_STATE_DRAINING ||
>  			dev->state == COMP_STATE_PAUSED)
>  			ret = host_stop(dev);
>  		break;
> -	case COMP_CMD_RELEASE:
> -		/* channel is released by DAI */
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
> -	case COMP_CMD_START:
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
>  	case COMP_CMD_SUSPEND:
>  	case COMP_CMD_RESUME:
>  		break;
> +	case COMP_CMD_START:
> +	case COMP_CMD_PAUSE:
> +	case COMP_CMD_RELEASE:
>  	default:
> +		ret = comp_set_state(dev, cmd);
>  		break;
>  	}
>  
> diff --git a/src/audio/src.c b/src/audio/src.c
> index 0279841..0ecb48c 100644
> --- a/src/audio/src.c
> +++ b/src/audio/src.c
> @@ -381,7 +381,7 @@ static int src_cmd(struct comp_dev *dev, int cmd, void *data)
>  {
>  	trace_src("SCm");
>  	struct comp_data *cd = comp_get_drvdata(dev);
> -	int i;
> +	int i, ret = 0;
>  
>  	switch (cmd) {
>  	case COMP_CMD_SRC:
> @@ -400,35 +400,15 @@ static int src_cmd(struct comp_dev *dev, int cmd, void *data)
>  
>  		break;
>  	case COMP_CMD_START:
> -		trace_src("SSt");
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
>  	case COMP_CMD_STOP:
> -		trace_src("SSp");
> -		if (dev->state == COMP_STATE_RUNNING ||
> -			dev->state == COMP_STATE_DRAINING ||
> -			dev->state == COMP_STATE_PAUSED) {
> -			comp_buffer_reset(dev);
> -			dev->state = COMP_STATE_SETUP;
> -		}
> -		break;
>  	case COMP_CMD_PAUSE:
> -		trace_src("SPe");
> -		/* only support pausing for running */
> -		if (dev->state == COMP_STATE_RUNNING)
> -			dev->state = COMP_STATE_PAUSED;
> -
> -		break;
>  	case COMP_CMD_RELEASE:
> -		trace_src("SRl");
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
>  	default:
> -		trace_src("SDf");
> +		ret = comp_set_state(dev, cmd);
>  		break;
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  /* copy and process stream data from source to sink buffers */
> diff --git a/src/audio/tone.c b/src/audio/tone.c
> index cbc50ac..305563e 100644
> --- a/src/audio/tone.c
> +++ b/src/audio/tone.c
> @@ -433,6 +433,7 @@ static int tone_cmd(struct comp_dev *dev, int cmd, void *data)
>  {
>  	struct comp_data *cd = comp_get_drvdata(dev);
>  	struct sof_ipc_comp_tone *ct;
> +	int ret = 0;
>  
>  	trace_tone("tri");
>  
> @@ -456,36 +457,15 @@ static int tone_cmd(struct comp_dev *dev, int cmd, void *data)
>  		tonegen_unmute(&cd->sg);
>  		break;
>  	case COMP_CMD_START:
> -		trace_tone("TSt");
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
>  	case COMP_CMD_STOP:
> -		trace_tone("TSp");
> -		if (dev->state == COMP_STATE_RUNNING ||
> -			dev->state == COMP_STATE_DRAINING ||
> -			dev->state == COMP_STATE_PAUSED) {
> -			comp_buffer_reset(dev);
> -			dev->state = COMP_STATE_SETUP;
> -		}
> -		break;
>  	case COMP_CMD_PAUSE:
> -		trace_tone("TPe");
> -		/* only support pausing for running */
> -		if (dev->state == COMP_STATE_RUNNING)
> -			dev->state = COMP_STATE_PAUSED;
> -
> -		break;
>  	case COMP_CMD_RELEASE:
> -		trace_tone("TRl");
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
>  	default:
> -		trace_tone("TDf");
> -
> +		ret = comp_set_state(dev, cmd);
>  		break;
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  /* copy and process stream data from source to sink buffers */
> diff --git a/src/audio/volume.c b/src/audio/volume.c
> index 93a3839..2e097d5 100644
> --- a/src/audio/volume.c
> +++ b/src/audio/volume.c
> @@ -385,7 +385,7 @@ static int volume_cmd(struct comp_dev *dev, int cmd, void *data)
>  {
>  	struct comp_data *cd = comp_get_drvdata(dev);
>  	struct sof_ipc_ctrl_values *cv;
> -	int i, j;
> +	int i, j, ret = 0;
>  
>  	trace_volume("cmd");
>  
> @@ -425,29 +425,15 @@ static int volume_cmd(struct comp_dev *dev, int cmd, void *data)
>  		work_schedule_default(&cd->volwork, VOL_RAMP_US);
>  		break;
>  	case COMP_CMD_START:
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
>  	case COMP_CMD_STOP:
> -		if (dev->state == COMP_STATE_RUNNING ||
> -		    dev->state == COMP_STATE_DRAINING ||
> -		    dev->state == COMP_STATE_PAUSED) {
> -			comp_buffer_reset(dev);
> -			dev->state = COMP_STATE_SETUP;
> -		}
> -		break;
>  	case COMP_CMD_PAUSE:
> -		/* only support pausing for running */
> -		if (dev->state == COMP_STATE_RUNNING)
> -			dev->state = COMP_STATE_PAUSED;
> -		break;
>  	case COMP_CMD_RELEASE:
> -		dev->state = COMP_STATE_RUNNING;
> -		break;
>  	default:
> +		ret = comp_set_state(dev, cmd);
>  		break;
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  /* copy and process stream data from source to sink buffers */
> diff --git a/src/include/reef/audio/component.h b/src/include/reef/audio/component.h
> index e73a22a..cbd2356 100644
> --- a/src/include/reef/audio/component.h
> +++ b/src/include/reef/audio/component.h
> @@ -215,6 +215,9 @@ static inline void comp_free(struct comp_dev *dev)
>  	dev->drv->ops.free(dev);
>  }
>  
> +/* component state set */
> +int comp_set_state(struct comp_dev *dev, int cmd);
> +
>  /* component parameter init - mandatory */
>  static inline int comp_params(struct comp_dev *dev)
>  {


---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


More information about the Sound-open-firmware mailing list