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

2016-11-22 17:54 GMT+01:00 Jyri Sarha jsarha@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@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