ALSA: hda: hdmi: Hint matching between input devices and pcm devices

ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[David Henningsson]

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

Since modern HDMI cards often have more than one output pin and thus
input device, we need to know which one has actually been plugged in.

This patch adds a name hint that indicates which PCM device is connected 
to which pin.

To do that, the jack creation has been deferred to build_controls, i e, 
after the PCM devices have been created.

Would be great to have Stephen look through this patch quickly before 
it's committed.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

[-- Attachment #2: 0001-ALSA-hda-hdmi-Hint-matching-between-input-devices-an.patch --]
[-- Type: text/x-patch, Size: 2421 bytes --]

>From ab5ef7898ca0a6e86401428a3ca77f05db9eb7c9 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Tue, 23 Aug 2011 16:56:03 +0200
Subject: [PATCH] ALSA: hda: hdmi: Hint matching between input devices and pcm devices

Since modern HDMI cards often have more than one output pin and thus
input device, we need to know which one has actually been plugged in.

This patch adds a name hint that indicates which PCM device is connected 
to which pin.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_hdmi.c |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 19cb72d..8c83ec4 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -967,19 +967,12 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 
 	per_pin->pin_nid = pin_nid;
 
-	err = snd_hda_input_jack_add(codec, pin_nid,
-				     SND_JACK_VIDEOOUT, NULL);
-	if (err < 0)
-		return err;
-
 	err = hdmi_read_pin_conn(codec, pin_idx);
 	if (err < 0)
 		return err;
 
 	spec->num_pins++;
 
-	hdmi_present_sense(codec, pin_nid, eld);
-
 	return 0;
 }
 
@@ -1162,6 +1155,25 @@ static int generic_hdmi_build_pcms(struct hda_codec *codec)
 	return 0;
 }
 
