[alsa-devel] I want to improve Sigma-Tel STAC9227 driver / How to implement support for multiple ADC

[Takashi Iwai]

At Thu, 23 Aug 2007 17:09:04 +0300,
Maxim Levitsky wrote:
> 
> On Wednesday 22 August 2007 17:51:47 Takashi Iwai wrote:
> > At Mon, 20 Aug 2007 04:48:52 +0300,
> > Maxim Levitsky wrote:
> > > 
> > > Hello,
> > > 
> > > I am a user of Sigma-Tel STAC9227 intel hda based chip.
> > > I have studied its documentation, and found that alsa driver doesn't support 
> > > number of its features.
> > > 
> > > 
> > > 1) it has "analog loop-back" which means it can mix sound just before it 
> > > enters ADC0 to output of DAC0 (headphones/front).
> > > For me it is the most important feature, since I have a tv card with analog 
> > > output.
> > 
> > I couldn't find it in the datasheet...  Is it just a matter of amp or
> > widget connection?
> 
> Well, I was looking for it for about a month :-)
> 
> Look at AGF node , DACMode verb (FEB/7EB), bit ADCMixDAC 

Ah, OK such a hidden one.  Thanks.


> > > 2) it has the ability to "swap" left/right channels, and this can be very 
> > > useful for LFE/center channel
> > 
> > This would be relatively easy (if we really need).
> > 
> > > 3) It has a VolumeKnob "widget" , that allows to decrease volume of all DACs 
> > > _after_ their own volumes were applied, and I think I turn it to "master 
> > > volume"
> > 
> > The problem is how to know whether this widget is really used or not.
> > (That is, whether the volume knob is implemented in this hardware)
> > 
> I tested it, and it works

Yeah, I suppose it works for you, but the question is whether we can
enable it unconditionally, even for machines without volume knobs.


> > > Those things below aren't supported by windows driver ether, but that doesn't 
> > > mean we can't support that :-)
> > > 
> > > 4) this chip has 3 ADC, each capable recording from different source, but alsa 
> > > uses only first.
> > 
> > OK, that's a bug.
> > 
> > > 5) I have 5.1 configuration, so DAC1 is unused.
> > > It is possible to connect it to "front" mic, and get 7.1 sound card
> > > I think about a option "Use front mic as output", for 3Stack configuration
> > 
> > Hm, but still too tricky to set up in that way.
> > You cannot imagine easily such a switch means 7.1 output.
> Why, we already have Mic as output/Line-in as output,
> so lets add Front mic as output (I already did that)

And, I think the current design is confusing.  I'd love to change
that...


> > For other codecs, we have "Channel Mode" enum.  The pins are set up
> > according to this value.  But, sigmatel code doesn't have this enum
> > list but rather individual switches.  It's slightly confusing, I
> > think.  From the easiness POV, I prefer the channel mode enum
> > solution.
> > 
> > > 1,2,3 are easy to implement, since those are just mixer switches.
> > 
> > Not really.  patch_sigmatel.c is written to be as generic as
> > possible.  Thus, if you add something special for a specific device,
> > you should be careful not to break others.
> I will read datasheets of all other Sigmatels that this driver support
> 
> as for  7.1 support I told, it shouldn't break anything:
> the code checks only in case we have one output (line-out) for DAC connected to mic and line in, 
> if it there it adds them to output pin list.
> Only in that case I will also check if front mic is connected to a DAC, and if so, I will add it to output list too as a side channel
> I already did that , and it work here.

Well, what I meant is that the exact same code is used for other codec
chips, not only STAC9227.  So, we have to be sure that it works for
other codecs, too.  Anyway, the code would speak better, so let's see
a patch :)


> > > But I have a question about 4:
> > > 
> > > Now the driver has buggy support for second ADC, it record from it, but fails 
> > > to connect it to any input source, and unmute it.
> > > 
> > > I can fix that and trivially add support for third ADC, but current driver 
> > > makes all ADCs a sub-streams of a single  PCM device.
> > > 
> > > I think that sub-streams are meant to be used only by playback PCMs that have 
> > > hardware support of mixing,
> > 
> > No, it's misunderstanding :)
> Great, I already fixed support for all ADCs to be a substreams of single PCM
> Just one question:
> 
> This chip has two-level gain on each adc,
> first is a four step boost in 10 db values, and it is currently used in driver,
> And a more fine tuned inPortnVol, that is currently unused.
> 
> I have three options:
> 
> 1) use  inPortnVol for capture volume, and forget about that crude boost,

Then you won't get enough high gain, I suppose?


> 2) impement complex control that will emulate single volume, but will change both
> 3) impement two controls (the easiest thing)
> 
> What shall I do?

Personally I prefer 2 since it's the easiest option for _users_.
Unless the implementation gets really complicated, let's try to
provide a simple interface as much as possible.


> > > so I feel that is is better  to make  each ADC to 
> > > be a separate PCM device, with separate  mixer controls (enabled/input 
> > > source/gain), and I want to ask you whenever you agree with me.
> > 
> > ... thus I disagree.  And, the mixer controls can be provided
> > separately anyway (as other codecs already do).
> > 
> > > Also I noticed that dsnoop chokes on record streams with more that one 
> > > substream, it fails with error, when I try to open second arecord.
> > 
> > Yes, and note that it's the intentional behavior.
> > 
> > The problem is _not_ that the secondary capture is assigned to the
> > secondary substream.  As long as you use the default PCM, it'll be not
> > accessed anyway.  The problem is rather that currently the overall
> > design doesn't suit for the multi-stream environment.
> > 
> > We provide "default" PCM for the generic purpose.  That's fine.  It's
> > a mighty ace that can be used for everything.  Now we have a second
> > guy.  But we have no standard symbolic name for him.  It's possible to
> > access via hw:0,0,1.  It's however ugly, cryptic and even bare.  We
> > need to provide a more understandable symbolic PCM definition with the
> > decoration of plugins.  This is what is missing so far.  Thus it's
> > basically not a driver issue but an alsa-lib issue.
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> 
> 
> Hi,
> 
> 	Thanks for reply, 
> I already started doing the above things, it feels easy to me, soon I will send a patch series 
> that fixed/add features that I told here about.

OK, that's great.  Let's discuss further with a patch.


Thanks,

Takashi