Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/libgfxinit/+/42364 )
Change subject: gfxtest: Fix build failure with newer toolchain ......................................................................
gfxtest: Fix build failure with newer toolchain
Looks like there's a new warning that requires us to have the same ordering in both a record definition and its memory layout definition. This is not the case on the `Pixel_Type` record in gfxtest, so gfxtest would not build with the GNAT 10.1.0 toolchain because of the warning.
Since we define colors using positional notation, reordering the fields will result in the wrong colors being displayed. Account for that with the `Pixel_RGBA` inline function, which simply reorders the components.
Change-Id: Ic4b6cfd7f933ffaeeac389e45e211483d35fc6ac Signed-off-by: Angel Pons th3fanbus@gmail.com --- M gfxtest/hw-gfx-gma-gfx_test.adb 1 file changed, 11 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/64/42364/1
diff --git a/gfxtest/hw-gfx-gma-gfx_test.adb b/gfxtest/hw-gfx-gma-gfx_test.adb index e5bc393..63ab099 100644 --- a/gfxtest/hw-gfx-gma-gfx_test.adb +++ b/gfxtest/hw-gfx-gma-gfx_test.adb @@ -51,10 +51,11 @@ end loop; end Restore_GTT;
+ -- FIXME: Intel Hardware enforces BGRA byte order. type Pixel_Type is record - Red : Byte; - Green : Byte; Blue : Byte; + Green : Byte; + Red : Byte; Alpha : Byte; end record;
@@ -65,11 +66,14 @@ Alpha at 3 range 0 .. 7; end record;
- White : constant Pixel_Type := (255, 255, 255, 255); - Black : constant Pixel_Type := ( 0, 0, 0, 255); - Red : constant Pixel_Type := (255, 0, 0, 255); - Green : constant Pixel_Type := ( 0, 255, 0, 255); - Blue : constant Pixel_Type := ( 0, 0, 255, 255); + function Pixel_RGBA (R, G, B, A : Byte) return Pixel_Type is + ((Red => R, Green => G, Blue => B, Alpha => A)) with Inline; + + White : constant Pixel_Type := Pixel_RGBA (255, 255, 255, 255); + Black : constant Pixel_Type := Pixel_RGBA ( 0, 0, 0, 255); + Red : constant Pixel_Type := Pixel_RGBA (255, 0, 0, 255); + Green : constant Pixel_Type := Pixel_RGBA ( 0, 255, 0, 255); + Blue : constant Pixel_Type := Pixel_RGBA ( 0, 0, 255, 255);
function Pixel_To_Word (P : Pixel_Type) return Word32 with
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/42364 )
Change subject: gfxtest: Fix build failure with newer toolchain ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
How does it look?
https://review.coreboot.org/c/libgfxinit/+/42364/1/gfxtest/hw-gfx-gma-gfx_te... File gfxtest/hw-gfx-gma-gfx_test.adb:
https://review.coreboot.org/c/libgfxinit/+/42364/1/gfxtest/hw-gfx-gma-gfx_te... PS1, Line 178: when GMA.Tertiary => (Map (Xp, Yp), Map (Xn, Yp), Map (Xn, Yn), 255)); I guess we rotate colors here. no idea how it looks like. I don't mind as long as it works and doesn't cause eye cancer.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/42364 )
Change subject: gfxtest: Fix build failure with newer toolchain ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/42364/1/gfxtest/hw-gfx-gma-gfx_te... File gfxtest/hw-gfx-gma-gfx_test.adb:
https://review.coreboot.org/c/libgfxinit/+/42364/1/gfxtest/hw-gfx-gma-gfx_te... PS1, Line 178: when GMA.Tertiary => (Map (Xp, Yp), Map (Xn, Yp), Map (Xn, Yn), 255));
I guess we rotate colors here. no idea how it looks like. I don't mind as […]
I haven't been able to test
Hello Nico Huber, Matt DeVillier, Arthur Heymans,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/libgfxinit/+/42364
to look at the new patch set (#2).
Change subject: gfxtest: Fix build failure with newer toolchain ......................................................................
gfxtest: Fix build failure with newer toolchain
Looks like there's a new warning that requires us to have the same ordering in both a record definition and its memory layout definition. This is not the case on the `Pixel_Type` record in gfxtest, so gfxtest would not build with the GNAT 10.1.0 toolchain because of the warning.
Since we define colors using positional notation, reordering the fields will result in the wrong colors being displayed. Account for that with the `Pixel_RGBA` inline function, which simply reorders the components.
Tested on Asus P8Z77-V LX2 with two displays, gfxtest is still dangerous for people with epilepsy. It does not look uglier than before, though.
Change-Id: Ic4b6cfd7f933ffaeeac389e45e211483d35fc6ac Signed-off-by: Angel Pons th3fanbus@gmail.com --- M gfxtest/hw-gfx-gma-gfx_test.adb 1 file changed, 11 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/64/42364/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/42364 )
Change subject: gfxtest: Fix build failure with newer toolchain ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/42364/1/gfxtest/hw-gfx-gma-gfx_te... File gfxtest/hw-gfx-gma-gfx_test.adb:
https://review.coreboot.org/c/libgfxinit/+/42364/1/gfxtest/hw-gfx-gma-gfx_te... PS1, Line 178: when GMA.Tertiary => (Map (Xp, Yp), Map (Xn, Yp), Map (Xn, Yn), 255));
I haven't been able to test
I made a separate commit, it doesn't really make a difference IMHO.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/42364 )
Change subject: gfxtest: Fix build failure with newer toolchain ......................................................................
Patch Set 2: Code-Review+1
This indeed fixes the build failure with Debian Sid/unstable and GNAT 10.1.0.
``` COMPILE gfxtest/hw-gfx-gma-gfx_test.o gcc -Wuninitialized -Wall -Werror -pipe -g -Wstrict-aliasing -Wshadow -fno-common -fomit-frame-pointer -ffunction-sections -fdata-sections -gnatA -gnatec=../libhwbase/dest//debug.adc -gnatp -gnatwa.eeD.HHTU.U.W.Y -gnatyN -Ibuild/common/ -Icommon/ -Icommon/dyncpu/ -Icommon/haswell_shared/ -Icommon/skylake/ -Igfxtest/ -I../libhwbase/dest//include -c -o build/gfxtest/hw-gfx-gma-gfx_test.o gfxtest/hw-gfx-gma-gfx_test.adb hw-gfx-gma-gfx_test.adb:55:07: warning: component clause out of order with respect to declaration hw-gfx-gma-gfx_test.adb:62:13: warning: component clause out of order with respect to declaration make: *** [../libhwbase/dest/Makefile:248: build/gfxtest/hw-gfx-gma-gfx_test.o] Fehler 1 ```
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/libgfxinit/+/42364 )
Change subject: gfxtest: Fix build failure with newer toolchain ......................................................................
Abandoned
Prefer CB:43556
-
Angel Pons (Code Review) -
Nico Huber (Code Review)
-
Paul Menzel (Code Review)