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;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]);