Change in ...coreboot[master]: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters"

List overview All Threads
Download

newer

older

Change in ...coreboot[master]:...

Puthikorn Voravootivat (Code Review)

27 Feb 2019 27 Feb '19

11:02 p.m.

Hello Aaron Durbin, Todd Broch, Sumeet R Pawnikar, build bot (Jenkins), Martin Roth, Furquan Shaikh,

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

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

to review the following change.

Change subject: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters" ......................................................................

Revert "mb/google/poppy/variants/atlas: Update DPTF parameters"

This reverts commit 5e90ef8c356099e42612bc97976c67092d0810ff.

Reason for revert: The 1s internal causes early throttle in usage spike. (log in b/123895423#comment3)

BUG=b:113101335 BRANCH=None TEST=learning from Nocturne

Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 --- M src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/31655/1

diff --git a/src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl b/src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl index 85eb891..685f4dd 100644 --- a/src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl @@ -50,7 +50,7 @@

Name (DTRT, Package () { /* CPU Throttle Effect on CPU */ - Package () { _SB.PCI0.B0D4, _SB.PCI0.B0D4, 100, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.B0D4, _SB.PCI0.B0D4, 100, 50, 0, 0, 0, 0 },

/* CPU Throttle Effect on TSR0 */ Package () { _SB.PCI0.B0D4, _SB.DPTF.TSR0, 100, 600, 0, 0, 0, 0 },

-- To view, visit https://review.coreboot.org/c/coreboot/+/31655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Gerrit-Change-Number: 31655 Gerrit-PatchSet: 1 Gerrit-Owner: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Gerrit-Reviewer: Todd Broch tbroch@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newchange

Attachments:

attachment.htm (text/html — 3.5 KB)

Show replies by date

Puthikorn Voravootivat (Code Review)

27 Feb 27 Feb

11:06 p.m.

Hello Aaron Durbin, Todd Broch, Sumeet R Pawnikar, Caveh Jalali, build bot (Jenkins), Martin Roth, Furquan Shaikh,

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

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

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

Change subject: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters" ......................................................................

Revert "mb/google/poppy/variants/atlas: Update DPTF parameters"

This reverts commit 5e90ef8c356099e42612bc97976c67092d0810ff.

Reason for revert: The 1s interval causes early throttle in usage spike. (log in b/123895423#comment3)

BUG=b:113101335 BRANCH=None TEST=learning from Nocturne

Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 --- M src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/31655/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/31655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Gerrit-Change-Number: 31655 Gerrit-PatchSet: 3 Gerrit-Owner: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Caveh Jalali caveh@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Gerrit-Reviewer: Todd Broch tbroch@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Sumeet R Pawnikar (Code Review)

28 Feb 28 Feb

9:33 a.m.

Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31655 )

Change subject: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters" ......................................................................

Patch Set 3:

Please, add me in bug 123895423, would like to understand #comment3 before commenting here. Thanks. The original change was to overcome messages like 'CPU0: Package temperature above threshold' in syslog during any heavy workload/activity.

-- To view, visit https://review.coreboot.org/c/coreboot/+/31655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Gerrit-Change-Number: 31655 Gerrit-PatchSet: 3 Gerrit-Owner: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Caveh Jalali caveh@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Gerrit-Reviewer: Todd Broch tbroch@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 28 Feb 2019 08:33:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Puthikorn Voravootivat (Code Review)

7:55 p.m.

Puthikorn Voravootivat has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31655 )

Change subject: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters" ......................................................................

Patch Set 3:

@Sumeet

It's effectively do the same thing as CL:29044

-- To view, visit https://review.coreboot.org/c/coreboot/+/31655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Gerrit-Change-Number: 31655 Gerrit-PatchSet: 3 Gerrit-Owner: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Caveh Jalali caveh@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Gerrit-Reviewer: Todd Broch tbroch@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 Feb 2019 18:55:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Puthikorn Voravootivat (Code Review)

7:55 p.m.

Puthikorn Voravootivat has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31655 )

Change subject: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters" ......................................................................

Patch Set 3:

...

Patch Set 3:

@Sumeet

It's effectively do the same thing as CL:29044

Wrong link. https://review.coreboot.org/c/coreboot/+/29044

-- To view, visit https://review.coreboot.org/c/coreboot/+/31655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Gerrit-Change-Number: 31655 Gerrit-PatchSet: 3 Gerrit-Owner: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Caveh Jalali caveh@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Gerrit-Reviewer: Todd Broch tbroch@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 Feb 2019 18:55:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Puthikorn Voravootivat (Code Review)

1 Mar 1 Mar

12:29 a.m.

Hello Aaron Durbin, Sumeet R Pawnikar, Todd Broch, Caveh Jalali, build bot (Jenkins), Martin Roth, Furquan Shaikh,

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

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

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

Change subject: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters" ......................................................................

Revert "mb/google/poppy/variants/atlas: Update DPTF parameters"

This reverts commit 5e90ef8c356099e42612bc97976c67092d0810ff.

Reason for revert: The 1s interval causes early throttle in usage spike. (log in b/123895423#comment3)

BUG=b:113101335 BRANCH=None TEST=learning from Nocturne

Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Signed-off-by: Puthikorn Voravootivat puthik@chromium.org --- M src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/31655/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/31655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Gerrit-Change-Number: 31655 Gerrit-PatchSet: 5 Gerrit-Owner: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Caveh Jalali caveh@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Gerrit-Reviewer: Todd Broch tbroch@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Puthikorn Voravootivat (Code Review)

12:30 a.m.

Puthikorn Voravootivat has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31655 )

Change subject: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters" ......................................................................

Patch Set 5:

Patch #4 / #5 is just rebase to master and add signed-off to make build bot happy

-- To view, visit https://review.coreboot.org/c/coreboot/+/31655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Gerrit-Change-Number: 31655 Gerrit-PatchSet: 5 Gerrit-Owner: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Caveh Jalali caveh@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Gerrit-Reviewer: Todd Broch tbroch@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 Feb 2019 23:30:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Sumeet R Pawnikar (Code Review)

4:42 p.m.

Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31655 )

