On Wed, Feb 08, 2012 at 09:36:21AM +0100, David Henningsson wrote:

On 02/07/2012 08:48 PM, Mark Brown wrote:

I'd like these to match the names currently used in HDA, like this:

{ SW_MICROPHONE_INSERT, "Mic" },

It seems odd to abbreviate this and nothing else but I'm not that fussed either way.

{ SW_LINEOUT_INSERT, "Line-Out" }, { SW_VIDEOOUT_INSERT, "Video-Out" },

For these it's really not normal English to hyphenate them, it looks strange to do so.

{ SW_LINEIN_INSERT, "Line" },

It seems odd and a bit undescriptive to not specify the description here when it is specified for the outputs?

Actually, it matters less if we settle on the standard you set above, or what the HDA currently does, as long as the names are the same.

Except for Mic where I don't mind either way I'd rather bring the HDA names into (but note that the idea is to remove the HDA specific jack controls - I think these names are used in other places though?).

On Wed, Feb 08, 2012 at 02:35:29PM +0100, David Henningsson wrote:

On 02/08/2012 12:46 PM, Mark Brown wrote:

...

Except for Mic where I don't mind either way I'd rather bring the HDA names into (but note that the idea is to remove the HDA specific jack controls - I think these names are used in other places though?).

I'll let Takashi have the final say about the names. But note that for HDA these names will not be enough, e g, we might have one "Front Mic" and one "Rear Mic" and need to know which one is which.

Remember that once these are merged properly we'll be adding the jack name on the front so the bare names will only exist for one or two commits - doing so sensibly would mean changing the kctl API and doing a refactor on that when the immediate goal is to make it private seems like more trouble than it's worth.

On Fri, Feb 10, 2012 at 11:55:10AM +0100, Takashi Iwai wrote:

OTOH, we can take an optional directional suffix, i.e. "[location] base [direction] [channel]", too. For example, base can be "Video" or "Line", and direction can be "Out" or "In".

These names are going to get pretty verbose very easily. Might it be worth putting some of this (and also other information like colour) into TLV information rather than part of the base jack name? So long as we can extract a name for the jack itself (which is probably the location in your example above) I think we're fine.

I think the main thing is to clean up this mess for 3.4 so we don't get too many confused userspaces out there.

On 02/10/2012 11:55 AM, Takashi Iwai wrote:

At Wed, 08 Feb 2012 14:35:29 +0100, David Henningsson wrote:

...

On 02/08/2012 12:46 PM, Mark Brown wrote:

...

On Wed, Feb 08, 2012 at 09:36:21AM +0100, David Henningsson wrote:

...

On 02/07/2012 08:48 PM, Mark Brown wrote:

...

I'd like these to match the names currently used in HDA, like this:

...

{ SW_MICROPHONE_INSERT, "Mic" },

It seems odd to abbreviate this and nothing else but I'm not that fussed either way.

...

{ SW_LINEOUT_INSERT, "Line-Out" }, { SW_VIDEOOUT_INSERT, "Video-Out" },

For these it's really not normal English to hyphenate them, it looks strange to do so.

I guess it could be easier to parse if we avoid spaces in names,

Exactly, it was the reason.

Given that the normal desktop user does not usually look at these values these days, I'd say being parser friendly weighs heavier than being "normal English".

...

but right now there is no such parser AFAIK anyway, so it does not matter much. Video-Out is not present in HDA; I did that change to make it consistent with "Line-Out".

...

...

{ SW_LINEIN_INSERT, "Line" },

It seems odd and a bit undescriptive to not specify the description here when it is specified for the outputs?

Historically, "Line" represents an line input in ALSA control names. But it wouldn't be bad to an explicit directional notation, too.

...

...

...

Actually, it matters less if we settle on the standard you set above, or what the HDA currently does, as long as the names are the same.

Except for Mic where I don't mind either way I'd rather bring the HDA names into (but note that the idea is to remove the HDA specific jack controls - I think these names are used in other places though?).

I'll let Takashi have the final say about the names. But note that for HDA these names will not be enough, e g, we might have one "Front Mic" and one "Rear Mic" and need to know which one is which.

What I have in my mind is a form like "[location] base [channel]". The location prefix (e.g. Front, Rear) is optional, and also the channel suffix, too.

I have no strong opinion Whether to allow a space in the base name. In my patch, I chose hyphens just to make parsing easier.

OTOH, we can take an optional directional suffix, i.e. "[location] base [direction] [channel]", too. For example, base can be "Video" or "Line", and direction can be "Out" or "In".

I'd like to hear rather comments from others.

