Re: [PATCH v3 01/11] drm/tilcdc: Enable sync lost error and recovery handling for rev 1 LCDC

2016-11-22 17:54 GMT+01:00 Jyri Sarha jsarha@ti.com:

Revision 1 LCDC support also sync lost errors and can benefit from sync lost recovery routine.

Signed-off-by: Jyri Sarha jsarha@ti.com

drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 33 +++++++++++++++++---------------- drivers/gpu/drm/tilcdc/tilcdc_regs.h | 1 + 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index c787349..5260eb2 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -113,6 +113,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_UNDERFLOW_INT_ENA);
            tilcdc_set(dev, LCDC_DMA_CTRL_REG,
                    LCDC_V1_END_OF_FRAME_INT_ENA);
  

@@ -130,7 +131,7 @@ 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,
  

•           tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_SYNC_LOST_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);
  

@@ -845,6 +846,21 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underflow", __func__, stat);

•   if (stat & LCDC_SYNC_LOST) {
  

•           dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
  

•                               __func__, stat);
  

•           tilcdc_crtc->frame_intact = false;
  

•           if (tilcdc_crtc->sync_lost_count++ >
  

•               SYNC_LOST_COUNT_LIMIT) {
  

•                   dev_err(dev->dev, "%s(0x%08x): Sync lost flood detected, recovering", __func__, stat);
  

•                   queue_work(system_wq,
  

•                              &tilcdc_crtc->recover_work);
  

•                   tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
  

•                                LCDC_SYNC_LOST);
  

•                   tilcdc_crtc->sync_lost_count = 0;
  

•           }
  

•   }
  

We need to at least have tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE) here - otherwise the recovery job doesn't even start - as soon as we re-enable interrupts, we get a sync_lost irq again.

•   /* For revision 2 only */
    if (priv->rev == 2) {
            if (stat & LCDC_FRAME_DONE) {
  

@@ -852,21 +868,6 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) wake_up(&tilcdc_crtc->frame_done_wq); }

•           if (stat & LCDC_SYNC_LOST) {
  

•                   dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
  

•                                       __func__, stat);
  

•                   tilcdc_crtc->frame_intact = false;
  

•                   if (tilcdc_crtc->sync_lost_count++ >
  

•                       SYNC_LOST_COUNT_LIMIT) {
  

•                           dev_err(dev->dev, "%s(0x%08x): Sync lost flood detected, recovering", __func__, stat);
  

•                           queue_work(system_wq,
  

•                                      &tilcdc_crtc->recover_work);
  

•                           tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
  

•                                        LCDC_SYNC_LOST);
  

•                           tilcdc_crtc->sync_lost_count = 0;
  

•                   }
  

•           }
  

•           /* 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 f57c0d6..beb8c21 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h @@ -61,6 +61,7 @@ #define LCDC_V2_UNDERFLOW_INT_ENA BIT(5) #define LCDC_V1_PL_INT_ENA BIT(4) #define LCDC_V2_PL_INT_ENA BIT(6) +#define LCDC_V1_SYNC_LOST_ENA BIT(5)

I'd say we call it LCDC_V1_SYNC_LOST_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