Hello Iru Cai,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/41658
to review the following change.
Change subject: nb/intel/haswell: Add Crystal Well PCI IDs ......................................................................
nb/intel/haswell: Add Crystal Well PCI IDs
From a log of a machine using Crystal Well CPU [1], Crystal Well CPUs use some new PCI IDs. Without this patch, the Crystal Well northbridge cannot be initialized in ramstage, thus the machine cannot boot. The PCI IDs of Crystal Well related devices can be found online [2].
Not tested on real hardware yet.
[1] https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/DNHLQ... [2] https://pci-ids.ucw.cz/read/PC/8086
Change-Id: Icfe55323fd06187148c788ebfa7b679b6944e4f3 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M src/northbridge/intel/haswell/minihd.c M src/northbridge/intel/haswell/northbridge.c M src/northbridge/intel/haswell/pcie.c 3 files changed, 6 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/41658/1
diff --git a/src/northbridge/intel/haswell/minihd.c b/src/northbridge/intel/haswell/minihd.c index f8a16e2..0a0a861 100644 --- a/src/northbridge/intel/haswell/minihd.c +++ b/src/northbridge/intel/haswell/minihd.c @@ -97,7 +97,7 @@ .ops_pci = &minihd_pci_ops, };
-static const unsigned short pci_device_ids[] = { 0x0a0c, 0x0c0c, 0 }; +static const unsigned short pci_device_ids[] = { 0x0a0c, 0x0c0c, 0x0d0c, 0 };
static const struct pci_driver haswell_minihd __pci_driver = { .ops = &minihd_ops, diff --git a/src/northbridge/intel/haswell/northbridge.c b/src/northbridge/intel/haswell/northbridge.c index a728e0e..c1bc245 100644 --- a/src/northbridge/intel/haswell/northbridge.c +++ b/src/northbridge/intel/haswell/northbridge.c @@ -466,6 +466,7 @@ 0x0c04, /* Mobile */ 0x0a04, /* ULT */ 0x0c08, /* Server */ + 0x0d00, 0x0d04, /* Crystal Well */ 0 };
diff --git a/src/northbridge/intel/haswell/pcie.c b/src/northbridge/intel/haswell/pcie.c index 74f11dd..73e909e 100644 --- a/src/northbridge/intel/haswell/pcie.c +++ b/src/northbridge/intel/haswell/pcie.c @@ -65,7 +65,10 @@ #endif };
-static const unsigned short pci_device_ids[] = { 0x0c01, 0x0c05, 0x0c09, 0x0c0d, 0 }; +static const unsigned short pci_device_ids[] = { + 0x0c01, 0x0c05, 0x0c09, 0x0c0d, + 0x0d01, 0x0d05, 0x0d09, /* Crystal Well */ + 0 };
static const struct pci_driver pch_pcie __pci_driver = { .ops = &device_ops,
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41658 )
Change subject: nb/intel/haswell: Add Crystal Well PCI IDs ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/41658/1/src/northbridge/intel/haswe... File src/northbridge/intel/haswell/northbridge.c:
https://review.coreboot.org/c/coreboot/+/41658/1/src/northbridge/intel/haswe... PS1, Line 469: 0x0d00, 0x0d04, /* Crystal Well */ Can we add each on a separate line for consistency, please?
0x0d00 is Desktop, and 0x0d04 is Mobile
Also add 0x0d08 as Crystalwell Server, by extrapolation
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph, Iru Cai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41658
to look at the new patch set (#2).
Change subject: nb/intel/haswell: Add Crystal Well PCI IDs ......................................................................
nb/intel/haswell: Add Crystal Well PCI IDs
From a log of a machine using Crystal Well CPU [1], Crystal Well CPUs use some new PCI IDs. Without this patch, the Crystal Well northbridge cannot be initialized in ramstage, thus the machine cannot boot. Some PCI IDs of Crystal Well related devices can be found in the PCI ID database [2].
Tested with i5-4570R (with LGA1150 mod) on ASRock H81M-HDS. The board boots to SeaBIOS with boot screen displayed on HDMI output, and then boots Arch Linux on a USB disk.
[1] https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/DNHLQ... [2] https://pci-ids.ucw.cz/read/PC/8086
Change-Id: Icfe55323fd06187148c788ebfa7b679b6944e4f3 Signed-off-by: Iru Cai mytbk920423@gmail.com --- M src/northbridge/intel/haswell/gma.c M src/northbridge/intel/haswell/minihd.c M src/northbridge/intel/haswell/northbridge.c M src/northbridge/intel/haswell/pcie.c 4 files changed, 15 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/41658/2
Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41658 )
Change subject: nb/intel/haswell: Add Crystal Well PCI IDs ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41658/1/src/northbridge/intel/haswe... File src/northbridge/intel/haswell/northbridge.c:
https://review.coreboot.org/c/coreboot/+/41658/1/src/northbridge/intel/haswe... PS1, Line 469: 0x0d00, 0x0d04, /* Crystal Well */
Can we add each on a separate line for consistency, please? […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41658 )
Change subject: nb/intel/haswell: Add Crystal Well PCI IDs ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/41658/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41658/2//COMMIT_MSG@16 PS2, Line 16: with boot screen displayed on HDMI output Out of curiosity, is this with libgfxinit?
Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41658 )
Change subject: nb/intel/haswell: Add Crystal Well PCI IDs ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41658/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41658/2//COMMIT_MSG@16 PS2, Line 16: with boot screen displayed on HDMI output
Out of curiosity, is this with libgfxinit?
Yes, with libgfxinit. console log: https://paste.debian.net/1158334
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41658 )
Change subject: nb/intel/haswell: Add Crystal Well PCI IDs ......................................................................
nb/intel/haswell: Add Crystal Well PCI IDs
From a log of a machine using Crystal Well CPU [1], Crystal Well CPUs use some new PCI IDs. Without this patch, the Crystal Well northbridge cannot be initialized in ramstage, thus the machine cannot boot. Some PCI IDs of Crystal Well related devices can be found in the PCI ID database [2].
Tested with i5-4570R (with LGA1150 mod) on ASRock H81M-HDS. The board boots to SeaBIOS with boot screen displayed on HDMI output, and then boots Arch Linux on a USB disk.
[1] https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/DNHLQ... [2] https://pci-ids.ucw.cz/read/PC/8086
Change-Id: Icfe55323fd06187148c788ebfa7b679b6944e4f3 Signed-off-by: Iru Cai mytbk920423@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41658 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/haswell/gma.c M src/northbridge/intel/haswell/minihd.c M src/northbridge/intel/haswell/northbridge.c M src/northbridge/intel/haswell/pcie.c 4 files changed, 15 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/northbridge/intel/haswell/gma.c b/src/northbridge/intel/haswell/gma.c index 68072ff..c466c09 100644 --- a/src/northbridge/intel/haswell/gma.c +++ b/src/northbridge/intel/haswell/gma.c @@ -108,6 +108,11 @@ case 0x8086042a: /* GT3 Server */ case 0x80860a26: /* GT3 ULT */
+ case 0x80860d22: /* GT3e Desktop */ + case 0x80860d16: /* GT1 Mobile 4+3 */ + case 0x80860d26: /* GT2 Mobile 4+3, GT3e Mobile */ + case 0x80860d36: /* GT3 Mobile 4+3 */ + new_vendev = 0x80860406; /* GT1 Mobile */ break; } @@ -515,11 +520,12 @@ 0x0402, /* Desktop GT1 */ 0x0412, /* Desktop GT2 */ 0x0422, /* Desktop GT3 */ + 0x0d22, /* Desktop GT3e */ 0x0406, /* Mobile GT1 */ 0x0416, /* Mobile GT2 */ 0x0426, /* Mobile GT3 */ 0x0d16, /* Mobile 4+3 GT1 */ - 0x0d26, /* Mobile 4+3 GT2 */ + 0x0d26, /* Mobile 4+3 GT2, Mobile GT3e */ 0x0d36, /* Mobile 4+3 GT3 */ 0x0a06, /* ULT GT1 */ 0x0a16, /* ULT GT2 */ diff --git a/src/northbridge/intel/haswell/minihd.c b/src/northbridge/intel/haswell/minihd.c index de2ce06..1bfbfe5 100644 --- a/src/northbridge/intel/haswell/minihd.c +++ b/src/northbridge/intel/haswell/minihd.c @@ -92,7 +92,7 @@ .ops_pci = &pci_dev_ops_pci, };
-static const unsigned short pci_device_ids[] = { 0x0a0c, 0x0c0c, 0 }; +static const unsigned short pci_device_ids[] = { 0x0a0c, 0x0c0c, 0x0d0c, 0 };
static const struct pci_driver haswell_minihd __pci_driver = { .ops = &minihd_ops, diff --git a/src/northbridge/intel/haswell/northbridge.c b/src/northbridge/intel/haswell/northbridge.c index 99621c2..0a5af4f 100644 --- a/src/northbridge/intel/haswell/northbridge.c +++ b/src/northbridge/intel/haswell/northbridge.c @@ -520,6 +520,9 @@ 0x0c04, /* Mobile */ 0x0a04, /* ULT */ 0x0c08, /* Server */ + 0x0d00, /* Crystal Well Desktop */ + 0x0d04, /* Crystal Well Mobile */ + 0x0d08, /* Crystal Well Server (by extrapolation) */ 0 };
diff --git a/src/northbridge/intel/haswell/pcie.c b/src/northbridge/intel/haswell/pcie.c index a47a2f5..d9e77a7 100644 --- a/src/northbridge/intel/haswell/pcie.c +++ b/src/northbridge/intel/haswell/pcie.c @@ -61,7 +61,10 @@ #endif };
-static const unsigned short pci_device_ids[] = { 0x0c01, 0x0c05, 0x0c09, 0x0c0d, 0 }; +static const unsigned short pci_device_ids[] = { + 0x0c01, 0x0c05, 0x0c09, 0x0c0d, + 0x0d01, 0x0d05, 0x0d09, /* Crystal Well */ + 0 };
static const struct pci_driver pch_pcie __pci_driver = { .ops = &device_ops,
-
Angel Pons (Code Review) -
Iru Cai (vimacs) (Code Review)
-
Patrick Georgi (Code Review)