I think both naming schemes are good, and I'm not too worried about them being too verbose. If we run into names being longer than the string length we could back off and drop the location, I guess.

If we extend the thought about applying this scheme for mixer control names as well, we might run into controls that apply to two outputs but not the third, and then the complexity will increase, like "Front HP,Line-Out Playback Switch". (And what if the Headphone is located on the Front side and the Line-Out is on the back, etc) But then we run into the entire story of exposing a graph instead, I guess. :-/

Also don't forget about HDA HDMI, where we have four jacks but only one or two being physically connected, and we need to map these against a PCM device.

On 02/10/2012 04:50 PM, Mark Brown wrote:

On Fri, Feb 10, 2012 at 02:08:52PM +0100, David Henningsson wrote:

...

On 02/10/2012 11:55 AM, Takashi Iwai wrote:

...

David Henningsson wrote:

...

...

...

I guess it could be easier to parse if we avoid spaces in names,

Exactly, it was the reason.

Given that the normal desktop user does not usually look at these values these days, I'd say being parser friendly weighs heavier than being "normal English".

Not all the world is desktop.

The normal end user of the embedded system does not look at those values either.

...

...

OTOH, we can take an optional directional suffix, i.e. "[location] base [direction] [channel]", too. For example, base can be "Video" or "Line", and direction can be "Out" or "In".

...

...

I'd like to hear rather comments from others.

...

I think both naming schemes are good, and I'm not too worried about them being too verbose. If we run into names being longer than the string length we could back off and drop the location, I guess.

It's a complete pain for actually working with them and doing development - it renders badly in UIs (think about alsamixer for example, or people looking at things on 80 column terminals) and isn't friendly to people typing things in.

So your suggestion was, to avoid "Front Headphone" and "Rear Headphone" because the names were too long, and instead have "Headphone" and "Headphone,index=1" and have "Front" and "Rear" read out of a TLV?

That will be worse for everyone, both those doing work with them, looking in alsamixer, etc.

At Fri, 10 Feb 2012 16:39:46 +0000, Mark Brown wrote:

On Fri, Feb 10, 2012 at 05:09:35PM +0100, David Henningsson wrote:

...

On 02/10/2012 04:50 PM, Mark Brown wrote:

...

...

...

Given that the normal desktop user does not usually look at these values these days, I'd say being parser friendly weighs heavier than being "normal English".

...

...

Not all the world is desktop.

...

The normal end user of the embedded system does not look at those values either.

There's rather a lot of people who do embedded development and do need to look at this stuff, though. There's a real cost to raising the bar for humans to understand things.

Hm, I'm afraid that this is getting apart from the real worth discussion. I'm interested in whether "Line-Out" is acceptable enough, or it's a matter of taste, just like Marmite is acceptable as the normal English breakfasts.

...

...

...

I think both naming schemes are good, and I'm not too worried about them being too verbose. If we run into names being longer than the string length we could back off and drop the location, I guess.

...

...

It's a complete pain for actually working with them and doing development - it renders badly in UIs (think about alsamixer for example, or people looking at things on 80 column terminals) and isn't friendly to people typing things in.

...

So your suggestion was, to avoid "Front Headphone" and "Rear Headphone" because the names were too long, and instead have "Headphone" and "Headphone,index=1" and have "Front" and "Rear" read out of a TLV?

No, some of the information should go in the name but we keep on coming up with more and more things to add and when we end up with "Left Silver Headset Jack Headphone" or whatever (which would be a fairly complete description of half the headset jack on one of my laptops) we probably ought to be cutting some of that out.

Actually, in my very first version, there was no location prefix or channel suffix but it was only a simple name like "Headphone" with index numbers. Then, I faced a problem when trying to integration with the input-jack layer. The input-jack doesn't provide the index number. So, you can't register the same name correctly. Thus I tried to create a unique name string as input-jack id.

Now, looking back to your implementation, the kctl jack is created with a name string based on the switch type, not from the name string passed to snd_jack_new() itself. Why not passing the string as is? If multiple kctls are needed (e.g. for headset), you can either create a new name or just create the multiple instances with the same name but give extra TLVs to identify which type it is. Or, you can create an enum control so that both the input-jack and the kctl-jack correspond as 1:1.

In other words, if 1:1 mapping isn't needed, we don't have to create a different name string at all. Just create "Jack Detect" controls with different indices and with corresponding TLVs. This would be enough (could be even easier) for a machine parser.

And, it seems that there is no standard rule defined for the id string of input-jack. If 1:1 model is used, we'd need to define this rule.

