Angel Pons would like Felix Singer, Nico Huber, Arthur Heymans and Patrick Rudolph to review this change.

View Change

nb/intel/sandybridge: Clean up stepping logic

Do not combine the host bridge device ID with the CPU stepping.

Change-Id: I27ad609eb53b96987ad5445301b5392055fa4ea1
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
---
M src/cpu/intel/model_206ax/model_206ax.h
M src/northbridge/intel/sandybridge/gma.c
M src/northbridge/intel/sandybridge/northbridge.c
M src/northbridge/intel/sandybridge/sandybridge.h
4 files changed, 47 insertions(+), 41 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/48408/1
diff --git a/src/cpu/intel/model_206ax/model_206ax.h b/src/cpu/intel/model_206ax/model_206ax.h
index eb340ad..b6e2d65 100644
--- a/src/cpu/intel/model_206ax/model_206ax.h
+++ b/src/cpu/intel/model_206ax/model_206ax.h
@@ -3,6 +3,7 @@
#ifndef _CPU_INTEL_MODEL_206AX_H
#define _CPU_INTEL_MODEL_206AX_H

+#include <arch/cpu.h>
#include <stdint.h>

/* SandyBridge/IvyBridge bus clock is fixed at 100MHz */
@@ -88,4 +89,9 @@
int cpu_config_tdp_levels(void);
int get_platform_id(void);

+static inline u8 cpu_stepping(void)
+{
+ return cpuid_eax(1) & 0xf;
+}
+
#endif
diff --git a/src/northbridge/intel/sandybridge/gma.c b/src/northbridge/intel/sandybridge/gma.c
index f75dfda..86e12b0 100644
--- a/src/northbridge/intel/sandybridge/gma.c
+++ b/src/northbridge/intel/sandybridge/gma.c
@@ -4,6 +4,7 @@
#include <device/mmio.h>
#include <console/console.h>
#include <bootmode.h>
+#include <cpu/intel/model_206ax/model_206ax.h>
#include <delay.h>
#include <device/device.h>
#include <device/pci.h>
@@ -301,6 +302,8 @@