+static int generic_hdmi_build_jack(struct hda_codec *codec, int pin_idx)
+{
+	int err;
+	char hdmi_str[32];
+	struct hdmi_spec *spec = codec->spec;
+	struct hdmi_spec_per_pin *per_pin = &spec->pins[pin_idx];
+	int pcmdev = spec->pcm_rec[pin_idx].device;
+
+	snprintf(hdmi_str, sizeof(hdmi_str), "HDMI/DP (pcm %d)", pcmdev);
+
+	err = snd_hda_input_jack_add(codec, per_pin->pin_nid,
+			     SND_JACK_VIDEOOUT, pcmdev > 0 ? hdmi_str : NULL);
+	if (err < 0)
+		return err;
+
+	hdmi_present_sense(codec, per_pin->pin_nid, &per_pin->sink_eld);
+	return 0;
+}
+
 static int generic_hdmi_build_controls(struct hda_codec *codec)
 {
 	struct hdmi_spec *spec = codec->spec;
@@ -1170,6 +1182,11 @@ static int generic_hdmi_build_controls(struct hda_codec *codec)
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = &spec->pins[pin_idx];
+
+		err = generic_hdmi_build_jack(codec, pin_idx);
+		if (err < 0)
+			return err;
+
 		err = snd_hda_create_spdif_out_ctls(codec,
 						    per_pin->pin_nid,
 						    per_pin->mux_nids[0]);
-- 
1.7.4.1


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

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[Stephen Warren]

David Henningsson wrote at Tuesday, August 23, 2011 9:12 AM:
> Since modern HDMI cards often have more than one output pin and thus
> input device, we need to know which one has actually been plugged in.
> 
> This patch adds a name hint that indicates which PCM device is connected
> to which pin.
> 
> To do that, the jack creation has been deferred to build_controls, i e,
> after the PCM devices have been created.
> 
> Would be great to have Stephen look through this patch quickly before
> it's committed.

Structurally, I think this looks OK. The only question I have is the 
string format:

"HDMI/DP (pcm %d)", pcmdev

* Is there a 1:1 mapping between the internal pcmdev numbers and what
alsa-lib presents to clients? Thinking about ALSA device numbering, it's
like "hw:1,3", so includes the card number too; should that be in the
string?

* Would it be better to make the string completely generic - i.e. not
include "HDMI/DP", but rather something like "ALSA PCM %d", or even
"ALSA PCM hw:%d,%d" so that the same format could be used for non-HDMI/
DP PCMs in the future?

-- 
nvpublic

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[David Henningsson]

On 2011-08-23 17:51, Stephen Warren wrote:
> David Henningsson wrote at Tuesday, August 23, 2011 9:12 AM:
>> Since modern HDMI cards often have more than one output pin and thus
>> input device, we need to know which one has actually been plugged in.
>>
>> This patch adds a name hint that indicates which PCM device is connected
>> to which pin.
>>
>> To do that, the jack creation has been deferred to build_controls, i e,
>> after the PCM devices have been created.
>>
>> Would be great to have Stephen look through this patch quickly before
>> it's committed.

Thanks for the review!

> Structurally, I think this looks OK. The only question I have is the
> string format:
>
> "HDMI/DP (pcm %d)", pcmdev
>
> * Is there a 1:1 mapping between the internal pcmdev numbers and what
> alsa-lib presents to clients?

That's very much up to the configuration in alsa-lib, but I would say 
that alsa-lib in general does that, and especially for HDA.

> Thinking about ALSA device numbering, it's
> like "hw:1,3", so includes the card number too; should that be in the
> string?

The matchup against the card can be done both in sysfs, and using the 
card name, which is later prefixed to that string. The end result would 
be something like "HDA NVidia HDMI/DP (pcm 3)"

It's possible though, and somewhat simpler than matching against sysfs 
to add the card number as well.

> * Would it be better to make the string completely generic - i.e. not
> include "HDMI/DP", but rather something like "ALSA PCM %d", or even
> "ALSA PCM hw:%d,%d" so that the same format could be used for non-HDMI/
> DP PCMs in the future?

That would probably be bad, as there would be no way to distinguish 
between e g "Headphone" and "Headphone 2". This is a HDMI specific 
problem in general, as all analog outputs and inputs are at ,0 and SPDIF 
at ,1. (Although there might be exceptions, the "Independent HP" thing 
of the VIA drivers come to mind.)

Up to now, I believe the string in the name has been pretty arbitrarily. 
E g, I have "Headphone" on one machine and "HP Out" on another. Should 
we try to establish a format where various parameters can be added, it 
might be good to try something like ",name=value". Should we add card as 
well, the result would be e g: "HDA NVidia HDMI/DP,card=2,pcm=3"

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[Stephen Warren]

David Henningsson wrote at Tuesday, August 23, 2011 10:54 PM:
> On 2011-08-23 17:51, Stephen Warren wrote:
> > David Henningsson wrote at Tuesday, August 23, 2011 9:12 AM:
> >> Since modern HDMI cards often have more than one output pin and thus
> >> input device, we need to know which one has actually been plugged in.
> >>
> >> This patch adds a name hint that indicates which PCM device is connected
> >> to which pin.
> >>
> >> To do that, the jack creation has been deferred to build_controls, i e,
> >> after the PCM devices have been created.
> >>
> >> Would be great to have Stephen look through this patch quickly before
> >> it's committed.
> 
> Thanks for the review!
> 
> > Structurally, I think this looks OK. The only question I have is the
> > string format:
> >
> > "HDMI/DP (pcm %d)", pcmdev
> >
> > * Is there a 1:1 mapping between the internal pcmdev numbers and what
> > alsa-lib presents to clients?
> 
> That's very much up to the configuration in alsa-lib, but I would say
> that alsa-lib in general does that, and especially for HDA.
> 
> > Thinking about ALSA device numbering, it's
> > like "hw:1,3", so includes the card number too; should that be in the
> > string?
> 
> The matchup against the card can be done both in sysfs, and using the
> card name, which is later prefixed to that string. The end result would
> be something like "HDA NVidia HDMI/DP (pcm 3)"

OK, so if the string that user-space sees already has the card name, that's
probably enough in practice, although it still seems a little free-form.

One issue with the above string: There's no delimiter that user-space can
use to extract just the card name from the whole string. Perhaps add a
":" at the start so there's a delimiter?

"HDA NVidia: HDMI/DP (pcm 3)"
"HDA NVidia: pcm 3"

?

> It's possible though, and somewhat simpler than matching against sysfs
> to add the card number as well.
> 
> > * Would it be better to make the string completely generic - i.e. not
> > include "HDMI/DP", but rather something like "ALSA PCM %d", or even
> > "ALSA PCM hw:%d,%d" so that the same format could be used for non-HDMI/
> > DP PCMs in the future?
> 
> That would probably be bad, as there would be no way to distinguish
> between e g "Headphone" and "Headphone 2".

Are those two different analog outputs from the same PCM device?

If so, I don't see a problem with having both those jack's input devices
reporting the same PCM name; user-space will just see 2 matches and know
that the PCM device is useful if either is plugged in.

If not, I'm afraid I don't have enough context to understand your comment.

> This is a HDMI specific
> problem in general, as all analog outputs and inputs are at ,0 and SPDIF
> at ,1. (Although there might be exceptions, the "Independent HP" thing
> of the VIA drivers come to mind.)

Hmm. I don't see it as HDMI specific. If there's a random input device
today that represents the analog output's jack detection, how does user-
space know the input device is for analog (sub-device 0) at all? Isn't
the exact same issue present for any kind of output; I don't how the
,0/,1/,3 numbering affects this. Sorry!

> Up to now, I believe the string in the name has been pretty arbitrarily.
> E g, I have "Headphone" on one machine and "HP Out" on another. Should
> we try to establish a format where various parameters can be added, it
> might be good to try something like ",name=value". Should we add card as
> well, the result would be e g: "HDA NVidia HDMI/DP,card=2,pcm=3"

That sounds pretty sane to me.

-- 
nvpublic

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[David Henningsson]

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

On 08/24/2011 11:21 PM, Stephen Warren wrote:
> David Henningsson wrote at Tuesday, August 23, 2011 10:54 PM:
>> On 2011-08-23 17:51, Stephen Warren wrote:
>>> David Henningsson wrote at Tuesday, August 23, 2011 9:12 AM:
>>>> Since modern HDMI cards often have more than one output pin and thus
>>>> input device, we need to know which one has actually been plugged in.
>>>>
>>>> This patch adds a name hint that indicates which PCM device is connected
>>>> to which pin.
>>>>
>>>> To do that, the jack creation has been deferred to build_controls, i e,
>>>> after the PCM devices have been created.
>>>>
>>>> Would be great to have Stephen look through this patch quickly before
>>>> it's committed.
>>
>> Thanks for the review!
>>
>>> Structurally, I think this looks OK. The only question I have is the
>>> string format:
>>>
>>> "HDMI/DP (pcm %d)", pcmdev
>>>
>>> * Is there a 1:1 mapping between the internal pcmdev numbers and what
>>> alsa-lib presents to clients?
>>
>> That's very much up to the configuration in alsa-lib, but I would say
>> that alsa-lib in general does that, and especially for HDA.
>>
>>> Thinking about ALSA device numbering, it's
>>> like "hw:1,3", so includes the card number too; should that be in the
>>> string?
>>
>> The matchup against the card can be done both in sysfs, and using the
>> card name, which is later prefixed to that string. The end result would
>> be something like "HDA NVidia HDMI/DP (pcm 3)"
>
> OK, so if the string that user-space sees already has the card name, that's
> probably enough in practice, although it still seems a little free-form.
>
> One issue with the above string: There's no delimiter that user-space can
> use to extract just the card name from the whole string. Perhaps add a
> ":" at the start so there's a delimiter?

Or a ","

> "HDA NVidia: HDMI/DP (pcm 3)"
> "HDA NVidia: pcm 3"
>
> ?
>
>> It's possible though, and somewhat simpler than matching against sysfs
>> to add the card number as well.
>>
>>> * Would it be better to make the string completely generic - i.e. not
>>> include "HDMI/DP", but rather something like "ALSA PCM %d", or even
>>> "ALSA PCM hw:%d,%d" so that the same format could be used for non-HDMI/
>>> DP PCMs in the future?
>>
>> That would probably be bad, as there would be no way to distinguish
>> between e g "Headphone" and "Headphone 2".
>
> Are those two different analog outputs from the same PCM device?

Yes.

> If so, I don't see a problem with having both those jack's input devices
> reporting the same PCM name; user-space will just see 2 matches and know
> that the PCM device is useful if either is plugged in.

The problem is not that they have the same name, the problem is with 
removing "HDMI/DP", or in the analog case "Headphones" and "Headphones 
2". This is used to determine which ALSA volume controls one should 
control when the user wants to change volume. (The PCM device is usually 
useful regardless, e g with internal speakers which don't have a jack.)

> If not, I'm afraid I don't have enough context to understand your comment.
>
>> This is a HDMI specific
>> problem in general, as all analog outputs and inputs are at ,0 and SPDIF
>> at ,1. (Although there might be exceptions, the "Independent HP" thing
>> of the VIA drivers come to mind.)
>
> Hmm. I don't see it as HDMI specific. If there's a random input device
> today that represents the analog output's jack detection, how does user-
> space know the input device is for analog (sub-device 0) at all? Isn't
> the exact same issue present for any kind of output; I don't how the
> ,0/,1/,3 numbering affects this. Sorry!

As for the actual numbering, this is hardcoded in 
hda_codec.c:get_empty_pcm_device().

The jack input devices solve two different problems for HDMI and analog:

For HDMI, my problem is that I don't know which PCM device to send the 
output to (because there are four). The jack input device, with the 
patch, fixes that problem.

For Analog output, my problem is that I don't know which volume controls 
are affecting the current output path. If the Headphone is plugged in, 
this might be "Master" and "Headphone" whereas if it is unplugged, it 
might be "Master" and "Speaker" instead.
In theory, analog outputs could show up at both devices 0,2,4 and 5, but 
in practice, all of them always [1] show up at PCM device 0.

>> Up to now, I believe the string in the name has been pretty arbitrarily.
>> E g, I have "Headphone" on one machine and "HP Out" on another. Should
>> we try to establish a format where various parameters can be added, it
>> might be good to try something like ",name=value". Should we add card as
>> well, the result would be e g: "HDA NVidia HDMI/DP,card=2,pcm=3"
>
> That sounds pretty sane to me.

Ok, I'm attaching a patch where the only difference is that the format 
has changed according to the ,name=value syntax.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

[1] Possibly with the "Independent HP" exception of the VIA drivers, and 
maybe some more exotic exception that I've forgotten.

[-- Attachment #2: 0001-ALSA-hda-hdmi-Hint-matching-between-input-devices-an.patch --]
[-- Type: text/x-patch, Size: 2419 bytes --]

>From ab5ef7898ca0a6e86401428a3ca77f05db9eb7c9 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Tue, 23 Aug 2011 16:56:03 +0200
Subject: [PATCH] ALSA: hda: hdmi: Hint matching between input devices and pcm devices

Since modern HDMI cards often have more than one output pin and thus
input device, we need to know which one has actually been plugged in.

This patch adds a name hint that indicates which PCM device is connected 
to which pin.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_hdmi.c |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 19cb72d..8c83ec4 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -967,19 +967,12 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 
 	per_pin->pin_nid = pin_nid;
 
-	err = snd_hda_input_jack_add(codec, pin_nid,
-				     SND_JACK_VIDEOOUT, NULL);
-	if (err < 0)
-		return err;
-
 	err = hdmi_read_pin_conn(codec, pin_idx);
 	if (err < 0)
 		return err;
 
 	spec->num_pins++;
 
-	hdmi_present_sense(codec, pin_nid, eld);
-
 	return 0;
 }
 
@@ -1162,6 +1155,25 @@ static int generic_hdmi_build_pcms(struct hda_codec *codec)
 	return 0;
 }
 
+static int generic_hdmi_build_jack(struct hda_codec *codec, int pin_idx)
+{
+	int err;
+	char hdmi_str[32];
+	struct hdmi_spec *spec = codec->spec;
+	struct hdmi_spec_per_pin *per_pin = &spec->pins[pin_idx];
+	int pcmdev = spec->pcm_rec[pin_idx].device;
+
+	snprintf(hdmi_str, sizeof(hdmi_str), "HDMI/DP,pcm=%d", pcmdev);
+
+	err = snd_hda_input_jack_add(codec, per_pin->pin_nid,
+			     SND_JACK_VIDEOOUT, pcmdev > 0 ? hdmi_str : NULL);
+	if (err < 0)
+		return err;
+
+	hdmi_present_sense(codec, per_pin->pin_nid, &per_pin->sink_eld);
+	return 0;
+}
+
 static int generic_hdmi_build_controls(struct hda_codec *codec)
 {
 	struct hdmi_spec *spec = codec->spec;
@@ -1170,6 +1182,11 @@ static int generic_hdmi_build_controls(struct hda_codec *codec)
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = &spec->pins[pin_idx];
+
+		err = generic_hdmi_build_jack(codec, pin_idx);
+		if (err < 0)
+			return err;
+
 		err = snd_hda_create_spdif_out_ctls(codec,
 						    per_pin->pin_nid,
 						    per_pin->mux_nids[0]);
-- 
1.7.4.1


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

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[Stephen Warren]

David Henningsson wrote at Thursday, August 25, 2011 1:14 AM:
> On 08/24/2011 11:21 PM, Stephen Warren wrote:
> > David Henningsson wrote at Tuesday, August 23, 2011 10:54 PM:
> >> On 2011-08-23 17:51, Stephen Warren wrote:
> >>> David Henningsson wrote at Tuesday, August 23, 2011 9:12 AM:
> >>>> Since modern HDMI cards often have more than one output pin and thus
> >>>> input device, we need to know which one has actually been plugged in.
> >>>>
> >>>> This patch adds a name hint that indicates which PCM device is connected
> >>>> to which pin.
...
> >>> Structurally, I think this looks OK. The only question I have is the
> >>> string format:
> >>>
> >>> "HDMI/DP (pcm %d)", pcmdev
...
> >>> Thinking about ALSA device numbering, it's
> >>> like "hw:1,3", so includes the card number too; should that be in the
> >>> string?
> >>
> >> The matchup against the card can be done both in sysfs, and using the
> >> card name, which is later prefixed to that string. The end result would
> >> be something like "HDA NVidia HDMI/DP (pcm 3)"
> >
> > OK, so if the string that user-space sees already has the card name, that's
> > probably enough in practice, although it still seems a little free-form.
> >
> > One issue with the above string: There's no delimiter that user-space can
> > use to extract just the card name from the whole string. Perhaps add a
> > ":" at the start so there's a delimiter?
> 
> Or a ","

Sure.

> > "HDA NVidia: HDMI/DP (pcm 3)"
> > "HDA NVidia: pcm 3"
> >
> > ?
> >
> >> It's possible though, and somewhat simpler than matching against sysfs
> >> to add the card number as well.
> >>
> >>> * Would it be better to make the string completely generic - i.e. not
> >>> include "HDMI/DP", but rather something like "ALSA PCM %d", or even
> >>> "ALSA PCM hw:%d,%d" so that the same format could be used for non-HDMI/
> >>> DP PCMs in the future?
> >>
> >> That would probably be bad, as there would be no way to distinguish
> >> between e g "Headphone" and "Headphone 2".
> >
> > Are those two different analog outputs from the same PCM device?
> 
> Yes.
> 
> > If so, I don't see a problem with having both those jack's input devices
> > reporting the same PCM name; user-space will just see 2 matches and know
> > that the PCM device is useful if either is plugged in.
> 
> The problem is not that they have the same name, the problem is with
> removing "HDMI/DP",

Ah, OK. That makes sense now.

...
> The jack input devices solve two different problems for HDMI and analog:
> 
> For HDMI, my problem is that I don't know which PCM device to send the
> output to (because there are four). The jack input device, with the
> patch, fixes that problem.
> 
> For Analog output, my problem is that I don't know which volume controls
> are affecting the current output path. If the Headphone is plugged in,
> this might be "Master" and "Headphone" whereas if it is unplugged, it
> might be "Master" and "Speaker" instead.
>
> In theory, analog outputs could show up at both devices 0,2,4 and 5, but
> in practice, all of them always [1] show up at PCM device 0.

OK. I think those are just two aspects of the same thing; we always need to
know which PCM to use and which controls to use; it's just that in practice
sometimes that information is also implied by other means.

So, I don't think a fully general solution hurts.

> >> Up to now, I believe the string in the name has been pretty arbitrarily.
> >> E g, I have "Headphone" on one machine and "HP Out" on another. Should
> >> we try to establish a format where various parameters can be added, it
> >> might be good to try something like ",name=value". Should we add card as
> >> well, the result would be e g: "HDA NVidia HDMI/DP,card=2,pcm=3"
> >
> > That sounds pretty sane to me.
> 
> Ok, I'm attaching a patch where the only difference is that the format
> has changed according to the ,name=value syntax.

Just one question: If the core code prefixes the card name onto this string,
the HDMI code will pass in "HDMI/DP,pcm=3" and the core code will join the 
prefix on the front using a space, yielding "HDA NVidia HDMI/DP,pcm=3". Can
we make the joining use a comma so that spaces within fields don't confuse
the comma, i.e. "HDA NVidia,HDMI/DP,pcm=3"? Otherwise, does the card name
end at the first space, the second, the last? (None of these work in all
the cases considering that the second field might be "Headphones 2" with a
space in it.

-- 
nvpublic

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[David Henningsson]

On 2011-08-25 19:41, Stephen Warren wrote:
> David Henningsson wrote at Thursday, August 25, 2011 1:14 AM:
>> On 08/24/2011 11:21 PM, Stephen Warren wrote:
>>> David Henningsson wrote at Tuesday, August 23, 2011 10:54 PM:
>>>> On 2011-08-23 17:51, Stephen Warren wrote:
>>>>> David Henningsson wrote at Tuesday, August 23, 2011 9:12 AM:
>>>>>> Since modern HDMI cards often have more than one output pin and thus
>>>>>> input device, we need to know which one has actually been plugged in.
>>>>>>
>>>>>> This patch adds a name hint that indicates which PCM device is connected
>>>>>> to which pin.
> ...
>>>>> Structurally, I think this looks OK. The only question I have is the
>>>>> string format:
>>>>>
>>>>> "HDMI/DP (pcm %d)", pcmdev
> ...
>>>>> Thinking about ALSA device numbering, it's
>>>>> like "hw:1,3", so includes the card number too; should that be in the
>>>>> string?
>>>>
>>>> The matchup against the card can be done both in sysfs, and using the
>>>> card name, which is later prefixed to that string. The end result would
>>>> be something like "HDA NVidia HDMI/DP (pcm 3)"
>>>
>>> OK, so if the string that user-space sees already has the card name, that's
>>> probably enough in practice, although it still seems a little free-form.
>>>
>>> One issue with the above string: There's no delimiter that user-space can
>>> use to extract just the card name from the whole string. Perhaps add a
>>> ":" at the start so there's a delimiter?
>>
>> Or a ","
>
> Sure.
>
>>> "HDA NVidia: HDMI/DP (pcm 3)"
>>> "HDA NVidia: pcm 3"
>>>
>>> ?
>>>
>>>> It's possible though, and somewhat simpler than matching against sysfs
>>>> to add the card number as well.
>>>>
>>>>> * Would it be better to make the string completely generic - i.e. not
>>>>> include "HDMI/DP", but rather something like "ALSA PCM %d", or even
>>>>> "ALSA PCM hw:%d,%d" so that the same format could be used for non-HDMI/
>>>>> DP PCMs in the future?
>>>>
>>>> That would probably be bad, as there would be no way to distinguish
>>>> between e g "Headphone" and "Headphone 2".
>>>
>>> Are those two different analog outputs from the same PCM device?
>>
>> Yes.
>>
>>> If so, I don't see a problem with having both those jack's input devices
>>> reporting the same PCM name; user-space will just see 2 matches and know
>>> that the PCM device is useful if either is plugged in.
>>
>> The problem is not that they have the same name, the problem is with
>> removing "HDMI/DP",
>
> Ah, OK. That makes sense now.
>
> ...
>> The jack input devices solve two different problems for HDMI and analog:
>>
>> For HDMI, my problem is that I don't know which PCM device to send the
>> output to (because there are four). The jack input device, with the
>> patch, fixes that problem.
>>
>> For Analog output, my problem is that I don't know which volume controls
>> are affecting the current output path. If the Headphone is plugged in,
>> this might be "Master" and "Headphone" whereas if it is unplugged, it
>> might be "Master" and "Speaker" instead.
>>
>> In theory, analog outputs could show up at both devices 0,2,4 and 5, but
>> in practice, all of them always [1] show up at PCM device 0.
>
> OK. I think those are just two aspects of the same thing; we always need to
> know which PCM to use and which controls to use; it's just that in practice
> sometimes that information is also implied by other means.
>
> So, I don't think a fully general solution hurts.
>
>>>> Up to now, I believe the string in the name has been pretty arbitrarily.
>>>> E g, I have "Headphone" on one machine and "HP Out" on another. Should
>>>> we try to establish a format where various parameters can be added, it
>>>> might be good to try something like ",name=value". Should we add card as
>>>> well, the result would be e g: "HDA NVidia HDMI/DP,card=2,pcm=3"
>>>
>>> That sounds pretty sane to me.
>>
>> Ok, I'm attaching a patch where the only difference is that the format
>> has changed according to the ,name=value syntax.
>
> Just one question: If the core code prefixes the card name onto this string,
> the HDMI code will pass in "HDMI/DP,pcm=3" and the core code will join the
> prefix on the front using a space, yielding "HDA NVidia HDMI/DP,pcm=3". Can
> we make the joining use a comma so that spaces within fields don't confuse
> the comma, i.e. "HDA NVidia,HDMI/DP,pcm=3"? Otherwise, does the card name
> end at the first space, the second, the last? (None of these work in all
> the cases considering that the second field might be "Headphones 2" with a
> space in it.
>

So, I don't mind a "fully general solution" as long as we during a 
transitional period allow for a random string (without commas) before 
the first comma, followed by ",name=value" strings. Value can contain 
anything except commas, including spaces.
So let's work towards ",card=NVidia,type=HDMI/DP,pcm=3" (and maybe add e 
g location or other stuff from pin default config as needed) but for now 
it will be okay with "HDA NVidia HDMI/DP,pcm=3".

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[Pierre-Louis Bossart]

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


> Since modern HDMI cards often have more than one output pin and thus
> input device, we need to know which one has actually been plugged in.
> This patch adds a name hint that indicates which PCM device is connected
> to which pin.

I've been thinking about this, and there's some additional work needed
for the jack-detection to be useful.
User-space code will need at some point to rely on the ELD information
to know what the HDMI receiver supports, eg to enable/disable
passthrough. I hacked a while ago a small patch to make the ELD bytes
available in a control (see attached). It seems to work but I wasn't too
sure how to expose it. 
Should we define a convention for the name of this control as well? Or
is there a way to link a control to a specify PCM device?
Thanks for your feedback,
-Pierre

[-- Attachment #2: eld.patch --]
[-- Type: text/x-patch, Size: 4819 bytes --]

diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
index 28ce17d..01d5226 100644
--- a/sound/pci/hda/hda_eld.c
+++ b/sound/pci/hda/hda_eld.c
@@ -24,6 +24,8 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <sound/core.h>
+#include <sound/control.h>
+#include <sound/asoundef.h>
 #include <asm/unaligned.h>
 #include "hda_codec.h"
 #include "hda_local.h"
@@ -318,6 +320,14 @@ int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid)
 						 AC_DIPSIZE_ELD_BUF);
 }
 
+
+static struct snd_kcontrol_new eld_bytes_ctl = {
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+	.name = "ELD Bytes",
+	.info = snd_hdmi_eld_ctl_info,
+	.get = snd_hdmi_eld_ctl_get
+};
+
 int snd_hdmi_get_eld(struct hdmi_eld *eld,
 		     struct hda_codec *codec, hda_nid_t nid)
 {
@@ -325,6 +335,8 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
 	int ret;
 	int size;
 	unsigned char *buf;
+	struct snd_kcontrol *kctl;
+	int err;
 
 	if (!eld->eld_valid)
 		return -ENOENT;
@@ -335,21 +347,30 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
 		snd_printd(KERN_INFO "HDMI: ELD buf size is 0, force 128\n");
 		size = 128;
 	}
-	if (size < ELD_FIXED_BYTES || size > PAGE_SIZE) {
+	if (size < ELD_FIXED_BYTES || size > ELD_MAX_SIZE || size > PAGE_SIZE) {
 		snd_printd(KERN_INFO "HDMI: invalid ELD buf size %d\n", size);
 		return -ERANGE;
 	}
 
-	buf = kmalloc(size, GFP_KERNEL);
-	if (!buf)
+	/* add control for ELD Bytes */
+	kctl = snd_ctl_new1(&eld_bytes_ctl, codec);
+	if (!kctl)
 		return -ENOMEM;
+	err = snd_hda_ctl_add(codec, nid, kctl);
+	if (err < 0)
+		return err;
+	kctl->private_value = nid;
+	printk(KERN_ERR "plb: added control \n");
+
+	/* update info */
+	eld->eld_size = size;
+	buf = eld->eld_buffer;
 
 	for (i = 0; i < size; i++)
 		buf[i] = hdmi_get_eld_byte(codec, nid, i);
 
 	ret = hdmi_update_eld(eld, buf, size);
 
-	kfree(buf);
 	return ret;
 }
 
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index 9ed4b0d..1f237ef 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -609,6 +609,7 @@ struct cea_sad {
 };
 
 #define ELD_FIXED_BYTES	20
+#define ELD_MAX_SIZE    256
 #define ELD_MAX_MNL	16
 #define ELD_MAX_SAD	16
 
@@ -633,6 +634,7 @@ struct hdmi_eld {
 	int	spk_alloc;
 	int	sad_count;
 	struct cea_sad sad[ELD_MAX_SAD];
+	char    eld_buffer[ELD_MAX_SIZE];
 #ifdef CONFIG_PROC_FS
 	struct snd_info_entry *proc_entry;
 #endif
@@ -643,6 +645,10 @@ int snd_hdmi_get_eld(struct hdmi_eld *, struct hda_codec *, hda_nid_t);
 void snd_hdmi_show_eld(struct hdmi_eld *eld);
 void snd_hdmi_eld_update_pcm_info(struct hdmi_eld *eld,
 			      struct hda_pcm_stream *hinfo);
+int snd_hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_info *uinfo);
+int snd_hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol);
 
 #ifdef CONFIG_PROC_FS
 int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld,
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 19cb72d..4a11e93 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -324,6 +324,62 @@ static int cvt_nid_to_cvt_index(struct hdmi_spec *spec, hda_nid_t cvt_nid)
 	return -EINVAL;
 }
 
