Patches to azx/patch_realtek.c

Patches to azx/patch_realtek.c

[Stephen Warren]

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

Attached are my patches to azx/realtek.c, which implement a fair amount
of what I was talking about in my previous email. Each patch builds on
the previous one.

* realtek.1.diff

White-space changes (strip any trailing whitespace from all lines in the
file)

* realtek.2.diff

Don't use hard-coded literal values for channels_max and
multiout.max_channels. Such literals aren't tied to the underlying
fivestack_modes or threestack_modes data.

Instead of literals, get the values direct from the spec->channel_mode
array - it's the authoritative source.

* realtek.3.diff

Remove spec->board_config. Only alc880_init uses it, and we would rather
parameterize the code by extending the spec data-structure, than having
N functions have knowledge of all the different PCB layouts.

Therefore, remove spec->board_config so we won't be tempted to use it.

* realtek.4.diff

spec->front_panel is no longer used, after Takashi's latest changes. So,
remove it from the structure.

* realtek.5.diff

Stop functions directly referencing global variables for configuration
tables - put pointers to the tables into the spec data structure.

This allows alc880_init to point spec at different sets of data
structures if it needs, and also stops code from knowing about
spec->five_stack, and doing different things depending on the board
type.

So, remove spec->five_stack, since it's no longer used.

-- 
Stephen Warren, Software Engineer, NVIDIA, Fort Collins, CO
swarren@nvidia.com        http://www.nvidia.com/
swarren@wwwdotorg.org     http://www.wwwdotorg.org/pgp.html

[-- Attachment #2: realtek.1.diff --]
[-- Type: application/octet-stream, Size: 3355 bytes --]

--- patch_realtek.c.0	2004-12-15 08:25:14.465996032 -0700
+++ patch_realtek.c.1	2004-12-15 08:26:41.137819912 -0700
@@ -1,6 +1,6 @@
 /*
  * Universal Interface for Intel High Definition Audio Codec
- * 
+ *
  * HD audio interface patch for ALC 260/880 codecs
  *
  * Copyright (c) 2004 PeiSen Hou <pshou@realtek.com.tw>
@@ -118,7 +118,7 @@
 	struct alc_spec *spec = codec->spec;
 	return snd_hda_input_mux_info(spec->input_mux, uinfo);
 }
- 
+
 static int alc_mux_enum_get(snd_kcontrol_t *kcontrol, snd_ctl_elem_value_t *ucontrol)
 {
 	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
@@ -250,7 +250,7 @@
 	{ 0x1a, AC_VERB_SET_AMP_GAIN_MUTE, 0xb000 },
 	{ } /* end */
 };
