Meera Ravindranath has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37662 )
Change subject: soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT ......................................................................
soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT
Provide the PM1_TMR information in the FADT even if PmTimerDisabled is set because PM timer emulation is enabled via MSR 121h so the timer will still work and can be used by things like Tianocore and Windows.
Porting from https://review.coreboot.org/c/coreboot/+/23510/
Change-Id: Ie3d592623f3a84051477ffe83a0cf0daf30dd36f Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com --- M src/soc/intel/cannonlake/acpi.c M src/soc/intel/icelake/acpi.c M src/soc/intel/tigerlake/acpi.c 3 files changed, 24 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/37662/1
diff --git a/src/soc/intel/cannonlake/acpi.c b/src/soc/intel/cannonlake/acpi.c index 6846594..6d1970b 100644 --- a/src/soc/intel/cannonlake/acpi.c +++ b/src/soc/intel/cannonlake/acpi.c @@ -179,16 +179,14 @@ const struct soc_intel_cannonlake_config *config; config = config_of_soc();
- if (!config->PmTimerDisabled) { - fadt->pm_tmr_blk = pmbase + PM1_TMR; - fadt->pm_tmr_len = 4; - fadt->x_pm_tmr_blk.space_id = 1; - fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; - fadt->x_pm_tmr_blk.bit_offset = 0; - fadt->x_pm_tmr_blk.access_size = 0; - fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; - fadt->x_pm_tmr_blk.addrh = 0x0; - } + fadt->pm_tmr_blk = pmbase + PM1_TMR; + fadt->pm_tmr_len = 4; + fadt->x_pm_tmr_blk.space_id = 1; + fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; + fadt->x_pm_tmr_blk.bit_offset = 0; + fadt->x_pm_tmr_blk.access_size = 0; + fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; + fadt->x_pm_tmr_blk.addrh = 0x0;
if (config->s0ix_enable) fadt->flags |= ACPI_FADT_LOW_PWR_IDLE_S0; diff --git a/src/soc/intel/icelake/acpi.c b/src/soc/intel/icelake/acpi.c index 4089679..728cfb1 100644 --- a/src/soc/intel/icelake/acpi.c +++ b/src/soc/intel/icelake/acpi.c @@ -172,16 +172,14 @@
config_t *config = config_of_soc();
- if (!config->PmTimerDisabled) { - fadt->pm_tmr_blk = pmbase + PM1_TMR; - fadt->pm_tmr_len = 4; - fadt->x_pm_tmr_blk.space_id = 1; - fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; - fadt->x_pm_tmr_blk.bit_offset = 0; - fadt->x_pm_tmr_blk.access_size = 0; - fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; - fadt->x_pm_tmr_blk.addrh = 0x0; - } + fadt->pm_tmr_blk = pmbase + PM1_TMR; + fadt->pm_tmr_len = 4; + fadt->x_pm_tmr_blk.space_id = 1; + fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; + fadt->x_pm_tmr_blk.bit_offset = 0; + fadt->x_pm_tmr_blk.access_size = 0; + fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; + fadt->x_pm_tmr_blk.addrh = 0x0;
if (config->s0ix_enable) fadt->flags |= ACPI_FADT_LOW_PWR_IDLE_S0; diff --git a/src/soc/intel/tigerlake/acpi.c b/src/soc/intel/tigerlake/acpi.c index 225f4e8..5e04c9a 100644 --- a/src/soc/intel/tigerlake/acpi.c +++ b/src/soc/intel/tigerlake/acpi.c @@ -171,16 +171,14 @@
config_t *config = config_of_soc();
- if (!config->PmTimerDisabled) { - fadt->pm_tmr_blk = pmbase + PM1_TMR; - fadt->pm_tmr_len = 4; - fadt->x_pm_tmr_blk.space_id = 1; - fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; - fadt->x_pm_tmr_blk.bit_offset = 0; - fadt->x_pm_tmr_blk.access_size = 0; - fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; - fadt->x_pm_tmr_blk.addrh = 0x0; - } + fadt->pm_tmr_blk = pmbase + PM1_TMR; + fadt->pm_tmr_len = 4; + fadt->x_pm_tmr_blk.space_id = 1; + fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; + fadt->x_pm_tmr_blk.bit_offset = 0; + fadt->x_pm_tmr_blk.access_size = 0; + fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; + fadt->x_pm_tmr_blk.addrh = 0x0;
if (config->s0ix_enable) fadt->flags |= ACPI_FADT_LOW_PWR_IDLE_S0;
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37662 )
Change subject: soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT ......................................................................
Patch Set 1:
Please add the explanation in the commit message as inline comment as well, to make sure everyone looking at the code understands how it works.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37662 )
Change subject: soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/37662/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37662/1//COMMIT_MSG@13 PS1, Line 13: Porting from https://review.coreboot.org/c/coreboot/+/23510/ URLs can break with time and the referenced commit is already merged. So it would be better to use the commit hash (which can't change anymore), e.g.
Porting from 662b6cb3ed (soc/intel/skylake: Always add PM1_TMR block to FADT).
(output of `git log -1 --format='%h (%s)' 662b6cb`)
Hello Patrick Rudolph, Aamir Bohra, Maulik V Vaghela, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37662
to look at the new patch set (#2).
Change subject: soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT ......................................................................
soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT
Provide the PM1_TMR information in the FADT even if PmTimerDisabled is set because PM timer emulation is enabled via MSR 121h so the timer will still work and can be used by things like Tianocore and Windows.
Porting from Porting from 662b6cb3ed (soc/intel/skylake: Always add PM1_TMR block to FADT).
Change-Id: Ie3d592623f3a84051477ffe83a0cf0daf30dd36f Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com --- M src/soc/intel/cannonlake/acpi.c M src/soc/intel/icelake/acpi.c M src/soc/intel/tigerlake/acpi.c 3 files changed, 24 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/37662/2
Hello Patrick Rudolph, Aamir Bohra, Maulik V Vaghela, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37662
to look at the new patch set (#3).
Change subject: soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT ......................................................................
soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT
Provide the PM1_TMR information in the FADT even if PmTimerDisabled is set because PM timer emulation is enabled via MSR 121h so the timer will still work and can be used by things like Tianocore and Windows.
Porting from 662b6cb3ed (soc/intel/skylake: Always add PM1_TMR block to FADT).
Change-Id: Ie3d592623f3a84051477ffe83a0cf0daf30dd36f Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com --- M src/soc/intel/cannonlake/acpi.c M src/soc/intel/icelake/acpi.c M src/soc/intel/tigerlake/acpi.c 3 files changed, 24 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/37662/3
Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37662 )
Change subject: soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37662/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37662/1//COMMIT_MSG@13 PS1, Line 13: Porting from https://review.coreboot.org/c/coreboot/+/23510/
URLs can break with time and the referenced commit is already […]
Thanks, Nico!
Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37662 )
Change subject: soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37662/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37662/1//COMMIT_MSG@13 PS1, Line 13: Porting from https://review.coreboot.org/c/coreboot/+/23510/
Thanks, Nico!
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37662 )
Change subject: soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT ......................................................................
soc/intel/{icl,cnl,tgl}: Always add PM1_TMR block to FADT
Provide the PM1_TMR information in the FADT even if PmTimerDisabled is set because PM timer emulation is enabled via MSR 121h so the timer will still work and can be used by things like Tianocore and Windows.
Porting from 662b6cb3ed (soc/intel/skylake: Always add PM1_TMR block to FADT).
Change-Id: Ie3d592623f3a84051477ffe83a0cf0daf30dd36f Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37662 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/cannonlake/acpi.c M src/soc/intel/icelake/acpi.c M src/soc/intel/tigerlake/acpi.c 3 files changed, 24 insertions(+), 30 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/acpi.c b/src/soc/intel/cannonlake/acpi.c index 6846594..6d1970b 100644 --- a/src/soc/intel/cannonlake/acpi.c +++ b/src/soc/intel/cannonlake/acpi.c @@ -179,16 +179,14 @@ const struct soc_intel_cannonlake_config *config; config = config_of_soc();
- if (!config->PmTimerDisabled) { - fadt->pm_tmr_blk = pmbase + PM1_TMR; - fadt->pm_tmr_len = 4; - fadt->x_pm_tmr_blk.space_id = 1; - fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; - fadt->x_pm_tmr_blk.bit_offset = 0; - fadt->x_pm_tmr_blk.access_size = 0; - fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; - fadt->x_pm_tmr_blk.addrh = 0x0; - } + fadt->pm_tmr_blk = pmbase + PM1_TMR; + fadt->pm_tmr_len = 4; + fadt->x_pm_tmr_blk.space_id = 1; + fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; + fadt->x_pm_tmr_blk.bit_offset = 0; + fadt->x_pm_tmr_blk.access_size = 0; + fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; + fadt->x_pm_tmr_blk.addrh = 0x0;
if (config->s0ix_enable) fadt->flags |= ACPI_FADT_LOW_PWR_IDLE_S0; diff --git a/src/soc/intel/icelake/acpi.c b/src/soc/intel/icelake/acpi.c index 4089679..728cfb1 100644 --- a/src/soc/intel/icelake/acpi.c +++ b/src/soc/intel/icelake/acpi.c @@ -172,16 +172,14 @@
config_t *config = config_of_soc();
- if (!config->PmTimerDisabled) { - fadt->pm_tmr_blk = pmbase + PM1_TMR; - fadt->pm_tmr_len = 4; - fadt->x_pm_tmr_blk.space_id = 1; - fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; - fadt->x_pm_tmr_blk.bit_offset = 0; - fadt->x_pm_tmr_blk.access_size = 0; - fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; - fadt->x_pm_tmr_blk.addrh = 0x0; - } + fadt->pm_tmr_blk = pmbase + PM1_TMR; + fadt->pm_tmr_len = 4; + fadt->x_pm_tmr_blk.space_id = 1; + fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; + fadt->x_pm_tmr_blk.bit_offset = 0; + fadt->x_pm_tmr_blk.access_size = 0; + fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; + fadt->x_pm_tmr_blk.addrh = 0x0;
if (config->s0ix_enable) fadt->flags |= ACPI_FADT_LOW_PWR_IDLE_S0; diff --git a/src/soc/intel/tigerlake/acpi.c b/src/soc/intel/tigerlake/acpi.c index 225f4e8..5e04c9a 100644 --- a/src/soc/intel/tigerlake/acpi.c +++ b/src/soc/intel/tigerlake/acpi.c @@ -171,16 +171,14 @@
config_t *config = config_of_soc();
- if (!config->PmTimerDisabled) { - fadt->pm_tmr_blk = pmbase + PM1_TMR; - fadt->pm_tmr_len = 4; - fadt->x_pm_tmr_blk.space_id = 1; - fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; - fadt->x_pm_tmr_blk.bit_offset = 0; - fadt->x_pm_tmr_blk.access_size = 0; - fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; - fadt->x_pm_tmr_blk.addrh = 0x0; - } + fadt->pm_tmr_blk = pmbase + PM1_TMR; + fadt->pm_tmr_len = 4; + fadt->x_pm_tmr_blk.space_id = 1; + fadt->x_pm_tmr_blk.bit_width = fadt->pm_tmr_len * 8; + fadt->x_pm_tmr_blk.bit_offset = 0; + fadt->x_pm_tmr_blk.access_size = 0; + fadt->x_pm_tmr_blk.addrl = pmbase + PM1_TMR; + fadt->x_pm_tmr_blk.addrh = 0x0;
if (config->s0ix_enable) fadt->flags |= ACPI_FADT_LOW_PWR_IDLE_S0;
-
Meera Ravindranath (Code Review) -
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)