All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: HDA: Add jack detection for HDMI
@ 2011-05-17 13:46 David Henningsson
  2011-05-17 15:46 ` Takashi Iwai
                   ` (3 more replies)
  0 siblings, 4 replies; 34+ messages in thread
From: David Henningsson @ 2011-05-17 13:46 UTC (permalink / raw)
  To: ALSA Development Mailing List, Takashi Iwai

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

Just as for headphones and microphone jacks, this patch adds reporting
of HDMI jack status through the input layer.

I considered making additional SND_JACK_* constants for HDMI and 
Displayport instead of going with SND_JACK_VIDEOOUT, but it didn't seem 
worth the additions, and breakage of compiling with old kernels, etc. 
Let me know if you think otherwise and I'll prepare a second patch for 
that.

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

[-- Attachment #2: 0001-ALSA-HDA-Add-jack-detection-for-HDMI.patch --]
[-- Type: text/x-patch, Size: 2297 bytes --]

>From d47239ae3d352b898649d0c15869dedd032ccbd7 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Tue, 17 May 2011 15:39:19 +0200
Subject: [PATCH] ALSA: HDA: Add jack detection for HDMI

Just as for headphones and microphone jacks, this patch adds reporting
of HDMI jack status through the input layer.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/hda_codec.c  |    2 ++
 sound/pci/hda/patch_hdmi.c |   15 +++++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index c63f376..8edd998 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -5055,6 +5055,8 @@ static const char *get_jack_default_name(struct hda_codec *codec, hda_nid_t nid,
 		return "Line-out";
 	case SND_JACK_HEADSET:
 		return "Headset";
+	case SND_JACK_VIDEOOUT:
+		return "HDMI/DP";
 	default:
 		return "Misc";
 	}
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 6eb209d..16c1505 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -33,6 +33,7 @@
 #include <linux/slab.h>
 #include <linux/moduleparam.h>
 #include <sound/core.h>
+#include <sound/jack.h>
 #include "hda_codec.h"
 #include "hda_local.h"
 
@@ -720,6 +721,8 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 				  &spec->sink_eld[index]);
 		/* TODO: do real things about ELD */
 	}
+
+	snd_hda_input_jack_report(codec, tag);
 }
 
 static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
@@ -919,6 +922,17 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 		return -E2BIG;
 	}
 
+#ifdef CONFIG_SND_HDA_INPUT_JACK
+	{
+		int err;
+		err = snd_hda_input_jack_add(codec, pin_nid,
+					     SND_JACK_VIDEOOUT, NULL);
+		if (err < 0)
+			return err;
+		snd_hda_input_jack_report(codec, pin_nid);
+	}
+#endif
+
 	hdmi_present_sense(codec, pin_nid, &spec->sink_eld[spec->num_pins]);
 
 	spec->pin[spec->num_pins] = pin_nid;
@@ -1120,6 +1134,7 @@ static void generic_hdmi_free(struct hda_codec *codec)
 
 	for (i = 0; i < spec->num_pins; i++)
 		snd_hda_eld_proc_free(codec, &spec->sink_eld[i]);
+	snd_hda_input_jack_free(codec);
 
 	kfree(spec);
 }
-- 
1.7.4.1


[-- Attachment #3: 0001-hda-emu-update-jack.h-to-match-latest-kernel-version.patch --]
[-- Type: text/x-patch, Size: 887 bytes --]

>From cdc4030a2028efdd1c36f2dc37402ea6971e1b12 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Tue, 17 May 2011 15:43:01 +0200
Subject: [PATCH] hda-emu: update jack.h to match latest kernel version

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 include/sound/jack.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 77247d0..a6a4f21 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -12,6 +12,9 @@ enum snd_jack_types {
 	SND_JACK_MICROPHONE	= 0x0002,
 	SND_JACK_HEADSET	= SND_JACK_HEADPHONE | SND_JACK_MICROPHONE,
 	SND_JACK_LINEOUT	= 0x0004,
+	SND_JACK_MECHANICAL	= 0x0008, /* If detected separately */
+	SND_JACK_VIDEOOUT	= 0x0010,
+	SND_JACK_AVOUT		= SND_JACK_LINEOUT | SND_JACK_VIDEOOUT,
 };
 
 struct snd_jack {
-- 
1.7.4.1


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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 13:46 [PATCH] ALSA: HDA: Add jack detection for HDMI David Henningsson
@ 2011-05-17 15:46 ` Takashi Iwai
  2011-05-19  9:55   ` David Henningsson
  2011-05-17 16:00 ` Stephen Warren
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 34+ messages in thread
From: Takashi Iwai @ 2011-05-17 15:46 UTC (permalink / raw)
  To: David Henningsson; +Cc: ALSA Development Mailing List

At Tue, 17 May 2011 15:46:43 +0200,
David Henningsson wrote:
> 
> Just as for headphones and microphone jacks, this patch adds reporting
> of HDMI jack status through the input layer.
> 
> I considered making additional SND_JACK_* constants for HDMI and 
> Displayport instead of going with SND_JACK_VIDEOOUT, but it didn't seem 
> worth the additions, and breakage of compiling with old kernels, etc. 
> Let me know if you think otherwise and I'll prepare a second patch for 
> that.

Did you test it with the actual machine, right?
If it's working, I'm fine to add it.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 13:46 [PATCH] ALSA: HDA: Add jack detection for HDMI David Henningsson
  2011-05-17 15:46 ` Takashi Iwai
@ 2011-05-17 16:00 ` Stephen Warren
  2011-05-17 16:08   ` Takashi Iwai
  2011-05-18 10:02 ` Takashi Iwai
  2011-05-20 21:59 ` Stephen Warren
  3 siblings, 1 reply; 34+ messages in thread
From: Stephen Warren @ 2011-05-17 16:00 UTC (permalink / raw)
  To: David Henningsson, ALSA Development Mailing List, Takashi Iwai

David Henningsson wrote at Tuesday, May 17, 2011 7:47 AM:
> Just as for headphones and microphone jacks, this patch adds reporting
> of HDMI jack status through the input layer.

Interesting. I was asked to look into this, but you beat me to it; thanks!

A couple questions though:

a) Is it possible to report more information alongside the plug events,
such as ELD/EDID content? Or, is the idea that the kernel sends a plug
event, and then user-space retrieves that information via some other
API? I don't think there's an API to retrieve ELD information at present
though right? Although certainly it'd make sense for that to be a 
completely separate patch.

b) We might discuss how to tie ALSA jacks/ports/PCMs to X displays in
some way, so that we can represent to the user which specific active
monitor is the one that supports audio (e.g. in a 2-head setup with both
monitors connected over HDMI, yet only 1 monitor supports audio).

I note that the ELD data contains a port_id field, which might be usable
for this purpose. Could this contain an xrandr value, or an NV-CTRL
(NVIDIA's proprietary control protocol) display ID, or something similar?

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 16:00 ` Stephen Warren
@ 2011-05-17 16:08   ` Takashi Iwai
  2011-05-17 17:09     ` pl bossart
  0 siblings, 1 reply; 34+ messages in thread
From: Takashi Iwai @ 2011-05-17 16:08 UTC (permalink / raw)
  To: Stephen Warren; +Cc: ALSA Development Mailing List, David Henningsson

At Tue, 17 May 2011 09:00:51 -0700,
Stephen Warren wrote:
> 
> David Henningsson wrote at Tuesday, May 17, 2011 7:47 AM:
> > Just as for headphones and microphone jacks, this patch adds reporting
> > of HDMI jack status through the input layer.
> 
> Interesting. I was asked to look into this, but you beat me to it; thanks!
> 
> A couple questions though:
> 
> a) Is it possible to report more information alongside the plug events,
> such as ELD/EDID content? Or, is the idea that the kernel sends a plug
> event, and then user-space retrieves that information via some other
> API? I don't think there's an API to retrieve ELD information at present
> though right? Although certainly it'd make sense for that to be a 
> completely separate patch.

A simple approach would be adding a control element containing
byte-array of ELD/EDID.

> b) We might discuss how to tie ALSA jacks/ports/PCMs to X displays in
> some way, so that we can represent to the user which specific active
> monitor is the one that supports audio (e.g. in a 2-head setup with both
> monitors connected over HDMI, yet only 1 monitor supports audio).

Yeah, most tight coupling with display vs audio-port is needed.

> I note that the ELD data contains a port_id field, which might be usable
> for this purpose. Could this contain an xrandr value, or an NV-CTRL
> (NVIDIA's proprietary control protocol) display ID, or something similar?

Hm, rather it's a question to X side, i.e. what information is exposed
via xrandr.


Takashi

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 16:08   ` Takashi Iwai
@ 2011-05-17 17:09     ` pl bossart
  2011-05-17 17:31       ` Takashi Iwai
  0 siblings, 1 reply; 34+ messages in thread
From: pl bossart @ 2011-05-17 17:09 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: ALSA Development Mailing List, Stephen Warren, David Henningsson

>> a) Is it possible to report more information alongside the plug events,
>> such as ELD/EDID content? Or, is the idea that the kernel sends a plug
>> event, and then user-space retrieves that information via some other
>> API? I don't think there's an API to retrieve ELD information at present
>> though right? Although certainly it'd make sense for that to be a
>> completely separate patch.
>
> A simple approach would be adding a control element containing
> byte-array of ELD/EDID.

