Srinidhi N Kaushik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
drivers/intel/gma: Add Kconfig option for vbt data size
From Tigerlake FSP v3373 onwards vbt binary size changed from 8KiB to 9KiB. This change adds Kconfig option to choose vbt data size based on platform.
BUG=b:171401992 BRANCH=none TEST=build and boot delbin and verify fw screen is loaded
Signed-off-by: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Change-Id: Ia294fc94ce759666fb664dfdb910ecd403e6a2e9 --- M src/drivers/intel/gma/Kconfig M src/drivers/intel/gma/opregion.c 2 files changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/47151/1
diff --git a/src/drivers/intel/gma/Kconfig b/src/drivers/intel/gma/Kconfig index c515888..f4eb000 100644 --- a/src/drivers/intel/gma/Kconfig +++ b/src/drivers/intel/gma/Kconfig @@ -99,6 +99,11 @@ default y if NORTHBRIDGE_INTEL_GM45 || NORTHBRIDGE_INTEL_IRONLAKE default n
+config VBT_DATA_SIZE + int + default 9 if SOC_INTEL_TIGERLAKE + default 8 + if GFX_GMA
config GFX_GMA_DYN_CPU diff --git a/src/drivers/intel/gma/opregion.c b/src/drivers/intel/gma/opregion.c index 8f1d2e6..359d498 100644 --- a/src/drivers/intel/gma/opregion.c +++ b/src/drivers/intel/gma/opregion.c @@ -19,7 +19,7 @@ return "vbt.bin"; }
-static char vbt_data[9 * KiB]; +static char vbt_data[CONFIG_VBT_DATA_SIZE * KiB]; static size_t vbt_data_sz;
void *locate_vbt(size_t *vbt_size)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47151/1/src/drivers/intel/gma/Kconf... File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/47151/1/src/drivers/intel/gma/Kconf... PS1, Line 102: config VBT_DATA_SIZE trailing whitespace
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47151
to look at the new patch set (#2).
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
drivers/intel/gma: Add Kconfig option for vbt data size
From Tigerlake FSP v3373 onwards vbt binary size changed from 8KiB to 9KiB. This change adds Kconfig option to choose vbt data size based on platform.
BUG=b:171401992 BRANCH=none TEST=build and boot delbin and verify fw screen is loaded
Signed-off-by: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Change-Id: Ia294fc94ce759666fb664dfdb910ecd403e6a2e9 --- M src/drivers/intel/gma/Kconfig M src/drivers/intel/gma/opregion.c 2 files changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/47151/2
Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 2: Code-Review+1
Dossym Nurmukhanov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 2: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 2:
(3 comments)
I wonder why we don't use the heap (malloc()) instead. We wouldn't need additional (Kconfig) knowledge that way.
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/Kconf... File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/Kconf... PS2, Line 102: config VBT_DATA_SIZE Please move this somewhere above the `GFX_GMA_*` options.
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/Kconf... PS2, Line 103: int We usually use `hex` and absolute values for sizes, alternatively you can mention the unit in the name, e.g. VBT_DATA_SIZE_KB.
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/opreg... File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/opreg... PS2, Line 22: 9 Please mention in the commit message that this was changed recently. Without knowing that, it looks like you decrease the size for everything but Tiger Lake.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Dossym Nurmukhanov, Raj Astekar, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47151
to look at the new patch set (#3).
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
drivers/intel/gma: Add Kconfig option for vbt data size
From Tigerlake FSP v3373 onwards vbt binary size changed from 8KiB to 9KiB. Commit cf5d58328fe004d967466be42de62d6bab4c3133 had changed the size from 8 to 9 Kib. This change adds Kconfig option to choose vbt data size based on platform.
BUG=b:171401992 BRANCH=none TEST=build and boot delbin and verify fw screen is loaded
Signed-off-by: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Change-Id: Ia294fc94ce759666fb664dfdb910ecd403e6a2e9 --- M src/drivers/intel/gma/Kconfig M src/drivers/intel/gma/opregion.c 2 files changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/47151/3
Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 3: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/Kconf... File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/Kconf... PS2, Line 102: config VBT_DATA_SIZE
Please move this somewhere above the `GFX_GMA_*` options.
Done
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/Kconf... PS2, Line 103: int
We usually use `hex` and absolute values for sizes, alternatively […]
Done
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/opreg... File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/47151/2/src/drivers/intel/gma/opreg... PS2, Line 22: 9
Please mention in the commit message that this was changed recently. […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 3:
Patch Set 2:
(3 comments)
I wonder why we don't use the heap (malloc()) instead. We wouldn't need additional (Kconfig) knowledge that way.
That would work. If we do that, we should probably bump up the heap sizes for all the platforms using this driver.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47151/3/src/drivers/intel/gma/Kconf... File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/47151/3/src/drivers/intel/gma/Kconf... PS3, Line 64: default 9 if SOC_INTEL_TIGERLAKE Wouldn't it be better to do this in soc/intel/tigerlake rather than adding a check here? I suspect that as we go forward, we will see more platforms requiring bigger VBT_DATA_SIZE_KB. In that case, it would be best to have this configuration live within SoC Kconfig rather than changing the driver Kconfig for every platform and adding "if SOC_*".
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Dossym Nurmukhanov, Raj Astekar, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47151
to look at the new patch set (#4).
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
drivers/intel/gma: Add Kconfig option for vbt data size
From Tigerlake FSP v3373 onwards vbt binary size changed from 8KiB to 9KiB. Commit cf5d58328fe004d967466be42de62d6bab4c3133 had changed the size from 8 to 9 Kib. This change adds Kconfig option to choose vbt data size based on platform.
BUG=b:171401992 BRANCH=none TEST=build and boot delbin and verify fw screen is loaded
Signed-off-by: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Change-Id: Ia294fc94ce759666fb664dfdb910ecd403e6a2e9 --- M src/drivers/intel/gma/Kconfig M src/drivers/intel/gma/opregion.c 2 files changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/47151/4
Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/47151/3/src/drivers/intel/gma/Kconf... File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/47151/3/src/drivers/intel/gma/Kconf... PS3, Line 64: default 9 if SOC_INTEL_TIGERLAKE
Wouldn't it be better to do this in soc/intel/tigerlake rather than adding a check here? I suspect t […]
Agree, Done
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 4: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 4: Code-Review+2
Thanks for the updates :)
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 4: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47151/4/src/drivers/intel/gma/Kconf... File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/47151/4/src/drivers/intel/gma/Kconf... PS4, Line 64: 8 Since the change is split into two separate CLs, you will have to ensure that both changes land as a single unit in chromium tree. Else, volteer would be broken if this change lands and not the following.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47151/4/src/drivers/intel/gma/Kconf... File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/c/coreboot/+/47151/4/src/drivers/intel/gma/Kconf... PS4, Line 64: 8
Since the change is split into two separate CLs, you will have to ensure that both changes land as a […]
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47151 )
Change subject: drivers/intel/gma: Add Kconfig option for vbt data size ......................................................................
drivers/intel/gma: Add Kconfig option for vbt data size
From Tigerlake FSP v3373 onwards vbt binary size changed from 8KiB to 9KiB. Commit cf5d58328fe004d967466be42de62d6bab4c3133 had changed the size from 8 to 9 Kib. This change adds Kconfig option to choose vbt data size based on platform.
BUG=b:171401992 BRANCH=none TEST=build and boot delbin and verify fw screen is loaded
Signed-off-by: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Change-Id: Ia294fc94ce759666fb664dfdb910ecd403e6a2e9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/47151 Reviewed-by: Wonkyu Kim wonkyu.kim@intel.com Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/gma/Kconfig M src/drivers/intel/gma/opregion.c 2 files changed, 5 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Furquan Shaikh: Looks good to me, approved Srinidhi N Kaushik: Looks good to me, but someone else must approve Wonkyu Kim: Looks good to me, but someone else must approve
diff --git a/src/drivers/intel/gma/Kconfig b/src/drivers/intel/gma/Kconfig index c515888..3f75ab9 100644 --- a/src/drivers/intel/gma/Kconfig +++ b/src/drivers/intel/gma/Kconfig @@ -59,6 +59,10 @@ config INTEL_GMA_LIBGFXINIT_EDID bool
+config VBT_DATA_SIZE_KB + int + default 8 + config GFX_GMA_ANALOG_I2C_HDMI_B bool
diff --git a/src/drivers/intel/gma/opregion.c b/src/drivers/intel/gma/opregion.c index 8f1d2e6..ea05124 100644 --- a/src/drivers/intel/gma/opregion.c +++ b/src/drivers/intel/gma/opregion.c @@ -19,7 +19,7 @@ return "vbt.bin"; }
-static char vbt_data[9 * KiB]; +static char vbt_data[CONFIG_VBT_DATA_SIZE_KB * KiB]; static size_t vbt_data_sz;
void *locate_vbt(size_t *vbt_size)
-
build bot (Jenkins) (Code Review) -
Dossym Nurmukhanov (Code Review)
-
Furquan Shaikh (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Srinidhi N Kaushik (Code Review)
-
Wonkyu Kim (Code Review)