[PATCH 1/4] drm/omap: fix output enable/disable sequence

[Rob Clark]

On Wed, Apr 2, 2014 at 8:37 AM, Tomi Valkeinen <tomi.valkeinen at ti.com> wrote:
> At the moment it's quite easy to get the following errors when the HDMI
> output is enabled or disabled:
>
> [drm:omap_crtc_error_irq] *ERROR* tv: errors: 00008000
>
> The reason for the errors is that the omapdrm driver doesn't properly
> handle the sync-lost irqs that happen when enabling the DIGIT crtc,
> which is used for HDMI and analog TV. The driver does disable the
> sync-lost irq properly, but it fails to wait until the output has been
> fully enabled (i.e. the first vsync), so the sync-lost errors are still
> seen occasionally.
>
> This patch makes the omapdrm act the same way as the omapfb does:
>
> - When enabling a display, we'll wait for the first vsync.
> - When disabling a display, we'll wait for framedone if available, or
>   odd and even vsyncs.
>
> These changes make sure the output is fully enabled or disabled at the
> end of the function.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> Reported-by: Sanjay Singh Rawat <sanjay.rawat at linaro.org>

Reviewed-by: Rob Clark <robdclark at gmail.com>

> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 46 ++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index 4313bb0a49a6..e7b643c178a6 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -527,38 +527,46 @@ static void set_enabled(struct drm_crtc *crtc, bool enable)
>         struct drm_device *dev = crtc->dev;
>         struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>         enum omap_channel channel = omap_crtc->channel;
> -       struct omap_irq_wait *wait = NULL;
> +       struct omap_irq_wait *wait;
> +       u32 framedone_irq, vsync_irq;
> +       int ret;
>
>         if (dispc_mgr_is_enabled(channel) == enable)
>                 return;
>
> -       /* ignore sync-lost irqs during enable/disable */
> +       /*
> +        * Digit output produces some sync lost interrupts during the first
> +        * frame when enabling, so we need to ignore those.
> +        */
>         omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
>
> -       if (dispc_mgr_get_framedone_irq(channel)) {
> -               if (!enable) {
> -                       wait = omap_irq_wait_init(dev,
> -                                       dispc_mgr_get_framedone_irq(channel), 1);
> -               }
> +       framedone_irq = dispc_mgr_get_framedone_irq(channel);
> +       vsync_irq = dispc_mgr_get_vsync_irq(channel);
> +
> +       if (enable) {
> +               wait = omap_irq_wait_init(dev, vsync_irq, 1);
>         } else {
>                 /*
> -                * When we disable digit output, we need to wait until fields
> -                * are done.  Otherwise the DSS is still working, and turning
> -                * off the clocks prevents DSS from going to OFF mode. And when
> -                * enabling, we need to wait for the extra sync losts
> +                * When we disable the digit output, we need to wait for
> +                * FRAMEDONE to know that DISPC has finished with the output.
> +                *
> +                * OMAP2/3 does not have FRAMEDONE irq for digit output, and in
> +                * that case we need to use vsync interrupt, and wait for both
> +                * even and odd frames.
>                  */
> -               wait = omap_irq_wait_init(dev,
> -                               dispc_mgr_get_vsync_irq(channel), 2);
> +
> +               if (framedone_irq)
> +                       wait = omap_irq_wait_init(dev, framedone_irq, 1);
> +               else
> +                       wait = omap_irq_wait_init(dev, vsync_irq, 2);
>         }
>
>         dispc_mgr_enable(channel, enable);
>
> -       if (wait) {
> -               int ret = omap_irq_wait(dev, wait, msecs_to_jiffies(100));
> -               if (ret) {
> -                       dev_err(dev->dev, "%s: timeout waiting for %s\n",
> -                                       omap_crtc->name, enable ? "enable" : "disable");
> -               }
> +       ret = omap_irq_wait(dev, wait, msecs_to_jiffies(100));
> +       if (ret) {
> +               dev_err(dev->dev, "%s: timeout waiting for %s\n",
> +                               omap_crtc->name, enable ? "enable" : "disable");
>         }
>
>         omap_irq_register(crtc->dev, &omap_crtc->error_irq);
> --
> 1.8.3.2
>