Skip to contentSkip to conversation

ac97: YMF743 missing controls support

PATCH In review Keita Maehara 2 participants 7 messages Sep 18, 2007 – Sep 19, 2007

lore
Share & tools
lore

Keita Maehara

author
#0 lore raw

PATCH 2/2 ac97: YMF743 missing controls support

From: Keita Maehara <[email protected]>

These patches enable a few YMF743 controls (Tone/3D/IEC958) that won't be detected with the current version of ALSA.

Signed-off-by Keita Maehara <[email protected]>

---

The second one contains following changes:

- A chip-specific SPDIF support for YMF743 (It doesn't have AC97 standard SPDIF registers seen on YMF753).

- The implementation for "IEC958 Playback Source" and "IEC958 Mute" are identical to the ones for YMF753. But there is no "IEC958 Output Pin" for YMF743.

pci/ac97/ac97_codec.c +9 -2
diff -r 92978f62cc03 pci/ac97/ac97_codec.c
@@ -176,7 +176,7 @@ static const struct ac97_codec_id snd_ac
 { 0x574d4C09, 0xffffffff, "WM9709",		NULL,		NULL},
 { 0x574d4C12, 0xffffffff, "WM9711,WM9712",	patch_wolfson11, NULL},
 { 0x574d4c13, 0xffffffff, "WM9713,WM9714",	patch_wolfson13, NULL, AC97_DEFAULT_POWER_OFF},
-{ 0x594d4800, 0xffffffff, "YMF743",		NULL,		NULL },
+{ 0x594d4800, 0xffffffff, "YMF743",		patch_yamaha_ymf743,	NULL },
 { 0x594d4802, 0xffffffff, "YMF752",		NULL,		NULL },
 { 0x594d4803, 0xffffffff, "YMF753",		patch_yamaha_ymf753,	NULL },
 { 0x83847600, 0xffffffff, "STAC9700,83,84",	patch_sigmatel_stac9700,	NULL },

@@ -779,6 +779,12 @@ static int snd_ac97_spdif_default_put(st
 		change |= snd_ac97_update_bits_nolock(ac97, AC97_CXR_AUDIO_MISC, 
 						      AC97_CXR_SPDIF_MASK | AC97_CXR_COPYRGT,
 						      v);
+	} else if (ac97->id == AC97_ID_YMF743) {
+		change |= snd_ac97_update_bits_nolock(ac97,
+						      AC97_YMF7X3_DIT_CTRL,
+						      0xff38,
+						      ((val << 4) & 0xff00) |
+						      ((val << 2) & 0x0038));
 	} else {
 		unsigned short extst = snd_ac97_read_cache(ac97, AC97_EXTENDED_STATUS);
 		snd_ac97_update_bits_nolock(ac97, AC97_EXTENDED_STATUS, AC97_EA_SPDIF, 0); /* turn off */

@@ -1375,7 +1381,8 @@ static int snd_ac97_mixer_build(struct s
 			for (idx = 0; idx < 2; idx++) {
 				if ((err = snd_ctl_add(card, kctl = snd_ac97_cnew(&snd_ac97_controls_tone[idx], ac97))) < 0)
 					return err;
-				if (ac97->id == AC97_ID_YMF753) {
+				if (ac97->id == AC97_ID_YMF743 ||
+				    ac97->id == AC97_ID_YMF753) {
 					kctl->private_value &= ~(0xff << 16);
 					kctl->private_value |= 7 << 16;
 				}
pci/ac97/ac97_id.h +1 -0
diff -r 92978f62cc03 pci/ac97/ac97_id.h
@@ -54,6 +54,7 @@
 #define AC97_ID_ALC658		0x414c4780
 #define AC97_ID_ALC658D		0x414c4781
 #define AC97_ID_ALC850		0x414c4790
+#define AC97_ID_YMF743		0x594d4800
 #define AC97_ID_YMF753		0x594d4803
 #define AC97_ID_VT1616		0x49434551
 #define AC97_ID_CM9738		0x434d4941
pci/ac97/ac97_patch.c +58 -16

92 lines not on this page · raw message

pci/ac97/ac97_proc.c +6 -2
diff -r 92978f62cc03 pci/ac97/ac97_proc.c
@@ -236,10 +236,14 @@ static void snd_ac97_proc_read_main(stru
 		val = snd_ac97_read(ac97, AC97_PCM_MIC_ADC_RATE);
 		snd_iprintf(buffer, "PCM MIC ADC      : %iHz\n", val);
 	}
-	if ((ext & AC97_EI_SPDIF) || (ac97->flags & AC97_CS_SPDIF)) {
+	if ((ext & AC97_EI_SPDIF) || (ac97->flags & AC97_CS_SPDIF) ||
+	    (ac97->id == AC97_ID_YMF743)) {
 	        if (ac97->flags & AC97_CS_SPDIF)
 			val = snd_ac97_read(ac97, AC97_CSR_SPDIF);
-		else
+	        else if (ac97->id == AC97_ID_YMF743) {
+			val = snd_ac97_read(ac97, AC97_YMF7X3_DIT_CTRL);
+			val = 0x2000 | (val & 0xff00) >> 4 | (val & 0x38) >> 2;
+		} else
 			val = snd_ac97_read(ac97, AC97_SPDIF);
 
 		snd_iprintf(buffer, "SPDIF Control    :%s%s%s%s Category=0x%x Generation=%i%s%s%s\n",
#1 lore raw

At Wed, 19 Sep 2007 00:50:54 +0900,

Quoted 7 lines · Keita Maehara

From: Keita Maehara <[email protected]>

These patches enable a few YMF743 controls (Tone/3D/IEC958) that won't be detected with the current version of ALSA.

Signed-off-by Keita Maehara <[email protected]>

The changes look good to me, but I'd like to ask you to rewrite the patch to follow the standard coding style. Yes, the original ac97 code doesn't follow, but at least, we should do it right for the newly added/changed codes. For example, keep lines within 80 chars, fold lines appropriately, avoid if ((err = ...)) style, etc.

In doubt, you can run scripts/checkpatch.pl in the linux kernel tree.

Thanks,

Takashi

author
#2 lore raw

At Tue, 18 Sep 2007 18:04:02 +0200,

Quoted 8 lines · Takashi Iwai

At Wed, 19 Sep 2007 00:50:54 +0900,

Quoted 6 lines · Keita Maehara

From: Keita Maehara <[email protected]>

These patches enable a few YMF743 controls (Tone/3D/IEC958) that won't be detected with the current version of ALSA.

Quoted 7 lines
Quoted 1 line
Signed-off-by Keita Maehara <[email protected]>

The changes look good to me, but I'd like to ask you to rewrite the patch to follow the standard coding style. Yes, the original ac97 code doesn't follow, but at least, we should do it right for the newly added/changed codes. For example, keep lines within 80 chars, fold lines appropriately, avoid if ((err = ...)) style, etc.

I was wondering whether to follow the coding style for them indeed but understood the policy now. Sorry to bother you.

Quoted 1 line

In doubt, you can run scripts/checkpatch.pl in the linux kernel tree.

Now they're checkpatch-clean anyway. I've put the second version at
http://people.debian.org/~maehara/alsa-ymf743.

Signature · 1 line
Keita Maehara <[email protected]>
#3 lore raw

At Wed, 19 Sep 2007 08:47:23 +0900,

Quoted 20 lines · Keita Maehara

At Tue, 18 Sep 2007 18:04:02 +0200,

Quoted 8 lines · Takashi Iwai

At Wed, 19 Sep 2007 00:50:54 +0900,

Quoted 6 lines · Keita Maehara

From: Keita Maehara <[email protected]>

These patches enable a few YMF743 controls (Tone/3D/IEC958) that won't be detected with the current version of ALSA.

Quoted 7 lines
Quoted 1 line
Signed-off-by Keita Maehara <[email protected]>

The changes look good to me, but I'd like to ask you to rewrite the patch to follow the standard coding style. Yes, the original ac97 code doesn't follow, but at least, we should do it right for the newly added/changed codes. For example, keep lines within 80 chars, fold lines appropriately, avoid if ((err = ...)) style, etc.

Quoted 7 lines

I was wondering whether to follow the coding style for them indeed but understood the policy now. Sorry to bother you.

Quoted 1 line

In doubt, you can run scripts/checkpatch.pl in the linux kernel tree.

Now they're checkpatch-clean anyway. I've put the second version at
http://people.debian.org/~maehara/alsa-ymf743.

Could you repost here again? Then I could give further review comments (if any).

thanks,

Takashi

author
#4 lore raw

At Wed, 19 Sep 2007 13:06:38 +0200,

Quoted 5 lines · Takashi Iwai
Quoted 2 lines

Now they're checkpatch-clean anyway. I've put the second version at
http://people.debian.org/~maehara/alsa-ymf743.

Could you repost here again? Then I could give further review comments (if any).

Okay, here it is.

From: Keita Maehara <[email protected]>

These patches enable some YMF743 controls (Tone/3D/IEC958) that won't be detected with the current version of ALSA.

Signed-off-by Keita Maehara <[email protected]>

---

The first one contains only cosmetic changes to share a few YMF753-specific symbols with YMF743.

include/ac97_codec.h +3 -3
diff -r 0028e39ead78 include/ac97_codec.h
@@ -345,9 +345,9 @@
 #define AC97_ALC650_GPIO_STATUS		0x78
 #define AC97_ALC650_CLOCK		0x7a
 
-/* specific - Yamaha YMF753 */
-#define AC97_YMF753_DIT_CTRL2	0x66	/* DIT Control 2 */
-#define AC97_YMF753_3D_MODE_SEL	0x68	/* 3D Mode Select */
+/* specific - Yamaha YMF7x3 */
+#define AC97_YMF7X3_DIT_CTRL	0x66	/* DIT Control (YMF743) / 2 (YMF753) */
+#define AC97_YMF7X3_3D_MODE_SEL	0x68	/* 3D Mode Select */
 
 /* specific - C-Media */
 #define AC97_CM9738_VENDOR_CTRL	0x5a
pci/ac97/ac97_patch.c +49 -33

171 lines not on this page · raw message

author
#5 lore raw

At Wed, 19 Sep 2007 20:28:22 +0900,

Quoted 1 line · Keita Maehara

Okay, here it is.

And this is the second part.

From: Keita Maehara <[email protected]>

These patches enable a few YMF743 controls (Tone/3D/IEC958) that won't be detected with the current version of ALSA.

Signed-off-by Keita Maehara <[email protected]>

---

The second one contains following changes:

- A chip-specific SPDIF support for YMF743 (It doesn't have AC97 standard SPDIF registers seen on YMF753).

- The implementation for "IEC958 Playback Source" and "IEC958 Mute" are identical to the ones for YMF753. But there is no "IEC958 Output Pin" for YMF743.

pci/ac97/ac97_codec.c +9 -2
diff -r a543470c3455 pci/ac97/ac97_codec.c
@@ -176,7 +176,7 @@ static const struct ac97_codec_id snd_ac
 { 0x574d4C09, 0xffffffff, "WM9709",		NULL,		NULL},
 { 0x574d4C12, 0xffffffff, "WM9711,WM9712",	patch_wolfson11, NULL},
 { 0x574d4c13, 0xffffffff, "WM9713,WM9714",	patch_wolfson13, NULL, AC97_DEFAULT_POWER_OFF},
-{ 0x594d4800, 0xffffffff, "YMF743",		NULL,		NULL },
+{ 0x594d4800, 0xffffffff, "YMF743",		patch_yamaha_ymf743,	NULL },
 { 0x594d4802, 0xffffffff, "YMF752",		NULL,		NULL },
 { 0x594d4803, 0xffffffff, "YMF753",		patch_yamaha_ymf753,	NULL },
 { 0x83847600, 0xffffffff, "STAC9700,83,84",	patch_sigmatel_stac9700,	NULL },

@@ -779,6 +779,12 @@ static int snd_ac97_spdif_default_put(st
 		change |= snd_ac97_update_bits_nolock(ac97, AC97_CXR_AUDIO_MISC, 
 						      AC97_CXR_SPDIF_MASK | AC97_CXR_COPYRGT,
 						      v);
+	} else if (ac97->id == AC97_ID_YMF743) {
+		change |= snd_ac97_update_bits_nolock(ac97,
+						      AC97_YMF7X3_DIT_CTRL,
+						      0xff38,
+						      ((val << 4) & 0xff00) |
+						      ((val << 2) & 0x0038));
 	} else {
 		unsigned short extst = snd_ac97_read_cache(ac97, AC97_EXTENDED_STATUS);
 		snd_ac97_update_bits_nolock(ac97, AC97_EXTENDED_STATUS, AC97_EA_SPDIF, 0); /* turn off */

@@ -1375,7 +1381,8 @@ static int snd_ac97_mixer_build(struct s
 			for (idx = 0; idx < 2; idx++) {
 				if ((err = snd_ctl_add(card, kctl = snd_ac97_cnew(&snd_ac97_controls_tone[idx], ac97))) < 0)
 					return err;
-				if (ac97->id == AC97_ID_YMF753) {
+				if (ac97->id == AC97_ID_YMF743 ||
+				    ac97->id == AC97_ID_YMF753) {
 					kctl->private_value &= ~(0xff << 16);
 					kctl->private_value |= 7 << 16;
 				}
pci/ac97/ac97_id.h +1 -0
diff -r a543470c3455 pci/ac97/ac97_id.h
@@ -54,6 +54,7 @@
 #define AC97_ID_ALC658		0x414c4780
 #define AC97_ID_ALC658D		0x414c4781
 #define AC97_ID_ALC850		0x414c4790
+#define AC97_ID_YMF743		0x594d4800
 #define AC97_ID_YMF753		0x594d4803
 #define AC97_ID_VT1616		0x49434551
 #define AC97_ID_CM9738		0x434d4941
pci/ac97/ac97_patch.c +69 -22

109 lines not on this page · raw message

pci/ac97/ac97_proc.c +6 -2
diff -r a543470c3455 pci/ac97/ac97_proc.c
@@ -236,10 +236,14 @@ static void snd_ac97_proc_read_main(stru
 		val = snd_ac97_read(ac97, AC97_PCM_MIC_ADC_RATE);
 		snd_iprintf(buffer, "PCM MIC ADC      : %iHz\n", val);
 	}
-	if ((ext & AC97_EI_SPDIF) || (ac97->flags & AC97_CS_SPDIF)) {
+	if ((ext & AC97_EI_SPDIF) || (ac97->flags & AC97_CS_SPDIF) ||
+	    (ac97->id == AC97_ID_YMF743)) {
 	        if (ac97->flags & AC97_CS_SPDIF)
 			val = snd_ac97_read(ac97, AC97_CSR_SPDIF);
-		else
+		else if (ac97->id == AC97_ID_YMF743) {
+			val = snd_ac97_read(ac97, AC97_YMF7X3_DIT_CTRL);
+			val = 0x2000 | (val & 0xff00) >> 4 | (val & 0x38) >> 2;
+		} else
 			val = snd_ac97_read(ac97, AC97_SPDIF);
 
 		snd_iprintf(buffer, "SPDIF Control    :%s%s%s%s Category=0x%x Generation=%i%s%s%s\n",
#6 lore raw

At Wed, 19 Sep 2007 20:34:53 +0900,

Quoted 6 lines · Keita Maehara

At Wed, 19 Sep 2007 20:28:22 +0900,

Quoted 1 line · Keita Maehara

Okay, here it is.

Quoted 8 lines

And this is the second part.

From: Keita Maehara <[email protected]>

These patches enable a few YMF743 controls (Tone/3D/IEC958) that won't be detected with the current version of ALSA.

Signed-off-by Keita Maehara <[email protected]>

Thanks! Now applied to HG tree.

Takashi