Are there any examples of such controls? Or are we talking about a new
kind of control?
We could really use this ELD information in PulseAudio now that the
passthrough mode is in git master; for now the codecs supported by the
receiver are hard-coded, not a very reliable solution...
-Pierre

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 17:09     ` pl bossart
@ 2011-05-17 17:31       ` Takashi Iwai
  2011-05-17 20:51         ` pl bossart
  2011-05-18 15:43         ` pl bossart
  0 siblings, 2 replies; 34+ messages in thread
From: Takashi Iwai @ 2011-05-17 17:31 UTC (permalink / raw)
  To: pl bossart
  Cc: ALSA Development Mailing List, Stephen Warren, David Henningsson

At Tue, 17 May 2011 12:09:47 -0500,
pl bossart wrote:
> 
> >> a) Is it possible to report more information alongside the plug events,
> >> such as ELD/EDID content? Or, is the idea that the kernel sends a plug
> >> event, and then user-space retrieves that information via some other
> >> API? I don't think there's an API to retrieve ELD information at present
> >> though right? Although certainly it'd make sense for that to be a
> >> completely separate patch.
> >
> > A simple approach would be adding a control element containing
> > byte-array of ELD/EDID.
> 
> Are there any examples of such controls? Or are we talking about a new
> kind of control?

Look for SNDRV_CTL_ELEM_TYPE_BYTES.  Some codecs provide these.

> We could really use this ELD information in PulseAudio now that the
> passthrough mode is in git master; for now the codecs supported by the
> receiver are hard-coded, not a very reliable solution...


Takashi

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 17:31       ` Takashi Iwai
@ 2011-05-17 20:51         ` pl bossart
  2011-05-17 21:42           ` Stephen Warren
  2011-05-18 15:43         ` pl bossart
  1 sibling, 1 reply; 34+ messages in thread
From: pl bossart @ 2011-05-17 20:51 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: ALSA Development Mailing List, Stephen Warren, David Henningsson

>> >> a) Is it possible to report more information alongside the plug events,
>> >> such as ELD/EDID content? Or, is the idea that the kernel sends a plug
>> >> event, and then user-space retrieves that information via some other
>> >> API? I don't think there's an API to retrieve ELD information at present
>> >> though right? Although certainly it'd make sense for that to be a
>> >> completely separate patch.
>> >
>> > A simple approach would be adding a control element containing
>> > byte-array of ELD/EDID.
>>
>> Are there any examples of such controls? Or are we talking about a new
>> kind of control?
>
> Look for SNDRV_CTL_ELEM_TYPE_BYTES.  Some codecs provide these.

Thanks for the pointer. looks simple enough to expose the ELD bytes.
This type of element can store up to 512 bytes, enough to store the
ELD header+baseline fields (260 bytes tops). I don't think userspace
would want to muck with vendor-specific information?
we may need an array of ELD controls in case there are several
monitors. Not sure how to represent which one is actually used. The
ELD is linked to a specific nid (node id), we'd need to link this to
the audio device #?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 20:51         ` pl bossart
@ 2011-05-17 21:42           ` Stephen Warren
  2011-05-17 22:11             ` pl bossart
  0 siblings, 1 reply; 34+ messages in thread
From: Stephen Warren @ 2011-05-17 21:42 UTC (permalink / raw)
  To: pl bossart, Takashi Iwai
  Cc: ALSA Development Mailing List, Henningsson, David

pl bossart wrote at Tuesday, May 17, 2011 2:51 PM:
> >> >> a) Is it possible to report more information alongside the plug events,
> >> >> such as ELD/EDID content? Or, is the idea that the kernel sends a plug
> >> >> event, and then user-space retrieves that information via some other
> >> >> API? I don't think there's an API to retrieve ELD information at present
> >> >> though right? Although certainly it'd make sense for that to be a
> >> >> completely separate patch.
> >> >
> >> > A simple approach would be adding a control element containing
> >> > byte-array of ELD/EDID.
> >>
> >> Are there any examples of such controls? Or are we talking about a new
> >> kind of control?
> >
> > Look for SNDRV_CTL_ELEM_TYPE_BYTES.  Some codecs provide these.
> 
> Thanks for the pointer. looks simple enough to expose the ELD bytes.
> This type of element can store up to 512 bytes, enough to store the
> ELD header+baseline fields (260 bytes tops). I don't think userspace
> would want to muck with vendor-specific information?

Isn't the ELD limited to 256 bytes, since verb F2F has an 8-bit offset
parameter?