+int snd_hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_info *uinfo)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct hdmi_spec *spec;
+	struct hdmi_spec_per_pin *per_pin;
+	struct hdmi_eld *eld;
+	int pin_nid;
+	int pin_idx;
+
+	printk(KERN_ERR "plb: in ctl_info routine\n");
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
+
+	pin_nid = kcontrol->private_value;
+	spec = codec->spec;
+
+	pin_idx = pin_nid_to_pin_index(spec, pin_nid);
+	if (pin_idx<0) {
+		uinfo->count = 0;
+	} else {
+		per_pin = &spec->pins[pin_idx];
+		eld = &per_pin->sink_eld;
+		uinfo->count = eld->eld_size;
+	}
+	return 0;
+}
+
+int snd_hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct hdmi_spec *spec;
+	struct hdmi_spec_per_pin *per_pin;
+	struct hdmi_eld *eld;
+	int pin_nid;
+	int pin_idx;
+
+	printk(KERN_ERR "plb: in ctl_get routine\n");
+	pin_nid = kcontrol->private_value;
+
+	spec = codec->spec;
+	pin_nid = kcontrol->private_value;
+
+	pin_idx = pin_nid_to_pin_index(spec, pin_nid);
+	if (pin_idx<0) {
+		memset(ucontrol->value.bytes.data, 0, ELD_MAX_SIZE);
+		printk(KERN_ERR "eld_get: no info found\n");
+	} else {
+		per_pin = &spec->pins[pin_idx];
+		eld = &per_pin->sink_eld;
+		printk(KERN_ERR "eld_get: copying %d bytes\n", eld->eld_size);
+		memcpy(ucontrol->value.bytes.data, eld->eld_buffer, eld->eld_size);
+	}
+	return 0;
+}
+
 #ifdef BE_PARANOID
 static void hdmi_get_dip_index(struct hda_codec *codec, hda_nid_t pin_nid,
 				int *packet_index, int *byte_index)

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

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[David Henningsson]

