[PATCH v3 11/11] drm/tilcdc: Enable frame done irq and functionality for LCDC rev 1

[Bartosz Golaszewski]

2016-11-22 17:54 GMT+01:00 Jyri Sarha <jsarha at ti.com>:
> We should wait for the last frame to complete before shutting things
> down also on LCDC rev 1.
>
> Signed-off-by: Jyri Sarha <jsarha at ti.com>
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 34 +++++++++++++++++-----------------
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h |  1 +
>  2 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 1a1ff8d..f251546 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -183,7 +183,7 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
>
>         if (priv->rev == 1) {
>                 tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
> -                       LCDC_V1_SYNC_LOST_ENA |
> +                       LCDC_V1_SYNC_LOST_ENA | LCDC_V1_FRAME_DONE_ENA |
>                         LCDC_V1_UNDERFLOW_INT_ENA);
>                 tilcdc_set(dev, LCDC_DMA_CTRL_REG,
>                         LCDC_V1_END_OF_FRAME_INT_ENA);
> @@ -201,7 +201,8 @@ static void tilcdc_crtc_disable_irqs(struct drm_device *dev)
>
>         /* disable irqs that we might have enabled: */
>         if (priv->rev == 1) {
> -               tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_SYNC_LOST_ENA |
> +               tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
> +                       LCDC_V1_SYNC_LOST_ENA | LCDC_V1_FRAME_DONE_ENA |
>                         LCDC_V1_UNDERFLOW_INT_ENA | LCDC_V1_PL_INT_ENA);
>                 tilcdc_clear(dev, LCDC_DMA_CTRL_REG,
>                         LCDC_V1_END_OF_FRAME_INT_ENA);
> @@ -261,6 +262,7 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool shutdown)
>         struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct tilcdc_drm_private *priv = dev->dev_private;
> +       int ret;
>
>         mutex_lock(&tilcdc_crtc->enable_lock);
>         if (shutdown)
> @@ -273,17 +275,15 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool shutdown)
>         tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
>
>         /*
> -        * if necessary wait for framedone irq which will still come
> -        * before putting things to sleep..
> +        * Wait for framedone irq which will still come before putting
> +        * things to sleep..
>          */
> -       if (priv->rev == 2) {
> -               int ret = wait_event_timeout(tilcdc_crtc->frame_done_wq,
> -                                            tilcdc_crtc->frame_done,
> -                                            msecs_to_jiffies(500));
> -               if (ret == 0)
> -                       dev_err(dev->dev, "%s: timeout waiting for framedone\n",
> -                               __func__);
> -       }
> +       ret = wait_event_timeout(tilcdc_crtc->frame_done_wq,
> +                                tilcdc_crtc->frame_done,
> +                                msecs_to_jiffies(500));
> +       if (ret == 0)
> +               dev_err(dev->dev, "%s: timeout waiting for framedone\n",
> +                       __func__);
>
>         drm_crtc_vblank_off(crtc);
>
> @@ -938,13 +938,13 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
>                 }
>         }
>
> +       if (stat & LCDC_FRAME_DONE) {
> +               tilcdc_crtc->frame_done = true;
> +               wake_up(&tilcdc_crtc->frame_done_wq);
> +       }

This seems to be a general quirk with interrupts on rev1, but if we
don't disable the FRAME_DONE interrupt here, then - similarly to
SYNC_LOST - we get stuck with an interrupt flood.

> +
>         /* For revision 2 only */
>         if (priv->rev == 2) {
> -               if (stat & LCDC_FRAME_DONE) {
> -                       tilcdc_crtc->frame_done = true;
> -                       wake_up(&tilcdc_crtc->frame_done_wq);
> -               }
> -
>                 /* Indicate to LCDC that the interrupt service routine has
>                  * completed, see 13.3.6.1.6 in AM335x TRM.
>                  */
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> index 56dbfbd..4e6975a 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
> @@ -67,6 +67,7 @@
>  #define LCDC_V1_PL_INT_ENA                       BIT(4)
>  #define LCDC_V2_PL_INT_ENA                       BIT(6)
>  #define LCDC_V1_SYNC_LOST_ENA                    BIT(5)
> +#define LCDC_V1_FRAME_DONE_ENA                   BIT(3)

I'd call it LCDC_V1_FRAME_DONE_INT_ENA for consistency.

Thanks,
Bartosz Golaszewski

>  #define LCDC_MONOCHROME_MODE                     BIT(1)
>  #define LCDC_RASTER_ENABLE                       BIT(0)
>  #define LCDC_TFT_ALT_ENABLE                      BIT(23)
> --
> 1.9.1
>