I think it'd be a good idea to just dump the entire ELD out to user-space
for future flexibility. It's trivial for user-space to ignore the vendor-
specific data (since it's always after the standardized data).

> we may need an array of ELD controls in case there are several
> monitors. Not sure how to represent which one is actually used. The
> ELD is linked to a specific nid (node id), we'd need to link this to
> the audio device #?

Perhaps the vendor-specific data space could be useful for matching audio
ports to X displays; there is a 64-bit Port_ID in the ELD that might be
used for that, or we could define X/Linux/Unix as the vendor, and specify
the format of the vendor-specific data in a way that defines this mapping.

That said, the internal APIs our graphics driver uses to write the ELD
is currently limited to 96 bytes (the size of the standardized section
of the ELD). I'm not sure yet if that's simply because 96 bytes was all
that was needed, or if that also ended up being encoded as a HW design
limitation.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 21:42           ` Stephen Warren
@ 2011-05-17 22:11             ` pl bossart
  2011-05-17 23:14               ` Stephen Warren
  0 siblings, 1 reply; 34+ messages in thread
From: pl bossart @ 2011-05-17 22:11 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Takashi Iwai, ALSA Development Mailing List, David Henningsson

> Isn't the ELD limited to 256 bytes, since verb F2F has an 8-bit offset
> parameter?

I guess you are right. I saw some checks in the hda code where the ELD
size is gt PAGE_SIZE. Confusing really.

> I think it'd be a good idea to just dump the entire ELD out to user-space
> for future flexibility. It's trivial for user-space to ignore the vendor-
> specific data (since it's always after the standardized data).

Right, if this fits in 512 bytes no reason to drop anything.

> Perhaps the vendor-specific data space could be useful for matching audio
> ports to X displays; there is a 64-bit Port_ID in the ELD that might be
> used for that, or we could define X/Linux/Unix as the vendor, and specify
> the format of the vendor-specific data in a way that defines this mapping.

Sounds complicated...The spec says this PortID field is "a ""canned""
field that would be populated through implementation specific mean of
default programming before the graphic driver is loaded". Duh?
The standard "Monitor Name String" may be easier to use...

> That said, the internal APIs our graphics driver uses to write the ELD
> is currently limited to 96 bytes (the size of the standardized section
> of the ELD). I'm not sure yet if that's simply because 96 bytes was all
> that was needed, or if that also ended up being encoded as a HW design
> limitation.

Looks like the max for ELDv2 is 80 bytes for the baseline+4 for the header.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 22:11             ` pl bossart
@ 2011-05-17 23:14               ` Stephen Warren
  0 siblings, 0 replies; 34+ messages in thread
From: Stephen Warren @ 2011-05-17 23:14 UTC (permalink / raw)
  To: pl bossart; +Cc: Takashi Iwai, ALSA Development Mailing List, David Henningsson

pl bossart wrote at Tuesday, May 17, 2011 4:12 PM:
>...
> > Perhaps the vendor-specific data space could be useful for matching audio
> > ports to X displays; there is a 64-bit Port_ID in the ELD that might be
> > used for that, or we could define X/Linux/Unix as the vendor, and specify
> > the format of the vendor-specific data in a way that defines this mapping.
> 
> Sounds complicated...The spec says this PortID field is "a ""canned""
> field that would be populated through implementation specific mean of
> default programming before the graphic driver is loaded". Duh?

I'm pretty sure our HW doesn't initialize this to anything meaningful before
the video driver runs.

> The standard "Monitor Name String" may be easier to use...

Do you mean co-opt this field and store alternate data in it? The real
monitor name for both monitors in my dual-head setup are the same...

> > That said, the internal APIs our graphics driver uses to write the ELD
> > is currently limited to 96 bytes (the size of the standardized section
> > of the ELD). I'm not sure yet if that's simply because 96 bytes was all
> > that was needed, or if that also ended up being encoded as a HW design
> > limitation.
> 
> Looks like the max for ELDv2 is 80 bytes for the baseline+4 for the
> header.

Oh yeah; I'd calculated assuming SADs were 4 bytes but they're 3.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 13:46 [PATCH] ALSA: HDA: Add jack detection for HDMI David Henningsson
  2011-05-17 15:46 ` Takashi Iwai
  2011-05-17 16:00 ` Stephen Warren
@ 2011-05-18 10:02 ` Takashi Iwai
  2011-05-20 21:59 ` Stephen Warren
  3 siblings, 0 replies; 34+ messages in thread
From: Takashi Iwai @ 2011-05-18 10:02 UTC (permalink / raw)
  To: David Henningsson; +Cc: ALSA Development Mailing List

At Tue, 17 May 2011 15:46:43 +0200,
David Henningsson wrote:
> 
>  static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
> @@ -919,6 +922,17 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>  		return -E2BIG;
>  	}
>  
> +#ifdef CONFIG_SND_HDA_INPUT_JACK
> +	{
> +		int err;
> +		err = snd_hda_input_jack_add(codec, pin_nid,
> +					     SND_JACK_VIDEOOUT, NULL);
> +		if (err < 0)
> +			return err;
> +		snd_hda_input_jack_report(codec, pin_nid);
> +	}
> +#endif

You don't need ifdef here since dummy functions are defined when
CONFIG_SND_HDA_INPUT_JACK=n.  Then don't need to make a block here,
as you won't get an unused-variable compile warning.


Takashi

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 17:31       ` Takashi Iwai
  2011-05-17 20:51         ` pl bossart
@ 2011-05-18 15:43         ` pl bossart
  2011-05-18 15:49           ` Takashi Iwai
  1 sibling, 1 reply; 34+ messages in thread
From: pl bossart @ 2011-05-18 15:43 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: ALSA Development Mailing List, Stephen Warren, David Henningsson

>> > A simple approach would be adding a control element containing
>> > byte-array of ELD/EDID.
>>
>> Are there any examples of such controls? Or are we talking about a new
>> kind of control?
>
> Look for SNDRV_CTL_ELEM_TYPE_BYTES.  Some codecs provide these.

I started doing some work on this, adding a persistent 256-byte buffer
to store the ELD data instead of dynamically allocating and freeing
the buffer. I am kind of lost with the controls though, never looked
at this part of ALSA before.
In all the HDA code, all controls have an IFACE_MIXER type, while all
the code that uses ELEM_TYPE_BYTES the type is IFACE_PCM. Does this
matter at all?
Likewise, there's a 'generic_hdmi_build_controls' routine in which I
could add the ELD control, but it uses cvt structures. I think it
would make more sense to relate ELD controls to pins, as done with the
proc information.
Quite a learning experience...

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-18 15:43         ` pl bossart
@ 2011-05-18 15:49           ` Takashi Iwai
  0 siblings, 0 replies; 34+ messages in thread
From: Takashi Iwai @ 2011-05-18 15:49 UTC (permalink / raw)
  To: pl bossart
  Cc: ALSA Development Mailing List, Stephen Warren, David Henningsson

At Wed, 18 May 2011 10:43:36 -0500,
pl bossart wrote:
> 
> >> > A simple approach would be adding a control element containing
> >> > byte-array of ELD/EDID.
> >>
> >> Are there any examples of such controls? Or are we talking about a new
> >> kind of control?
> >
> > Look for SNDRV_CTL_ELEM_TYPE_BYTES.  Some codecs provide these.
> 
> I started doing some work on this, adding a persistent 256-byte buffer
> to store the ELD data instead of dynamically allocating and freeing
> the buffer. I am kind of lost with the controls though, never looked
> at this part of ALSA before.
> In all the HDA code, all controls have an IFACE_MIXER type, while all
> the code that uses ELEM_TYPE_BYTES the type is IFACE_PCM. Does this
> matter at all?

Not much.  It's fine to keep it in IFACE_MIXER.
But, IFACE_PCM might be a better choice since ELD is actually bound
with codec-port, i.e. a PCM stream, and all mixer apps look at these
unparseable IFACE_MIXER elements.

> Likewise, there's a 'generic_hdmi_build_controls' routine in which I
> could add the ELD control, but it uses cvt structures. I think it
> would make more sense to relate ELD controls to pins, as done with the
> proc information.

Yeah, it'd make more sense.  Currently it's a bit messy there, as we
handle cvt and pin in mixture somehow.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 15:46 ` Takashi Iwai
@ 2011-05-19  9:55   ` David Henningsson
  2011-05-19 10:06     ` Takashi Iwai
  2011-05-19 16:57     ` Stephen Warren
  0 siblings, 2 replies; 34+ messages in thread
From: David Henningsson @ 2011-05-19  9:55 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA Development Mailing List

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

On 2011-05-17 17:46, Takashi Iwai wrote:
> At Tue, 17 May 2011 15:46:43 +0200,
> David Henningsson wrote:
>>
>> Just as for headphones and microphone jacks, this patch adds reporting
>> of HDMI jack status through the input layer.
>>
>> I considered making additional SND_JACK_* constants for HDMI and
>> Displayport instead of going with SND_JACK_VIDEOOUT, but it didn't seem
>> worth the additions, and breakage of compiling with old kernels, etc.
>> Let me know if you think otherwise and I'll prepare a second patch for
>> that.
>
> Did you test it with the actual machine, right?
> If it's working, I'm fine to add it.

To be honest; it's partially working, or rather it's working in the 
sense that it follows the eld proc file. It's also working in hda-emu.

I've tried it on one Nvidia (with binary drivers), and one Intel 
Graphics and well, and both seem to have the same problem essentially: 
There is no hotplug event coming in (through hdmi_unsol_event) when a 
monitor is removed. But with this patch in perhaps the graphics driver 
writers will feel more motivated to fix it? :-)

Note that the hotplug event is not coming in when you actually plug the 
cable but when you detect displays and/or apply the monitor 
configuration change.

I'm attaching a new version of the patch according to your preference, 
in hopes that it will be in 2.6.40.

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

[-- Attachment #2: 0001-ALSA-HDA-Add-jack-detection-for-HDMI.patch --]
[-- Type: text/x-patch, Size: 2514 bytes --]

>From 0be95828e59250af983c86b3ec49d1ea81d8ec82 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Thu, 19 May 2011 11:46:03 +0200
Subject: [PATCH] ALSA: HDA: Add jack detection for HDMI

Just as for headphones and microphone jacks, this patch adds reporting
of HDMI jack status through the input layer.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/hda_codec.c  |    2 ++
 sound/pci/hda/patch_hdmi.c |   11 +++++++++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index c63f376..8edd998 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -5055,6 +5055,8 @@ static const char *get_jack_default_name(struct hda_codec *codec, hda_nid_t nid,
 		return "Line-out";
 	case SND_JACK_HEADSET:
 		return "Headset";
+	case SND_JACK_VIDEOOUT:
+		return "HDMI/DP";
 	default:
 		return "Misc";
 	}
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 6eb209d..7348296 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -33,6 +33,7 @@
 #include <linux/slab.h>
 #include <linux/moduleparam.h>
 #include <sound/core.h>
+#include <sound/jack.h>
 #include "hda_codec.h"
 #include "hda_local.h"
 
@@ -720,6 +721,8 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 				  &spec->sink_eld[index]);
 		/* TODO: do real things about ELD */
 	}
+
+	snd_hda_input_jack_report(codec, tag);
 }
 
 static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
@@ -912,6 +915,7 @@ static void hdmi_present_sense(struct hda_codec *codec, hda_nid_t pin_nid,
 static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 {
 	struct hdmi_spec *spec = codec->spec;
+	int err;
 
 	if (spec->num_pins >= MAX_HDMI_PINS) {
 		snd_printk(KERN_WARNING
@@ -919,6 +923,12 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 		return -E2BIG;
 	}
 
+	err = snd_hda_input_jack_add(codec, pin_nid,
+				     SND_JACK_VIDEOOUT, NULL);
+	if (err < 0)
+		return err;
+	snd_hda_input_jack_report(codec, pin_nid);
+
 	hdmi_present_sense(codec, pin_nid, &spec->sink_eld[spec->num_pins]);
 
 	spec->pin[spec->num_pins] = pin_nid;
@@ -1120,6 +1130,7 @@ static void generic_hdmi_free(struct hda_codec *codec)
 
 	for (i = 0; i < spec->num_pins; i++)
 		snd_hda_eld_proc_free(codec, &spec->sink_eld[i]);
+	snd_hda_input_jack_free(codec);
 
 	kfree(spec);
 }
-- 
1.7.4.1


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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-19  9:55   ` David Henningsson
@ 2011-05-19 10:06     ` Takashi Iwai
  2011-05-19 10:24       ` Setting invalid samplerate Torsten Schenk
  2011-05-23 21:49       ` [PATCH] ALSA: HDA: Add jack detection for HDMI Stephen Warren
  2011-05-19 16:57     ` Stephen Warren
  1 sibling, 2 replies; 34+ messages in thread
From: Takashi Iwai @ 2011-05-19 10:06 UTC (permalink / raw)
  To: David Henningsson; +Cc: ALSA Development Mailing List

At Thu, 19 May 2011 11:55:21 +0200,
David Henningsson wrote:
> 
> On 2011-05-17 17:46, Takashi Iwai wrote:
> > At Tue, 17 May 2011 15:46:43 +0200,
> > David Henningsson wrote:
> >>
> >> Just as for headphones and microphone jacks, this patch adds reporting
> >> of HDMI jack status through the input layer.
> >>
> >> I considered making additional SND_JACK_* constants for HDMI and
> >> Displayport instead of going with SND_JACK_VIDEOOUT, but it didn't seem
> >> worth the additions, and breakage of compiling with old kernels, etc.
> >> Let me know if you think otherwise and I'll prepare a second patch for
> >> that.
> >
> > Did you test it with the actual machine, right?
> > If it's working, I'm fine to add it.
> 
> To be honest; it's partially working, or rather it's working in the 
> sense that it follows the eld proc file. It's also working in hda-emu.
> 
> I've tried it on one Nvidia (with binary drivers), and one Intel 
> Graphics and well, and both seem to have the same problem essentially: 
> There is no hotplug event coming in (through hdmi_unsol_event) when a 
> monitor is removed. But with this patch in perhaps the graphics driver 
> writers will feel more motivated to fix it? :-)
> 
> Note that the hotplug event is not coming in when you actually plug the 
> cable but when you detect displays and/or apply the monitor 
> configuration change.
> 
> I'm attaching a new version of the patch according to your preference, 
> in hopes that it will be in 2.6.40.

