Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966385AbcKAOWs (ORCPT ); Tue, 1 Nov 2016 10:22:48 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:37364 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966245AbcKAOWr (ORCPT ); Tue, 1 Nov 2016 10:22:47 -0400 From: Jyri Sarha Subject: Re: [PATCH v5 2/2] drm: tilcdc: clear the sync lost bit in crtc isr To: Bartosz Golaszewski , Tomi Valkeinen , David Airlie , Kevin Hilman , Michael Turquette , Sekhar Nori References: <1477923567-1610-1-git-send-email-bgolaszewski@baylibre.com> <1477923567-1610-3-git-send-email-bgolaszewski@baylibre.com> CC: LKML , linux-drm , Peter Ujfalusi , arm-soc Message-ID: Date: Tue, 1 Nov 2016 16:22:16 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1477923567-1610-3-git-send-email-bgolaszewski@baylibre.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5391 Lines: 137 On 10/31/16 16:19, Bartosz Golaszewski wrote: > The frame synchronization error happens when the DMA engine attempts > to read what it believes to be the first word of the video buffer but > it cannot be recognized as such or when the LCDC is starved of data > due to insufficient bandwidth of the system interconnect. > > On some SoCs (notably: da850) the memory settings do not meet the > LCDC throughput requirements even after increasing the memory > controller command re-ordering and the LDCD master peripheral > priority, sometimes causing the sync lost error (typically when > changing the resolution). > > When the sync lost error occurs, simply reset the input FIFO in the > DMA controller unless a sync lost flood is detected in which case > disable the interrupt. > I don't think the current behaviour after detecting a sync lost flood is really good for ver2 either. The flood is an indication of an error situation on LCDC IP and the picture on the screen may be corrupted. Simply turning off the interrupt does not make the problem (specifically the screen corruption) to go away. I have a patch for an alternative approach that turns off the display and resets the LCDC and turns it back on again. It fixes the sync lost flood and the screen corruption (usually shifting to right and rolling over to left hand side). According to my experiments the toggling of raster to off and back on again does not fix the the flood or the the screen corruption. I think it is about time for me the send my sync lost flood patch forward. Then you could extend it to work also on rev1 LCDC. Best regards, Jyri > Signed-off-by: Bartosz Golaszewski > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 50 ++++++++++++++++++++++++++---------- > drivers/gpu/drm/tilcdc/tilcdc_regs.h | 1 + > 2 files changed, 37 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > index 937697d..c4c6323 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -163,7 +163,7 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev) > > if (priv->rev == 1) { > tilcdc_set(dev, LCDC_RASTER_CTRL_REG, > - LCDC_V1_UNDERFLOW_INT_ENA); > + LCDC_V1_UNDERFLOW_INT_ENA | LCDC_V1_SYNC_LOST_ENA); > tilcdc_set(dev, LCDC_DMA_CTRL_REG, > LCDC_V1_END_OF_FRAME_INT_ENA); > } else { > @@ -181,7 +181,9 @@ 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_UNDERFLOW_INT_ENA | LCDC_V1_PL_INT_ENA); > + LCDC_V1_UNDERFLOW_INT_ENA | > + LCDC_V1_PL_INT_ENA | > + LCDC_V1_SYNC_LOST_ENA); > tilcdc_clear(dev, LCDC_DMA_CTRL_REG, > LCDC_V1_END_OF_FRAME_INT_ENA); > } else { > @@ -885,24 +887,44 @@ 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, disabling the interrupt", __func__, stat); > - tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, > - LCDC_SYNC_LOST); > - } > - } > - > /* Indicate to LCDC that the interrupt service routine has > * completed, see 13.3.6.1.6 in AM335x TRM. > */ > tilcdc_write(dev, LCDC_END_OF_INT_IND_REG, 0); > } > > + 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, disabling the interrupt", > + __func__, stat); > + if (priv->rev == 2) > + tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, > + LCDC_SYNC_LOST); > + else if (priv->rev == 1) > + tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, > + LCDC_V1_SYNC_LOST_ENA); > + } > + > + if (priv->rev == 2) { > + /* > + * Indicate to LCDC that the interrupt service routine > + * has completed, see 13.3.6.1.6 in AM335x TRM. > + */ > + tilcdc_write(dev, LCDC_END_OF_INT_IND_REG, 0); > + } else if (priv->rev == 1) { > + /* Reset the input FIFO in the DMA controller. */ > + tilcdc_clear(dev, > + LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE); > + tilcdc_set(dev, > + LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE); > + } > + } > + > return IRQ_HANDLED; > } > > 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) > #define LCDC_MONOCHROME_MODE BIT(1) > #define LCDC_RASTER_ENABLE BIT(0) > #define LCDC_TFT_ALT_ENABLE BIT(23) >