Mario Scheithauer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35242 )
Change subject: mb/siemens/mc_apl5: Disable IGD if no EDID data available ......................................................................
mb/siemens/mc_apl5: Disable IGD if no EDID data available
To avoid possible panel failures due to incorrect timing settings for PTN3460, the internal graphic device should be disabled.
Change-Id: Ie0b9ed99fb78461bb48d6f2ff328643cd8c2cd15 Signed-off-by: Mario Scheithauer mario.scheithauer@siemens.com --- M src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c 1 file changed, 25 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/35242/1
diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c b/src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c index f6fed97..2cc7dd4 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c @@ -16,9 +16,32 @@ #include <console/console.h> #include <hwilib.h> #include <device/i2c_simple.h> +#include <device/pci_ops.h> +#include <soc/pci_devs.h> #include <types.h> #include <variant/ptn3460.h>
+static void igd_disable(void) +{ + struct device *root_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + uint8_t deven; + uint16_t ggc; + + /* GMCH Graphics Control Register */ + ggc = pci_read_config16(root_dev, 0x50); + /* Set size of Graphics Translation Table Memory (GGMS) [7:6] + * to 0 and select 0 MB for Graphics Memory (GMS) [15:8]. */ + ggc &= ~(0xffc0); + /* Disable IGD VGA (IVD). */ + ggc |= 0x2; + pci_write_config16(root_dev, 0x50, ggc); + /* Device Enable Register */ + deven = pci_read_config8(root_dev, 0x54); + /* Disable IGD device (D2F0EN). */ + deven &= ~(0x10); + pci_write_config8(root_dev, 0x54, deven); +} + /** * This function sets up the DP2LVDS-converter to be used with the appropriate * lcd panel. @@ -42,6 +65,8 @@ /* Get all needed information from hwinfo block. */ if (hwilib_get_field(Edid, edid_data, sizeof(edid_data)) != sizeof(edid_data)) { + /* Disable IGD to avoid panel failures. */ + igd_disable(); printk(BIOS_ERR, "LCD: No EDID data available in %s\n", hwi_block); return 1;
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35242 )
Change subject: mb/siemens/mc_apl5: Disable IGD if no EDID data available ......................................................................
Patch Set 1: Code-Review+1
How would the panel fail if the IGD is left enabled?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35242 )
Change subject: mb/siemens/mc_apl5: Disable IGD if no EDID data available ......................................................................
Patch Set 1:
Maybe you would want to transform the PTN3460 code into a driver. The ptn3460.c file for mc_apl1, mc_apl4 and mc_apl5 only differs in a few comments and some config values. And if mc_apl5 is having issues without this patch, then mc_apl1 and mc_apl4 boards might have them as well.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35242 )
Change subject: mb/siemens/mc_apl5: Disable IGD if no EDID data available ......................................................................
Patch Set 1:
Patch Set 1:
Maybe you would want to transform the PTN3460 code into a driver. The ptn3460.c file for mc_apl1, mc_apl4 and mc_apl5 only differs in a few comments and some config values. And if mc_apl5 is having issues without this patch, then mc_apl1 and mc_apl4 boards might have them as well.
Transferring the PTN code into a driver is planned for a long time already. Unfortunately fsp_baytrail does not have its i2c driver ready to be used as coreboots i2c bus (work is already ongoing) so that PTN code needs to be there at least for mc_tcu3. But we are on the way to sort it out. mc_apl1 and mc_apl4 does not have the same issue as on these boards the LCD panel is fixed to the board and therefore the right EDID data is available at any time.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35242 )
Change subject: mb/siemens/mc_apl5: Disable IGD if no EDID data available ......................................................................
Patch Set 1: Code-Review+2
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35242 )
Change subject: mb/siemens/mc_apl5: Disable IGD if no EDID data available ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Maybe you would want to transform the PTN3460 code into a driver. The ptn3460.c file for mc_apl1, mc_apl4 and mc_apl5 only differs in a few comments and some config values. And if mc_apl5 is having issues without this patch, then mc_apl1 and mc_apl4 boards might have them as well.
Transferring the PTN code into a driver is planned for a long time already. Unfortunately fsp_baytrail does not have its i2c driver ready to be used as coreboots i2c bus (work is already ongoing) so that PTN code needs to be there at least for mc_tcu3. But we are on the way to sort it out. mc_apl1 and mc_apl4 does not have the same issue as on these boards the LCD panel is fixed to the board and therefore the right EDID data is available at any time.
Thanks Werner for the explanation.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35242 )
Change subject: mb/siemens/mc_apl5: Disable IGD if no EDID data available ......................................................................
mb/siemens/mc_apl5: Disable IGD if no EDID data available
To avoid possible panel failures due to incorrect timing settings for PTN3460, the internal graphic device should be disabled.
Change-Id: Ie0b9ed99fb78461bb48d6f2ff328643cd8c2cd15 Signed-off-by: Mario Scheithauer mario.scheithauer@siemens.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35242 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Werner Zeh werner.zeh@siemens.com --- M src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c 1 file changed, 25 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Werner Zeh: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c b/src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c index f6fed97..2cc7dd4 100644 --- a/src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c +++ b/src/mainboard/siemens/mc_apl1/variants/mc_apl5/ptn3460.c @@ -16,9 +16,32 @@ #include <console/console.h> #include <hwilib.h> #include <device/i2c_simple.h> +#include <device/pci_ops.h> +#include <soc/pci_devs.h> #include <types.h> #include <variant/ptn3460.h>
+static void igd_disable(void) +{ + struct device *root_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + uint8_t deven; + uint16_t ggc; + + /* GMCH Graphics Control Register */ + ggc = pci_read_config16(root_dev, 0x50); + /* Set size of Graphics Translation Table Memory (GGMS) [7:6] + * to 0 and select 0 MB for Graphics Memory (GMS) [15:8]. */ + ggc &= ~(0xffc0); + /* Disable IGD VGA (IVD). */ + ggc |= 0x2; + pci_write_config16(root_dev, 0x50, ggc); + /* Device Enable Register */ + deven = pci_read_config8(root_dev, 0x54); + /* Disable IGD device (D2F0EN). */ + deven &= ~(0x10); + pci_write_config8(root_dev, 0x54, deven); +} + /** * This function sets up the DP2LVDS-converter to be used with the appropriate * lcd panel. @@ -42,6 +65,8 @@ /* Get all needed information from hwinfo block. */ if (hwilib_get_field(Edid, edid_data, sizeof(edid_data)) != sizeof(edid_data)) { + /* Disable IGD to avoid panel failures. */ + igd_disable(); printk(BIOS_ERR, "LCD: No EDID data available in %s\n", hwi_block); return 1;
-
Angel Pons (Code Review) -
Mario Scheithauer (Code Review)
-
Patrick Georgi (Code Review)
-
Werner Zeh (Code Review)