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

[Stephen Warren <swarren@nvidia.com>]

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