[M] Change in coreboot[main]: mb/google/rauru: Add edp driver in mainboard

[Bincai Liu (Code Review)]

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu.

Bincai Liu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85951?usp=email )

Change subject: mb/google/rauru: Add edp driver in mainboard
......................................................................

Patch Set 4:

(10 comments)

File src/mainboard/google/rauru/chromeos.c:

https://review.coreboot.org/c/coreboot/+/85951/comment/f6ac00fd_c70e0692?usp... :
PS2, Line 60: struct lb_gpio edp_pwm_gpios[] = {
            : 		{GPIO_BL_PWM_1V8.id, ACTIVE_HIGH, -1, "PWM control"},
            : 	};
            : 	lb_add_gpios(gpios, edp_pwm_gpios, ARRAY_SIZE(edp_pwm_gpios));
            : 	struct lb_gpio backlight_gpios[] = {
            : 		{GPIO_AP_EDP_BKLTEN.id, ACTIVE_HIGH, -1, "backlight enable"},
            : 	};
> Use a single array.

Done

File src/mainboard/google/rauru/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/85951/comment/479e64eb_5ef08970?usp... :
PS2, Line 12: Panel
> PANEL

Done

File src/mainboard/google/rauru/gpio.h:

https://review.coreboot.org/c/coreboot/+/85951/comment/20a660af_10a3120a?usp... :
PS2, Line 18: #define GPIO_BL_PWM_1V8		GPIO(DISP_PWM)
> leave one blank line on above

Done

File src/mainboard/google/rauru/panel.c:

https://review.coreboot.org/c/coreboot/+/85951/comment/7c59444f_704286dc?usp... :
PS2, Line 3: #include <boardid.h>
           : #include <cbfs.h>
           : #include <delay.h>
           : #include <fw_config.h>
           : #include <gpio.h>
           : #include <soc/ddp.h>
           : #include <soc/dsi.h>
           : #include <soc/gpio_common.h>
           : #include <soc/mtcmos.h>
> remove unused headers

Done

https://review.coreboot.org/c/coreboot/+/85951/comment/dccbff0f_487a3474?usp... :
PS2, Line 39: Panel
> PANEL

Done

File src/mainboard/google/rauru/panel_navi.c:

PS2:
> Rename to panel_oled.c, and rename `get_navi_description` to `get_oled_description`. […]

i take a look of previous project, it seems that they also name panel_xxx.c, xxx is project name.

https://review.coreboot.org/c/coreboot/+/85951/comment/20b13609_3c54abff?usp... :
PS2, Line 3: #include <boardid.h>
           : #include <cbfs.h>
           : #include <delay.h>
           : #include <fw_config.h>
           : #include <gpio.h>
           : #include <soc/ddp.h>
           : #include <soc/dsi.h>
           : #include <soc/gpio_common.h>
           : #include <soc/mtcmos.h>
> please remove unused headers.

Done

https://review.coreboot.org/c/coreboot/+/85951/comment/26ce4a7f_193838c4?usp... :
PS2, Line 15: struct panel_description *get_navi_description(void);
> Move to panel.h.

it seems that we need create a new header panel.h and then include this panel.h in panel.c?

https://review.coreboot.org/c/coreboot/+/85951/comment/cd70c272_ad31ba9e?usp... :
PS2, Line 24: 	mdelay(400);
> Is this delay required if we enable the backlight in the payload (depthcharge) ?

i think this delay is till need

https://review.coreboot.org/c/coreboot/+/85951/comment/3c0cc929_94a511e1?usp... :
PS2, Line 25: , 1);
            : 	gpio_output(GPIO_BL_PWM_1V8, 1);
> I think we should pull down these two pins here and pull them up in the payload.

do you mean that we can enable backlight in payload?