[Sound-open-firmware] [PATCH] sof: Turn IRQs off globally when send command to pipeline component

[Liam Girdwood]

On Tue, 2018-03-27 at 13:50 -0500, Pierre-Louis Bossart wrote:
> 
> On 03/27/2018 01:34 PM, Liam Girdwood wrote:
> > On Tue, 2018-03-27 at 10:37 -0500, Pierre-Louis Bossart wrote:
> > > On 3/27/18 4:17 AM, Wu Zhigang wrote:
> > > > When pipeline processing the command, we have to turn off the IRQs.
> > > > If not, the DMA IRQ will interrupt the pipeline state set process,
> > > > when pipeline is scheduled, the states inconsistence will cause xrun.
> > > 
> > > we disable the irqs but then will walk the graph from host to DAI with
> > > component_op_downstream to pass a COMPS_OPS_CMD with in turn will call
> > > all .cmd callbacks from all components
> > > 
> > > This could take a significant amount of time and lead to interrupts
> > > unrelated to that specific pipeline not being serviced, leading to xruns.
> > > 
> > > what am I missing?
> > > 
> > 
> > The pipeline cmd just walks the pipeline and updates the component state
> > alongside calling comp_cmd() on each component (START, STOP, PAUSE etc).
> > This is
> > very fast in practice, about 20us on BYT. I agree that we do need to be
> > clear
> > this is in atomic context (like ALSA trigger) and that any component de-
> > init/cleanup must be done in comp_reset().
> > 
> > I will be writing some docs to go alongside 1.2 so I can include this in the
> > programming documentation.
> 
> ok, but then we need to go back to the existing components. Just looking 
> at e.g. eq_fir and src the assumption of fast operation is not right, 
> there are things like the code below which go beyond just a state change.
> 
>      switch (cmd) {
>      case COMP_CMD_SET_VALUE:
>          ret = fir_cmd_set_value(dev, cdata);
>          break;
>      case COMP_CMD_SET_DATA:
>          ret = fir_cmd_set_data(dev, cdata);
>          break;
>      }
> 
> 
>      if (cmd == COMP_CMD_SET_VALUE)
>          ret = src_ctrl_cmd(dev, cdata);
> 
> Or it could be that we need a 'fast' state change and an additional hook 
> for actual commands?

We probably want to keep the trigger() like commands atomic and add another
operation for non atomic get/set parameter type calls. It's not a big change.

I can tackle it tomorrow.

Liam

> 
> > 
> > Liam
> 
>