Change in coreboot[master]: mb/google/hatch: Add noise mitgation setting for dratini/jinlon

[Chen Wisley (Code Review)]

Hello Wisley Chen,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/38212

to review the following change.

Change subject: mb/google/hatch: Add noise mitgation setting for dratini/jinlon
......................................................................

mb/google/hatch: Add noise mitgation setting for dratini/jinlon

Enable acoustic noise mitgation, the slow slew rates are fast time dived by 8
and disable Fast PKG C State Ramp(IA, GT, SA).

BRANCH=hatch
BUG=b:143501884
TEST=build and verify that noise reduce.

Change-Id: I65f47288a7b1da98296fdba87ab5ca0c3a567aaf
Signed-off-by: Wisley Chen 
---
M src/mainboard/google/hatch/variants/dratini/overridetree.cb
M src/mainboard/google/hatch/variants/jinlon/overridetree.cb
2 files changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/38212/1

diff --git a/src/mainboard/google/hatch/variants/dratini/overridetree.cb b/src/mainboard/google/hatch/variants/dratini/overridetree.cb
index c6a9ed7..3cde046 100644
--- a/src/mainboard/google/hatch/variants/dratini/overridetree.cb
+++ b/src/mainboard/google/hatch/variants/dratini/overridetree.cb
@@ -15,6 +15,15 @@
    	[PchSerialIoIndexUART2] = PchSerialIoDisabled,
    }"

+	# VR Slew rate setting
+	register "AcousticNoiseMitigation" = "1"
+	register "SlowSlewRateForIa" = "2"
+	register "SlowSlewRateForGt" = "2"
+	register "SlowSlewRateForSa" = "2"
+	register "FastPkgCRampDisableIa" = "1"
+	register "FastPkgCRampDisableGt" = "1"
+	register "FastPkgCRampDisableSa" = "1"
+
    # Intel Common SoC Config
    #+-------------------+---------------------------+
    #| Field             |  Value                    |
diff --git a/src/mainboard/google/hatch/variants/jinlon/overridetree.cb b/src/mainboard/google/hatch/variants/jinlon/overridetree.cb
index c9613d2..f3f6c3b 100644
--- a/src/mainboard/google/hatch/variants/jinlon/overridetree.cb
+++ b/src/mainboard/google/hatch/variants/jinlon/overridetree.cb
@@ -17,6 +17,15 @@
    	[PchSerialIoIndexUART2] = PchSerialIoDisabled,
    }"

+	# VR Slew rate setting
+	register "AcousticNoiseMitigation" = "1"
+	register "SlowSlewRateForIa" = "2"
+	register "SlowSlewRateForGt" = "2"
+	register "SlowSlewRateForSa" = "2"
+	register "FastPkgCRampDisableIa" = "1"
+	register "FastPkgCRampDisableGt" = "1"
+	register "FastPkgCRampDisableSa" = "1"
+
    # Intel Common SoC Config
    #+-------------------+---------------------------+
    #| Field             |  Value                    |

[Chen Wisley (Code Review)]

Chen Wisley has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitgation setting for dratini/jinlon
......................................................................

Patch Set 2:

This change is ready for review.

[Frans Hendriks (Code Review)]

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitgation setting for dratini/jinlon
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38212/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38212/2//COMMIT_MSG@7 
PS2, Line 7: mb/google/hatch: Add noise mitgation setting for dratini/jinlon
mitigation

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Chen Wisley (Code Review)]

Hello Paul Fagerburg, Wisley Chen, Philip Chen, Tim Wawrzynczak, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38212

to look at the new patch set (#3).

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

mb/google/hatch: Add noise mitigation setting for dratini/jinlon

Enable acoustic noise mitgation, the slow slew rates are fast time dived by 8
and disable Fast PKG C State Ramp(IA, GT, SA).

BRANCH=hatch
BUG=b:143501884
TEST=build and verify that noise reduce.

Change-Id: I65f47288a7b1da98296fdba87ab5ca0c3a567aaf
Signed-off-by: Wisley Chen 
---
M src/mainboard/google/hatch/variants/dratini/overridetree.cb
M src/mainboard/google/hatch/variants/jinlon/overridetree.cb
2 files changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/38212/3

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Chen Wisley (Code Review)]

