Change in ...coreboot[master]: libgfxinit: Add options to limit framebuffer size

[Nico Huber (Code Review)]

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30585

Change subject: libgfxinit: Add options to limit framebuffer size
......................................................................

libgfxinit: Add options to limit framebuffer size

Change-Id: I47fba063629260c3a2854caf7a73f1a1e933d063
Signed-off-by: Nico Huber 
---
M src/device/Kconfig
M src/drivers/intel/gma/hires_fb/gma.adb
2 files changed, 16 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/30585/1

diff --git a/src/device/Kconfig b/src/device/Kconfig
index 85f1d65..f868d2f 100644
--- a/src/device/Kconfig
+++ b/src/device/Kconfig
@@ -445,6 +445,16 @@
    def_bool y
    depends on VBE_LINEAR_FRAMEBUFFER || GENERIC_LINEAR_FRAMEBUFFER

+config LINEAR_FRAMEBUFFER_MAX_WIDTH
+	int "Maximum width in pixels"
+	depends on MAINBOARD_USE_LIBGFXINIT && LINEAR_FRAMEBUFFER
+	default 4096
+
+config LINEAR_FRAMEBUFFER_MAX_HEIGHT
+	int "Maximum height in pixels"
+	depends on MAINBOARD_USE_LIBGFXINIT && LINEAR_FRAMEBUFFER
+	default 4096
+
 endmenu # "Display"

config SMBUS_HAS_AUX_CHANNELS
diff --git a/src/drivers/intel/gma/hires_fb/gma.adb b/src/drivers/intel/gma/hires_fb/gma.adb
index bd14132..1437ce2 100644
--- a/src/drivers/intel/gma/hires_fb/gma.adb
+++ b/src/drivers/intel/gma/hires_fb/gma.adb
@@ -1,3 +1,7 @@
+with CB.Config;
+
+use CB.Config;
+
 with HW.GFX;
 with HW.GFX.Framebuffer_Filler;
 with HW.GFX.GMA;
@@ -60,8 +64,8 @@

success : boolean;

-      min_h : pos32 := pos32'last;
-      min_v : pos32 := pos32'last;
+      min_h : pos32 := LINEAR_FRAMEBUFFER_MAX_WIDTH;
+      min_v : pos32 := LINEAR_FRAMEBUFFER_MAX_HEIGHT;
    begin
       lightup_ok := 0;

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30585 )

Change subject: libgfxinit: Add options to limit framebuffer size
......................................................................

Patch Set 2: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30585 )

Change subject: libgfxinit: Add options to limit framebuffer size
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/30585/2/src/drivers/intel/gma/hires_fb/gma.a... 
File src/drivers/intel/gma/hires_fb/gma.adb:

https://review.coreboot.org/#/c/30585/2/src/drivers/intel/gma/hires_fb/gma.a... 
PS2, Line 67: LINEAR_FRAMEBUFFER_MAX_WIDTH
I think CB.Config.LINEAR_FRAMEBUFFER_MAX_WIDTH is better documentation where the value comes from than use-ing it.

[Nico Huber (Code Review)]

Hello Kyösti Mälkki, Patrick Rudolph, Arthur Heymans, Matt DeVillier, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: libgfxinit: Add options to limit framebuffer size
......................................................................

libgfxinit: Add options to limit framebuffer size

Add maximum width and height options and set the default to 2560x1600
(WQXGA). The framebuffer will be scaled up to the displays' native
resolutions. So this should help with tiny fonts on high-DPI displays.

For laptops, reasonable defaults can be set at the mainboard level.

Change-Id: I47fba063629260c3a2854caf7a73f1a1e933d063
Signed-off-by: Nico Huber 
---
M src/device/Kconfig
M src/drivers/intel/gma/hires_fb/gma.adb
2 files changed, 22 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/30585/3

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30585 )