static void gma_pm_init_pre_vbios(struct device *dev)
{
+ const bool is_sandy = is_sandybridge();
+
u32 reg32;

printk(BIOS_DEBUG, "GT Power Management Init\n");
@@ -309,7 +312,7 @@
if (!gtt_res || !gtt_res->base)
return;

- if (bridge_silicon_revision() < IVB_STEP_C0) {
+ if (is_sandy || cpu_stepping() < IVB_STEP_C0) {
/* 1: Enable force wake */
gtt_write(0xa18c, 0x00000001);
gtt_poll(0x130090, (1 << 0), (1 << 0));
@@ -319,14 +322,14 @@
gtt_poll(0x130040, (1 << 0), (1 << 0));
}

- if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) {
+ if (is_sandy) {
/* 1d: Set GTT+0x42004 [15:14]=11 (SnB C1+) */
reg32 = gtt_read(0x42004);
reg32 |= (1 << 14) | (1 << 15);
gtt_write(0x42004, reg32);
}

- if (bridge_silicon_revision() >= IVB_STEP_A0) {
+ if (!is_sandy) {
/* Display Reset Acknowledge Settings */
reg32 = gtt_read(0x45010);
reg32 |= (1 << 1) | (1 << 0);
@@ -335,7 +338,7 @@

/* 2: Get GT SKU from GTT+0x911c[13] */
reg32 = gtt_read(0x911c);
- if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) {
+ if (is_sandy) {
if (reg32 & (1 << 13)) {
printk(BIOS_DEBUG, "SNB GT1 Power Meter Weights\n");
gtt_write_powermeter(snb_pm_gt1);
@@ -384,13 +387,12 @@
reg32 = gtt_read(0xa180);
reg32 |= (1 << 26) | (1 << 31);
/* (bit 20=1 for SNB step D1+ / IVB A0+) */
- if (bridge_silicon_revision() >= SNB_STEP_D1)
+ if (!is_sandy || cpu_stepping() >= SNB_STEP_D1)
reg32 |= (1 << 20);
gtt_write(0xa180, reg32);

/* 6a: for SnB step D2+ only */
- if (((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) &&
- (bridge_silicon_revision() >= SNB_STEP_D2)) {
+ if (is_sandy && cpu_stepping() >= SNB_STEP_D2) {
reg32 = gtt_read(0x9400);
reg32 |= (1 << 7);
gtt_write(0x9400, reg32);
@@ -402,16 +404,16 @@
gtt_poll(0x941c, (1 << 1), (0 << 1));
}

- if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_IVB) {
+ if (is_sandy) {
+ /* 6b: Clocking reset controls */
+ gtt_write(0x9424, 0x00000000);
+ } else {
reg32 = gtt_read(0x907c);
reg32 |= (1 << 16);
gtt_write(0x907c, reg32);

/* 6b: Clocking reset controls */
gtt_write(0x9424, 0x00000001);
- } else {
- /* 6b: Clocking reset controls */
- gtt_write(0x9424, 0x00000000);
}

/* 7 */
@@ -503,7 +505,7 @@
printk(BIOS_DEBUG, "GT Power Management Init (post VBIOS)\n");

/* 15: Deassert Force Wake */
- if (bridge_silicon_revision() < IVB_STEP_C0) {
+ if (is_sandybridge() || cpu_stepping() < IVB_STEP_C0) {
gtt_write(0xa18c, gtt_read(0xa18c) & ~1);
gtt_poll(0x130090, (1 << 0), (0 << 0));
} else {
diff --git a/src/northbridge/intel/sandybridge/northbridge.c b/src/northbridge/intel/sandybridge/northbridge.c
index 541bf73..942d135 100644
--- a/src/northbridge/intel/sandybridge/northbridge.c
+++ b/src/northbridge/intel/sandybridge/northbridge.c
@@ -4,31 +4,26 @@
#include <acpi/acpi.h>
#include <commonlib/helpers.h>
#include <device/pci_ops.h>
-#include <stdint.h>
#include <delay.h>
#include <cpu/intel/model_206ax/model_206ax.h>
#include <cpu/x86/msr.h>
#include <device/device.h>
#include <device/pci.h>
#include <device/pci_ids.h>
+#include <types.h>
#include "chip.h"
#include "sandybridge.h"
#include <cpu/intel/smm_reloc.h>

-static int bridge_revision_id = -1;
-
/* IGD UMA memory */
static uint64_t uma_memory_base = 0;
static uint64_t uma_memory_size = 0;

-int bridge_silicon_revision(void)
+bool is_sandybridge(void)
{
- if (bridge_revision_id < 0) {
- uint8_t stepping = cpuid_eax(1) & 0x0f;
- uint8_t bridge_id = pci_read_config16(pcidev_on_root(0, 0), PCI_DEVICE_ID);
- bridge_revision_id = (bridge_id & 0xf0) | stepping;
- }
- return bridge_revision_id;
+ const uint16_t bridge_id = pci_read_config16(pcidev_on_root(0, 0), PCI_DEVICE_ID);
+
+ return (bridge_id & BASE_REV_MASK) == BASE_REV_SNB;
}

/* Reserve everything between A segment and 1MB:
@@ -115,7 +110,7 @@
CONFIG_CHROMEOS_RAMOOPS_RAM_SIZE >> 10);
#endif

- if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) {
+ if (is_sandybridge()) {
/* Required for SandyBridge sighting 3715511 */
bad_ram_resource(dev, index++, 0x20000000 >> 10, 0x00200000 >> 10);
bad_ram_resource(dev, index++, 0x40000000 >> 10, 0x00200000 >> 10);
@@ -259,6 +254,8 @@

static void northbridge_dmi_init(struct device *dev)
{
+ const bool is_sandy = is_sandybridge();
+
u32 reg32;

/* Clear error status bits */
@@ -266,7 +263,7 @@
DMIBAR32(DMICESTS) = 0xffffffff;

/* Steps prior to DMI ASPM */
- if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) {
+ if (is_sandy) {
reg32 = DMIBAR32(0x250);
reg32 &= ~((1 << 22) | (1 << 20));
reg32 |= (1 << 21);
@@ -277,12 +274,12 @@
reg32 |= (1 << 29);
DMIBAR32(DMILLTC) = reg32;

- if (bridge_silicon_revision() >= SNB_STEP_D0) {
+ if (is_sandy && cpu_stepping() >= SNB_STEP_D0) {
reg32 = DMIBAR32(0x1f8);
reg32 |= (1 << 16);
DMIBAR32(0x1f8) = reg32;

- } else if (bridge_silicon_revision() >= SNB_STEP_D1) {
+ } else if (is_sandy && cpu_stepping() >= SNB_STEP_D1) {
reg32 = DMIBAR32(0x1f8);
reg32 &= ~(1 << 26);
reg32 |= (1 << 16);
@@ -294,7 +291,7 @@
}

/* Enable ASPM on SNB link, should happen before PCH link */
- if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) {
+ if (is_sandy) {
reg32 = DMIBAR32(0xd04);
reg32 |= (1 << 4);
DMIBAR32(0xd04) = reg32;
@@ -376,7 +373,10 @@
bridge_type = MCHBAR32(SAPMTIMERS);
bridge_type &= ~0xff;

- if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_IVB) {
+ if (is_sandybridge()) {
+ /* 20h for Sandybridge */
+ bridge_type |= 0x20;
+ } else {
/* Enable Power Aware Interrupt Routing */
u8 pair = MCHBAR8(INTRDIRCTL);
pair &= ~0x0f; /* Clear 3:0 */
@@ -385,9 +385,6 @@

/* 30h for IvyBridge */
bridge_type |= 0x30;
- } else {
- /* 20h for Sandybridge */
- bridge_type |= 0x20;
}
MCHBAR32(SAPMTIMERS) = bridge_type;

diff --git a/src/northbridge/intel/sandybridge/sandybridge.h b/src/northbridge/intel/sandybridge/sandybridge.h
index 8f9d118..e5f2132 100644
--- a/src/northbridge/intel/sandybridge/sandybridge.h
+++ b/src/northbridge/intel/sandybridge/sandybridge.h
@@ -9,22 +9,22 @@
#define BASE_REV_MASK 0x50

/* SandyBridge CPU stepping */
-#define SNB_STEP_D0 (BASE_REV_SNB + 5) /* Also J0 */
-#define SNB_STEP_D1 (BASE_REV_SNB + 6)
-#define SNB_STEP_D2 (BASE_REV_SNB + 7) /* Also J1/Q0 */
+#define SNB_STEP_D0 5 /* Also J0 */
+#define SNB_STEP_D1 6
+#define SNB_STEP_D2 7 /* Also J1/Q0 */

/* IvyBridge CPU stepping */
-#define IVB_STEP_A0 (BASE_REV_IVB + 0)
-#define IVB_STEP_B0 (BASE_REV_IVB + 2)
-#define IVB_STEP_C0 (BASE_REV_IVB + 4)
-#define IVB_STEP_K0 (BASE_REV_IVB + 5)
-#define IVB_STEP_D0 (BASE_REV_IVB + 6)
+#define IVB_STEP_A0 0
+#define IVB_STEP_B0 2
+#define IVB_STEP_C0 4
+#define IVB_STEP_K0 5
+#define IVB_STEP_D0 6

#include "memmap.h"

/* Everything below this line is ignored in the DSDT */
#ifndef __ACPI__
-#include <stdint.h>
+#include <types.h>

/* Chipset types */
enum platform_type {
@@ -87,8 +87,9 @@

#ifndef __ASSEMBLER__

+bool is_sandybridge(void);
+
void intel_sandybridge_finalize_smm(void);
-int bridge_silicon_revision(void);
void systemagent_early_init(void);
void sandybridge_init_iommu(void);
void sandybridge_late_initialization(void);

To view, visit change 48408. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I27ad609eb53b96987ad5445301b5392055fa4ea1
Gerrit-Change-Number: 48408
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-MessageType: newchange