Chen Wisley has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38212/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38212/2//COMMIT_MSG@7 
PS2, Line 7: mb/google/hatch: Add noise mitgation setting for dratini/jinlon
> mitigation

Done

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Frans Hendriks (Code Review)]

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 3: Code-Review+1

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 3:

(3 comments)

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@9 
PS3, Line 9: mitgation
mitigation

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@10 
PS3, Line 10: Ramp(IA, GT, SA)
Space before (.

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@14 
PS3, Line 14: TEST=build and verify that noise reduce.
Do you have hard numbers in db?

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Chen Wisley (Code Review)]

Hello Paul Fagerburg, Wisley Chen, Frans Hendriks, Philip Chen, Tim Wawrzynczak, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38212

to look at the new patch set (#4).

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

mb/google/hatch: Add noise mitigation setting for dratini/jinlon

Enable acoustic noise mitigation, the slow slew rates are fast time dived by 8
and disable Fast PKG C State Ramp (IA, GT, SA).

BRANCH=hatch
BUG=b:143501884
TEST=build and verify that noise reduce.

Change-Id: I65f47288a7b1da98296fdba87ab5ca0c3a567aaf
Signed-off-by: Wisley Chen 
---
M src/mainboard/google/hatch/variants/dratini/overridetree.cb
M src/mainboard/google/hatch/variants/jinlon/overridetree.cb
2 files changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/38212/4

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38212/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38212/4//COMMIT_MSG@9 
PS4, Line 9: dived
divided?

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Paul Fagerburg (Code Review)]

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@9 
PS3, Line 9: mitgation
> mitigation

What is the impact of the changes in the slew rates, aside from acoustic noise mitigation? Is there any other discernible change?

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@14 
PS3, Line 14: TEST=build and verify that noise reduce.
> Do you have hard numbers in db?

+1

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Chen Wisley (Code Review)]

Hello Paul Fagerburg, Wisley Chen, Frans Hendriks, Philip Chen, Tim Wawrzynczak, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38212

to look at the new patch set (#5).

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

mb/google/hatch: Add noise mitigation setting for dratini/jinlon

Enable acoustic noise mitigation, the slow slew rates are fast time divided by 8
and disable Fast PKG C State Ramp (IA, GT, SA).

BRANCH=hatch
BUG=b:143501884
TEST=build and verify that noise reduce.

Change-Id: I65f47288a7b1da98296fdba87ab5ca0c3a567aaf
Signed-off-by: Wisley Chen 
---
M src/mainboard/google/hatch/variants/dratini/overridetree.cb
M src/mainboard/google/hatch/variants/jinlon/overridetree.cb
2 files changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/38212/5

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Chen Wisley (Code Review)]

Chen Wisley has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 5:

(3 comments)

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@9 
PS3, Line 9: mitgation
> What is the impact of the changes in the slew rates, aside from acoustic noise mitigation? Is there  […]

The SW option is provided in intel doc 575216 and the same solution is applied on hatch board.

Intel replied on issue tracker:
The acoustic noise mitigation option may impact interrupt response time, device latency to memory, exit latency from C-states, IO configuration, power and performance…etc.

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@10 
PS3, Line 10: Ramp(IA, GT, SA)
> Space before (.

Done

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@14 
PS3, Line 14: TEST=build and verify that noise reduce.
> +1

let me check with our power team

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Chen Wisley (Code Review)]

Chen Wisley has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@14 
PS3, Line 14: TEST=build and verify that noise reduce.
> let me check with our power team

There is test report with HW/SW solution on issue tracker. We also can hear the noise reduce

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 5: Code-Review+1

The information from your answers should be in the commit message.

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 5: Code-Review+2

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 5:

