Re: [PATCH v3 0/3] introduce device_buffer

[Takashi Iwai <tiwai@suse.de>]

At Fri, 31 Aug 2012 20:23:18 +0530,
Vinod Koul wrote:
> 
> On Fri, 2012-08-31 at 12:41 +0200, Jaroslav Kysela wrote:
> > Date 31.8.2012 03:14, Vinod Koul wrote:
> > > Old wine in a new bottle with added flavour now.
> > > Based on our discussions in LPC, now we will report the combined delay value as
> > > sum of the buffering done in driver and hardware. This is used to report delay
> > 
> > 
> > > Additionally we tell pcm what is buffer in device/driver, this is used to check
> > > for error calulcations
> > 
> > I'm afraid, but this won't work for the mmaped appl_ptr / hw_ptr,
> > because the checks are done outside the kernel - in alsa-lib. I believe
> > that this API should be enhanced, too. But the question is, how to pass
> > two values (hwptr, device_buffer) atomically to the user space without
> > locks for x86.
> Yes for mmap case, tis needs to be done in lib and that can't be done
> without ABI/API changes??
> Is there a reserved field we can perhaps use for this

We've discussed about this issue at Plumbers in the last week, and
found out that yet different solutions would be needed:

- The problem is found in two places, the XRUN check and the DRAINING
  check.  We need to handle both cases.

- In XRUN check, when hw_ptr == appl_ptr, it's really an XRUN _unless_
  the PCM state is DRAINING.  This is no matter whether the device has
  a device DMA buffer.  The reason is that the hardware is supposed to
  fetch the new data at this point.  Thus, it would fetch the wrong
  data if we check hw_ptr + device_buffer as proposed.

- If it's in DRAINING state, the trigger STOP should be issued not at
  the point hw_ptr == appl_ptr but at the adjusted position like this
  proposal.

Then yet another question came up: what is the exact definition of
runtime->delay.  If this is supposed to be the time of the on-flight
samples in the hardware device buffer (but not FIFO that can't be
controlled by trigger on/off), we'd just need to apply runtime->delay
for DRAINING position check.  No need for the new additional field.

OTOH, if runtime->delay is the total delay including the hardware
device buffer _and_ the hardware FIFO, a new field would be needed for
the check above.  Note that FIFO in this context is a kind of FIFO
things that can't be stopped by the DMA controller.  This is supposed
to be small in usual cases, but not guaranteed to be so.  For example,
in ASoC abstraction, FIFO can be meant to be the delay in the codec
side.  This might be miliseconds long.


Takashi