| Message ID | 1396442280-6189-1-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 2, 2014 at 8:37 AM, Tomi Valkeinen <tomi.valkeinen@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@ti.com> > Reported-by: Sanjay Singh Rawat <sanjay.rawat@linaro.org> Reviewed-by: Rob Clark <robdclark@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 >
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);
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@ti.com> Reported-by: Sanjay Singh Rawat <sanjay.rawat@linaro.org> --- drivers/gpu/drm/omapdrm/omap_crtc.c | 46 ++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 19 deletions(-)