[coreboot-gerrit] [L] Change in coreboot[main]: soc/mediatek: Add common implementation to configure display

[Yidi Lin (Code Review)]

Attention is currently required from: Hung-Te Lin, Paul Menzel, Yu-Ping Wu.

Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79776?usp=email )

Change subject: soc/mediatek: Add common implementation to configure display
......................................................................

Patch Set 3:

(8 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/79776/comment/74cf15d8_a15c9370 :
PS1, Line 13: mtcmos_display_power_on
> nit: `mtcmos_display_power_on()`

Done

https://review.coreboot.org/c/coreboot/+/79776/comment/a8ac3594_3846ba1c :
PS1, Line 28: soc
> nit: `mediatek`

Done

https://review.coreboot.org/c/coreboot/+/79776/comment/a1870b3d_fc062035 :
PS1, Line 29: refacts
> Do you mean `refactors`?

Done

Commit Message:

https://review.coreboot.org/c/coreboot/+/79776/comment/ea2dcaf5_576b9bd7 :
PS2, Line 7: implmentation
> typo

Done

File src/soc/mediatek/common/display.c:

https://review.coreboot.org/c/coreboot/+/79776/comment/6d3536f8_e5736714 :
PS1, Line 13: configure_display
> Consider renaming it to `mtk_display_init`?

Done

https://review.coreboot.org/c/coreboot/+/79776/comment/2cf671d9_a609d393 :
PS1, Line 42: 		if (panel->get_edid && panel->get_edid(panel) < 0)
> I think we just need a local variable to store panel_serializable_data. […]

```
struct panel_serializable_data {
        struct edid edid;  /* edid info of this panel */
        u8 init[]; /* A packed array of panel_init_command */
};
```

Since `init` is always pointing to the address next to `edid`, we can't pass `panel_serializable_data` variable and copy the serializable data from `get_mipi_cmd_from_cbfs`. I pass the pointer to get `buffer` address instead.

File src/soc/mediatek/common/include/soc/ddp_common.h:

https://review.coreboot.org/c/coreboot/+/79776/comment/17bf8740_eaa63fed :
PS1, Line 142: enum disp_path_sel {
> I think this should be put in mediatek/common/.../display.h, and then include display. […]

Done

File src/soc/mediatek/common/include/soc/display.h:

https://review.coreboot.org/c/coreboot/+/79776/comment/baf55bd7_fb98ccbe :
PS1, Line 13: configure_panel_backlight
> configure_backlight

Done