Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30246 )
Change subject: soc/intel/skl/graphics: Implement panel setup
......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/30246/2/src/soc/intel/skylake/graphics.c
File src/soc/intel/skylake/graphics.c:
https://review.coreboot.org/#/c/30246/2/src/soc/intel/skylake/graphics.c@87
PS2, Line 87: /* Start with a 50% duty cycle. */
> Add this to the commit message?
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/30246
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I762a77c8df023a4c14af502af5edfeeb961da1ae
Gerrit-Change-Number: 30246
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jeremy Soller <jackpot51(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 06 Jun 2019 15:35:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Hello Patrick Rudolph, Arthur Heymans, Jeremy Soller, Matt DeVillier, Paul Menzel, Thomas Heijligen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30246
to look at the new patch set (#3).
Change subject: soc/intel/skl/graphics: Implement panel setup
......................................................................
soc/intel/skl/graphics: Implement panel setup
Logs from Linux' i915 suggest that not even the FSP/GOP takes proper
care of this. The sequence is mostly the same as on older platforms,
with a slightly different configuration of the backlight PWM.
We light the panel up with 50% PWM duty cycle. This often results in
an already rather high perceived brightness, but shouldn't be too
blinding.
Change-Id: I762a77c8df023a4c14af502af5edfeeb961da1ae
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M src/soc/intel/skylake/chip.h
M src/soc/intel/skylake/graphics.c
2 files changed, 76 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/30246/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/30246
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I762a77c8df023a4c14af502af5edfeeb961da1ae
Gerrit-Change-Number: 30246
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jeremy Soller <jackpot51(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30246 )
Change subject: soc/intel/skl/graphics: Implement panel setup
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/30246/2/src/soc/intel/skylake/graphics.c
File src/soc/intel/skylake/graphics.c:
https://review.coreboot.org/#/c/30246/2/src/soc/intel/skylake/graphics.c@87
PS2, Line 87: /* Start with a 50% duty cycle. */
Add this to the commit message?
--
To view, visit https://review.coreboot.org/c/coreboot/+/30246
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I762a77c8df023a4c14af502af5edfeeb961da1ae
Gerrit-Change-Number: 30246
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jeremy Soller <jackpot51(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 06 Jun 2019 15:24:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Hello Patrick Rudolph, Paul Menzel, Patrick Georgi, Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/33244
to review the following change.
Change subject: Revert "libpayload: Reset PS/2 keyboard"
......................................................................
Revert "libpayload: Reset PS/2 keyboard"
Documentation is scarce on the matter, however the related coreboot
code suggests that after the ACK, the keyboard also sends the result
of the self test (passed/failed). It looks like this result is never
consumed here, probably resulting in further confusion for later com-
mands.
Let's revert this for now (if it's not too late for the 4.10 release)
and break things later again. IMHO, due to the fact that there are
dozens of different keyboard controller and keyboard implementations
and no accurate specification followed, such changes should be tested
on a lot of hardware before merge.
This reverts commit a99ed13e3397bc536012120aab8cadb827913863.
This reverts commit 7ae606f57f0b3d450ae748141b0e2367041b27d3.
Change-Id: I4d4304d5d8a01e013feac61016c59bcaeea81140
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M payloads/libpayload/drivers/i8042/i8042.h
M payloads/libpayload/drivers/i8042/keyboard.c
2 files changed, 5 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/33244/1
diff --git a/payloads/libpayload/drivers/i8042/i8042.h b/payloads/libpayload/drivers/i8042/i8042.h
index e864ac9..643167e 100644
--- a/payloads/libpayload/drivers/i8042/i8042.h
+++ b/payloads/libpayload/drivers/i8042/i8042.h
@@ -63,7 +63,6 @@
#define I8042_KBCMD_EN 0xf4
#define I8042_KBCMD_DEFAULT_DIS 0xf5
#define I8042_KBCMD_SET_DEFAULT 0xf6
-#define I8042_KBCMD_ACK 0xfa
#define I8042_KBCMD_RESEND 0xfe
#define I8042_KBCMD_RESET 0xff
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c
index 240385c..3e5f988 100644
--- a/payloads/libpayload/drivers/i8042/keyboard.c
+++ b/payloads/libpayload/drivers/i8042/keyboard.c
@@ -172,7 +172,7 @@
{
i8042_write_data(cmd);
- return i8042_wait_read_ps2() == I8042_KBCMD_ACK;
+ return i8042_wait_read_ps2() == 0xfa;
}
int keyboard_havechar(void)
@@ -317,42 +317,27 @@
/* Enable first PS/2 port */
i8042_cmd(I8042_CMD_EN_KB);
- /* Reset keyboard and self test (keyboard side) */
- ret = keyboard_cmd(I8042_KBCMD_RESET);
- if (!ret) {
- printf("ERROR: Keyboard reset failed!\n");
- return;
- }
-
/* Set scancode set 1 */
ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE);
- if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) {
- printf("ERROR: Keyboard set scancode failed!\n");
+ if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE))
return;
- }
ret = keyboard_cmd(I8042_SCANCODE_SET_1);
- if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) {
- printf("ERROR: Keyboard scancode set#1 failed!\n");
+ if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE))
return;
- }
/*
* Set default parameters.
* Fix for broken QEMU ps/2 make scancodes.
*/
ret = keyboard_cmd(0xf6);
- if (!ret) {
- printf("ERROR: Keyboard set default params failed!\n");
+ if (!ret)
return;
- }
/* Enable scanning */
ret = keyboard_cmd(I8042_KBCMD_EN);
- if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) {
- printf("ERROR: Keyboard enable scanning failed!\n");
+ if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE))
return;
- }
console_add_input_driver(&cons);
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/33244
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4d4304d5d8a01e013feac61016c59bcaeea81140
Gerrit-Change-Number: 33244
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newchange
Hello Patrick Rudolph, Arthur Heymans, Jeremy Soller, Matt DeVillier, Paul Menzel, Thomas Heijligen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30246
to look at the new patch set (#2).
Change subject: soc/intel/skl/graphics: Implement panel setup
......................................................................
soc/intel/skl/graphics: Implement panel setup
Logs from Linux' i915 suggest that not even the FSP/GOP takes proper
care of this. The sequence is mostly the same as on older platforms,
with a slightly different configuration of the backlight PWM.
Change-Id: I762a77c8df023a4c14af502af5edfeeb961da1ae
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M src/soc/intel/skylake/chip.h
M src/soc/intel/skylake/graphics.c
2 files changed, 76 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/30246/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/30246
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I762a77c8df023a4c14af502af5edfeeb961da1ae
Gerrit-Change-Number: 30246
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jeremy Soller <jackpot51(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/29926
to look at the new patch set (#42).
Change subject: [Test] Remaining unused variables
......................................................................
[Test] Remaining unused variables
Change-Id: Ic3dde296ab4b84add8a8ab6a3bb5ecb65da2c158
Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr>
---
M Makefile.inc
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/29926/42
--
To view, visit https://review.coreboot.org/c/coreboot/+/29926
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic3dde296ab4b84add8a8ab6a3bb5ecb65da2c158
Gerrit-Change-Number: 29926
Gerrit-PatchSet: 42
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/29926
to look at the new patch set (#41).
Change subject: [Test] Remaining unused variables
......................................................................
[Test] Remaining unused variables
Change-Id: Ic3dde296ab4b84add8a8ab6a3bb5ecb65da2c158
Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr>
---
M Makefile.inc
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/29926/41
--
To view, visit https://review.coreboot.org/c/coreboot/+/29926
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic3dde296ab4b84add8a8ab6a3bb5ecb65da2c158
Gerrit-Change-Number: 29926
Gerrit-PatchSet: 41
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset