| Message ID | 1647503611-13144-2-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 |
Hello Xinlei, Thanks for your patch, and there are something I want to know: On Thu, 2022-03-17 at 15:53 +0800, xinlei.lee@mediatek.com wrote: > From: Jitao Shi <jitao.shi@mediatek.com> > > Old sequence: > 1. Pull the MIPI signal high > 2. Delay & Dsi_reset > 3. Set the dsi timing register > 4. dsi clk & lanes leave ulp mode and enter hs mode > > New sequence: > 1. Set the dsi timing register > 2. Pull the MIPI signal high > 3. Delay & Dsi_reset > 4. dsi clk & lanes leave ulp mode and enter hs mode > Could you explain why you want to change original power on sequence? From this commit message, I don't know why you want to do this. If the reason is in cover letter, I think you should move them here. > In the new sequence 2 & 3 & 4 will be moved to dsi_enbale in later > patch. I think this is no need to describe here. BRs, Rex > > 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 | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index ccb0511b9cd5..262c027d8c2f 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -649,14 +649,14 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) > mtk_dsi_reset_engine(dsi); > mtk_dsi_phy_timconfig(dsi); > > - mtk_dsi_rxtx_control(dsi); > - usleep_range(30, 100); > - mtk_dsi_reset_dphy(dsi); > mtk_dsi_ps_control_vact(dsi); > mtk_dsi_set_vm_cmd(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);
Hi, Xinlei: On Thu, 2022-03-17 at 15:53 +0800, xinlei.lee@mediatek.com wrote: > From: Jitao Shi <jitao.shi@mediatek.com> > > Old sequence: > 1. Pull the MIPI signal high > 2. Delay & Dsi_reset > 3. Set the dsi timing register > 4. dsi clk & lanes leave ulp mode and enter hs mode > > New sequence: > 1. Set the dsi timing register > 2. Pull the MIPI signal high > 3. Delay & Dsi_reset > 4. dsi clk & lanes leave ulp mode and enter hs mode > > In the new sequence 2 & 3 & 4 will be moved to dsi_enbale in later > patch. I think there would be one patch in 5.9 make the wrong sequence, so add 'Fixes' tag to indicate which patch make the wrong sequence. Use the term correct/wrong instead old/new sequence. I still do not understand what is the sequence after apply this patch? Does the sequence is this after apply this patch? 1. Set the dsi timing register 2. Pull the MIPI signal high 3. Delay & Dsi_reset 4. dsi clk & lanes leave ulp mode and enter hs mode Regards, CK > > 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 | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index ccb0511b9cd5..262c027d8c2f 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -649,14 +649,14 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) > mtk_dsi_reset_engine(dsi); > mtk_dsi_phy_timconfig(dsi); > > - mtk_dsi_rxtx_control(dsi); > - usleep_range(30, 100); > - mtk_dsi_reset_dphy(dsi); > mtk_dsi_ps_control_vact(dsi); > mtk_dsi_set_vm_cmd(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);
On Thu, 2022-03-17 at 19:13 +0800, Rex-BC Chen wrote: > Hello Xinlei, > > Thanks for your patch, and there are something I want to know: > > On Thu, 2022-03-17 at 15:53 +0800, xinlei.lee@mediatek.com wrote: > > From: Jitao Shi <jitao.shi@mediatek.com> > > > > Old sequence: > > 1. Pull the MIPI signal high > > 2. Delay & Dsi_reset > > 3. Set the dsi timing register > > 4. dsi clk & lanes leave ulp mode and enter hs mode > > > > New sequence: > > 1. Set the dsi timing register > > 2. Pull the MIPI signal high > > 3. Delay & Dsi_reset > > 4. dsi clk & lanes leave ulp mode and enter hs mode > > > > Could you explain why you want to change original power on sequence? > From this commit message, I don't know why you want to do this. > If the reason is in cover letter, I think you should move them here. > > > In the new sequence 2 & 3 & 4 will be moved to dsi_enbale in later > > patch. > > I think this is no need to describe here. > > BRs, > Rex > > > > 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 | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > > b/drivers/gpu/drm/mediatek/mtk_dsi.c > > index ccb0511b9cd5..262c027d8c2f 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > > @@ -649,14 +649,14 @@ static int mtk_dsi_poweron(struct mtk_dsi > > *dsi) > > mtk_dsi_reset_engine(dsi); > > mtk_dsi_phy_timconfig(dsi); > > > > - mtk_dsi_rxtx_control(dsi); > > - usleep_range(30, 100); > > - mtk_dsi_reset_dphy(dsi); > > mtk_dsi_ps_control_vact(dsi); > > mtk_dsi_set_vm_cmd(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); > > Hi Rex: Thanks for your review! Explain to you that this change does not actually affect the function of dsi. We have verified this. After this modification, it is convenient to move them into the if (!dsi->lanes_ready) condition, which is just an adjustment action. If you still have questions about my later explanation, please give another suggestion. Thanks! xinlei
On Mon, 2022-03-21 at 17:36 +0800, CK Hu wrote: > Hi, Xinlei: > > On Thu, 2022-03-17 at 15:53 +0800, xinlei.lee@mediatek.com wrote: > > From: Jitao Shi <jitao.shi@mediatek.com> > > > > Old sequence: > > 1. Pull the MIPI signal high > > 2. Delay & Dsi_reset > > 3. Set the dsi timing register > > 4. dsi clk & lanes leave ulp mode and enter hs mode > > > > New sequence: > > 1. Set the dsi timing register > > 2. Pull the MIPI signal high > > 3. Delay & Dsi_reset > > 4. dsi clk & lanes leave ulp mode and enter hs mode > > > > In the new sequence 2 & 3 & 4 will be moved to dsi_enbale in later > > patch. > > I think there would be one patch in 5.9 make the wrong sequence, so > add > 'Fixes' tag to indicate which patch make the wrong sequence. Use the > term correct/wrong instead old/new sequence. > > I still do not understand what is the sequence after apply this > patch? > > Does the sequence is this after apply this patch? > 1. Set the dsi timing register > 2. Pull the MIPI signal high > 3. Delay & Dsi_reset > 4. dsi clk & lanes leave ulp mode and enter hs mode > > Regards, > CK > > > > > 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 | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > > b/drivers/gpu/drm/mediatek/mtk_dsi.c > > index ccb0511b9cd5..262c027d8c2f 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > > @@ -649,14 +649,14 @@ static int mtk_dsi_poweron(struct mtk_dsi > > *dsi) > > mtk_dsi_reset_engine(dsi); > > mtk_dsi_phy_timconfig(dsi); > > > > - mtk_dsi_rxtx_control(dsi); > > - usleep_range(30, 100); > > - mtk_dsi_reset_dphy(dsi); > > mtk_dsi_ps_control_vact(dsi); > > mtk_dsi_set_vm_cmd(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); > > Hi CK: Thanks for your review! You are right, the sequence after patching is: 1. Set the dsi timing register 2. Pull the MIPI signal high 3. Delay & Dsi_reset 4. dsi clk & lanes leave ulp mode and enter hs mode This modification will not affect the dsi function, just to put the operation of pulling up the mipi signal in poweron together to facilitate the separation from the poweron function later. I will add the "Fixes" tag here as well. BR! xinlei
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index ccb0511b9cd5..262c027d8c2f 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -649,14 +649,14 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) mtk_dsi_reset_engine(dsi); mtk_dsi_phy_timconfig(dsi); - mtk_dsi_rxtx_control(dsi); - usleep_range(30, 100); - mtk_dsi_reset_dphy(dsi); mtk_dsi_ps_control_vact(dsi); mtk_dsi_set_vm_cmd(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);