| From 9ad593f6d326e7a4664e3856520f6c042f82a37f Mon Sep 17 00:00:00 2001 |
| From: Takashi Iwai <tiwai@suse.de> |
| Date: Fri, 16 May 2008 12:34:47 +0200 |
| Subject: ALSA: hda - Fix DMA position inaccuracy |
| |
| From: Takashi Iwai <tiwai@suse.de> |
| |
| commit 9ad593f6d326e7a4664e3856520f6c042f82a37f upstream |
| |
| Many HD-audio controllers seem inaccurate about the IRQ timing of |
| PCM period updates. This has caused problems on audio quality; e.g. |
| JACK doesn't work with two periods. |
| |
| This patch fixes the problem by checking the current DMA position |
| at IRQ handler and delays the period-update via a workq if it's |
| inaccurate. |
| |
| Signed-off-by: Takashi Iwai <tiwai@suse.de> |
| Signed-off-by: Jaroslav Kysela <perex@perex.cz> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| sound/pci/hda/hda_intel.c | 124 +++++++++++++++++++++++++++++++++++++++------- |
| 1 file changed, 106 insertions(+), 18 deletions(-) |
| |
| --- a/sound/pci/hda/hda_intel.c |
| +++ b/sound/pci/hda/hda_intel.c |
| @@ -285,6 +285,7 @@ struct azx_dev { |
| u32 *posbuf; /* position buffer pointer */ |
| |
| unsigned int bufsize; /* size of the play buffer in bytes */ |
| + unsigned int period_bytes; /* size of the period in bytes */ |
| unsigned int frags; /* number for period in the play buffer */ |
| unsigned int fifo_size; /* FIFO size */ |
| |
| @@ -301,11 +302,10 @@ struct azx_dev { |
| */ |
| unsigned char stream_tag; /* assigned stream */ |
| unsigned char index; /* stream index */ |
| - /* for sanity check of position buffer */ |
| - unsigned int period_intr; |
| |
| unsigned int opened :1; |
| unsigned int running :1; |
| + unsigned int irq_pending: 1; |
| }; |
| |
| /* CORB/RIRB */ |
| @@ -369,6 +369,9 @@ struct azx { |
| |
| /* for debugging */ |
| unsigned int last_cmd; /* last issued command (to sync) */ |
| + |
| + /* for pending irqs */ |
| + struct work_struct irq_pending_work; |
| }; |
| |
| /* driver types */ |
| @@ -908,6 +911,8 @@ static void azx_init_pci(struct azx *chi |
| } |
| |
| |
| +static int azx_position_ok(struct azx *chip, struct azx_dev *azx_dev); |
| + |
| /* |
| * interrupt handler |
| */ |
| @@ -930,11 +935,18 @@ static irqreturn_t azx_interrupt(int irq |
| azx_dev = &chip->azx_dev[i]; |
| if (status & azx_dev->sd_int_sta_mask) { |
| azx_sd_writeb(azx_dev, SD_STS, SD_INT_MASK); |
| - if (azx_dev->substream && azx_dev->running) { |
| - azx_dev->period_intr++; |
| + if (!azx_dev->substream || !azx_dev->running) |
| + continue; |
| + /* check whether this IRQ is really acceptable */ |
| + if (azx_position_ok(chip, azx_dev)) { |
| + azx_dev->irq_pending = 0; |
| spin_unlock(&chip->reg_lock); |
| snd_pcm_period_elapsed(azx_dev->substream); |
| spin_lock(&chip->reg_lock); |
| + } else { |
| + /* bogus IRQ, process it later */ |
| + azx_dev->irq_pending = 1; |
| + schedule_work(&chip->irq_pending_work); |
| } |
| } |
| } |
| @@ -973,6 +985,7 @@ static int azx_setup_periods(struct snd_ |
| azx_sd_writel(azx_dev, SD_BDLPU, 0); |
| |
| period_bytes = snd_pcm_lib_period_bytes(substream); |
| + azx_dev->period_bytes = period_bytes; |
| periods = azx_dev->bufsize / period_bytes; |
| |
| /* program the initial BDL entries */ |
| @@ -1421,27 +1434,16 @@ static int azx_pcm_trigger(struct snd_pc |
| return 0; |
| } |
| |
| -static snd_pcm_uframes_t azx_pcm_pointer(struct snd_pcm_substream *substream) |
| +static unsigned int azx_get_position(struct azx *chip, |
| + struct azx_dev *azx_dev) |
| { |
| - struct azx_pcm *apcm = snd_pcm_substream_chip(substream); |
| - struct azx *chip = apcm->chip; |
| - struct azx_dev *azx_dev = get_azx_dev(substream); |
| unsigned int pos; |
| |
| if (chip->position_fix == POS_FIX_POSBUF || |
| chip->position_fix == POS_FIX_AUTO) { |
| /* use the position buffer */ |
| pos = le32_to_cpu(*azx_dev->posbuf); |
| - if (chip->position_fix == POS_FIX_AUTO && |
| - azx_dev->period_intr == 1 && !pos) { |
| - printk(KERN_WARNING |
| - "hda-intel: Invalid position buffer, " |
| - "using LPIB read method instead.\n"); |
| - chip->position_fix = POS_FIX_NONE; |
| - goto read_lpib; |
| - } |
| } else { |
| - read_lpib: |
| /* read LPIB */ |
| pos = azx_sd_readl(azx_dev, SD_LPIB); |
| if (chip->position_fix == POS_FIX_FIFO) |
| @@ -1449,7 +1451,90 @@ static snd_pcm_uframes_t azx_pcm_pointer |
| } |
| if (pos >= azx_dev->bufsize) |
| pos = 0; |
| - return bytes_to_frames(substream->runtime, pos); |
| + return pos; |
| +} |
| + |
| +static snd_pcm_uframes_t azx_pcm_pointer(struct snd_pcm_substream *substream) |
| +{ |
| + struct azx_pcm *apcm = snd_pcm_substream_chip(substream); |
| + struct azx *chip = apcm->chip; |
| + struct azx_dev *azx_dev = get_azx_dev(substream); |
| + return bytes_to_frames(substream->runtime, |
| + azx_get_position(chip, azx_dev)); |
| +} |
| + |
| +/* |
| + * Check whether the current DMA position is acceptable for updating |
| + * periods. Returns non-zero if it's OK. |
| + * |
| + * Many HD-audio controllers appear pretty inaccurate about |
| + * the update-IRQ timing. The IRQ is issued before actually the |
| + * data is processed. So, we need to process it afterwords in a |
| + * workqueue. |
| + */ |
| +static int azx_position_ok(struct azx *chip, struct azx_dev *azx_dev) |
| +{ |
| + unsigned int pos; |
| + |
| + pos = azx_get_position(chip, azx_dev); |
| + if (chip->position_fix == POS_FIX_AUTO) { |
| + if (!pos) { |
| + printk(KERN_WARNING |
| + "hda-intel: Invalid position buffer, " |
| + "using LPIB read method instead.\n"); |
| + chip->position_fix = POS_FIX_NONE; |
| + pos = azx_get_position(chip, azx_dev); |
| + } else |
| + chip->position_fix = POS_FIX_POSBUF; |
| + } |
| + |
| + if (pos % azx_dev->period_bytes > azx_dev->period_bytes / 2) |
| + return 0; /* NG - it's below the period boundary */ |
| + return 1; /* OK, it's fine */ |
| +} |
| + |
| +/* |
| + * The work for pending PCM period updates. |
| + */ |
| +static void azx_irq_pending_work(struct work_struct *work) |
| +{ |
| + struct azx *chip = container_of(work, struct azx, irq_pending_work); |
| + int i, pending; |
| + |
| + for (;;) { |
| + pending = 0; |
| + spin_lock_irq(&chip->reg_lock); |
| + for (i = 0; i < chip->num_streams; i++) { |
| + struct azx_dev *azx_dev = &chip->azx_dev[i]; |
| + if (!azx_dev->irq_pending || |
| + !azx_dev->substream || |
| + !azx_dev->running) |
| + continue; |
| + if (azx_position_ok(chip, azx_dev)) { |
| + azx_dev->irq_pending = 0; |
| + spin_unlock(&chip->reg_lock); |
| + snd_pcm_period_elapsed(azx_dev->substream); |
| + spin_lock(&chip->reg_lock); |
| + } else |
| + pending++; |
| + } |
| + spin_unlock_irq(&chip->reg_lock); |
| + if (!pending) |
| + return; |
| + cond_resched(); |
| + } |
| +} |
| + |
| +/* clear irq_pending flags and assure no on-going workq */ |
| +static void azx_clear_irq_pending(struct azx *chip) |
| +{ |
| + int i; |
| + |
| + spin_lock_irq(&chip->reg_lock); |
| + for (i = 0; i < chip->num_streams; i++) |
| + chip->azx_dev[i].irq_pending = 0; |
| + spin_unlock_irq(&chip->reg_lock); |
| + flush_scheduled_work(); |
| } |
| |
| static struct snd_pcm_ops azx_pcm_ops = { |
| @@ -1676,6 +1761,7 @@ static int azx_suspend(struct pci_dev *p |
| int i; |
| |
| snd_power_change_state(card, SNDRV_CTL_POWER_D3hot); |
| + azx_clear_irq_pending(chip); |
| for (i = 0; i < AZX_MAX_PCMS; i++) |
| snd_pcm_suspend_all(chip->pcm[i]); |
| if (chip->initialized) |
| @@ -1732,6 +1818,7 @@ static int azx_free(struct azx *chip) |
| int i; |
| |
| if (chip->initialized) { |
| + azx_clear_irq_pending(chip); |
| for (i = 0; i < chip->num_streams; i++) |
| azx_stream_stop(chip, &chip->azx_dev[i]); |
| azx_stop_chip(chip); |
| @@ -1857,6 +1944,7 @@ static int __devinit azx_create(struct s |
| chip->irq = -1; |
| chip->driver_type = driver_type; |
| chip->msi = enable_msi; |
| + INIT_WORK(&chip->irq_pending_work, azx_irq_pending_work); |
| |
| chip->position_fix = check_position_fix(chip, position_fix[dev]); |
| check_probe_mask(chip, dev); |