On 7/18/2019 5:24 PM, Pierre-Louis Bossart wrote:
On 7/18/19 3:42 AM, Rajwa, Marcin wrote:
On 7/18/2019 7:06 AM, Keyon Jie wrote:
On 2019/7/18 上午11:35, Pierre-Louis Bossart wrote:
On 7/17/19 10:11 PM, Keyon Jie wrote:
From: Marcin Rajwa <marcin.rajwa@xxxxxxxxxxxxxxx>
In some cases, FW might need to use the host_period_bytes field to
synchronize the DMA copying (with host side) but the driver does not
it's your right to edit my suggested wording, but the notion of
'synchronization' is far from clear. it's my understanding that the
host_period_bytes defines the DMA transfer size requested by the
firmware, which isn't a value that matters to the host except for
rewind usages.
Hi Pierre, here the host_period_bytes is requested by host, FW has
its own period size, and DMA will transfer data in FW buffer period
size. It works like this:
FW buffer[period 0, period 1, ...] <==> DMA <==> host/alsa
buffer[host_period 0, host_priod 1, ...]
We need this host_preiod_bytes information in FW to do fast
draining(e.g. record 2 seconds data within 10ms) in mmap capture, we
are slowing down the draining in smaller host_period_bytes cases,
otherwise, arecord can't read the buffer in time and overrun will
happen.
Maybe the wording "synchronize" here is inaccurate, how about
something like this:
"FW might need to use the host_period_bytes field to configure and
control the DMA copying speed but the driver does not..."
Hi,
we need *host_period_bytes *in FW to properly drain data - by
properly I mean to no override host buffer but in the same time to
avoid situation when host is waiting for data and doesn't get it. The
former is known as *overrun *while the later *underrun**.
*
So that's why I originally used the word *"to synchronize" *because
it best reflects the use of this variable in FW.
*The problem *- host establishes the *period_size/**host_period_bytes
*and uses it as a "data copy tempo controller" meaning it will copy
data buffered in its own buffer (copied there by FW) in *period_size
*time intervals. Now, in regular copy (real time stream) firmware
doesn't
need to care about how fast host copy because the host buffer is big
enough to store even several FW periods (each one or few
milliseconds) without overriding it. That is why we did not need
this *period_size *variable before. However the draining task is very
different ball game - it copies *2,1 seconds* of data as fast as
possible to the host. Therefore we are very prone to *XRUNs*
*The solution* - in FW we need to know how often host copies data out
from its own buffer and this information is stored in
*host_period_bytes, *lets send this information down to firmware.
Now, the FW knowing this can fill the host buffer and wait the time
calculated by *host_period_bytes*
before next copy. This way we copy as much/fast as we can and in the
same time we are safe that host will handle this and no XRUN will
ever happen.
We knew from Day1 that draining faster than real-time could
potentially lead to the driver detecting overflows or timeouts. It's
been documented left and right, with an assumption that the ring
buffer is actually big enough to contain all the data stored in the DSP.
@Pierre, indeed the buffer that kernel allocates is big enough to store
all the data. However *arecord* introduces its own buffer which is just
a multiple of *period_sizes *- and it is the buffer which overflows.
Before I provide more feedback, can you clarify if the
'host_period_bytes' is the same value as the ALSA period size (in
bytes)? And what would be the value when the no_irq mode is used?
Yes, this is the same value. It is obtained by *params_period_bytes**()*
in kernel.
*no_irq mode *- it does not affect the value of *host_period_bytes
*after my patch has been applied. Before my patch however, driver and FW
used this value to send stream position information from FW to kernel.
In short, when *(hda && hda->no_ipc_position)*
then *ipc_params->host_period_bytes = 0; *was set in kernel.**Firmware
then, read this *host_period_bytes = 0 *and treated it as "OK does not
send any IPC regarding stream position". So once *no_ipc_position *was
set we lost information about *host_period_bytes *in the FW.
So all my patches in FW and Kernel do is to *relax****host_period_bytes
*and introduce new parameter dedicated for this stream position IPC. At
that time we called it *no_period_irq *to resemble old name but now I
think it would be better if we rename it to something like
*no_stream_position *as it is more informative. What do you think?
Hope it helped to understand the need of *host_period_bytes *in the
firmware.
**
Marcin
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
