All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Make HDMI ELD usable for hotplug
@ 2013-02-19 10:51 David Henningsson
  2013-02-19 10:51 ` [PATCH v3 1/5] ALSA: hda - hdmi: ELD shouldn't be valid after unplug David Henningsson
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: David Henningsson @ 2013-02-19 10:51 UTC (permalink / raw)
  To: tiwai, alsa-devel; +Cc: fengguang.wu, pierre-louis.bossart, David Henningsson

Changes since v2: Added refactoring patch to split hdmi_eld into
hdmi_eld and parsed_hdmi_eld.

David Henningsson (5):
  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: Refactor hdmi_eld into parsed_hdmi_eld
  ALSA: hda - hdmi: Protect ELD buffer
  ALSA: hda - hdmi: Notify userspace when ELD control changes

 sound/pci/hda/hda_eld.c    |   47 ++++++++++++++----------
 sound/pci/hda/hda_local.h  |   23 +++++++-----
 sound/pci/hda/patch_hdmi.c |   86 +++++++++++++++++++++++++++++++++-----------
 3 files changed, 107 insertions(+), 49 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/5] ALSA: hda - hdmi: ELD shouldn't be valid after unplug
  2013-02-19 10:51 [PATCH v3 0/5] Make HDMI ELD usable for hotplug David Henningsson
@ 2013-02-19 10:51 ` David Henningsson
  2013-02-19 10:51 ` [PATCH v3 2/5] ALSA: hda - hdmi: Do not expose eld data when eld is invalid David Henningsson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: David Henningsson @ 2013-02-19 10:51 UTC (permalink / raw)
  To: tiwai, alsa-devel
  Cc: fengguang.wu, pierre-louis.bossart, David Henningsson, stable

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/5] ALSA: hda - hdmi: Do not expose eld data when eld is invalid
  2013-02-19 10:51 [PATCH v3 0/5] Make HDMI ELD usable for hotplug David Henningsson
  2013-02-19 10:51 ` [PATCH v3 1/5] ALSA: hda - hdmi: ELD shouldn't be valid after unplug David Henningsson
