Re: [PATCH] STAC92xx : add support for missing features

Re: [PATCH] STAC92xx : add support for missing features

[Takashi Iwai]

At Tue, 28 Aug 2007 21:41:32 +0300,
Maxim Levitsky wrote:
> 
> > > About 3 (volume-knob), we'd need a bit more checks and tests.  Does
> > > the volume knob widget work always as a mater control regardless
> > > whetehr the real hardware component exists or not?  I.e. can we use it
> > > blindly?
> According to datasheet, it works, also driver already assumes it there, and sets it to maximum volume
> see hda_verb stac922x_core_init, hda_verb d965_core_init, hda_verb stac927x_core_init,hda_verb stac9205_core_init

Interesting.  So, writing to the volume knob widget actually works
just like a normal mixer amp?

> > > 
> > > Tobin, Matt, could you test these patches, especially patch 3, with
> > > your hardwares?  Anyone who has STAC92xx hardwares are welcome for
> > > testing it, too!
> > > 
> > > 
> > > Thanks,
> > > 
> > > Takashi
> > 
> 
> Hi, 
> 
> Thanks for review, I will fix all things you pointed out, but
> I am coding against latest linus' kernel tree, and snd_ctl_boolean_mono_info() isn't there, and thus I didn't use it, 
> I will download latest alsa tree, and compile against it
> 
> But I have a question: can I only complile snd-hda-intel from that tree, and leave kernel's sound core modules?

If you build from alsa-driver tree, then yes, only modules.

You can get use kernel.git perex/alsa.git mm branch for the latest
ALSA codes, too:
	http://git.kernel.org/?p=linux/kernel/git/perex/alsa.git;a=shortlog;h=mm


thanks,

Takashi

Re: [PATCH] STAC92xx : add support for missing features

[Maxim Levitsky]

2007/8/29, Takashi Iwai <tiwai@suse.de>:
> At Tue, 28 Aug 2007 21:41:32 +0300,
> Maxim Levitsky wrote:
> >
> > > > About 3 (volume-knob), we'd need a bit more checks and tests.  Does
> > > > the volume knob widget work always as a mater control regardless
> > > > whetehr the real hardware component exists or not?  I.e. can we use it
> > > > blindly?
> > According to datasheet, it works, also driver already assumes it there, and sets it to maximum volume
> > see hda_verb stac922x_core_init, hda_verb d965_core_init, hda_verb stac927x_core_init,hda_verb stac9205_core_init
>
> Interesting.  So, writing to the volume knob widget actually works
> just like a normal mixer amp?
Yes, but volumeknob doesn't support standard verbs that amps do, thus
I coded it as a custom control, but generally it is a amp
>
> > > >
> > > > Tobin, Matt, could you test these patches, especially patch 3, with
> > > > your hardwares?  Anyone who has STAC92xx hardwares are welcome for
> > > > testing it, too!
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > Takashi
> > >
> >
> > Hi,
> >
> > Thanks for review, I will fix all things you pointed out, but
> > I am coding against latest linus' kernel tree, and snd_ctl_boolean_mono_info() isn't there, and thus I didn't use it,
> > I will download latest alsa tree, and compile against it
> >
> > But I have a question: can I only complile snd-hda-intel from that tree, and leave kernel's sound core modules?
>
> If you build from alsa-driver tree, then yes, only modules.
>
> You can get use kernel.git perex/alsa.git mm branch for the latest
> ALSA codes, too:
>         http://git.kernel.org/?p=linux/kernel/git/perex/alsa.git;a=shortlog;h=mm
>
>
> thanks,
>
> Takashi
>


Hi,
  Thanks for information, I will send updated patches soon.

Best regards,
       Maxim Levitsky

Re: [PATCH 1/5] HDA : patch_sigmatel.c : cleanups

[Takashi Iwai]

At Tue, 28 Aug 2007 12:46:01 +0200,
I wrote:
> 
> At Tue, 28 Aug 2007 05:07:48 +0300,
> Maxim Levitsky wrote:
> > 
> > From c9392b0293f7d39e40e4a5e07e7b140d5a385079 Mon Sep 17 00:00:00 2001
> > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > Date: Mon, 27 Aug 2007 22:49:49 +0300
> > Subject: [PATCH] HDA : patch_sigmatel.c : cleanups
> 
> OK, it's a nice clean-up.  Can be merged as is.

Now merged to ALSA HG tree.

If the fixes for other patches are ready, please let me know.
I'll merge them to HG tree, too.


