[alsa-devel] [PATCH v4 0/5] ALSA: jack: Refactoring for jack kctls

[Jie, Yang]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Wednesday, April 08, 2015 12:07 AM
> To: Jie, Yang
> Cc: alsa-devel at alsa-project.org; broonie at kernel.org; Girdwood, Liam R
> Subject: Re: [alsa-devel] [PATCH v4 0/5] ALSA: jack: Refactoring for jack kctls
> 
> At Fri,  3 Apr 2015 20:07:13 +0800,
> Jie Yang wrote:
> >
> > Currently only hda will create kctls for hda jacks, for asoc, people
> > may need create jack kctls in specific driver.
> >
> > Here we are introducing kctls for each jack, by creating kctls and add
> > them to jack controls list (considering exist of combo jack). At the
> > same time, we will report events for each control in the list.
> >
> > With this new implementation, both HDA and soc jack kctls works fine,
> > they are compatible for old HDA jack kctls, too.
> >
> > For soc,
> > a. snd_jack_new() with NULL jack_kctl; b. for each pin, call
> > snd_jack_add_kctls() to add one kctl(it will call snd_ctl_add() to add
> > kctl to card);
> 
> So, if I understand correctly, there is no association between input jack and
> ctl jack names at all?  The former is created via
> snd_jack_new() while the latter is created from each pin name string.

Yes, I finally thought out this solution, it provide enough flexibility for jack kctl
Naming -- leave it to ASoC specific driver, each machine driver can define
its jacks and kctl names for each jack(via jack pins name). 

> 
> > For hda,
> > a. call snd_jack_kctl_new()(in __snd_hda_jack_add_kctl()) to new a
> > jack_kctl(also new kctl); and use snd_hda_ctl_add to add kctl to card;
> > b. new a hda_jack_tbl, appoint jack_tbl->jack_kctl = jack_kctl; c. for
> > non-phantom jack, call snd_jack_new() with this jack_kctl, then the
> > jack_kctl will be added to the new created jack;
> 
> This would work, yes.  But, I have some uneasy feeling, something not well
> digested...
 
I thought out this solution, aiming to change the exist HDA jack kctl
logic smallest, which may be safest for compatibility.

And I tested on some HDA platforms, the created kctls are totally same with
those generated by the existed upstream code.

And, the worst case I can imagine, is that we will fallback: don't change any 
HDA related code(not apply patch 3/5: ALSA: hda - Update to use the new
jack kctls method), then HDA input jack keep totally same with before.

> 
> Ideally, we want a single API for representing both input and kctl jacks.  Now,
> with this implementation, essentially it's still two API calls -- snd_jack_new()
> for input jacks and snd_jack_kctl_new() for kctl jacks.  (Actually oxygen
 
For jack creating, we use the same API -- snd_jack_new();

For kctl creating, yes, we use different APIs:
snd_jack_kctl_new() for input jacks(HDA),
snd_jack_add_kctls() for kctl jacks(ASoC).

There are 2 reasons that I made them different:
1. a. for HDA phantom jack, in the old/exist logic, __snd_hda_jack_add_kctl()
will also call snd_kctl_jack_new() and snd_hda_ctl_add(), it will create kctls
and add them to card(also assigning some arrays, they are different with calling
snd_ctl_add() only, which is what we will do for ASoC kctl adding);
b. for HDA input/phantom jack, all of those occurs before calling snd_jack_new().

So, we have to split jack new and kctl new functions here, because for HDA
phantom/input jack and ASoC kctl jack, they are quite different here.

2. we may need generate kctl name for ASoC kctls(snd_jack_kctl_name_gen(), 
removing suffix " Jack" as you proposed before, or everything needed after);
but for HDA input jack kctls, the naming has been finished before calling
__snd_hda_jack_add_kctl(), we should not change them anymore.

> driver still seems lacking kctl jack because of this.)  The obvious difference
> from the current code is that there is an internal association from kctls to
> input-jack object (although the relation is invisible in outside).  Hmm...

Yes, in my solution, we provide two jack kctls creating method:
1. create kctl  --> [created jack with this kctl] --> [add more kctls to the jack]
...
2. create a jack --> [add kctls to the jack]...

HDA input jack use the 1st method, and ASoC kctl jack use the 2nd method.
And, this is only my suggested usage for HDA and ASoC now, we can change
it anytime needed. What the most important is, we provided these two
methods and enough flexibility...

thanks,
~Keyon

> 
> 
> thanks,
> 
> Takashi
> 
> > Changes in v4:
> > 1. use snd_ctl_find_id() to get avaliable index; 2. add initial kctl
> > for snd_jack_new(compatible for HDA); 3. add struct snd_jack_kctl *
> > field to struct hda_jack_tbl; 4. new kctls for soc jack during jack
> > pins creating.
> > 5. add a patch to remove exporting snd_kctl_jack_new().
> >
> > Changes in v3:
> > 1. replace bit index with bit mask in jack_kctl; 2. add exception for
> > SND_JACK_HEADSET and SND_JACK_AVOUT, only create one jack kctl for
> > these two combo jacks, respectively.
> > 3. add NULL check, mem kfree, and fix some potential risk.
> >
> > Change in v2:
> > 1. define jack_kctl struct, and put jack kctl related stuff there; 2.
> > add a patch to remove the existing controls for HDA jack.
> >
> > Jie Yang (5):
> >   ALSA: jack: create jack kcontrols for every jack input
> >   ALSA: jack: add a parameter to pass kctl for snd_jack_new
> >   ALSA: hda - Update to use the new jack kctls method
> >   ASoC: jack: create kctls according to jack pins info
> >   ALSA: jack: remove export snd_kctl_jack_new()
> >
> >  include/sound/jack.h            |  25 ++++++-
> >  sound/core/ctljack.c            |   1 -
> >  sound/core/jack.c               | 152
> +++++++++++++++++++++++++++++++++++++++-
> >  sound/pci/hda/hda_jack.c        |  52 +++++++-------
> >  sound/pci/hda/hda_jack.h        |   4 +-
> >  sound/pci/oxygen/xonar_wm87x6.c |   2 +-
> >  sound/soc/soc-jack.c            |   3 +-
> >  7 files changed, 200 insertions(+), 39 deletions(-)
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel at alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >