Re: [PATCH] drm/tve200: Stabilize enable/disable

[Daniel Vetter]

On Tue, Sep 1, 2020 at 7:52 PM Linus Walleij  wrote:
>
> On Thu, Aug 20, 2020 at 10:32 PM Linus Walleij  
> wrote:
>
> > The TVE200 will occasionally print a bunch of lost interrupts
> > and similar dmesg messages, sometimes during boot and sometimes
> > after disabling and coming back to enablement. This is probably
> > because the hardware is left in an unknown state by the boot
> > loader that displays a logo.
> >
> > This can be fixed by bringing the controller into a known state
> > by resetting the controller while enabling it. We retry reset 5
> > times like the vendor driver does. We also put the controller
> > into reset before de-clocking it and clear all interrupts before
> > enabling the vblank IRQ.
> >
> > This makes the video enable/disable/enable cycle rock solid
> > on the D-Link DIR-685. Tested extensively.
> >
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Linus Walleij 
>
> Would someone have mercy on this patch and review or
> at least ACK it so I can merge it?

Does what it says on the label, looks symmetric, and "do this five
times for luck" is a classic.

Acked-by: Daniel Vetter 

The irq reset looks a bit like maybe separate patch, but *shrug*,
since your description says you're missing interrupts, not that you
have too many. But can't hurt (and maybe if we have spurious ones it
then looks like the next vblank went missing, so makes some sense).

Cheers, Daniel

> I offer any reviews in return, on stuff I understand, such
> as panel drivers.


>
> Best regards,
> Linus Walleij
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tve200: Stabilize enable/disable

[Linus Walleij]

On Thu, Aug 20, 2020 at 10:32 PM Linus Walleij  wrote:

> The TVE200 will occasionally print a bunch of lost interrupts
> and similar dmesg messages, sometimes during boot and sometimes
> after disabling and coming back to enablement. This is probably
> because the hardware is left in an unknown state by the boot
> loader that displays a logo.
>
> This can be fixed by bringing the controller into a known state
> by resetting the controller while enabling it. We retry reset 5
> times like the vendor driver does. We also put the controller
> into reset before de-clocking it and clear all interrupts before
> enabling the vblank IRQ.
>
> This makes the video enable/disable/enable cycle rock solid
> on the D-Link DIR-685. Tested extensively.
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Linus Walleij 

Would someone have mercy on this patch and review or
at least ACK it so I can merge it?

I offer any reviews in return, on stuff I understand, such
as panel drivers.

Best regards,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/tve200: Stabilize enable/disable

[Linus Walleij]

The TVE200 will occasionally print a bunch of lost interrupts
and similar dmesg messages, sometimes during boot and sometimes
after disabling and coming back to enablement. This is probably
because the hardware is left in an unknown state by the boot
loader that displays a logo.

This can be fixed by bringing the controller into a known state
by resetting the controller while enabling it. We retry reset 5
times like the vendor driver does. We also put the controller
into reset before de-clocking it and clear all interrupts before
enabling the vblank IRQ.

This makes the video enable/disable/enable cycle rock solid
on the D-Link DIR-685. Tested extensively.

Cc: sta...@vger.kernel.org
Signed-off-by: Linus Walleij 
---
 drivers/gpu/drm/tve200/tve200_display.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tve200/tve200_display.c 
b/drivers/gpu/drm/tve200/tve200_display.c
index d733bbc4ac0e..17ff24d999d1 100644
--- a/drivers/gpu/drm/tve200/tve200_display.c
+++ b/drivers/gpu/drm/tve200/tve200_display.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -130,9 +131,25 @@ static void tve200_display_enable(struct 
drm_simple_display_pipe *pipe,
struct drm_connector *connector = priv->connector;
u32 format = fb->format->format;
u32 ctrl1 = 0;
+   int retries;
 
clk_prepare_enable(priv->clk);
 
+   /* Reset the TVE200 and wait for it to come back online */
+   writel(TVE200_CTRL_4_RESET, priv->regs + TVE200_CTRL_4);
+   for (retries = 0; retries < 5; retries++) {
+   usleep_range(3, 5);
+   if (readl(priv->regs + TVE200_CTRL_4) & TVE200_CTRL_4_RESET)
+   continue;
+   else
+   break;
+   }
+   if (retries == 5 &&
+   readl(priv->regs + TVE200_CTRL_4) & TVE200_CTRL_4_RESET) {
+   dev_err(drm->dev, "can't get hardware out of reset\n");
+   return;
+   }
+
/* Function 1 */
ctrl1 |= TVE200_CTRL_CSMODE;
/* Interlace mode for CCIR656: parameterize? */
@@ -230,8 +247,9 @@ static void tve200_display_disable(struct 
drm_simple_display_pipe *pipe)
 
drm_crtc_vblank_off(crtc);
 
-   /* Disable and Power Down */
+   /* Disable put into reset and Power Down */
writel(0, priv->regs + TVE200_CTRL);
+   writel(TVE200_CTRL_4_RESET, priv->regs + TVE200_CTRL_4);
 
clk_disable_unprepare(priv->clk);
 }
@@ -279,6 +297,8 @@ static int tve200_display_enable_vblank(struct 
drm_simple_display_pipe *pipe)
struct drm_device *drm = crtc->dev;
struct tve200_drm_dev_private *priv = drm->dev_private;
 
+   /* Clear any IRQs and enable */
+   writel(0xFF, priv->regs + TVE200_INT_CLR);
writel(TVE200_INT_V_STATUS, priv->regs + TVE200_INT_EN);
return 0;
 }
-- 
2.26.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel