[alsa-devel] [PATCH v3 3/5] ALSA: hda - hdmi: Refactor hdmi_eld into parsed_hdmi_eld
David Henningsson
david.henningsson at canonical.com
Tue Feb 19 12:43:46 CET 2013
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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ALSA-hda-hdmi-Refactor-hdmi_eld-into-parsed_hdmi_eld.patch
Type: text/x-patch
Size: 10035 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20130219/d996f023/attachment-0001.bin>
More information about the Alsa-devel
mailing list