| Message ID | 20220804194325.1596554-1-nfraprado@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/mediatek: dsi: Move mtk_dsi_stop() call back to mtk_dsi_poweroff() | expand |
On Mon, Sep 19, 2022 at 4:39 PM Nícolas F. R. A. Prado <nfraprado@collabora.com> wrote: > > As the comment right before the mtk_dsi_stop() call advises, > mtk_dsi_stop() should only be called after > mtk_drm_crtc_atomic_disable(). That's because that function calls > drm_crtc_wait_one_vblank(), which requires the vblank irq to be enabled. > > Previously mtk_dsi_stop(), being in mtk_dsi_poweroff() and guarded by a > refcount, would only be called at the end of > mtk_drm_crtc_atomic_disable(), through the call to mtk_crtc_ddp_hw_fini(). > Commit cde7e2e35c28 ("drm/mediatek: Separate poweron/poweroff from > enable/disable and define new funcs") moved the mtk_dsi_stop() call to > mtk_output_dsi_disable(), causing it to be called before > mtk_drm_crtc_atomic_disable(), and consequently generating vblank > timeout warnings during suspend. > > Move the mtk_dsi_stop() call back to mtk_dsi_poweroff() so that we have > a working vblank irq during mtk_drm_crtc_atomic_disable() and stop > getting vblank timeout warnings. > > Fixes: cde7e2e35c28 ("drm/mediatek: Separate poweron/poweroff from enable/disable and define new funcs") > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > Tested-by: Hsin-Yi Wang <hsinyi@chromium.org> Tested on mt8183 jacuzzi and mt8186 that this patch fixes the vblank warning. > --- > > drivers/gpu/drm/mediatek/mtk_dsi.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 9cc406e1eee1..f8ad59771551 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -685,6 +685,16 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) > if (--dsi->refcount != 0) > return; > > + /* > + * mtk_dsi_stop() and mtk_dsi_start() is asymmetric, since > + * mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(), > + * which needs irq for vblank, and mtk_dsi_stop() will disable irq. > + * mtk_dsi_start() needs to be called in mtk_output_dsi_enable(), > + * after dsi is fully set. > + */ > + mtk_dsi_stop(dsi); > + > + mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); > mtk_dsi_reset_engine(dsi); > mtk_dsi_lane0_ulp_mode_enter(dsi); > mtk_dsi_clk_ulp_mode_enter(dsi); > @@ -735,17 +745,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi) > if (!dsi->enabled) > return; > > - /* > - * mtk_dsi_stop() and mtk_dsi_start() is asymmetric, since > - * mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(), > - * which needs irq for vblank, and mtk_dsi_stop() will disable irq. > - * mtk_dsi_start() needs to be called in mtk_output_dsi_enable(), > - * after dsi is fully set. > - */ > - mtk_dsi_stop(dsi); > - > - mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); > - > dsi->enabled = false; > } > > -- > 2.37.1 >
Il 19/09/22 10:40, Hsin-Yi Wang ha scritto: > On Mon, Sep 19, 2022 at 4:39 PM Nícolas F. R. A. Prado > <nfraprado@collabora.com> wrote: >> >> As the comment right before the mtk_dsi_stop() call advises, >> mtk_dsi_stop() should only be called after >> mtk_drm_crtc_atomic_disable(). That's because that function calls >> drm_crtc_wait_one_vblank(), which requires the vblank irq to be enabled. >> >> Previously mtk_dsi_stop(), being in mtk_dsi_poweroff() and guarded by a >> refcount, would only be called at the end of >> mtk_drm_crtc_atomic_disable(), through the call to mtk_crtc_ddp_hw_fini(). >> Commit cde7e2e35c28 ("drm/mediatek: Separate poweron/poweroff from >> enable/disable and define new funcs") moved the mtk_dsi_stop() call to >> mtk_output_dsi_disable(), causing it to be called before >> mtk_drm_crtc_atomic_disable(), and consequently generating vblank >> timeout warnings during suspend. >> >> Move the mtk_dsi_stop() call back to mtk_dsi_poweroff() so that we have >> a working vblank irq during mtk_drm_crtc_atomic_disable() and stop >> getting vblank timeout warnings. >> >> Fixes: cde7e2e35c28 ("drm/mediatek: Separate poweron/poweroff from enable/disable and define new funcs") >> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> >> > Tested-by: Hsin-Yi Wang <hsinyi@chromium.org> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
On 9/19/22 21:32, AngeloGioacchino Del Regno wrote: > Il 19/09/22 10:40, Hsin-Yi Wang ha scritto: >> On Mon, Sep 19, 2022 at 4:39 PM Nícolas F. R. A. Prado >> <nfraprado@collabora.com> wrote: >>> >>> As the comment right before the mtk_dsi_stop() call advises, >>> mtk_dsi_stop() should only be called after >>> mtk_drm_crtc_atomic_disable(). That's because that function calls >>> drm_crtc_wait_one_vblank(), which requires the vblank irq to be enabled. >>> >>> Previously mtk_dsi_stop(), being in mtk_dsi_poweroff() and guarded by a >>> refcount, would only be called at the end of >>> mtk_drm_crtc_atomic_disable(), through the call to >>> mtk_crtc_ddp_hw_fini(). >>> Commit cde7e2e35c28 ("drm/mediatek: Separate poweron/poweroff from >>> enable/disable and define new funcs") moved the mtk_dsi_stop() call to >>> mtk_output_dsi_disable(), causing it to be called before >>> mtk_drm_crtc_atomic_disable(), and consequently generating vblank >>> timeout warnings during suspend. >>> >>> Move the mtk_dsi_stop() call back to mtk_dsi_poweroff() so that we have >>> a working vblank irq during mtk_drm_crtc_atomic_disable() and stop >>> getting vblank timeout warnings. >>> >>> Fixes: cde7e2e35c28 ("drm/mediatek: Separate poweron/poweroff from >>> enable/disable and define new funcs") >>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> >>> >> Tested-by: Hsin-Yi Wang <hsinyi@chromium.org> >> > > Reviewed-by: AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> > > Tested suspend/resume work properly on mt8188 and mt8186 . Tested-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 9cc406e1eee1..f8ad59771551 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -685,6 +685,16 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) if (--dsi->refcount != 0) return; + /* + * mtk_dsi_stop() and mtk_dsi_start() is asymmetric, since + * mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(), + * which needs irq for vblank, and mtk_dsi_stop() will disable irq. + * mtk_dsi_start() needs to be called in mtk_output_dsi_enable(), + * after dsi is fully set. + */ + mtk_dsi_stop(dsi); + + mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); mtk_dsi_reset_engine(dsi); mtk_dsi_lane0_ulp_mode_enter(dsi); mtk_dsi_clk_ulp_mode_enter(dsi); @@ -735,17 +745,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi) if (!dsi->enabled) return; - /* - * mtk_dsi_stop() and mtk_dsi_start() is asymmetric, since - * mtk_dsi_stop() should be called after mtk_drm_crtc_atomic_disable(), - * which needs irq for vblank, and mtk_dsi_stop() will disable irq. - * mtk_dsi_start() needs to be called in mtk_output_dsi_enable(), - * after dsi is fully set. - */ - mtk_dsi_stop(dsi); - - mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); - dsi->enabled = false; }
As the comment right before the mtk_dsi_stop() call advises, mtk_dsi_stop() should only be called after mtk_drm_crtc_atomic_disable(). That's because that function calls drm_crtc_wait_one_vblank(), which requires the vblank irq to be enabled. Previously mtk_dsi_stop(), being in mtk_dsi_poweroff() and guarded by a refcount, would only be called at the end of mtk_drm_crtc_atomic_disable(), through the call to mtk_crtc_ddp_hw_fini(). Commit cde7e2e35c28 ("drm/mediatek: Separate poweron/poweroff from enable/disable and define new funcs") moved the mtk_dsi_stop() call to mtk_output_dsi_disable(), causing it to be called before mtk_drm_crtc_atomic_disable(), and consequently generating vblank timeout warnings during suspend. Move the mtk_dsi_stop() call back to mtk_dsi_poweroff() so that we have a working vblank irq during mtk_drm_crtc_atomic_disable() and stop getting vblank timeout warnings. Fixes: cde7e2e35c28 ("drm/mediatek: Separate poweron/poweroff from enable/disable and define new funcs") Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- drivers/gpu/drm/mediatek/mtk_dsi.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)