[alsa-devel] es1938 - patch trying to improve capture hw pointer reads

[Takashi Iwai]

At Fri, 25 Jan 2008 01:25:14 +0100,
Hermann Lauer wrote:
> 
> On Thu, Jan 24, 2008 at 04:40:18PM +0100, Takashi Iwai wrote:
> > At Thu, 24 Jan 2008 16:21:02 +0100,
> > Hermann Lauer wrote:
> > > 
> > > On Thu, Jan 24, 2008 at 12:47:46PM +0100, Takashi Iwai wrote:
> > > ...
> > > > Thanks for the patch.
> > > > The change look OK to me.  Another idea would be to use DMAADDR only
> > > > when DMACOUNT goes mad.
> > > 
> > > The problem is to decide when you have read garbage from DMACOUNT.
> > > DMAADDR is better in the first place, because you know the range
> > > in 24 bit which is valid (DMACOUNT is 16bit only). But my patch tries
> > > to make the failure probability much more lower by reading and comparing
> > > both, so from which one you calculate the pointer at the end is only 
> > > a cosmetic issue.
> > 
> > Then a question arises - why DMAADDR isn't used as the primary source
> > at all?  Is it less stable than DMACOUNT in some cases?
> 
> I don't know and the current implementation contains only a "don't ask why - AB"
> comment.
> 
> To reiterate: My proposed patch is about minimising the chance that a
> wrong hw pointer value is given back due to garbage read from the chip. If
> you are interested, I can send you debugging output which shows
> that both DMAADDR and DMACOUNT are read out as bogus values
> from time to time - one or the another, or both. I tried some
> status flags on the chip, but alway found cases where it's
> not clear if and which one of the two is correct.

Yeah, it would be helpful to understand the problem more correctly.

> So reading both of them and comparing them seems to be the
> best way for me to detect that there has been a readout error.
> Which one to give back is without having more information
> about the chip internals in my opinion only a matter of taste.
> The proposed patch allows a jitter of one byte there, but that
> could be set to zero at the cost of a slightly increased
> failure rate.

BTW, don't get me wrong - I'm not against your proposal.  The back-off
is a safe option, as already mentioned.

> > > > But the back-off to the last pointer is
> > > > propbably a safer solution.
> > > 
> > > I'm glad to hear the alsa design allow's this trick - or is
> > > that even the recommeded way to deal with when the hardware pointer
> > > could be temporarily not determined ?
> > 
> > Well, not really.  In theory, it works even if the driver doesn't
> > provide the accurate position.  But, the driver must provide the
> > position in the current period, i.e. the accuracy (latency) must be
> > smaller than the period size.  Thus, it's more safer to keep tracking
> > the current period index (e.g. incrementing at each period-update irq)
> > and check whether the last ptr is over it.
> 
> Fortunately I have not seen any signs of read errors during the interrupt
> processing. Probably this would change if the interrupt processing latency
> is high.

Probably.  In the other case, if some other device blocks the shared
irq line, then the irq handle gets sloppy.  In such a case, the sound
driver gets an irq for the update of two periods instead of one.

> > For example, suppose the buffer = 256 x 4 periods.  The last ptr is
> > recorded at 256.  Now, at the real position 512, an irq is issued.
> > The irq handler calls snd_pcm_period_elapsed(), which calls the
> > pointer callback in the PCM core.  If the read error occurs at this
> > moment, we can't back up to the last position 256.  We are supposed to
> > be at position 512 or more.
> 
> Just to make that clear to me: If the irq handler calls
> snd_pcm_period_elapsed() in your example, the alsa middle layer will treat
> the first 256 byte as written completely by the chip even
> if the pointer callback will say there is still one frame not stored
> for that period.
> (I see the name of the function answering my qestion, but are
> curious how the middle layer implementation behaves in this situation)

Yes.  The return value from the pointer callback is the most trusted
information the PCM core receives.

> That can play a role with the handling of the off-by-one bug of the chip.

What is the problem exactly?


Thanks,

Takashi