| Message ID | 20190528073908.633-1-hsinyi@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] gpu/drm: mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable() | expand |
Hi, Hsin-Yi: On Tue, 2019-05-28 at 15:39 +0800, Hsin-Yi Wang wrote: > mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(), which needs > ovl irq for drm_crtc_wait_one_vblank(), since after mtk_dsi_stop() is called, > ovl irq will be disabled. If drm_crtc_wait_one_vblank() is called after last > irq, it will timeout with this message: "vblank wait timed out on crtc 0". This > happens sometimes when turning off the screen. > > In drm_atomic_helper.c#disable_outputs(), > the calling sequence when turning off the screen is: > > 1. mtk_dsi_encoder_disable() > --> mtk_output_dsi_disable() > --> mtk_dsi_stop(); // sometimes make vblank timeout in atomic_disable > --> mtk_dsi_poweroff(); > 2. mtk_drm_crtc_atomic_disable() > --> drm_crtc_wait_one_vblank(); > ... > --> mtk_dsi_ddp_stop() > --> mtk_dsi_poweroff(); > > mtk_dsi_poweroff() has reference count design, change to make mtk_dsi_stop() > called in mtk_dsi_poweroff() when refcount is 0. > > Fixes: 0707632b5bac ("drm/mediatek: update DSI sub driver flow for sending commands to panel") > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> > --- > change log v2->v3: > * remove unnecessary codes in unbind > * based on discussion in v2, if we move mtk_dsi_start() to mtk_dsi_poweron(), > in order to make mtk_dsi_start() and mtk_dsi_stop() symmetric, will results in > no irq for panel with bridge. So we keep mtk_dsi_start() in original place. I think we've already discussed in [1]. I need a reason to understand this is hardware behavior or software bug. If this is a software bug, we need to fix the bug and code could be symmetric. [1] http://lists.infradead.org/pipermail/linux-mediatek/2019-March/018423.html Regards, CK > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > index b00eb2d2e086..b7f829ecd3ad 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -630,6 +630,8 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) > if (--dsi->refcount != 0) > return; > > + mtk_dsi_stop(dsi); > + > if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) { > if (dsi->panel) { > if (drm_panel_unprepare(dsi->panel)) { > @@ -696,7 +698,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi) > } > } > > - mtk_dsi_stop(dsi); > mtk_dsi_poweroff(dsi); > > dsi->enabled = false;
On Tue, May 28, 2019 at 4:53 PM CK Hu <ck.hu@mediatek.com> wrote: > I think we've already discussed in [1]. I need a reason to understand > this is hardware behavior or software bug. If this is a software bug, we > need to fix the bug and code could be symmetric. > > [1] > http://lists.infradead.org/pipermail/linux-mediatek/2019-March/018423.html > Hi CK, Jitao has replied in v2[1] " mtk_dsi_start must after dsi full setting. If you put it in mtk_dsi_ddp_start, mtk_dsi_set_mode won't work. DSI will keep cmd mode. So you see no irq. ... " [1] https://lore.kernel.org/patchwork/patch/1052505/#1276270 Thanks
Hi, Hsin-Yi: On Thu, 2019-05-30 at 10:55 +0800, Hsin-Yi Wang wrote: > On Tue, May 28, 2019 at 4:53 PM CK Hu <ck.hu@mediatek.com> wrote: > > > I think we've already discussed in [1]. I need a reason to understand > > this is hardware behavior or software bug. If this is a software bug, we > > need to fix the bug and code could be symmetric. > > > > [1] > > http://lists.infradead.org/pipermail/linux-mediatek/2019-March/018423.html > > > Hi CK, > > Jitao has replied in v2[1] > " > mtk_dsi_start must after dsi full setting. > If you put it in mtk_dsi_ddp_start, mtk_dsi_set_mode won't work. DSI > will keep cmd mode. So you see no irq. > ... > " > > [1] https://lore.kernel.org/patchwork/patch/1052505/#1276270 > > Thanks OK, this looks the hardware behavior, so I would like you to add comment in the code to describe why we need this asymmetric behavior. The description should be clear so that we could know how to modify the code flow in future. Regards, CK
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index b00eb2d2e086..b7f829ecd3ad 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -630,6 +630,8 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) if (--dsi->refcount != 0) return; + mtk_dsi_stop(dsi); + if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) { if (dsi->panel) { if (drm_panel_unprepare(dsi->panel)) { @@ -696,7 +698,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi) } } - mtk_dsi_stop(dsi); mtk_dsi_poweroff(dsi); dsi->enabled = false;
mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(), which needs ovl irq for drm_crtc_wait_one_vblank(), since after mtk_dsi_stop() is called, ovl irq will be disabled. If drm_crtc_wait_one_vblank() is called after last irq, it will timeout with this message: "vblank wait timed out on crtc 0". This happens sometimes when turning off the screen. In drm_atomic_helper.c#disable_outputs(), the calling sequence when turning off the screen is: 1. mtk_dsi_encoder_disable() --> mtk_output_dsi_disable() --> mtk_dsi_stop(); // sometimes make vblank timeout in atomic_disable --> mtk_dsi_poweroff(); 2. mtk_drm_crtc_atomic_disable() --> drm_crtc_wait_one_vblank(); ... --> mtk_dsi_ddp_stop() --> mtk_dsi_poweroff(); mtk_dsi_poweroff() has reference count design, change to make mtk_dsi_stop() called in mtk_dsi_poweroff() when refcount is 0. Fixes: 0707632b5bac ("drm/mediatek: update DSI sub driver flow for sending commands to panel") Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> --- change log v2->v3: * remove unnecessary codes in unbind * based on discussion in v2, if we move mtk_dsi_start() to mtk_dsi_poweron(), in order to make mtk_dsi_start() and mtk_dsi_stop() symmetric, will results in no irq for panel with bridge. So we keep mtk_dsi_start() in original place. --- drivers/gpu/drm/mediatek/mtk_dsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)