(3 comments)

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@9 
PS3, Line 9: mitgation
> The SW option is provided in intel doc 575216 and the same solution is applied on hatch board. […]

Ack

https://review.coreboot.org/c/coreboot/+/38212/3//COMMIT_MSG@14 
PS3, Line 14: TEST=build and verify that noise reduce.
> There is test report with HW/SW solution on issue tracker. […]

This is the recommended step by Intel. Any higher and the slew rates have much greater impact on the latencies mentioned elsewhere on this issue. Also doing some cap swapping on one of the rails to mitigate noise.

https://review.coreboot.org/c/coreboot/+/38212/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38212/4//COMMIT_MSG@9 
PS4, Line 9: dived
> divided?

Done

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[Shelley Chen (Code Review)]

Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

mb/google/hatch: Add noise mitigation setting for dratini/jinlon

Enable acoustic noise mitigation, the slow slew rates are fast time divided by 8
and disable Fast PKG C State Ramp (IA, GT, SA).

BRANCH=hatch
BUG=b:143501884
TEST=build and verify that noise reduce.

Change-Id: I65f47288a7b1da98296fdba87ab5ca0c3a567aaf
Signed-off-by: Wisley Chen 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38212
Reviewed-by: Paul Menzel 
Reviewed-by: Tim Wawrzynczak 
Reviewed-by: Frans Hendriks 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/google/hatch/variants/dratini/overridetree.cb
M src/mainboard/google/hatch/variants/jinlon/overridetree.cb
2 files changed, 18 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Frans Hendriks: Looks good to me, but someone else must approve
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/mainboard/google/hatch/variants/dratini/overridetree.cb b/src/mainboard/google/hatch/variants/dratini/overridetree.cb
index f820629..5c30a5a 100644
--- a/src/mainboard/google/hatch/variants/dratini/overridetree.cb
+++ b/src/mainboard/google/hatch/variants/dratini/overridetree.cb
@@ -17,6 +17,15 @@
    	[PchSerialIoIndexUART2] = PchSerialIoDisabled,
    }"

+	# VR Slew rate setting
+	register "AcousticNoiseMitigation" = "1"
+	register "SlowSlewRateForIa" = "2"
+	register "SlowSlewRateForGt" = "2"
+	register "SlowSlewRateForSa" = "2"
+	register "FastPkgCRampDisableIa" = "1"
+	register "FastPkgCRampDisableGt" = "1"
+	register "FastPkgCRampDisableSa" = "1"
+
    # Intel Common SoC Config
    #+-------------------+---------------------------+
    #| Field             |  Value                    |
diff --git a/src/mainboard/google/hatch/variants/jinlon/overridetree.cb b/src/mainboard/google/hatch/variants/jinlon/overridetree.cb
index c9613d2..f3f6c3b 100644
--- a/src/mainboard/google/hatch/variants/jinlon/overridetree.cb
+++ b/src/mainboard/google/hatch/variants/jinlon/overridetree.cb
@@ -17,6 +17,15 @@
    	[PchSerialIoIndexUART2] = PchSerialIoDisabled,
    }"

+	# VR Slew rate setting
+	register "AcousticNoiseMitigation" = "1"
+	register "SlowSlewRateForIa" = "2"
+	register "SlowSlewRateForGt" = "2"
+	register "SlowSlewRateForSa" = "2"
+	register "FastPkgCRampDisableIa" = "1"
+	register "FastPkgCRampDisableGt" = "1"
+	register "FastPkgCRampDisableSa" = "1"
+
    # Intel Common SoC Config
    #+-------------------+---------------------------+
    #| Field             |  Value                    |

Change in coreboot[master]: mb/google/hatch: Add noise mitigation setting for dratini/jinlon

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38212 )

Change subject: mb/google/hatch: Add noise mitigation setting for dratini/jinlon
......................................................................

Patch Set 6:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3
Emulation targets:
EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/458
EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/457
EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/456

Please note: This test is under development and might not be accurate at all!