Thanks,

Takashi

Re: [PATCH 1/5] HDA : patch_sigmatel.c : cleanups

[Maxim Levitsky]

On Friday 31 August 2007 13:56:19 Takashi Iwai wrote:
> At Tue, 28 Aug 2007 12:46:01 +0200,
> I wrote:
> > 
> > At Tue, 28 Aug 2007 05:07:48 +0300,
> > Maxim Levitsky wrote:
> > > 
> > > From c9392b0293f7d39e40e4a5e07e7b140d5a385079 Mon Sep 17 00:00:00 2001
> > > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > > Date: Mon, 27 Aug 2007 22:49:49 +0300
> > > Subject: [PATCH] HDA : patch_sigmatel.c : cleanups
> > 
> > OK, it's a nice clean-up.  Can be merged as is.
> 
> Now merged to ALSA HG tree.
> 
> If the fixes for other patches are ready, please let me know.
> I'll merge them to HG tree, too.
> 
> 
> Thanks,
> 
> Takashi
> 


Hi,

I run into unexpected problems:
latest hg adds a power saving mode to hda codec, and while the idea is nice, it plays very bad with my patches.

1) The analog loop-back becomes a nightmare, since power-saving code turns the codec off, but for loopback to function it has to be powered.
I tried to add a snd_hda_power_up/down calls to .put function, and while it did stop the unwanted powerdown of codec, on resume from ram , analog loopback vanishes.
any access to device (arecord/aplay or any mixer change make analog loop work again).
Can you tell me how I properly tell the core that user _uses_ the device, and device should be powered.

2) on resume all playback apps hang, and recording (arecord) exits with message "Resource temporary unavailable"
restarting apps help.

Looking for your comments,
	Best regards,
		Maxim Levitsky

Re: [PATCH 1/5] HDA : patch_sigmatel.c : cleanups

[Takashi Iwai]

At Sat, 1 Sep 2007 11:11:24 +0300,
Maxim Levitsky wrote:
> 
> On Friday 31 August 2007 13:56:19 Takashi Iwai wrote:
> > At Tue, 28 Aug 2007 12:46:01 +0200,
> > I wrote:
> > > 
> > > At Tue, 28 Aug 2007 05:07:48 +0300,
> > > Maxim Levitsky wrote:
> > > > 
> > > > From c9392b0293f7d39e40e4a5e07e7b140d5a385079 Mon Sep 17 00:00:00 2001
> > > > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > > > Date: Mon, 27 Aug 2007 22:49:49 +0300
> > > > Subject: [PATCH] HDA : patch_sigmatel.c : cleanups
> > > 
> > > OK, it's a nice clean-up.  Can be merged as is.
> > 
> > Now merged to ALSA HG tree.
> > 
> > If the fixes for other patches are ready, please let me know.
> > I'll merge them to HG tree, too.
> > 
> > 
> > Thanks,
> > 
> > Takashi
> > 
> 
> 
> Hi,
> 
> I run into unexpected problems:
> latest hg adds a power saving mode to hda codec, and while the idea is nice, it plays very bad with my patches.
> 
> 1) The analog loop-back becomes a nightmare, since power-saving code turns the codec off, but for loopback to function it has to be powered.
> I tried to add a snd_hda_power_up/down calls to .put function, and while it did stop the unwanted powerdown of codec, on resume from ram , analog loopback vanishes.
> any access to device (arecord/aplay or any mixer change make analog loop work again).
> Can you tell me how I properly tell the core that user _uses_ the device, and device should be powered.
> 
> 2) on resume all playback apps hang, and recording (arecord) exits with message "Resource temporary unavailable"
> restarting apps help.

First, you need to use snd_hda_codec_write_cache() for values to be
resumed, especially in control put callbacks.  The amp values accessed
via snd_hda_codec_amp_update() are always cached, but
snd_hda_codec_write() doesn't cache.

The second thing is analog-loopback.  The driver should keep the power
when the analog-loopback is on, and enables power-saving again when
it's off.  So far, I thought only the amp controls, but now it's a
different one.

In this case, the put callback would need to call snd_hda_pwoer_up()
and snd_hda_power_down() appropriately to turn on/off power-save.
They should be called only at transition.  Namely, when 0->1
transition of spec->aloopback occurs in put callback, it calls
snd_hda_power_up().  When 1->0 occurs, call snd_hda_power_down().


Takashi

Re: [PATCH 1/5] HDA : patch_sigmatel.c : cleanups

