* [PATCH] drm: imx: Fix occasional screen corruption on modeset.
@ 2020-07-07 15:56 ` Martin Fuzzey
0 siblings, 0 replies; 6+ messages in thread
From: Martin Fuzzey @ 2020-07-07 15:56 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Shawn Guo, Sascha Hauer, dri-devel, linux-kernel
When performing a modeset the atomic core calls
ipu_crtc_atomic_disable() which switches off the DC and DI.
When we immediately restart as in the modeset case this sometimes
leads to corruption at the bottom of the screen looking like a mirror
image of the top.
The exact reason isn't understood but it seems timing related.
This was observed on i.MX6DL on a system that does 2 mode
transitions on boot (fbcon->android boot animation->android homescreen)
then no more during normal operation resulting in corruption
about once every 10 boots that lasted for variable durations
from a few seconds to several hours.
Dumping the buffers confirmed that they were correct in memory,
just the display was wrong.
For tests the problem was reproduced systematically by forcing
a full modeset every 10 frames even if when not required.
Leaving the DC and DI on if the CRTC is staying on fixes this.
Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
---
drivers/gpu/drm/imx/ipuv3-crtc.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 63c0284..9137b64 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -84,8 +84,15 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
- ipu_dc_disable_channel(ipu_crtc->dc);
- ipu_di_disable(ipu_crtc->di);
+ /*
+ * If we are just doing a modeset don't disable dc/di as that
+ * sometimes leads to corruption at the bottom of the screen
+ */
+ if (!crtc->state->active) {
+ ipu_dc_disable_channel(ipu_crtc->dc);
+ ipu_di_disable(ipu_crtc->di);
+ }
+
/*
* Planes must be disabled before DC clock is removed, as otherwise the
* attached IDMACs will be left in undefined state, possibly hanging
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] drm: imx: Fix occasional screen corruption on modeset.
@ 2020-07-07 15:56 ` Martin Fuzzey
0 siblings, 0 replies; 6+ messages in thread
From: Martin Fuzzey @ 2020-07-07 15:56 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Sascha Hauer, Shawn Guo, linux-kernel, dri-devel
When performing a modeset the atomic core calls
ipu_crtc_atomic_disable() which switches off the DC and DI.
When we immediately restart as in the modeset case this sometimes
leads to corruption at the bottom of the screen looking like a mirror
image of the top.
The exact reason isn't understood but it seems timing related.
This was observed on i.MX6DL on a system that does 2 mode
transitions on boot (fbcon->android boot animation->android homescreen)
then no more during normal operation resulting in corruption
about once every 10 boots that lasted for variable durations
from a few seconds to several hours.
Dumping the buffers confirmed that they were correct in memory,
just the display was wrong.
For tests the problem was reproduced systematically by forcing
a full modeset every 10 frames even if when not required.
Leaving the DC and DI on if the CRTC is staying on fixes this.
Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
---
drivers/gpu/drm/imx/ipuv3-crtc.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 63c0284..9137b64 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -84,8 +84,15 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
- ipu_dc_disable_channel(ipu_crtc->dc);
- ipu_di_disable(ipu_crtc->di);
+ /*
+ * If we are just doing a modeset don't disable dc/di as that
+ * sometimes leads to corruption at the bottom of the screen
+ */
+ if (!crtc->state->active) {
+ ipu_dc_disable_channel(ipu_crtc->dc);
+ ipu_di_disable(ipu_crtc->di);
+ }
+
/*
* Planes must be disabled before DC clock is removed, as otherwise the
* attached IDMACs will be left in undefined state, possibly hanging
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: imx: Fix occasional screen corruption on modeset.
2020-07-07 15:56 ` Martin Fuzzey
@ 2020-07-08 8:31 ` Philipp Zabel
-1 siblings, 0 replies; 6+ messages in thread
From: Philipp Zabel @ 2020-07-08 8:31 UTC (permalink / raw)
To: Martin Fuzzey; +Cc: Shawn Guo, Sascha Hauer, dri-devel, linux-kernel
Hi Martin,
On Tue, 2020-07-07 at 17:56 +0200, Martin Fuzzey wrote:
> When performing a modeset the atomic core calls
> ipu_crtc_atomic_disable() which switches off the DC and DI.
>
> When we immediately restart as in the modeset case this sometimes
> leads to corruption at the bottom of the screen looking like a mirror
> image of the top.
Could this be just a panel getting confused because the pixel clock is
disabled, or is there really an issue with the IPU? Have you tried just
keeping clk_di_pixel enabled in ipu_di_disable(), but continuing
to disable DI and DC?
> The exact reason isn't understood but it seems timing related.
>
> This was observed on i.MX6DL on a system that does 2 mode
> transitions on boot (fbcon->android boot animation->android homescreen)
> then no more during normal operation resulting in corruption
> about once every 10 boots that lasted for variable durations
> from a few seconds to several hours.
>
> Dumping the buffers confirmed that they were correct in memory,
> just the display was wrong.
>
> For tests the problem was reproduced systematically by forcing
> a full modeset every 10 frames even if when not required.
>
> Leaving the DC and DI on if the CRTC is staying on fixes this.
>
> Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
> ---
> drivers/gpu/drm/imx/ipuv3-crtc.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 63c0284..9137b64 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -84,8 +84,15 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
> struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
> struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
>
> - ipu_dc_disable_channel(ipu_crtc->dc);
> - ipu_di_disable(ipu_crtc->di);
> + /*
> + * If we are just doing a modeset don't disable dc/di as that
> + * sometimes leads to corruption at the bottom of the screen
> + */
> + if (!crtc->state->active) {
> + ipu_dc_disable_channel(ipu_crtc->dc);
> + ipu_di_disable(ipu_crtc->di);
Just removing ipu_di_disable() leaks a clk_prepare_enable refcount on
the di->clk_di_pixel clock.
Also this is followed by an ipu_dc_disable(), which should remove the DC
module's clock if this is the only display. So the DC should be disabled
anyway.
> + }
> +
> /*
> * Planes must be disabled before DC clock is removed, as otherwise the
> * attached IDMACs will be left in undefined state, possibly hanging
regards
Philipp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: imx: Fix occasional screen corruption on modeset.
@ 2020-07-08 8:31 ` Philipp Zabel
0 siblings, 0 replies; 6+ messages in thread
From: Philipp Zabel @ 2020-07-08 8:31 UTC (permalink / raw)
To: Martin Fuzzey; +Cc: Sascha Hauer, Shawn Guo, linux-kernel, dri-devel
Hi Martin,
On Tue, 2020-07-07 at 17:56 +0200, Martin Fuzzey wrote:
> When performing a modeset the atomic core calls
> ipu_crtc_atomic_disable() which switches off the DC and DI.
>
> When we immediately restart as in the modeset case this sometimes
> leads to corruption at the bottom of the screen looking like a mirror
> image of the top.
Could this be just a panel getting confused because the pixel clock is
disabled, or is there really an issue with the IPU? Have you tried just
keeping clk_di_pixel enabled in ipu_di_disable(), but continuing
to disable DI and DC?
> The exact reason isn't understood but it seems timing related.
>
> This was observed on i.MX6DL on a system that does 2 mode
> transitions on boot (fbcon->android boot animation->android homescreen)
> then no more during normal operation resulting in corruption
> about once every 10 boots that lasted for variable durations
> from a few seconds to several hours.
>
> Dumping the buffers confirmed that they were correct in memory,
> just the display was wrong.
>
> For tests the problem was reproduced systematically by forcing
> a full modeset every 10 frames even if when not required.
>
> Leaving the DC and DI on if the CRTC is staying on fixes this.
>
> Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
> ---
> drivers/gpu/drm/imx/ipuv3-crtc.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 63c0284..9137b64 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -84,8 +84,15 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc,
> struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
> struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent);
>
> - ipu_dc_disable_channel(ipu_crtc->dc);
> - ipu_di_disable(ipu_crtc->di);
> + /*
> + * If we are just doing a modeset don't disable dc/di as that
> + * sometimes leads to corruption at the bottom of the screen
> + */
> + if (!crtc->state->active) {
> + ipu_dc_disable_channel(ipu_crtc->dc);
> + ipu_di_disable(ipu_crtc->di);
Just removing ipu_di_disable() leaks a clk_prepare_enable refcount on
the di->clk_di_pixel clock.
Also this is followed by an ipu_dc_disable(), which should remove the DC
module's clock if this is the only display. So the DC should be disabled
anyway.
> + }
> +
> /*
> * Planes must be disabled before DC clock is removed, as otherwise the
> * attached IDMACs will be left in undefined state, possibly hanging
regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: imx: Fix occasional screen corruption on modeset.
2020-07-08 8:31 ` Philipp Zabel
@ 2020-07-08 16:53 ` Fuzzey, Martin
-1 siblings, 0 replies; 6+ messages in thread
From: Fuzzey, Martin @ 2020-07-08 16:53 UTC (permalink / raw)
To: Philipp Zabel
Cc: Shawn Guo, Sascha Hauer, ML dri-devel, Linux-Kernel@Vger. Kernel. Org
Hi Philipp,
thanks for the quick reply.
On Wed, 8 Jul 2020 at 10:31, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Could this be just a panel getting confused because the pixel clock is
> disabled, or is there really an issue with the IPU? Have you tried just
> keeping clk_di_pixel enabled in ipu_di_disable(), but continuing
> to disable DI and DC?
>
I've tried that now but still observed the corruption.
> > The exact reason isn't understood but it seems timing related.
> >
Also just adding a msleep(2) or a mdelay(20) at the end of
ipu_crtc_atomic_disable() makes the problem go away too.
Obviously I'm not suggesting doing that, just that it may help
understand what is going on.
>
> Just removing ipu_di_disable() leaks a clk_prepare_enable refcount on
> the di->clk_di_pixel clock.
>
> Also this is followed by an ipu_dc_disable(), which should remove the DC
> module's clock if this is the only display. So the DC should be disabled
> anyway.
>
True.
How about doing just ipu_crtc_disable_planes() and
drm_crtc_vblank_off() in the active (modeset) case.
and in ipu_crtc_atomic_enable() only doing the stuff (which doesn't
touch the planes) if old_state->active == false ?
That will fix the clock refcount problem and seems generally better as
tearing down everything just to do a modeset
seems a bit heavy handed.
I've tested that and it works too but it's probably better to discuss
this some more before sending a new patch.
Regards,
Martin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: imx: Fix occasional screen corruption on modeset.
@ 2020-07-08 16:53 ` Fuzzey, Martin
0 siblings, 0 replies; 6+ messages in thread
From: Fuzzey, Martin @ 2020-07-08 16:53 UTC (permalink / raw)
To: Philipp Zabel
Cc: Sascha Hauer, Shawn Guo, Linux-Kernel@Vger. Kernel. Org, ML dri-devel
Hi Philipp,
thanks for the quick reply.
On Wed, 8 Jul 2020 at 10:31, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Could this be just a panel getting confused because the pixel clock is
> disabled, or is there really an issue with the IPU? Have you tried just
> keeping clk_di_pixel enabled in ipu_di_disable(), but continuing
> to disable DI and DC?
>
I've tried that now but still observed the corruption.
> > The exact reason isn't understood but it seems timing related.
> >
Also just adding a msleep(2) or a mdelay(20) at the end of
ipu_crtc_atomic_disable() makes the problem go away too.
Obviously I'm not suggesting doing that, just that it may help
understand what is going on.
>
> Just removing ipu_di_disable() leaks a clk_prepare_enable refcount on
> the di->clk_di_pixel clock.
>
> Also this is followed by an ipu_dc_disable(), which should remove the DC
> module's clock if this is the only display. So the DC should be disabled
> anyway.
>
True.
How about doing just ipu_crtc_disable_planes() and
drm_crtc_vblank_off() in the active (modeset) case.
and in ipu_crtc_atomic_enable() only doing the stuff (which doesn't
touch the planes) if old_state->active == false ?
That will fix the clock refcount problem and seems generally better as
tearing down everything just to do a modeset
seems a bit heavy handed.
I've tested that and it works too but it's probably better to discuss
this some more before sending a new patch.
Regards,
Martin
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-09 7:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-07 15:56 [PATCH] drm: imx: Fix occasional screen corruption on modeset Martin Fuzzey
2020-07-07 15:56 ` Martin Fuzzey
2020-07-08 8:31 ` Philipp Zabel
2020-07-08 8:31 ` Philipp Zabel
2020-07-08 16:53 ` Fuzzey, Martin
2020-07-08 16:53 ` Fuzzey, Martin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.