On 08/30/2011 12:14 AM, Pierre-Louis Bossart wrote:
>
>> Since modern HDMI cards often have more than one output pin and thus
>> input device, we need to know which one has actually been plugged in.
>> This patch adds a name hint that indicates which PCM device is connected
>> to which pin.
>
> I've been thinking about this, and there's some additional work needed
> for the jack-detection to be useful.
> User-space code will need at some point to rely on the ELD information
> to know what the HDMI receiver supports, eg to enable/disable
> passthrough.

Meanwhile, at the PulseAudio side, Colin (and Arun?) have made it 
possible to set capabilities manually in PulseAudio, based on the 
assumption that the ELD information is often wrong anyway. (?)

> I hacked a while ago a small patch to make the ELD bytes
> available in a control (see attached). It seems to work but I wasn't too
> sure how to expose it.

I'm assuming that if you have four HDMI pins (or codecs) that will be 
four "ELD Bytes" controls?

Also, are we sure we want to expose something as driver specific as ELD 
bytes through that interface, or do we want to do something more 
generic? (That said, exposing ELD bytes could also be an interim 
solution while thinking about something more generic.)

> Should we define a convention for the name of this control as well?

Note that my patch doesn't change the name of a control but an input device.

> Or is there a way to link a control to a specify PCM device?