OK, as the jack report itself doesn't play a big role yet so far for
HD-audio, let's get it in, and give pressure to graphics guys :)

The remove event itself is detected in the graphic side (sent as udev
event), so it should be possible to propagate it somehow.  But, I'm
not quite sure whether this is an issue in the video driver code, or
it's a hardware design.


thanks,

Takashi


> 
> -- 
> David Henningsson, Canonical Ltd.
> http://launchpad.net/~diwic
> [2 0001-ALSA-HDA-Add-jack-detection-for-HDMI.patch <text/x-patch (7bit)>]
> >From 0be95828e59250af983c86b3ec49d1ea81d8ec82 Mon Sep 17 00:00:00 2001
> From: David Henningsson <david.henningsson@canonical.com>
> Date: Thu, 19 May 2011 11:46:03 +0200
> Subject: [PATCH] ALSA: HDA: Add jack detection for HDMI
> 
> Just as for headphones and microphone jacks, this patch adds reporting
> of HDMI jack status through the input layer.
> 
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  sound/pci/hda/hda_codec.c  |    2 ++
>  sound/pci/hda/patch_hdmi.c |   11 +++++++++++
>  2 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index c63f376..8edd998 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -5055,6 +5055,8 @@ static const char *get_jack_default_name(struct hda_codec *codec, hda_nid_t nid,
>  		return "Line-out";
>  	case SND_JACK_HEADSET:
>  		return "Headset";
> +	case SND_JACK_VIDEOOUT:
> +		return "HDMI/DP";
>  	default:
>  		return "Misc";
>  	}
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 6eb209d..7348296 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -33,6 +33,7 @@
>  #include <linux/slab.h>
>  #include <linux/moduleparam.h>
>  #include <sound/core.h>
> +#include <sound/jack.h>
>  #include "hda_codec.h"
>  #include "hda_local.h"
>  
> @@ -720,6 +721,8 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
>  				  &spec->sink_eld[index]);
>  		/* TODO: do real things about ELD */
>  	}
> +
> +	snd_hda_input_jack_report(codec, tag);
>  }
>  
>  static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
> @@ -912,6 +915,7 @@ static void hdmi_present_sense(struct hda_codec *codec, hda_nid_t pin_nid,
>  static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>  {
>  	struct hdmi_spec *spec = codec->spec;
> +	int err;
>  
>  	if (spec->num_pins >= MAX_HDMI_PINS) {
>  		snd_printk(KERN_WARNING
> @@ -919,6 +923,12 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>  		return -E2BIG;
>  	}
>  
> +	err = snd_hda_input_jack_add(codec, pin_nid,
> +				     SND_JACK_VIDEOOUT, NULL);
> +	if (err < 0)
> +		return err;
> +	snd_hda_input_jack_report(codec, pin_nid);
> +
>  	hdmi_present_sense(codec, pin_nid, &spec->sink_eld[spec->num_pins]);
>  
>  	spec->pin[spec->num_pins] = pin_nid;
> @@ -1120,6 +1130,7 @@ static void generic_hdmi_free(struct hda_codec *codec)
>  
>  	for (i = 0; i < spec->num_pins; i++)
>  		snd_hda_eld_proc_free(codec, &spec->sink_eld[i]);
> +	snd_hda_input_jack_free(codec);
>  
>  	kfree(spec);
>  }
> -- 
> 1.7.4.1
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Setting invalid samplerate
  2011-05-19 10:06     ` Takashi Iwai
@ 2011-05-19 10:24       ` Torsten Schenk
  2011-05-19 10:32         ` Torsten Schenk
  2011-05-19 10:55         ` Clemens Ladisch
  2011-05-23 21:49       ` [PATCH] ALSA: HDA: Add jack detection for HDMI Stephen Warren
  1 sibling, 2 replies; 34+ messages in thread
From: Torsten Schenk @ 2011-05-19 10:24 UTC (permalink / raw)
  To: alsa-devel

Hello all,

I'm working on the driver for the dmx 6fire usb. On my sourceforge page, users have acknowledged that the driver is working fine. I don't know whether the alsa mailing list is the correct one; but since Ubuntu 11.04, I get complaints about the card not working anymore, dmesg says "invalid samplerate 64000 in prepare". I could reproduce the error on my own Ubuntu. I started to wonder why pulseaudio wants to use 64kHz since the card doesn't support it and I also didn't enable the SNDRV_PCM_RATE_64000 flag.

The hardware description is as follows:
	.rates = SNDRV_PCM_RATE_44100 |
		SNDRV_PCM_RATE_48000 |
		SNDRV_PCM_RATE_88200 |
		SNDRV_PCM_RATE_96000 |
		SNDRV_PCM_RATE_176400 |
		SNDRV_PCM_RATE_192000,

	.rate_min = 44100,
	.rate_max = 192000,

I wrote a small test program to see what happens if I try to set a samplerate of 48000. set_rate_near always returned 64kHz. A nasty workaround is to enable just 88.2kHz. But then not all programs run (f.ex. milkytracker refused to playback sound). So what is going on here and what could be done about it?

Thanks,
Torsten

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Setting invalid samplerate
  2011-05-19 10:24       ` Setting invalid samplerate Torsten Schenk
@ 2011-05-19 10:32         ` Torsten Schenk
  2011-05-19 10:55         ` Clemens Ladisch
  1 sibling, 0 replies; 34+ messages in thread
From: Torsten Schenk @ 2011-05-19 10:32 UTC (permalink / raw)
  To: alsa-devel

As addition: I used the small tool I found somewhere in the internet to display all samplerates that alsa allows to be set on the card. The result was:

Device: hw:1 (type: HW)
Access types: MMAP_INTERLEAVED RW_INTERLEAVED
Formats: S24_LE S32_LE
Channels: 1 2 3 4 5 6
Sample rates: 64000 88200 192000
Interrupt interval: 1041-256000 us
Buffer size: 2083-512000 us

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Setting invalid samplerate
  2011-05-19 10:24       ` Setting invalid samplerate Torsten Schenk
  2011-05-19 10:32         ` Torsten Schenk
@ 2011-05-19 10:55         ` Clemens Ladisch
  2011-05-19 11:28           ` Torsten Schenk
  1 sibling, 1 reply; 34+ messages in thread
From: Clemens Ladisch @ 2011-05-19 10:55 UTC (permalink / raw)
  To: Torsten Schenk, Dan Carpenter; +Cc: alsa-devel

Torsten Schenk wrote:
> I'm working on the driver for the dmx 6fire usb.  ... 
> since Ubuntu 11.04, I get complaints about the card not working anymore,

I'd guess that would be commit 82d90313980b:
| ALSA: USB: 6fire: signedness bug in usb6fire_pcm_prepare()
| 
| rt->rate is an unsigned char so it's never equal to -1.  It's not a huge
| problem because the invalid rate is caught inside the call to
| usb6fire_pcm_set_rate() which returns -EINVAL.  But if we fix the test
| then it prints out the correct error message so that's good.
| 
| Signed-off-by: Dan Carpenter <error27@gmail.com>
| 
| --- a/sound/usb/6fire/pcm.c
| +++ b/sound/usb/6fire/pcm.c
| @@ -493,13 +493,12 @@ static int usb6fire_pcm_prepare(struct snd_pcm_substream *alsa_sub)
|  	sub->period_off = 0;
|  
|  	if (rt->stream_state == STREAM_DISABLED) {
| -		rt->rate = -1;
|  		for (i = 0; i < ARRAY_SIZE(rates); i++)
|  			if (alsa_rt->rate == rates[i]) {
|  				rt->rate = i;
|  				break;
|  			}
| -		if (rt->rate == -1) {
| +		if (i == ARRAY_SIZE(rates)) {
|  			mutex_unlock(&rt->stream_mutex);
|  			snd_printk("invalid rate %d in prepare.\n",
|  					alsa_rt->rate);

This fixed not a bug but just a symptom of the actual bug, that rt->rate
is not signed.

There are still "rt->rate = -1" initializations, and usb6fire_pcm_open()
then does:

	if (rt->rate >= 0)
		alsa_rt->hw.rates = rates_alsaid[rt->rate];


Regards,
Clemens

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Setting invalid samplerate
  2011-05-19 10:55         ` Clemens Ladisch
@ 2011-05-19 11:28           ` Torsten Schenk
  2011-05-19 11:36             ` Daniel Mack
  0 siblings, 1 reply; 34+ messages in thread
From: Torsten Schenk @ 2011-05-19 11:28 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: alsa-devel, Dan Carpenter

That did the job indeed. I still wonder why it worked in so many cases... However, I will soon create a patch and submit it.

Thanks,
Torsten

---- On Thu, 19 May 2011 12:55:28 +0200 Clemens Ladisch  wrote ---- 

>Torsten Schenk wrote: 
>> I'm working on the driver for the dmx 6fire usb. ... 
>> since Ubuntu 11.04, I get complaints about the card not working anymore, 
> 
>I'd guess that would be commit 82d90313980b: 
>| ALSA: USB: 6fire: signedness bug in usb6fire_pcm_prepare() 
>| 
>| rt->rate is an unsigned char so it's never equal to -1. It's not a huge 
>| problem because the invalid rate is caught inside the call to 
>| usb6fire_pcm_set_rate() which returns -EINVAL. But if we fix the test 
>| then it prints out the correct error message so that's good. 
>| 
>| Signed-off-by: Dan Carpenter  
>| 
>| --- a/sound/usb/6fire/pcm.c 
>| +++ b/sound/usb/6fire/pcm.c 
>| @@ -493,13 +493,12 @@ static int usb6fire_pcm_prepare(struct snd_pcm_substream *alsa_sub) 
>|     sub->period_off = 0; 
>| 
>|     if (rt->stream_state == STREAM_DISABLED) { 
>| -        rt->rate = -1; 
>|         for (i = 0; i < ARRAY_SIZE(rates); i++) 
>|             if (alsa_rt->rate == rates[i]) { 
>|                 rt->rate = i; 
>|                 break; 
>|             } 
>| -        if (rt->rate == -1) { 
>| +        if (i == ARRAY_SIZE(rates)) { 
>|             mutex_unlock(&rt->stream_mutex); 
>|             snd_printk("invalid rate %d in prepare.", 
>|                     alsa_rt->rate); 
> 
>This fixed not a bug but just a symptom of the actual bug, that rt->rate 
>is not signed. 
> 
>There are still "rt->rate = -1" initializations, and usb6fire_pcm_open() 
>then does: 
> 
>    if (rt->rate >= 0) 
>        alsa_rt->hw.rates = rates_alsaid[rt->rate]; 
> 
> 
>Regards, 
>Clemens 
>_______________________________________________ 
>Alsa-devel mailing list 
>Alsa-devel@alsa-project.org 
>http://mailman.alsa-project.org/mailman/listinfo/alsa-devel 
>

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Setting invalid samplerate
  2011-05-19 11:28           ` Torsten Schenk
@ 2011-05-19 11:36             ` Daniel Mack
  0 siblings, 0 replies; 34+ messages in thread
From: Daniel Mack @ 2011-05-19 11:36 UTC (permalink / raw)
  To: Torsten Schenk; +Cc: alsa-devel, Clemens Ladisch, Dan Carpenter

On Thu, May 19, 2011 at 1:28 PM, Torsten Schenk <torsten.schenk@zoho.com> wrote:
> That did the job indeed. I still wonder why it worked in so many cases... However, I will soon create a patch and submit it.

Remember to add a "Cc: stable@kernel.org" in the commit log if you
want to have it backported to stable series of the kernel.

Daniel

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-19  9:55   ` David Henningsson
  2011-05-19 10:06     ` Takashi Iwai
@ 2011-05-19 16:57     ` Stephen Warren
  2011-05-19 22:45       ` David Henningsson
  1 sibling, 1 reply; 34+ messages in thread
From: Stephen Warren @ 2011-05-19 16:57 UTC (permalink / raw)
  To: David Henningsson, Takashi Iwai; +Cc: ALSA Development Mailing List

David Henningsson wrote at Thursday, May 19, 2011 3:55 AM:
> On 2011-05-17 17:46, Takashi Iwai wrote:
> > At Tue, 17 May 2011 15:46:43 +0200,
> > David Henningsson wrote:
> >>
> >> Just as for headphones and microphone jacks, this patch adds reporting
> >> of HDMI jack status through the input layer.
>...
> 
> To be honest; it's partially working, or rather it's working in the
> sense that it follows the eld proc file. It's also working in hda-emu.
> 
> I've tried it on one Nvidia (with binary drivers), and one Intel
> Graphics and well, and both seem to have the same problem essentially:
> There is no hotplug event coming in (through hdmi_unsol_event) when a
> monitor is removed. But with this patch in perhaps the graphics driver
> writers will feel more motivated to fix it? :-)
> 
> Note that the hotplug event is not coming in when you actually plug the
> cable but when you detect displays and/or apply the monitor
> configuration change.

That's certainly the expected behavior of our drivers at present.

The ELD data and PD/ELDV bits are only updated when we perform a modeset
operation, which only happens in response to an explicit user request,
either with XRandR or through our nvidia-settings application.

I'm not sure if there's a standard that says what our driver's behavior
should be or not; I could argue that the current behavior makes sense to
some degree, so that if the user unplugs their monitor then replugs it,
we don't end up reprogramming all the display hardware. If we did
disable the HDMI output path when a monitor was unplugged, given that I
think the idea is that new paths don't get auto-programmed by the X
driver when a new device is plugged in, but instead wait until XRandR is
used to configure them, in which case if we'd just turned off all the
display outputs, how would the user configure the new one without being
able to see the desktop?

(While this may or may not be ideal, I'm simply pointing this out to
indicate this behavior doesn't indicate any issue with David's patch)

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-19 16:57     ` Stephen Warren
@ 2011-05-19 22:45       ` David Henningsson
  2011-05-19 22:51         ` Stephen Warren
  0 siblings, 1 reply; 34+ messages in thread
From: David Henningsson @ 2011-05-19 22:45 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Takashi Iwai, ALSA Development Mailing List

On 2011-05-19 18:57, Stephen Warren wrote:
> David Henningsson wrote at Thursday, May 19, 2011 3:55 AM:
>> On 2011-05-17 17:46, Takashi Iwai wrote:
>>> At Tue, 17 May 2011 15:46:43 +0200,
>>> David Henningsson wrote:
>>>>
>>>> Just as for headphones and microphone jacks, this patch adds reporting
>>>> of HDMI jack status through the input layer.
>> ...
>>
>> To be honest; it's partially working, or rather it's working in the
>> sense that it follows the eld proc file. It's also working in hda-emu.
>>
>> I've tried it on one Nvidia (with binary drivers), and one Intel
>> Graphics and well, and both seem to have the same problem essentially:
>> There is no hotplug event coming in (through hdmi_unsol_event) when a
>> monitor is removed. But with this patch in perhaps the graphics driver
>> writers will feel more motivated to fix it? :-)
>>
>> Note that the hotplug event is not coming in when you actually plug the
>> cable but when you detect displays and/or apply the monitor
>> configuration change.
>
> That's certainly the expected behavior of our drivers at present.
>
> The ELD data and PD/ELDV bits are only updated when we perform a modeset
> operation, which only happens in response to an explicit user request,
> either with XRandR or through our nvidia-settings application.
>
> I'm not sure if there's a standard that says what our driver's behavior
> should be or not; I could argue that the current behavior makes sense to
> some degree, so that if the user unplugs their monitor then replugs it,
> we don't end up reprogramming all the display hardware. If we did
> disable the HDMI output path when a monitor was unplugged, given that I
> think the idea is that new paths don't get auto-programmed by the X
> driver when a new device is plugged in, but instead wait until XRandR is
> used to configure them, in which case if we'd just turned off all the
> display outputs, how would the user configure the new one without being
> able to see the desktop?
>
> (While this may or may not be ideal, I'm simply pointing this out to
> indicate this behavior doesn't indicate any issue with David's patch)

Ok, thanks. Just to clarify my current problem - if I start 
nvidia-settings, I'll find my current screen through DVI and an extra 
HDMI screen that is disabled. If I choose to enable this screen (by 
clicking Apply on the "X Server Display Information" tab), I get a 
hotplug event through patch_hdmi.c:hdmi_unsol_event, telling me that the 
monitor is now present. So far, that's what's expected.
However, if I then disable that second HDMI screen and click Apply 
again, I *don't* get an hdmi_unsol_event telling me that the monitor is 
now not present. This seems inconsistent to me.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-19 22:45       ` David Henningsson
@ 2011-05-19 22:51         ` Stephen Warren
  2011-06-09 20:59           ` Stephen Warren
  0 siblings, 1 reply; 34+ messages in thread
From: Stephen Warren @ 2011-05-19 22:51 UTC (permalink / raw)
  To: David Henningsson; +Cc: Takashi Iwai, ALSA Development Mailing List

David Henningsson wrote at Thursday, May 19, 2011 4:45 PM:
>...
> Ok, thanks. Just to clarify my current problem - if I start
> nvidia-settings, I'll find my current screen through DVI and an extra
> HDMI screen that is disabled. If I choose to enable this screen (by
> clicking Apply on the "X Server Display Information" tab), I get a
> hotplug event through patch_hdmi.c:hdmi_unsol_event, telling me that the
> monitor is now present. So far, that's what's expected.
> However, if I then disable that second HDMI screen and click Apply
> again, I *don't* get an hdmi_unsol_event telling me that the monitor is
> now not present. This seems inconsistent to me.

OK, now that's definitely a bug. I'll file one internally.

Thanks for reporting this.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-17 13:46 [PATCH] ALSA: HDA: Add jack detection for HDMI David Henningsson
                   ` (2 preceding siblings ...)
  2011-05-18 10:02 ` Takashi Iwai
@ 2011-05-20 21:59 ` Stephen Warren
  2011-05-21  6:25   ` David Henningsson
  3 siblings, 1 reply; 34+ messages in thread
From: Stephen Warren @ 2011-05-20 21:59 UTC (permalink / raw)
  To: David Henningsson, ALSA Development Mailing List, Takashi Iwai

David Henningsson wrote at Tuesday, May 17, 2011 7:47 AM:
> Just as for headphones and microphone jacks, this patch adds reporting
> of HDMI jack status through the input layer.
> 
> I considered making additional SND_JACK_* constants for HDMI and
> Displayport instead of going with SND_JACK_VIDEOOUT, but it didn't seem
> worth the additions, and breakage of compiling with old kernels, etc.
> Let me know if you think otherwise and I'll prepare a second patch for
> that.

One more thought on this:

Some of our older HW (MCP6x, MCP7x chipsets) doesn't support unsolicited
responses, nor PD/ELDV/ELD data. If PulseAudio is updated to listen to
these new jack events on kernels where the input files are present,
that'll presumably cause it never to allow use of the HDMI sinks on those
chipsets. I don't know if other vendors have this issue or not.

In that case, can we do one of:

* Not create the jack/input files at all.

* Immediately mark the jack as "present".

I assume the latter would be easier for user-space to deal with; it'd be
more in line with the kernel abstracting HW differences.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-20 21:59 ` Stephen Warren
@ 2011-05-21  6:25   ` David Henningsson
  2011-05-21  7:37     ` Takashi Iwai
  2011-05-23 15:29     ` Stephen Warren
  0 siblings, 2 replies; 34+ messages in thread
From: David Henningsson @ 2011-05-21  6:25 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Takashi Iwai, ALSA Development Mailing List

On 2011-05-20 23:59, Stephen Warren wrote:
> David Henningsson wrote at Tuesday, May 17, 2011 7:47 AM:
>> Just as for headphones and microphone jacks, this patch adds reporting
>> of HDMI jack status through the input layer.
>>
>> I considered making additional SND_JACK_* constants for HDMI and
>> Displayport instead of going with SND_JACK_VIDEOOUT, but it didn't seem
>> worth the additions, and breakage of compiling with old kernels, etc.
>> Let me know if you think otherwise and I'll prepare a second patch for
>> that.
>
> One more thought on this:
>
> Some of our older HW (MCP6x, MCP7x chipsets) doesn't support unsolicited
> responses, nor PD/ELDV/ELD data. If PulseAudio is updated to listen to
> these new jack events on kernels where the input files are present,
> that'll presumably cause it never to allow use of the HDMI sinks on those
> chipsets. I don't know if other vendors have this issue or not.
>
> In that case, can we do one of:
>
> * Not create the jack/input files at all.
>
> * Immediately mark the jack as "present".
>
> I assume the latter would be easier for user-space to deal with; it'd be
> more in line with the kernel abstracting HW differences.
>

Ok, thanks for the input (no pun intended ;-) ). If they support 
presence detect, we could create a polling loop. Otherwise, don't create 
the jack.

Is this autodetectable or do we have to quirk it?

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-21  6:25   ` David Henningsson
@ 2011-05-21  7:37     ` Takashi Iwai
  2011-05-23 15:29     ` Stephen Warren
  1 sibling, 0 replies; 34+ messages in thread
From: Takashi Iwai @ 2011-05-21  7:37 UTC (permalink / raw)
  To: David Henningsson; +Cc: ALSA Development Mailing List, Stephen Warren

At Sat, 21 May 2011 08:25:57 +0200,
David Henningsson wrote:
> 
> On 2011-05-20 23:59, Stephen Warren wrote:
> > David Henningsson wrote at Tuesday, May 17, 2011 7:47 AM:
> >> Just as for headphones and microphone jacks, this patch adds reporting
> >> of HDMI jack status through the input layer.
> >>
> >> I considered making additional SND_JACK_* constants for HDMI and
> >> Displayport instead of going with SND_JACK_VIDEOOUT, but it didn't seem
> >> worth the additions, and breakage of compiling with old kernels, etc.
> >> Let me know if you think otherwise and I'll prepare a second patch for
> >> that.
> >
> > One more thought on this:
> >
> > Some of our older HW (MCP6x, MCP7x chipsets) doesn't support unsolicited
> > responses, nor PD/ELDV/ELD data. If PulseAudio is updated to listen to
> > these new jack events on kernels where the input files are present,
> > that'll presumably cause it never to allow use of the HDMI sinks on those
> > chipsets. I don't know if other vendors have this issue or not.
> >
> > In that case, can we do one of:
> >
> > * Not create the jack/input files at all.
> >
> > * Immediately mark the jack as "present".
> >
> > I assume the latter would be easier for user-space to deal with; it'd be
> > more in line with the kernel abstracting HW differences.
> >
> 
> Ok, thanks for the input (no pun intended ;-) ). If they support 
> presence detect, we could create a polling loop. Otherwise, don't create 
> the jack.

In that case, input-polldev would be an option, too.
Of course, this will need some change in the input-jack layer.


Takashi

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-21  6:25   ` David Henningsson
  2011-05-21  7:37     ` Takashi Iwai
@ 2011-05-23 15:29     ` Stephen Warren
  1 sibling, 0 replies; 34+ messages in thread
From: Stephen Warren @ 2011-05-23 15:29 UTC (permalink / raw)
  To: David Henningsson; +Cc: Takashi Iwai, ALSA Development Mailing List

David Henningsson wrote at Saturday, May 21, 2011 12:26 AM:
> On 2011-05-20 23:59, Stephen Warren wrote:
> > David Henningsson wrote at Tuesday, May 17, 2011 7:47 AM:
> >> Just as for headphones and microphone jacks, this patch adds reporting
> >> of HDMI jack status through the input layer.
> >
> > Some of our older HW (MCP6x, MCP7x chipsets) doesn't support unsolicited
> > responses, nor PD/ELDV/ELD data. If PulseAudio is updated to listen to
> > these new jack events on kernels where the input files are present,
> > that'll presumably cause it never to allow use of the HDMI sinks on those
> > chipsets. I don't know if other vendors have this issue or not.
> 
> Ok, thanks for the input (no pun intended ;-) ). If they support
> presence detect, we could create a polling loop. Otherwise, don't create
> the jack.

There's no unsolicited response, nor is there any way to read any of the
PD/ELD data as far as I know, nor any way for the graphics driver to pass
it into the audio HW I presume.

> Is this autodetectable or do we have to quirk it?

The relevant NVIDIA chips have custom .patch functions in
snd_hda_preset_hdmi[], which end up not calling snd_hda_eld_proc_new()
for the pins in question. I note that many of the ATI/AMD codecs are in
the same boat here. Presumably, that function sets up some data structure
that could easily be keyed off of.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-19 10:06     ` Takashi Iwai
  2011-05-19 10:24       ` Setting invalid samplerate Torsten Schenk
@ 2011-05-23 21:49       ` Stephen Warren
  2011-05-24  5:39         ` Takashi Iwai
  1 sibling, 1 reply; 34+ messages in thread
From: Stephen Warren @ 2011-05-23 21:49 UTC (permalink / raw)
  To: Takashi Iwai, David Henningsson; +Cc: ALSA Development Mailing List

Takashi Iwai wrote at Thursday, May 19, 2011 4:06 AM:
> At Thu, 19 May 2011 11:55:21 +0200,
> David Henningsson wrote:
> >
> > On 2011-05-17 17:46, Takashi Iwai wrote:
> > > At Tue, 17 May 2011 15:46:43 +0200,
> > > David Henningsson wrote:
> > >>
> > >> Just as for headphones and microphone jacks, this patch adds reporting
> > >> of HDMI jack status through the input layer.
> > >>
>...
> 
> OK, as the jack report itself doesn't play a big role yet so far for
> HD-audio, let's get it in, and give pressure to graphics guys :)

I've been testing a preliminary fix for this internally. I found that
the /proc ELD files correctly report monitor_present and eld_valid when
we hot unplug a monitor, but the other fields in the ELD file are not
cleared out to their default state. As best I can tell, this is an ALSA
driver issue, since our display driver only fills in the other fields
when ELDV==1.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-23 21:49       ` [PATCH] ALSA: HDA: Add jack detection for HDMI Stephen Warren
@ 2011-05-24  5:39         ` Takashi Iwai
  2011-05-24 17:27           ` Stephen Warren
  2011-05-24 21:00           ` Stephen Warren
  0 siblings, 2 replies; 34+ messages in thread
From: Takashi Iwai @ 2011-05-24  5:39 UTC (permalink / raw)
  To: Stephen Warren; +Cc: ALSA Development Mailing List, David Henningsson

At Mon, 23 May 2011 14:49:15 -0700,
Stephen Warren wrote:
> 
> Takashi Iwai wrote at Thursday, May 19, 2011 4:06 AM:
> > At Thu, 19 May 2011 11:55:21 +0200,
> > David Henningsson wrote:
> > >
> > > On 2011-05-17 17:46, Takashi Iwai wrote:
> > > > At Tue, 17 May 2011 15:46:43 +0200,
> > > > David Henningsson wrote:
> > > >>
> > > >> Just as for headphones and microphone jacks, this patch adds reporting
> > > >> of HDMI jack status through the input layer.
> > > >>
> >...
> > 
> > OK, as the jack report itself doesn't play a big role yet so far for
> > HD-audio, let's get it in, and give pressure to graphics guys :)
> 
> I've been testing a preliminary fix for this internally. I found that
> the /proc ELD files correctly report monitor_present and eld_valid when
> we hot unplug a monitor, but the other fields in the ELD file are not
> cleared out to their default state. As best I can tell, this is an ALSA
> driver issue, since our display driver only fills in the other fields
> when ELDV==1.

Then it's just showing stale data.  The eld fields are updated only
when the data is valid.  The patch below should fix the confusion.


thanks,

Takashi

---
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
index 74b0560..cd96b1d 100644
--- a/sound/pci/hda/hda_eld.c
+++ b/sound/pci/hda/hda_eld.c
@@ -477,6 +477,8 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
 
 	snd_iprintf(buffer, "monitor_present\t\t%d\n", e->monitor_present);
 	snd_iprintf(buffer, "eld_valid\t\t%d\n", e->eld_valid);
+	if (!e->eld_valid)
+		return;
 	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name);
 	snd_iprintf(buffer, "connection_type\t\t%s\n",
 				eld_connection_type_names[e->conn_type]);

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-24  5:39         ` Takashi Iwai
@ 2011-05-24 17:27           ` Stephen Warren
  2011-05-24 18:09             ` Takashi Iwai
  2011-05-24 21:00           ` Stephen Warren
  1 sibling, 1 reply; 34+ messages in thread
From: Stephen Warren @ 2011-05-24 17:27 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Mailing List, David Henningsson, ALSA

Takashi Iwai wrote at Monday, May 23, 2011 11:40 PM:
> At Mon, 23 May 2011 14:49:15 -0700, Stephen Warren wrote:
> > ...
> > I've been testing a preliminary fix for this internally. I found that
> > the /proc ELD files correctly report monitor_present and eld_valid when
> > we hot unplug a monitor, but the other fields in the ELD file are not
> > cleared out to their default state. As best I can tell, this is an ALSA
> > driver issue, since our display driver only fills in the other fields
> > when ELDV==1.
> 
> Then it's just showing stale data.  The eld fields are updated only
> when the data is valid.  The patch below should fix the confusion.

> ---
> diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
> index 74b0560..cd96b1d 100644
> --- a/sound/pci/hda/hda_eld.c
> +++ b/sound/pci/hda/hda_eld.c
> @@ -477,6 +477,8 @@ static void hdmi_print_eld_info(struct snd_info_entry
> *entry,
> 
>  	snd_iprintf(buffer, "monitor_present\t\t%d\n", e->monitor_present);
>  	snd_iprintf(buffer, "eld_valid\t\t%d\n", e->eld_valid);
> +	if (!e->eld_valid)
> +		return;
>  	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name);
>  	snd_iprintf(buffer, "connection_type\t\t%s\n",
>  				eld_connection_type_names[e->conn_type]);

Takashi,

Your patch looks fine as far as the reporting side goes.

Should we also modify hdmi_intrinsic_event() as follows:

	if (pind && eldv) {
		hdmi_get_show_eld(codec, spec->pin[index],
				  &spec->sink_eld[index]);
		/* TODO: do real things about ELD */
	}
+	else {
+		memset(&spec->sink_eld[index], 0, sizeof(spec->sink_eld[index]));
+	}

... to make sure that if something accidentally uses the ELD data
without checking eld_valid, it'll get obviously bad data instead of
valid-looking-but-stale data? Right now, this isn't an issue, but if
we start pushing the ELD into user-space through a binary control,
it'd be nice if the ELD content were consistent in this way.

Hmm. Actually, hdmi_eld isn't storing the raw ELD bytes, but a parsed
representation of them, so this may not be entirely relevant.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-24 17:27           ` Stephen Warren
@ 2011-05-24 18:09             ` Takashi Iwai
  2011-05-24 19:18               ` Stephen Warren
  0 siblings, 1 reply; 34+ messages in thread
From: Takashi Iwai @ 2011-05-24 18:09 UTC (permalink / raw)
  To: Stephen Warren; +Cc: ALSA Development Mailing List, David Henningsson

At Tue, 24 May 2011 10:27:45 -0700,
Stephen Warren wrote:
> 
> Takashi Iwai wrote at Monday, May 23, 2011 11:40 PM:
> > At Mon, 23 May 2011 14:49:15 -0700, Stephen Warren wrote:
> > > ...
> > > I've been testing a preliminary fix for this internally. I found that
> > > the /proc ELD files correctly report monitor_present and eld_valid when
> > > we hot unplug a monitor, but the other fields in the ELD file are not
> > > cleared out to their default state. As best I can tell, this is an ALSA
> > > driver issue, since our display driver only fills in the other fields
> > > when ELDV==1.
> > 
> > Then it's just showing stale data.  The eld fields are updated only
> > when the data is valid.  The patch below should fix the confusion.
> 
> > ---
> > diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
> > index 74b0560..cd96b1d 100644
> > --- a/sound/pci/hda/hda_eld.c
> > +++ b/sound/pci/hda/hda_eld.c
> > @@ -477,6 +477,8 @@ static void hdmi_print_eld_info(struct snd_info_entry
> > *entry,
> > 
> >  	snd_iprintf(buffer, "monitor_present\t\t%d\n", e->monitor_present);
> >  	snd_iprintf(buffer, "eld_valid\t\t%d\n", e->eld_valid);
> > +	if (!e->eld_valid)
> > +		return;
> >  	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name);
> >  	snd_iprintf(buffer, "connection_type\t\t%s\n",
> >  				eld_connection_type_names[e->conn_type]);
> 
> Takashi,
> 
> Your patch looks fine as far as the reporting side goes.
> 
> Should we also modify hdmi_intrinsic_event() as follows:
> 
> 	if (pind && eldv) {
> 		hdmi_get_show_eld(codec, spec->pin[index],
> 				  &spec->sink_eld[index]);
> 		/* TODO: do real things about ELD */
> 	}
> +	else {
> +		memset(&spec->sink_eld[index], 0, sizeof(spec->sink_eld[index]));
> +	}
> 
> ... to make sure that if something accidentally uses the ELD data
> without checking eld_valid, it'll get obviously bad data instead of
> valid-looking-but-stale data? Right now, this isn't an issue, but if
> we start pushing the ELD into user-space through a binary control,
> it'd be nice if the ELD content were consistent in this way.

It sounds safer, but I'd call memset before setting monitor_present
and eld_valid as below.

> Hmm. Actually, hdmi_eld isn't storing the raw ELD bytes, but a parsed
> representation of them, so this may not be entirely relevant.

True.  Still clearing it explicitly would be better for avoiding
unintentional leaks in future.


thanks,

Takashi

---
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 3229018..4dbe8d6 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -707,6 +707,7 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 	if (index < 0)
 		return;
 
+	memset(&spec->sink_eld[index], 0, sizeof(spec->sink_eld[index]));
 	if (spec->old_pin_detect) {
 		if (pind)
 			hdmi_present_sense(codec, tag, &spec->sink_eld[index]);

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-24 18:09             ` Takashi Iwai
@ 2011-05-24 19:18               ` Stephen Warren
  0 siblings, 0 replies; 34+ messages in thread
From: Stephen Warren @ 2011-05-24 19:18 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Mailing List, David Henningsson, ALSA

Takashi Iwai wrote at Tuesday, May 24, 2011 12:10 PM:
> > Takashi Iwai wrote at Monday, May 23, 2011 11:40 PM:
> > > ---
> > > diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
> > > index 74b0560..cd96b1d 100644
> > > --- a/sound/pci/hda/hda_eld.c
> > > +++ b/sound/pci/hda/hda_eld.c
> > > @@ -477,6 +477,8 @@ static void hdmi_print_eld_info(struct snd_info_entry
> > > *entry,
> > >
> > >  	snd_iprintf(buffer, "monitor_present\t\t%d\n", e->monitor_present);
> > >  	snd_iprintf(buffer, "eld_valid\t\t%d\n", e->eld_valid);
> > > +	if (!e->eld_valid)
> > > +		return;
> > >  	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name);
> > >  	snd_iprintf(buffer, "connection_type\t\t%s\n",
> > >  				eld_connection_type_names[e->conn_type]);

> ---
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 3229018..4dbe8d6 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -707,6 +707,7 @@ static void hdmi_intrinsic_event(struct hda_codec
> *codec, unsigned int res)
>  	if (index < 0)
>  		return;
> 
> +	memset(&spec->sink_eld[index], 0, sizeof(spec->sink_eld[index]));
>  	if (spec->old_pin_detect) {
>  		if (pind)
>  			hdmi_present_sense(codec, tag, &spec->sink_eld[index]);

Both:
Tested-by: Stephen Warren <swarren@nvidia.com>

Thanks.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-24  5:39         ` Takashi Iwai
  2011-05-24 17:27           ` Stephen Warren
@ 2011-05-24 21:00           ` Stephen Warren
  1 sibling, 0 replies; 34+ messages in thread
From: Stephen Warren @ 2011-05-24 21:00 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Mailing List, David Henningsson, ALSA

Takashi Iwai wrote at Monday, May 23, 2011 11:40 PM:
> diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
> index 74b0560..cd96b1d 100644
> --- a/sound/pci/hda/hda_eld.c
> +++ b/sound/pci/hda/hda_eld.c
> @@ -477,6 +477,8 @@ static void hdmi_print_eld_info(struct snd_info_entry
> *entry,
> 
>  	snd_iprintf(buffer, "monitor_present\t\t%d\n", e->monitor_present);
>  	snd_iprintf(buffer, "eld_valid\t\t%d\n", e->eld_valid);
> +	if (!e->eld_valid)
> +		return;

Thinking about this more, that condition should be:

    if (!(eld->monitor_present && eld->eld_valid))

I think. Of course, graphics drivers shouldn't be setting ELDV without
PD, but best to be safe within ALSA.

As background, it looks like one of our codecs might be inverting the
PD bit (an issue different to the existing old_pin_detect WAR), which
ends up causing that combination.

Related, there are a few inconsistencies in the way patch_hdmi.c handles
ELD retrieval; hdmi_present_sense looks only at ELDV, whereas
hdmi_intrinsic_event looks at both PD and ELDV. I plan to make a patch
to solve that and the WAR for the NVIDIA HW PD handling above. I can
roll your change into that if you want.

>  	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name);
>  	snd_iprintf(buffer, "connection_type\t\t%s\n",
>  				eld_connection_type_names[e->conn_type]);

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] ALSA: HDA: Add jack detection for HDMI
  2011-05-19 22:51         ` Stephen Warren
@ 2011-06-09 20:59           ` Stephen Warren
  0 siblings, 0 replies; 34+ messages in thread
From: Stephen Warren @ 2011-06-09 20:59 UTC (permalink / raw)
  To: David Henningsson; +Cc: Takashi Iwai, ALSA Development Mailing List

Stephen Warren wrote at Thursday, May 19, 2011 4:51 PM:
> David Henningsson wrote at Thursday, May 19, 2011 4:45 PM:
> >...
> > Ok, thanks. Just to clarify my current problem - if I start
> > nvidia-settings, I'll find my current screen through DVI and an extra
> > HDMI screen that is disabled. If I choose to enable this screen (by
> > clicking Apply on the "X Server Display Information" tab), I get a
> > hotplug event through patch_hdmi.c:hdmi_unsol_event, telling me that the
> > monitor is now present. So far, that's what's expected.
> > However, if I then disable that second HDMI screen and click Apply
> > again, I *don't* get an hdmi_unsol_event telling me that the monitor is
> > now not present. This seems inconsistent to me.
> 
> OK, now that's definitely a bug. I'll file one internally.

This should be fixed in the 275.09.04 pre-release driver that we just
released.

-- 
nvpublic

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2011-06-09 20:59 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-17 13:46 [PATCH] ALSA: HDA: Add jack detection for HDMI David Henningsson
2011-05-17 15:46 ` Takashi Iwai
2011-05-19  9:55   ` David Henningsson
2011-05-19 10:06     ` Takashi Iwai
2011-05-19 10:24       ` Setting invalid samplerate Torsten Schenk
2011-05-19 10:32         ` Torsten Schenk
2011-05-19 10:55         ` Clemens Ladisch
2011-05-19 11:28           ` Torsten Schenk
2011-05-19 11:36             ` Daniel Mack
2011-05-23 21:49       ` [PATCH] ALSA: HDA: Add jack detection for HDMI Stephen Warren
2011-05-24  5:39         ` Takashi Iwai
2011-05-24 17:27           ` Stephen Warren
2011-05-24 18:09             ` Takashi Iwai
2011-05-24 19:18               ` Stephen Warren
2011-05-24 21:00           ` Stephen Warren
2011-05-19 16:57     ` Stephen Warren
2011-05-19 22:45       ` David Henningsson
2011-05-19 22:51         ` Stephen Warren
2011-06-09 20:59           ` Stephen Warren
2011-05-17 16:00 ` Stephen Warren
2011-05-17 16:08   ` Takashi Iwai
2011-05-17 17:09     ` pl bossart
2011-05-17 17:31       ` Takashi Iwai
2011-05-17 20:51         ` pl bossart
2011-05-17 21:42           ` Stephen Warren
2011-05-17 22:11             ` pl bossart
2011-05-17 23:14               ` Stephen Warren
2011-05-18 15:43         ` pl bossart
2011-05-18 15:49           ` Takashi Iwai
2011-05-18 10:02 ` Takashi Iwai
2011-05-20 21:59 ` Stephen Warren
2011-05-21  6:25   ` David Henningsson
2011-05-21  7:37     ` Takashi Iwai
2011-05-23 15:29     ` Stephen Warren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.