Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38400 )
Change subject: soc/mediatek/mt8183: reduce the hbp and hfp for phy timing ......................................................................
Patch Set 3:
(12 comments)
https://review.coreboot.org/c/coreboot/+/38400/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38400/1//COMMIT_MSG@9 PS1, Line 9: transfer
tranfers
Done
https://review.coreboot.org/c/coreboot/+/38400/1//COMMIT_MSG@12 PS1, Line 12: calc
calculated
Done
https://review.coreboot.org/c/coreboot/+/38400/1//COMMIT_MSG@14 PS1, Line 14: So dsi driver reduces the hbp and hfp to keep the line time.
Please give exact time values.
Done
https://review.coreboot.org/c/coreboot/+/38400/1/src/soc/mediatek/common/dsi... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/38400/1/src/soc/mediatek/common/dsi... PS1, Line 74: int data_rate
That argument doesn’t seem to be used anymore.
Done
https://review.coreboot.org/c/coreboot/+/38400/1/src/soc/mediatek/common/dsi... PS1, Line 201: * bytes_per_pixel - 10;
Formatting shouldn’t be changed.
Done
https://review.coreboot.org/c/coreboot/+/38400/2/src/soc/mediatek/common/dsi... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/38400/2/src/soc/mediatek/common/dsi... PS2, Line 74: data_rate
to make it easier sync with kernel code, you can rename the params : […]
Ack
https://review.coreboot.org/c/coreboot/+/38400/2/src/soc/mediatek/common/dsi... PS2, Line 78: (8 * 1000)
Why not (8 * KHz)?
Seems like the phy timing config is identical to kernel implementation so it's easier to use same name and code from there - that's why this is 1000 instead of KHz.
https://review.coreboot.org/c/coreboot/+/38400/2/src/soc/mediatek/common/dsi... PS2, Line 79: 4 * 1000
4000
Ack
https://review.coreboot.org/c/coreboot/+/38400/2/src/soc/mediatek/common/dsi... PS2, Line 80: 10 * 1000
10000
Ack
https://review.coreboot.org/c/coreboot/+/38400/2/src/soc/mediatek/common/dsi... PS2, Line 85: + 2
no need to do / 2?
Ack
https://review.coreboot.org/c/coreboot/+/38400/2/src/soc/mediatek/common/dsi... PS2, Line 90: clk_hs_trail
hs_trail is not available (set in next line). […]
Ack
https://review.coreboot.org/c/coreboot/+/38400/2/src/soc/mediatek/common/dsi... PS2, Line 93: (8 * 1000)
8000
Ack