Hi,
recently Pierre questioned about the validity of COMBO positioning mode, and it turned out that the position reporting can be handled better if we take the difference between LPIB and DMAPOS as runtime->delay. (In short, LPIB points the position where the data fed to the codec, and DMAPOS points the data fetched to DMA.)
We cooked up a test patch below and it seems working well on a few machines I've tested. I think we might have still chance to put this into 3.7, as this would be a good improvement. The patch below is a combined work of two fixes: fixing the better stream start by always using SSYNC bit, and adjusting the position and runtime->delay.
Of course, the biggest question is the test coverage. David, could you check whether this doesn't break so much on (some) machines you can test? Of course, if any other people can test it, it'll be a great help, too.
The patch should be applied to the latest sound.git tree master branch, but it should be applicable (and easily resolvable) for older versions. The patch extends this new mode only for recent Intel chips, just to be sure.
thanks,
Takashi
--- diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 66fefd1..b1d99d0 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -555,6 +555,7 @@ enum { #define AZX_DCAPS_ALIGN_BUFSIZE (1 << 22) /* buffer size alignment */ #define AZX_DCAPS_4K_BDLE_BOUNDARY (1 << 23) /* BDLE in 4k boundary */ #define AZX_DCAPS_POSFIX_COMBO (1 << 24) /* Use COMBO as default */ +#define AZX_DCAPS_COUNT_LPIB_DELAY (1 << 25) /* Take LPIB as delay */
/* quirks for ATI SB / AMD Hudson */ #define AZX_DCAPS_PRESET_ATI_SB \ @@ -2003,14 +2004,14 @@ static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd) }
spin_lock(&chip->reg_lock); - if (nsync > 1) { - /* first, set SYNC bits of corresponding streams */ - if (chip->driver_caps & AZX_DCAPS_OLD_SSYNC) - azx_writel(chip, OLD_SSYNC, - azx_readl(chip, OLD_SSYNC) | sbits); - else - azx_writel(chip, SSYNC, azx_readl(chip, SSYNC) | sbits); - } + + /* first, set SYNC bits of corresponding streams */ + if (chip->driver_caps & AZX_DCAPS_OLD_SSYNC) + azx_writel(chip, OLD_SSYNC, + azx_readl(chip, OLD_SSYNC) | sbits); + else + azx_writel(chip, SSYNC, azx_readl(chip, SSYNC) | sbits); + snd_pcm_group_for_each_entry(s, substream) { if (s->pcm->card != substream->pcm->card) continue; @@ -2028,8 +2029,6 @@ static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd) } spin_unlock(&chip->reg_lock); if (start) { - if (nsync == 1) - return 0; /* wait until all FIFOs get ready */ for (timeout = 5000; timeout; timeout--) { nwait = 0; @@ -2062,16 +2061,14 @@ static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd) cpu_relax(); } } - if (nsync > 1) { - spin_lock(&chip->reg_lock); - /* reset SYNC bits */ - if (chip->driver_caps & AZX_DCAPS_OLD_SSYNC) - azx_writel(chip, OLD_SSYNC, - azx_readl(chip, OLD_SSYNC) & ~sbits); - else - azx_writel(chip, SSYNC, azx_readl(chip, SSYNC) & ~sbits); - spin_unlock(&chip->reg_lock); - } + spin_lock(&chip->reg_lock); + /* reset SYNC bits */ + if (chip->driver_caps & AZX_DCAPS_OLD_SSYNC) + azx_writel(chip, OLD_SSYNC, + azx_readl(chip, OLD_SSYNC) & ~sbits); + else + azx_writel(chip, SSYNC, azx_readl(chip, SSYNC) & ~sbits); + spin_unlock(&chip->reg_lock); return 0; }
@@ -2164,6 +2161,28 @@ static unsigned int azx_get_position(struct azx *chip,
if (pos >= azx_dev->bufsize) pos = 0; + + /* calculate runtime delay from LPIB */ + if (azx_dev->substream->runtime && + chip->position_fix[stream] == POS_FIX_POSBUF && + (chip->driver_caps & AZX_DCAPS_COUNT_LPIB_DELAY)) { + unsigned int lpib_pos = azx_sd_readl(azx_dev, SD_LPIB); + int delay; + if (stream == SNDRV_PCM_STREAM_PLAYBACK) + delay = pos - lpib_pos; + else + delay = lpib_pos - pos; + if (delay < 0) + delay += azx_dev->bufsize; + if (delay >= azx_dev->period_bytes) { + snd_printdd("delay %d > period_bytes %d\n", + delay, azx_dev->period_bytes); + delay = 0; /* something is wrong */ + } + azx_dev->substream->runtime->delay = + bytes_to_frames(azx_dev->substream->runtime, delay); + } + return pos; }
@@ -3511,34 +3530,34 @@ static DEFINE_PCI_DEVICE_TABLE(azx_ids) = { /* CPT */ { PCI_DEVICE(0x8086, 0x1c20), .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_COMBO }, + AZX_DCAPS_BUFSIZE | AZX_DCAPS_COUNT_LPIB_DELAY }, /* PBG */ { PCI_DEVICE(0x8086, 0x1d20), .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE}, + AZX_DCAPS_BUFSIZE | AZX_DCAPS_COUNT_LPIB_DELAY }, /* Panther Point */ { PCI_DEVICE(0x8086, 0x1e20), .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_COMBO }, + AZX_DCAPS_BUFSIZE | AZX_DCAPS_COUNT_LPIB_DELAY }, /* Lynx Point */ { PCI_DEVICE(0x8086, 0x8c20), .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_COMBO }, + AZX_DCAPS_BUFSIZE | AZX_DCAPS_COUNT_LPIB_DELAY }, /* Lynx Point-LP */ { PCI_DEVICE(0x8086, 0x9c20), .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_COMBO }, + AZX_DCAPS_BUFSIZE | AZX_DCAPS_COUNT_LPIB_DELAY }, /* Lynx Point-LP */ { PCI_DEVICE(0x8086, 0x9c21), .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_COMBO }, + AZX_DCAPS_BUFSIZE | AZX_DCAPS_COUNT_LPIB_DELAY }, /* Haswell */ { PCI_DEVICE(0x8086, 0x0c0c), .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_COMBO }, + AZX_DCAPS_BUFSIZE | AZX_DCAPS_COUNT_LPIB_DELAY }, { PCI_DEVICE(0x8086, 0x0d0c), .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP | - AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_COMBO }, + AZX_DCAPS_BUFSIZE | AZX_DCAPS_COUNT_LPIB_DELAY }, /* SCH */ { PCI_DEVICE(0x8086, 0x811b), .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP | @@ -3575,7 +3594,8 @@ static DEFINE_PCI_DEVICE_TABLE(azx_ids) = { { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_ANY_ID), .class = PCI_CLASS_MULTIMEDIA_HD_AUDIO << 8, .class_mask = 0xffffff, - .driver_data = AZX_DRIVER_ICH | AZX_DCAPS_BUFSIZE }, + .driver_data = AZX_DRIVER_ICH | + AZX_DCAPS_BUFSIZE | AZX_DCAPS_COUNT_LPIB_DELAY }, /* ATI SB 450/600/700/800/900 */ { PCI_DEVICE(0x1002, 0x437b), .driver_data = AZX_DRIVER_ATI | AZX_DCAPS_PRESET_ATI_SB },