Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46578 )
Change subject: mb/google/asurada: Initialize display ......................................................................
Patch Set 17:
(15 comments)
very close & thanks!
let's try to give the GPIOs correct names from schematics.
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... File src/mainboard/google/asurada/mainboard.c:
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 52: GPIO_MIPIBRDG_INT_ODL GPIO_EDPBRDG_INT_ODL
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 53: GPIO_MIPIBRDG_PWREN GPIO_EDPBRDG_PWREN
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 54: GPIO_MIPIBRDG_RST_L_1V8 GPIO_EDPBRDG_RST_ODL
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 55: GPIO_MIPIBRDG_PP3300_EN GPIO_EN_PP3300_EDP_DX
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 56: GPIO_MIPIBRDG_PP1800_EN GPIO_EN_PP1800_EDPBRDG_DX
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 57: GPIO_MIPIBRDG_PP1000_EN GPIO_EN_PP1000_EDPBRDG
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 58: GPIO_PP3300_PANEL GPIO_EN_PP3300_DISPLAY_DX
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 51: /* GPIO names */ : #define GPIO_MIPIBRDG_INT_ODL GPIO(EINT6) /* 6 */ : #define GPIO_MIPIBRDG_PWREN GPIO(DSI_TE) /* 41 */ : #define GPIO_MIPIBRDG_RST_L_1V8 GPIO(LCM_RST) /* 42 */ : #define GPIO_MIPIBRDG_PP3300_EN GPIO(PERIPHERAL_EN1) /* 127 */ : #define GPIO_MIPIBRDG_PP1800_EN GPIO(PERIPHERAL_EN2) /* 128 */ : #define GPIO_MIPIBRDG_PP1000_EN GPIO(PERIPHERAL_EN3) /* 129 */ : #define GPIO_PP3300_PANEL GPIO(CAM_CLK3) /* 136 */ : #define should be moved to before register_reset_to_bl31 (first function)
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 64: GPIO(KPROW1) Add #defines for mapping this to schematics:
#define GPIO_AP_EDP_BKLTEN GPIO(KPROW1) #define GPIO_BL_PWM_1V8 GPIO(DISP_PWM)
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 82: u8 const u8
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 88: " "%s: ....", __func__);
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 93: " "%s: ....", __func__);
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 97: " "%s: ....", __func__);
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 116: MIPI_DSI_MODE_EOT_PACKET since we're not checking configs, just merge this to previous line.
https://review.coreboot.org/c/coreboot/+/46578/17/src/mainboard/google/asura... PS17, Line 208: if (!configure_display()) : printk(BIOS_ERR, "%s: Failed to init display.\n", : __func__); since now every failure in configure_display will output messages, I think we don't need to repeat here; just do
if (display_init_required()) configure_display();
And move the [printk(BIOS_INFO, "%s: Starting display init.\n", __func__);] to starting of configure_display.