Though thinking about it duplicating all the individual bits of information in TLVs would be helpful to automatic parsing, we could possibly even do something clever like make minimal subset names that contained only the bits that were unique on a given system if we were bored enough.

BTW, I found another issue. With this implementation, you can't build kjack ctl without input-jack. I can work around it for HD-audio by adding more ifdef, but it'd be ugly...

Oh, by the way the extcon (Android switch class more or less) code looks to be coming along quite well which is going to be a third way of getting jack information out to userspace.

Yeah, we enjoy freedom :)

thanks,

Takashi

At Mon, 13 Feb 2012 07:44:59 -0800, Mark Brown wrote:

On Mon, Feb 13, 2012 at 02:56:55PM +0100, Takashi Iwai wrote:

...

Mark Brown wrote:

...

...

There's rather a lot of people who do embedded development and do need to look at this stuff, though. There's a real cost to raising the bar for humans to understand things.

...

Hm, I'm afraid that this is getting apart from the real worth discussion. I'm interested in whether "Line-Out" is acceptable enough, or it's a matter of taste, just like Marmite is acceptable as the normal English breakfasts.

Well, I dunno - you did suggest adding direction modifiers to the jack names which would both be a more general solution to the problem and clean up the naming issue.

OK, let's postpone about it.

...

with the input-jack layer. The input-jack doesn't provide the index number. So, you can't register the same name correctly. Thus I tried to create a unique name string as input-jack id.

It's really never been the intention to have an "input-jack" layer -

I called it just to distinguish with kctl jack, not intended to stress that it's an input layer.

the API is there to hide all the input layer stuff it can from the individual audio drivers. The only thing that leaks out is the button remapping which I couldn't think of anything more sensible to do with them.

...

Now, looking back to your implementation, the kctl jack is created with a name string based on the switch type, not from the name string passed to snd_jack_new() itself. Why not passing the string as is?

We can't pass it as-is since we'll need multiple controls per jack - literally all the systems I have available to test on would fail if they used the same name as they all have headset jacks. As I said further up the thread my plan had been to refactor the name generation code once the separate kctl code is gone and they can be merged into the same source file which makes this a lot easier.

Makes sense.

With these patches I'm mainly looking for a quick and simple refactoring to get rid of the duplication - once we've got rid of the duplicate driver APIs then we can refactor the internals of the implementation however we like but there's a pretty pressing need to fix the driver API and get all the drivers doing the same thing to userspace even if it's not ideal.

Hm, then I'd say let's hold on. Merging this half-baked stuff is rather confusing at this point since the use of kctl jack isn't targeted yet for ASoC.

I believe what we need _now_ is to decide the policy (i.e. element naming rule and TLV definitions) for kctl jacks quickly.

...

If multiple kctls are needed (e.g. for headset), you can either create a new name or just create the multiple instances with the same name but give extra TLVs to identify which type it is. Or, you can create an enum control so that both the input-jack and the kctl-jack correspond as 1:1.

This stuff is why I've been asking you about the ABI and general semantics for the new controls, I was never sure what the intention was

• when you said they were just plain booleans I just went ahead and

created multiple controls as that seemed to be what the current idiom was.

I don't think an enumeration is going to scale so well given the number of possible combinations you can get on more complex jacks.

OK.

...

In other words, if 1:1 mapping isn't needed, we don't have to create a different name string at all. Just create "Jack Detect" controls with different indices and with corresponding TLVs. This would be enough (could be even easier) for a machine parser.

That'd work I think, though I would create a new control for each physical jack at least just for clarity.

They are indeed individual ctl elements. They just share the same name and are distinguished by the index numbers. For the parser program, it would be even easier because it needs to look only for elements containing the single string. The rest is the parse of TLVs of gathered elements, which would be needed no matter whether we have multiple name strings or not.

...

And, it seems that there is no standard rule defined for the id string of input-jack. If 1:1 model is used, we'd need to define this rule.

The input stack doesn't have the same concept of machine parsable names for things that ALSA does (honestly that's pretty unique to ALSA within the kernel) - the names it users are all free form text intended to be meaningful to humans only.

Yeah, maybe. But, if you have two headset jacks, how would you identify with the current jack stuff...?

...

BTW, I found another issue. With this implementation, you can't build kjack ctl without input-jack. I can work around it for HD-audio by adding more ifdef, but it'd be ugly...

I really don't see this as a problem; it seems vanishingly unlikely that any realistic system would want to disable the input API and need accessory detection. Obviously the end goal here is that you wouldn't be able to have HDA specific hacks in the code as everything will be handled by the ALSA core code...

Yeah, the more code may be moved toward the core once when we decide the standards.

