Yu-Ping Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46626 )
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
soc/mediatek/mt8183: Fix pq module size config
For pq module size registers such as DISP_AAL_SIZE, the high bits should be HSIZE, while low bits should be VSIZE.
BUG=b:171167210 TEST=none BRANCH=kukui
Change-Id: I4b6aedf9a3ca133fcbe9cb88b99a13d228233e24 Signed-off-by: Yu-Ping Wu yupingso@google.com --- M src/soc/mediatek/mt8183/ddp.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/46626/1
diff --git a/src/soc/mediatek/mt8183/ddp.c b/src/soc/mediatek/mt8183/ddp.c index eba3f5e..395c821 100644 --- a/src/soc/mediatek/mt8183/ddp.c +++ b/src/soc/mediatek/mt8183/ddp.c @@ -34,7 +34,7 @@ static void enable_pq(struct disp_pq_regs *const regs, u32 width, u32 height, int enable_relay) { - write32(®s->size, height << 16 | width); + write32(®s->size, width << 16 | height); if (enable_relay) write32(®s->cfg, PQ_RELAY_MODE); write32(®s->en, PQ_EN);
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46626 )
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46626/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46626/1//COMMIT_MSG@8 PS1, Line 8: Please summarize the bug/problem, as access to the bug report is restricted.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46626 )
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
Patch Set 1: Code-Review+2
Hello Hung-Te Lin, build bot (Jenkins), yongqiang niu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46626
to look at the new patch set (#2).
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
soc/mediatek/mt8183: Fix pq module size config
For pq module size registers such as DISP_AAL_SIZE, the high bits should be HSIZE, while low bits should be VSIZE. Fix the incorrect settings for these registers where width and height are reversed.
BUG=b:171167210 TEST=none BRANCH=kukui
Change-Id: I4b6aedf9a3ca133fcbe9cb88b99a13d228233e24 Signed-off-by: Yu-Ping Wu yupingso@google.com --- M src/soc/mediatek/mt8183/ddp.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/46626/2
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46626 )
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46626/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46626/1//COMMIT_MSG@8 PS1, Line 8:
Please summarize the bug/problem, as access to the bug report is restricted.
The bug description is much the same as the this commit message.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46626 )
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
Patch Set 2: -Code-Review
(1 comment)
https://review.coreboot.org/c/coreboot/+/46626/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46626/1//COMMIT_MSG@8 PS1, Line 8:
The bug description is much the same as the this commit message.
The description here is more for "how", not "what" (problem).
Usually we want to describe what and how, for example
soc/mediatek/mt8183: Fix incorrect display on large panels
[brief intro for when/how that issue is reproduced]
[brief description on how to fix it]
I know you found this when tracing the details in ddp implementation, but can you figure out some potential issue that may generate failure if this is not fixed?
For example how PQ will be used, or how this may make things wrong if H/V sizes are large/small?
Hello Hung-Te Lin, build bot (Jenkins), yongqiang niu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46626
to look at the new patch set (#3).
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
soc/mediatek/mt8183: Fix pq module size config
For pq module size registers such as DISP_AAL_SIZE, the high bits should be HSIZE, while low bits should be VSIZE. Fix the incorrect settings for these registers where width and height are reversed.
According the MediaTek, there is no practical impact on mt8183 devices, but it's still nice to get this fixed to avoid future confusion.
BUG=b:171167210 TEST=none BRANCH=kukui
Change-Id: I4b6aedf9a3ca133fcbe9cb88b99a13d228233e24 Signed-off-by: Yu-Ping Wu yupingso@google.com --- M src/soc/mediatek/mt8183/ddp.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/46626/3
Attention is currently required from: Hung-Te Lin. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46626 )
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/46626/comment/118fcfa3_3c86965d PS1, Line 8:
The description here is more for "how", not "what" (problem). […]
Done
Attention is currently required from: Hung-Te Lin. Hello Hung-Te Lin, build bot (Jenkins), yongqiang niu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46626
to look at the new patch set (#4).
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
soc/mediatek/mt8183: Fix pq module size config
For pq module size registers such as DISP_AAL_SIZE, the high bits should be HSIZE, while low bits should be VSIZE. Fix the incorrect settings for these registers where width and height are reversed.
According to MediaTek, there is no practical impact on mt8183 devices, but it's still nice to get this fixed to avoid future confusion.
BUG=b:171167210 TEST=none BRANCH=kukui
Change-Id: I4b6aedf9a3ca133fcbe9cb88b99a13d228233e24 Signed-off-by: Yu-Ping Wu yupingso@google.com --- M src/soc/mediatek/mt8183/ddp.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/46626/4
Attention is currently required from: Yu-Ping Wu. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46626 )
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46626 )
Change subject: soc/mediatek/mt8183: Fix pq module size config ......................................................................
soc/mediatek/mt8183: Fix pq module size config
For pq module size registers such as DISP_AAL_SIZE, the high bits should be HSIZE, while low bits should be VSIZE. Fix the incorrect settings for these registers where width and height are reversed.
According to MediaTek, there is no practical impact on mt8183 devices, but it's still nice to get this fixed to avoid future confusion.
BUG=b:171167210 TEST=none BRANCH=kukui
Change-Id: I4b6aedf9a3ca133fcbe9cb88b99a13d228233e24 Signed-off-by: Yu-Ping Wu yupingso@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46626 Reviewed-by: Hung-Te Lin hungte@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/mediatek/mt8183/ddp.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved
diff --git a/src/soc/mediatek/mt8183/ddp.c b/src/soc/mediatek/mt8183/ddp.c index eba3f5e..395c821 100644 --- a/src/soc/mediatek/mt8183/ddp.c +++ b/src/soc/mediatek/mt8183/ddp.c @@ -34,7 +34,7 @@ static void enable_pq(struct disp_pq_regs *const regs, u32 width, u32 height, int enable_relay) { - write32(®s->size, height << 16 | width); + write32(®s->size, width << 16 | height); if (enable_relay) write32(®s->cfg, PQ_RELAY_MODE); write32(®s->en, PQ_EN);
-
Hung-Te Lin (Code Review) -
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Yu-Ping Wu (Code Review)