Re: [Sound-open-firmware] [PATCH v3 05/14] ASoC: SOF: Add PCM operations support

Pierre-Louis Bossart
2018-12-12T16:10:17Z

On 12/12/18 9:43 AM, Takashi Iwai wrote:
> On Wed, 12 Dec 2018 16:29:23 +0100,
> Pierre-Louis Bossart wrote:
>>>> + snd_sof_pcm_platform_trigger(sdev, substream, cmd);
>>>> +
>>>> + /* send IPC to the DSP */
>>>> + ret = sof_ipc_tx_message(sdev->ipc, stream.hdr.cmd, &stream,
>>>> + sizeof(stream), &reply, sizeof(reply));
>>>> +
>>>> + return ret;
>>> ... so the whole trigger action is non-atomic PCM only?
>> Not sure if I fully understand your point here. The trigger does
>> indeed need an IPC to proceed, but the front-ends are marked as such
>> with the .nonatomic field set to true. Not sure how different this is
>> from existing atom/sst or Skylake drivers.
> Hm, so this relies purely on FE, and it seems that we have no check
> whether FE and BE match wrt nonatomic PCM ops. It has to be
> addressed, but it's a different topic.
>
> At most, you can put some notes in the code that all these are
> non-atomic ops. This makes it clearer, for example, that spinlock
> with irqsave is often redundant.
Yes, noted, we'll check the spinlock use and add comments as need when
operations are non-atomic. Thanks for the suggestions, much appreciated.