To my mind we should just support everything and let userspace worry about deciding what to use, this isn't the sort of thing which should require a kernel rebuild to configure and it's not like there's any fundamental problem or massive overhead caused by having the devices show up.

That's one option, yes. OTOH, people always want to diet the kernel code. It's a good dream that is hardly to achieve in reality (for human beings, too), though.

So, at this point, I can list the following policies for kctl jacks:

A. all kctls have one name string with different indices. Each jack is distinguished by the TLV provided to each kctl.

B. kctls are built with only a base name ("Headphone", "Mic") with different indices and TLVs.

C. kctls contain unique names ([location] base [direction] [channel]) optionally with TLVs

It'd be clear when thinking some real cases. Assume a headset jack containing a headphone and a mic switch: A: two kctls with TLV JACK_HEADSET but with TYPE_HEADPHONE and TYPE_MIC, respectively. B1: two kctls with the same "Headset" name, with TLV TYPE_HEADPHONE and TYPE_MIC B2: separate to a kctl with "Headphone" and a kctl with "Mic" C1: same as B1 C2: same as B2

When an extra headphone jack is added to the above, A: another kctl with TLV_JACK_HEADPHONE, TYPE_HEADPHONE B1: A kctl "Headphone" with TLV TYPE_HEADPHONE B2: A kctl "Headphone" with index 1 C1: same as B1 C2: A kctl "Rear Headphone" (and rename other "Front Headphone")

Yet more another headphone jack on left, A: another kctl with TLV_JACK_HEADPHONE, TYPE_HEADPHONE, LOC_LEFT B1: A kctl "Headphone" index 1 with TLV TYPE_HEADPHONE, LOC_LEFT B2: A kctl "Headphone" index 2 with LOC_LEFT C1: A kctl "Left Headphone" (rename other "Rear Headphone") C2: A kctl "Left Headphone"

... or does it make things more complicated? :)

thanks,

Takashi

At Mon, 13 Feb 2012 11:23:10 -0800, Mark Brown wrote:

On Mon, Feb 13, 2012 at 06:40:23PM +0100, Takashi Iwai wrote:

...

Mark Brown wrote:

...

...

With these patches I'm mainly looking for a quick and simple refactoring to get rid of the duplication - once we've got rid of the duplicate driver APIs then we can refactor the internals of the implementation however we like but there's a pretty pressing need to fix the driver API and get all the drivers doing the same thing to userspace even if it's not ideal.

...

Hm, then I'd say let's hold on. Merging this half-baked stuff is rather confusing at this point since the use of kctl jack isn't targeted yet for ASoC.

...

I believe what we need _now_ is to decide the policy (i.e. element naming rule and TLV definitions) for kctl jacks quickly.

Now that the kctl jacks are there I'm getting people asking me about it often enough so I'd like to see it merged.

Yeah, if things were easy, I'd be happy to merge. But, judging from the situation, I see no big reason to hurry too much.

...

...

...

In other words, if 1:1 mapping isn't needed, we don't have to create a different name string at all. Just create "Jack Detect" controls with different indices and with corresponding TLVs. This would be enough (could be even easier) for a machine parser.

...

...

That'd work I think, though I would create a new control for each physical jack at least just for clarity.

...

They are indeed individual ctl elements. They just share the same name and are distinguished by the index numbers. For the parser program, it would be even easier because it needs to look only for elements containing the single string. The rest is the parse of TLVs of gathered elements, which would be needed no matter whether we have multiple name strings or not.

I think we're mostly agreeing here - what I'd like is to be able to give each jack a descriptive name which shows up in the control name ("Headset", "Docking Station") along with "Jack" or some other keyword. The descriptive name could be optional, but it seems nice to put it in the control name.

Yes. My concern is then how to map the existing jack API to the new name. Thus I proposed some examples.

...

So, at this point, I can list the following policies for kctl jacks:

...

A. all kctls have one name string with different indices. Each jack is distinguished by the TLV provided to each kctl.

...

B. kctls are built with only a base name ("Headphone", "Mic") with different indices and TLVs.

...

C. kctls contain unique names ([location] base [direction] [channel]) optionally with TLVs

I think I prefer B here with the base name being a descriptive name (which could often be like the above) that should be unique within the card, even if the driver just adds numbers.

... and David wrote:

At Tue, 14 Feb 2012 08:20:50 +0100, David Henningsson wrote:

I would vote for "C2", although I would probably prefer "[location] base [channel]" over "[location] base [direction] [channel]", as direction is superfluous given "base". This is also what the current implementation offers, and what I've based my PulseAudio patches on.