Unfortunately not - maybe this is something to discuss at Linuxcon Prague?

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[Clemens Ladisch]

Pierre-Louis Bossart wrote:
> User-space code will need at some point to rely on the ELD information
> to know what the HDMI receiver supports, eg to enable/disable
> passthrough. I hacked a while ago a small patch to make the ELD bytes
> available in a control (see attached). It seems to work but I wasn't too
> sure how to expose it.

For something device-specific as this, a _BYTES control is the only
choice.  An alternative would be to have one control for each field in
the ELD.

If the driver can detect ELD changes, it must inform userspace that
the control has changed.  If there is no ELD, the _get callback should
return an error, and the control should have been set to unreadable.

> Should we define a convention for the name of this control as well?

If software is supposed to be able to find this control for any HDMI
device.

> Or is there a way to link a control to a specify PCM device?

Yes: set .iface to _PCM instead of _MIXER (this will also prevent
'normal' mixers from showing this control, but a _BYTE control wouldn't
be shown anyway), and set .device (and .subdevice, if appropriate) to
the PCM device's (sub)device number.


Regards,
Clemens

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[Pierre-Louis Bossart]

Hi Clemens,

> If the driver can detect ELD changes, it must inform userspace that
> the control has changed.  If there is no ELD, the _get callback should
> return an error, and the control should have been set to unreadable.

Bear with my ignorance of controls, but how does the driver 'inform
userspace that the control has changed'?

> > Or is there a way to link a control to a specify PCM device?
> 
> Yes: set .iface to _PCM instead of _MIXER (this will also prevent
> 'normal' mixers from showing this control, but a _BYTE control wouldn't
> be shown anyway), and set .device (and .subdevice, if appropriate) to
> the PCM device's (sub)device number.

This is exactly what I wanted, it makes things simpler. Thanks for the
explanation.
-Pierre

Re: ALSA: hda: hdmi: Hint matching between input devices and pcm devices

[Takashi Iwai]

At Tue, 30 Aug 2011 08:01:50 -0500,
Pierre-Louis Bossart wrote:
> 
> Hi Clemens,
> 
> > If the driver can detect ELD changes, it must inform userspace that
> > the control has changed.  If there is no ELD, the _get callback should
> > return an error, and the control should have been set to unreadable.
> 
> Bear with my ignorance of controls, but how does the driver 'inform
> userspace that the control has changed'?

The control API has a notification mechanism.  You can find some
drivers calling snd_ctl_notify().


Takashi