[Maxim Levitsky]

On Saturday 01 September 2007 14:49:29 Takashi Iwai wrote:
> At Sat, 1 Sep 2007 11:11:24 +0300,
> Maxim Levitsky wrote:
> > 
> > On Friday 31 August 2007 13:56:19 Takashi Iwai wrote:
> > > At Tue, 28 Aug 2007 12:46:01 +0200,
> > > I wrote:
> > > > 
> > > > At Tue, 28 Aug 2007 05:07:48 +0300,
> > > > Maxim Levitsky wrote:
> > > > > 
> > > > > From c9392b0293f7d39e40e4a5e07e7b140d5a385079 Mon Sep 17 00:00:00 2001
> > > > > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > > > > Date: Mon, 27 Aug 2007 22:49:49 +0300
> > > > > Subject: [PATCH] HDA : patch_sigmatel.c : cleanups
> > > > 
> > > > OK, it's a nice clean-up.  Can be merged as is.
> > > 
> > > Now merged to ALSA HG tree.
> > > 
> > > If the fixes for other patches are ready, please let me know.
> > > I'll merge them to HG tree, too.
> > > 
> > > 
> > > Thanks,
> > > 
> > > Takashi
> > > 
> > 
> > 
> > Hi,
> > 
> > I run into unexpected problems:
> > latest hg adds a power saving mode to hda codec, and while the idea is nice, it plays very bad with my patches.
> > 
> > 1) The analog loop-back becomes a nightmare, since power-saving code turns the codec off, but for loopback to function it has to be powered.
> > I tried to add a snd_hda_power_up/down calls to .put function, and while it did stop the unwanted powerdown of codec, on resume from ram , analog loopback vanishes.
> > any access to device (arecord/aplay or any mixer change make analog loop work again).
> > Can you tell me how I properly tell the core that user _uses_ the device, and device should be powered.
> > 
> > 2) on resume all playback apps hang, and recording (arecord) exits with message "Resource temporary unavailable"
> > restarting apps help.
> 
> First, you need to use snd_hda_codec_write_cache() for values to be
> resumed, especially in control put callbacks.  The amp values accessed
> via snd_hda_codec_amp_update() are always cached, but
> snd_hda_codec_write() doesn't cache.
> 
> The second thing is analog-loopback.  The driver should keep the power
> when the analog-loopback is on, and enables power-saving again when
> it's off.  So far, I thought only the amp controls, but now it's a
> different one.
> 
> In this case, the put callback would need to call snd_hda_pwoer_up()
> and snd_hda_power_down() appropriately to turn on/off power-save.
> They should be called only at transition.  Namely, when 0->1
> transition of spec->aloopback occurs in put callback, it calls
> snd_hda_power_up().  When 1->0 occurs, call snd_hda_power_down().
> 
> 
> Takashi
> 

Hi,
	Well, did you read my email?  :-)

	I did call snd_hda_power_up/down in .put callback, but as I said, on resume, driver didn't powered up the codec, and as usual any access to device
	powers up the codec.

	Well and I found another bug,
	The problem is that on resume all "use mic/line-in as output' controls are disabled, and I know why:

	.resume() before introduction of snd_hda_codec_write_cache() / dynamic power managment:
		stac92xx_init() -> inits inputs/outputs of chip (sets all dynamic pins to output mode)

		snd_hda_resume_ctls() > hacky, but working way of resuming mixer controls : read from memory, write to device
		

	.resume() now:
		stac92xx_init() -> inits inputs/outputs of chip (sets all dynamic pins to output mode), and touches the command cache.
		snd_hda_codec_resume_cache() writes (modifed) cache to codec, thus making dynamic outputs be in input mode.

	I suggest skipping call to stac92xx_init()  at all, after all no initialization is nessesary, since all commands are in cache.

Best regards,
	Maxim Levitsky

Re: [PATCH 1/5] HDA : patch_sigmatel.c : cleanups

[Takashi Iwai]

