Hello Yu-Ping Wu, yongqiang niu, Julius Werner, You-Cheng Syu, jitao shi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34771
to look at the new patch set (#8).
Change subject: soc/mediatek: dsi: Refactor PHY timing calculation
......................................................................
soc/mediatek: dsi: Refactor PHY timing calculation
The PHY timing should be calculated by data rate (Mbps). However for 8173
some values were hard-coded so we want to introduce a mtk_phy_timing
structure and a weak function mtk_dsi_override_phy_timing that allows
per-SOC customization to fine tune PHY timings.
BUG=b:80501386,b:117254947
TEST=make -j # board = oak and boots
Change-Id: I1176ca06dda026029ff431aca7f9e21479eed670
Signed-off-by: Hung-Te Lin <hungte(a)chromium.org>
---
M src/soc/mediatek/common/dsi.c
M src/soc/mediatek/common/include/soc/dsi_common.h
M src/soc/mediatek/mt8173/dsi.c
3 files changed, 74 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/34771/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/34771
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1176ca06dda026029ff431aca7f9e21479eed670
Gerrit-Change-Number: 34771
Gerrit-PatchSet: 8
Gerrit-Owner: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: You-Cheng Syu <youcheng(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: jitao shi <jitao.shi(a)mediatek.com>
Gerrit-Reviewer: yongqiang niu <yongqiang.niu(a)mediatek.com>
Gerrit-MessageType: newpatchset
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34771 )
Change subject: soc/mediatek: dsi: Refactor PHY timing calculation
......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34771/6/src/soc/mediatek/common/ds…
File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34771/6/src/soc/mediatek/common/ds…
PS6, Line 100: timcon0 = phy_timing->lpx | phy_timing->da_hs_prepare << 8 |
: phy_timing->da_hs_zero << 16 | phy_timing->da_hs_trail << 24;
> Does it make sense to define a macro similar to the following? Then much code under soc/mediatek can […]
That's an interesting idea, but I think the intent was different.
What they did there was really to encode a string into a word (for mark, signal, or magic identifiers). However here we are constructing values for a register. it's simply coincident that many of the calls have 4 fields, each in 8 bytes. But if we want to really write this in a better way, you should deal with field, shift, length, mask, ... etc and usually in the end it's probably not better than v = a | b << 8 | c << 16 | d << 24, or
write32(&v, a << REG_FIELD_A_SHIFT | b << REG_FIELD_B_SHIFT, ...)
So I think we're probably staying with this until something better is done.
--
To view, visit https://review.coreboot.org/c/coreboot/+/34771
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1176ca06dda026029ff431aca7f9e21479eed670
Gerrit-Change-Number: 34771
Gerrit-PatchSet: 7
Gerrit-Owner: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: You-Cheng Syu <youcheng(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: jitao shi <jitao.shi(a)mediatek.com>
Gerrit-Reviewer: yongqiang niu <yongqiang.niu(a)mediatek.com>
Gerrit-Comment-Date: Thu, 08 Aug 2019 03:06:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-MessageType: comment
Hello Yu-Ping Wu, yongqiang niu, Julius Werner, You-Cheng Syu, jitao shi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34770
to look at the new patch set (#5).
Change subject: soc/mediatek: dsi: Unify format to bpp conversion
......................................................................
soc/mediatek: dsi: Unify format to bpp conversion
The 'bpp' was referred to both 'bits per pixel' and 'bytes per pixel' in
MTK DSI driver and should be corrected. By this change we now always
consider 'bpp' as 'bits per pixel', and rename the variables for other
cases.
BUG=b:80501386,b:117254947
TEST=make -j # board = oak and boots
Change-Id: Ibd405220b73859e5592c68f498af07eef8d7edbc
Signed-off-by: Hung-Te Lin <hungte(a)chromium.org>
---
M src/soc/mediatek/common/dsi.c
M src/soc/mediatek/common/include/soc/dsi_common.h
M src/soc/mediatek/mt8173/dsi.c
3 files changed, 29 insertions(+), 34 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/34770/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/34770
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibd405220b73859e5592c68f498af07eef8d7edbc
Gerrit-Change-Number: 34770
Gerrit-PatchSet: 5
Gerrit-Owner: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: You-Cheng Syu <youcheng(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: jitao shi <jitao.shi(a)mediatek.com>
Gerrit-Reviewer: yongqiang niu <yongqiang.niu(a)mediatek.com>
Gerrit-MessageType: newpatchset
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34771 )
Change subject: soc/mediatek: dsi: Refactor PHY timing calculation
......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34771/6//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/34771/6//COMMIT_MSG@9
PS6, Line 9: calculate
calculated
https://review.coreboot.org/c/coreboot/+/34771/6/src/soc/mediatek/common/ds…
File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34771/6/src/soc/mediatek/common/ds…
PS6, Line 100: timcon0 = phy_timing->lpx | phy_timing->da_hs_prepare << 8 |
: phy_timing->da_hs_zero << 16 | phy_timing->da_hs_trail << 24;
Does it make sense to define a macro similar to the following? Then much code under soc/mediatek can be simplified.
#define STRING_TO_UINT32(a, b, c, d) ((UINT32) ((d << 24) | (c << 16) | (b << 8) | a))
(from in amd/agesa/f16kb/Proc/GNB/Common/Gnb.h)
--
To view, visit https://review.coreboot.org/c/coreboot/+/34771
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1176ca06dda026029ff431aca7f9e21479eed670
Gerrit-Change-Number: 34771
Gerrit-PatchSet: 6
Gerrit-Owner: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: You-Cheng Syu <youcheng(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: jitao shi <jitao.shi(a)mediatek.com>
Gerrit-Reviewer: yongqiang niu <yongqiang.niu(a)mediatek.com>
Gerrit-Comment-Date: Thu, 08 Aug 2019 02:30:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment