[Sound-open-firmware] [alsa-devel] [PATCH v5 05/14] ASoC: SOF: Add PCM operations support

[Pierre-Louis Bossart]

On 4/5/19 7:30 AM, Takashi Iwai wrote:
> On Thu, 04 Apr 2019 21:13:43 +0200,
> Ranjani Sridharan wrote:
>>
>>>> +
>>>> +	/* set runtime config */
>>>> +	runtime->hw.info = SNDRV_PCM_INFO_MMAP |
>>>> +			  SNDRV_PCM_INFO_MMAP_VALID |
>>>> +			  SNDRV_PCM_INFO_INTERLEAVED |
>>>> +			  SNDRV_PCM_INFO_PAUSE |
>>>> +			  SNDRV_PCM_INFO_RESUME |
>>>> +			  SNDRV_PCM_INFO_NO_PERIOD_WAKEUP;
>>>
>>> Does SOF support the full resume?  That is, the stream gets resumed
>>> at
>>> the exact position that was suspended.  Most devices can't, hence
>>> they
>>> don't set *_INFO_RESUME flag and let user-space restarting.
>> Hi Takashi,
>>
>> Thanks for your comment. The SOF driver definitely cannot guarantee to
>> resume from the exact same position due to the fact that triggers for
>> the host dma and link dma are not synchronized and also IPC scheduling
>> will affect this. So we should remove INOF_RESUME from hw.info.
>>
>> I do one follow up question for you. When a stream resumes/restarts
>> after suspend, we have the need for restoring the hw_params in the SOF
>> driver prior to handling the START trigger. I noticed that the
>> legacy/skylake driver does not seem to do this though. Do you have any
>> insights on what we might be missing in the SOF driver?
> 
> So, SOF mandates the re-setup of the hw params in BE at resume?
> Then the right place to do is in the prepare callback.  Without
> SNDRV_PCM_INFO_RESUME flag, the PCM core won't trigger RESUME, but
> returns -ENOSYS.  Then user-space will do prepare, and start with the
> normal trigger.

Takashi, some of us at Intel are a bit nervous about returning an error 
to force the application to jump to its error-handling code (typically 
only done for xruns and not always rock-solid) and as a side effect get 
a prepare ioctl. I checked that e.g. CRAS does support this mode but I 
wonder how many application developers know about this. I personally didn't.

Moreover, it seems that the default behavior is rather to support 
RESUME, even if the hardware can't precisely restart from the same 
position, so we are concerned about exposing new errors when SOF is 
selected instead of legacy drivers (Baytrail, Atom-SST and Skylake-SST).

In short, is this really your recommendation to remove this INFO_RESUME 
flag? And if yes, should we demote all existing Intel drivers and remove 
this flag as well?

Thanks for your feedback
-Pierre