Change subject: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters" ......................................................................

Patch Set 5: Code-Review+2

I have checked bug comments. This CL looks fine for me.

-- To view, visit https://review.coreboot.org/c/coreboot/+/31655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Gerrit-Change-Number: 31655 Gerrit-PatchSet: 5 Gerrit-Owner: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Caveh Jalali caveh@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Gerrit-Reviewer: Todd Broch tbroch@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 01 Mar 2019 15:42:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

4 Mar 4 Mar

2:23 p.m.

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31655 )

Change subject: Revert "mb/google/poppy/variants/atlas: Update DPTF parameters" ......................................................................

Revert "mb/google/poppy/variants/atlas: Update DPTF parameters"

This reverts commit 5e90ef8c356099e42612bc97976c67092d0810ff.

Reason for revert: The 1s interval causes early throttle in usage spike. (log in b/123895423#comment3)

BUG=b:113101335 BRANCH=None TEST=learning from Nocturne

Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Signed-off-by: Puthikorn Voravootivat puthik@chromium.org Reviewed-on: https://review.coreboot.org/c/31655 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl 1 file changed, 1 insertion(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Sumeet R Pawnikar: Looks good to me, approved

diff --git a/src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl b/src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl index 44aed8b..751a537 100644 --- a/src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl +++ b/src/mainboard/google/poppy/variants/atlas/include/variant/acpi/dptf.asl @@ -50,7 +50,7 @@

Name (DTRT, Package () { /* CPU Throttle Effect on CPU */ - Package () { _SB.PCI0.B0D4, _SB.PCI0.B0D4, 100, 10, 0, 0, 0, 0 }, + Package () { _SB.PCI0.B0D4, _SB.PCI0.B0D4, 100, 50, 0, 0, 0, 0 },

/* CPU Throttle Effect on Ambient */ Package () { _SB.PCI0.B0D4, _SB.DPTF.TSR0, 100, 600, 0, 0, 0, 0 },

-- To view, visit https://review.coreboot.org/c/coreboot/+/31655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6467b51eb937b89b4c08641f36266544c8fa176 Gerrit-Change-Number: 31655 Gerrit-PatchSet: 6 Gerrit-Owner: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Caveh Jalali caveh@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Puthikorn Voravootivat puthik@chromium.org Gerrit-Reviewer: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Gerrit-Reviewer: Todd Broch tbroch@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

2091

days inactive

2096

days old

coreboot-gerrit@coreboot.org

8 comments

3 participants

tags (0)

participants (3)

Patrick Georgi (Code Review)
Puthikorn Voravootivat (Code Review)
Sumeet R Pawnikar (Code Review)