Actually, C > B, thus we may take an approach of C optionally. In both cases, we'd need an id string or such (e.g. "Jack" suffix) to indicate that it's a jack kctl. If this is satisfied, the rest prefix and suffix can be fully optional.

That is, for those who don't need the additional information (yet), just pass the base name. For multiple items, use the multiple indices. This is equivalent with B.

For those who care about the multiple items (e.g. HD-audio), they can pass the more verbose name. It's C.

The rest is the job of the user-space side parser. It'll gather all items with "Jack" suffix, then trim location, direction or channel prefix/suffix. It shouldn't be too hard.

Yes, it's redundant. One can represent it in both ways via TLV or name string. But it works even now (before the TLV definitions), that's the point.

A and B requires additional work as we would need an API in alsa-lib from where you can read and interpret the TLVs, and stuff like amixer needs to understand it too make it reasonable easy to work with and test. And C gives the option to add TLVs later whenever we feel the need for it, so it seems to be an all-win.

Honestly speaking, I think the plan A isn't that bad. The suffix look-up and filtering as mentioned above looks slightly messy. It'd be much simpler in both kernel and user-space implementations. The biggest drawback is that David would need to rewrite his patch to match with this scheme :) And, of course, it's not visible with the current alsa-utils stuff. The verbose name string would be visible by alsactl or whatever even in the current version, yes.

Also; as this graph exposing thing is unlikely to be implemented in all layers of the audio stack any time soon, maybe C2 is also the one that gives the most obvious matching between mixer kcontrols and jack kcontrols? I'd like to move in this direction; not only because we currently do not have the graph, but also because that even if we have it, userspace apps choosing not to implement it will have a good option.

IMO, this graph things should be considered separately at this moment. The name string itself won't be always enough to identify the connected kctls. If any, we can add a new TLV to represent the associated kctls, as the driver already knows which widget NID is assigned to which control. This would be simpler and more robust, I guess.

thanks,

Takashi

At Wed, 22 Feb 2012 17:18:44 +0000, Mark Brown wrote:

On Wed, Feb 22, 2012 at 05:52:17PM +0100, Takashi Iwai wrote:

...

Mark Brown wrote:

...

...

Now that the kctl jacks are there I'm getting people asking me about it often enough so I'd like to see it merged.

...

Yeah, if things were easy, I'd be happy to merge. But, judging from the situation, I see no big reason to hurry too much.

I'm not sure what problems you see here - all the issues that are being discussed here are about the kctl interface to applications, there's no issues I can see with the in-kernel interfaces.

First of all, we don't agree yet which naming rule to be applied. Since your patch assumes the case B, i.e. constant names corresponding only to the key type (HEADPHONE, etc), it's incompatible with the current implementation in HD-audio.

That is, once when the patch is merged, the kctl expression will be forcibly to case B but without TLV yet, because kctls will be created automatically when the jack instance is created.

In other words, if I merge your patch now, the only solution for HD-audio side for the time being is to disable CONFIG_SND_HDA_INPUT_JACK. That's why I hesitate to merge it now. And, it's why I prefer defining the naming rule at first, thus refining the implementation not to conflict with the existing one.

Takashi

At Wed, 22 Feb 2012 18:54:38 +0000, Mark Brown wrote:

On Wed, Feb 22, 2012 at 06:34:49PM +0100, Takashi Iwai wrote:

...

First of all, we don't agree yet which naming rule to be applied. Since your patch assumes the case B, i.e. constant names corresponding only to the key type (HEADPHONE, etc), it's incompatible with the current implementation in HD-audio.

As I've *repeatedly* said the idea was to prefix the name once we'd got stuff merged into the same file (which means ripping out the HDA specific code)

Even if so, it can't be merged until this prefix stuff comes in. As mentioned, otherwise it conflicts.

Alternatively, deprecate CONFIG_SND_HDA_INPUT_JACK. This is another option.

but if we're going to have to completely rework the ABI before you'll consider trying to fix things up in kernel then there's no point bothering.

Right. If we change.

...

In other words, if I merge your patch now, the only solution for HD-audio side for the time being is to disable CONFIG_SND_HDA_INPUT_JACK. That's why I hesitate to merge it now.

Well, that whole config option just shouldn't be there in the first place but that's another story...

Yes, but it's really too late to grumble. This was merged years ago.

Meanwhile, we can mark it deprecated now as we provide an alternative solution in 3.3 frame. Then drop it in 3.4 or 3.5 where your patch comes in. THis sounds feasible.

...

And, it's why I prefer defining the naming rule at first, thus refining the implementation not to conflict with the existing one.

