Change in coreboot[master]: soc/intel/jasperlake: Update acoustic noise related parameters
Maulik V Vaghela has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
soc/intel/jasperlake: Update acoustic noise related parameters
We need to fill Acoustic noise mitigation related UPDs only in case when acoustic noise mitigation is enabled. This will also clarify the user that they need to enable Acoustic noise mitigation while using this config in mainboard.
BUG=None BRANCH=dedede TEST=Check that UPD values are getting filled correctly.
Change-Id: I0cf4ccfced13b0d32b3d20713eace63e66945332 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 9 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/49187/1
diff --git a/src/soc/intel/jasperlake/fsp_params.c b/src/soc/intel/jasperlake/fsp_params.c index c03e9dd..0ccc461 100644 --- a/src/soc/intel/jasperlake/fsp_params.c +++ b/src/soc/intel/jasperlake/fsp_params.c @@ -237,12 +237,16 @@ config->PchPmPwrCycDur);
/* Fill Acoustic noise mitigation related configuration */ - params->FastPkgCRampDisable[0] = config->FastPkgCRampDisable; - params->SlowSlewRate[0] = config->SlowSlewRate; + params->AcousticNoiseMitigation = config->AcousticNoiseMitigation; - params->PreWake = config->PreWake; - params->RampUp = config->RampUp; - params->RampDown = config->RampDown; + + if (params->AcousticNoiseMitigation) { + params->FastPkgCRampDisable[0] = config->FastPkgCRampDisable; + params->SlowSlewRate[0] = config->SlowSlewRate; + params->PreWake = config->PreWake; + params->RampUp = config->RampUp; + params->RampDown = config->RampDown; + }
/* Override/Fill FSP Silicon Param for mainboard */ mainboard_silicon_init_params(params);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... PS1, Line 240: trailing whitespace
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... PS1, Line 240:
trailing whitespace
Check for superfluous whitespace in the tree (lint-stable-003-whitespace): File src/soc/intel/jasperlake/fsp_params.c:240: has lines ending with whitespace. test failed
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... PS1, Line 244: 0 For which VR domain is this? Why is the assumption here that the mainboard setting is for VR domain-0 only? Please see TGL as an example of how this is setup for other platforms: https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/tigerlake/c...
Hello build bot (Jenkins), Furquan Shaikh, Krishna P Bhat D, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/49187
to look at the new patch set (#2).
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
soc/intel/jasperlake: Update acoustic noise related parameters
We need to fill Acoustic noise mitigation related UPDs only in case when acoustic noise mitigation is enabled. This will also clarify the user that they need to enable Acoustic noise mitigation while using this config in mainboard.
BUG=None BRANCH=dedede TEST=Check that UPD values are getting filled correctly.
Change-Id: I0cf4ccfced13b0d32b3d20713eace63e66945332 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 9 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/49187/2
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... PS1, Line 240:
Check for superfluous whitespace in the tree (lint-stable-003-whitespace): File src/soc/intel/jasper […]
Done
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... PS1, Line 244: 0
For which VR domain is this? Why is the assumption here that the mainboard setting is for VR domain- […]
Hi Furquan, Jasperlake only supports VR domain 0 and it doesn't have multiple VR domains as in TGL. That is the reason we only set it for VR domain 0. Reference: JSL EDS vol1 (Document# 613601, chapter 3.4.1)
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... PS1, Line 244: 0
Hi Furquan, […]
Can you please mention that as a comment and also mention the name of the VR domain for clarity.
Hello build bot (Jenkins), Furquan Shaikh, Krishna P Bhat D, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/49187
to look at the new patch set (#3).
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
soc/intel/jasperlake: Update acoustic noise related parameters
We need to fill Acoustic noise mitigation related UPDs only in case when acoustic noise mitigation is enabled. This will also clarify the user that they need to enable Acoustic noise mitigation while using this config in mainboard.
We're only filling UPD for domain VR index 0 since there is only one VR domain for JSL (VCCIN VR). Reference: JSL EDS (Document# 613601) (Chapter 3.4)
BUG=None BRANCH=dedede TEST=UPD values are getting filled correctly when Acoustic noise mitigation is enabled.
Change-Id: I0cf4ccfced13b0d32b3d20713eace63e66945332 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/fsp_params.c 2 files changed, 15 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/49187/3
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... PS1, Line 244: 0
Can you please mention that as a comment and also mention the name of the VR domain for clarity.
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/ch... PS3, Line 356: VR domains VCCIN VR domain
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/ch... PS3, Line 363: VR domains VCCIN VR domain
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/1/src/soc/intel/jasperlake/fs... PS1, Line 244: 0
Done
Thanks Maulik!
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/fs... PS3, Line 243: if (params->AcousticNoiseMitigation) { Can you please add a comment here as well that there is a single VR domain i.e. VCCIN?
Hello build bot (Jenkins), Furquan Shaikh, Krishna P Bhat D, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/49187
to look at the new patch set (#4).
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
soc/intel/jasperlake: Update acoustic noise related parameters
We need to fill Acoustic noise mitigation related UPDs only in case when acoustic noise mitigation is enabled. This will also clarify the user that they need to enable Acoustic noise mitigation while using this config in mainboard.
We're only filling UPD for domain VR index 0 since there is only one VR domain for JSL (VCCIN VR). Reference: JSL EDS (Document# 613601) (Chapter 3.4)
BUG=None BRANCH=dedede TEST=UPD values are getting filled correctly when Acoustic noise mitigation is enabled.
Change-Id: I0cf4ccfced13b0d32b3d20713eace63e66945332 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/fsp_params.c 2 files changed, 21 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/49187/4
Hello build bot (Jenkins), Furquan Shaikh, Krishna P Bhat D, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/49187
to look at the new patch set (#5).
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
soc/intel/jasperlake: Update acoustic noise related parameters
We need to fill Acoustic noise mitigation related UPDs only in case when acoustic noise mitigation is enabled. This will also clarify the user that they need to enable Acoustic noise mitigation while using this config in mainboard.
We're only filling UPD for domain VR index 0 since there is only one VR domain for JSL (VCCIN VR). Reference: JSL EDS (Document# 613601) (Chapter 3.4)
BUG=None BRANCH=dedede TEST=UPD values are getting filled correctly when Acoustic noise mitigation is enabled.
Change-Id: I0cf4ccfced13b0d32b3d20713eace63e66945332 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/fsp_params.c 2 files changed, 21 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/49187/5
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/ch... PS3, Line 356: VR domains
VCCIN VR domain
Done
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/ch... PS3, Line 363: VR domains
VCCIN VR domain
Done
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/3/src/soc/intel/jasperlake/fs... PS3, Line 243: if (params->AcousticNoiseMitigation) {
Can you please add a comment here as well that there is a single VR domain i.e. […]
Done
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49187/5/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49187/5/src/soc/intel/jasperlake/fs... PS5, Line 241: * JSL only has single VR domain (VCCIN VR), thus filling only index 0 for trailing whitespace
Hello build bot (Jenkins), Furquan Shaikh, Krishna P Bhat D, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/49187
to look at the new patch set (#6).
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
soc/intel/jasperlake: Update acoustic noise related parameters
We need to fill Acoustic noise mitigation related UPDs only in case when acoustic noise mitigation is enabled. This will also clarify the user that they need to enable Acoustic noise mitigation while using this config in mainboard.
We're only filling UPD for domain VR index 0 since there is only one VR domain for JSL (VCCIN VR). Reference: JSL EDS (Document# 613601) (Chapter 3.4)
BUG=None BRANCH=dedede TEST=UPD values are getting filled correctly when Acoustic noise mitigation is enabled.
Change-Id: I0cf4ccfced13b0d32b3d20713eace63e66945332 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/fsp_params.c 2 files changed, 21 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/49187/6
Attention is currently required from: Maulik V Vaghela. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 6: Code-Review+2
Attention is currently required from: Maulik V Vaghela. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 6: Code-Review+2
Attention is currently required from: Maulik V Vaghela. Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
Patch Set 6: Code-Review+2
Karthik Ramasubramanian has submitted this change. ( https://review.coreboot.org/c/coreboot/+/49187 )
Change subject: soc/intel/jasperlake: Update acoustic noise related parameters ......................................................................
soc/intel/jasperlake: Update acoustic noise related parameters
We need to fill Acoustic noise mitigation related UPDs only in case when acoustic noise mitigation is enabled. This will also clarify the user that they need to enable Acoustic noise mitigation while using this config in mainboard.
We're only filling UPD for domain VR index 0 since there is only one VR domain for JSL (VCCIN VR). Reference: JSL EDS (Document# 613601) (Chapter 3.4)
BUG=None BRANCH=dedede TEST=UPD values are getting filled correctly when Acoustic noise mitigation is enabled.
Change-Id: I0cf4ccfced13b0d32b3d20713eace63e66945332 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/49187 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Karthik Ramasubramanian kramasub@google.com Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Evan Green evgreen@chromium.org --- M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/fsp_params.c 2 files changed, 21 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Evan Green: Looks good to me, approved Karthik Ramasubramanian: Looks good to me, approved
diff --git a/src/soc/intel/jasperlake/chip.h b/src/soc/intel/jasperlake/chip.h index 884071f..3b81339 100644 --- a/src/soc/intel/jasperlake/chip.h +++ b/src/soc/intel/jasperlake/chip.h @@ -353,18 +353,23 @@ uint8_t FivrSpreadSpectrum;
/* - * Disable Fast Slew Rate for Deep Package C States for VR domains + * Disable Fast Slew Rate for Deep Package C States for VCCIN VR domain * Disable Fast Slew Rate for Deep Package C States based on * Acoustic Noise Mitigation feature enabled. */ uint8_t FastPkgCRampDisable;
/* - * Slew Rate configuration for Deep Package C States for VR domains + * Slew Rate configuration for Deep Package C States for VCCIN VR domain * based on Acoustic Noise Mitigation feature enabled. * 0: Fast/2 ; 1: Fast/4; 2: Fast/8; 3: Fast/16 */ - uint8_t SlowSlewRate; + enum { + SlewRateFastBy2 = 0, + SlewRateFastBy4, + SlewRateFastBy8, + SlewRateFastBy16 + } SlowSlewRate;
/* * Enable or Disable Acoustic Noise Mitigation feature. diff --git a/src/soc/intel/jasperlake/fsp_params.c b/src/soc/intel/jasperlake/fsp_params.c index c03e9dd..eefbf6c 100644 --- a/src/soc/intel/jasperlake/fsp_params.c +++ b/src/soc/intel/jasperlake/fsp_params.c @@ -236,13 +236,20 @@ config->PchPmSlpS3MinAssert, config->PchPmSlpAMinAssert, config->PchPmPwrCycDur);
- /* Fill Acoustic noise mitigation related configuration */ - params->FastPkgCRampDisable[0] = config->FastPkgCRampDisable; - params->SlowSlewRate[0] = config->SlowSlewRate; + /* + * Fill Acoustic noise mitigation related configuration + * JSL only has single VR domain (VCCIN VR), thus filling only index 0 for + * Slew rate and FastPkgCRamp for VR0 only. + */ params->AcousticNoiseMitigation = config->AcousticNoiseMitigation; - params->PreWake = config->PreWake; - params->RampUp = config->RampUp; - params->RampDown = config->RampDown; + + if (params->AcousticNoiseMitigation) { + params->FastPkgCRampDisable[0] = config->FastPkgCRampDisable; + params->SlowSlewRate[0] = config->SlowSlewRate; + params->PreWake = config->PreWake; + params->RampUp = config->RampUp; + params->RampDown = config->RampDown; + }
/* Override/Fill FSP Silicon Param for mainboard */ mainboard_silicon_init_params(params);
-
build bot (Jenkins) (Code Review) -
Evan Green (Code Review)
-
Furquan Shaikh (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Maulik V Vaghela (Code Review)