At Sun, 2 Sep 2007 11:28:43 +0300,
Maxim Levitsky wrote:
> 
> On Saturday 01 September 2007 14:49:29 Takashi Iwai wrote:
> > At Sat, 1 Sep 2007 11:11:24 +0300,
> > Maxim Levitsky wrote:
> > > 
> > > On Friday 31 August 2007 13:56:19 Takashi Iwai wrote:
> > > > At Tue, 28 Aug 2007 12:46:01 +0200,
> > > > I wrote:
> > > > > 
> > > > > At Tue, 28 Aug 2007 05:07:48 +0300,
> > > > > Maxim Levitsky wrote:
> > > > > > 
> > > > > > From c9392b0293f7d39e40e4a5e07e7b140d5a385079 Mon Sep 17 00:00:00 2001
> > > > > > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > > > > > Date: Mon, 27 Aug 2007 22:49:49 +0300
> > > > > > Subject: [PATCH] HDA : patch_sigmatel.c : cleanups
> > > > > 
> > > > > OK, it's a nice clean-up.  Can be merged as is.
> > > > 
> > > > Now merged to ALSA HG tree.
> > > > 
> > > > If the fixes for other patches are ready, please let me know.
> > > > I'll merge them to HG tree, too.
> > > > 
> > > > 
> > > > Thanks,
> > > > 
> > > > Takashi
> > > > 
> > > 
> > > 
> > > Hi,
> > > 
> > > I run into unexpected problems:
> > > latest hg adds a power saving mode to hda codec, and while the idea is nice, it plays very bad with my patches.
> > > 
> > > 1) The analog loop-back becomes a nightmare, since power-saving code turns the codec off, but for loopback to function it has to be powered.
> > > I tried to add a snd_hda_power_up/down calls to .put function, and while it did stop the unwanted powerdown of codec, on resume from ram , analog loopback vanishes.
> > > any access to device (arecord/aplay or any mixer change make analog loop work again).
> > > Can you tell me how I properly tell the core that user _uses_ the device, and device should be powered.
> > > 
> > > 2) on resume all playback apps hang, and recording (arecord) exits with message "Resource temporary unavailable"
> > > restarting apps help.
> > 
> > First, you need to use snd_hda_codec_write_cache() for values to be
> > resumed, especially in control put callbacks.  The amp values accessed
> > via snd_hda_codec_amp_update() are always cached, but
> > snd_hda_codec_write() doesn't cache.
> > 
> > The second thing is analog-loopback.  The driver should keep the power
> > when the analog-loopback is on, and enables power-saving again when
> > it's off.  So far, I thought only the amp controls, but now it's a
> > different one.
> > 
> > In this case, the put callback would need to call snd_hda_pwoer_up()
> > and snd_hda_power_down() appropriately to turn on/off power-save.
> > They should be called only at transition.  Namely, when 0->1
> > transition of spec->aloopback occurs in put callback, it calls
> > snd_hda_power_up().  When 1->0 occurs, call snd_hda_power_down().
> > 
> > 
> > Takashi
> > 
> 
> Hi,
> 	Well, did you read my email?  :-)

Yeah, but not carefully as I was in travel.

> 	I did call snd_hda_power_up/down in .put callback, but as I said, on resume, driver didn't powered up the codec, and as usual any access to device
> 	powers up the codec.

Well, did you use snd_hda_codec_write_cache() there?
Otherwise the changed analog-loopback setup won't be called.

> 	Well and I found another bug,
> 	The problem is that on resume all "use mic/line-in as output' controls are disabled, and I know why:
> 
> 	.resume() before introduction of snd_hda_codec_write_cache() / dynamic power managment:
> 		stac92xx_init() -> inits inputs/outputs of chip (sets all dynamic pins to output mode)
> 
> 		snd_hda_resume_ctls() > hacky, but working way of resuming mixer controls : read from memory, write to device
> 		
> 
> 	.resume() now:
> 		stac92xx_init() -> inits inputs/outputs of chip (sets all dynamic pins to output mode), and touches the command cache.
> 		snd_hda_codec_resume_cache() writes (modifed) cache to codec, thus making dynamic outputs be in input mode.
> 
> 	I suggest skipping call to stac92xx_init()  at all, after all no initialization is nessesary, since all commands are in cache.

Some stuff in stac92xx_init() isn't cached, so we cannot remove it
completely.  I fixed it now on HG tree.  Give it a try.


Takashi

Re: [PATCH 1/5] HDA : patch_sigmatel.c : cleanups

[Maxim Levitsky]