It seems like it's a bit late to worry about the naming scheme to that extent given that the existing ABI has already managed to get into one kernel release and looks like it's also going to get into 3.4 as well.

Which ABI? The kctl jack hasn't been in released kernels yet. It was first merged in 3.3. So I really want to fix up the naming rule as soon as possible before 3.3 release. For example, with or with hyphen. It's an easy task to fix in the code once when decided.

Right now the only tractable fix I can see is to implement something approximating the HDA ABI in ASoC but that just makes things even worse from a maintability point of view.

Well, as I already wrote, the prefix and suffix are supposed to be optional. So, the current HD-audio implementation doesn't block your interpretation in jack.c by itself. The only blockages are that the jack.c implementation doesn't provide enough for HD-audio requirement (with the first version), and the detailed naming rule like hyphening isn't decided yet.

Takashi

At Wed, 22 Feb 2012 20:55:30 +0000, Mark Brown wrote:

On Wed, Feb 22, 2012 at 09:35:08PM +0100, Takashi Iwai wrote:

...

Mark Brown wrote:

...

...

As I've *repeatedly* said the idea was to prefix the name once we'd got stuff merged into the same file (which means ripping out the HDA specific code)

...

Even if so, it can't be merged until this prefix stuff comes in. As mentioned, otherwise it conflicts.

Well, yeah. But if there's an insistance that the new ABI is reworked as well then it's not worthwhile. It's a pretty simple change to make, I just didn't want to faff around with the HDA code to discuss the core interface - IIRC I mostly posted the patch to make it clear that both APIs just use sets of booleans.

OK.

...

Alternatively, deprecate CONFIG_SND_HDA_INPUT_JACK. This is another option.

I think we should do that anyway, just unconditionally enable the feature.

OK.

...

Meanwhile, we can mark it deprecated now as we provide an alternative solution in 3.3 frame. Then drop it in 3.4 or 3.5 where your patch comes in. THis sounds feasible.

We're well past the point at which we can change 3.3 substantially.

Yes, but it's not too late to put a DEPRECATED marker in Kconfig right now, at least. It's no removal yet, so no problem for 3.3.

...

...

It seems like it's a bit late to worry about the naming scheme to that extent given that the existing ABI has already managed to get into one kernel release and looks like it's also going to get into 3.4 as well.

...

Which ABI? The kctl jack hasn't been in released kernels yet. It was first merged in 3.3. So I really want to fix up the naming rule as soon as possible before 3.3 release. For example, with or with

We're pretty committed to what's in 3.3 though. If it's just the hyphen that's a fairly meaningless difference in terms of the stuff I'm talking about, it's trivial to change. But if you guys decide to go with things like adding TLV information or whatever then that's more involved and I can't see that getting in at this point, we'd need to remove the kctl jacks from the userspace interface for 3.3 or accept churn.

As I showed, the implementation case C, with the verbose name string and _optional_ TLV, doesn't conflict with both the current HD-audio code and your future patch. That is, it can be well "Mic" if it's a single object. First when you need to distinguish multiple items or enumerate with attributes, you can choose either the prefix/suffix in the name string or the base name with a TLV info: "Rear Mic" is equivalent with "Mic" with TLV_POS_REAR.

This looks like the most feasible option to me.

It did also make it out in the last alsa-driver release IIRC.

Bah, the alsa-driver was released by some reason I don't know of.

I really think the release schedule must be defined in a more strict but easier way. For example, just release alsa-driver at each time the kernel is released. It allows user of the older kernel to run the driver on the latest kernel (not the latest development version!).

Well, it's another topic, and should be discussed in a different thread...

...

hyphen. It's an easy task to fix in the code once when decided.

I mostly agree, I'm pushing to merge now because it seems like we can make the change before or after merging the APIs and I don't want to get to the merge window without the two interfaces being brought into line because there's still ongoing discussion of exactly what the userspace interface for the control jacks should look like.

Right.

...

...

Right now the only tractable fix I can see is to implement something approximating the HDA ABI in ASoC but that just makes things even worse from a maintability point of view.

...

Well, as I already wrote, the prefix and suffix are supposed to be optional. So, the current HD-audio implementation doesn't block your interpretation in jack.c by itself. The only blockages are that the jack.c implementation doesn't provide enough for HD-audio requirement

So when I submit a patch removing the kctl jack support from HDA in favour of something along the lines of the current code you'll be OK with that so long as there's also something there to use the card naming (I'd expect I'd end up submitting that as a followup patch for the sake of making the individual patches clearer)?

