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 16:
(9 comments)
thanks. in short, we really don't need the panel.h design given there will be only one panel (edp) supported. please revise and do only mainboard.c. Thanks!
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... File src/mainboard/google/asurada/mainboard.c:
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... PS16, Line 53: /* Default implementation for boards without panels defined yet. */ : struct panel_description __weak *get_panel_description(int panel_id) : { : printk(BIOS_ERR, "%s: ERROR: No panels defined for board: %s.\n", : __func__, CONFIG_MAINBOARD_PART_NUMBER); : return NULL; : } remove this
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... PS16, Line 70: if (panel->power_on) { : panel->power_on(); : return; : } remove this
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... PS16, Line 84: /* TODO(hungte) Create a dedicated panel_id() in board_id.c */ : int panel_id = sku_id() >> 4; we don't do this
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... PS16, Line 87: struct panel_description *panel = get_panel_description(panel_id); : if (!panel) { : printk(BIOS_ERR, "%s: Panel %d is not supported.\n", : __func__, panel_id); : return NULL; : } : assert(panel->s); no need
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... PS16, Line 95: const struct edid *edid = &panel->s->edid; : const char *name = edid->ascii_string; : if (name[0] == '\0') : name = "unknown name"; : printk(BIOS_INFO, "%s: Found ID %d: '%s %s' %dx%d@%dHz\n", __func__, : panel_id, edid->manufacturer_name, name, edid->mode.ha, : edid->mode.va, edid->mode.refresh); : return panel; merge these to configure_display
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... PS16, Line 201: if (display_init_required()) { : printk(BIOS_INFO, "%s: Starting display init.\n", __func__); : if (!configure_display()) : printk(BIOS_ERR, "%s: Failed to init display.\n", : __func__); : } else { : printk(BIOS_INFO, "%s: Skipped display init.\n", __func__); : } : move this to after bl31, or even the last of init sequence. we want to config emmc/sdcard/usb as early as possible.
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... File src/mainboard/google/asurada/panel_anx7625.c:
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... PS16, Line 15: /* Disable backlight before turning on bridge */ : gpio_output(GPIO(KPROW1), 0); : gpio_output(GPIO(DISP_PWM), 0); : gpio_output(GPIO_PP3300_PANEL, 1); : : /* Turn on bridge */ : gpio_output(GPIO_MIPIBRDG_RST_L_1V8, 0); : gpio_output(GPIO_MIPIBRDG_PP1000_EN, 1); : gpio_output(GPIO_MIPIBRDG_PP1800_EN, 1); : gpio_output(GPIO_MIPIBRDG_PP3300_EN, 1); : mdelay(2); : gpio_output(GPIO_MIPIBRDG_PWREN, 1); : mdelay(10); : gpio_output(GPIO_MIPIBRDG_RST_L_1V8, 1); merge this back to mainboard.c, and revise with configure_pnel_backlight
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... PS16, Line 31: static void dummy_power_on(void) : { : /* The panel has been already powered on when getting panel information : * so we should do nothing here. : */ : } : : static struct panel_serializable_data anx7625_data = { : .orientation = LB_FB_ORIENTATION_NORMAL, : .init = { INIT_END_CMD }, : }; : : static struct panel_description anx7625_panel = { : .s = &anx7625_data, : .power_on = dummy_power_on, : }; no need
https://review.coreboot.org/c/coreboot/+/46578/16/src/mainboard/google/asura... PS16, Line 50: /* To read panel EDID, we have to first power on anx7625. */ : power_on_anx7625(); : : u8 i2c_bus = 3; : mtk_i2c_bus_init(i2c_bus); : : if (anx7625_init(i2c_bus)) { : printk(BIOS_ERR, "Can't init ANX7625 bridge.\n"); : return NULL; : } : struct edid *edid = &anx7625_data.edid; : if (anx7625_dp_get_edid(i2c_bus, edid)) { : printk(BIOS_ERR, "Can't get panel's edid.\n"); : return NULL; : } : if (anx7625_dp_start(i2c_bus, edid) < 0) { : printk(BIOS_ERR, "Can't start display via ANX7625.\n"); : return NULL; : } : move to mainboard.c