On Monday 03 September 2007 10:39:29 Takashi Iwai wrote:
> At Sun, 2 Sep 2007 11:28:43 +0300,
> Maxim Levitsky wrote:
> > 
> > On Saturday 01 September 2007 14:49:29 Takashi Iwai wrote:
> > > At Sat, 1 Sep 2007 11:11:24 +0300,
> > > Maxim Levitsky wrote:
> > > > 
> > > > On Friday 31 August 2007 13:56:19 Takashi Iwai wrote:
> > > > > At Tue, 28 Aug 2007 12:46:01 +0200,
> > > > > I wrote:
> > > > > > 
> > > > > > At Tue, 28 Aug 2007 05:07:48 +0300,
> > > > > > Maxim Levitsky wrote:
> > > > > > > 
> > > > > > > From c9392b0293f7d39e40e4a5e07e7b140d5a385079 Mon Sep 17 00:00:00 2001
> > > > > > > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > > > > > > Date: Mon, 27 Aug 2007 22:49:49 +0300
> > > > > > > Subject: [PATCH] HDA : patch_sigmatel.c : cleanups
> > > > > > 
> > > > > > OK, it's a nice clean-up.  Can be merged as is.
> > > > > 
> > > > > Now merged to ALSA HG tree.
> > > > > 
> > > > > If the fixes for other patches are ready, please let me know.
> > > > > I'll merge them to HG tree, too.
> > > > > 
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > Takashi
> > > > > 
> > > > 
> > > > 
> > > > Hi,
> > > > 
> > > > I run into unexpected problems:
> > > > latest hg adds a power saving mode to hda codec, and while the idea is nice, it plays very bad with my patches.
> > > > 
> > > > 1) The analog loop-back becomes a nightmare, since power-saving code turns the codec off, but for loopback to function it has to be powered.
> > > > I tried to add a snd_hda_power_up/down calls to .put function, and while it did stop the unwanted powerdown of codec, on resume from ram , analog loopback vanishes.
> > > > any access to device (arecord/aplay or any mixer change make analog loop work again).
> > > > Can you tell me how I properly tell the core that user _uses_ the device, and device should be powered.
> > > > 
> > > > 2) on resume all playback apps hang, and recording (arecord) exits with message "Resource temporary unavailable"
> > > > restarting apps help.
> > > 
> > > First, you need to use snd_hda_codec_write_cache() for values to be
> > > resumed, especially in control put callbacks.  The amp values accessed
> > > via snd_hda_codec_amp_update() are always cached, but
> > > snd_hda_codec_write() doesn't cache.
> > > 
> > > The second thing is analog-loopback.  The driver should keep the power
> > > when the analog-loopback is on, and enables power-saving again when
> > > it's off.  So far, I thought only the amp controls, but now it's a
> > > different one.
> > > 
> > > In this case, the put callback would need to call snd_hda_pwoer_up()
> > > and snd_hda_power_down() appropriately to turn on/off power-save.
> > > They should be called only at transition.  Namely, when 0->1
> > > transition of spec->aloopback occurs in put callback, it calls
> > > snd_hda_power_up().  When 1->0 occurs, call snd_hda_power_down().
> > > 
> > > 
> > > Takashi
> > > 
> > 
> > Hi,
> > 	Well, did you read my email?  :-)
> 
> Yeah, but not carefully as I was in travel.
> 
> > 	I did call snd_hda_power_up/down in .put callback, but as I said, on resume, driver didn't powered up the codec, and as usual any access to device
> > 	powers up the codec.
> 
> Well, did you use snd_hda_codec_write_cache() there?
> Otherwise the changed analog-loopback setup won't be called.
> 
> > 	Well and I found another bug,
> > 	The problem is that on resume all "use mic/line-in as output' controls are disabled, and I know why:
> > 
> > 	.resume() before introduction of snd_hda_codec_write_cache() / dynamic power managment:
> > 		stac92xx_init() -> inits inputs/outputs of chip (sets all dynamic pins to output mode)
> > 
> > 		snd_hda_resume_ctls() > hacky, but working way of resuming mixer controls : read from memory, write to device
> > 		
> > 
> > 	.resume() now:
> > 		stac92xx_init() -> inits inputs/outputs of chip (sets all dynamic pins to output mode), and touches the command cache.
> > 		snd_hda_codec_resume_cache() writes (modifed) cache to codec, thus making dynamic outputs be in input mode.
> > 
> > 	I suggest skipping call to stac92xx_init()  at all, after all no initialization is nessesary, since all commands are in cache.
> 
> Some stuff in stac92xx_init() isn't cached, so we cannot remove it
> completely.  I fixed it now on HG tree.  Give it a try.
> 
> 
> Takashi
> 

Hi,

Yes , 'Mic/Line-out as ouput' switches work fine now after suspend-resume,
your patch works fine. Thanks.

Best regards,
	Maxim Levitsky