LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v3] gpu/drm: mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable()
@ 2019-05-28  7:39 Hsin-Yi Wang
  2019-05-28  8:53 ` CK Hu
  0 siblings, 1 reply; 4+ messages in thread
From: Hsin-Yi Wang @ 2019-05-28  7:39 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: CK Hu, Philipp Zabel, David Airlie, Daniel Vetter,
	Matthias Brugger, dri-devel, linux-mediatek, linux-kernel

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(-)

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;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] gpu/drm: mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable()
  2019-05-28  7:39 [PATCH v3] gpu/drm: mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable() Hsin-Yi Wang
@ 2019-05-28  8:53 ` CK Hu
  2019-05-30  2:55   ` Hsin-Yi Wang
  0 siblings, 1 reply; 4+ messages in thread
From: CK Hu @ 2019-05-28  8:53 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: linux-arm-kernel, Philipp Zabel, David Airlie, linux-kernel,
	dri-devel, Matthias Brugger, linux-mediatek, Daniel Vetter

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;



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] gpu/drm: mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable()
  2019-05-28  8:53 ` CK Hu
@ 2019-05-30  2:55   ` Hsin-Yi Wang
  2019-05-30  7:10     ` CK Hu
  0 siblings, 1 reply; 4+ messages in thread
From: Hsin-Yi Wang @ 2019-05-30  2:55 UTC (permalink / raw)
  To: CK Hu
  Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Philipp Zabel, David Airlie, lkml, dri-devel, Matthias Brugger,
	linux-mediatek, Daniel Vetter

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] gpu/drm: mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable()
  2019-05-30  2:55   ` Hsin-Yi Wang
@ 2019-05-30  7:10     ` CK Hu
  0 siblings, 0 replies; 4+ messages in thread
From: CK Hu @ 2019-05-30  7:10 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Philipp Zabel, David Airlie, lkml, dri-devel, Matthias Brugger,
	linux-mediatek, Daniel Vetter

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-05-30  7:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-28  7:39 [PATCH v3] gpu/drm: mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable() Hsin-Yi Wang
2019-05-28  8:53 ` CK Hu
2019-05-30  2:55   ` Hsin-Yi Wang
2019-05-30  7:10     ` CK Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).