Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31458
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
drivers/intel/gma: Export Read_EDID() to C
Change-Id: Icf802904c569e621ca3b3105b6107936776c5cee Signed-off-by: Nico Huber nico.h@gmx.de --- M src/drivers/intel/gma/Kconfig M src/drivers/intel/gma/Makefile.inc A src/drivers/intel/gma/gma.adb M src/drivers/intel/gma/gma.ads M src/drivers/intel/gma/libgfxinit.h 5 files changed, 67 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/31458/1
diff --git a/src/drivers/intel/gma/Kconfig b/src/drivers/intel/gma/Kconfig index 4f897fa..f8f50c3 100644 --- a/src/drivers/intel/gma/Kconfig +++ b/src/drivers/intel/gma/Kconfig @@ -54,6 +54,9 @@ Select this option for Atom-based platforms which use the SWSMISCI register (0xe0) rather than the SWSCI register (0xe8).
+config INTEL_GMA_LIBGFXINIT_EDID + bool + config GFX_GMA_ANALOG_I2C_HDMI_B bool
@@ -69,7 +72,7 @@ || NORTHBRIDGE_INTEL_NEHALEM || NORTHBRIDGE_INTEL_SANDYBRIDGE \ || NORTHBRIDGE_INTEL_IVYBRIDGE || NORTHBRIDGE_INTEL_HASWELL \ || SOC_INTEL_BROADWELL || SOC_INTEL_SKYLAKE || SOC_INTEL_APOLLOLAKE - depends on MAINBOARD_USE_LIBGFXINIT + depends on MAINBOARD_USE_LIBGFXINIT || INTEL_GMA_LIBGFXINIT_EDID select RAMSTAGE_LIBHWBASE
config GFX_GMA_INTERNAL_IS_EDP diff --git a/src/drivers/intel/gma/Makefile.inc b/src/drivers/intel/gma/Makefile.inc index e128ad6..cea319e 100644 --- a/src/drivers/intel/gma/Makefile.inc +++ b/src/drivers/intel/gma/Makefile.inc @@ -50,7 +50,7 @@
subdirs-y += ../../../../3rdparty/libgfxinit
-ramstage-y += gma.ads +ramstage-y += gma.ads gma.adb
ramstage-$(CONFIG_MAINBOARD_USE_LIBGFXINIT) += gma-gfx_init.ads ifeq ($(CONFIG_LINEAR_FRAMEBUFFER),y) diff --git a/src/drivers/intel/gma/gma.adb b/src/drivers/intel/gma/gma.adb new file mode 100644 index 0000000..10885e6 --- /dev/null +++ b/src/drivers/intel/gma/gma.adb @@ -0,0 +1,37 @@ +with HW.GFX.GMA; +with HW.GFX.GMA.Display_Probing; + +use HW.GFX.GMA; + +package body GMA is + + function read_edid + (raw_edid : out HW.GFX.EDID.Raw_EDID_Data; + port : in Interfaces.C.int) + return Interfaces.C.int + is + use type Interfaces.C.int; + success : Boolean := true; + begin + if port not in Active_Port_Type'Pos (Active_Port_Type'First) + .. Active_Port_Type'Pos (Active_Port_Type'Last) + then + raw_edid := (others => 0); + return -2; + else + if not HW.GFX.GMA.Is_Initialized then + HW.GFX.GMA.Initialize (Success => success); + end if; + if success then + HW.GFX.GMA.Display_Probing.Read_EDID + (raw_edid, Active_Port_Type'Val (port), success); + end if; + if success then + return 0; + else + return -1; + end if; + end if; + end read_edid; + +end GMA; diff --git a/src/drivers/intel/gma/gma.ads b/src/drivers/intel/gma/gma.ads index a6ce3a4..0b4b66b 100644 --- a/src/drivers/intel/gma/gma.ads +++ b/src/drivers/intel/gma/gma.ads @@ -1,2 +1,14 @@ +with Interfaces.C; + +with HW.GFX.EDID; + package GMA is + + function read_edid + (raw_edid : out HW.GFX.EDID.Raw_EDID_Data; + Port : in Interfaces.C.int) + return Interfaces.C.int + with + Export, Convention => C, External_Name => "gma_read_edid"; + end GMA; diff --git a/src/drivers/intel/gma/libgfxinit.h b/src/drivers/intel/gma/libgfxinit.h index c67870e..c4a8a5b 100644 --- a/src/drivers/intel/gma/libgfxinit.h +++ b/src/drivers/intel/gma/libgfxinit.h @@ -14,6 +14,19 @@ #ifndef DRIVERS_INTEL_GMA_LIBGFXINIT_H #define DRIVERS_INTEL_GMA_LIBGFXINIT_H
+enum { + GMA_PORT_DISABLED, + GMA_PORT_INTERNAL, + GMA_PORT_DP1, + GMA_PORT_DP2, + GMA_PORT_DP3, + GMA_PORT_HDMI1, /* or DVI */ + GMA_PORT_HDMI2, /* or DVI */ + GMA_PORT_HDMI3, /* or DVI */ + GMA_PORT_ANALOG, +}; + void gma_gfxinit(int *lightup_ok); +int gma_read_edid(unsigned char edid[], int port);
#endif
Hello Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31458
to look at the new patch set (#2).
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
drivers/intel/gma: Export Read_EDID() to C
Change-Id: Icf802904c569e621ca3b3105b6107936776c5cee Signed-off-by: Nico Huber nico.h@gmx.de --- M src/drivers/intel/gma/Kconfig M src/drivers/intel/gma/Makefile.inc A src/drivers/intel/gma/gma.adb M src/drivers/intel/gma/gma.ads M src/drivers/intel/gma/libgfxinit.h 5 files changed, 67 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/31458/2
Duncan Laurie has uploaded a new patch set (#3) to the change originally created by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/31458 )
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
drivers/intel/gma: Export Read_EDID() to C
Change-Id: Icf802904c569e621ca3b3105b6107936776c5cee Signed-off-by: Nico Huber nico.h@gmx.de --- M src/drivers/intel/gma/Kconfig M src/drivers/intel/gma/Makefile.inc A src/drivers/intel/gma/gma.adb M src/drivers/intel/gma/gma.ads M src/drivers/intel/gma/libgfxinit.h 5 files changed, 67 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/31458/3
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31458 )
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/31458/3/src/drivers/intel/gma/gma.ads File src/drivers/intel/gma/gma.ads:
https://review.coreboot.org/#/c/31458/3/src/drivers/intel/gma/gma.ads@7 PS3, Line 7: read_edid Read_EDID?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31458 )
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... File src/drivers/intel/gma/gma.ads:
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... PS3, Line 7: read_edid
Read_EDID?
Not resolved yet?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31458 )
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... File src/drivers/intel/gma/gma.ads:
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... PS3, Line 7: read_edid
Not resolved yet?
Sorry, the whole series is stalled by the zero review progress on the other side (libgfxinit).
Now that I'm looking at it:
I break with Ada conventions when I write things in lower case, I know. The idea was to try it out here in coreboot how it looks like when we align the style more with our C. Doesn't look very well when it's mixed with existing interfaces in Camel_Case, but when you write some more lines, it starts to work out, imho: Easier to type and to read.
As I'm still the only one writing Ada code in the coreboot repo :'( I didn't think about discussing conventions yet.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31458 )
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... File src/drivers/intel/gma/gma.ads:
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... PS3, Line 7: read_edid
Sorry, the whole series is stalled by the zero review progress on the […]
Ack. IMHO, having different code style conventions for C and Ada helps to tell things apart rather quickly, because of the associations I do:
- 'snake_lower_case' is usually C - 'Snake_Capitalized_Case' is usually Ada - 'CamelCase' is C code that needs fixing/a rewrite
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31458 )
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... File src/drivers/intel/gma/gma.ads:
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... PS3, Line 7: read_edid
Ack. […]
Anything to do here?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31458 )
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
Patch Set 6: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31458 )
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... File src/drivers/intel/gma/gma.ads:
https://review.coreboot.org/c/coreboot/+/31458/3/src/drivers/intel/gma/gma.a... PS3, Line 7: read_edid
Anything to do here?
It seems not.
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31458 )
Change subject: drivers/intel/gma: Export Read_EDID() to C ......................................................................
drivers/intel/gma: Export Read_EDID() to C
Change-Id: Icf802904c569e621ca3b3105b6107936776c5cee Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/31458 Reviewed-by: Patrick Georgi pgeorgi@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/gma/Kconfig M src/drivers/intel/gma/Makefile.inc A src/drivers/intel/gma/gma.adb M src/drivers/intel/gma/gma.ads M src/drivers/intel/gma/libgfxinit.h 5 files changed, 67 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/drivers/intel/gma/Kconfig b/src/drivers/intel/gma/Kconfig index 56c5d43..75d2687 100644 --- a/src/drivers/intel/gma/Kconfig +++ b/src/drivers/intel/gma/Kconfig @@ -54,6 +54,9 @@ Select this option for Atom-based platforms which use the SWSMISCI register (0xe0) rather than the SWSCI register (0xe8).
+config INTEL_GMA_LIBGFXINIT_EDID + bool + config GFX_GMA_ANALOG_I2C_HDMI_B bool
@@ -71,7 +74,7 @@ || SOC_INTEL_BROADWELL || SOC_INTEL_SKYLAKE || SOC_INTEL_APOLLOLAKE \ || SOC_INTEL_KABYLAKE || SOC_INTEL_COFFEELAKE \ || SOC_INTEL_WHISKEYLAKE - depends on MAINBOARD_USE_LIBGFXINIT + depends on MAINBOARD_USE_LIBGFXINIT || INTEL_GMA_LIBGFXINIT_EDID select RAMSTAGE_LIBHWBASE
config GFX_GMA_INTERNAL_IS_EDP diff --git a/src/drivers/intel/gma/Makefile.inc b/src/drivers/intel/gma/Makefile.inc index e128ad6..cea319e 100644 --- a/src/drivers/intel/gma/Makefile.inc +++ b/src/drivers/intel/gma/Makefile.inc @@ -50,7 +50,7 @@
subdirs-y += ../../../../3rdparty/libgfxinit
-ramstage-y += gma.ads +ramstage-y += gma.ads gma.adb
ramstage-$(CONFIG_MAINBOARD_USE_LIBGFXINIT) += gma-gfx_init.ads ifeq ($(CONFIG_LINEAR_FRAMEBUFFER),y) diff --git a/src/drivers/intel/gma/gma.adb b/src/drivers/intel/gma/gma.adb new file mode 100644 index 0000000..10885e6 --- /dev/null +++ b/src/drivers/intel/gma/gma.adb @@ -0,0 +1,37 @@ +with HW.GFX.GMA; +with HW.GFX.GMA.Display_Probing; + +use HW.GFX.GMA; + +package body GMA is + + function read_edid + (raw_edid : out HW.GFX.EDID.Raw_EDID_Data; + port : in Interfaces.C.int) + return Interfaces.C.int + is + use type Interfaces.C.int; + success : Boolean := true; + begin + if port not in Active_Port_Type'Pos (Active_Port_Type'First) + .. Active_Port_Type'Pos (Active_Port_Type'Last) + then + raw_edid := (others => 0); + return -2; + else + if not HW.GFX.GMA.Is_Initialized then + HW.GFX.GMA.Initialize (Success => success); + end if; + if success then + HW.GFX.GMA.Display_Probing.Read_EDID + (raw_edid, Active_Port_Type'Val (port), success); + end if; + if success then + return 0; + else + return -1; + end if; + end if; + end read_edid; + +end GMA; diff --git a/src/drivers/intel/gma/gma.ads b/src/drivers/intel/gma/gma.ads index a6ce3a4..0b4b66b 100644 --- a/src/drivers/intel/gma/gma.ads +++ b/src/drivers/intel/gma/gma.ads @@ -1,2 +1,14 @@ +with Interfaces.C; + +with HW.GFX.EDID; + package GMA is + + function read_edid + (raw_edid : out HW.GFX.EDID.Raw_EDID_Data; + Port : in Interfaces.C.int) + return Interfaces.C.int + with + Export, Convention => C, External_Name => "gma_read_edid"; + end GMA; diff --git a/src/drivers/intel/gma/libgfxinit.h b/src/drivers/intel/gma/libgfxinit.h index c67870e..c4a8a5b 100644 --- a/src/drivers/intel/gma/libgfxinit.h +++ b/src/drivers/intel/gma/libgfxinit.h @@ -14,6 +14,19 @@ #ifndef DRIVERS_INTEL_GMA_LIBGFXINIT_H #define DRIVERS_INTEL_GMA_LIBGFXINIT_H
+enum { + GMA_PORT_DISABLED, + GMA_PORT_INTERNAL, + GMA_PORT_DP1, + GMA_PORT_DP2, + GMA_PORT_DP3, + GMA_PORT_HDMI1, /* or DVI */ + GMA_PORT_HDMI2, /* or DVI */ + GMA_PORT_HDMI3, /* or DVI */ + GMA_PORT_ANALOG, +}; + void gma_gfxinit(int *lightup_ok); +int gma_read_edid(unsigned char edid[], int port);
#endif
-
Angel Pons (Code Review) -
Duncan Laurie (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)