At Tue, 28 Apr 2009 13:30:09 +0200 (CEST), Jaroslav Kysela wrote: > > On Tue, 28 Apr 2009, Takashi Iwai wrote: > > > At Tue, 28 Apr 2009 12:15:25 +0200 (CEST), > > Jaroslav Kysela wrote: > >> > >> On Tue, 28 Apr 2009, Takashi Iwai wrote: > >> > >>> Well, I think it'd be better, at this moment for 2.6.30, to allow > >>> problematic drivers to skip the jiffies check. Adding the fundamental > >>> change at this late stage is bad, especially if the change wasn't > >>> tested much by many others and has an influence on user-side API. > >>> > >>> So, as I proposed, we can simply add the pcm info check, and add > >>> BATCH flag to needed drivers. Of course, it still requires some > >>> more testing, but basically it'll take back to the old behavior and > >>> should be safer. > >>> > >>> Meanwhile, applying the delay account patch now for 2.6.31 should be > >>> good (ealier is better). It doesn't conflict with the info flag > >>> check, so we can work parallel. > >> > >> Could we just add the runtime->delay variable without touching other code > >> (pcm status), to correct the jiffies based check now? > > > > That's possible, of course. > > > >> It seems like a good > >> step forward to me and the lowlevel drivers will be more prepared for next > >> PCM midlevel code changes. > > > > Well, but I guess providing the proper FIFO size isn't trivial to each > > driver. There are many devices, as Mark suggested, which may be > > broken right now, and we can't determine all h/w specs and test them. > > > > OTOH, adding INFO_BATCH is fairly easy, because it can be found from > > the pointer callback implementation of each driver. On these devices, > > the jiffies check doesn't help much anyway because it cannot update > > the hwptr any other than the period size. So, skipping jiffies check > > is logically correct as a regression fix. > > I don't see much difference between adding INFO_BATCH and adding > runtime->delay = (runtime->period_size)-1 to avoid jiffies check for one > period to these drivers (with some notice to author of the driver to > correct this value). INFO_BATCH is the correct flag for such drivers. They should have the flag, independent from the fix. Also, setting -1 to runtime->delay is not what intended for the delay accounting. Starting from a (sort of) abuse doesn't sound good... > INFO_BATCH is currently in: > > /soc/fsl/mpc5200_psc_i2s.c > /soc/au1x/dbdma2.c > /soc/sh/dma-sh7760.c > /usb/usbaudio.c (which works with the current jiffies check at least for > 44100+ rates) > > Another (much cleaner) possibility is to make jiffies check conditional to > xrun_check() for 2.6.30 and do not bother with the INFO_BATCH flag. > > Jiffies check helped me to debug HDA/intel8x0 pointer problems but cannot > be triggered until driver is really buggy - the HDA/intel8x0 driver should > be fixed now (I know about one intel8x0 pointer problem to this time which > triggers this check, but there is still something wrong with returned > hardware pointer because audio clicks anyway - no regression from previous > state). That makes sense, too. Maybe even safer. Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
