Shaunak Saha has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
soc/intel/tigerlake: Add Acoustic features
Expose the following FSP UPD interface into coreboot, which is the following: AcousticNoiseMitigation FastPkgCRampDisable SlowSlewRateFor AcousticNoiseMitigation is the UPD for Acoustic Noise Mitigation. FastPkgCRampDisable UPD's are to disable Fast Slew Rate for Deep Package C States for VR domains. SlowSlewRate UPD's are the Slew Rate configuration for Deep Package C States for VR domains. Slew rates are fast time divided by 2.
BUG=b:153015585 BRANCH=none TEST= Measure the change in noise level by changing the UPD values.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I1924a3bac8beb16a9d841891696f9a3dea0d425f --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 36 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/45052/1
diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h index 2da63ed..8d2483e 100644 --- a/src/soc/intel/tigerlake/chip.h +++ b/src/soc/intel/tigerlake/chip.h @@ -145,6 +145,35 @@ /* Wake Enable Bitmap for USB3 ports */ uint16_t usb3_wake_enable_bitmap;
+ /* + * Acoustic Noise Mitigation + * 0 - Disable + * 1 - Enable noise mitigation + */ + uint8_t AcousticNoiseMitigation; + + /* + * Offset 0x054B - Disable Fast Slew Rate for Deep Package + * C States for VR domains. Disable Fast Slew Rate for Deep + * Package C States based on Acoustic Noise Mitigation feature + * enabled. + * 0 - False + * 1 - True + */ + uint8_t FastPkgCRampDisable[5]; + + /* + * Offset 0x0550 - Slew Rate configuration for Deep Package + * C States for VR domains. Slew Rate configuration for Deep + * Package C States for VR domains based on Acoustic Noise + * Mitigation feature enabled. + * 0 - Fast/2 + * 1 - Fast/4 + * 2 - Fast/8 + * 3 - Fast/16 + */ + uint8_t SlowSlewRate[5]; + /* SATA related */ uint8_t SataEnable; uint8_t SataMode; diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c index 1601c2c..39baa67 100644 --- a/src/soc/intel/tigerlake/fsp_params.c +++ b/src/soc/intel/tigerlake/fsp_params.c @@ -247,6 +247,13 @@ } }
+ params->AcousticNoiseMitigation = config->AcousticNoiseMitigation; + for (i = 0; i < ARRAY_SIZE(config->SlowSlewRate); i++) + params->SlowSlewRate[i] = config->SlowSlewRate[i]; + + for (i = 0; i < ARRAY_SIZE(config->FastPkgCRampDisable); i++) + params->FastPkgCRampDisable[i] = config->FastPkgCRampDisable[i]; + /* Enable TCPU for processor thermal control */ params->Device4Enable = config->Device4Enable;
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45052/1/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/45052/1/src/soc/intel/tigerlake/chi... PS1, Line 163: 5 Can you indicate in the comment which domains these array indices correspond to?
https://review.coreboot.org/c/coreboot/+/45052/1/src/soc/intel/tigerlake/chi... PS1, Line 175: space here instead of tab
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Eric Herrmann, Duncan Laurie, Tim Wawrzynczak, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45052
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
soc/intel/tigerlake: Add Acoustic features
Expose the following FSP UPD interface into coreboot, which is the following: AcousticNoiseMitigation FastPkgCRampDisable SlowSlewRateFor AcousticNoiseMitigation is the UPD for Acoustic Noise Mitigation. FastPkgCRampDisable UPD's are to disable Fast Slew Rate for Deep Package C States for VR domains. SlowSlewRate UPD's are the Slew Rate configuration for Deep Package C States for VR domains. Slew rates are fast time divided by 2.
BUG=b:153015585 BRANCH=none TEST= Measure the change in noise level by changing the UPD values.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I1924a3bac8beb16a9d841891696f9a3dea0d425f --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 36 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/45052/2
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45052/1/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/45052/1/src/soc/intel/tigerlake/chi... PS1, Line 163: 5
Can you indicate in the comment which domains these array indices correspond to?
Done
https://review.coreboot.org/c/coreboot/+/45052/1/src/soc/intel/tigerlake/chi... PS1, Line 175:
space here instead of tab
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 2:
I think this needs to be rebased on top of CB:45630
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 3:
Patch Set 2:
I think this needs to be rebased on top of CB:45630
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/chi... PS3, Line 157: IA,GT,SA,VLCC and FIVR Probably add enums for these?
enum vr_domains { VR_DOMAIN_IA, VR_DOMAIN_GT, VR_DOMAIN_SA, VR_DOMAIN_VLCC, VR_DOMAIN_FIVR, VR_DOMAIN_MAX };
VR_DOMAIN_MAX can be used for FastPkgCRampDisable and SlowSlewRate here and mainboard devicetree can use VR_DOMAIN_* when performing the configuration.
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/chi... PS3, Line 168: 0 - Fast/2 : * 1 - Fast/4 : * 2 - Fast/8 : * 3 - Fast/16 : */ Add enums for these?
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/fsp... PS3, Line 259: for (i = 0; i < ARRAY_SIZE(config->SlowSlewRate); i++) Use memcpy here and below?
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/chi... PS3, Line 157: IA,GT,SA,VLCC and FIVR
Probably add enums for these? […]
ok.Will Fix.
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/chi... PS3, Line 168: 0 - Fast/2 : * 1 - Fast/4 : * 2 - Fast/8 : * 3 - Fast/16 : */
Add enums for these?
OK. Will use enums.
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/fsp... PS3, Line 259: for (i = 0; i < ARRAY_SIZE(config->SlowSlewRate); i++)
Use memcpy here and below?
ok.Will use memcpy.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Eric Herrmann, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45052
to look at the new patch set (#4).
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
soc/intel/tigerlake: Add Acoustic features
Expose the following FSP UPD interface into coreboot, which is the following: AcousticNoiseMitigation FastPkgCRampDisable SlowSlewRateFor AcousticNoiseMitigation is the UPD for Acoustic Noise Mitigation. FastPkgCRampDisable UPD's are to disable Fast Slew Rate for Deep Package C States for VR domains. SlowSlewRate UPD's are the Slew Rate configuration for Deep Package C States for VR domains. Slew rates are fast time divided by 2.
BUG=b:153015585 BRANCH=none TEST= Measure the change in noise level by changing the UPD values.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I1924a3bac8beb16a9d841891696f9a3dea0d425f --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 62 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/45052/4
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45052/5/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/45052/5/src/soc/intel/tigerlake/chi... PS5, Line 153: /* : * VR domains. The domains are IA,GT,SA,VLCC and FIVR. : */ : enum { : VR_DOMAIN_IA, : VR_DOMAIN_GT, : VR_DOMAIN_SA, : VR_DOMAIN_VLCC, : VR_DOMAIN_FIVR, : VR_DOMAIN_MAX : } vr_domains; : : /* : * Slew Rate configuration for Deep Package C States for VR domain. : * They are fast time divided by 2. : * 0 - Fast/2 : * 1 - Fast/4 : * 2 - Fast/8 : * 3 - Fast/16 : */ : enum { : SLEW_FAST_2, : SLEW_FAST_4, : SLEW_FAST_8, : SLEW_FAST_16 : } slew_rate; : In addition to defining the enum values here, this also adds two (seemingly unused) fields called vr_domains and slew_rate to `struct soc_intel_tigerlake_config`. They can be defined outside of the structure (you already added FastPkgCRampDisable & SlowSlewRate)
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Eric Herrmann, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45052
to look at the new patch set (#7).
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
soc/intel/tigerlake: Add Acoustic features
Expose the following FSP UPD interface into coreboot, which is the following: AcousticNoiseMitigation FastPkgCRampDisable SlowSlewRateFor AcousticNoiseMitigation is the UPD for Acoustic Noise Mitigation. FastPkgCRampDisable UPD's are to disable Fast Slew Rate for Deep Package C States for VR domains. SlowSlewRate UPD's are the Slew Rate configuration for Deep Package C States for VR domains. Slew rates are fast time divided by 2.
BUG=b:153015585 BRANCH=none TEST= Measure the change in noise level by changing the UPD values.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I1924a3bac8beb16a9d841891696f9a3dea0d425f --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 62 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/45052/7
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45052/5/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/45052/5/src/soc/intel/tigerlake/chi... PS5, Line 153: /* : * VR domains. The domains are IA,GT,SA,VLCC and FIVR. : */ : enum { : VR_DOMAIN_IA, : VR_DOMAIN_GT, : VR_DOMAIN_SA, : VR_DOMAIN_VLCC, : VR_DOMAIN_FIVR, : VR_DOMAIN_MAX : } vr_domains; : : /* : * Slew Rate configuration for Deep Package C States for VR domain. : * They are fast time divided by 2. : * 0 - Fast/2 : * 1 - Fast/4 : * 2 - Fast/8 : * 3 - Fast/16 : */ : enum { : SLEW_FAST_2, : SLEW_FAST_4, : SLEW_FAST_8, : SLEW_FAST_16 : } slew_rate; :
In addition to defining the enum values here, this also adds two (seemingly unused) fields called vr […]
Done
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 8: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 8: Code-Review+2
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45052/8/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45052/8/src/soc/intel/tigerlake/fsp... PS8, Line 260: for (i = 0; i < ARRAY_SIZE(config->SlowSlewRate); i++) Why not do this as a single memcpy using ARRAY_SIZE * sizeof array element?
https://review.coreboot.org/c/coreboot/+/45052/8/src/soc/intel/tigerlake/fsp... PS8, Line 264: for (i = 0; i < ARRAY_SIZE(config->FastPkgCRampDisable); i++) Why not do this as a single memcpy using ARRAY_SIZE * sizeof array element?
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Eric Herrmann, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45052
to look at the new patch set (#9).
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
soc/intel/tigerlake: Add Acoustic features
Expose the following FSP UPD interface into coreboot, which is the following: AcousticNoiseMitigation FastPkgCRampDisable SlowSlewRateFor AcousticNoiseMitigation is the UPD for Acoustic Noise Mitigation. FastPkgCRampDisable UPD's are to disable Fast Slew Rate for Deep Package C States for VR domains. SlowSlewRate UPD's are the Slew Rate configuration for Deep Package C States for VR domains. Slew rates are fast time divided by 2.
BUG=b:153015585 BRANCH=none TEST= Measure the change in noise level by changing the UPD values.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I1924a3bac8beb16a9d841891696f9a3dea0d425f --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 60 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/45052/9
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... PS9, Line 261: * nit: spaces around operators please
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... PS9, Line 261: sizeof(uint8_t) May be better to be more explicit here: sizeof(config->SlowSlewRate[0])
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... PS9, Line 264: * nit: spaces around operators please
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... PS9, Line 264: sizeof(uint8_t) May be better to be more explicit here: sizeof(config->FastPkgCRampDisable[0])
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45052/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45052/9//COMMIT_MSG@11 PS9, Line 11: AcousticNoiseMitigation : FastPkgCRampDisable : SlowSlewRateFor Can this be formatted as a list, e.g., ``` - AcousticNoiseMitigation - FastPkgCRampDisable - SlowSlewRate ```
https://review.coreboot.org/c/coreboot/+/45052/9//COMMIT_MSG@14 PS9, Line 14: AcousticNoiseMitigation is the UPD for Acoustic Noise Mitigation. : FastPkgCRampDisable UPD's are to disable Fast Slew Rate for Deep Package : C States for VR domains. : SlowSlewRate UPD's are the Slew Rate configuration for Deep Package C : States for VR domains. Slew rates are fast time divided by 2. This is already in the comments, I don't think it's required here
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Eric Herrmann, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45052
to look at the new patch set (#10).
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
soc/intel/tigerlake: Add Acoustic features
Expose the following FSP UPD interface into coreboot: - AcousticNoiseMitigation - FastPkgCRampDisable - SlowSlewRateFor
BUG=b:153015585 BRANCH=none TEST= Measure the change in noise level by changing the UPD values.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I1924a3bac8beb16a9d841891696f9a3dea0d425f --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 61 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/45052/10
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 10:
(6 comments)
https://review.coreboot.org/c/coreboot/+/45052/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45052/9//COMMIT_MSG@11 PS9, Line 11: AcousticNoiseMitigation : FastPkgCRampDisable : SlowSlewRateFor
Can this be formatted as a list, e.g., […]
Done
https://review.coreboot.org/c/coreboot/+/45052/9//COMMIT_MSG@14 PS9, Line 14: AcousticNoiseMitigation is the UPD for Acoustic Noise Mitigation. : FastPkgCRampDisable UPD's are to disable Fast Slew Rate for Deep Package : C States for VR domains. : SlowSlewRate UPD's are the Slew Rate configuration for Deep Package C : States for VR domains. Slew rates are fast time divided by 2.
This is already in the comments, I don't think it's required here
Done
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... PS9, Line 261: sizeof(uint8_t)
May be better to be more explicit here: sizeof(config->SlowSlewRate[0])
Done
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... PS9, Line 261: *
nit: spaces around operators please
Done
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... PS9, Line 264: sizeof(uint8_t)
May be better to be more explicit here: sizeof(config->FastPkgCRampDisable[0])
Done
https://review.coreboot.org/c/coreboot/+/45052/9/src/soc/intel/tigerlake/fsp... PS9, Line 264: *
nit: spaces around operators please
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 10: Code-Review+2
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 10: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 10:
Still need to mark comments as resolved 😊
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/chi... PS3, Line 157: IA,GT,SA,VLCC and FIVR
ok.Will Fix.
Done
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/chi... PS3, Line 168: 0 - Fast/2 : * 1 - Fast/4 : * 2 - Fast/8 : * 3 - Fast/16 : */
OK. Will use enums.
Done
https://review.coreboot.org/c/coreboot/+/45052/8/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45052/8/src/soc/intel/tigerlake/fsp... PS8, Line 260: for (i = 0; i < ARRAY_SIZE(config->SlowSlewRate); i++)
Why not do this as a single memcpy using ARRAY_SIZE * sizeof array element?
Done
https://review.coreboot.org/c/coreboot/+/45052/8/src/soc/intel/tigerlake/fsp... PS8, Line 264: for (i = 0; i < ARRAY_SIZE(config->FastPkgCRampDisable); i++)
Why not do this as a single memcpy using ARRAY_SIZE * sizeof array element?
Done
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 10:
Patch Set 10:
Still need to mark comments as resolved 😊
interesting, on the main page its showing me "all comment resolved" but when i went and checked all the comments separately there were unresolved ones. Hopefully all are resolved now. Thanks Tim 😊
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45052/3/src/soc/intel/tigerlake/fsp... PS3, Line 259: for (i = 0; i < ARRAY_SIZE(config->SlowSlewRate); i++)
ok.Will use memcpy.
Ack
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45052 )
Change subject: soc/intel/tigerlake: Add Acoustic features ......................................................................
soc/intel/tigerlake: Add Acoustic features
Expose the following FSP UPD interface into coreboot: - AcousticNoiseMitigation - FastPkgCRampDisable - SlowSlewRateFor
BUG=b:153015585 BRANCH=none TEST= Measure the change in noise level by changing the UPD values.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I1924a3bac8beb16a9d841891696f9a3dea0d425f Reviewed-on: https://review.coreboot.org/c/coreboot/+/45052 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 61 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Shaunak Saha: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h index bcf71b1..fb6cda0 100644 --- a/src/soc/intel/tigerlake/chip.h +++ b/src/soc/intel/tigerlake/chip.h @@ -68,6 +68,33 @@ | LPM_S0i3_0 | LPM_S0i3_1 | LPM_S0i3_2 | LPM_S0i3_3 | LPM_S0i3_4, };
+/* + * VR domains. The domains are IA,GT,SA,VLCC and FIVR. + */ +enum vr_domains { + VR_DOMAIN_IA, + VR_DOMAIN_GT, + VR_DOMAIN_SA, + VR_DOMAIN_VLCC, + VR_DOMAIN_FIVR, + VR_DOMAIN_MAX +}; + +/* + * Slew Rate configuration for Deep Package C States for VR domain. + * They are fast time divided by 2. + * 0 - Fast/2 + * 1 - Fast/4 + * 2 - Fast/8 + * 3 - Fast/16 + */ +enum slew_rate { + SLEW_FAST_2, + SLEW_FAST_4, + SLEW_FAST_8, + SLEW_FAST_16 +}; + struct soc_intel_tigerlake_config {
/* Common struct containing soc config data required by common code */ @@ -143,6 +170,32 @@ /* Wake Enable Bitmap for USB3 ports */ uint16_t usb3_wake_enable_bitmap;
+ /* + * Acoustic Noise Mitigation + * 0 - Disable + * 1 - Enable noise mitigation + */ + uint8_t AcousticNoiseMitigation; + + /* + * Offset 0x054B - Disable Fast Slew Rate for Deep Package + * C States for VR domains. Disable Fast Slew Rate for Deep + * Package C States based on Acoustic Noise Mitigation feature + * enabled. The domains are IA,GT,SA,VLCC and FIVR. + * 0 - False + * 1 - True + */ + uint8_t FastPkgCRampDisable[VR_DOMAIN_MAX]; + + /* + * Offset 0x0550 - Slew Rate configuration for Deep Package + * C States for VR domains. Slew Rate configuration for Deep + * Package C States for VR domains based on Acoustic Noise + * Mitigation feature enabled. The domains are IA,GT,SA,VLCC and FIVR. + * Slew rates are defined as enum slew_rate. + */ + uint8_t SlowSlewRate[VR_DOMAIN_MAX]; + /* SATA related */ uint8_t SataEnable; uint8_t SataMode; diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c index 38f444b..887241b 100644 --- a/src/soc/intel/tigerlake/fsp_params.c +++ b/src/soc/intel/tigerlake/fsp_params.c @@ -256,6 +256,14 @@ } }
+ params->AcousticNoiseMitigation = config->AcousticNoiseMitigation; + memcpy(¶ms->SlowSlewRate, &config->SlowSlewRate, + ARRAY_SIZE(config->SlowSlewRate) * sizeof(config->SlowSlewRate[0])); + + memcpy(¶ms->FastPkgCRampDisable, &config->FastPkgCRampDisable, + ARRAY_SIZE(config->FastPkgCRampDisable) * + sizeof(config->FastPkgCRampDisable[0])); + /* Enable TCPU for processor thermal control */ params->Device4Enable = config->Device4Enable;
-
Duncan Laurie (Code Review) -
Furquan Shaikh (Code Review)
-
Nick Vaccaro (Code Review)
-
Shaunak Saha (Code Review)
-
Tim Wawrzynczak (Code Review)
-
Wonkyu Kim (Code Review)