cs46xx thinkpad resume fix - #bug1946

cs46xx thinkpad resume fix - #bug1946

[Pavel Kysilka]

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

   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>

 Pavel Kysilka



________ Information from NOD32 ________
This message was checked by NOD32 Antivirus System for Linux Mail Servers.
http://www.eset.com

[-- Attachment #2: cs46xx_thinkpad_resume_fix.diff --]
[-- Type: text/plain, Size: 441 bytes --]

diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c
index bef1f6d..73f66d7 100644
--- a/sound/pci/cs46xx/cs46xx_lib.c
+++ b/sound/pci/cs46xx/cs46xx_lib.c
@@ -3715,6 +3715,7 @@ int snd_cs46xx_resume(struct pci_dev *pci)
 	chip->active_ctrl(chip, 1); /* force to on */
 
 	snd_cs46xx_chip_init(chip);
+	snd_cs46xx_start_dsp(chip); /* upload dsp code */
 
 #if 0
 	snd_cs46xx_codec_write(chip, BA0_AC97_GENERAL_PURPOSE, 

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: cs46xx thinkpad resume fix - #bug1946

[Takashi Iwai]

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.


Takashi

Re: cs46xx thinkpad resume fix - #bug1946

[Takashi Iwai]

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

Re: cs46xx thinkpad resume fix - #bug1946

[Pavel Kysilka]

On Tue Jul 17, 2007 at 12:50:21PM +0200, Takashi Iwai wrote:

  Hi Takashi,

good news below.

> At Mon, 16 Jul 2007 11:24:45 +0200,
> I wrote:
> > 
> > 
<cut/>
> > 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?

Patch works perfectly.

Tested option:
- kernel compiled with and without CONFIG_SND_CS46XX_NEW_DSP=y
- play pcm stream, suspend, resume and pcm stream is playing
- play pcm stream, stop playing pcm stream, suspend, resume and play pcm stream

If you need log from resume from notebook, I'll send you this log.

> 
> 
> thanks,
> 
> Takashi
> 

 Many thanks
    Pavel


________ Information from NOD32 ________
This message was checked by NOD32 Antivirus System for Linux Mail Servers.
http://www.eset.com

Re: cs46xx thinkpad resume fix - #bug1946

[chris]

Would this also apply to bug #305?

I'm happy to test this on CentOS 5 then Fedora 7 on a 600X, and write up 
the needed steps for ThinkWiki until the patch is included in the 
distributions.

Chris



On Wed, 18 Jul 2007, Pavel Kysilka wrote:

> On Tue Jul 17, 2007 at 12:50:21PM +0200, Takashi Iwai wrote:
>
>  Hi Takashi,
>
> good news below.
>
>> At Mon, 16 Jul 2007 11:24:45 +0200,
>> I wrote:
>>>
>>>
> <cut/>
>>> 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?
>
> Patch works perfectly.
>
> Tested option:
> - kernel compiled with and without CONFIG_SND_CS46XX_NEW_DSP=y
> - play pcm stream, suspend, resume and pcm stream is playing
> - play pcm stream, stop playing pcm stream, suspend, resume and play pcm stream
>
> If you need log from resume from notebook, I'll send you this log.
>
>>
>>
>> thanks,
>>
>> Takashi
>>
>
> Many thanks
>    Pavel
>
>
> ________ Information from NOD32 ________
> This message was checked by NOD32 Antivirus System for Linux Mail Servers.
> http://www.eset.com
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

Re: cs46xx thinkpad resume fix - #bug1946

[Takashi Iwai]

At Wed, 18 Jul 2007 08:51:45 -0500 (CDT),
chris@idlelion.net wrote:
> 
> Would this also apply to bug #305?

Possible.

> I'm happy to test this on CentOS 5 then Fedora 7 on a 600X, and write up 
> the needed steps for ThinkWiki until the patch is included in the 
> distributions.

You can either download the HG repo or take the latest daily snapshot
from
	ftp://ftp.suse.com/pub/projects/alsa/snapshot/driver

I applied the patch today (July 19), so hg20070720 snapshot will
include it.

If the mirror HG repo (hg-mirror.alsa-project.org) is out of sync, you
can try hg.alsa-project.org instead.


Takashi

Re: cs46xx thinkpad resume fix - #bug1946

[Takashi Iwai]

At Wed, 18 Jul 2007 00:26:42 +0200,
Pavel Kysilka wrote:
> 
> On Tue Jul 17, 2007 at 12:50:21PM +0200, Takashi Iwai wrote:
> 
>   Hi Takashi,
> 
> good news below.
> 
> > At Mon, 16 Jul 2007 11:24:45 +0200,
> > I wrote:
> > > 
> > > 
> <cut/>
> > > 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?
> 
> Patch works perfectly.
> 
> Tested option:
> - kernel compiled with and without CONFIG_SND_CS46XX_NEW_DSP=y
> - play pcm stream, suspend, resume and pcm stream is playing
> - play pcm stream, stop playing pcm stream, suspend, resume and play pcm stream
> 
> If you need log from resume from notebook, I'll send you this log.

Thanks for testing!  That's enough, so far.  I applied the patch to HG
tree now.


Takashi