[alsa-devel] [PATCH v2 0/4] Make HDMI ELD usable for hotplug
New day, new patches. This time there is a proper lock around access to the ELD buffer. I hope this satisfies the previous comments.
I've also tested the patches together with some PulseAudio patches I'm writing and the notification works just fine. The ELD data shows up 300 ms after hotplug, so it's likely the first repoll giving me new ELD data).
David Henningsson (4): ALSA: hda - hdmi: ELD shouldn't be valid after unplug ALSA: hda - hdmi: Do not expose eld data when eld is invalid ALSA: hda - hdmi: protect access to ELD buffer ALSA: hda - hdmi: Notify userspace when ELD control changes
sound/pci/hda/hda_eld.c | 12 ++++++-- sound/pci/hda/hda_local.h | 3 ++ sound/pci/hda/patch_hdmi.c | 67 +++++++++++++++++++++++++++++++++++--------- 3 files changed, 66 insertions(+), 16 deletions(-)
Currently, eld_valid is never set to false, except at kernel module load time. This patch makes sure that eld is no longer valid when the cable is (hot-)unplugged.
Cc: stable@kernel.org Signed-off-by: David Henningsson david.henningsson@canonical.com --- sound/pci/hda/patch_hdmi.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index b9af281b..32adaa6 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1176,6 +1176,7 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) "HDMI status: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n", codec->addr, pin_nid, eld->monitor_present, eld_valid);
+ eld->eld_valid = false; if (eld_valid) { if (!snd_hdmi_get_eld(eld, codec, pin_nid)) snd_hdmi_show_eld(eld);
Previously, it was possible to read the eld data of the previous monitor connected. This should not be allowed.
Also refactor the function slightly.
Signed-off-by: David Henningsson david.henningsson@canonical.com --- sound/pci/hda/patch_hdmi.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 32adaa6..7a73dfb 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -343,14 +343,16 @@ static int 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 *spec = codec->spec; + struct hdmi_eld *eld; int pin_idx;
- spec = codec->spec; uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
pin_idx = kcontrol->private_value; - uinfo->count = spec->pins[pin_idx].sink_eld.eld_size; + eld = &spec->pins[pin_idx].sink_eld; + + uinfo->count = eld->eld_valid ? eld->eld_size : 0;
return 0; } @@ -359,14 +361,21 @@ static int 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 *spec = codec->spec; + struct hdmi_eld *eld; int pin_idx;
- spec = codec->spec; pin_idx = kcontrol->private_value; + eld = &spec->pins[pin_idx].sink_eld; + + if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data)) { + snd_BUG(); + return -EINVAL; + }
- memcpy(ucontrol->value.bytes.data, - spec->pins[pin_idx].sink_eld.eld_buffer, ELD_MAX_SIZE); + if (eld->eld_valid) + memcpy(ucontrol->value.bytes.data, eld->eld_buffer, + eld->eld_size);
return 0; }
Because the eld buffer can be simultaneously accessed from both workqueue context (updating) and process context (kcontrol read), we need to protect it with a spinlock to guarantee consistency.
To avoid holding the spinlock while reading the ELD info from the codec, we introduce a temporary eld buffer.
Signed-off-by: David Henningsson david.henningsson@canonical.com --- sound/pci/hda/hda_eld.c | 12 +++++++++--- sound/pci/hda/hda_local.h | 3 +++ sound/pci/hda/patch_hdmi.c | 32 +++++++++++++++++++++++++------- 3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index 4c054f4..766f011 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -490,7 +490,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, struct hdmi_eld *e = entry->private_data; char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE]; int i; - static char *eld_versoin_names[32] = { + static char *eld_version_names[32] = { "reserved", "reserved", "CEA-861D or below", @@ -505,15 +505,18 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, [4 ... 7] = "reserved" };
+ spin_lock(&e->lock); 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) + if (!e->eld_valid) { + spin_unlock(&e->lock); 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]); snd_iprintf(buffer, "eld_version\t\t[0x%x] %s\n", e->eld_ver, - eld_versoin_names[e->eld_ver]); + eld_version_names[e->eld_ver]); snd_iprintf(buffer, "edid_version\t\t[0x%x] %s\n", e->cea_edid_ver, cea_edid_version_names[e->cea_edid_ver]); snd_iprintf(buffer, "manufacture_id\t\t0x%x\n", e->manufacture_id); @@ -530,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
for (i = 0; i < e->sad_count; i++) hdmi_print_sad_info(i, e->sad + i, buffer); + spin_unlock(&e->lock); }
static void hdmi_write_eld_info(struct snd_info_entry *entry, @@ -542,6 +546,7 @@ static void hdmi_write_eld_info(struct snd_info_entry *entry, long long val; unsigned int n;
+ spin_lock(&e->lock); while (!snd_info_get_line(buffer, line, sizeof(line))) { if (sscanf(line, "%s %llx", name, &val) != 2) continue; @@ -593,6 +598,7 @@ static void hdmi_write_eld_info(struct snd_info_entry *entry, e->sad_count = n + 1; } } + spin_unlock(&e->lock); }
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index 05f1d59..b2d6a9e 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -712,8 +712,11 @@ struct cea_sad {
/* * ELD: EDID Like Data + * + * Note: The order of these fields is used in hdmi_present_sense */ struct hdmi_eld { + spinlock_t lock; bool monitor_present; bool eld_valid; int eld_size; diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 7a73dfb..52b1afc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -91,6 +91,7 @@ struct hdmi_spec { struct hda_pcm pcm_rec[MAX_HDMI_PINS]; unsigned int channels_max; /* max over all cvts */
+ struct hdmi_eld temp_eld; /* * Non-generic ATI/NVIDIA specific */ @@ -352,7 +353,9 @@ static int hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol, pin_idx = kcontrol->private_value; eld = &spec->pins[pin_idx].sink_eld;
+ spin_lock(&eld->lock); uinfo->count = eld->eld_valid ? eld->eld_size : 0; + spin_unlock(&eld->lock);
return 0; } @@ -368,7 +371,9 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol, pin_idx = kcontrol->private_value; eld = &spec->pins[pin_idx].sink_eld;
+ spin_lock(&eld->lock); if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data)) { + spin_unlock(&eld->lock); snd_BUG(); return -EINVAL; } @@ -376,6 +381,7 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol, if (eld->eld_valid) memcpy(ucontrol->value.bytes.data, eld->eld_buffer, eld->eld_size); + spin_unlock(&eld->lock);
return 0; } @@ -1162,7 +1168,9 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx) static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) { struct hda_codec *codec = per_pin->codec; - struct hdmi_eld *eld = &per_pin->sink_eld; + struct hdmi_spec *spec = codec->spec; + struct hdmi_eld *eld = &spec->temp_eld; + struct hdmi_eld *pin_eld = &per_pin->sink_eld; hda_nid_t pin_nid = per_pin->pin_nid; /* * Always execute a GetPinSense verb here, even when called from @@ -1173,28 +1181,37 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) * the unsolicited response to avoid custom WARs. */ int present = snd_hda_pin_sense(codec, pin_nid); - bool eld_valid = false; + bool update_eld = false;
memset(eld, 0, offsetof(struct hdmi_eld, eld_buffer));
eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE); if (eld->monitor_present) - eld_valid = !!(present & AC_PINSENSE_ELDV); + eld->eld_valid = !!(present & AC_PINSENSE_ELDV);
_snd_printd(SND_PR_VERBOSE, "HDMI status: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n", - codec->addr, pin_nid, eld->monitor_present, eld_valid); + codec->addr, pin_nid, eld->monitor_present, eld->eld_valid);
- eld->eld_valid = false; - if (eld_valid) { - if (!snd_hdmi_get_eld(eld, codec, pin_nid)) + if (eld->eld_valid) { + if (!snd_hdmi_get_eld(eld, codec, pin_nid)) { snd_hdmi_show_eld(eld); + update_eld = true; + } else if (repoll) { queue_delayed_work(codec->bus->workq, &per_pin->work, msecs_to_jiffies(300)); } } + + spin_lock(&pin_eld->lock); + if (pin_eld->eld_valid && !eld->eld_valid) + update_eld = true; + if (update_eld) + memcpy(&pin_eld->monitor_present, &eld->monitor_present, + sizeof(struct hdmi_eld) - sizeof(spinlock_t)); + spin_unlock(&pin_eld->lock); }
static void hdmi_repoll_eld(struct work_struct *work) @@ -1662,6 +1679,7 @@ static int generic_hdmi_init_per_pins(struct hda_codec *codec) struct hdmi_eld *eld = &per_pin->sink_eld;
per_pin->codec = codec; + spin_lock_init(&eld->lock); INIT_DELAYED_WORK(&per_pin->work, hdmi_repoll_eld); snd_hda_eld_proc_new(codec, eld, pin_idx); }
At Tue, 19 Feb 2013 09:47:36 +0100, David Henningsson wrote:
Because the eld buffer can be simultaneously accessed from both workqueue context (updating) and process context (kcontrol read), we need to protect it with a spinlock to guarantee consistency.
We can't use a spinlock for snd_iprintf(), as this involves with vmalloc(). Use a mutex intead.
To avoid holding the spinlock while reading the ELD info from the codec, we introduce a temporary eld buffer.
Looking through the patches, it's better to split to a few parts now:
struct parsed_hdmi_eld { bool monitor_present; .... };
struct hdmi_eld { char eld_buffer[ELD_MAX_SIZE]; struct parsed_hdmi_eld info; struct mutex lock; #ifdef CONFIG_PROC_FS struct snd_info_entry *proc_entry; #endif };
In that way, you can avoid the hack like offset() calculation in patch_hdmi.c. The comparison can be done only for eld_buffer[], too.
thanks,
Takashi
Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_eld.c | 12 +++++++++--- sound/pci/hda/hda_local.h | 3 +++ sound/pci/hda/patch_hdmi.c | 32 +++++++++++++++++++++++++------- 3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index 4c054f4..766f011 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -490,7 +490,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, struct hdmi_eld *e = entry->private_data; char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE]; int i;
- static char *eld_versoin_names[32] = {
- static char *eld_version_names[32] = { "reserved", "reserved", "CEA-861D or below",
@@ -505,15 +505,18 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, [4 ... 7] = "reserved" };
- spin_lock(&e->lock); 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)
- if (!e->eld_valid) {
spin_unlock(&e->lock);- } 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]); snd_iprintf(buffer, "eld_version\t\t[0x%x] %s\n", e->eld_ver,
eld_versoin_names[e->eld_ver]);
eld_version_names[e->eld_ver]);@@ -530,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
for (i = 0; i < e->sad_count; i++) hdmi_print_sad_info(i, e->sad + i, buffer);
- spin_unlock(&e->lock);
}
static void hdmi_write_eld_info(struct snd_info_entry *entry, @@ -542,6 +546,7 @@ static void hdmi_write_eld_info(struct snd_info_entry *entry, long long val; unsigned int n;
- spin_lock(&e->lock); while (!snd_info_get_line(buffer, line, sizeof(line))) { if (sscanf(line, "%s %llx", name, &val) != 2) continue;
@@ -593,6 +598,7 @@ static void hdmi_write_eld_info(struct snd_info_entry *entry, e->sad_count = n + 1; } }
- spin_unlock(&e->lock);
}
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index 05f1d59..b2d6a9e 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -712,8 +712,11 @@ struct cea_sad {
/*
- ELD: EDID Like Data
*/
- Note: The order of these fields is used in hdmi_present_sense
struct hdmi_eld {
- spinlock_t lock; bool monitor_present; bool eld_valid; int eld_size;
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 7a73dfb..52b1afc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -91,6 +91,7 @@ struct hdmi_spec { struct hda_pcm pcm_rec[MAX_HDMI_PINS]; unsigned int channels_max; /* max over all cvts */
- struct hdmi_eld temp_eld; /*
*/
- Non-generic ATI/NVIDIA specific
@@ -352,7 +353,9 @@ static int hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol, pin_idx = kcontrol->private_value; eld = &spec->pins[pin_idx].sink_eld;
spin_lock(&eld->lock); uinfo->count = eld->eld_valid ? eld->eld_size : 0;
spin_unlock(&eld->lock);
return 0;
} @@ -368,7 +371,9 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol, pin_idx = kcontrol->private_value; eld = &spec->pins[pin_idx].sink_eld;
- spin_lock(&eld->lock); if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data)) {
spin_unlock(&eld->lock);@@ -376,6 +381,7 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol, if (eld->eld_valid) memcpy(ucontrol->value.bytes.data, eld->eld_buffer, eld->eld_size);
spin_unlock(&eld->lock);
return 0;
} @@ -1162,7 +1168,9 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx) static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) { struct hda_codec *codec = per_pin->codec;
- struct hdmi_eld *eld = &per_pin->sink_eld;
- struct hdmi_spec *spec = codec->spec;
- struct hdmi_eld *eld = &spec->temp_eld;
- struct hdmi_eld *pin_eld = &per_pin->sink_eld; hda_nid_t pin_nid = per_pin->pin_nid; /*
- Always execute a GetPinSense verb here, even when called from
@@ -1173,28 +1181,37 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) * the unsolicited response to avoid custom WARs. */ int present = snd_hda_pin_sense(codec, pin_nid);
- bool eld_valid = false;
bool update_eld = false;
memset(eld, 0, offsetof(struct hdmi_eld, eld_buffer));
eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE); if (eld->monitor_present)
eld_valid = !!(present & AC_PINSENSE_ELDV);
eld->eld_valid = !!(present & AC_PINSENSE_ELDV);_snd_printd(SND_PR_VERBOSE, "HDMI status: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n",
codec->addr, pin_nid, eld->monitor_present, eld_valid);
codec->addr, pin_nid, eld->monitor_present, eld->eld_valid);
- eld->eld_valid = false;
- if (eld_valid) {
if (!snd_hdmi_get_eld(eld, codec, pin_nid))
- if (eld->eld_valid) {
if (!snd_hdmi_get_eld(eld, codec, pin_nid)) { snd_hdmi_show_eld(eld);update_eld = true;}- spin_lock(&pin_eld->lock);
- if (pin_eld->eld_valid && !eld->eld_valid)
update_eld = true;- if (update_eld)
memcpy(&pin_eld->monitor_present, &eld->monitor_present,sizeof(struct hdmi_eld) - sizeof(spinlock_t));- spin_unlock(&pin_eld->lock);
}
static void hdmi_repoll_eld(struct work_struct *work) @@ -1662,6 +1679,7 @@ static int generic_hdmi_init_per_pins(struct hda_codec *codec) struct hdmi_eld *eld = &per_pin->sink_eld;
per_pin->codec = codec;
spin_lock_init(&eld->lock);-- 1.7.9.5
On 02/19/2013 10:15 AM, Takashi Iwai wrote:
At Tue, 19 Feb 2013 09:47:36 +0100, David Henningsson wrote:
Because the eld buffer can be simultaneously accessed from both workqueue context (updating) and process context (kcontrol read), we need to protect it with a spinlock to guarantee consistency.
We can't use a spinlock for snd_iprintf(), as this involves with vmalloc(). Use a mutex intead.
Ok, thanks for spotting.
To avoid holding the spinlock while reading the ELD info from the codec, we introduce a temporary eld buffer.
Looking through the patches, it's better to split to a few parts now:
struct parsed_hdmi_eld { bool monitor_present; .... };
struct hdmi_eld { char eld_buffer[ELD_MAX_SIZE]; struct parsed_hdmi_eld info; struct mutex lock; #ifdef CONFIG_PROC_FS struct snd_info_entry *proc_entry; #endif };
In that way, you can avoid the hack like offset() calculation in patch_hdmi.c. The comparison can be done only for eld_buffer[], too.
This sounds like a good idea. I will do it. I think eld_valid and monitor_present should be in hdmi_eld and not parsed_hdmi_eld, because those fields do not come from the raw ELD info.
thanks,
Takashi
Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_eld.c | 12 +++++++++--- sound/pci/hda/hda_local.h | 3 +++ sound/pci/hda/patch_hdmi.c | 32 +++++++++++++++++++++++++------- 3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index 4c054f4..766f011 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -490,7 +490,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, struct hdmi_eld *e = entry->private_data; char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE]; int i;
- static char *eld_versoin_names[32] = {
- static char *eld_version_names[32] = { "reserved", "reserved", "CEA-861D or below",
@@ -505,15 +505,18 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, [4 ... 7] = "reserved" };
- spin_lock(&e->lock); 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)
- if (!e->eld_valid) {
spin_unlock(&e->lock);- } 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]); snd_iprintf(buffer, "eld_version\t\t[0x%x] %s\n", e->eld_ver,
eld_versoin_names[e->eld_ver]);
eld_version_names[e->eld_ver]);@@ -530,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
for (i = 0; i < e->sad_count; i++) hdmi_print_sad_info(i, e->sad + i, buffer);
spin_unlock(&e->lock); }
static void hdmi_write_eld_info(struct snd_info_entry *entry,
@@ -542,6 +546,7 @@ static void hdmi_write_eld_info(struct snd_info_entry *entry, long long val; unsigned int n;
- spin_lock(&e->lock); while (!snd_info_get_line(buffer, line, sizeof(line))) { if (sscanf(line, "%s %llx", name, &val) != 2) continue;
@@ -593,6 +598,7 @@ static void hdmi_write_eld_info(struct snd_info_entry *entry, e->sad_count = n + 1; } }
- spin_unlock(&e->lock); }
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index 05f1d59..b2d6a9e 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -712,8 +712,11 @@ struct cea_sad {
/*
- ELD: EDID Like Data
*/ struct hdmi_eld {
- Note: The order of these fields is used in hdmi_present_sense
- spinlock_t lock; bool monitor_present; bool eld_valid; int eld_size;
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 7a73dfb..52b1afc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -91,6 +91,7 @@ struct hdmi_spec { struct hda_pcm pcm_rec[MAX_HDMI_PINS]; unsigned int channels_max; /* max over all cvts */
- struct hdmi_eld temp_eld; /*
*/
- Non-generic ATI/NVIDIA specific
@@ -352,7 +353,9 @@ static int hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol, pin_idx = kcontrol->private_value; eld = &spec->pins[pin_idx].sink_eld;
spin_lock(&eld->lock); uinfo->count = eld->eld_valid ? eld->eld_size : 0;
spin_unlock(&eld->lock);
return 0; }
@@ -368,7 +371,9 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol, pin_idx = kcontrol->private_value; eld = &spec->pins[pin_idx].sink_eld;
- spin_lock(&eld->lock); if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data)) {
spin_unlock(&eld->lock);@@ -376,6 +381,7 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol, if (eld->eld_valid) memcpy(ucontrol->value.bytes.data, eld->eld_buffer, eld->eld_size);
spin_unlock(&eld->lock);
return 0; }
@@ -1162,7 +1168,9 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx) static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) { struct hda_codec *codec = per_pin->codec;
- struct hdmi_eld *eld = &per_pin->sink_eld;
- struct hdmi_spec *spec = codec->spec;
- struct hdmi_eld *eld = &spec->temp_eld;
- struct hdmi_eld *pin_eld = &per_pin->sink_eld; hda_nid_t pin_nid = per_pin->pin_nid; /*
- Always execute a GetPinSense verb here, even when called from
@@ -1173,28 +1181,37 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) * the unsolicited response to avoid custom WARs. */ int present = snd_hda_pin_sense(codec, pin_nid);
- bool eld_valid = false;
bool update_eld = false;
memset(eld, 0, offsetof(struct hdmi_eld, eld_buffer));
eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE); if (eld->monitor_present)
eld_valid = !!(present & AC_PINSENSE_ELDV);
eld->eld_valid = !!(present & AC_PINSENSE_ELDV);_snd_printd(SND_PR_VERBOSE, "HDMI status: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n",
codec->addr, pin_nid, eld->monitor_present, eld_valid);
codec->addr, pin_nid, eld->monitor_present, eld->eld_valid);
- eld->eld_valid = false;
- if (eld_valid) {
if (!snd_hdmi_get_eld(eld, codec, pin_nid))
if (eld->eld_valid) {
if (!snd_hdmi_get_eld(eld, codec, pin_nid)) { snd_hdmi_show_eld(eld);update_eld = true;}else if (repoll) { queue_delayed_work(codec->bus->workq, &per_pin->work, msecs_to_jiffies(300)); } }
spin_lock(&pin_eld->lock);
if (pin_eld->eld_valid && !eld->eld_valid)
update_eld = true;if (update_eld)
memcpy(&pin_eld->monitor_present, &eld->monitor_present,sizeof(struct hdmi_eld) - sizeof(spinlock_t));spin_unlock(&pin_eld->lock); }
static void hdmi_repoll_eld(struct work_struct *work)
@@ -1662,6 +1679,7 @@ static int generic_hdmi_init_per_pins(struct hda_codec *codec) struct hdmi_eld *eld = &per_pin->sink_eld;
per_pin->codec = codec;
spin_lock_init(&eld->lock);-- 1.7.9.5
ELD validity can change during the lifetime of a presence detect, so we need to be able to listen for changes on the ELD control.
Signed-off-by: David Henningsson david.henningsson@canonical.com --- sound/pci/hda/patch_hdmi.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 52b1afc..bfaa0ee 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -75,6 +75,7 @@ struct hdmi_spec_per_pin { struct hda_codec *codec; struct hdmi_eld sink_eld; struct delayed_work work; + struct snd_kcontrol *eld_ctl; int repoll_count; bool non_pcm; bool chmap_set; /* channel-map override by ALSA API? */ @@ -411,6 +412,7 @@ static int hdmi_create_eld_ctl(struct hda_codec *codec, int pin_idx, if (err < 0) return err;
+ spec->pins[pin_idx].eld_ctl = kctl; return 0; }
@@ -1182,6 +1184,7 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) */ int present = snd_hda_pin_sense(codec, pin_nid); bool update_eld = false; + bool eld_changed = false;
memset(eld, 0, offsetof(struct hdmi_eld, eld_buffer));
@@ -1208,10 +1211,20 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) spin_lock(&pin_eld->lock); if (pin_eld->eld_valid && !eld->eld_valid) update_eld = true; - if (update_eld) - memcpy(&pin_eld->monitor_present, &eld->monitor_present, - sizeof(struct hdmi_eld) - sizeof(spinlock_t)); + if (update_eld) { + void *dest = &pin_eld->monitor_present; + void *src = &eld->monitor_present; + size_t size = sizeof(struct hdmi_eld) - sizeof(spinlock_t); + eld_changed = memcmp(dest, src, size) != 0; + if (eld_changed) + memcpy(dest, src, size); + } spin_unlock(&pin_eld->lock); + + if (eld_changed) + snd_ctl_notify(codec->bus->card, + SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO, + &per_pin->eld_ctl->id); }
static void hdmi_repoll_eld(struct work_struct *work)
participants (2)
-
David Henningsson -
Takashi Iwai