[PATCH] soundwire: intel: trap TRIGGER_SUSPEND in .trigger callback
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Mon Aug 2 17:46:33 CEST 2021
>>>>>> The trigger callback is handled in the stream lock atomically,
>>>>>> and are you sure that you want to operate a possibly heavy task there?
>>>>>
>>>>> It's a good objection that we didn't think of.
>>>>
>>>> Doesn't Intel use non atomic trigger to send IPCs which anyway
>>>> involve code which can sleep..?
>>>
>>> sof_sdw.c doesn't seem setting it?
>>
>> Yes I think init_dai_link() should set it. Maybe Pierre/Bard would know why.
>
> init_dai_link() is to assign dai link elements only. No IPC is needed.
The 'nonatomic' concept is only used for an FE dailink which expose a
PCM device:
soc-pcm.c: pcm->nonatomic = rtd->dai_link->nonatomic;
Setting a BE dailink as 'nonatomic' would not accomplish much since BEs
use the 'no_pcm' option.
So the question is: is there any issue with sending an IPC in a DAI
trigger callback? This is not very different from sending a command on a
bus btw, I see a similar example for SLIMbus devices:
wcd9335.c: case SNDRV_PCM_TRIGGER_SUSPEND:
wcd9335.c- case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
wcd9335.c- slim_stream_unprepare(dai_data->sruntime);
wcd9335.c- slim_stream_disable(dai_data->sruntime);
int slim_stream_unprepare(struct slim_stream_runtime *stream)
{
int i;
for (i = 0; i < stream->num_ports; i++)
slim_disconnect_port(stream, &stream->ports[i]);
static int slim_disconnect_port(struct slim_stream_runtime *stream,
struct slim_port *port)
{
struct slim_device *sdev = stream->dev;
u8 wbuf[1];
struct slim_val_inf msg = {0, 1, NULL, wbuf, NULL};
u8 mc = SLIM_MSG_MC_DISCONNECT_PORT;
DEFINE_SLIM_LDEST_TXN(txn, mc, 5, stream->dev->laddr, &msg);
wbuf[0] = port->id;
port->ch.state = SLIM_CH_STATE_DISCONNECTED;
port->state = SLIM_PORT_DISCONNECTED;
return slim_do_transfer(sdev->ctrl, &txn);
}
Such commands may take time...
More information about the Alsa-devel
mailing list