Change in ...coreboot[master]: src/device/Kconfig: Add Kconfig options to choose a default VESA mode
Hello Mike Banon,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31324
to review the following change.
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
src/device/Kconfig: Add Kconfig options to choose a default VESA mode
Introduce the FRAMEBUFFER_VESA_DEFAULT_*** options which could be used to choose a default VESA mode at motherboard's Kconfig. If FRAMEBUFFER_VESA_DEFAULT_CUSTOM if false because no custom default has been selected, choose FRAMEBUFFER_VESA_MODE_117.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I4bf44002d67351195c448a2a7479daede7fcb0ef --- M src/device/Kconfig 1 file changed, 104 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/31324/1
diff --git a/src/device/Kconfig b/src/device/Kconfig index b90b15f..25301c8 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -263,9 +263,112 @@
if FRAMEBUFFER_SET_VESA_MODE
+config FRAMEBUFFER_VESA_DEFAULT_CUSTOM + bool +config FRAMEBUFFER_VESA_DEFAULT_100 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_101 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_102 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_103 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_104 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_105 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_106 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_107 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_108 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_109 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_10A + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_10B + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_10C + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_10D + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_10E + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_10F + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_110 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_111 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_112 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_113 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_114 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_115 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_116 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_117 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM +config FRAMEBUFFER_VESA_DEFAULT_118 + bool + select FRAMEBUFFER_VESA_DEFAULT_CUSTOM + choice prompt "framebuffer graphics resolution" - default FRAMEBUFFER_VESA_MODE_117 + default FRAMEBUFFER_VESA_MODE_100 if FRAMEBUFFER_VESA_DEFAULT_100 + default FRAMEBUFFER_VESA_MODE_101 if FRAMEBUFFER_VESA_DEFAULT_101 + default FRAMEBUFFER_VESA_MODE_102 if FRAMEBUFFER_VESA_DEFAULT_102 + default FRAMEBUFFER_VESA_MODE_103 if FRAMEBUFFER_VESA_DEFAULT_103 + default FRAMEBUFFER_VESA_MODE_104 if FRAMEBUFFER_VESA_DEFAULT_104 + default FRAMEBUFFER_VESA_MODE_105 if FRAMEBUFFER_VESA_DEFAULT_105 + default FRAMEBUFFER_VESA_MODE_106 if FRAMEBUFFER_VESA_DEFAULT_106 + default FRAMEBUFFER_VESA_MODE_107 if FRAMEBUFFER_VESA_DEFAULT_107 + default FRAMEBUFFER_VESA_MODE_108 if FRAMEBUFFER_VESA_DEFAULT_108 + default FRAMEBUFFER_VESA_MODE_109 if FRAMEBUFFER_VESA_DEFAULT_109 + default FRAMEBUFFER_VESA_MODE_10A if FRAMEBUFFER_VESA_DEFAULT_10A + default FRAMEBUFFER_VESA_MODE_10B if FRAMEBUFFER_VESA_DEFAULT_10B + default FRAMEBUFFER_VESA_MODE_10C if FRAMEBUFFER_VESA_DEFAULT_10C + default FRAMEBUFFER_VESA_MODE_10D if FRAMEBUFFER_VESA_DEFAULT_10D + default FRAMEBUFFER_VESA_MODE_10E if FRAMEBUFFER_VESA_DEFAULT_10E + default FRAMEBUFFER_VESA_MODE_10F if FRAMEBUFFER_VESA_DEFAULT_10F + default FRAMEBUFFER_VESA_MODE_110 if FRAMEBUFFER_VESA_DEFAULT_110 + default FRAMEBUFFER_VESA_MODE_111 if FRAMEBUFFER_VESA_DEFAULT_111 + default FRAMEBUFFER_VESA_MODE_112 if FRAMEBUFFER_VESA_DEFAULT_112 + default FRAMEBUFFER_VESA_MODE_113 if FRAMEBUFFER_VESA_DEFAULT_113 + default FRAMEBUFFER_VESA_MODE_114 if FRAMEBUFFER_VESA_DEFAULT_114 + default FRAMEBUFFER_VESA_MODE_115 if FRAMEBUFFER_VESA_DEFAULT_115 + default FRAMEBUFFER_VESA_MODE_116 if FRAMEBUFFER_VESA_DEFAULT_116 + default FRAMEBUFFER_VESA_MODE_117 if FRAMEBUFFER_VESA_DEFAULT_117 + default FRAMEBUFFER_VESA_MODE_118 if FRAMEBUFFER_VESA_DEFAULT_118 + default FRAMEBUFFER_VESA_MODE_117 if !FRAMEBUFFER_VESA_DEFAULT_CUSTOM help This option sets the resolution used for the coreboot framebuffer (and bootsplash screen).
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31324 )
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31324/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31324/1//COMMIT_MSG@12 PS1, Line 12: FRAMEBUFFER_VESA_MODE_117 Please add what resolution and so on hat is.
Hello Mike Banon, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31324
to look at the new patch set (#2).
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
src/device/Kconfig: Add Kconfig options to choose a default VESA mode
Introduce the FRAMEBUFFER_VESA_DEFAULT_*** options which could be used to choose a default VESA mode at motherboard's Kconfig. If FRAMEBUFFER_VESA_DEFAULT_CUSTOM if false because no custom default has been selected, choose FRAMEBUFFER_VESA_MODE_117 - 1024x768 resolution 64k-color (5:6:5) mode.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I4bf44002d67351195c448a2a7479daede7fcb0ef --- M src/device/Kconfig 1 file changed, 104 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/31324/2
mikeb mikeb has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31324 )
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31324/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31324/1//COMMIT_MSG@12 PS1, Line 12: FRAMEBUFFER_VESA_MODE_117
Please add what resolution and so on hat is.
Thank you for your interest! Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31324 )
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/31324/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31324/3//COMMIT_MSG@11 PS3, Line 11: if is
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31324 )
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
Patch Set 3:
Why add 100 lines to fix one board if we can fix all boards without adding any?
Hello Paul Menzel, Mike Banon, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31324
to look at the new patch set (#4).
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
src/device/Kconfig: Add Kconfig options to choose a default VESA mode
Introduce the FRAMEBUFFER_VESA_DEFAULT_*** options which could be used to choose a default VESA mode at motherboard's Kconfig. If FRAMEBUFFER_VESA_DEFAULT_CUSTOM is false because no custom default has been selected, choose FRAMEBUFFER_VESA_MODE_117 - 1024x768 resolution 64k-color (5:6:5) mode.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I4bf44002d67351195c448a2a7479daede7fcb0ef --- M src/device/Kconfig 1 file changed, 104 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/31324/4
mikeb mikeb has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31324 )
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
Patch Set 4:
Patch Set 3:
Why add 100 lines to fix one board if we can fix all boards without adding any?
Sorry for this, I was not sure that all the coreboot-supported boards (even the old ones with ancient hardware) would be working fine at 1024x768 but 16.8M-color mode. But if there is confidence, please let me know and I would make a new change where the default VESA mode would be switched from 117h to 118h for all the boards.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31324 )
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
Patch Set 4:
Why add 100 lines to fix one board if we can fix all boards without adding any?
Sorry for this, I was not sure that all the coreboot-supported boards (even the old ones with ancient hardware) would be working fine at 1024x768 but 16.8M-color mode. But if there is confidence, please let me know and I would make a new change where the default VESA mode would be switched from 117h to 118h for all the boards.
I think it's worth the risk. a) it's only a default anyway, b) there aren't any ancient boards left in the tree and c) there seem to be few people left that run option roms in coreboot (maybe there'll be more again if AMD keeps their proprietary BIOS forever strategy).
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31324 )
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
Patch Set 4:
Superseded by setting 118h a global default in commit 749fe1e ?
mikeb mikeb has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31324 )
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
Patch Set 4:
Patch Set 4:
Superseded by setting 118h a global default in commit 749fe1e ?
Thank you, yes it could be abandoned now. If needed in the future, it will be there.
mikeb mikeb has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31324 )
Change subject: src/device/Kconfig: Add Kconfig options to choose a default VESA mode ......................................................................
Abandoned
Superseeded by CB:31595
-
Kyösti Mälkki (Code Review) -
mikeb mikeb (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)