[Sound-open-firmware] [PATCH] comp: enforce state transition checking for each component.
Make sure each component checks the state transition prior to performing any action for the new state. This ensure that only supported transitions are used and any non supported transition is flagged as an error.
This also removes DRAIN state and performs some cleanup on mixer state handling.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/audio/component.c | 7 +++--- src/audio/dai.c | 16 ++++-------- src/audio/eq_fir.c | 10 +++++--- src/audio/eq_iir.c | 28 ++++----------------- src/audio/host.c | 15 +++++------ src/audio/mixer.c | 51 +++++++++++++++++++++----------------- src/audio/src.c | 14 +++++------ src/audio/tone.c | 14 +++++------ src/audio/volume.c | 15 +++++------ src/include/reef/audio/component.h | 1 - src/ipc/intel-ipc.c | 3 --- 11 files changed, 74 insertions(+), 100 deletions(-)
diff --git a/src/audio/component.c b/src/audio/component.c index 1cf4799..ec170c6 100644 --- a/src/audio/component.c +++ b/src/audio/component.c @@ -126,6 +126,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) dev->state = COMP_STATE_ACTIVE; } else { trace_comp_error("CES"); + trace_value(dev->state); ret = -EINVAL; } break; @@ -134,6 +135,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) dev->state = COMP_STATE_ACTIVE; } else { trace_comp_error("CEr"); + trace_value(dev->state); ret = -EINVAL; } break; @@ -142,6 +144,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) dev->state = COMP_STATE_READY; } else { trace_comp_error("CEs"); + trace_value(dev->state); ret = -EINVAL; } break; @@ -151,13 +154,11 @@ int comp_set_state(struct comp_dev *dev, int cmd) dev->state = COMP_STATE_PAUSED; else { trace_comp_error("CEp"); + trace_value(dev->state); ret = -EINVAL; } break; default: - trace_comp_error("CEd"); - trace_value(cmd); - ret = -EINVAL; break; }
diff --git a/src/audio/dai.c b/src/audio/dai.c index cf8de72..e22d21d 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -444,20 +444,18 @@ static int dai_reset(struct comp_dev *dev) static int dai_cmd(struct comp_dev *dev, int cmd, void *data) { struct dai_data *dd = comp_get_drvdata(dev); -// struct sof_ipc_ctrl_values *ctrl = data; int ret;
trace_dai("cmd"); tracev_value(cmd);
+ ret = comp_set_state(dev, cmd); + if (ret < 0) + return ret; + switch (cmd) { - case COMP_CMD_PAUSE: - case COMP_CMD_STOP: - dev->state = COMP_STATE_PAUSED; - break; case COMP_CMD_RELEASE: case COMP_CMD_START: - ret = dma_start(dd->dma, dd->chan); if (ret < 0) return ret; @@ -465,16 +463,12 @@ static int dai_cmd(struct comp_dev *dev, int cmd, void *data)
/* update starting wallclock */ platform_dai_wallclock(dev, &dd->wallclock); - dev->state = COMP_STATE_ACTIVE; - break; - case COMP_CMD_SUSPEND: - case COMP_CMD_RESUME: break; default: break; }
- return 0; + return ret; }
/* copy and process stream data from source to sink buffers */ diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c index ef3b7d2..ba42643 100644 --- a/src/audio/eq_fir.c +++ b/src/audio/eq_fir.c @@ -389,16 +389,18 @@ static int eq_fir_cmd(struct comp_dev *dev, int cmd, void *data)
trace_src("cmd");
+ ret = comp_set_state(dev, cmd); + if (ret < 0) + return ret; + switch (cmd) { case COMP_CMD_SET_DATA: ret = fir_cmd(dev, cdata); break; - case COMP_CMD_START: case COMP_CMD_STOP: - case COMP_CMD_PAUSE: - case COMP_CMD_RELEASE: + comp_buffer_reset(dev); + break; default: - ret = comp_set_state(dev, cmd); break; }
diff --git a/src/audio/eq_iir.c b/src/audio/eq_iir.c index 4a4cff1..995721b 100644 --- a/src/audio/eq_iir.c +++ b/src/audio/eq_iir.c @@ -392,36 +392,18 @@ static int eq_iir_cmd(struct comp_dev *dev, int cmd, void *data)
trace_eq_iir("cmd");
+ ret = comp_set_state(dev, cmd); + if (ret < 0) + return ret; + switch (cmd) { case COMP_CMD_SET_DATA: ret = iir_cmd(dev, cdata); break; - case COMP_CMD_START: - trace_eq_iir("EFs"); - dev->state = COMP_STATE_ACTIVE; - break; case COMP_CMD_STOP: - trace_eq_iir("ESp"); - if (dev->state == COMP_STATE_ACTIVE || - dev->state == COMP_STATE_PAUSED) { - comp_buffer_reset(dev); - dev->state = COMP_STATE_READY; - } - break; - case COMP_CMD_PAUSE: - trace_eq_iir("EPe"); - /* only support pausing for running */ - if (dev->state == COMP_STATE_ACTIVE) - dev->state = COMP_STATE_PAUSED; - - break; - case COMP_CMD_RELEASE: - trace_eq_iir("ERl"); - dev->state = COMP_STATE_ACTIVE; + comp_buffer_reset(dev); break; default: - trace_eq_iir("EDf"); - ret = -EINVAL; break; }
diff --git a/src/audio/host.c b/src/audio/host.c index 8aa29ad..a1d312a 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -278,7 +278,7 @@ static struct comp_dev *host_new(struct sof_ipc_comp *comp)
/* init posn data. TODO: other fields */ hd->posn.comp_id = comp->id; - + dev->state = COMP_STATE_READY; return dev;
error: @@ -549,19 +549,16 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data)
trace_host("cmd");
+ ret = comp_set_state(dev, cmd); + if (ret < 0) + return ret; + switch (cmd) { case COMP_CMD_PAUSE: case COMP_CMD_STOP: - if (dev->state == COMP_STATE_ACTIVE) - ret = host_stop(dev); - break; - case COMP_CMD_SUSPEND: - case COMP_CMD_RESUME: + ret = host_stop(dev); break; - case COMP_CMD_START: - case COMP_CMD_RELEASE: default: - ret = comp_set_state(dev, cmd); break; }
diff --git a/src/audio/mixer.c b/src/audio/mixer.c index 85aa601..3861b7c 100644 --- a/src/audio/mixer.c +++ b/src/audio/mixer.c @@ -151,55 +151,60 @@ static int mixer_params(struct comp_dev *dev) return 0; }
-static int mixer_status_change(struct comp_dev *dev/* , uint32_t target_state */) +static int mixer_source_status_count(struct comp_dev *mixer, uint32_t status) { - int finish = 0; struct comp_buffer *source; struct list_item * blist; - uint32_t stream_target = COMP_STATE_INIT; + int count = 0;
- /* calculate the highest status between input streams */ - list_for_item(blist, &dev->bsource_list) { + /* count source with state == status */ + list_for_item(blist, &mixer->bsource_list) { source = container_of(blist, struct comp_buffer, sink_list); - if (source->source->state > stream_target) - stream_target = source->source->state; + if (source->source->state == status) + count++; }
- if (dev->state == stream_target) - finish = 1; - else - dev->state = stream_target; + return count; +} + +static inline int mixer_sink_status(struct comp_dev *mixer) +{ + struct comp_buffer *sink;
- return finish; + sink = list_first_item(&mixer->bsink_list, struct comp_buffer, + source_list); + return sink->sink->state; }
/* used to pass standard and bespoke commands (with data) to component */ static int mixer_cmd(struct comp_dev *dev, int cmd, void *data) { - int finish = 0; + int ret;
trace_mixer("cmd");
+ ret = comp_set_state(dev, cmd); + if (ret < 0) + return ret; + switch(cmd) { case COMP_CMD_START: - trace_mixer("MSa"); - case COMP_CMD_PAUSE: case COMP_CMD_RELEASE: - case COMP_CMD_DRAIN: - case COMP_CMD_SUSPEND: - case COMP_CMD_RESUME: - finish = mixer_status_change(dev); + if (mixer_sink_status(dev) == COMP_STATE_ACTIVE) + return 1; /* no need to go downstream */ break; + case COMP_CMD_PAUSE: case COMP_CMD_STOP: - finish = mixer_status_change(dev); - if (finish == 0) - comp_buffer_reset(dev); + if (mixer_source_status_count(dev, COMP_STATE_ACTIVE) > 0) { + dev->state = COMP_STATE_ACTIVE; + return 1; /* no need to go downstream */ + } break; default: break; }
- return finish; + return 0; /* send cmd downstream */ }
/* diff --git a/src/audio/src.c b/src/audio/src.c index bc32f95..1f84a5a 100644 --- a/src/audio/src.c +++ b/src/audio/src.c @@ -292,6 +292,7 @@ static struct comp_dev *src_new(struct sof_ipc_comp *comp) for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) src_polyphase_reset(&cd->src[i]);
+ dev->state = COMP_STATE_READY; return dev; }
@@ -489,17 +490,16 @@ static int src_cmd(struct comp_dev *dev, int cmd, void *data) struct sof_ipc_ctrl_data *cdata = data; int ret = 0;
- trace_src("SCm"); + trace_src("cmd"); + + ret = comp_set_state(dev, cmd); + if (ret < 0) + return ret;
switch (cmd) { case COMP_CMD_SET_VALUE: return src_ctrl_cmd(dev, cdata); - case COMP_CMD_START: - case COMP_CMD_STOP: - case COMP_CMD_PAUSE: - case COMP_CMD_RELEASE: default: - ret = comp_set_state(dev, cmd); break; }
@@ -564,7 +564,7 @@ static int src_reset(struct comp_dev *dev) for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) src_polyphase_reset(&cd->src[i]);
- dev->state = COMP_STATE_INIT; + dev->state = COMP_STATE_READY; return 0; }
diff --git a/src/audio/tone.c b/src/audio/tone.c index aec8a72..94d9aa7 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -395,6 +395,7 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) /* Reset tone generator and set channels volumes to default */ tonegen_reset(&cd->sg);
+ dev->state = COMP_STATE_READY; return dev; }
@@ -472,17 +473,16 @@ static int tone_cmd(struct comp_dev *dev, int cmd, void *data) struct sof_ipc_ctrl_data *cdata = data; int ret = 0;
- trace_tone("tri"); + trace_tone("cmd"); + + ret = comp_set_state(dev, cmd); + if (ret < 0) + return ret;
switch (cmd) { case COMP_CMD_SET_VALUE: return tone_ctrl_cmd(dev, cdata); - case COMP_CMD_START: - case COMP_CMD_STOP: - case COMP_CMD_PAUSE: - case COMP_CMD_RELEASE: default: - ret = comp_set_state(dev, cmd); break; }
@@ -550,7 +550,7 @@ static int tone_reset(struct comp_dev *dev) /* Initialize with the defaults */ tonegen_reset(&cd->sg);
- dev->state = COMP_STATE_INIT; + dev->state = COMP_STATE_READY;
return 0; } diff --git a/src/audio/volume.c b/src/audio/volume.c index 45bbe27..960a144 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -466,25 +466,22 @@ static int volume_ctrl_get_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c static int volume_cmd(struct comp_dev *dev, int cmd, void *data) { struct sof_ipc_ctrl_data *cdata = data; - int ret = 0; + int ret;
trace_volume("cmd");
+ ret = comp_set_state(dev, cmd); + if (ret < 0) + return ret; + switch (cmd) { case COMP_CMD_SET_VALUE: return volume_ctrl_set_cmd(dev, cdata); case COMP_CMD_GET_VALUE: return volume_ctrl_get_cmd(dev, cdata); - case COMP_CMD_START: - case COMP_CMD_STOP: - case COMP_CMD_PAUSE: - case COMP_CMD_RELEASE: default: - ret = comp_set_state(dev, cmd); - break; + return ret; } - - 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 d092210..5ae8084 100644 --- a/src/include/reef/audio/component.h +++ b/src/include/reef/audio/component.h @@ -76,7 +76,6 @@ #define COMP_CMD_START 1 /* start component stream */ #define COMP_CMD_PAUSE 2 /* immediately pause the component stream */ #define COMP_CMD_RELEASE 3 /* release paused component stream */ -#define COMP_CMD_DRAIN 4 /* drain component buffers */ #define COMP_CMD_SUSPEND 5 /* suspend component */ #define COMP_CMD_RESUME 6 /* resume component */
diff --git a/src/ipc/intel-ipc.c b/src/ipc/intel-ipc.c index 752ea0c..92dfd73 100644 --- a/src/ipc/intel-ipc.c +++ b/src/ipc/intel-ipc.c @@ -394,9 +394,6 @@ static int ipc_stream_trigger(uint32_t header) case iCS(SOF_IPC_STREAM_TRIG_RELEASE): cmd = COMP_CMD_RELEASE; break; - case iCS(SOF_IPC_STREAM_TRIG_DRAIN): - cmd = COMP_CMD_DRAIN; - break; /* XRUN is special case- TODO */ case iCS(SOF_IPC_STREAM_TRIG_XRUN): return 0;
participants (1)
-
Liam Girdwood