drivers/gpu/drm/mediatek/mtk_dsi.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
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(-)
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
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>
© 2016 - 2024 Red Hat, Inc.