Re: [Xen-devel][PATCH 0/2] sndif: add explicit back and front synchronization

[Oleksandr Andrushchenko <andr2000@gmail.com>]

On 03/06/2018 02:52 PM, Takashi Iwai wrote:
> On Tue, 06 Mar 2018 13:05:16 +0100,
> Oleksandr Andrushchenko wrote:
>> On 03/06/2018 01:32 PM, Takashi Iwai wrote:
>>> On Tue, 06 Mar 2018 12:25:07 +0100,
>>> Oleksandr Andrushchenko wrote:
>>>> On 03/06/2018 12:52 PM, Takashi Iwai wrote:
>>>>> On Mon, 05 Feb 2018 09:24:58 +0100,
>>>>> Oleksandr Andrushchenko wrote:
>>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>>>
>>>>>> Hi, all!
>>>>>>
>>>>>> Foreword
>>>>>> ========
>>>>>>
>>>>>> This change is aimed to add support for explicit back and front
>>>>>> synchronization during playback and capture in response to comments
>>>>>> raised during upstream attempt of the para-virtualized sound frontend
>>>>>> driver for Xen [1], [2] and gather opinions from the relevant communities
>>>>>> (ALSA, Xen) on the change.
>>>>>>
>>>>>> The relevant backend is implemented as a user-space application [3]
>>>>>> and uses accompanying helper library [4].
>>>>>>
>>>>>> Both frontend driver and backend were tested on real HW running Xen hypervisor
>>>>>> (Renesas R-Car ARM based H3/M3 boards, x86) to make sure the proposed
>>>>>> solution does work.
>>>>>>
>>>>>> Rationale
>>>>>> =========
>>>>>>
>>>>>> During the first attempt to upstream the Linux front driver [5] number
>>>>>> of comments and concerns were raised, one of the biggest flaws in the
>>>>>> design were questioned by both Clemens Ladisch [6] and
>>>>>> Takashi Sakamoto [7]: the absence of synchronization between frontend
>>>>>> and backend during capture/playback. Two options were discussed:
>>>>>>
>>>>>> “In design of ALSA PCM core, drivers are expected to synchronize to
>>>>>> actual hardwares for semi-realtime data transmission. The
>>>>>> synchronization is done by two points:
>>>>>> 1) Interrupts to respond events from actual hardwares.
>>>>>> 2) Positions of actual data transmission in any serial sound interfaces
>>>>>>        of actual hardwares.
>>>>>> “
>>>>>>
>>>>>> and finally a change to the existing protocol was suggested:
>>>>>>
>>>>>> “In 'include/xen/interface/io/sndif.h', there's no functionalities I
>>>>>> described the above:
>>>>>> 1. notifications from DomU to Dom0 about the size of period for
>>>>>>        interrupts from actual hardwares. Or no way from Dom0 to DomU about
>>>>>>        the configured size of the period.
>>>>>> 2. notifications of the interrupts from actual hardwares to DomU.”
>>>>>>
>>>>>> This is implemented as a change to the sndif protocol and allows removing
>>>>>> period emulation:
>>>>>> 1. Introduced a new event channel from back to front
>>>>>> 2. New event with number of bytes played/captured (XENSND_EVT_CUR_POS,
>>>>>>       to be used for sending snd_pcm_period_elapsed at frontend (in Linux
>>>>>>       implementation). Sent in bytes, not frames to make the protocol
>>>>>>       generic and consistent)
>>>>>> 3. New request for playback/capture control (XENSND_OP_TRIGGER) with
>>>>>>       start/pause/stop/resume sub-ops
>>>>>> 4. Playback/capture buffer size is set on the backend side via
>>>>>>       XENSND_FIELD_BUFFER_SIZE XenStore entry
>>>>> So the new addition looks serving well for the point that was
>>>>> suggested in the previous thread.  As I see no frontend driver
>>>>> implementation, it's hard to tell about the details, but through a
>>>>> quick glance, the protocol should be OK.
>>>> Thank you, the driver is at [1]
>>>>> Now, going back to a big picture: I took a look at the previous
>>>>> patchset, and wonder what about the hw_params setup.  Basically the
>>>>> (frontend) application may request any size of buffer and periods
>>>>> unless the driver sets up the hw constraints at open callback.  That
>>>>> is, app may request even the 16 bytes of buffer size, or 1GB of
>>>>> buffer.  The periods aren't always integer, so it can be 1024 bytes of
>>>>> buffer with 400 bytes of periods.
>>>>>
>>>>> And, if such parameters are set up freely in the frontend side, how
>>>>> the backend is supposed to behave?  From the frontend POV, it expects
>>>>> receiving the wakeup/notification at each period processing (e.g. 400
>>>>> bytes in the case above).  But, the backend is another application, so
>>>>> how would it work for such requirements?  Am I missing something here?
>>>> Well, the frontend is not that free to decide as it might look like,
>>>> e.g. please see [2]. Basically part of hw_params configuration is written
>>>> to XenStore [3] as a part of domain configuration which depends on
>>>> system/backend
>>>> capabilities. E.g., we usually set buffer sizes to match real HW at
>>>> backend side
>>>> if we use ALSA and we have more freedom if we use PulseAudio there.
>>>> Finally, if backend decides that the requested buffer/period sizes are
>>>> not acceptable it will reject such a configuration.
>>> OK, that restricts minimally.  So at least there is the restriction /
>>> communication about the buffer size.  But it merely means the
>>> *maximum* buffer size is set.  Application may request still any
>>> shorter value than that.
>>>
>>> And, there are no restriction about period sizes (except for the
>>> periods_max, which is calculated from buffer_bytes_max).
>>> That is, application may request any size between them; and it expects
>>> the wake up by this value.
>>>
>>> I think that's a still missing stone in the design.
>> Well, so what would a real HW driver do in that case?
>> My understanding is that in this case SW can still request
>> something that HW can't do and driver will reject such configurations.
>> In my case, the role of that HW driver code which judges on if configuration
>> is acceptable just runs on the backend side, e.g. frontend driver is just
>> a proxy which talks to the backend to check if the backend can do what
>> requested.
>> And it is up to backend to decide.
>>
>> Does that sound reasonable or you have something else on your mind?
> Usually the hardware driver knows already the restrictions and sets up
> the rules via hw constraints at open callback.  There are lots of
> snd_pcm_hw_constraint_*() helpers (and the relevant ones) to give more
> additional rules for the parameter restrictions.  For example, if the
> periods must be aligned with the buffer size (i.e. buffer_size %
> period_size == 0) as on many devices, you can call like:
>    snd_pcm_hw_constraint_integer(substream->runtime,
> 	SNDRV_PCM_HW_PARAM_PERIODS);
> in the open callback.
You are right, I saw those in other drivers
> And, now an open question for XEN comes: what kind of restriction
> should be applied to the frontend.  Obviously it depends on the
> backend, so there must be some communication, and the restriction must
> be propagated at open, i.e. *before* actually hw_params is performed.
Could you please give me a hint of what those restrictions
could look like? E.g. map of supported buffer/period sizes, what else?
>
> thanks,
>
> Takashi
Thank you,
Oleksandr
>>> thanks,
>>>
>>> Takashi
>> Thank you,
>> Oleksandr
>>>>> thanks,
>>>>>
>>>>> Takashi
>>>>>
>>>>>
>>>>>> Waiting for your valuable comments,
>>>>>>
>>>>>> Thank you,
>>>>>> Oleksandr
>>>>>>
>>>>>> [1] https://github.com/andr2000/linux/commits/snd_upstream_v1
>>>>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/xen/interface/io/sndif.h
>>>>>> [3] https://github.com/xen-troops/snd_be
>>>>>> [4] https://github.com/xen-troops/libxenbe
>>>>>> [5] https://lkml.org/lkml/2017/8/7/363
>>>>>> [6] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123617.html
>>>>>> [7] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123744.html
>>>>>>
>>>>>>
>>>>>> Oleksandr Andrushchenko (2):
>>>>>>      sndif: introduce protocol version
>>>>>>      sndif: add explicit back and front synchronization
>>>>>>
>>>>>>     xen/include/public/io/sndif.h | 173 +++++++++++++++++++++++++++++++++++++++++-
>>>>>>     1 file changed, 170 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> -- 
>>>>>> 2.7.4
>>>>>>
>>>>>> _______________________________________________
>>>>>> Alsa-devel mailing list
>>>>>> Alsa-devel@alsa-project.org
>>>>>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>>> [1]
>>>> https://github.com/andr2000/linux/commits/tiwai_sound_for_next_pv_snd_upstream_v1
>>>> [2]
>>>> https://github.com/andr2000/linux/blob/tiwai_sound_for_next_pv_snd_upstream_v1/sound/xen/xen_snd_front_cfg.c#L239
>>>> [3] https://www.mail-archive.com/xen-devel@lists.xen.org/msg124356.html
>>>>

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel