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

[Yu-Ping Wu (Code Review)]

Attention is currently required from: Alvin Liu, Andy Hsu, Hung-Te Lin, Jarried Lin, Yidi Lin.

Yu-Ping Wu 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 4:

(16 comments)

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

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

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

https://review.coreboot.org/c/coreboot/+/85654/comment/7aeeeb91_75c25d46?usp... :
PS2, Line 193: void *buf_addr
> not done

Done

https://review.coreboot.org/c/coreboot/+/85654/comment/4d91f626_2c267f67?usp... :
PS2, Line 289: imgsize
> not done

Done

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

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

https://review.coreboot.org/c/coreboot/+/85654/comment/6d5dee70_5838876e?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 `hdr` exceeds `buf_size`.

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

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

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

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

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

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

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

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

https://review.coreboot.org/c/coreboot/+/85654/comment/59410c82_36b4fa10?usp... :
PS4, Line 64: (
remove

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

https://review.coreboot.org/c/coreboot/+/85654/comment/6ccda524_8fffb0e6?usp... :
PS4, Line 14: #define GPU_BUF(addr, size) \
            :         REGION(resv_mem_gpu, addr, size, 4K)
            : 
            : #define RESV_MEMORY_GPUEB(addr, size)\
            : 	REGION(resv_mem_gpueb, addr, size, 2M)
            : 
Already defined above.

https://review.coreboot.org/c/coreboot/+/85654/comment/540bd8a4_1da6b03c?usp... :
PS4, Line 76: Memory
memory

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

https://review.coreboot.org/c/coreboot/+/85654/comment/69782537_b1952b93?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?