If the patch will cover the missing pieces, i.e. filling the location, the channel and the multiple indices, I'm fine to remove the current hd-audio kctl jack code, yes.

thanks,

Takashi

On 02/13/2012 06:40 PM, Takashi Iwai wrote:

At Mon, 13 Feb 2012 07:44:59 -0800, Mark Brown wrote:

...

On Mon, Feb 13, 2012 at 02:56:55PM +0100, Takashi Iwai wrote:

...

Mark Brown wrote:

...

...

There's rather a lot of people who do embedded development and do need to look at this stuff, though. There's a real cost to raising the bar for humans to understand things.

...

Hm, I'm afraid that this is getting apart from the real worth discussion. I'm interested in whether "Line-Out" is acceptable enough, or it's a matter of taste, just like Marmite is acceptable as the normal English breakfasts.

Well, I dunno - you did suggest adding direction modifiers to the jack names which would both be a more general solution to the problem and clean up the naming issue.

OK, let's postpone about it.

I've seen enough "Line-Out"s in my life that I'm pretty sure everyone will understand it, even though "Line Out" is more common.

...

...

with the input-jack layer. The input-jack doesn't provide the index number. So, you can't register the same name correctly. Thus I tried to create a unique name string as input-jack id.

It's really never been the intention to have an "input-jack" layer -

I called it just to distinguish with kctl jack, not intended to stress that it's an input layer.

...

the API is there to hide all the input layer stuff it can from the individual audio drivers. The only thing that leaks out is the button remapping which I couldn't think of anything more sensible to do with them.

...

Now, looking back to your implementation, the kctl jack is created with a name string based on the switch type, not from the name string passed to snd_jack_new() itself. Why not passing the string as is?

We can't pass it as-is since we'll need multiple controls per jack - literally all the systems I have available to test on would fail if they used the same name as they all have headset jacks. As I said further up the thread my plan had been to refactor the name generation code once the separate kctl code is gone and they can be merged into the same source file which makes this a lot easier.

Makes sense.

...

With these patches I'm mainly looking for a quick and simple refactoring to get rid of the duplication - once we've got rid of the duplicate driver APIs then we can refactor the internals of the implementation however we like but there's a pretty pressing need to fix the driver API and get all the drivers doing the same thing to userspace even if it's not ideal.

Hm, then I'd say let's hold on. Merging this half-baked stuff is rather confusing at this point since the use of kctl jack isn't targeted yet for ASoC.

I believe what we need _now_ is to decide the policy (i.e. element naming rule and TLV definitions) for kctl jacks quickly.

...

...

If multiple kctls are needed (e.g. for headset), you can either create a new name or just create the multiple instances with the same name but give extra TLVs to identify which type it is. Or, you can create an enum control so that both the input-jack and the kctl-jack correspond as 1:1.

This stuff is why I've been asking you about the ABI and general semantics for the new controls, I was never sure what the intention was

• when you said they were just plain booleans I just went ahead and

created multiple controls as that seemed to be what the current idiom was.

I don't think an enumeration is going to scale so well given the number of possible combinations you can get on more complex jacks.

OK.

...

...

In other words, if 1:1 mapping isn't needed, we don't have to create a different name string at all. Just create "Jack Detect" controls with different indices and with corresponding TLVs. This would be enough (could be even easier) for a machine parser.

That'd work I think, though I would create a new control for each physical jack at least just for clarity.

They are indeed individual ctl elements. They just share the same name and are distinguished by the index numbers. For the parser program, it would be even easier because it needs to look only for elements containing the single string. The rest is the parse of TLVs of gathered elements, which would be needed no matter whether we have multiple name strings or not.

...

...

And, it seems that there is no standard rule defined for the id string of input-jack. If 1:1 model is used, we'd need to define this rule.

The input stack doesn't have the same concept of machine parsable names for things that ALSA does (honestly that's pretty unique to ALSA within the kernel) - the names it users are all free form text intended to be meaningful to humans only.

Yeah, maybe. But, if you have two headset jacks, how would you identify with the current jack stuff...?

...

...

BTW, I found another issue. With this implementation, you can't build kjack ctl without input-jack. I can work around it for HD-audio by adding more ifdef, but it'd be ugly...

I really don't see this as a problem; it seems vanishingly unlikely that any realistic system would want to disable the input API and need accessory detection. Obviously the end goal here is that you wouldn't be able to have HDA specific hacks in the code as everything will be handled by the ALSA core code...

Yeah, the more code may be moved toward the core once when we decide the standards.

...

To my mind we should just support everything and let userspace worry about deciding what to use, this isn't the sort of thing which should require a kernel rebuild to configure and it's not like there's any fundamental problem or massive overhead caused by having the devices show up.