@ 2013-02-19 10:51 ` David Henningsson
  2013-02-19 10:51 ` [PATCH v3 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld David Henningsson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: David Henningsson @ 2013-02-19 10:51 UTC (permalink / raw)
  To: tiwai, alsa-devel; +Cc: fengguang.wu, pierre-louis.bossart, David Henningsson

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;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld
  2013-02-19 10:51 [PATCH v3 0/5] Make HDMI ELD usable for hotplug David Henningsson
  2013-02-19 10:51 ` [PATCH v3 1/5] ALSA: hda - hdmi: ELD shouldn't be valid after unplug David Henningsson
  2013-02-19 10:51 ` [PATCH v3 2/5] ALSA: hda - hdmi: Do not expose eld data when eld is invalid David Henningsson
@ 2013-02-19 10:51 ` David Henningsson
  2013-02-19 11:07   ` Takashi Iwai
  2013-02-19 10:51 ` [PATCH v3 4/5] ALSA: hda - hdmi: Protect ELD buffer David Henningsson
  2013-02-19 10:51 ` [PATCH v3 5/5] ALSA: hda - hdmi: Notify userspace when ELD control changes David Henningsson
  4 siblings, 1 reply; 9+ messages in thread
From: David Henningsson @ 2013-02-19 10:51 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    |   41 ++++++++++++++++++++++-------------------
 sound/pci/hda/hda_local.h  |   22 +++++++++++++---------
 sound/pci/hda/patch_hdmi.c |   12 ++++++------
 3 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
index 4c054f4..aecda72 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)
+static int 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:
@@ -328,7 +326,7 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
 
 	/*
 	 * 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);
@@ -372,8 +370,11 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
 		buf[i] = val;
 	}
 
-	ret = hdmi_update_eld(eld, buf, size);
+	if ((ret = hdmi_parse_eld(&eld->info, buf, size)) < 0)
+		goto error;
 
+	eld->eld_size = size;
+	eld->eld_valid = true;
 error:
 	return ret;
 }
@@ -438,7 +439,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 +488,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 +507,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 +537,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 +554,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 +630,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 +647,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..11fd5f5 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,10 +731,14 @@ 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
@@ -742,8 +746,8 @@ struct hdmi_eld {
 
 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,
+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 7a73dfb..2ac6635 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -529,7 +529,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];
 	}
 
@@ -543,7 +543,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);
 
@@ -884,7 +884,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;
@@ -893,7 +893,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;
@@ -1114,7 +1114,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;
@@ -1188,7 +1188,7 @@ 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);
+			snd_hdmi_show_eld(&eld->info);
 		else if (repoll) {
 			queue_delayed_work(codec->bus->workq,
 					   &per_pin->work,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 4/5] ALSA: hda - hdmi: Protect ELD buffer
  2013-02-19 10:51 [PATCH v3 0/5] Make HDMI ELD usable for hotplug David Henningsson
                   ` (2 preceding siblings ...)
  2013-02-19 10:51 ` [PATCH v3 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld David Henningsson
@ 2013-02-19 10:51 ` David Henningsson
  2013-02-19 10:51 ` [PATCH v3 5/5] ALSA: hda - hdmi: Notify userspace when ELD control changes David Henningsson
  4 siblings, 0 replies; 9+ messages in thread
From: David Henningsson @ 2013-02-19 10:51 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.
---
 sound/pci/hda/hda_eld.c    |    8 +++++++-
 sound/pci/hda/hda_local.h  |    1 +
 sound/pci/hda/patch_hdmi.c |   44 +++++++++++++++++++++++++++++++++++---------
 3 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
index aecda72..439c2e6 100644
--- a/sound/pci/hda/hda_eld.c
+++ b/sound/pci/hda/hda_eld.c
@@ -507,10 +507,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]);
@@ -532,6 +535,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,
@@ -545,6 +549,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;
@@ -596,6 +601,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 11fd5f5..1f83dc3 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 2ac6635..0e1d69d 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;
 	}
@@ -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);
+	mutex_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,45 @@ 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;
 
 	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);
+	pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
+	if (pin_eld->monitor_present)
+		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->info);
+			update_eld = true;
+		}
 		else if (repoll) {
 			queue_delayed_work(codec->bus->workq,
 					   &per_pin->work,
 					   msecs_to_jiffies(300));
 		}
 	}
+
+	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)
@@ -1662,6 +1687,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] 9+ messages in thread

* [PATCH v3 5/5] ALSA: hda - hdmi: Notify userspace when ELD control changes
  2013-02-19 10:51 [PATCH v3 0/5] Make HDMI ELD usable for hotplug David Henningsson
                   ` (3 preceding siblings ...)
  2013-02-19 10:51 ` [PATCH v3 4/5] ALSA: hda - hdmi: Protect ELD buffer David Henningsson
@ 2013-02-19 10:51 ` David Henningsson
  4 siblings, 0 replies; 9+ messages in thread
From: David Henningsson @ 2013-02-19 10:51 UTC (permalink / raw)
  To: tiwai, alsa-devel; +Cc: fengguang.wu, pierre-louis.bossart, David Henningsson

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 |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 0e1d69d..b270dc0 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;
 }
 
@@ -1216,10 +1218,16 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 		if (eld_changed)
 			memcpy(pin_eld->eld_buffer, eld->eld_buffer,
 			       eld->eld_size);
+		eld_changed |= pin_eld->eld_size != eld->eld_size;
 		pin_eld->eld_size = eld->eld_size;
 		pin_eld->info = eld->info;
 	}
 	mutex_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)
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld
  2013-02-19 10:51 ` [PATCH v3 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld David Henningsson
@ 2013-02-19 11:07   ` Takashi Iwai
  2013-02-19 11:43     ` David Henningsson
  0 siblings, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2013-02-19 11:07 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel, fengguang.wu, pierre-louis.bossart

At Tue, 19 Feb 2013 11:51:12 +0100,
David Henningsson wrote:
> 
> 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>

You seem to have forgotten to change memset() call in
hdmi_present_sense().

One thing I feel a bit uneasy is that the whole struct hdmi_eld is
passed in the next patch as spec->temp_eld.  It contains uninitialized
proc entry and mutex.

If we split the data and the control objects, can we pass only the
data struct to snd_hdmi_get_eld() instead of passing the whole struct
hdmi_eld?  If so, the question is where should fields like
monitor_present, etc, belong to.


thanks,

Takashi

> ---
>  sound/pci/hda/hda_eld.c    |   41 ++++++++++++++++++++++-------------------
>  sound/pci/hda/hda_local.h  |   22 +++++++++++++---------
>  sound/pci/hda/patch_hdmi.c |   12 ++++++------
>  3 files changed, 41 insertions(+), 34 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
> index 4c054f4..aecda72 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)
> +static int 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:
> @@ -328,7 +326,7 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
>  
>  	/*
>  	 * 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);
> @@ -372,8 +370,11 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
>  		buf[i] = val;
>  	}
>  
> -	ret = hdmi_update_eld(eld, buf, size);
> +	if ((ret = hdmi_parse_eld(&eld->info, buf, size)) < 0)
> +		goto error;
>  
> +	eld->eld_size = size;
> +	eld->eld_valid = true;
>  error:
>  	return ret;
>  }
> @@ -438,7 +439,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 +488,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 +507,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 +537,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 +554,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 +630,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 +647,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..11fd5f5 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,10 +731,14 @@ 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
> @@ -742,8 +746,8 @@ struct hdmi_eld {
>  
>  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,
> +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 7a73dfb..2ac6635 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -529,7 +529,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];
>  	}
>  
> @@ -543,7 +543,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);
>  
> @@ -884,7 +884,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;
> @@ -893,7 +893,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;
> @@ -1114,7 +1114,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;
> @@ -1188,7 +1188,7 @@ 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);
> +			snd_hdmi_show_eld(&eld->info);
>  		else if (repoll) {
>  			queue_delayed_work(codec->bus->workq,
>  					   &per_pin->work,
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld
  2013-02-19 11:07   ` Takashi Iwai
@ 2013-02-19 11:43     ` David Henningsson
  2013-02-19 11:48       ` Takashi Iwai
  0 siblings, 1 reply; 9+ messages in thread
From: David Henningsson @ 2013-02-19 11:43 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, fengguang.wu, pierre-louis.bossart

[-- Attachment #1: Type: text/plain, Size: 10450 bytes --]

On 02/19/2013 12:07 PM, Takashi Iwai wrote:
> At Tue, 19 Feb 2013 11:51:12 +0100,
> David Henningsson wrote:
>>
>> 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>
>
> You seem to have forgotten to change memset() call in
> hdmi_present_sense().

Ok, will fix.

> One thing I feel a bit uneasy is that the whole struct hdmi_eld is
> passed in the next patch as spec->temp_eld.  It contains uninitialized
> proc entry and mutex.
>
> If we split the data and the control objects, can we pass only the
> data struct to snd_hdmi_get_eld() instead of passing the whole struct
> hdmi_eld?  If so, the question is where should fields like
> monitor_present, etc, belong to.

What do you think of the attached version of patch 3/5? If you like it 
I'll continue with fixing up 4/5 and 5/5 to match.

>
>
> thanks,
>
> Takashi
>
>> ---
>>   sound/pci/hda/hda_eld.c    |   41 ++++++++++++++++++++++-------------------
>>   sound/pci/hda/hda_local.h  |   22 +++++++++++++---------
>>   sound/pci/hda/patch_hdmi.c |   12 ++++++------
>>   3 files changed, 41 insertions(+), 34 deletions(-)
>>
>> diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
>> index 4c054f4..aecda72 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)
>> +static int 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:
>> @@ -328,7 +326,7 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
>>
>>   	/*
>>   	 * 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);
>> @@ -372,8 +370,11 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
>>   		buf[i] = val;
>>   	}
>>
>> -	ret = hdmi_update_eld(eld, buf, size);
>> +	if ((ret = hdmi_parse_eld(&eld->info, buf, size)) < 0)
>> +		goto error;
>>
>> +	eld->eld_size = size;
>> +	eld->eld_valid = true;
>>   error:
>>   	return ret;
>>   }
>> @@ -438,7 +439,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 +488,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 +507,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 +537,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 +554,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 +630,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 +647,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..11fd5f5 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,10 +731,14 @@ 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
>> @@ -742,8 +746,8 @@ struct hdmi_eld {
>>
>>   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,
>> +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 7a73dfb..2ac6635 100644
>> --- a/sound/pci/hda/patch_hdmi.c
>> +++ b/sound/pci/hda/patch_hdmi.c
>> @@ -529,7 +529,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];
>>   	}
>>
>> @@ -543,7 +543,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);
>>
>> @@ -884,7 +884,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;
>> @@ -893,7 +893,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;
>> @@ -1114,7 +1114,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;
>> @@ -1188,7 +1188,7 @@ 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);
>> +			snd_hdmi_show_eld(&eld->info);
>>   		else if (repoll) {
>>   			queue_delayed_work(codec->bus->workq,
>>   					   &per_pin->work,
>> --
>> 1.7.9.5
>>
>



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

[-- Attachment #2: 0001-ALSA-hda-hdmi-Refactor-hdmi_eld-into-parsed_hdmi_eld.patch --]
[-- Type: text/x-patch, Size: 10035 bytes --]

>From 80aca324ff81f630006aeb18f22b78f46428bad3 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Tue, 19 Feb 2013 10:59:34 +0100
Subject: [PATCH] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld

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 |   24 +++++++++++++++--------
 3 files changed, 54 insertions(+), 43 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 7a73dfb..dab147a 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -529,7 +529,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];
 	}
 
@@ -543,7 +543,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);
 
@@ -884,7 +884,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;
@@ -893,7 +893,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;
@@ -1114,7 +1114,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;
@@ -1185,10 +1185,18 @@ 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);
+		if (snd_hdmi_get_eld(codec, pin_nid, eld->eld_buffer,
+						     &eld->eld_size) < 0)
+			eld_valid = false;
+		if (eld_valid && 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


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld
  2013-02-19 11:43     ` David Henningsson
@ 2013-02-19 11:48       ` Takashi Iwai
  0 siblings, 0 replies; 9+ messages in thread
From: Takashi Iwai @ 2013-02-19 11:48 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel, fengguang.wu, pierre-louis.bossart

At Tue, 19 Feb 2013 12:43:46 +0100,
David Henningsson wrote:
> 
> On 02/19/2013 12:07 PM, Takashi Iwai wrote:
> > At Tue, 19 Feb 2013 11:51:12 +0100,
> > David Henningsson wrote:
> >>
> >> 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>
> >
> > You seem to have forgotten to change memset() call in
> > hdmi_present_sense().
> 
> Ok, will fix.
> 
> > One thing I feel a bit uneasy is that the whole struct hdmi_eld is
> > passed in the next patch as spec->temp_eld.  It contains uninitialized
> > proc entry and mutex.
> >
> > If we split the data and the control objects, can we pass only the
> > data struct to snd_hdmi_get_eld() instead of passing the whole struct
> > hdmi_eld?  If so, the question is where should fields like
> > monitor_present, etc, belong to.
> 
> What do you think of the attached version of patch 3/5? If you like it 
> I'll continue with fixing up 4/5 and 5/5 to match.

This looks better.  Thanks!


Takashi

> 
> >
> >
> > thanks,
> >
> > Takashi
> >
> >> ---
> >>   sound/pci/hda/hda_eld.c    |   41 ++++++++++++++++++++++-------------------
> >>   sound/pci/hda/hda_local.h  |   22 +++++++++++++---------
> >>   sound/pci/hda/patch_hdmi.c |   12 ++++++------
> >>   3 files changed, 41 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
> >> index 4c054f4..aecda72 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)
> >> +static int 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:
> >> @@ -328,7 +326,7 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
> >>
> >>   	/*
> >>   	 * 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);
> >> @@ -372,8 +370,11 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld,
> >>   		buf[i] = val;
> >>   	}
> >>
> >> -	ret = hdmi_update_eld(eld, buf, size);
> >> +	if ((ret = hdmi_parse_eld(&eld->info, buf, size)) < 0)
> >> +		goto error;
> >>
> >> +	eld->eld_size = size;
> >> +	eld->eld_valid = true;
> >>   error:
> >>   	return ret;
> >>   }
> >> @@ -438,7 +439,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 +488,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 +507,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 +537,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 +554,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 +630,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 +647,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..11fd5f5 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,10 +731,14 @@ 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
> >> @@ -742,8 +746,8 @@ struct hdmi_eld {
> >>
> >>   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,
> >> +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 7a73dfb..2ac6635 100644
> >> --- a/sound/pci/hda/patch_hdmi.c
> >> +++ b/sound/pci/hda/patch_hdmi.c
> >> @@ -529,7 +529,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];
> >>   	}
> >>
> >> @@ -543,7 +543,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);
> >>
> >> @@ -884,7 +884,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;
> >> @@ -893,7 +893,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;
> >> @@ -1114,7 +1114,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;
> >> @@ -1188,7 +1188,7 @@ 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);
> >> +			snd_hdmi_show_eld(&eld->info);
> >>   		else if (repoll) {
> >>   			queue_delayed_work(codec->bus->workq,
> >>   					   &per_pin->work,
> >> --
> >> 1.7.9.5
> >>
> >
> 
> 
> 
> -- 
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
> [2 0001-ALSA-hda-hdmi-Refactor-hdmi_eld-into-parsed_hdmi_eld.patch <text/x-patch (7bit)>]
> >From 80aca324ff81f630006aeb18f22b78f46428bad3 Mon Sep 17 00:00:00 2001
> From: David Henningsson <david.henningsson@canonical.com>
> Date: Tue, 19 Feb 2013 10:59:34 +0100
> Subject: [PATCH] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld
> 
> 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 |   24 +++++++++++++++--------
>  3 files changed, 54 insertions(+), 43 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 7a73dfb..dab147a 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -529,7 +529,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];
>  	}
>  
> @@ -543,7 +543,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);
>  
> @@ -884,7 +884,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;
> @@ -893,7 +893,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;
> @@ -1114,7 +1114,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;
> @@ -1185,10 +1185,18 @@ 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);
> +		if (snd_hdmi_get_eld(codec, pin_nid, eld->eld_buffer,
> +						     &eld->eld_size) < 0)
> +			eld_valid = false;
> +		if (eld_valid && 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	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-02-19 11:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-19 10:51 [PATCH v3 0/5] Make HDMI ELD usable for hotplug David Henningsson
2013-02-19 10:51 ` [PATCH v3 1/5] ALSA: hda - hdmi: ELD shouldn't be valid after unplug David Henningsson
2013-02-19 10:51 ` [PATCH v3 2/5] ALSA: hda - hdmi: Do not expose eld data when eld is invalid David Henningsson
2013-02-19 10:51 ` [PATCH v3 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld David Henningsson
2013-02-19 11:07   ` Takashi Iwai
2013-02-19 11:43     ` David Henningsson
2013-02-19 11:48       ` Takashi Iwai
2013-02-19 10:51 ` [PATCH v3 4/5] ALSA: hda - hdmi: Protect ELD buffer David Henningsson
2013-02-19 10:51 ` [PATCH v3 5/5] ALSA: hda - hdmi: Notify userspace when ELD control changes David Henningsson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.