| Message ID | 1649644308-8455-4-git-send-email-xinlei.lee@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cooperate with DSI RX devices to modify dsi funcs and delay mipi high to cooperate with panel sequence | expand |
Il 11/04/22 04:31, xinlei.lee@mediatek.com ha scritto: > From: Jitao Shi <jitao.shi@mediatek.com> > > To comply with the panel sequence, hold the mipi signal to LP00 before the dcs cmds transmission, > and pull the mipi signal high from LP00 to LP11 until the start of the dcs cmds transmission. > The normal panel timing is : > (1) pp1800 DC pull up > (2) avdd & avee AC pull high > (3) lcm_reset pull high -> pull low -> pull high > (4) Pull MIPI signal high (LP11) -> initial code -> send video data(HS mode) > The power-off sequence is reversed. > If dsi is not in cmd mode, then dsi will pull the mipi signal high in the mtk_output_dsi_enable function. > > Fixes: 2dd8075d2185 ("drm/mediatek: mtk_dsi: Use the drm_panel_bridge API") > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > Signed-off-by: Xinlei Lee <xinlei.lee@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > index cf76c53a1af6..9ad6f08c8bfe 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -203,6 +203,7 @@ struct mtk_dsi { > struct mtk_phy_timing phy_timing; > int refcount; > bool enabled; > + bool lanes_ready; > u32 irq_data; > wait_queue_head_t irq_wait_queue; > const struct mtk_dsi_driver_data *driver_data; > @@ -654,13 +655,6 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) > mtk_dsi_config_vdo_timing(dsi); > mtk_dsi_set_interrupt_enable(dsi); > > - mtk_dsi_rxtx_control(dsi); > - usleep_range(30, 100); > - mtk_dsi_reset_dphy(dsi); > - mtk_dsi_clk_ulp_mode_leave(dsi); > - mtk_dsi_lane0_ulp_mode_leave(dsi); > - mtk_dsi_clk_hs_mode(dsi, 0); > - > return 0; > err_disable_engine_clk: > clk_disable_unprepare(dsi->engine_clk); > @@ -689,6 +683,23 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) > clk_disable_unprepare(dsi->digital_clk); > > phy_power_off(dsi->phy); > + > + dsi->lanes_ready = false; > +} > + > +static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) > +{ > + if (!dsi->lanes_ready) { > + dsi->lanes_ready = true; > + mtk_dsi_rxtx_control(dsi); > + usleep_range(30, 100); > + mtk_dsi_reset_dphy(dsi); > + mtk_dsi_clk_ulp_mode_leave(dsi); > + mtk_dsi_lane0_ulp_mode_leave(dsi); > + mtk_dsi_clk_hs_mode(dsi, 0); > + msleep(20); This is a very long sleep, which wasn't present before this change. Please document the reasons why we need this 20ms sleep with a comment in the code. Regards, Angelo
On Mon, 2022-04-11 at 11:07 +0200, AngeloGioacchino Del Regno wrote: > Il 11/04/22 04:31, xinlei.lee@mediatek.com ha scritto: > > From: Jitao Shi <jitao.shi@mediatek.com> > > > > To comply with the panel sequence, hold the mipi signal to LP00 > > before the dcs cmds transmission, > > and pull the mipi signal high from LP00 to LP11 until the start of > > the dcs cmds transmission. > > The normal panel timing is : > > (1) pp1800 DC pull up > > (2) avdd & avee AC pull high > > (3) lcm_reset pull high -> pull low -> pull high > > (4) Pull MIPI signal high (LP11) -> initial code -> send video > > data(HS mode) > > The power-off sequence is reversed. > > If dsi is not in cmd mode, then dsi will pull the mipi signal high > > in the mtk_output_dsi_enable function. > > > > Fixes: 2dd8075d2185 ("drm/mediatek: mtk_dsi: Use the > > drm_panel_bridge API") > > > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > > Signed-off-by: Xinlei Lee <xinlei.lee@mediatek.com> > > --- > > drivers/gpu/drm/mediatek/mtk_dsi.c | 28 +++++++++++++++++++++-- > > ----- > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > > b/drivers/gpu/drm/mediatek/mtk_dsi.c > > index cf76c53a1af6..9ad6f08c8bfe 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > > @@ -203,6 +203,7 @@ struct mtk_dsi { > > struct mtk_phy_timing phy_timing; > > int refcount; > > bool enabled; > > + bool lanes_ready; > > u32 irq_data; > > wait_queue_head_t irq_wait_queue; > > const struct mtk_dsi_driver_data *driver_data; > > @@ -654,13 +655,6 @@ static int mtk_dsi_poweron(struct mtk_dsi > > *dsi) > > mtk_dsi_config_vdo_timing(dsi); > > mtk_dsi_set_interrupt_enable(dsi); > > > > - mtk_dsi_rxtx_control(dsi); > > - usleep_range(30, 100); > > - mtk_dsi_reset_dphy(dsi); > > - mtk_dsi_clk_ulp_mode_leave(dsi); > > - mtk_dsi_lane0_ulp_mode_leave(dsi); > > - mtk_dsi_clk_hs_mode(dsi, 0); > > - > > return 0; > > err_disable_engine_clk: > > clk_disable_unprepare(dsi->engine_clk); > > @@ -689,6 +683,23 @@ static void mtk_dsi_poweroff(struct mtk_dsi > > *dsi) > > clk_disable_unprepare(dsi->digital_clk); > > > > phy_power_off(dsi->phy); > > + > > + dsi->lanes_ready = false; > > +} > > + > > +static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) > > +{ > > + if (!dsi->lanes_ready) { > > + dsi->lanes_ready = true; > > + mtk_dsi_rxtx_control(dsi); > > + usleep_range(30, 100); > > + mtk_dsi_reset_dphy(dsi); > > + mtk_dsi_clk_ulp_mode_leave(dsi); > > + mtk_dsi_lane0_ulp_mode_leave(dsi); > > + mtk_dsi_clk_hs_mode(dsi, 0); > > + msleep(20); > > This is a very long sleep, which wasn't present before this change. > Please document the reasons why we need this 20ms sleep with a > comment > in the code. > > Regards, > Angelo > > Hello Xinlei, As Angelo mentioned. I think you should add this in commit message and driver comments. (Your reply in v3.) "The 20ms delay in mtk_dsi_lane_ready() is because dsi needs to give dsi_rx(panel) a reaction time after pulling up the mipi signal." BRs, Rex
On Mon, 2022-04-11 at 11:07 +0200, AngeloGioacchino Del Regno wrote: > Il 11/04/22 04:31, xinlei.lee@mediatek.com ha scritto: > > From: Jitao Shi <jitao.shi@mediatek.com> > > > > To comply with the panel sequence, hold the mipi signal to LP00 > > before the dcs cmds transmission, > > and pull the mipi signal high from LP00 to LP11 until the start of > > the dcs cmds transmission. > > The normal panel timing is : > > (1) pp1800 DC pull up > > (2) avdd & avee AC pull high > > (3) lcm_reset pull high -> pull low -> pull high > > (4) Pull MIPI signal high (LP11) -> initial code -> send video > > data(HS mode) > > The power-off sequence is reversed. > > If dsi is not in cmd mode, then dsi will pull the mipi signal high > > in the mtk_output_dsi_enable function. > > > > Fixes: 2dd8075d2185 ("drm/mediatek: mtk_dsi: Use the > > drm_panel_bridge API") > > > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > > Signed-off-by: Xinlei Lee <xinlei.lee@mediatek.com> > > --- > > drivers/gpu/drm/mediatek/mtk_dsi.c | 28 +++++++++++++++++++++-- > > ----- > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > > b/drivers/gpu/drm/mediatek/mtk_dsi.c > > index cf76c53a1af6..9ad6f08c8bfe 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > > @@ -203,6 +203,7 @@ struct mtk_dsi { > > struct mtk_phy_timing phy_timing; > > int refcount; > > bool enabled; > > + bool lanes_ready; > > u32 irq_data; > > wait_queue_head_t irq_wait_queue; > > const struct mtk_dsi_driver_data *driver_data; > > @@ -654,13 +655,6 @@ static int mtk_dsi_poweron(struct mtk_dsi > > *dsi) > > mtk_dsi_config_vdo_timing(dsi); > > mtk_dsi_set_interrupt_enable(dsi); > > > > - mtk_dsi_rxtx_control(dsi); > > - usleep_range(30, 100); > > - mtk_dsi_reset_dphy(dsi); > > - mtk_dsi_clk_ulp_mode_leave(dsi); > > - mtk_dsi_lane0_ulp_mode_leave(dsi); > > - mtk_dsi_clk_hs_mode(dsi, 0); > > - > > return 0; > > err_disable_engine_clk: > > clk_disable_unprepare(dsi->engine_clk); > > @@ -689,6 +683,23 @@ static void mtk_dsi_poweroff(struct mtk_dsi > > *dsi) > > clk_disable_unprepare(dsi->digital_clk); > > > > phy_power_off(dsi->phy); > > + > > + dsi->lanes_ready = false; > > +} > > + > > +static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) > > +{ > > + if (!dsi->lanes_ready) { > > + dsi->lanes_ready = true; > > + mtk_dsi_rxtx_control(dsi); > > + usleep_range(30, 100); > > + mtk_dsi_reset_dphy(dsi); > > + mtk_dsi_clk_ulp_mode_leave(dsi); > > + mtk_dsi_lane0_ulp_mode_leave(dsi); > > + mtk_dsi_clk_hs_mode(dsi, 0); > > + msleep(20); > > This is a very long sleep, which wasn't present before this change. > Please document the reasons why we need this 20ms sleep with a > comment > in the code. > > Regards, > Angelo > > Hi Angelo: Thanks for your review. As mentioned in the previous reply, it is because the time required to respond to dsi_rx is about one frame. I will add this explanation in the next edition of code & comments. Best Regards! xinlei
On Tue, 2022-04-12 at 15:43 +0800, Rex-BC Chen wrote: > On Mon, 2022-04-11 at 11:07 +0200, AngeloGioacchino Del Regno wrote: > > Il 11/04/22 04:31, xinlei.lee@mediatek.com ha scritto: > > > From: Jitao Shi <jitao.shi@mediatek.com> > > > > > > To comply with the panel sequence, hold the mipi signal to LP00 > > > before the dcs cmds transmission, > > > and pull the mipi signal high from LP00 to LP11 until the start > > > of > > > the dcs cmds transmission. > > > The normal panel timing is : > > > (1) pp1800 DC pull up > > > (2) avdd & avee AC pull high > > > (3) lcm_reset pull high -> pull low -> pull high > > > (4) Pull MIPI signal high (LP11) -> initial code -> send video > > > data(HS mode) > > > The power-off sequence is reversed. > > > If dsi is not in cmd mode, then dsi will pull the mipi signal > > > high > > > in the mtk_output_dsi_enable function. > > > > > > Fixes: 2dd8075d2185 ("drm/mediatek: mtk_dsi: Use the > > > drm_panel_bridge API") > > > > > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > > > Signed-off-by: Xinlei Lee <xinlei.lee@mediatek.com> > > > --- > > > drivers/gpu/drm/mediatek/mtk_dsi.c | 28 +++++++++++++++++++++-- > > > ----- > > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > > > b/drivers/gpu/drm/mediatek/mtk_dsi.c > > > index cf76c53a1af6..9ad6f08c8bfe 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > > > @@ -203,6 +203,7 @@ struct mtk_dsi { > > > struct mtk_phy_timing phy_timing; > > > int refcount; > > > bool enabled; > > > + bool lanes_ready; > > > u32 irq_data; > > > wait_queue_head_t irq_wait_queue; > > > const struct mtk_dsi_driver_data *driver_data; > > > @@ -654,13 +655,6 @@ static int mtk_dsi_poweron(struct mtk_dsi > > > *dsi) > > > mtk_dsi_config_vdo_timing(dsi); > > > mtk_dsi_set_interrupt_enable(dsi); > > > > > > - mtk_dsi_rxtx_control(dsi); > > > - usleep_range(30, 100); > > > - mtk_dsi_reset_dphy(dsi); > > > - mtk_dsi_clk_ulp_mode_leave(dsi); > > > - mtk_dsi_lane0_ulp_mode_leave(dsi); > > > - mtk_dsi_clk_hs_mode(dsi, 0); > > > - > > > return 0; > > > err_disable_engine_clk: > > > clk_disable_unprepare(dsi->engine_clk); > > > @@ -689,6 +683,23 @@ static void mtk_dsi_poweroff(struct mtk_dsi > > > *dsi) > > > clk_disable_unprepare(dsi->digital_clk); > > > > > > phy_power_off(dsi->phy); > > > + > > > + dsi->lanes_ready = false; > > > +} > > > + > > > +static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) > > > +{ > > > + if (!dsi->lanes_ready) { > > > + dsi->lanes_ready = true; > > > + mtk_dsi_rxtx_control(dsi); > > > + usleep_range(30, 100); > > > + mtk_dsi_reset_dphy(dsi); > > > + mtk_dsi_clk_ulp_mode_leave(dsi); > > > + mtk_dsi_lane0_ulp_mode_leave(dsi); > > > + mtk_dsi_clk_hs_mode(dsi, 0); > > > + msleep(20); > > > > This is a very long sleep, which wasn't present before this change. > > Please document the reasons why we need this 20ms sleep with a > > comment > > in the code. > > > > Regards, > > Angelo > > > > > > Hello Xinlei, > > As Angelo mentioned. > I think you should add this in commit message and driver comments. > (Your reply in v3.) > "The 20ms delay in mtk_dsi_lane_ready() is because dsi needs to give > dsi_rx(panel) a reaction time after pulling up the mipi signal." > > BRs, > Rex > Hey Rex: Thanks for your review and reminder. I will add this explanation in the next edition of code & comments. Best Regards! xinlei
Hi, Xinlei: On Mon, 2022-04-11 at 10:31 +0800, xinlei.lee@mediatek.com wrote: > From: Jitao Shi <jitao.shi@mediatek.com> > > To comply with the panel sequence, hold the mipi signal to LP00 > before the dcs cmds transmission, > and pull the mipi signal high from LP00 to LP11 until the start of > the dcs cmds transmission. > The normal panel timing is : > (1) pp1800 DC pull up > (2) avdd & avee AC pull high > (3) lcm_reset pull high -> pull low -> pull high > (4) Pull MIPI signal high (LP11) -> initial code -> send video > data(HS mode) > The power-off sequence is reversed. > If dsi is not in cmd mode, then dsi will pull the mipi signal high in > the mtk_output_dsi_enable function. > > Fixes: 2dd8075d2185 ("drm/mediatek: mtk_dsi: Use the drm_panel_bridge > API") > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > Signed-off-by: Xinlei Lee <xinlei.lee@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index cf76c53a1af6..9ad6f08c8bfe 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -203,6 +203,7 @@ struct mtk_dsi { > struct mtk_phy_timing phy_timing; > int refcount; > bool enabled; > + bool lanes_ready; > u32 irq_data; > wait_queue_head_t irq_wait_queue; > const struct mtk_dsi_driver_data *driver_data; > @@ -654,13 +655,6 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) > mtk_dsi_config_vdo_timing(dsi); > mtk_dsi_set_interrupt_enable(dsi); > > - mtk_dsi_rxtx_control(dsi); > - usleep_range(30, 100); > - mtk_dsi_reset_dphy(dsi); > - mtk_dsi_clk_ulp_mode_leave(dsi); > - mtk_dsi_lane0_ulp_mode_leave(dsi); > - mtk_dsi_clk_hs_mode(dsi, 0); > - > return 0; > err_disable_engine_clk: > clk_disable_unprepare(dsi->engine_clk); > @@ -689,6 +683,23 @@ static void mtk_dsi_poweroff(struct mtk_dsi > *dsi) > clk_disable_unprepare(dsi->digital_clk); > > phy_power_off(dsi->phy); > + > + dsi->lanes_ready = false; > +} > + > +static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) > +{ > + if (!dsi->lanes_ready) { > + dsi->lanes_ready = true; > + mtk_dsi_rxtx_control(dsi); > + usleep_range(30, 100); > + mtk_dsi_reset_dphy(dsi); > + mtk_dsi_clk_ulp_mode_leave(dsi); > + mtk_dsi_lane0_ulp_mode_leave(dsi); > + mtk_dsi_clk_hs_mode(dsi, 0); > + msleep(20); > + } else > + DRM_DEBUG("The dsi_lane is ready\n"); > } > > static void mtk_output_dsi_enable(struct mtk_dsi *dsi) > @@ -696,6 +707,7 @@ static void mtk_output_dsi_enable(struct mtk_dsi > *dsi) > if (dsi->enabled) > return; > > + mtk_dsi_lane_ready(dsi); > mtk_dsi_set_mode(dsi); > mtk_dsi_clk_hs_mode(dsi, 1); > > @@ -1001,6 +1013,8 @@ static ssize_t mtk_dsi_host_transfer(struct > mipi_dsi_host *host, > if (MTK_DSI_HOST_IS_READ(msg->type)) > irq_flag |= LPRX_RD_RDY_INT_FLAG; > > + mtk_dsi_lane_ready(dsi); In [1], YT has move mtk_dsi_lane_ready() before panel prepare for dsi- >panel case. Now you move mtk_dsi_lane_ready() after panel prepare, this may break dsi->panel case. Please provide a solution for both case. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dsi.c?h=v5.18-rc2&id=0707632b5bacc490f58dfbad741d586c06595ff3 Regards, CK > + > ret = mtk_dsi_host_send_cmd(dsi, msg, irq_flag); > if (ret) > goto restore_dsi_mode;
On Wed, 2022-04-13 at 16:31 +0800, CK Hu wrote: > Hi, Xinlei: > > On Mon, 2022-04-11 at 10:31 +0800, xinlei.lee@mediatek.com wrote: > > From: Jitao Shi <jitao.shi@mediatek.com> > > > > To comply with the panel sequence, hold the mipi signal to LP00 > > before the dcs cmds transmission, > > and pull the mipi signal high from LP00 to LP11 until the start of > > the dcs cmds transmission. > > The normal panel timing is : > > (1) pp1800 DC pull up > > (2) avdd & avee AC pull high > > (3) lcm_reset pull high -> pull low -> pull high > > (4) Pull MIPI signal high (LP11) -> initial code -> send video > > data(HS mode) > > The power-off sequence is reversed. > > If dsi is not in cmd mode, then dsi will pull the mipi signal high > > in > > the mtk_output_dsi_enable function. > > > > Fixes: 2dd8075d2185 ("drm/mediatek: mtk_dsi: Use the > > drm_panel_bridge > > API") > > > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > > Signed-off-by: Xinlei Lee <xinlei.lee@mediatek.com> > > --- > > drivers/gpu/drm/mediatek/mtk_dsi.c | 28 +++++++++++++++++++++----- > > -- > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > > b/drivers/gpu/drm/mediatek/mtk_dsi.c > > index cf76c53a1af6..9ad6f08c8bfe 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > > @@ -203,6 +203,7 @@ struct mtk_dsi { > > struct mtk_phy_timing phy_timing; > > int refcount; > > bool enabled; > > + bool lanes_ready; > > u32 irq_data; > > wait_queue_head_t irq_wait_queue; > > const struct mtk_dsi_driver_data *driver_data; > > @@ -654,13 +655,6 @@ static int mtk_dsi_poweron(struct mtk_dsi > > *dsi) > > mtk_dsi_config_vdo_timing(dsi); > > mtk_dsi_set_interrupt_enable(dsi); > > > > - mtk_dsi_rxtx_control(dsi); > > - usleep_range(30, 100); > > - mtk_dsi_reset_dphy(dsi); > > - mtk_dsi_clk_ulp_mode_leave(dsi); > > - mtk_dsi_lane0_ulp_mode_leave(dsi); > > - mtk_dsi_clk_hs_mode(dsi, 0); > > - > > return 0; > > err_disable_engine_clk: > > clk_disable_unprepare(dsi->engine_clk); > > @@ -689,6 +683,23 @@ static void mtk_dsi_poweroff(struct mtk_dsi > > *dsi) > > clk_disable_unprepare(dsi->digital_clk); > > > > phy_power_off(dsi->phy); > > + > > + dsi->lanes_ready = false; > > +} > > + > > +static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) > > +{ > > + if (!dsi->lanes_ready) { > > + dsi->lanes_ready = true; > > + mtk_dsi_rxtx_control(dsi); > > + usleep_range(30, 100); > > + mtk_dsi_reset_dphy(dsi); > > + mtk_dsi_clk_ulp_mode_leave(dsi); > > + mtk_dsi_lane0_ulp_mode_leave(dsi); > > + mtk_dsi_clk_hs_mode(dsi, 0); > > + msleep(20); > > + } else > > + DRM_DEBUG("The dsi_lane is ready\n"); > > } > > > > static void mtk_output_dsi_enable(struct mtk_dsi *dsi) > > @@ -696,6 +707,7 @@ static void mtk_output_dsi_enable(struct > > mtk_dsi > > *dsi) > > if (dsi->enabled) > > return; > > > > + mtk_dsi_lane_ready(dsi); > > mtk_dsi_set_mode(dsi); > > mtk_dsi_clk_hs_mode(dsi, 1); > > > > @@ -1001,6 +1013,8 @@ static ssize_t mtk_dsi_host_transfer(struct > > mipi_dsi_host *host, > > if (MTK_DSI_HOST_IS_READ(msg->type)) > > irq_flag |= LPRX_RD_RDY_INT_FLAG; > > > > + mtk_dsi_lane_ready(dsi); > > In [1], YT has move mtk_dsi_lane_ready() before panel prepare for > dsi- > > panel case. Now you move mtk_dsi_lane_ready() after panel prepare, > > this may break dsi->panel case. Please provide a solution for both > case. > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dsi.c?h=v5.18-rc2&id=0707632b5bacc490f58dfbad741d586c06595ff3 > > Regards, > CK > > > + > > ret = mtk_dsi_host_send_cmd(dsi, msg, irq_flag); > > if (ret) > > goto restore_dsi_mode; > > Hi CK: Because the order of dsi->panel in [1] is as follows (tv101 panel as an example): 1. dsi_poweron (lane_ready) 2. panel_prepare 3. panel_prepare_power 4. panel_init_cmd 5. dsi_host_transfer (actually send panel initial code) This modified order: 1. dsi_poweron 2. panel_prepare 3. panel_prepare_power 4. panel_init_cmd 5. dsi_host_transfer (lane_ready) It can be seen that the lane_ready is delayed closer to before sending the initial code, which is necessary for some panels with stricter timing requirements. And if this screen does not need to send initial code, it will also do lane_ready in output_dsi_enable, so that dsi can complete LP00->LP11- >HS mode. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dsi.c?h=v5.18-rc2&id=0707632b5bacc490f58dfbad741d586c06595ff3 Best Regards! xinlei
Hi, Xinlei: On Fri, 2022-04-15 at 09:58 +0800, xinlei.lee wrote: > On Wed, 2022-04-13 at 16:31 +0800, CK Hu wrote: > > Hi, Xinlei: > > > > On Mon, 2022-04-11 at 10:31 +0800, xinlei.lee@mediatek.com wrote: > > > From: Jitao Shi <jitao.shi@mediatek.com> > > > > > > To comply with the panel sequence, hold the mipi signal to LP00 > > > before the dcs cmds transmission, > > > and pull the mipi signal high from LP00 to LP11 until the start > > > of > > > the dcs cmds transmission. > > > The normal panel timing is : > > > (1) pp1800 DC pull up > > > (2) avdd & avee AC pull high > > > (3) lcm_reset pull high -> pull low -> pull high > > > (4) Pull MIPI signal high (LP11) -> initial code -> send video > > > data(HS mode) > > > The power-off sequence is reversed. > > > If dsi is not in cmd mode, then dsi will pull the mipi signal > > > high > > > in > > > the mtk_output_dsi_enable function. > > > > > > Fixes: 2dd8075d2185 ("drm/mediatek: mtk_dsi: Use the > > > drm_panel_bridge > > > API") > > > > > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > > > Signed-off-by: Xinlei Lee <xinlei.lee@mediatek.com> > > > --- > > > drivers/gpu/drm/mediatek/mtk_dsi.c | 28 +++++++++++++++++++++--- > > > -- > > > -- > > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > > > b/drivers/gpu/drm/mediatek/mtk_dsi.c > > > index cf76c53a1af6..9ad6f08c8bfe 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > > > @@ -203,6 +203,7 @@ struct mtk_dsi { > > > struct mtk_phy_timing phy_timing; > > > int refcount; > > > bool enabled; > > > + bool lanes_ready; > > > u32 irq_data; > > > wait_queue_head_t irq_wait_queue; > > > const struct mtk_dsi_driver_data *driver_data; > > > @@ -654,13 +655,6 @@ static int mtk_dsi_poweron(struct mtk_dsi > > > *dsi) > > > mtk_dsi_config_vdo_timing(dsi); > > > mtk_dsi_set_interrupt_enable(dsi); > > > > > > - mtk_dsi_rxtx_control(dsi); > > > - usleep_range(30, 100); > > > - mtk_dsi_reset_dphy(dsi); > > > - mtk_dsi_clk_ulp_mode_leave(dsi); > > > - mtk_dsi_lane0_ulp_mode_leave(dsi); > > > - mtk_dsi_clk_hs_mode(dsi, 0); > > > - > > > return 0; > > > err_disable_engine_clk: > > > clk_disable_unprepare(dsi->engine_clk); > > > @@ -689,6 +683,23 @@ static void mtk_dsi_poweroff(struct mtk_dsi > > > *dsi) > > > clk_disable_unprepare(dsi->digital_clk); > > > > > > phy_power_off(dsi->phy); > > > + > > > + dsi->lanes_ready = false; > > > +} > > > + > > > +static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) > > > +{ > > > + if (!dsi->lanes_ready) { > > > + dsi->lanes_ready = true; > > > + mtk_dsi_rxtx_control(dsi); > > > + usleep_range(30, 100); > > > + mtk_dsi_reset_dphy(dsi); > > > + mtk_dsi_clk_ulp_mode_leave(dsi); > > > + mtk_dsi_lane0_ulp_mode_leave(dsi); > > > + mtk_dsi_clk_hs_mode(dsi, 0); > > > + msleep(20); > > > + } else > > > + DRM_DEBUG("The dsi_lane is ready\n"); Once mtk_dsi_host_transfer() call this function first, then mtk_output_dsi_enable() would call again. This function would be called twice in normal sequence, so I think this debug information is not necessary. > > > } > > > > > > static void mtk_output_dsi_enable(struct mtk_dsi *dsi) > > > @@ -696,6 +707,7 @@ static void mtk_output_dsi_enable(struct > > > mtk_dsi > > > *dsi) > > > if (dsi->enabled) > > > return; > > > > > > + mtk_dsi_lane_ready(dsi); > > > mtk_dsi_set_mode(dsi); > > > mtk_dsi_clk_hs_mode(dsi, 1); > > > > > > @@ -1001,6 +1013,8 @@ static ssize_t mtk_dsi_host_transfer(struct > > > mipi_dsi_host *host, > > > if (MTK_DSI_HOST_IS_READ(msg->type)) > > > irq_flag |= LPRX_RD_RDY_INT_FLAG; > > > > > > + mtk_dsi_lane_ready(dsi); > > > > In [1], YT has move mtk_dsi_lane_ready() before panel prepare for > > dsi- > > > panel case. Now you move mtk_dsi_lane_ready() after panel > > > prepare, > > > > this may break dsi->panel case. Please provide a solution for both > > case. > > > > [1] > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dsi.c?h=v5.18-rc2&id=0707632b5bacc490f58dfbad741d586c06595ff3 > > > > Regards, > > CK > > > > > + > > > ret = mtk_dsi_host_send_cmd(dsi, msg, irq_flag); > > > if (ret) > > > goto restore_dsi_mode; > > > > > > Hi CK: > > Because the order of dsi->panel in [1] is as follows (tv101 panel as > an > example): > 1. dsi_poweron (lane_ready) > 2. panel_prepare > 3. panel_prepare_power > 4. panel_init_cmd > 5. dsi_host_transfer (actually send panel initial code) > > This modified order: > 1. dsi_poweron > 2. panel_prepare > 3. panel_prepare_power > 4. panel_init_cmd > 5. dsi_host_transfer (lane_ready) > > It can be seen that the lane_ready is delayed closer to before > sending > the initial code, which is necessary for some panels with stricter > timing requirements. > And if this screen does not need to send initial code, it will also > do > lane_ready in output_dsi_enable, so that dsi can complete LP00->LP11- > > HS mode. Understand. You call mtk_dsi_lane_ready() in both mtk_output_dsi_enable() and mtk_dsi_host_transfer(). The panel prepare may call mtk_dsi_host_transfer() and mtk_dsi_lane_ready() would be called. Regards, CK > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dsi.c?h=v5.18-rc2&id=0707632b5bacc490f58dfbad741d586c06595ff3 > > Best Regards! > xinlei >
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index cf76c53a1af6..9ad6f08c8bfe 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -203,6 +203,7 @@ struct mtk_dsi { struct mtk_phy_timing phy_timing; int refcount; bool enabled; + bool lanes_ready; u32 irq_data; wait_queue_head_t irq_wait_queue; const struct mtk_dsi_driver_data *driver_data; @@ -654,13 +655,6 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) mtk_dsi_config_vdo_timing(dsi); mtk_dsi_set_interrupt_enable(dsi); - mtk_dsi_rxtx_control(dsi); - usleep_range(30, 100); - mtk_dsi_reset_dphy(dsi); - mtk_dsi_clk_ulp_mode_leave(dsi); - mtk_dsi_lane0_ulp_mode_leave(dsi); - mtk_dsi_clk_hs_mode(dsi, 0); - return 0; err_disable_engine_clk: clk_disable_unprepare(dsi->engine_clk); @@ -689,6 +683,23 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) clk_disable_unprepare(dsi->digital_clk); phy_power_off(dsi->phy); + + dsi->lanes_ready = false; +} + +static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) +{ + if (!dsi->lanes_ready) { + dsi->lanes_ready = true; + mtk_dsi_rxtx_control(dsi); + usleep_range(30, 100); + mtk_dsi_reset_dphy(dsi); + mtk_dsi_clk_ulp_mode_leave(dsi); + mtk_dsi_lane0_ulp_mode_leave(dsi); + mtk_dsi_clk_hs_mode(dsi, 0); + msleep(20); + } else + DRM_DEBUG("The dsi_lane is ready\n"); } static void mtk_output_dsi_enable(struct mtk_dsi *dsi) @@ -696,6 +707,7 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi) if (dsi->enabled) return; + mtk_dsi_lane_ready(dsi); mtk_dsi_set_mode(dsi); mtk_dsi_clk_hs_mode(dsi, 1); @@ -1001,6 +1013,8 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host, if (MTK_DSI_HOST_IS_READ(msg->type)) irq_flag |= LPRX_RD_RDY_INT_FLAG; + mtk_dsi_lane_ready(dsi); + ret = mtk_dsi_host_send_cmd(dsi, msg, irq_flag); if (ret) goto restore_dsi_mode;