snd-aoa: g5 tas codec problems

snd-aoa: g5 tas codec problems

[Benjamin Herrenschmidt]

Ok, so now I have it working on the G5 :)

(Patch below)

There are several issues (I would say in addition to what I listed
earlier):

 - Maybe it's me or maybe it's just too complicated, but I haven't quite
grasped the whole interaction between the soundbus,i2sbus,fabric and
codecs... especially initialisation ordering. It would be nice if we
could spend some time going through that and simplifying :) It leads to
at least one of the problems

 - The patch fixes a couple of nits related to having the modules
built-in: soundbus must really be a subsys_initcall() so it's
initialized before anybody else, and I've put the soundbus/ dir before
the codecs in the link order because the TAS is unhappy if loaded before
i2s (see below)

 - The TAS is a nasty beast. It needs the i2s clocks enabled or it goes
bunk... That's the problem with the G5. I've added a clock notifier and
reset it completely when the clocks come back, that's what fixes the G5
sound (looks like it loses state somewhat when not clocked). It would be
nice to streamline/cleanup some of the TAS handling, maybe with register
shadows like darwin or just with proper state variables to re-consitute
the whole thing and have a "fast mode" load since we need to do it so
often

 - Because of the above, starting to play a sound 1- takes some time to
init things and 2- clacks (setting the mutes on TAS before losing clocks
isn't enough, I think the fact that it goes bonk makes it parasite the
analog outputs). We need to also mute the amps around that. I haven't
quite figured how to do that from i2sbus though :) Also, we should try
(if not already the case) to cache our clock/i2s state so that
subsequent prepare() don't try to change things that are already ok.
That way we avoid having to blast the codec each time. But right now,
the important thing is to add mutes. People with external amplifiers
will really not like those clacs...

Note that the patch applies on top of Andreas latest one fixing the irq
on latest git.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Index: linux-irq-work/sound/aoa/soundbus/core.c
===================================================================
--- linux-irq-work.orig/sound/aoa/soundbus/core.c	2006-06-23 13:22:14.000000000 +1000
+++ linux-irq-work/sound/aoa/soundbus/core.c	2006-07-07 15:58:25.000000000 +1000
@@ -246,5 +246,5 @@
 }
 EXPORT_SYMBOL_GPL(soundbus_unregister_driver);
 
-module_init(soundbus_init);
+subsys_initcall(soundbus_init);
 module_exit(soundbus_exit);
Index: linux-irq-work/arch/powerpc/platforms/powermac/feature.c
===================================================================
--- linux-irq-work.orig/arch/powerpc/platforms/powermac/feature.c	2006-07-01 13:51:11.000000000 +1000
+++ linux-irq-work/arch/powerpc/platforms/powermac/feature.c	2006-07-07 16:13:20.000000000 +1000
@@ -2394,6 +2394,7 @@
 
 	return func(node, param, value);
 }
