Hung-Te Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h M src/soc/mediatek/mt8173/include/soc/dsi.h 4 files changed, 150 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/34773/1
diff --git a/src/mainboard/google/oak/mainboard.c b/src/mainboard/google/oak/mainboard.c index 0dce17d..bac6fc5 100644 --- a/src/mainboard/google/oak/mainboard.c +++ b/src/mainboard/google/oak/mainboard.c @@ -234,7 +234,7 @@ edid_set_framebuffer_bits_per_pixel(&edid, 32, 0);
mtk_ddp_init(); - ret = mtk_dsi_init(mipi_dsi_flags, MIPI_DSI_FMT_RGB888, 4, &edid); + ret = mtk_dsi_init(mipi_dsi_flags, MIPI_DSI_FMT_RGB888, 4, &edid, NULL); if (ret < 0) { printk(BIOS_ERR, "dsi init fail\n"); return; diff --git a/src/soc/mediatek/common/dsi.c b/src/soc/mediatek/common/dsi.c index 17b127a..a269fe9 100644 --- a/src/soc/mediatek/common/dsi.c +++ b/src/soc/mediatek/common/dsi.c @@ -13,6 +13,7 @@ * GNU General Public License for more details. */
+#include <assert.h> #include <device/mmio.h> #include <console/console.h> #include <device/mmio.h> @@ -223,7 +224,134 @@ write32(&dsi0->dsi_start, 1); }
-int mtk_dsi_init(u32 mode_flags, u32 format, u32 lanes, const struct edid *edid) +static bool mtk_dsi_is_read_command(u32 type) +{ + switch (type) { + case MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM: + case MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM: + case MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM: + case MIPI_DSI_DCS_READ: + return true; + } + return false; +} + +static void mtk_dsi_cmdq(u8 *data, u8 len, u32 type) +{ + struct stopwatch sw; + u8 *tx_buf = data; + u8 cmdq_size; + u32 reg_val, cmdq_mask, i, config, cmdq_off, intsta_0; + + while (read32(&dsi0->dsi_intsta) & (1 << 31)) { + printk(BIOS_ERR, "%s wait dsi no busy\n", __func__); + mdelay(20); + } + + write32(&dsi0->dsi_intsta, 0); + + if (mtk_dsi_is_read_command(type)) + config = BTA; + else + config = (len > 2) ? LONG_PACKET : SHORT_PACKET; + + if (len > 2) { + cmdq_size = 1 + (len + 3) / 4; + cmdq_off = 4; + cmdq_mask = CONFIG | DATA_ID | DATA_0 | DATA_1; + reg_val = (len << 16) | (type << 8) | config; + } else { + cmdq_size = 1; + cmdq_off = 2; + cmdq_mask = CONFIG | DATA_ID; + reg_val = (type << 8) | config; + } + + for (i = 0; i < ARRAY_SIZE(dsi0->dsi_cmdq); i++) + write32(&dsi0->dsi_cmdq[i], 0); + + u8 *cmdq0 = (u8 *)&dsi0->dsi_cmdq[0]; + for (i = 0; i < len; i++) { + clrsetbits_le32(cmdq0 + ((cmdq_off + i) & (0xfffffffc)), + (0xff << (((i + cmdq_off) & 3) * 8)), + tx_buf[i] << (((i + cmdq_off) & 3) * 8)); + } + + clrsetbits_le32(&dsi0->dsi_cmdq[0], cmdq_mask, reg_val); + clrsetbits_le32(&dsi0->dsi_cmdq_size, CMDQ_SIZE, cmdq_size); + mtk_dsi_start(); + + stopwatch_init_usecs_expire(&sw, 400); + do { + intsta_0 = read32(&dsi0->dsi_intsta); + if (intsta_0 & CMD_DONE_INT_FLAG) + break; + udelay(4); + } while (!stopwatch_expired(&sw)); + + if (!(intsta_0 & CMD_DONE_INT_FLAG)) + printk(BIOS_ERR, "dsi send cmd time-out(400uS)\n"); +} + +static void mtk_dsi_send_init_commands(struct lcm_init_table *init_cmd) +{ + assert(init_cmd); + + for (; init_cmd->cmd != LCM_END_CMD; init_cmd++) { + u32 cmd = init_cmd->cmd, len = init_cmd->len; + u32 type; + + switch (cmd) { + case LCM_DELAY_CMD: + mdelay(len); + continue; + + case LCM_DCS_CMD: + switch (len) { + case 0: + return; + case 1: + type = MIPI_DSI_DCS_SHORT_WRITE; + break; + case 2: + type = MIPI_DSI_DCS_SHORT_WRITE_PARAM; + break; + default: + type = MIPI_DSI_DCS_LONG_WRITE; + break; + } + break; + + case LCM_GENERIC_CMD: + switch (len) { + case 0: + type = MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM; + break; + case 1: + type = MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM; + break; + case 2: + type = MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM; + break; + default: + type = MIPI_DSI_GENERIC_LONG_WRITE; + break; + } + break; + + default: + printk(BIOS_ERR, "%s: Unknown cmd: %d, abort\n", + __func__, cmd); + return; + + } + assert(len <= sizeof(init_cmd->data)); + mtk_dsi_cmdq(init_cmd->data, len, type); + } +} + +int mtk_dsi_init(u32 mode_flags, u32 format, u32 lanes, + const struct edid *edid, struct lcm_init_table *init_cmd) { int data_rate; u32 bpp = mtk_dsi_get_bits_per_pixel(format); @@ -239,9 +367,10 @@ mtk_dsi_rxtx_control(mode_flags, lanes); mtk_dsi_clk_hs_mode_disable(); mtk_dsi_config_vdo_timing(mode_flags, format, lanes, edid, &phy_timing); - mtk_dsi_set_mode(mode_flags); mtk_dsi_clk_hs_mode_enable(); - + if (init_cmd) + mtk_dsi_send_init_commands(init_cmd); + mtk_dsi_set_mode(mode_flags); mtk_dsi_start();
return 0; diff --git a/src/soc/mediatek/common/include/soc/dsi_common.h b/src/soc/mediatek/common/include/soc/dsi_common.h index 3e72c58..8597b60 100644 --- a/src/soc/mediatek/common/include/soc/dsi_common.h +++ b/src/soc/mediatek/common/include/soc/dsi_common.h @@ -40,6 +40,18 @@ u8 clk_hs_exit; };
+/* Definitions for cmd in lcm_init_table */ +#define LCM_END_CMD 0 +#define LCM_DELAY_CMD 1 +#define LCM_GENERIC_CMD 2 +#define LCM_DCS_CMD 3 + +struct lcm_init_table { + u32 cmd; + u32 len; + u8 data[7]; +}; + /* Functions that each SOC should provide. */ void mtk_dsi_reset(void); /* mtk_dsi_phy_clk_setting should return the data rate in Mbps. */ @@ -49,7 +61,8 @@ void mtk_dsi_override_phy_timing(struct mtk_phy_timing *timing);
/* Public API provided in common/dsi.c */ -int mtk_dsi_init(u32 mode_flags, u32 format, u32 lanes, - const struct edid *edid); +int mtk_dsi_bpp_from_format(u32 format); +int mtk_dsi_init(u32 mode_flags, u32 format, u32 lanes, const struct edid *edid, + struct lcm_init_table *init_cmd);
#endif // SOC_MEDIATEK_DSI_COMMON_H diff --git a/src/soc/mediatek/mt8173/include/soc/dsi.h b/src/soc/mediatek/mt8173/include/soc/dsi.h index e57bfc5..96d0aad 100644 --- a/src/soc/mediatek/mt8173/include/soc/dsi.h +++ b/src/soc/mediatek/mt8173/include/soc/dsi.h @@ -85,13 +85,13 @@ u8 reserved4[16]; u32 dsi_vm_cmd_con; u8 reserved5[204]; - u32 dsi_cmdq0; + u32 dsi_cmdq[128]; };
check_member(dsi_regs, dsi_phy_lccon, 0x104); check_member(dsi_regs, dsi_phy_timecon3, 0x11c); check_member(dsi_regs, dsi_vm_cmd_con, 0x130); -check_member(dsi_regs, dsi_cmdq0, 0x200); +check_member(dsi_regs, dsi_cmdq, 0x200); static struct dsi_regs *const dsi0 = (void *)DSI0_BASE; static struct dsi_regs *const dsi1 = (void *)DSI1_BASE;
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/+/34773
to look at the new patch set (#3).
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h M src/soc/mediatek/mt8173/include/soc/dsi.h 4 files changed, 150 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/34773/3
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/+/34773
to look at the new patch set (#4).
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h 3 files changed, 150 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/34773/4
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/+/34773
to look at the new patch set (#6).
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h 3 files changed, 150 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/34773/6
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/+/34773
to look at the new patch set (#9).
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h 3 files changed, 150 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/34773/9
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34773/10/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/10/src/soc/mediatek/common/ds... PS10, Line 307: mtk_dsi_start(); Already called inside mtk_dsi_init(), is this intended?
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34773/10/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/10/src/soc/mediatek/common/ds... PS10, Line 307: mtk_dsi_start();
Already called inside mtk_dsi_init(), is this intended?
Yes this is intended. Without that DSI won't fetch the cmdq.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 11: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/34773/10/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/10/src/soc/mediatek/common/ds... PS10, Line 307: mtk_dsi_start();
Yes this is intended. Without that DSI won't fetch the cmdq.
Ack
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 11:
(8 comments)
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 271: while (read32(&dsi0->dsi_intsta) & (1 << 31)) { 1u
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 272: printk(BIOS_ERR, "%s wait dsi no busy\n", __func__); Please use more elaborate error messages.
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 273: mdelay(20); 20 ms is quite a lot. Why that much?
Already use the stopwatch here?
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 274: } Should their be an abort condition after a certain time to avoid an infinite loop?
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 312: if (intsta_0 & CMD_DONE_INT_FLAG) The negative could be moved in the while condition.
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 315: } while (!stopwatch_expired(&sw)); Please print out as a debug message, how long it took.
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 318: printk(BIOS_ERR, "dsi send cmd time-out(400uS)\n"); Space before (, and spell: 400 us
dsi send cmd time-out (400 us)
Error level messages should be user understandable.
dsi snd cmd timed out after 400 us, panel probably won’t work
or something similar.
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 368: printk(BIOS_ERR, "%s: Unknown cmd: %d, abort\n", … abort panel initialization?
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 11:
(8 comments)
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 271: while (read32(&dsi0->dsi_intsta) & (1 << 31)) {
1u
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 272: printk(BIOS_ERR, "%s wait dsi no busy\n", __func__);
Please use more elaborate error messages.
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 273: mdelay(20);
20 ms is quite a lot. Why that much? […]
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 274: }
Should their be an abort condition after a certain time to avoid an infinite loop?
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 312: if (intsta_0 & CMD_DONE_INT_FLAG)
The negative could be moved in the while condition.
We don't want to do udelay() when the read has right flags set, so the check can't be in while.
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 315: } while (!stopwatch_expired(&sw));
Please print out as a debug message, how long it took.
that's something > 400us right? I don't think such debug message would be helpful, maybe skip?
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 318: printk(BIOS_ERR, "dsi send cmd time-out(400uS)\n");
Space before (, and spell: 400 us […]
Done
https://review.coreboot.org/c/coreboot/+/34773/11/src/soc/mediatek/common/ds... PS11, Line 368: printk(BIOS_ERR, "%s: Unknown cmd: %d, abort\n",
… abort panel initialization?
Done
Hello Yu-Ping Wu, Nicolas Boichat, 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/+/34773
to look at the new patch set (#12).
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h 3 files changed, 159 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/34773/12
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 12:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 265: u32 delay_us, nit: Is it really worth allowing the caller to pass a custom delay here? Generally, I think you don't need an explicit delay at all to busy wait on a register. All udelay() does is busy-wait on a timer register instead, doesn't really make a difference.
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 314: write32(&dsi0->dsi_cmdq[i], 0); Is this necessary? I have a feeling that this is only here because the weird read-modify-write for each byte thing below may have you keep stale data from a previous command if your length isn't divisible by 4. If you fix that, maybe this won't be necessary?
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 318: clrsetbits_le32(cmdq0 + ((cmdq_off + i) & (0xfffffffc)), This is pretty nasty. You're reading and writing each peripheral register four times. Can't we rather do something like (replacing some of the above as well... the whole cmdq_size/cmdq_off calculation stuff seems more trouble than it's worth to me, honestly):
if (len <= 2) { uint32_t val = (type << 8) | config; for (j = 0; j < MIN(2, len); j++) val |= tx_buf[j] << (j + 2); write32(&dsi0->dsi_cmdq[0], val); write32(&dsi0->dsi_cmdq_size, 1); } else { write32(&dsi0->dsi_cmdq[0], (len << 16) | (type << 8) | config); for (i = 0; i < len; i += 4) { uint32_t val = 0; for (j = 0; j < MIN(len - i, 4); j++) val |= tx_buf[i + j] << j; write32(&dsi0->dsi_cmdq[i/4 + 1], val); } write32(&dsi0->dsi_cmdq_size, 1 + DIV_ROUND_UP(len, 4)); }
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 324: clrsetbits_le32(&dsi0->dsi_cmdq_size, CMDQ_SIZE, cmdq_size); nit: Does this need to be a clrsetbits? The rest of this driver isn't shy about setting unused bits to 0 either.
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 333: assert(init); nit: maybe just do the
if (!init) return;
in here so you don't need to check in the caller?
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 355: type = MIPI_DSI_DCS_LONG_WRITE; A lot of this reminds me of Rockchip code, we should probably spend some time looking for things to factor out and generalize from our various MIPI implementations some day... but not today.
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/in... File src/soc/mediatek/common/include/soc/dsi_common.h:
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/in... PS12, Line 334: u16 len; See my earlier comments about structure packing
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 265: u32 delay_us,
nit: Is it really worth allowing the caller to pass a custom delay here? Generally, I think you don' […]
edit: I just remembered, we have wait_us() now... so you can probably use that instead of a custom helper here.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 318: clrsetbits_le32(cmdq0 + ((cmdq_off + i) & (0xfffffffc)),
This is pretty nasty. You're reading and writing each peripheral register four times. […]
Actually, I've seen this same pattern over and over again, almost every SPI/I2C/whatever driver has the same problem with their FIFO registers. I think this would be a good opportunity to add a helper macro that deals with the tricky bits of this operation. I've tried to write that with CB:34817. With that, I think you could reduce this to:
u32 prefix = (type << 8) | config; int prefsz = 2; if (len > 2) { prefsz = 4; prefix |= len << 16; } buffer_to_fifo32_prefix(tx_buf, prefix, prefsz, len + prefsz, &dsi0->dsi_cmdq, 4, 4); write32(&dsi0->dsi_cmdq_size, DIV_ROUND_UP(len + prefsz, 4));
Let me know what you think!
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 13:
(6 comments)
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 265: u32 delay_us,
edit: I just remembered, we have wait_us() now... […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 314: write32(&dsi0->dsi_cmdq[i], 0);
Is this necessary? I have a feeling that this is only here because the weird read-modify-write for e […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 318: clrsetbits_le32(cmdq0 + ((cmdq_off + i) & (0xfffffffc)),
Actually, I've seen this same pattern over and over again, almost every SPI/I2C/whatever driver has […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 324: clrsetbits_le32(&dsi0->dsi_cmdq_size, CMDQ_SIZE, cmdq_size);
nit: Does this need to be a clrsetbits? The rest of this driver isn't shy about setting unused bits […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 333: assert(init);
nit: maybe just do the […]
Done
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/ds... PS12, Line 355: type = MIPI_DSI_DCS_LONG_WRITE;
A lot of this reminds me of Rockchip code, we should probably spend some time looking for things to […]
Ack
Hello Yu-Ping Wu, Nicolas Boichat, Julius Werner, yongqiang niu, You-Cheng Syu, jitao shi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34773
to look at the new patch set (#15).
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h 3 files changed, 139 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/34773/15
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 15: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/34773/15/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/15/src/soc/mediatek/common/ds... PS15, Line 278: 20000 nit: we have wait_ms() too
https://review.coreboot.org/c/coreboot/+/34773/15/src/soc/mediatek/common/ds... PS15, Line 293: MIN nit: doesn't really need the MIN here
Hello Yu-Ping Wu, Nicolas Boichat, Julius Werner, yongqiang niu, You-Cheng Syu, jitao shi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34773
to look at the new patch set (#17).
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h 3 files changed, 139 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/34773/17
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 17:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34773/15/src/soc/mediatek/common/ds... File src/soc/mediatek/common/dsi.c:
https://review.coreboot.org/c/coreboot/+/34773/15/src/soc/mediatek/common/ds... PS15, Line 278: 20000
nit: we have wait_ms() too
Done
https://review.coreboot.org/c/coreboot/+/34773/15/src/soc/mediatek/common/ds... PS15, Line 293: MIN
nit: doesn't really need the MIN here
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/in... File src/soc/mediatek/common/include/soc/dsi_common.h:
https://review.coreboot.org/c/coreboot/+/34773/12/src/soc/mediatek/common/in... PS12, Line 334: u16 len;
See my earlier comments about structure packing
Ack
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 17: Code-Review+2
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
Patch Set 19: Code-Review+2
Hello Yu-Ping Wu, Nicolas Boichat, Julius Werner, yongqiang niu, You-Cheng Syu, jitao shi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34773
to look at the new patch set (#20).
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h 3 files changed, 139 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/34773/20
Julius Werner has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34773 )
Change subject: soc/mediatek: dsi: Support sending MIPI init commands ......................................................................
soc/mediatek: dsi: Support sending MIPI init commands
For systems with real MIPI panels (8173/oak was using PS8640 eDP bridge), we have to send DCS commands to initialize panel.
BUG=b:80501386,b:117254947 TEST=make -j # board = oak and boots
Change-Id: Ie7c824873465ac82a95bcb0ed67b8b9866987008 Signed-off-by: Hung-Te Lin hungte@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/34773 Reviewed-by: Julius Werner jwerner@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/oak/mainboard.c M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h 3 files changed, 139 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Hung-Te Lin: Looks good to me, approved
diff --git a/src/mainboard/google/oak/mainboard.c b/src/mainboard/google/oak/mainboard.c index 0dce17d..bac6fc5 100644 --- a/src/mainboard/google/oak/mainboard.c +++ b/src/mainboard/google/oak/mainboard.c @@ -234,7 +234,7 @@ edid_set_framebuffer_bits_per_pixel(&edid, 32, 0);
mtk_ddp_init(); - ret = mtk_dsi_init(mipi_dsi_flags, MIPI_DSI_FMT_RGB888, 4, &edid); + ret = mtk_dsi_init(mipi_dsi_flags, MIPI_DSI_FMT_RGB888, 4, &edid, NULL); if (ret < 0) { printk(BIOS_ERR, "dsi init fail\n"); return; diff --git a/src/soc/mediatek/common/dsi.c b/src/soc/mediatek/common/dsi.c index 392b02d..fffe51f 100644 --- a/src/soc/mediatek/common/dsi.c +++ b/src/soc/mediatek/common/dsi.c @@ -13,6 +13,7 @@ * GNU General Public License for more details. */
+#include <assert.h> #include <device/mmio.h> #include <console/console.h> #include <device/mmio.h> @@ -256,7 +257,124 @@ write32(&dsi0->dsi_start, 1); }
-int mtk_dsi_init(u32 mode_flags, u32 format, u32 lanes, const struct edid *edid) +static bool mtk_dsi_is_read_command(u32 type) +{ + switch (type) { + case MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM: + case MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM: + case MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM: + case MIPI_DSI_DCS_READ: + return true; + } + return false; +} + +static void mtk_dsi_cmdq(const u8 *data, u8 len, u32 type) +{ + const u8 *tx_buf = data; + u32 config; + int i, j; + + if (!wait_ms(20, !(read32(&dsi0->dsi_intsta) & DSI_BUSY))) { + printk(BIOS_ERR, "%s: cannot get DSI ready for sending commands" + " after 20ms and the panel may not work properly.\n", + __func__); + return; + } + write32(&dsi0->dsi_intsta, 0); + + if (mtk_dsi_is_read_command(type)) + config = BTA; + else + config = (len > 2) ? LONG_PACKET : SHORT_PACKET; + + if (len <= 2) { + uint32_t val = (type << 8) | config; + for (i = 0; i < len; i++) + val |= tx_buf[i] << (i + 2) * 8; + write32(&dsi0->dsi_cmdq[0], val); + write32(&dsi0->dsi_cmdq_size, 1); + } else { + /* TODO(hungte) Replace by buffer_to_fifo32_prefix */ + write32(&dsi0->dsi_cmdq[0], (len << 16) | (type << 8) | config); + for (i = 0; i < len; i += 4) { + uint32_t val = 0; + for (j = 0; j < MIN(len - i, 4); j++) + val |= tx_buf[i + j] << j * 8; + write32(&dsi0->dsi_cmdq[i / 4 + 1], val); + } + write32(&dsi0->dsi_cmdq_size, 1 + DIV_ROUND_UP(len, 4)); + } + + mtk_dsi_start(); + + if (!wait_us(400, read32(&dsi0->dsi_intsta) & CMD_DONE_INT_FLAG)) { + printk(BIOS_ERR, "%s: failed sending DSI command, " + "panel may not work.\n", __func__); + return; + } +} + +static void mtk_dsi_send_init_commands(const struct lcm_init_command *init) +{ + if (!init) + return; + + for (; init->cmd != LCM_END_CMD; init++) { + u32 cmd = init->cmd, len = init->len; + u32 type; + + switch (cmd) { + case LCM_DELAY_CMD: + mdelay(len); + continue; + + case LCM_DCS_CMD: + switch (len) { + case 0: + return; + case 1: + type = MIPI_DSI_DCS_SHORT_WRITE; + break; + case 2: + type = MIPI_DSI_DCS_SHORT_WRITE_PARAM; + break; + default: + type = MIPI_DSI_DCS_LONG_WRITE; + break; + } + break; + + case LCM_GENERIC_CMD: + switch (len) { + case 0: + type = MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM; + break; + case 1: + type = MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM; + break; + case 2: + type = MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM; + break; + default: + type = MIPI_DSI_GENERIC_LONG_WRITE; + break; + } + break; + + default: + printk(BIOS_ERR, "%s: Unknown cmd: %d, " + "abort panel initialization.\n", __func__, cmd); + return; + + } + assert(len <= sizeof(init->data)); + mtk_dsi_cmdq(init->data, len, type); + } +} + +int mtk_dsi_init(u32 mode_flags, u32 format, u32 lanes, const struct edid *edid, + const struct lcm_init_command *init_commands) { int data_rate; u32 bits_per_pixel = mtk_dsi_get_bits_per_pixel(format); @@ -272,9 +390,9 @@ mtk_dsi_rxtx_control(mode_flags, lanes); mtk_dsi_clk_hs_mode_disable(); mtk_dsi_config_vdo_timing(mode_flags, format, lanes, edid, &phy_timing); - mtk_dsi_set_mode(mode_flags); mtk_dsi_clk_hs_mode_enable(); - + mtk_dsi_send_init_commands(init_commands); + mtk_dsi_set_mode(mode_flags); mtk_dsi_start();
return 0; diff --git a/src/soc/mediatek/common/include/soc/dsi_common.h b/src/soc/mediatek/common/include/soc/dsi_common.h index 0738876..3f4a47d 100644 --- a/src/soc/mediatek/common/include/soc/dsi_common.h +++ b/src/soc/mediatek/common/include/soc/dsi_common.h @@ -85,14 +85,14 @@ u8 reserved4[16]; u32 dsi_vm_cmd_con; u8 reserved5[204]; - u32 dsi_cmdq0; + u32 dsi_cmdq[128]; }; static struct dsi_regs *const dsi0 = (void *)DSI0_BASE;
check_member(dsi_regs, dsi_phy_lccon, 0x104); check_member(dsi_regs, dsi_phy_timecon3, 0x11c); check_member(dsi_regs, dsi_vm_cmd_con, 0x130); -check_member(dsi_regs, dsi_cmdq0, 0x200); +check_member(dsi_regs, dsi_cmdq, 0x200);
/* DSI_INTSTA */ enum { @@ -324,6 +324,18 @@ u32 d_phy; };
+/* Definitions for cmd in lcm_init_command */ +#define LCM_END_CMD 0 +#define LCM_DELAY_CMD 1 +#define LCM_GENERIC_CMD 2 +#define LCM_DCS_CMD 3 + +struct lcm_init_command { + u16 cmd; + u16 len; + u8 data[8]; +}; + /* Functions that each SOC should provide. */ void mtk_dsi_reset(void); void mtk_dsi_configure_mipi_tx(int data_rate, u32 lanes); @@ -332,7 +344,8 @@ void mtk_dsi_override_phy_timing(struct mtk_phy_timing *timing);
/* Public API provided in common/dsi.c */ -int mtk_dsi_init(u32 mode_flags, u32 format, u32 lanes, - const struct edid *edid); +int mtk_dsi_bpp_from_format(u32 format); +int mtk_dsi_init(u32 mode_flags, u32 format, u32 lanes, const struct edid *edid, + const struct lcm_init_command *init_commands);
#endif /* SOC_MEDIATEK_DSI_COMMON_H */
-
Hung-Te Lin (Code Review) -
Julius Werner (Code Review)
-
Paul Menzel (Code Review)
-
Yu-Ping Wu (Code Review)