-	
+
 static struct alc_channel_mode fivestack_modes[2] = {
 	{ 6, fivestack_ch6_init },
 	{ 8, fivestack_ch8_init },
@@ -748,19 +748,19 @@
 	{ .pci_vendor = 0x8086, .pci_device = 0xe200, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe201, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe202, .config = ALC880_3ST },
-	{ .pci_vendor = 0x8086, .pci_device = 0xe203, .config = ALC880_3ST }, 
+	{ .pci_vendor = 0x8086, .pci_device = 0xe203, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe204, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe205, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe206, .config = ALC880_3ST },
-	{ .pci_vendor = 0x8086, .pci_device = 0xe207, .config = ALC880_3ST }, 
+	{ .pci_vendor = 0x8086, .pci_device = 0xe207, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe208, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe209, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe20a, .config = ALC880_3ST },
-	{ .pci_vendor = 0x8086, .pci_device = 0xe20b, .config = ALC880_3ST }, 
+	{ .pci_vendor = 0x8086, .pci_device = 0xe20b, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe20c, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe20d, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe20e, .config = ALC880_3ST },
-	{ .pci_vendor = 0x8086, .pci_device = 0xe20f, .config = ALC880_3ST }, 
+	{ .pci_vendor = 0x8086, .pci_device = 0xe20f, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe210, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe211, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xe214, .config = ALC880_3ST },
@@ -772,11 +772,11 @@
 	{ .pci_vendor = 0x8086, .pci_device = 0xe404, .config = ALC880_3ST },
 	{ .pci_vendor = 0x8086, .pci_device = 0xa101, .config = ALC880_3ST },
 	{ .pci_vendor = 0x107b, .pci_device = 0x3031, .config = ALC880_3ST },
-	{ .pci_vendor = 0x107b, .pci_device = 0x4036, .config = ALC880_3ST },   	
+	{ .pci_vendor = 0x107b, .pci_device = 0x4036, .config = ALC880_3ST },
 	{ .pci_vendor = 0x107b, .pci_device = 0x4037, .config = ALC880_3ST },
 	{ .pci_vendor = 0x107b, .pci_device = 0x4038, .config = ALC880_3ST },
 	{ .pci_vendor = 0x107b, .pci_device = 0x4040, .config = ALC880_3ST },
-	{ .pci_vendor = 0x107b, .pci_device = 0x4041, .config = ALC880_3ST }, 
+	{ .pci_vendor = 0x107b, .pci_device = 0x4041, .config = ALC880_3ST },
 
 	/* Back 3 jack, front 2 jack (Internal add Aux-In) */
 	{ .pci_vendor = 0x1025, .pci_device = 0xe310, .config = ALC880_3ST },

[-- Attachment #3: realtek.2.diff --]
[-- Type: application/octet-stream, Size: 1241 bytes --]

--- patch_realtek.c.1	2004-12-15 08:26:41.137819912 -0700
+++ patch_realtek.c.2	2004-12-15 08:46:07.431516320 -0700
@@ -700,10 +700,10 @@
 	info->stream[SNDRV_PCM_STREAM_PLAYBACK] = alc880_pcm_analog_playback;
 	info->stream[SNDRV_PCM_STREAM_CAPTURE] = alc880_pcm_analog_capture;
 
-	if (spec->five_stack)
-		info->stream[SNDRV_PCM_STREAM_PLAYBACK].channels_max = 8;
-	else
-		info->stream[SNDRV_PCM_STREAM_PLAYBACK].channels_max = 6;
+	info->stream[SNDRV_PCM_STREAM_PLAYBACK].channels_max = spec->channel_mode[0].channels;
+	if (spec->channel_mode[1].channels > info->stream[SNDRV_PCM_STREAM_PLAYBACK].channels_max) {
+	    info->stream[SNDRV_PCM_STREAM_PLAYBACK].channels_max = spec->channel_mode[1].channels;
+	}
 
 	if (spec->multiout.dig_out_nid || spec->dig_in_nid) {
 		codec->num_pcms++;
@@ -837,12 +837,11 @@
 	}
 	if (spec->board_config == ALC880_5ST || spec->board_config == ALC880_5ST_DIG) {
 		spec->five_stack = 1;
-		spec->multiout.max_channels = 6;
 		spec->channel_mode = fivestack_modes;
 	} else {
-		spec->multiout.max_channels = 2;
 		spec->channel_mode = threestack_modes;
 	}
+	spec->multiout.max_channels = spec->channel_mode[0].channels;
 
 	spec->multiout.num_dacs = 4;
 	spec->multiout.dac_nids = alc880_dac_nids;

[-- Attachment #4: realtek.3.diff --]
[-- Type: application/octet-stream, Size: 1263 bytes --]

--- patch_realtek.c.2	2004-12-15 08:46:07.431516320 -0700
+++ patch_realtek.c.3	2004-12-15 08:48:01.087238024 -0700
@@ -41,7 +41,6 @@
 };
 
 struct alc_spec {
-	int board_config;
 	unsigned int five_stack: 1;
 	unsigned int front_panel: 1;
 
@@ -813,18 +812,19 @@
 static int patch_alc880(struct hda_codec *codec)
 {
 	struct alc_spec *spec;
+	int board_config;
 
 	spec = kcalloc(1, sizeof(*spec), GFP_KERNEL);
 	if (spec == NULL)
 		return -ENOMEM;
 
 	codec->spec = spec;
-	spec->board_config = snd_hda_check_board_config(codec, alc880_cfg_tbl);
-	if (spec->board_config < 0) {
+	board_config = snd_hda_check_board_config(codec, alc880_cfg_tbl);
+	if (board_config < 0) {
 		snd_printd(KERN_INFO "hda_codec: Unknown model for ALC880\n");
-		spec->board_config = ALC880_MINIMAL;
+		board_config = ALC880_MINIMAL;
 	}
-	switch (spec->board_config) {
+	switch (board_config) {
 	case ALC880_3ST_DIG:
 	case ALC880_5ST_DIG:
 		spec->multiout.dig_out_nid = ALC880_DIGOUT_NID;
@@ -835,7 +835,7 @@
 		spec->front_panel = 1;
 		break;
 	}
-	if (spec->board_config == ALC880_5ST || spec->board_config == ALC880_5ST_DIG) {
+	if (board_config == ALC880_5ST || board_config == ALC880_5ST_DIG) {
 		spec->five_stack = 1;
 		spec->channel_mode = fivestack_modes;
 	} else {

[-- Attachment #5: realtek.4.diff --]
[-- Type: application/octet-stream, Size: 603 bytes --]

--- patch_realtek.c.3	2004-12-15 08:48:01.087238024 -0700
+++ patch_realtek.c.4	2004-12-15 08:49:53.243187728 -0700
@@ -42,7 +42,8 @@
 
 struct alc_spec {
 	unsigned int five_stack: 1;
-	unsigned int front_panel: 1;
+
+	/* codec parameterization */
 
 	/* playback */
 	struct hda_multi_out multiout;
@@ -828,11 +829,9 @@
 	case ALC880_3ST_DIG:
 	case ALC880_5ST_DIG:
 		spec->multiout.dig_out_nid = ALC880_DIGOUT_NID;
-		spec->front_panel = 1;
 		break;
 	case ALC880_3ST:
 	case ALC880_5ST:
-		spec->front_panel = 1;
 		break;
 	}
 	if (board_config == ALC880_5ST || board_config == ALC880_5ST_DIG) {

[-- Attachment #6: realtek.5.diff --]
[-- Type: application/octet-stream, Size: 3151 bytes --]

--- patch_realtek.c.4	2004-12-15 08:49:53.243187728 -0700
+++ patch_realtek.c.5	2004-12-15 09:11:02.151284376 -0700
@@ -41,9 +41,14 @@
 };
 
 struct alc_spec {
-	unsigned int five_stack: 1;
-
 	/* codec parameterization */
+	snd_kcontrol_new_t *base_mixer;
+	int num_base_mixer;
+
+	snd_kcontrol_new_t *side_mixer;
+	int num_side_mixer;
+
+	struct hda_verb *init_verbs;
 
 	/* playback */
 	struct hda_multi_out multiout;
@@ -60,7 +65,8 @@
 	/* channel model */
 	const struct alc_channel_mode *channel_mode;
 
-	struct hda_pcm pcm_rec[2];	/* PCM information */
+	/* PCM information */
+	struct hda_pcm pcm_rec[2];
 };
 
 /* DAC/ADC assignment */
@@ -367,13 +373,13 @@
 	struct alc_spec *spec = codec->spec;
 	int err;
 
-	err = snd_hda_add_new_ctls(codec, alc880_base_mixer,
-				   ARRAY_SIZE(alc880_base_mixer));
+	err = snd_hda_add_new_ctls(codec, spec->base_mixer,
+				   spec->num_base_mixer);
 	if (err < 0)
 		return err;
-	if (spec->five_stack) {
-		err = snd_hda_add_new_ctls(codec, alc880_side_mixer,
-					   ARRAY_SIZE(alc880_side_mixer));
+	if (spec->side_mixer && spec->num_side_mixer) {
+		err = snd_hda_add_new_ctls(codec, spec->side_mixer,
+					   spec->num_side_mixer);
 		if (err < 0)
 			return err;
 	}
@@ -543,10 +549,7 @@
 static int alc880_init(struct hda_codec *codec)
 {
 	struct alc_spec *spec = codec->spec;
-	if (spec->five_stack)
-		snd_hda_sequence_write(codec, alc880_five_stack_volume);
-	else
-		snd_hda_sequence_write(codec, alc880_three_stack_volume);
+	snd_hda_sequence_write(codec, spec->init_verbs);
 	return 0;
 }
 
@@ -559,9 +562,9 @@
 	struct alc_spec *spec = codec->spec;
 
 	alc880_init(codec);
-	snd_hda_resume_ctls(codec, alc880_base_mixer, ARRAY_SIZE(alc880_base_mixer));
-	if (spec->five_stack)
-		snd_hda_resume_ctls(codec, alc880_side_mixer, ARRAY_SIZE(alc880_side_mixer));
+	snd_hda_resume_ctls(codec, spec->base_mixer, spec->num_base_mixer);
+	if (spec->side_mixer && spec->num_side_mixer)
+		snd_hda_resume_ctls(codec, spec->side_mixer, spec->num_side_mixer);
 	if (spec->multiout.dig_out_nid)
 		snd_hda_resume_spdif_out(codec);
 
@@ -825,6 +828,10 @@
 		snd_printd(KERN_INFO "hda_codec: Unknown model for ALC880\n");
 		board_config = ALC880_MINIMAL;
 	}
+
+	spec->base_mixer = alc880_base_mixer;
+	spec->num_base_mixer = ARRAY_SIZE(alc880_base_mixer);
+
 	switch (board_config) {
 	case ALC880_3ST_DIG:
 	case ALC880_5ST_DIG:
@@ -834,15 +841,19 @@
 	case ALC880_5ST:
 		break;
 	}
+
 	if (board_config == ALC880_5ST || board_config == ALC880_5ST_DIG) {
-		spec->five_stack = 1;
+		spec->side_mixer = alc880_side_mixer;
+		spec->num_side_mixer = ARRAY_SIZE(alc880_side_mixer);
+		spec->init_verbs = alc880_five_stack_volume;
 		spec->channel_mode = fivestack_modes;
 	} else {
+		spec->init_verbs = alc880_three_stack_volume;
 		spec->channel_mode = threestack_modes;
 	}
-	spec->multiout.max_channels = spec->channel_mode[0].channels;
 
-	spec->multiout.num_dacs = 4;
+	spec->multiout.max_channels = spec->channel_mode[0].channels;
+	spec->multiout.num_dacs = ARRAY_SIZE(alc880_dac_nids);
 	spec->multiout.dac_nids = alc880_dac_nids;
 	spec->multiout.hp_nid = 0x03; /* rear-surround NID */
 

Re: Patches to azx/patch_realtek.c

[Takashi Iwai]

At Wed, 15 Dec 2004 13:47:26 -0800,
Stephen Warren wrote:
> 
> * realtek.4.diff
> 
> spec->front_panel is no longer used, after Takashi's latest changes. So,
> remove it from the structure.

I'd like to keep this information in case we'll need some hacks for
the front panel switching or any boards without front panel in
future.


> * realtek.5.diff
> 
> Stop functions directly referencing global variables for configuration
> tables - put pointers to the tables into the spec data structure.
> 
> This allows alc880_init to point spec at different sets of data
> structures if it needs, and also stops code from knowing about
> spec->five_stack, and doing different things depending on the board
> type.

If we do this, let's merge alc260 code, too :)

Maybe it's better to use the array of mixer elem lists than
having separate spec->base_mixer and spec->side_mixer as the
generalization.

Anyway, thanks for the patch.
It's ok to merge all these patches to a one (the whole changes
shouldn't be too big).  But please don't forget to provide
signed-off-by line.


Takashi


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/

RE: Patches to azx/patch_realtek.c

[Stephen Warren]

From: Takashi Iwai [mailto:tiwai@suse.de] 
> 
> At Wed, 15 Dec 2004 13:47:26 -0800,
> Stephen Warren wrote:
> > 
> > * realtek.4.diff
> > 
> > spec->front_panel is no longer used, after Takashi's latest 
> changes. So,
> > remove it from the structure.
> 
> I'd like to keep this information in case we'll need some hacks for
> the front panel switching or any boards without front panel in
> future.

OK. I can put that back in. But looking at your latest code, it says
this:

	switch (spec->board_config) {
	case ALC880_3ST_DIG:
	case ALC880_5ST_DIG:
		spec->digital = 1;
		spec->front_panel = 1;
		break;
	case ALC880_3ST:
	case ALC880_5ST:
		spec->front_panel = 1;
		break;
	}

Which would set front_panel all the time, except for ACL880_MINIMAL. Is
this correct?

> > * realtek.5.diff
> > 
> > Stop functions directly referencing global variables for 
> configuration
> > tables - put pointers to the tables into the spec data structure.
> > 
> > This allows alc880_init to point spec at different sets of data
> > structures if it needs, and also stops code from knowing about
> > spec->five_stack, and doing different things depending on the board
> > type.
> 
> If we do this, let's merge alc260 code, too :)

That's the next patch in the series:-)

> Maybe it's better to use the array of mixer elem lists than
> having separate spec->base_mixer and spec->side_mixer as the
> generalization.

So, have this: ?

struct alc_mixer {
    snd_kcontrol_new_t *controls;
    int num_controls;
};

struct alc_spec {
    struct alc_mixer mixers[2];
    int num_mixers;
};

And have all the code loop over mixers[0..num_mixers-1]

By the way, when an array of had_verb is passed to
snd_hda_sequence_write, this function looks for a "blank" entry in the
array to stop at. However, when passing arrays of snd_kcontrol_new_t to
snd_hda_add_new_ctls, we have to tell it the size, which means we have
to store that too. Can we make the utility code that processes arrays of
snd_kcontrol_new_t look for empty entries too?

-- 
Stephen Warren, Software Engineer, NVIDIA, Fort Collins, CO
swarren@nvidia.com        http://www.nvidia.com/
swarren@wwwdotorg.org     http://www.wwwdotorg.org/pgp.html


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/

Re: Patches to azx/patch_realtek.c

[Takashi Iwai]

At Wed, 15 Dec 2004 14:34:44 -0800,
Stephen Warren wrote:
> 
> From: Takashi Iwai [mailto:tiwai@suse.de] 
> > 
> > At Wed, 15 Dec 2004 13:47:26 -0800,
> > Stephen Warren wrote:
> > > 
> > > * realtek.4.diff
> > > 
> > > spec->front_panel is no longer used, after Takashi's latest 
> > changes. So,
> > > remove it from the structure.
> > 
> > I'd like to keep this information in case we'll need some hacks for
> > the front panel switching or any boards without front panel in
> > future.
> 
> OK. I can put that back in. But looking at your latest code, it says
> this:
> 
> 	switch (spec->board_config) {
> 	case ALC880_3ST_DIG:
> 	case ALC880_5ST_DIG:
> 		spec->digital = 1;
> 		spec->front_panel = 1;
> 		break;
> 	case ALC880_3ST:
> 	case ALC880_5ST:
> 		spec->front_panel = 1;
> 		break;
> 	}
> 
> Which would set front_panel all the time, except for ACL880_MINIMAL. Is
> this correct?

Yes, it seems that all boards have a front panel, so far.

> > > * realtek.5.diff
> > > 
> > > Stop functions directly referencing global variables for 
> > configuration
> > > tables - put pointers to the tables into the spec data structure.
> > > 
> > > This allows alc880_init to point spec at different sets of data
> > > structures if it needs, and also stops code from knowing about
> > > spec->five_stack, and doing different things depending on the board
> > > type.
> > 
> > If we do this, let's merge alc260 code, too :)
> 
> That's the next patch in the series:-)

Fine.

> > Maybe it's better to use the array of mixer elem lists than
> > having separate spec->base_mixer and spec->side_mixer as the
> > generalization.
> 
> So, have this: ?
> 
> struct alc_mixer {
>     snd_kcontrol_new_t *controls;
>     int num_controls;
> };
> 
> struct alc_spec {
>     struct alc_mixer mixers[2];
>     int num_mixers;
> };
> 
> And have all the code loop over mixers[0..num_mixers-1]

That looks OK.  num_controls can be removed and thus struct alc_mixer
won't be needed when we change snd_hda_add_new_ctls() as you suggested
below.

> By the way, when an array of had_verb is passed to
> snd_hda_sequence_write, this function looks for a "blank" entry in the
> array to stop at. However, when passing arrays of snd_kcontrol_new_t to
> snd_hda_add_new_ctls, we have to tell it the size, which means we have
> to store that too. Can we make the utility code that processes arrays of
> snd_kcontrol_new_t look for empty entries too?

Yes, I had the same idea.  snd_hda_add_new_ctls() and
snd_hda_resume_ctls() can check entry->name == NULL.


Takashi


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/