[PATCH] First part of heterogenous dualchannel support

[Carl-Daniel Hailfinger]

First part of heterogenous dualchannel support.

Do not allow non-identical DIMMs yet, but prepare the code.

Calculate tCL related settings per DIMM in a dual channel setup. The
check for compatibility will come in a later patch, but since DIMMs
still have to be identical, this does not hurt.

Factor out tRC calculation to prepare for per-DIMM calculation.

Add diagnostic messages to tRC code.

Test booted to FILO, behaviour is identical if you ignore the added
debug messages (which are switched off by default).

Signed-off-by: Carl-Daniel Hailfinger

Index: LinuxBIOSv2-asus_m2a-vm/src/northbridge/amd/amdk8/raminit_f.c
===================================================================
--- LinuxBIOSv2-asus_m2a-vm/src/northbridge/amd/amdk8/raminit_f.c	(Revision 3866)
+++ LinuxBIOSv2-asus_m2a-vm/src/northbridge/amd/amdk8/raminit_f.c	(Arbeitskopie)
@@ -1823,26 +1823,36 @@
     * by both the memory controller and the dimms.
     */
    for (i = 0; i < DIMM_SOCKETS; i++) {
-		u32 spd_device = ctrl->channel0[i];
+		u32 spd_device;

printk_raminit("1.1 dimm_mask: %08x\n", meminfo->dimm_mask);
-		if (!(meminfo->dimm_mask & (1 << i))) {
-			if (meminfo->dimm_mask & (1 << (DIMM_SOCKETS + i))) { /* channelB only? */
-				spd_device = ctrl->channel1[i];
-			} else {
+		printk_raminit("i: %08x\n",i);
+
+		if (meminfo->dimm_mask & (1 << i)) {
+			spd_device = ctrl->channel0[i];
+			printk_raminit("Channel 0 settings:\n");
+
+			switch (find_optimum_spd_latency(spd_device, &min_latency, &min_cycle_time)) {
+			case -1:
+				goto hw_error;
+				break;
+			case 1:
    			continue;
    		}
    	}
+		if (meminfo->dimm_mask & (1 << (DIMM_SOCKETS + i))) {
+			spd_device = ctrl->channel1[i];
+			printk_raminit("Channel 1 settings:\n");

-		printk_raminit("i: %08x\n",i);
-
-		switch (find_optimum_spd_latency(spd_device, &min_latency, &min_cycle_time)) {
-		case -1:
-			goto hw_error;
-			break;
-		case 1:
-			continue;
-		}	
+			switch (find_optimum_spd_latency(spd_device, &min_latency, &min_cycle_time)) {
+			case -1:
+				goto hw_error;
+				break;
+			case 1:
+				continue;
+			}
+		}
+		
    }
    /* Make a second pass through the dimms and disable
     * any that cannot support the selected memclk and cas latency.
@@ -1941,37 +1951,55 @@
    valuex =  fraction [value & 0x7];
    return valuex;
 }
-static int update_dimm_Trc(const struct mem_controller *ctrl,
-			    const struct mem_param *param,
-			    int i, long dimm_mask)
+
+int get_dimm_Trc_clocks(u32 spd_device, const struct mem_param *param)
 {
-	unsigned clocks, old_clocks;
-	uint32_t dtl;
    int value;
    int value2;
-	u32 spd_device = ctrl->channel0[i];
-
-	if (!(dimm_mask & (1 << i)) && (dimm_mask & (1 << (DIMM_SOCKETS + i)))) { /* channelB only? */
-		spd_device = ctrl->channel1[i];
-	}
-
+	int clocks;
    value = spd_read_byte(spd_device, SPD_TRC);
-	if (value < 0) return -1;
+	if (value < 0)
+		return -1;
+	printk_raminit("update_dimm_Trc: tRC (41) = %08x\n", value);

value2 = spd_read_byte(spd_device, SPD_TRC -1);
    value <<= 2;
    value += convert_to_1_4(value2>>4);

-	value *=10;
+	value *= 10;
+	printk_raminit("update_dimm_Trc: tRC final value = %i\n", value);

clocks = (value + param->divisor - 1)/param->divisor;
+	printk_raminit("update_dimm_Trc: clocks = %i\n", clocks);

if (clocks < DTL_TRC_MIN) {
+#warning We should die here or at least disable this bank.
+		printk_notice("update_dimm_Trc: can't refresh fast enough, "
+			"want %i clocks, can %i clocks\n", clocks, DTL_TRC_MIN);
    	clocks = DTL_TRC_MIN;
    }
+	return clocks;
+}
+
+static int update_dimm_Trc(const struct mem_controller *ctrl,
+			    const struct mem_param *param,
+			    int i, long dimm_mask)
+{
+	int clocks, old_clocks;
+	uint32_t dtl;
+	u32 spd_device = ctrl->channel0[i];
+
+	if (!(dimm_mask & (1 << i)) && (dimm_mask & (1 << (DIMM_SOCKETS + i)))) { /* channelB only? */
+		spd_device = ctrl->channel1[i];
+	}
+
+	clocks = get_dimm_Trc_clocks(spd_device, param);
+	if (clocks == -1)
+		return clocks;
    if (clocks > DTL_TRC_MAX) {
    	return 0;
    }
+	printk_raminit("update_dimm_Trc: clocks after adjustment = %i\n", clocks);

dtl = pci_read_config32(ctrl->f2, DRAM_TIMING_LOW);
    old_clocks = ((dtl >> DTL_TRC_SHIFT) & DTL_TRC_MASK) + DTL_TRC_BASE;

[PATCH] First part of heterogenous dualchannel support

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> Calculate tCL related settings per DIMM in a dual channel setup.

..
> +++ LinuxBIOSv2-asus_m2a-vm/src/northbridge/amd/amdk8/raminit_f.c	(Arbeitskopie)
>    /* Make a second pass through the dimms and disable
>     * any that cannot support the selected memclk and cas latency.

This (existing) comment is reassuring.
> +int get_dimm_Trc_clocks(u32 spd_device, const struct mem_param *param)

..
> if (clocks < DTL_TRC_MIN) {
> +#warning We should die here or at least disable this bank.
> printk_notice("update_dimm_Trc: can't refresh fast enough, "
> 	"want %i clocks, can %i clocks\n", clocks, DTL_TRC_MIN);
> clocks = DTL_TRC_MIN;
> }
> return clocks;
>
> +}

This (existing) code not so much. How to combine the two?

Acked-by: Peter Stuge

[PATCH] First part of heterogenous dualchannel support

[Carl-Daniel Hailfinger]

On 16.01.2009 04:33, Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
> > Calculate tCL related settings per DIMM in a dual channel setup.
>
> ..
> > +++ LinuxBIOSv2-asus_m2a-vm/src/northbridge/amd/amdk8/raminit_f.c	(Arbeitskopie)
> >    /* Make a second pass through the dimms and disable
> >     * any that cannot support the selected memclk and cas latency.
>
> This (existing) comment is reassuring.

Unfortunately, it only checks one DIMM per channel, so the comment is a
bit misleading. I'll update that code as a next step, but probably won't
finish it before the end of the week, so if anybody has time...
> > +int get_dimm_Trc_clocks(u32 spd_device, const struct mem_param *param)
>
> ..
> > if (clocks < DTL_TRC_MIN) {
> > +#warning We should die here or at least disable this bank.
> > printk_notice("update_dimm_Trc: can't refresh fast enough, "
> > 	"want %i clocks, can %i clocks\n", clocks, DTL_TRC_MIN);
> > clocks = DTL_TRC_MIN;
> > }
> > return clocks;
> >
> > +}
>
> This (existing) code not so much.

I thought a warning would be in order instead of ignoring a possibly
catastrophical misconfiguration. However, I am not totally sure, so
adding a die() there was too extreme for my taste.
I'd appreciate any insight on the code in question.
> How to combine the two?

Unfortunately, they are unrelated code paths, so combining them is not
feasible.
> Acked-by: Peter Stuge

Thanks, r3867.

Regards,
Carl-Daniel