diff mbox series

[v3,4/4] drm/mediatek: Add pull-down MIPI operation in mtk_dsi_poweroff function

Message ID 1647503611-13144-5-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

Commit Message

Xinlei Lee (李昕磊) March 17, 2022, 7:53 a.m. UTC
From: Xinlei Lee <xinlei.lee@mediatek.com>

In the dsi_enable function, mtk_dsi_rxtx_control is to
pull up the MIPI signal operation. Before dsi_disable,
MIPI should also be pulled down by writing a register instead of disabling dsi.

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 | 2 ++
 1 file changed, 2 insertions(+)

Comments

Rex-BC Chen (陳柏辰) March 17, 2022, 12:20 p.m. UTC | #1
Hello Xinlei,

On Thu, 2022-03-17 at 15:53 +0800, xinlei.lee@mediatek.com wrote:
> From: Xinlei Lee <xinlei.lee@mediatek.com>
> 
> In the dsi_enable function, mtk_dsi_rxtx_control is to
> pull up the MIPI signal operation. Before dsi_disable,
> MIPI should also be pulled down by writing a register instead of
> disabling dsi.
> 

What will happen if you do not pulled down the mipi before disable dsi?
What's differnet for this two setting?

> 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 | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index b509d59235e2..1c6a75a46b67 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -676,6 +676,8 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
>  	mtk_dsi_reset_engine(dsi);
>  	mtk_dsi_lane0_ulp_mode_enter(dsi);
>  	mtk_dsi_clk_ulp_mode_enter(dsi);
> +	/* set the lane number as 0 */
> +	writel(0, dsi->regs + DSI_TXRX_CTRL);

So set lane num to 0 means pull down mipi?

BRs,
Rex

>  
>  	mtk_dsi_disable(dsi);
>
Xinlei Lee (李昕磊) March 22, 2022, 10 a.m. UTC | #2
On Thu, 2022-03-17 at 20:20 +0800, Rex-BC Chen wrote:
> Hello Xinlei,
> 
> On Thu, 2022-03-17 at 15:53 +0800, xinlei.lee@mediatek.com wrote:
> > From: Xinlei Lee <xinlei.lee@mediatek.com>
> > 
> > In the dsi_enable function, mtk_dsi_rxtx_control is to
> > pull up the MIPI signal operation. Before dsi_disable,
> > MIPI should also be pulled down by writing a register instead of
> > disabling dsi.
> > 
> 
> What will happen if you do not pulled down the mipi before disable
> dsi?
> What's differnet for this two setting?
> 
> > 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 | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index b509d59235e2..1c6a75a46b67 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -676,6 +676,8 @@ static void mtk_dsi_poweroff(struct mtk_dsi
> > *dsi)
> >  	mtk_dsi_reset_engine(dsi);
> >  	mtk_dsi_lane0_ulp_mode_enter(dsi);
> >  	mtk_dsi_clk_ulp_mode_enter(dsi);
> > +	/* set the lane number as 0 */
> > +	writel(0, dsi->regs + DSI_TXRX_CTRL);
> 
> So set lane num to 0 means pull down mipi?
> 
> BRs,
> Rex
> 
> >  
> >  	mtk_dsi_disable(dsi);
> >  
> 
> 

Hi rex:

1. 
If you disable dsi without pulling the mipi signal low, the value of
the register will still maintain the setting of the mipi signal being
pulled high. 
After resume, even if the mipi signal is not pulled high, it will still
be in the high state.

2.So set lane num to 0 means pull down mipi
=> yes

Do you have any suggestions on the next version?

Best Regards!
xinlei
Rex-BC Chen (陳柏辰) March 23, 2022, 11:54 a.m. UTC | #3
On Tue, 2022-03-22 at 18:00 +0800, xinlei.lee wrote:
> On Thu, 2022-03-17 at 20:20 +0800, Rex-BC Chen wrote:
> > Hello Xinlei,
> > 
> > On Thu, 2022-03-17 at 15:53 +0800, xinlei.lee@mediatek.com wrote:
> > > From: Xinlei Lee <xinlei.lee@mediatek.com>
> > > 
> > > In the dsi_enable function, mtk_dsi_rxtx_control is to
> > > pull up the MIPI signal operation. Before dsi_disable,
> > > MIPI should also be pulled down by writing a register instead of
> > > disabling dsi.
> > > 
> > 
> > What will happen if you do not pulled down the mipi before disable
> > dsi?
> > What's differnet for this two setting?
> > 
> > > 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 | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > index b509d59235e2..1c6a75a46b67 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > @@ -676,6 +676,8 @@ static void mtk_dsi_poweroff(struct mtk_dsi
> > > *dsi)
> > >  	mtk_dsi_reset_engine(dsi);
> > >  	mtk_dsi_lane0_ulp_mode_enter(dsi);
> > >  	mtk_dsi_clk_ulp_mode_enter(dsi);
> > > +	/* set the lane number as 0 */

Hello Xinlei,

I think you can write this comment more detailed, like
"/* set the lane number as 0 to pull down mipi */"

> > > +	writel(0, dsi->regs + DSI_TXRX_CTRL);
> > 
> > So set lane num to 0 means pull down mipi?
> > 
> > BRs,
> > Rex
> > 
> > >  
> > >  	mtk_dsi_disable(dsi);
> > >  
> > 
> > 
> 
> Hi rex:
> 
> 1. 
> If you disable dsi without pulling the mipi signal low, the value of
> the register will still maintain the setting of the mipi signal being
> pulled high. 
> After resume, even if the mipi signal is not pulled high, it will
> still
> be in the high state.
> 

I think you can describe this in commit message


BRs,
Rex

> 2.So set lane num to 0 means pull down mipi
> => yes
> 
> Do you have any suggestions on the next version?
> 
> Best Regards!
> xinlei
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index b509d59235e2..1c6a75a46b67 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -676,6 +676,8 @@  static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
 	mtk_dsi_reset_engine(dsi);
 	mtk_dsi_lane0_ulp_mode_enter(dsi);
 	mtk_dsi_clk_ulp_mode_enter(dsi);
+	/* set the lane number as 0 */
+	writel(0, dsi->regs + DSI_TXRX_CTRL);
 
 	mtk_dsi_disable(dsi);