That's one option, yes. OTOH, people always want to diet the kernel code. It's a good dream that is hardly to achieve in reality (for human beings, too), though.

So, at this point, I can list the following policies for kctl jacks:

A. all kctls have one name string with different indices. Each jack is distinguished by the TLV provided to each kctl.

B. kctls are built with only a base name ("Headphone", "Mic") with different indices and TLVs.

C. kctls contain unique names ([location] base [direction] [channel]) optionally with TLVs

I would vote for "C2", although I would probably prefer "[location] base [channel]" over "[location] base [direction] [channel]", as direction is superfluous given "base". This is also what the current implementation offers, and what I've based my PulseAudio patches on.

A and B requires additional work as we would need an API in alsa-lib from where you can read and interpret the TLVs, and stuff like amixer needs to understand it too make it reasonable easy to work with and test. And C gives the option to add TLVs later whenever we feel the need for it, so it seems to be an all-win.

Also; as this graph exposing thing is unlikely to be implemented in all layers of the audio stack any time soon, maybe C2 is also the one that gives the most obvious matching between mixer kcontrols and jack kcontrols? I'd like to move in this direction; not only because we currently do not have the graph, but also because that even if we have it, userspace apps choosing not to implement it will have a good option.

It'd be clear when thinking some real cases. Assume a headset jack containing a headphone and a mic switch: A: two kctls with TLV JACK_HEADSET but with TYPE_HEADPHONE and TYPE_MIC, respectively. B1: two kctls with the same "Headset" name, with TLV TYPE_HEADPHONE and TYPE_MIC B2: separate to a kctl with "Headphone" and a kctl with "Mic" C1: same as B1 C2: same as B2

When an extra headphone jack is added to the above, A: another kctl with TLV_JACK_HEADPHONE, TYPE_HEADPHONE B1: A kctl "Headphone" with TLV TYPE_HEADPHONE B2: A kctl "Headphone" with index 1 C1: same as B1 C2: A kctl "Rear Headphone" (and rename other "Front Headphone")

Yet more another headphone jack on left, A: another kctl with TLV_JACK_HEADPHONE, TYPE_HEADPHONE, LOC_LEFT B1: A kctl "Headphone" index 1 with TLV TYPE_HEADPHONE, LOC_LEFT B2: A kctl "Headphone" index 2 with LOC_LEFT C1: A kctl "Left Headphone" (rename other "Rear Headphone") C2: A kctl "Left Headphone"

... or does it make things more complicated? :)

thanks,

Takashi

2012/2/10, Takashi Iwai tiwai@suse.de:

I'd like to hear rather comments from others.

1) The problem of core jack api which is tailor made for embedded device, it assume there is only one sound card

when the desktop computer have more than one sound cards which have jack detect there is no mapping to which sound card has the events.

How to differentiate . noboard hdmi port and graphic card's hdmi ?

2) if the objective of adding jack detection is for trapping plug/unplug event to trigger the reconfiguration of audio server

http://thread.gmane.org/gmane.linux.alsa.devel/89781/focus=89788

does the audio server require to know the initial state of the hdmi or just need the trigger event ?

3) There are laptops (e.g. alienware) which have two HP jacks and Mic jack which need jack retasking for support surround5.1 since the name of volume controls cannot be rename , the only way is to perform a hda-reconfig or early patching

can those input device be deleted during the hda-reconfig ?

On Wed, Feb 22, 2012 at 05:28:56PM +0100, Takashi Iwai wrote:

I'm fine to apply the first patch, certainly it's just a define. Or just commit into your tree with my ack: Reviewed-by: Takashi Iwai tiwai@suse.de

Seems like it'd be sensible to push it through your tree, there's nothing particularly ASoC related about it and hopefully none of this stuff will have any impact on anything except HDA (as we'll need to back out the bodges in there).

At Wed, 22 Feb 2012 17:41:01 +0100, Takashi Iwai wrote:

At Wed, 22 Feb 2012 16:34:12 +0000, Mark Brown wrote:

...

On Wed, Feb 22, 2012 at 05:28:56PM +0100, Takashi Iwai wrote:

...

I'm fine to apply the first patch, certainly it's just a define. Or just commit into your tree with my ack: Reviewed-by: Takashi Iwai tiwai@suse.de

Seems like it'd be sensible to push it through your tree, there's nothing particularly ASoC related about it and hopefully none of this stuff will have any impact on anything except HDA (as we'll need to back out the bodges in there).

OK. I'll inform you when it's merged.

I merged now the first patch in topic/jack branch.

Takashi