Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48657 )
Change subject: soc/intel/xeon_sp: Move DMICTL lock ......................................................................
soc/intel/xeon_sp: Move DMICTL lock
On SKX FPM-M does not return if this is set too early.
Tested on OCP/Tiogapass, boots.
Change-Id: Ib8ef7bab36bfd4b62988768753d10b4d7b7d567f Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/xeon_sp/cpx/chip.c M src/soc/intel/xeon_sp/include/soc/pch.h M src/soc/intel/xeon_sp/pch.c M src/soc/intel/xeon_sp/skx/chip.c 4 files changed, 9 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/48657/1
diff --git a/src/soc/intel/xeon_sp/cpx/chip.c b/src/soc/intel/xeon_sp/cpx/chip.c index 6beebdf..592a316 100644 --- a/src/soc/intel/xeon_sp/cpx/chip.c +++ b/src/soc/intel/xeon_sp/cpx/chip.c @@ -79,6 +79,7 @@ fsp_silicon_init(false); override_hpet_ioapic_bdf(); pch_enable_ioapic(); + pch_lock_dmictl(); setup_lapic(); p2sb_unhide(); } diff --git a/src/soc/intel/xeon_sp/include/soc/pch.h b/src/soc/intel/xeon_sp/include/soc/pch.h index 0be14ae..156a22a 100644 --- a/src/soc/intel/xeon_sp/include/soc/pch.h +++ b/src/soc/intel/xeon_sp/include/soc/pch.h @@ -10,5 +10,6 @@ #endif
void override_hpet_ioapic_bdf(void); +void pch_lock_dmictl(void);
#endif /* _SOC_PCH_H_ */ diff --git a/src/soc/intel/xeon_sp/pch.c b/src/soc/intel/xeon_sp/pch.c index 44824ca..d2496b1 100644 --- a/src/soc/intel/xeon_sp/pch.c +++ b/src/soc/intel/xeon_sp/pch.c @@ -40,9 +40,6 @@ reg32 = (0x3f << 18) | ACPI_BASE_ADDRESS | 1; pcr_write32(PID_DMI, PCR_DMI_ACPIBA, reg32); pcr_write32(PID_DMI, PCR_DMI_ACPIBDID, 0x23a8); - - reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL); - pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK); }
void bootblock_pch_init(void) @@ -69,3 +66,9 @@ p2sb_set_ioapic_bdf(ioapic_bdf); p2sb_set_hpet_bdf(hpet_bdf); } + +void pch_lock_dmictl(void) +{ + uint32_t reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL); + pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK); +} diff --git a/src/soc/intel/xeon_sp/skx/chip.c b/src/soc/intel/xeon_sp/skx/chip.c index 0505ea7..f101973 100644 --- a/src/soc/intel/xeon_sp/skx/chip.c +++ b/src/soc/intel/xeon_sp/skx/chip.c @@ -45,6 +45,7 @@ printk(BIOS_DEBUG, "coreboot: calling fsp_silicon_init\n"); fsp_silicon_init(false); override_hpet_ioapic_bdf(); + pch_lock_dmictl(); }
static void soc_final(void *data)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48657 )
Change subject: soc/intel/xeon_sp: Move DMICTL lock ......................................................................
Patch Set 1: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/48657/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48657/1//COMMIT_MSG@9 PS1, Line 9: FPM FSP
https://review.coreboot.org/c/coreboot/+/48657/1/src/soc/intel/xeon_sp/cpx/c... File src/soc/intel/xeon_sp/cpx/chip.c:
https://review.coreboot.org/c/coreboot/+/48657/1/src/soc/intel/xeon_sp/cpx/c... PS1, Line 82: pch_lock_dmictl(); Could this be moved to chip_final() instead?
https://review.coreboot.org/c/coreboot/+/48657/1/src/soc/intel/xeon_sp/pch.c File src/soc/intel/xeon_sp/pch.c:
https://review.coreboot.org/c/coreboot/+/48657/1/src/soc/intel/xeon_sp/pch.c... PS1, Line 72: uint32_t reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL); : pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK); pcr_or32 exists, btw
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48657 )
Change subject: soc/intel/xeon_sp: Move DMICTL lock ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48657/1/src/soc/intel/xeon_sp/cpx/c... File src/soc/intel/xeon_sp/cpx/chip.c:
https://review.coreboot.org/c/coreboot/+/48657/1/src/soc/intel/xeon_sp/cpx/c... PS1, Line 82: pch_lock_dmictl();
Could this be moved to chip_final() instead?
Needs to happen before LockConfig iirc.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48657 )
Change subject: soc/intel/xeon_sp: Move DMICTL lock ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/48657/1/src/soc/intel/xeon_sp/cpx/c... File src/soc/intel/xeon_sp/cpx/chip.c:
https://review.coreboot.org/c/coreboot/+/48657/1/src/soc/intel/xeon_sp/cpx/c... PS1, Line 82: pch_lock_dmictl();
Could this be moved to chip_final() instead? […]
Ack
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48657 )
Change subject: soc/intel/xeon_sp: Move DMICTL lock ......................................................................
Patch Set 1: Code-Review+2
Christian Walter has uploaded a new patch set (#2) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/48657 )
Change subject: soc/intel/xeon_sp: Move DMICTL lock ......................................................................
soc/intel/xeon_sp: Move DMICTL lock
On SKX FSP-M does not return if this is set too early.
Tested on OCP/Tiogapass, boots.
Change-Id: Ib8ef7bab36bfd4b62988768753d10b4d7b7d567f Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/xeon_sp/cpx/chip.c M src/soc/intel/xeon_sp/include/soc/pch.h M src/soc/intel/xeon_sp/pch.c M src/soc/intel/xeon_sp/skx/chip.c 4 files changed, 9 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/48657/2
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48657 )
Change subject: soc/intel/xeon_sp: Move DMICTL lock ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48657/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48657/1//COMMIT_MSG@9 PS1, Line 9: FPM
FSP
Ack
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48657 )
Change subject: soc/intel/xeon_sp: Move DMICTL lock ......................................................................
Patch Set 2: Code-Review+2
Marc Jones has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48657 )
Change subject: soc/intel/xeon_sp: Move DMICTL lock ......................................................................
soc/intel/xeon_sp: Move DMICTL lock
On SKX FSP-M does not return if this is set too early.
Tested on OCP/Tiogapass, boots.
Change-Id: Ib8ef7bab36bfd4b62988768753d10b4d7b7d567f Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/48657 Reviewed-by: Marc Jones marc@marcjonesconsulting.com Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Christian Walter christian.walter@9elements.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/xeon_sp/cpx/chip.c M src/soc/intel/xeon_sp/include/soc/pch.h M src/soc/intel/xeon_sp/pch.c M src/soc/intel/xeon_sp/skx/chip.c 4 files changed, 9 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Marc Jones: Looks good to me, approved Angel Pons: Looks good to me, approved Christian Walter: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/cpx/chip.c b/src/soc/intel/xeon_sp/cpx/chip.c index 6beebdf..592a316 100644 --- a/src/soc/intel/xeon_sp/cpx/chip.c +++ b/src/soc/intel/xeon_sp/cpx/chip.c @@ -79,6 +79,7 @@ fsp_silicon_init(false); override_hpet_ioapic_bdf(); pch_enable_ioapic(); + pch_lock_dmictl(); setup_lapic(); p2sb_unhide(); } diff --git a/src/soc/intel/xeon_sp/include/soc/pch.h b/src/soc/intel/xeon_sp/include/soc/pch.h index 0be14ae..156a22a 100644 --- a/src/soc/intel/xeon_sp/include/soc/pch.h +++ b/src/soc/intel/xeon_sp/include/soc/pch.h @@ -10,5 +10,6 @@ #endif
void override_hpet_ioapic_bdf(void); +void pch_lock_dmictl(void);
#endif /* _SOC_PCH_H_ */ diff --git a/src/soc/intel/xeon_sp/pch.c b/src/soc/intel/xeon_sp/pch.c index 44824ca..d2496b1 100644 --- a/src/soc/intel/xeon_sp/pch.c +++ b/src/soc/intel/xeon_sp/pch.c @@ -40,9 +40,6 @@ reg32 = (0x3f << 18) | ACPI_BASE_ADDRESS | 1; pcr_write32(PID_DMI, PCR_DMI_ACPIBA, reg32); pcr_write32(PID_DMI, PCR_DMI_ACPIBDID, 0x23a8); - - reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL); - pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK); }
void bootblock_pch_init(void) @@ -69,3 +66,9 @@ p2sb_set_ioapic_bdf(ioapic_bdf); p2sb_set_hpet_bdf(hpet_bdf); } + +void pch_lock_dmictl(void) +{ + uint32_t reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL); + pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK); +} diff --git a/src/soc/intel/xeon_sp/skx/chip.c b/src/soc/intel/xeon_sp/skx/chip.c index 0505ea7..f101973 100644 --- a/src/soc/intel/xeon_sp/skx/chip.c +++ b/src/soc/intel/xeon_sp/skx/chip.c @@ -45,6 +45,7 @@ printk(BIOS_DEBUG, "coreboot: calling fsp_silicon_init\n"); fsp_silicon_init(false); override_hpet_ioapic_bdf(); + pch_lock_dmictl(); }
static void soc_final(void *data)
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Christian Walter (Code Review)
-
Marc Jones (Code Review)