Change subject: libgfxinit: Add options to limit framebuffer size
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/30585/2/src/drivers/intel/gma/hires_fb/gma.a... 
File src/drivers/intel/gma/hires_fb/gma.adb:

https://review.coreboot.org/#/c/30585/2/src/drivers/intel/gma/hires_fb/gma.a... 
PS2, Line 67: LINEAR_FRAMEBUFFER_MAX_WIDTH
> I think CB.Config. […]

Done. Partially. Still use-ing CB. Might make sense to put the whole
package into CB scope? (another time)

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30585 )

Change subject: libgfxinit: Add options to limit framebuffer size
......................................................................

Patch Set 3: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30585 )

Change subject: libgfxinit: Add options to limit framebuffer size
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/#/c/30585/2/src/drivers/intel/gma/hires_fb/gma.a... 
File src/drivers/intel/gma/hires_fb/gma.adb:

https://review.coreboot.org/#/c/30585/2/src/drivers/intel/gma/hires_fb/gma.a... 
PS2, Line 67: LINEAR_FRAMEBUFFER_MAX_WIDTH
> Done. Partially. Still use-ing CB. Might make sense to put the whole […]

already much better, thanks!

[Patrick Georgi (Code Review)]

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

Change subject: libgfxinit: Add options to limit framebuffer size
......................................................................

libgfxinit: Add options to limit framebuffer size

Add maximum width and height options and set the default to 2560x1600
(WQXGA). The framebuffer will be scaled up to the displays' native
resolutions. So this should help with tiny fonts on high-DPI displays.

For laptops, reasonable defaults can be set at the mainboard level.

Change-Id: I47fba063629260c3a2854caf7a73f1a1e933d063
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/30585
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
Reviewed-by: Patrick Georgi 
---
M src/device/Kconfig
M src/drivers/intel/gma/hires_fb/gma.adb
2 files changed, 22 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/device/Kconfig b/src/device/Kconfig
index 85f1d65..b90b15f 100644
--- a/src/device/Kconfig
+++ b/src/device/Kconfig
@@ -445,6 +445,22 @@
    def_bool y
    depends on VBE_LINEAR_FRAMEBUFFER || GENERIC_LINEAR_FRAMEBUFFER

+config LINEAR_FRAMEBUFFER_MAX_WIDTH
+	int "Maximum width in pixels"
+	depends on LINEAR_FRAMEBUFFER && MAINBOARD_USE_LIBGFXINIT
+	default 2560
+	help
+	  Set the maximum width of the framebuffer. This may help with
+	  default fonts too tiny for high-resolution displays.
+
+config LINEAR_FRAMEBUFFER_MAX_HEIGHT
+	int "Maximum height in pixels"
+	depends on LINEAR_FRAMEBUFFER && MAINBOARD_USE_LIBGFXINIT
+	default 1600
+	help
+	  Set the maximum height of the framebuffer. This may help with
+	  default fonts too tiny for high-resolution displays.
+
 endmenu # "Display"

config SMBUS_HAS_AUX_CHANNELS
diff --git a/src/drivers/intel/gma/hires_fb/gma.adb b/src/drivers/intel/gma/hires_fb/gma.adb
index bd14132..e3553ca 100644
--- a/src/drivers/intel/gma/hires_fb/gma.adb
+++ b/src/drivers/intel/gma/hires_fb/gma.adb
@@ -1,3 +1,7 @@
+with CB.Config;
+
+use CB;
+
 with HW.GFX;
 with HW.GFX.Framebuffer_Filler;
 with HW.GFX.GMA;
@@ -60,8 +64,8 @@

success : boolean;

-      min_h : pos32 := pos32'last;
-      min_v : pos32 := pos32'last;
+      min_h : pos32 := Config.LINEAR_FRAMEBUFFER_MAX_WIDTH;
+      min_v : pos32 := Config.LINEAR_FRAMEBUFFER_MAX_HEIGHT;
    begin
       lightup_ok := 0;