At Mon, 16 Jul 2007 11:24:45 +0200, I wrote:

At Sat, 14 Jul 2007 17:55:52 +0200, Pavel Kysilka wrote:

...

Hi,

attached patch fix resume on thinkpad notebooks with cs46xx chip. Tested with Thinkpad A21m.

Patch is opposite latest linus git 2.6.22.

Bug #1946 may be closed.

Signed-off-by: Pavel Kysilka pavelk@bsys.cz

Oh, thanks for hunting this long-standing bug!

I think, however, it cannot be applied as it is. First, snd_cs46xx_start_dsp() calls cs46xx_dsp_load_module(), and the latter function isn't designed for the PM resume. We'd need a special resume function just for clearing area & reloading the dsp images according to the already loaded DSP modules.

Second, __devinit prefix for snd_cs46xx_start_dsp() has to be removed if it's called from the PM handler.

Looking at the code more deeply, I suspect whether it really works when CONFIG_SND_CS46XX_NEW_DSP=y. snd_cs46xx_start_dsp() involves with many calls that add SCBs and co, which will eventually overflow.

I made a patch blindly to do the resume work in a saner way, but of course, it's totally untested. Could you check whether it works?

thanks,

Takashi

diff -r 67544a207969 include/cs46xx.h --- a/include/cs46xx.h Tue Jul 17 11:52:24 2007 +0200 +++ b/include/cs46xx.h Tue Jul 17 12:49:37 2007 +0200 @@ -1723,6 +1723,10 @@ struct snd_cs46xx { struct snd_cs46xx_pcm *playback_pcm; unsigned int play_ctl; #endif + +#ifdef CONFIG_PM + u32 *saved_regs; +#endif };

int snd_cs46xx_create(struct snd_card *card, diff -r 67544a207969 include/cs46xx_dsp_spos.h --- a/include/cs46xx_dsp_spos.h Tue Jul 17 11:52:24 2007 +0200 +++ b/include/cs46xx_dsp_spos.h Tue Jul 17 12:49:37 2007 +0200 @@ -107,6 +107,7 @@ struct dsp_scb_descriptor { char scb_name[DSP_MAX_SCB_NAME]; u32 address; int index; + u32 *data;

struct dsp_scb_descriptor * sub_list_ptr; struct dsp_scb_descriptor * next_scb_ptr; @@ -127,6 +128,7 @@ struct dsp_task_descriptor { int size; u32 address; int index; + u32 *data; };

struct dsp_pcm_channel_descriptor { diff -r 67544a207969 pci/cs46xx/cs46xx_lib.c --- a/pci/cs46xx/cs46xx_lib.c Tue Jul 17 11:52:24 2007 +0200 +++ b/pci/cs46xx/cs46xx_lib.c Tue Jul 17 12:49:37 2007 +0200 @@ -2897,6 +2897,10 @@ static int snd_cs46xx_free(struct snd_cs } #endif +#ifdef CONFIG_PM + kfree(chip->saved_regs); +#endif + pci_disable_device(chip->pci); kfree(chip); return 0; @@ -3140,6 +3144,23 @@ static int snd_cs46xx_chip_init(struct s /* * start and load DSP */ + +static void cs46xx_enable_stream_irqs(struct snd_cs46xx *chip) +{ + unsigned int tmp; + + snd_cs46xx_pokeBA0(chip, BA0_HICR, HICR_IEV | HICR_CHGM); + + tmp = snd_cs46xx_peek(chip, BA1_PFIE); + tmp &= ~0x0000f03f; + snd_cs46xx_poke(chip, BA1_PFIE, tmp); /* playback interrupt enable */ + + tmp = snd_cs46xx_peek(chip, BA1_CIE); + tmp &= ~0x0000003f; + tmp |= 0x00000001; + snd_cs46xx_poke(chip, BA1_CIE, tmp); /* capture interrupt enable */ +} + int __devinit snd_cs46xx_start_dsp(struct snd_cs46xx *chip) { unsigned int tmp; @@ -3214,19 +3235,7 @@ int __devinit snd_cs46xx_start_dsp(struc

snd_cs46xx_proc_start(chip);

- /* - * Enable interrupts on the part. - */ - snd_cs46xx_pokeBA0(chip, BA0_HICR, HICR_IEV | HICR_CHGM); - - tmp = snd_cs46xx_peek(chip, BA1_PFIE); - tmp &= ~0x0000f03f; - snd_cs46xx_poke(chip, BA1_PFIE, tmp); /* playback interrupt enable */ - - tmp = snd_cs46xx_peek(chip, BA1_CIE); - tmp &= ~0x0000003f; - tmp |= 0x00000001; - snd_cs46xx_poke(chip, BA1_CIE, tmp); /* capture interrupt enable */ + cs46xx_enable_stream_irqs(chip); #ifndef CONFIG_SND_CS46XX_NEW_DSP /* set the attenuation to 0dB */ @@ -3665,11 +3674,19 @@ static struct cs_card_type __devinitdata * APM support */ #ifdef CONFIG_PM +static unsigned int saved_regs[] = { + BA0_ACOSV, + BA0_ASER_FADDR, + BA0_ASER_MASTER, + BA1_PVOL, + BA1_CVOL, +}; + int snd_cs46xx_suspend(struct pci_dev *pci, pm_message_t state) { struct snd_card *card = pci_get_drvdata(pci); struct snd_cs46xx *chip = card->private_data; - int amp_saved; + int i, amp_saved;

snd_power_change_state(card, SNDRV_CTL_POWER_D3hot); chip->in_suspend = 1; @@ -3679,6 +3696,10 @@ int snd_cs46xx_suspend(struct pci_dev *p

snd_ac97_suspend(chip->ac97[CS46XX_PRIMARY_CODEC_INDEX]); snd_ac97_suspend(chip->ac97[CS46XX_SECONDARY_CODEC_INDEX]); + + /* save some registers */ + for (i = 0; i < ARRAY_SIZE(saved_regs); i++) + chip->saved_regs[i] = snd_cs46xx_peekBA0(chip, saved_regs[i]);

amp_saved = chip->amplifier; /* turn off amp */ @@ -3698,7 +3719,7 @@ int snd_cs46xx_resume(struct pci_dev *pc { struct snd_card *card = pci_get_drvdata(pci); struct snd_cs46xx *chip = card->private_data; - int amp_saved; + int i, amp_saved;

pci_set_power_state(pci, PCI_D0); pci_restore_state(pci); @@ -3715,6 +3736,16 @@ int snd_cs46xx_resume(struct pci_dev *pc chip->active_ctrl(chip, 1); /* force to on */

snd_cs46xx_chip_init(chip); + + snd_cs46xx_reset(chip); +#ifdef CONFIG_SND_CS46XX_NEW_DSP + cs46xx_dsp_resume(chip); + /* restore some registers */ + for (i = 0; i < ARRAY_SIZE(saved_regs); i++) + snd_cs46xx_pokeBA0(chip, saved_regs[i], chip->saved_regs[i]); +#else + snd_cs46xx_download_image(chip); +#endif

#if 0 snd_cs46xx_codec_write(chip, BA0_AC97_GENERAL_PURPOSE, @@ -3729,6 +3760,13 @@ int snd_cs46xx_resume(struct pci_dev *pc

snd_ac97_resume(chip->ac97[CS46XX_PRIMARY_CODEC_INDEX]); snd_ac97_resume(chip->ac97[CS46XX_SECONDARY_CODEC_INDEX]); + + /* reset playback/capture */ + snd_cs46xx_set_play_sample_rate(chip, 8000); + snd_cs46xx_set_capture_sample_rate(chip, 8000); + snd_cs46xx_proc_start(chip); + + cs46xx_enable_stream_irqs(chip);

if (amp_saved) chip->amplifier_ctrl(chip, 1); /* turn amp on */ @@ -3896,6 +3934,15 @@ int __devinit snd_cs46xx_create(struct s snd_cs46xx_proc_init(card, chip);

+#ifdef CONFIG_PM + chip->saved_regs = kmalloc(sizeof(*chip->saved_regs) * + ARRAY_SIZE(saved_regs), GFP_KERNEL); + if (!chip->saved_regs) { + snd_cs46xx_free(chip); + return -ENOMEM; + } +#endif + chip->active_ctrl(chip, -1); /* disable CLKRUN */

snd_card_set_dev(card, &pci->dev); diff -r 67544a207969 pci/cs46xx/cs46xx_lib.h --- a/pci/cs46xx/cs46xx_lib.h Tue Jul 17 11:52:24 2007 +0200 +++ b/pci/cs46xx/cs46xx_lib.h Tue Jul 17 12:49:37 2007 +0200 @@ -86,6 +86,9 @@ struct dsp_spos_instance *cs46xx_dsp_spo struct dsp_spos_instance *cs46xx_dsp_spos_create (struct snd_cs46xx * chip); void cs46xx_dsp_spos_destroy (struct snd_cs46xx * chip); int cs46xx_dsp_load_module (struct snd_cs46xx * chip, struct dsp_module_desc * module); +#ifdef CONFIG_PM +int cs46xx_dsp_resume(struct snd_cs46xx * chip); +#endif struct dsp_symbol_entry *cs46xx_dsp_lookup_symbol (struct snd_cs46xx * chip, char * symbol_name, int symbol_type); #ifdef CONFIG_PROC_FS diff -r 67544a207969 pci/cs46xx/dsp_spos.c --- a/pci/cs46xx/dsp_spos.c Tue Jul 17 11:52:24 2007 +0200 +++ b/pci/cs46xx/dsp_spos.c Tue Jul 17 12:49:37 2007 +0200 @@ -306,13 +306,59 @@ void cs46xx_dsp_spos_destroy (struct sn mutex_unlock(&chip->spos_mutex); }

+static int dsp_load_parameter(struct snd_cs46xx *chip, + struct dsp_segment_desc *parameter) +{ + u32 doffset, dsize; + + if (!parameter) { + snd_printdd("dsp_spos: module got no parameter segment\n"); + return 0; + } + + doffset = (parameter->offset * 4 + DSP_PARAMETER_BYTE_OFFSET); + dsize = parameter->size * 4; + + snd_printdd("dsp_spos: " + "downloading parameter data to chip (%08x-%08x)\n", + doffset,doffset + dsize); + if (snd_cs46xx_download (chip, parameter->data, doffset, dsize)) { + snd_printk(KERN_ERR "dsp_spos: " + "failed to download parameter data to DSP\n"); + return -EINVAL; + } + return 0; +} + +static int dsp_load_sample(struct snd_cs46xx *chip, + struct dsp_segment_desc *sample) +{ + u32 doffset, dsize; + + if (!sample) { + snd_printdd("dsp_spos: module got no sample segment\n"); + return 0; + } + + doffset = (sample->offset * 4 + DSP_SAMPLE_BYTE_OFFSET); + dsize = sample->size * 4; + + snd_printdd("dsp_spos: downloading sample data to chip (%08x-%08x)\n", + doffset,doffset + dsize); + + if (snd_cs46xx_download (chip,sample->data,doffset,dsize)) { + snd_printk(KERN_ERR "dsp_spos: failed to sample data to DSP\n"); + return -EINVAL; + } + return 0; +} + int cs46xx_dsp_load_module (struct snd_cs46xx * chip, struct dsp_module_desc * module) { struct dsp_spos_instance * ins = chip->dsp_spos_instance; struct dsp_segment_desc * code = get_segment_desc (module,SEGTYPE_SP_PROGRAM); - struct dsp_segment_desc * parameter = get_segment_desc (module,SEGTYPE_SP_PARAMETER); - struct dsp_segment_desc * sample = get_segment_desc (module,SEGTYPE_SP_SAMPLE); u32 doffset, dsize; + int err;

if (ins->nmodules == DSP_MAX_MODULES - 1) { snd_printk(KERN_ERR "dsp_spos: to many modules loaded into DSP\n"); @@ -326,49 +372,20 @@ int cs46xx_dsp_load_module (struct snd_c snd_cs46xx_clear_BA1(chip, DSP_PARAMETER_BYTE_OFFSET, DSP_PARAMETER_BYTE_SIZE); }

- if (parameter == NULL) { - snd_printdd("dsp_spos: module got no parameter segment\n"); - } else { - if (ins->nmodules > 0) { - snd_printk(KERN_WARNING "dsp_spos: WARNING current parameter data may be overwriten!\n"); - } - - doffset = (parameter->offset * 4 + DSP_PARAMETER_BYTE_OFFSET); - dsize = parameter->size * 4; - - snd_printdd("dsp_spos: downloading parameter data to chip (%08x-%08x)\n", - doffset,doffset + dsize); - - if (snd_cs46xx_download (chip, parameter->data, doffset, dsize)) { - snd_printk(KERN_ERR "dsp_spos: failed to download parameter data to DSP\n"); - return -EINVAL; - } - } + err = dsp_load_parameter(chip, get_segment_desc(module, + SEGTYPE_SP_PARAMETER)); + if (err < 0) + return err;

if (ins->nmodules == 0) { snd_printdd("dsp_spos: clearing sample area\n"); snd_cs46xx_clear_BA1(chip, DSP_SAMPLE_BYTE_OFFSET, DSP_SAMPLE_BYTE_SIZE); }

- if (sample == NULL) { - snd_printdd("dsp_spos: module got no sample segment\n"); - } else { - if (ins->nmodules > 0) { - snd_printk(KERN_WARNING "dsp_spos: WARNING current sample data may be overwriten\n"); - } - - doffset = (sample->offset * 4 + DSP_SAMPLE_BYTE_OFFSET); - dsize = sample->size * 4; - - snd_printdd("dsp_spos: downloading sample data to chip (%08x-%08x)\n", - doffset,doffset + dsize); - - if (snd_cs46xx_download (chip,sample->data,doffset,dsize)) { - snd_printk(KERN_ERR "dsp_spos: failed to sample data to DSP\n"); - return -EINVAL; - } - } - + err = dsp_load_sample(chip, get_segment_desc(module, + SEGTYPE_SP_SAMPLE)); + if (err < 0) + return err;

if (ins->nmodules == 0) { snd_printdd("dsp_spos: clearing code area\n"); @@ -986,7 +1003,10 @@ _map_task_tree (struct snd_cs46xx *chip, return NULL; }

- strcpy(ins->tasks[ins->ntask].task_name,name); + if (name) + strcpy(ins->tasks[ins->ntask].task_name, name); + else + strcpy(ins->tasks[ins->ntask].task_name, "(NULL)"); ins->tasks[ins->ntask].address = dest; ins->tasks[ins->ntask].size = size;

@@ -995,7 +1015,8 @@ _map_task_tree (struct snd_cs46xx *chip, desc = (ins->tasks + ins->ntask); ins->ntask++;

- add_symbol (chip,name,dest,SYMBOL_PARAMETER); + if (name) + add_symbol (chip,name,dest,SYMBOL_PARAMETER); return desc; }

@@ -1006,6 +1027,7 @@ cs46xx_dsp_create_scb (struct snd_cs46xx

desc = _map_scb (chip,name,dest); if (desc) { + desc->data = scb_data; _dsp_create_scb(chip,scb_data,dest); } else { snd_printk(KERN_ERR "dsp_spos: failed to map SCB\n"); @@ -1023,6 +1045,7 @@ cs46xx_dsp_create_task_tree (struct snd_

desc = _map_task_tree (chip,name,dest,size); if (desc) { + desc->data = task_data; _dsp_create_task_tree(chip,task_data,dest,size); } else { snd_printk(KERN_ERR "dsp_spos: failed to map TASK\n"); @@ -1320,8 +1343,10 @@ int cs46xx_dsp_scb_and_task_init (struct 0x0000ffff };

- /* dirty hack ... */ - _dsp_create_task_tree (chip,(u32 *)&mix2_ostream_spb,WRITE_BACK_SPB,2); + if (!cs46xx_dsp_create_task_tree(chip, NULL, + (u32 *)&mix2_ostream_spb, + WRITE_BACK_SPB, 2)) + goto _fail_end; }

/* input sample converter */ @@ -1622,7 +1647,6 @@ static int cs46xx_dsp_async_init (struct return 0; }

- static void cs46xx_dsp_disable_spdif_hw (struct snd_cs46xx *chip) { struct dsp_spos_instance * ins = chip->dsp_spos_instance; @@ -1894,3 +1918,61 @@ int cs46xx_dsp_set_iec958_volume (struct

return 0; } + +#ifdef CONFIG_PM +int cs46xx_dsp_resume(struct snd_cs46xx * chip) +{ + struct dsp_spos_instance * ins = chip->dsp_spos_instance; + int i, err; + + /* clear parameter, sample and code areas */ + snd_cs46xx_clear_BA1(chip, DSP_PARAMETER_BYTE_OFFSET, + DSP_PARAMETER_BYTE_SIZE); + snd_cs46xx_clear_BA1(chip, DSP_SAMPLE_BYTE_OFFSET, + DSP_SAMPLE_BYTE_SIZE); + snd_cs46xx_clear_BA1(chip, DSP_CODE_BYTE_OFFSET, DSP_CODE_BYTE_SIZE); + + for (i = 0; i < ins->nmodules; i++) { + struct dsp_module_desc *module = &ins->modules[i]; + struct dsp_segment_desc *seg; + u32 doffset, dsize; + + seg = get_segment_desc(module, SEGTYPE_SP_PARAMETER); + err = dsp_load_parameter(chip, seg); + if (err < 0) + return err; + + seg = get_segment_desc(module, SEGTYPE_SP_SAMPLE); + err = dsp_load_sample(chip, seg); + if (err < 0) + return err; + + seg = get_segment_desc(module, SEGTYPE_SP_PROGRAM); + if (!seg) + continue; + + doffset = seg->offset * 4 + module->load_address * 4 + + DSP_CODE_BYTE_OFFSET; + dsize = seg->size * 4; + err = snd_cs46xx_download(chip, + ins->code.data + module->load_address, + doffset, dsize); + if (err < 0) + return err; + } + + for (i = 0; i < ins->ntask; i++) { + struct dsp_task_descriptor *t = &ins->tasks[i]; + _dsp_create_task_tree(chip, t->data, t->address, t->size); + } + + for (i = 0; i < ins->nscb; i++) { + struct dsp_scb_descriptor *s = &ins->scbs[i]; + if (s->deleted) + continue; + _dsp_create_scb(chip, s->data, s->address); + } + + return 0; +} +#endif