+EXPORT_SYMBOL_GPL(pmac_do_feature_call);
 
 static int __init probe_motherboard(void)
 {
Index: linux-irq-work/sound/aoa/Makefile
===================================================================
--- linux-irq-work.orig/sound/aoa/Makefile	2006-06-23 13:22:14.000000000 +1000
+++ linux-irq-work/sound/aoa/Makefile	2006-07-07 16:11:35.000000000 +1000
@@ -1,4 +1,4 @@
 obj-$(CONFIG_SND_AOA) += core/
-obj-$(CONFIG_SND_AOA) += codecs/
-obj-$(CONFIG_SND_AOA) += fabrics/
 obj-$(CONFIG_SND_AOA_SOUNDBUS) += soundbus/
+obj-$(CONFIG_SND_AOA) += fabrics/
+obj-$(CONFIG_SND_AOA) += codecs/
Index: linux-irq-work/sound/aoa/codecs/snd-aoa-codec-tas.c
===================================================================
--- linux-irq-work.orig/sound/aoa/codecs/snd-aoa-codec-tas.c	2006-07-07 15:46:30.000000000 +1000
+++ linux-irq-work/sound/aoa/codecs/snd-aoa-codec-tas.c	2006-07-07 17:45:06.000000000 +1000
@@ -75,19 +75,25 @@
 #include "../aoa.h"
 #include "../soundbus/soundbus.h"
 
-
 #define PFX "snd-aoa-codec-tas: "
 
+
 struct tas {
 	struct aoa_codec	codec;
 	struct i2c_client	i2c;
-	u32			muted_l:1, muted_r:1,
-				controls_created:1;
+	u32			mute_l:1, mute_r:1 ,
+				controls_created:1 ,
+				drc_enabled:1,
+				save_mute_l:1, save_mute_r:1,
+				hw_enabled:1;
 	u8			cached_volume_l, cached_volume_r;
 	u8			mixer_l[3], mixer_r[3];
 	u8			acr;
+	int			drc_range;
 };
 
+static int tas_reset_init(struct tas *tas);
+
 static struct tas *codec_to_tas(struct aoa_codec *codec)
 {
 	return container_of(codec, struct tas, codec);
@@ -101,6 +107,28 @@
 		return i2c_smbus_write_i2c_block_data(&tas->i2c, reg, len, data);
 }
 
+static void tas3004_set_drc(struct tas *tas)
+{
+	unsigned char val[6];
+
+	if (tas->drc_enabled)
+		val[0] = 0x50; /* 3:1 above threshold */
+	else
+		val[0] = 0x51; /* disabled */
+	val[1] = 0x02; /* 1:1 below threshold */
+	if (tas->drc_range > 0xef)
+		val[2] = 0xef;
+	else if (tas->drc_range < 0)
+		val[2] = 0x00;
+	else
+		val[2] = tas->drc_range;
+	val[3] = 0xb0;
+	val[4] = 0x60;
+	val[5] = 0xa0;
+
+	tas_write_reg(tas, TAS_REG_DRC, 6, val);
+}
+
 static void tas_set_volume(struct tas *tas)
 {
 	u8 block[6];
@@ -113,8 +141,8 @@
 	if (left > 177) left = 177;
 	if (right > 177) right = 177;
 
-	if (tas->muted_l) left = 0;
-	if (tas->muted_r) right = 0;
+	if (tas->mute_l) left = 0;
+	if (tas->mute_r) right = 0;
 
 	/* analysing the volume and mixer tables shows
 	 * that they are similar enough when we shift
@@ -202,7 +230,8 @@
 
 	tas->cached_volume_l = ucontrol->value.integer.value[0];
 	tas->cached_volume_r = ucontrol->value.integer.value[1];
-	tas_set_volume(tas);
+	if (tas->hw_enabled)
+		tas_set_volume(tas);
 	return 1;
 }
 
@@ -230,8 +259,8 @@
 {
 	struct tas *tas = snd_kcontrol_chip(kcontrol);
 
-	ucontrol->value.integer.value[0] = !tas->muted_l;
-	ucontrol->value.integer.value[1] = !tas->muted_r;
+	ucontrol->value.integer.value[0] = !tas->mute_l;
+	ucontrol->value.integer.value[1] = !tas->mute_r;
 	return 0;
 }
 
@@ -240,13 +269,14 @@
 {
 	struct tas *tas = snd_kcontrol_chip(kcontrol);
 
-	if (tas->muted_l == !ucontrol->value.integer.value[0]
-	 && tas->muted_r == !ucontrol->value.integer.value[1])
+	if (tas->mute_l == !ucontrol->value.integer.value[0]
+	 && tas->mute_r == !ucontrol->value.integer.value[1])
 		return 0;
 
-	tas->muted_l = !ucontrol->value.integer.value[0];
-	tas->muted_r = !ucontrol->value.integer.value[1];
-	tas_set_volume(tas);
+	tas->mute_l = !ucontrol->value.integer.value[0];
+	tas->mute_r = !ucontrol->value.integer.value[1];
+	if (tas->hw_enabled)
+		tas_set_volume(tas);
 	return 1;
 }
 
@@ -294,7 +324,8 @@
 	tas->mixer_l[idx] = ucontrol->value.integer.value[0];
 	tas->mixer_r[idx] = ucontrol->value.integer.value[1];
 
-	tas_set_mixer(tas);
+	if (tas->hw_enabled)
+		tas_set_mixer(tas);
 	return 1;
 }
 
@@ -310,7 +341,6 @@
 }
 
 MIXER_CONTROL(pcm1, "PCM1", 0);
-MIXER_CONTROL(pcm2, "PCM2", 1);
 MIXER_CONTROL(monitor, "Monitor", 2);
 
 static int tas_snd_capture_source_info(struct snd_kcontrol *kcontrol,
@@ -347,7 +377,8 @@
 		tas->acr |= TAS_ACR_INPUT_B;
 	if (oldacr == tas->acr)
 		return 0;
-	tas_write_reg(tas, TAS_REG_ACR, 1, &tas->acr);
+	if (tas->hw_enabled)
+		tas_write_reg(tas, TAS_REG_ACR, 1, &tas->acr);
 	return 1;
 }
 
@@ -400,26 +431,68 @@
 static int tas_reset_init(struct tas *tas)
 {
 	u8 tmp;
+
+	msleep(5);
 	tas->codec.gpio->methods->set_hw_reset(tas->codec.gpio, 0);
-	msleep(1);
+	msleep(5);
 	tas->codec.gpio->methods->set_hw_reset(tas->codec.gpio, 1);
-	msleep(1);
+	msleep(20);
 	tas->codec.gpio->methods->set_hw_reset(tas->codec.gpio, 0);
-	msleep(1);
-
-	tas->acr &= ~TAS_ACR_ANALOG_PDOWN;
-	tas->acr |= TAS_ACR_B_MONAUREAL | TAS_ACR_B_MON_SEL_RIGHT;
-	if (tas_write_reg(tas, TAS_REG_ACR, 1, &tas->acr))
-		return -ENODEV;
+	msleep(10);
 
 	tmp = TAS_MCS_SCLK64 | TAS_MCS_SPORT_MODE_I2S | TAS_MCS_SPORT_WL_24BIT;
 	if (tas_write_reg(tas, TAS_REG_MCS, 1, &tmp))
 		return -ENODEV;
 
+	tas->acr |= TAS_ACR_ANALOG_PDOWN | TAS_ACR_B_MONAUREAL |
+		TAS_ACR_B_MON_SEL_RIGHT;
+	if (tas_write_reg(tas, TAS_REG_ACR, 1, &tas->acr))
+		return -ENODEV;
+
 	tmp = 0;
 	if (tas_write_reg(tas, TAS_REG_MCS2, 1, &tmp))
 		return -ENODEV;
 
+	tas3004_set_drc(tas);
+
+	/* Set treble & bass to 0dB */
+	tmp = 114;
+	tas_write_reg(tas, TAS_REG_TREBLE, 1, &tmp);
+	tas_write_reg(tas, TAS_REG_BASS, 1, &tmp);
+
+	tas->acr &= ~TAS_ACR_ANALOG_PDOWN;
+	if (tas_write_reg(tas, TAS_REG_ACR, 1, &tas->acr))
+		return -ENODEV;
+
+	return 0;
+}
+
+static int tas_switch_clock(struct codec_info_item *cii, enum clock_switch clock)
+{
+	struct tas *tas = cii->codec_data;
+
+	switch(clock) {
+	case CLOCK_SWITCH_PREPARE_SLAVE:
+		/* Clocks are going away, mute mute mute */
+		tas->save_mute_l = tas->mute_l;
+		tas->save_mute_r = tas->mute_r;
+		tas->mute_l = tas->mute_l = 1;
+		tas_set_volume(tas);
+		tas->hw_enabled = 0;
+		break;
+	case CLOCK_SWITCH_SLAVE:
+		/* Clocks are back, re-init the codec */
+		tas->mute_l = tas->save_mute_l;
+		tas->mute_r = tas->save_mute_r;
+		tas_reset_init(tas);
+		tas_set_volume(tas);
+		tas_set_mixer(tas);
+		tas->hw_enabled = 1;
+		break;
+	default:
+		/* Do we want to return an error here ? */
+		return 0;
+	}
 	return 0;
 }
 
@@ -428,6 +501,7 @@
  * our i2c device is suspended, and then take note of that! */
 static int tas_suspend(struct tas *tas)
 {
+	tas->hw_enabled = 0;
 	tas->acr |= TAS_ACR_ANALOG_PDOWN;
 	tas_write_reg(tas, TAS_REG_ACR, 1, &tas->acr);
 	return 0;
@@ -439,6 +513,7 @@
 	tas_reset_init(tas);
 	tas_set_volume(tas);
 	tas_set_mixer(tas);
+	tas->hw_enabled = 1;
 	return 0;
 }
 
@@ -464,6 +539,7 @@
 	.bus_factor = 64,
 	.owner = THIS_MODULE,
 	.usable = tas_usable,
+	.switch_clock = tas_switch_clock,
 #ifdef CONFIG_PM
 	.suspend = _tas_suspend,
 	.resume = _tas_resume,
@@ -480,11 +556,6 @@
 		return -EINVAL;
 	}
 
