* [PATCH v5 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld
2013-02-19 15:11 [PATCH v5 0/5] Make HDMI ELD usable for hotplug David Henningsson
2013-02-19 15:11 ` [PATCH v5 1/5] ALSA: hda - hdmi: ELD shouldn't be valid after unplug David Henningsson
2013-02-19 15:11 ` [PATCH v5 2/5] ALSA: hda - hdmi: Do not expose eld data when eld is invalid David Henningsson
@ 2013-02-19 15:11 ` David Henningsson
2013-02-19 15:11 ` [PATCH v5 4/5] ALSA: hda - hdmi: Protect ELD buffer David Henningsson
2013-02-19 15:11 ` [PATCH v5 5/5] ALSA: hda - hdmi: Notify userspace when ELD control changes David Henningsson
4 siblings, 0 replies; 6+ messages in thread
From: David Henningsson @ 2013-02-19 15:11 UTC (permalink / raw)
To: tiwai, alsa-devel; +Cc: fengguang.wu, pierre-louis.bossart, David Henningsson
For better readability, the information that is parsed out of the
ELD data is now put into a separate parsed_hdmi_eld struct.
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
sound/pci/hda/hda_eld.c | 46 ++++++++++++++++++++------------------------
sound/pci/hda/hda_local.h | 27 ++++++++++++++++----------
sound/pci/hda/patch_hdmi.c | 28 ++++++++++++++++++---------
3 files changed, 57 insertions(+), 44 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
index 4c054f4..16066d7 100644
--- a/sound/pci/hda/hda_eld.c
+++ b/sound/pci/hda/hda_eld.c
@@ -246,8 +246,8 @@ static void hdmi_update_short_audio_desc(struct cea_sad *a,
/*
* Be careful, ELD buf could be totally rubbish!
*/
-static int hdmi_update_eld(struct hdmi_eld *e,
- const unsigned char *buf, int size)
+int snd_hdmi_parse_eld(struct parsed_hdmi_eld *e,
+ const unsigned char *buf, int size)
{
int mnl;
int i;
@@ -260,7 +260,6 @@ static int hdmi_update_eld(struct hdmi_eld *e,
goto out_fail;
}
- e->eld_size = size;
e->baseline_len = GRAB_BITS(buf, 2, 0, 8);
mnl = GRAB_BITS(buf, 4, 0, 5);
e->cea_edid_ver = GRAB_BITS(buf, 4, 5, 3);
@@ -305,7 +304,6 @@ static int hdmi_update_eld(struct hdmi_eld *e,
if (!e->spk_alloc)
e->spk_alloc = 0xffff;
- e->eld_valid = true;
return 0;
out_fail:
@@ -318,17 +316,16 @@ int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid)
AC_DIPSIZE_ELD_BUF);
}
-int snd_hdmi_get_eld(struct hdmi_eld *eld,
- struct hda_codec *codec, hda_nid_t nid)
+int snd_hdmi_get_eld(struct hda_codec *codec, hda_nid_t nid,
+ unsigned char *buf, int *eld_size)
{
int i;
int ret;
int size;
- unsigned char *buf;
/*
* ELD size is initialized to zero in caller function. If no errors and
- * ELD is valid, actual eld_size is assigned in hdmi_update_eld()
+ * ELD is valid, actual eld_size is assigned.
*/
size = snd_hdmi_get_eld_size(codec, nid);
@@ -343,8 +340,6 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
}
/* set ELD buffer */
- buf = eld->eld_buffer;
-
for (i = 0; i < size; i++) {
unsigned int val = hdmi_get_eld_data(codec, nid, i);
/*
@@ -372,8 +367,7 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
buf[i] = val;
}
- ret = hdmi_update_eld(eld, buf, size);
-
+ *eld_size = size;
error:
return ret;
}
@@ -438,7 +432,7 @@ void snd_print_channel_allocation(int spk_alloc, char *buf, int buflen)
buf[j] = '\0'; /* necessary when j == 0 */
}
-void snd_hdmi_show_eld(struct hdmi_eld *e)
+void snd_hdmi_show_eld(struct parsed_hdmi_eld *e)
{
int i;
@@ -487,10 +481,11 @@ static void hdmi_print_sad_info(int i, struct cea_sad *a,
static void hdmi_print_eld_info(struct snd_info_entry *entry,
struct snd_info_buffer *buffer)
{
- struct hdmi_eld *e = entry->private_data;
+ struct hdmi_eld *eld = entry->private_data;
+ struct parsed_hdmi_eld *e = &eld->info;
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 +500,15 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
[4 ... 7] = "reserved"
};
- 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)
+ snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present);
+ snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid);
+ if (!eld->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]);
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);
@@ -535,7 +530,8 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
static void hdmi_write_eld_info(struct snd_info_entry *entry,
struct snd_info_buffer *buffer)
{
- struct hdmi_eld *e = entry->private_data;
+ struct hdmi_eld *eld = entry->private_data;
+ struct parsed_hdmi_eld *e = &eld->info;
char line[64];
char name[64];
char *sname;
@@ -551,9 +547,9 @@ static void hdmi_write_eld_info(struct snd_info_entry *entry,
* eld_version edid_version
*/
if (!strcmp(name, "monitor_present"))
- e->monitor_present = val;
+ eld->monitor_present = val;
else if (!strcmp(name, "eld_valid"))
- e->eld_valid = val;
+ eld->eld_valid = val;
else if (!strcmp(name, "connection_type"))
e->conn_type = val;
else if (!strcmp(name, "port_id"))
@@ -627,7 +623,7 @@ void snd_hda_eld_proc_free(struct hda_codec *codec, struct hdmi_eld *eld)
#endif /* CONFIG_PROC_FS */
/* update PCM info based on ELD */
-void snd_hdmi_eld_update_pcm_info(struct hdmi_eld *eld,
+void snd_hdmi_eld_update_pcm_info(struct parsed_hdmi_eld *e,
struct hda_pcm_stream *hinfo)
{
u32 rates;
@@ -644,8 +640,8 @@ void snd_hdmi_eld_update_pcm_info(struct hdmi_eld *eld,
formats = SNDRV_PCM_FMTBIT_S16_LE;
maxbps = 16;
channels_max = 2;
- for (i = 0; i < eld->sad_count; i++) {
- struct cea_sad *a = &eld->sad[i];
+ for (i = 0; i < e->sad_count; i++) {
+ struct cea_sad *a = &e->sad[i];
rates |= a->rates;
if (a->channels > channels_max)
channels_max = a->channels;
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index 05f1d59..363cd48 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -713,10 +713,10 @@ struct cea_sad {
/*
* ELD: EDID Like Data
*/
-struct hdmi_eld {
- bool monitor_present;
- bool eld_valid;
- int eld_size;
+struct parsed_hdmi_eld {
+ /*
+ * all fields will be cleared before updating ELD
+ */
int baseline_len;
int eld_ver;
int cea_edid_ver;
@@ -731,19 +731,26 @@ struct hdmi_eld {
int spk_alloc;
int sad_count;
struct cea_sad sad[ELD_MAX_SAD];
- /*
- * all fields above eld_buffer will be cleared before updating ELD
- */
+};
+
+struct hdmi_eld {
+ bool monitor_present;
+ bool eld_valid;
+ int eld_size;
char eld_buffer[ELD_MAX_SIZE];
+ struct parsed_hdmi_eld info;
#ifdef CONFIG_PROC_FS
struct snd_info_entry *proc_entry;
#endif
};
int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid);
-int snd_hdmi_get_eld(struct hdmi_eld *, struct hda_codec *, hda_nid_t);
-void snd_hdmi_show_eld(struct hdmi_eld *eld);
-void snd_hdmi_eld_update_pcm_info(struct hdmi_eld *eld,
+int snd_hdmi_get_eld(struct hda_codec *codec, hda_nid_t nid,
+ unsigned char *buf, int *eld_size);
+int snd_hdmi_parse_eld(struct parsed_hdmi_eld *e,
+ const unsigned char *buf, int size);
+void snd_hdmi_show_eld(struct parsed_hdmi_eld *e);
+void snd_hdmi_eld_update_pcm_info(struct parsed_hdmi_eld *e,
struct hda_pcm_stream *hinfo);
#ifdef CONFIG_PROC_FS
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 6bcdd66..1e38191 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -531,7 +531,7 @@ static int hdmi_channel_allocation(struct hdmi_eld *eld, int channels)
* expand ELD's notions to match the ones used by Audio InfoFrame.
*/
for (i = 0; i < ARRAY_SIZE(eld_speaker_allocation_bits); i++) {
- if (eld->spk_alloc & (1 << i))
+ if (eld->info.spk_alloc & (1 << i))
spk_mask |= eld_speaker_allocation_bits[i];
}
@@ -545,7 +545,7 @@ static int hdmi_channel_allocation(struct hdmi_eld *eld, int channels)
}
}
- snd_print_channel_allocation(eld->spk_alloc, buf, sizeof(buf));
+ snd_print_channel_allocation(eld->info.spk_alloc, buf, sizeof(buf));
snd_printdd("HDMI: select CA 0x%x for %d-channel allocation: %s\n",
ca, channels, buf);
@@ -886,7 +886,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, int pin_idx,
ca = 0;
memset(&ai, 0, sizeof(ai));
- if (eld->conn_type == 0) { /* HDMI */
+ if (eld->info.conn_type == 0) { /* HDMI */
struct hdmi_audio_infoframe *hdmi_ai = &ai.hdmi;
hdmi_ai->type = 0x84;
@@ -895,7 +895,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, int pin_idx,
hdmi_ai->CC02_CT47 = channels - 1;
hdmi_ai->CA = ca;
hdmi_checksum_audio_infoframe(hdmi_ai);
- } else if (eld->conn_type == 1) { /* DisplayPort */
+ } else if (eld->info.conn_type == 1) { /* DisplayPort */
struct dp_audio_infoframe *dp_ai = &ai.dp;
dp_ai->type = 0x84;
@@ -1116,7 +1116,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
/* Restrict capabilities by ELD if this isn't disabled */
if (!static_hdmi_pcm && eld->eld_valid) {
- snd_hdmi_eld_update_pcm_info(eld, hinfo);
+ snd_hdmi_eld_update_pcm_info(&eld->info, hinfo);
if (hinfo->channels_min > hinfo->channels_max ||
!hinfo->rates || !hinfo->formats) {
per_cvt->assigned = 0;
@@ -1177,8 +1177,6 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
int present = snd_hda_pin_sense(codec, pin_nid);
bool eld_valid = 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);
@@ -1189,8 +1187,20 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
eld->eld_valid = false;
if (eld_valid) {
- if (!snd_hdmi_get_eld(eld, codec, pin_nid))
- snd_hdmi_show_eld(eld);
+ if (snd_hdmi_get_eld(codec, pin_nid, eld->eld_buffer,
+ &eld->eld_size) < 0)
+ eld_valid = false;
+ else {
+ memset(&eld->info, 0, sizeof(struct parsed_hdmi_eld));
+ if (snd_hdmi_parse_eld(&eld->info, eld->eld_buffer,
+ eld->eld_size) < 0)
+ eld_valid = false;
+ }
+
+ if (eld_valid) {
+ snd_hdmi_show_eld(&eld->info);
+ eld->eld_valid = true;
+ }
else if (repoll) {
queue_delayed_work(codec->bus->workq,
&per_pin->work,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v5 4/5] ALSA: hda - hdmi: Protect ELD buffer
2013-02-19 15:11 [PATCH v5 0/5] Make HDMI ELD usable for hotplug David Henningsson
` (2 preceding siblings ...)
2013-02-19 15:11 ` [PATCH v5 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld David Henningsson
@ 2013-02-19 15:11 ` David Henningsson
2013-02-19 15:11 ` [PATCH v5 5/5] ALSA: hda - hdmi: Notify userspace when ELD control changes David Henningsson
4 siblings, 0 replies; 6+ messages in thread
From: David Henningsson @ 2013-02-19 15:11 UTC (permalink / raw)
To: tiwai, alsa-devel; +Cc: fengguang.wu, pierre-louis.bossart, David Henningsson
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 mutex to guarantee consistency.
To avoid holding the mutex 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 | 8 ++++++-
sound/pci/hda/hda_local.h | 1 +
sound/pci/hda/patch_hdmi.c | 51 +++++++++++++++++++++++++++++++++-----------
3 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
index 16066d7..7dd8463 100644
--- a/sound/pci/hda/hda_eld.c
+++ b/sound/pci/hda/hda_eld.c
@@ -500,10 +500,13 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry,
[4 ... 7] = "reserved"
};
+ mutex_lock(&eld->lock);
snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present);
snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid);
- if (!eld->eld_valid)
+ if (!eld->eld_valid) {
+ mutex_unlock(&eld->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]);
@@ -525,6 +528,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);
+ mutex_unlock(&eld->lock);
}
static void hdmi_write_eld_info(struct snd_info_entry *entry,
@@ -538,6 +542,7 @@ static void hdmi_write_eld_info(struct snd_info_entry *entry,
long long val;
unsigned int n;
+ mutex_lock(&eld->lock);
while (!snd_info_get_line(buffer, line, sizeof(line))) {
if (sscanf(line, "%s %llx", name, &val) != 2)
continue;
@@ -589,6 +594,7 @@ static void hdmi_write_eld_info(struct snd_info_entry *entry,
e->sad_count = n + 1;
}
}
+ mutex_unlock(&eld->lock);
}
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index 363cd48..83b7486 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -739,6 +739,7 @@ struct hdmi_eld {
int eld_size;
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
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 1e38191..e77735d 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;
+ mutex_lock(&eld->lock);
uinfo->count = eld->eld_valid ? eld->eld_size : 0;
+ mutex_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;
+ mutex_lock(&eld->lock);
if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data)) {
+ mutex_unlock(&eld->lock);
snd_BUG();
return -EINVAL;
}
@@ -378,6 +383,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);
+ mutex_unlock(&eld->lock);
return 0;
}
@@ -1164,7 +1170,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
@@ -1175,38 +1183,56 @@ 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;
+ bool eld_changed = false;
- eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
- if (eld->monitor_present)
- eld_valid = !!(present & AC_PINSENSE_ELDV);
+ pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
+ if (pin_eld->monitor_present)
+ eld->eld_valid = !!(present & AC_PINSENSE_ELDV);
+ else
+ eld->eld_valid = false;
_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 (eld->eld_valid) {
if (snd_hdmi_get_eld(codec, pin_nid, eld->eld_buffer,
&eld->eld_size) < 0)
- eld_valid = false;
+ eld->eld_valid = false;
else {
memset(&eld->info, 0, sizeof(struct parsed_hdmi_eld));
if (snd_hdmi_parse_eld(&eld->info, eld->eld_buffer,
eld->eld_size) < 0)
- eld_valid = false;
+ eld->eld_valid = false;
}
- if (eld_valid) {
+ if (eld->eld_valid) {
snd_hdmi_show_eld(&eld->info);
- eld->eld_valid = true;
+ update_eld = true;
}
else if (repoll) {
queue_delayed_work(codec->bus->workq,
&per_pin->work,
msecs_to_jiffies(300));
+ return;
}
}
+
+ mutex_lock(&pin_eld->lock);
+ if (pin_eld->eld_valid && !eld->eld_valid)
+ update_eld = true;
+ if (update_eld) {
+ pin_eld->eld_valid = eld->eld_valid;
+ eld_changed = memcmp(pin_eld->eld_buffer, eld->eld_buffer,
+ eld->eld_size) != 0;
+ if (eld_changed)
+ memcpy(pin_eld->eld_buffer, eld->eld_buffer,
+ eld->eld_size);
+ pin_eld->eld_size = eld->eld_size;
+ pin_eld->info = eld->info;
+ }
+ mutex_unlock(&pin_eld->lock);
}
static void hdmi_repoll_eld(struct work_struct *work)
@@ -1674,6 +1700,7 @@ static int generic_hdmi_init_per_pins(struct hda_codec *codec)
struct hdmi_eld *eld = &per_pin->sink_eld;
per_pin->codec = codec;
+ mutex_init(&eld->lock);
INIT_DELAYED_WORK(&per_pin->work, hdmi_repoll_eld);
snd_hda_eld_proc_new(codec, eld, pin_idx);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread