[coreboot-gerrit] [L] Change in coreboot[main]: soc/mediatek/mt8196: Add GPUEB support

[Alvin Liu (Code Review)]

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

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

Change subject: soc/mediatek/mt8196: Add GPUEB support
......................................................................

Patch Set 5:

(16 comments)

File src/soc/mediatek/mt8196/Makefile.mk:

https://review.coreboot.org/c/coreboot/+/85654/comment/4bebfb52_a39e8136?usp... :
PS4, Line 81: 	$(CONFIG_GPUEB_FIRMWARE)
> sort

Done

File src/soc/mediatek/mt8196/gpueb.c:

https://review.coreboot.org/c/coreboot/+/85654/comment/43459b80_0a1cbfde?usp... :
PS4, Line 192: Fail
> failed

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/9c1ec38a_be1aac03?usp... :
PS4, Line 196: gpueb_
> remove `gpueb_` prefix

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/1dc52eea_86be8e54?usp... :
PS4, Line 203:
> die if `buf_size <= sizeof(*hdr)`

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/59795c20_b06e19f8?usp... :
PS4, Line 204: hdr->magic == GPUEB_PT_MAGIC
> Move this condition after the size check in line #205, because we should not access `hdr->magic` if  […]

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/a1f07a1a_b66763b9?usp... :
PS4, Line 205: gpueb_pt_hdr
> `*hdr`

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/05eb51b2_76014b47?usp... :
PS4, Line 247: auto_dma_fw[pt_id].fw_size <= GPUEB_SRAM_SIZE
> `auto_dma_fw[pt_id].fw_addr + auto_dma_fw[pt_id]. […]

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/c94b78db_9c66023d?usp... :
PS4, Line 279: 		       __func__, mfg0_pwr_con);
> merged to previous line

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/82a10483_265872cb?usp... :
PS4, Line 366: gpueb_platform_load
> gpueb_load

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/7d0d80b0_c70b0f9b?usp... :
PS4, Line 373: Load FW failed
> Failed to load FW

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/bcbe28ab_c2c4f98a?usp... :
PS4, Line 376: Failed
> Failed to wait for boot up

Done

File src/soc/mediatek/mt8196/include/soc/addressmap.h:

https://review.coreboot.org/c/coreboot/+/85654/comment/aa31e7d1_8ad3b85b?usp... :
PS4, Line 198: GPU_EB
> Either `GPUEB` or `GPU_EB`. Please be consistent with the name above.

Done

File src/soc/mediatek/mt8196/include/soc/gpueb_priv.h:

https://review.coreboot.org/c/coreboot/+/85654/comment/39e9ab7c_9f1389b8?usp... :
PS4, Line 64: (
> remove

Done

File src/soc/mediatek/mt8196/include/soc/memlayout.ld:

https://review.coreboot.org/c/coreboot/+/85654/comment/0671a571_ac891cb7?usp... :
PS4, Line 76: Memory
> memory

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/e2407187_6d1ca417?usp... :
PS4, Line 76: uP
> `microprocessor` (or is this supposed to mean `MCU`?)

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/252528dd_7ef2c4bc?usp... :
PS4, Line 76: 	/* Reserved Memory for the shared buffer between AP and uP */
> Could you explain more about the difference between these 2 regions?

Done