-	if (tas_reset_init(tas)) {
-		printk(KERN_ERR PFX "tas failed to initialise\n");
-		return -ENXIO;
-	}
-
 	if (tas->codec.soundbus_dev->attach_codec(tas->codec.soundbus_dev,
 						   aoa_get_card(),
 						   &tas_codec_info, tas)) {
@@ -508,10 +579,6 @@
 	if (err)
 		goto error;
 
-	err = aoa_snd_ctl_add(snd_ctl_new1(&pcm2_control, tas));
-	if (err)
-		goto error;
-
 	err = aoa_snd_ctl_add(snd_ctl_new1(&monitor_control, tas));
 	if (err)
 		goto error;
@@ -553,6 +620,7 @@
 	tas->i2c.driver = &tas_driver;
 	tas->i2c.adapter = adapter;
 	tas->i2c.addr = addr;
+	tas->drc_range = TAS3004_DRC_MAX;
 	strlcpy(tas->i2c.name, "tas audio codec", I2C_NAME_SIZE-1);
 
 	if (i2c_attach_client(&tas->i2c)) {
@@ -569,7 +637,9 @@
 	if (aoa_codec_register(&tas->codec)) {
 		goto detach;
 	}
-	printk(KERN_DEBUG "snd-aoa-codec-tas: created and attached tas instance\n");
+	printk(KERN_DEBUG
+	       "snd-aoa-codec-tas: tas found, addr 0x%02x on %s\n",
+	       addr, node->full_name);
 	return 0;
  detach:
 	i2c_detach_client(&tas->i2c);
@@ -657,3 +727,5 @@
 
 module_init(tas_init);
 module_exit(tas_exit);
+
+
Index: linux-irq-work/sound/aoa/codecs/snd-aoa-codec-tas.h
===================================================================
--- linux-irq-work.orig/sound/aoa/codecs/snd-aoa-codec-tas.h	2006-06-23 13:22:14.000000000 +1000
+++ linux-irq-work/sound/aoa/codecs/snd-aoa-codec-tas.h	2006-07-07 16:58:17.000000000 +1000
@@ -44,4 +44,12 @@
 #define TAS_REG_LEFT_BIQUAD6	0x10
 #define TAS_REG_RIGHT_BIQUAD6	0x19
 
+#define TAS_REG_LEFT_LOUDNESS		0x21
+#define TAS_REG_RIGHT_LOUDNESS		0x22
+#define TAS_REG_LEFT_LOUDNESS_GAIN	0x23
+#define TAS_REG_RIGHT_LOUDNESS_GAIN	0x24
+
+#define TAS3001_DRC_MAX		0x5f
+#define TAS3004_DRC_MAX		0xef
+
 #endif /* __SND_AOA_CODECTASH */
Index: linux-irq-work/sound/aoa/core/snd-aoa-gpio-pmf.c
===================================================================
--- linux-irq-work.orig/sound/aoa/core/snd-aoa-gpio-pmf.c	2006-06-23 13:22:14.000000000 +1000
+++ linux-irq-work/sound/aoa/core/snd-aoa-gpio-pmf.c	2006-07-07 16:10:25.000000000 +1000
@@ -14,9 +14,13 @@
 static void pmf_gpio_set_##name(struct gpio_runtime *rt, int on)\
 {								\
 	struct pmf_args args = { .count = 1, .u[0].v = !on };	\
-								\
+	int rc;							\
+							\
 	if (unlikely(!rt)) return;				\
-	pmf_call_function(rt->node, #name "-mute", &args);	\
+	rc = pmf_call_function(rt->node, #name "-mute", &args);	\
+	if (rc)							\
+		printk(KERN_WARNING "pmf_gpio_set_" #name	\
+		" failed, rc: %d\n", rc);			\
 	rt->implementation_private &= ~(1<<bit);		\
 	rt->implementation_private |= (!!on << bit);		\
 }								\
@@ -33,9 +37,13 @@
 static void pmf_gpio_set_hw_reset(struct gpio_runtime *rt, int on)
 {
 	struct pmf_args args = { .count = 1, .u[0].v = !!on };
+	int rc;
 
 	if (unlikely(!rt)) return;
-	pmf_call_function(rt->node, "hw-reset", &args);
+	rc = pmf_call_function(rt->node, "hw-reset", &args);
+	if (rc)
+		printk(KERN_WARNING "pmf_gpio_set_hw_reset"
+		       " failed, rc: %d\n", rc);
 }
 
 static void pmf_gpio_all_amps_off(struct gpio_runtime *rt)
Index: linux-irq-work/sound/aoa/fabrics/snd-aoa-fabric-layout.c
===================================================================
--- linux-irq-work.orig/sound/aoa/fabrics/snd-aoa-fabric-layout.c	2006-07-01 13:51:30.000000000 +1000
+++ linux-irq-work/sound/aoa/fabrics/snd-aoa-fabric-layout.c	2006-07-07 16:17:43.000000000 +1000
@@ -950,11 +950,12 @@
 	layout_id = (unsigned int *) get_property(sound, "layout-id", NULL);
 	if (!layout_id)
 		goto outnodev;
-	printk(KERN_INFO "snd-aoa-fabric-layout: found bus with layout %d ", *layout_id);
+	printk(KERN_INFO "snd-aoa-fabric-layout: found bus with layout %d\n",
+	       *layout_id);
 
 	layout = find_layout_by_id(*layout_id);
 	if (!layout) {
-		printk("(no idea how to handle)\n");
+		printk(KERN_ERR "snd-aoa-fabric-layout: unknown layout\n");
 		goto outnodev;
 	}
 
@@ -972,15 +973,17 @@
 	case 51: /* PowerBook5,4 */
 	case 58: /* Mac Mini */
 		ldev->gpio.methods = ftr_gpio_methods;
+		printk(KERN_DEBUG
+		       "snd-aoa-fabric-layout: Using PMF GPIOs\n");
 		break;
 	default:
 		ldev->gpio.methods = pmf_gpio_methods;
+		printk(KERN_DEBUG
+		       "snd-aoa-fabric-layout: Using direct GPIOs\n");
 	}
 	ldev->selfptr_headphone.ptr = ldev;
 	ldev->selfptr_lineout.ptr = ldev;
 	sdev->ofdev.dev.driver_data = ldev;
-
-	printk("(using)\n");
 	list_add(&ldev->list, &layouts_list);
 	layouts_list_items++;
 

Re: snd-aoa: g5 tas codec problems

[Benjamin Herrenschmidt]

On Fri, 2006-07-07 at 17:47 +1000, Benjamin Herrenschmidt wrote:

> Note that the patch applies on top of Andreas latest one fixing the irq
> on latest git.

Actually, it applies on top of Andres patch, plus my patch fixing the
resources on g5, updated to apply on top of latest git:

I'll do a combo patch later (or you can do one if you want)

Ben.

Index: linux-irq-work/sound/aoa/soundbus/i2sbus/i2sbus-core.c
===================================================================
--- linux-irq-work.orig/sound/aoa/soundbus/i2sbus/i2sbus-core.c	2006-07-07 15:40:53.000000000 +1000
+++ linux-irq-work/sound/aoa/soundbus/i2sbus/i2sbus-core.c	2006-07-07 15:45:06.000000000 +1000
@@ -7,13 +7,16 @@
  */
 
 #include <linux/module.h>
-#include <asm/macio.h>
-#include <asm/dbdma.h>
 #include <linux/pci.h>
 #include <linux/interrupt.h>
+#include <linux/dma-mapping.h>
+
 #include <sound/driver.h>
 #include <sound/core.h>
-#include <linux/dma-mapping.h>
+
+#include <asm/macio.h>
+#include <asm/dbdma.h>
+
 #include "../soundbus.h"
 #include "i2sbus.h"
 
@@ -24,6 +27,12 @@
  * string that macio puts into the relevant device */
 MODULE_ALIAS("of:Ni2sTi2sC");
 
+static int force;
+module_param(force, int, 0444);
+MODULE_PARM_DESC(force, "Force loading i2sbus even when"
+		 " no layout-id property is present");
+
+
 static struct of_device_id i2sbus_match[] = {
 	{ .name = "i2s" },
 	{ }
@@ -73,7 +82,7 @@
  	if (i2sdev->intfregs) iounmap(i2sdev->intfregs);
  	if (i2sdev->out.dbdma) iounmap(i2sdev->out.dbdma);
  	if (i2sdev->in.dbdma) iounmap(i2sdev->in.dbdma);
-	for (i=0;i<3;i++)
+	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++)
 		if (i2sdev->allocated_resource[i])
 			release_and_free_resource(i2sdev->allocated_resource[i]);
 	free_dbdma_descriptor_ring(i2sdev, &i2sdev->out.dbdma_ring);
@@ -101,10 +110,47 @@
 	return IRQ_HANDLED;
 }
 
-static int force;
-module_param(force, int, 0444);
-MODULE_PARM_DESC(force, "Force loading i2sbus even when"
-			" no layout-id property is present");
+/*
+ * XXX FIXME: We have to test the layout_id's here to get the proper way
+ * of mapping in various registers, thanks to bugs in Apple device-trees.
+ * Ideally, that should be handled by the layout fabric but doing so would
+ * require a little bit of shuffling around
+ */
+static int i2sbus_get_and_fixup_rsrc(struct device_node *np, int index,
+				     int layout, struct resource *res)
+{
+	struct device_node *parent;
+	int pindex, rc = -ENXIO;
+	u32 *reg;
+
+	/* Machines with layout 76 and 36 (K2 based) have a weird device
+	 * tree what we need to special case.
+	 * Normal machines just fetch the resource from the i2s-X node.
+	 * Darwin further divides normal machines into old and new layouts
+	 * with a subtely different code path but that doesn't seem necessary
+	 * in practice, they just bloated it. In addition, even on our K2
+	 * case the i2s-modem node, if we ever want to handle it, uses the
+	 * normal layout
+	 */
+	if (layout != 76 && layout != 36)
+		return of_address_to_resource(np, index, res);
+
+	parent = of_get_parent(np);
+	pindex = (index == aoa_resource_i2smmio) ? 0 : 1;
+	rc = of_address_to_resource(parent, pindex, res);
+	if (rc)
+		goto bail;
+	reg = (u32 *)get_property(np, "reg", NULL);
+	if (reg == NULL) {
+		rc = -ENXIO;
+		goto bail;
+	}
+	res->start += reg[index * 2];
+	res->end = res->start + reg[index * 2 + 1] - 1;
+ bail:
+	of_node_put(parent);
+	return rc;
+}
 
 /* FIXME: look at device node refcounting */
 static int i2sbus_add_dev(struct macio_dev *macio,
@@ -113,7 +159,8 @@
 {
 	struct i2sbus_dev *dev;
 	struct device_node *child = NULL, *sound = NULL;
-	int i;
+	struct resource *r;
+	int i, layout = 0;
 	static const char *rnames[] = { "i2sbus: %s (control)",
 					"i2sbus: %s (tx)",
 					"i2sbus: %s (rx)" };
@@ -144,8 +191,9 @@
 		u32 *layout_id;
 		layout_id = (u32*) get_property(sound, "layout-id", NULL);
 		if (layout_id) {
+			layout = *layout_id;
 			snprintf(dev->sound.modalias, 32,
-				 "sound-layout-%d", *layout_id);
+				 "sound-layout-%d", layout);
 			force = 1;
 		}
 	}
@@ -175,23 +223,32 @@
 	dev->bus_number = np->name[4] - 'a';
 	INIT_LIST_HEAD(&dev->sound.codec_list);
 
-	for (i=0;i<3;i++) {
+	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++) {
 		dev->interrupts[i] = -1;
-		snprintf(dev->rnames[i], sizeof(dev->rnames[i]), rnames[i], np->name);
+		snprintf(dev->rnames[i], sizeof(dev->rnames[i]), rnames[i],
+			 np->name);
 	}
-	for (i=0;i<3;i++) {
+	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++) {
 		int irq = irq_of_parse_and_map(np, i);
 		if (request_irq(irq, ints[i], 0, dev->rnames[i], dev))
 			goto err;
 		dev->interrupts[i] = irq;
 	}
 
-	for (i=0;i<3;i++) {
-		if (of_address_to_resource(np, i, &dev->resources[i]))
+	/* Resource handling is problematic as some device-trees contain
+	 * useless crap (ugh ugh ugh). We work around that here by calling
+	 * specific functions for calculating the appropriate resources.
+	 *
+	 * This will all be moved to macio_asic.c at one point
+	 */
+	for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++) {
+		if (i2sbus_get_and_fixup_rsrc(np,i,layout,&dev->resources[i]))
 			goto err;
-		/* if only we could use our resource dev->resources[i]...
+
+		/* If only we could use our resource dev->resources[i]...
 		 * but request_resource doesn't know about parents and
-		 * contained resources... */
+		 * contained resources...
+		 */
 		dev->allocated_resource[i] = 
 			request_mem_region(dev->resources[i].start,
 					   dev->resources[i].end -
@@ -203,12 +260,12 @@
 		}
 	}
 	/* should do sanity checking here about length of them */
-	dev->intfregs = ioremap(dev->resources[0].start,
-				dev->resources[0].end-dev->resources[0].start+1);
-	dev->out.dbdma = ioremap(dev->resources[1].start,
-			 	 dev->resources[1].end-dev->resources[1].start+1);
-	dev->in.dbdma = ioremap(dev->resources[2].start,
-				dev->resources[2].end-dev->resources[2].start+1);
+	r = &dev->resources[aoa_resource_i2smmio];
+	dev->intfregs = ioremap(r->start, r->end - r->start + 1);
+	r = &dev->resources[aoa_resource_txdbdma];
+	dev->out.dbdma = ioremap(r->start, r->end - r->start + 1);
+	r = &dev->resources[aoa_resource_rxdbdma];
+	dev->in.dbdma = ioremap(r->start, r->end - r->start + 1);
 	if (!dev->intfregs || !dev->out.dbdma || !dev->in.dbdma)
 		goto err;
 
Index: linux-irq-work/sound/aoa/soundbus/i2sbus/i2sbus.h
===================================================================
--- linux-irq-work.orig/sound/aoa/soundbus/i2sbus/i2sbus.h	2006-06-23 13:22:14.000000000 +1000
+++ linux-irq-work/sound/aoa/soundbus/i2sbus/i2sbus.h	2006-07-07 15:41:40.000000000 +1000
@@ -7,20 +7,22 @@
  */
 #ifndef __I2SBUS_H
 #define __I2SBUS_H
-#include <asm/dbdma.h>
 #include <linux/interrupt.h>
-#include <sound/pcm.h>
 #include <linux/spinlock.h>
 #include <linux/mutex.h>
+
+#include <sound/pcm.h>
+
 #include <asm/prom.h>
+#include <asm/pmac_feature.h>
+#include <asm/dbdma.h>
+
 #include "i2sbus-interface.h"
-#include "i2sbus-control.h"
 #include "../soundbus.h"
 
 struct i2sbus_control {
-	volatile struct i2s_control_regs __iomem *controlregs;
-	struct resource rsrc;
 	struct list_head list;
+	struct macio_chip *macio;
 };
 
 #define MAX_DBDMA_COMMANDS	32
@@ -45,6 +47,12 @@
 	volatile struct dbdma_regs __iomem *dbdma;
 };
 
+enum {
+	aoa_resource_i2smmio = 0,
+	aoa_resource_txdbdma,
+	aoa_resource_rxdbdma,
+};
+
 struct i2sbus_dev {
 	struct soundbus_dev sound;
 	struct macio_dev *macio;
Index: linux-irq-work/sound/aoa/soundbus/i2sbus/i2sbus-control.c
===================================================================
--- linux-irq-work.orig/sound/aoa/soundbus/i2sbus/i2sbus-control.c	2006-06-23 13:22:14.000000000 +1000
+++ linux-irq-work/sound/aoa/soundbus/i2sbus/i2sbus-control.c	2006-07-07 15:41:40.000000000 +1000
@@ -6,12 +6,16 @@
  * GPL v2, can be found in COPYING.
  */
 
-#include <asm/io.h>
+#include <linux/kernel.h>
 #include <linux/delay.h>
+
+#include <asm/io.h>
 #include <asm/prom.h>
 #include <asm/macio.h>
 #include <asm/pmac_feature.h>
 #include <asm/pmac_pfunc.h>
+#include <asm/keylargo.h>
+
 #include "i2sbus.h"
 
 int i2sbus_control_init(struct macio_dev* dev, struct i2sbus_control **c)
@@ -22,26 +26,12 @@
 
 	INIT_LIST_HEAD(&(*c)->list);
 
-	if (of_address_to_resource(dev->ofdev.node, 0, &(*c)->rsrc))
-		goto err;
-	/* we really should be using feature calls instead of mapping
-	 * these registers. It's safe for now since no one else is
-	 * touching them... */
-	(*c)->controlregs = ioremap((*c)->rsrc.start,
-				    sizeof(struct i2s_control_regs));
-	if (!(*c)->controlregs)
-		goto err;
-
+	(*c)->macio = dev->bus->chip;
 	return 0;
- err:
-	kfree(*c);
-	*c = NULL;
-	return -ENODEV;
 }
 
 void i2sbus_control_destroy(struct i2sbus_control *c)
 {
-	iounmap(c->controlregs);
 	kfree(c);
 }
 
@@ -93,19 +83,19 @@
 			  struct i2sbus_dev *i2sdev)
 {
 	struct pmf_args args = { .count = 0 };
-	int cc;
+	struct macio_chip *macio = c->macio;
 
 	if (i2sdev->enable)
 		return pmf_call_one(i2sdev->enable, &args);
 
+	if (macio == NULL || macio->base == NULL)
+		return -ENODEV;
 	switch (i2sdev->bus_number) {
 	case 0:
-		cc = in_le32(&c->controlregs->cell_control);
-		out_le32(&c->controlregs->cell_control, cc | CTRL_CLOCK_INTF_0_ENABLE);
+		MACIO_BIS(KEYLARGO_FCR1, KL1_I2S0_ENABLE);
 		break;
 	case 1:
-		cc = in_le32(&c->controlregs->cell_control);
-		out_le32(&c->controlregs->cell_control, cc | CTRL_CLOCK_INTF_1_ENABLE);
+		MACIO_BIS(KEYLARGO_FCR1, KL1_I2S1_ENABLE);
 		break;
 	default:
 		return -ENODEV;
@@ -118,7 +108,7 @@
 			int enable)
 {
 	struct pmf_args args = { .count = 0 };
-	int cc;
+	struct macio_chip *macio = c->macio;
 
 	switch (enable) {
 	case 0:
@@ -133,18 +123,21 @@
 		printk(KERN_ERR "i2sbus: INVALID CELL ENABLE VALUE\n");
 		return -ENODEV;
 	}
+
+	if (macio == NULL || macio->base == NULL)
+		return -ENODEV;
 	switch (i2sdev->bus_number) {
 	case 0:
-		cc = in_le32(&c->controlregs->cell_control);
-		cc &= ~CTRL_CLOCK_CELL_0_ENABLE;
-		cc |= enable * CTRL_CLOCK_CELL_0_ENABLE;
-		out_le32(&c->controlregs->cell_control, cc);
+		if (enable)
+			MACIO_BIS(KEYLARGO_FCR1, KL1_I2S0_CELL_ENABLE);
+		else
+			MACIO_BIC(KEYLARGO_FCR1, KL1_I2S0_CELL_ENABLE);
 		break;
 	case 1:
-		cc = in_le32(&c->controlregs->cell_control);
-		cc &= ~CTRL_CLOCK_CELL_1_ENABLE;
-		cc |= enable * CTRL_CLOCK_CELL_1_ENABLE;
-		out_le32(&c->controlregs->cell_control, cc);
+		if (enable)
+			MACIO_BIS(KEYLARGO_FCR1, KL1_I2S1_CELL_ENABLE);
+		else
+			MACIO_BIC(KEYLARGO_FCR1, KL1_I2S1_CELL_ENABLE);
 		break;
 	default:
 		return -ENODEV;
@@ -157,7 +150,7 @@
 			 int enable)
 {
 	struct pmf_args args = { .count = 0 };
-	int cc;
+	struct macio_chip *macio = c->macio;
 
 	switch (enable) {
 	case 0:
@@ -172,18 +165,20 @@
 		printk(KERN_ERR "i2sbus: INVALID CLOCK ENABLE VALUE\n");
 		return -ENODEV;
 	}
+	if (macio == NULL || macio->base == NULL)
+		return -ENODEV;
 	switch (i2sdev->bus_number) {
 	case 0:
-		cc = in_le32(&c->controlregs->cell_control);
-		cc &= ~CTRL_CLOCK_CLOCK_0_ENABLE;
-		cc |= enable * CTRL_CLOCK_CLOCK_0_ENABLE;
-		out_le32(&c->controlregs->cell_control, cc);
+		if (enable)
+			MACIO_BIS(KEYLARGO_FCR1, KL1_I2S0_CLK_ENABLE_BIT);
+		else
+			MACIO_BIC(KEYLARGO_FCR1, KL1_I2S0_CLK_ENABLE_BIT);
 		break;
 	case 1:
-		cc = in_le32(&c->controlregs->cell_control);
-		cc &= ~CTRL_CLOCK_CLOCK_1_ENABLE;
-		cc |= enable * CTRL_CLOCK_CLOCK_1_ENABLE;
-		out_le32(&c->controlregs->cell_control, cc);
+		if (enable)
+			MACIO_BIS(KEYLARGO_FCR1, KL1_I2S1_CLK_ENABLE_BIT);
+		else
+			MACIO_BIC(KEYLARGO_FCR1, KL1_I2S1_CLK_ENABLE_BIT);
 		break;
 	default:
 		return -ENODEV;
Index: linux-irq-work/sound/aoa/soundbus/i2sbus/i2sbus-control.h
===================================================================
--- linux-irq-work.orig/sound/aoa/soundbus/i2sbus/i2sbus-control.h	2006-06-23 13:22:14.000000000 +1000
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,37 +0,0 @@
-/*
- * i2sbus driver -- bus register definitions
- *
- * Copyright 2006 Johannes Berg <johannes@sipsolutions.net>
- *
- * GPL v2, can be found in COPYING.
- */
-#ifndef __I2SBUS_CONTROLREGS_H
-#define __I2SBUS_CONTROLREGS_H
-
-/* i2s control registers, at least what we know about them */
-
-#define __PAD(m,n) u8 __pad##m[n]
-#define _PAD(line, n) __PAD(line, n)
-#define PAD(n) _PAD(__LINE__, (n))
-struct i2s_control_regs {
-	PAD(0x38);
-	__le32 fcr0;		/* 0x38 (unknown) */
-	__le32 cell_control;	/* 0x3c (fcr1) */
-	__le32 fcr2;		/* 0x40 (unknown) */
-	__le32 fcr3;		/* 0x44 (fcr3) */
-	__le32 clock_control;	/* 0x48 (unknown) */
-	PAD(4);
-	/* total size: 0x50 bytes */
-}  __attribute__((__packed__));
-
-#define CTRL_CLOCK_CELL_0_ENABLE	(1<<10)
-#define CTRL_CLOCK_CLOCK_0_ENABLE	(1<<12)
-#define CTRL_CLOCK_SWRESET_0		(1<<11)
-#define CTRL_CLOCK_INTF_0_ENABLE	(1<<13)
-
-#define CTRL_CLOCK_CELL_1_ENABLE	(1<<17)
-#define CTRL_CLOCK_CLOCK_1_ENABLE	(1<<18)
-#define CTRL_CLOCK_SWRESET_1		(1<<19)
-#define CTRL_CLOCK_INTF_1_ENABLE	(1<<20)
-
-#endif /* __I2SBUS_CONTROLREGS_H */
Index: linux-irq-work/sound/aoa/codecs/snd-aoa-codec-tas.c
===================================================================
--- linux-irq-work.orig/sound/aoa/codecs/snd-aoa-codec-tas.c	2006-06-23 13:22:14.000000000 +1000
+++ linux-irq-work/sound/aoa/codecs/snd-aoa-codec-tas.c	2006-07-07 17:49:19.000000000 +1000
@@ -310,6 +310,7 @@
 }
 
 MIXER_CONTROL(pcm1, "PCM1", 0);
+MIXER_CONTROL(pcm2, "PCM2", 1);
 MIXER_CONTROL(monitor, "Monitor", 2);
 
 static int tas_snd_capture_source_info(struct snd_kcontrol *kcontrol,
@@ -507,6 +508,10 @@
 	if (err)
 		goto error;
 
+	err = aoa_snd_ctl_add(snd_ctl_new1(&pcm2_control, tas));
+	if (err)
+		goto error;
+
 	err = aoa_snd_ctl_add(snd_ctl_new1(&monitor_control, tas));
 	if (err)
 		goto error;

Re: snd-aoa: g5 tas codec problems

[Johannes Berg]

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

On Fri, 2006-07-07 at 17:47 +1000, Benjamin Herrenschmidt wrote:
> Ok, so now I have it working on the G5 :)

Great :)
Powerbook woke itself an hour ago, no idea why, sorry for confusing you
on irc.

>  - Maybe it's me or maybe it's just too complicated, but I haven't quite
> grasped the whole interaction between the soundbus,i2sbus,fabric and
> codecs... especially initialisation ordering. It would be nice if we
> could spend some time going through that and simplifying :) It leads to
> at least one of the problems

Yeah, well... I sorta know.

>  - The patch fixes a couple of nits related to having the modules
> built-in: soundbus must really be a subsys_initcall() so it's
> initialized before anybody else, and I've put the soundbus/ dir before
> the codecs in the link order because the TAS is unhappy if loaded before
> i2s (see below)

Right.

>  - The TAS is a nasty beast. It needs the i2s clocks enabled or it goes
> bunk... That's the problem with the G5. I've added a clock notifier and
> reset it completely when the clocks come back, that's what fixes the G5
> sound (looks like it loses state somewhat when not clocked). It would be
> nice to streamline/cleanup some of the TAS handling, maybe with register
> shadows like darwin or just with proper state variables to re-consitute
> the whole thing and have a "fast mode" load since we need to do it so
> often

Ahrg

>  - Because of the above, starting to play a sound 1- takes some time to
> init things and 2- clacks (setting the mutes on TAS before losing clocks
> isn't enough, I think the fact that it goes bonk makes it parasite the
> analog outputs). We need to also mute the amps around that. I haven't
> quite figured how to do that from i2sbus though :) Also, we should try
> (if not already the case) to cache our clock/i2s state so that
> subsequent prepare() don't try to change things that are already ok.
> That way we avoid having to blast the codec each time. But right now,
> the important thing is to add mutes. People with external amplifiers
> will really not like those clacs...

