Re: [alsa-devel] [PATCH] ALSA: HDA: Add jack detection for HDMI

24 May 2011

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


    