Right. The way I did that with the onyx is that I told the GPIOs to mute
all amps from the clock switch callback.

I see you added DRC too, thanks :) I'll take a closer look after
breakfast ;)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: snd-aoa: g5 tas codec problems

[Benjamin Herrenschmidt]

> I see you added DRC too, thanks :) I'll take a closer look after
> breakfast ;)

Hehe :) Oh I just added a routine to program it in hardware, not any
control to actually change it.

Ben.

Re: snd-aoa: g5 tas codec problems

[Johannes Berg]

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

On Fri, 2006-07-07 at 17:47 +1000, Benjamin Herrenschmidt wrote:
> Also, we should try
> (if not already the case) to cache our clock/i2s state so that
> subsequent prepare() don't try to change things that are already ok. 

I do that, the function reads the dws and sfr registers and exits early
if they match.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: snd-aoa: g5 tas codec problems

[Benjamin Herrenschmidt]

On Fri, 2006-07-07 at 10:49 +0200, Johannes Berg wrote:
> On Fri, 2006-07-07 at 17:47 +1000, Benjamin Herrenschmidt wrote:
> > Also, we should try
> > (if not already the case) to cache our clock/i2s state so that
> > subsequent prepare() don't try to change things that are already ok. 
> 
> I do that, the function reads the dws and sfr registers and exits early
> if they match.

Ok. Be careful that I've removed the initial init of TAS thinking we
always get to clock restart to do it in prepare()... might need to be
put back.

Ben.

Re: snd-aoa: g5 tas codec problems

[Johannes Berg]

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

On Fri, 2006-07-07 at 18:56 +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2006-07-07 at 10:49 +0200, Johannes Berg wrote:
> > On Fri, 2006-07-07 at 17:47 +1000, Benjamin Herrenschmidt wrote:
> > > Also, we should try
> > > (if not already the case) to cache our clock/i2s state so that
> > > subsequent prepare() don't try to change things that are already ok. 
> > 
> > I do that, the function reads the dws and sfr registers and exits early
> > if they match.
> 
> Ok. Be careful that I've removed the initial init of TAS thinking we
> always get to clock restart to do it in prepare()... might need to be
> put back.

Oh yes, we do, wonder why it even worked then since most of the time
we'll be using whatever the firmware does (44.1KHz,16bit).

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: snd-aoa: g5 tas codec problems

[Johannes Berg]

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

On Fri, 2006-07-07 at 17:50 +1000, Benjamin Herrenschmidt wrote:

> I'll do a combo patch later (or you can do one if you want)

Don't, I just made the whole thing a 9 patch series.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]