At module unload, omap_fbdev_free() gets called which releases the framebuffers. However, the framebuffers are still used by crtcs, and will be released only later at vsync. The driver doesn't wait for this, and goes on to release the rest of the resources, which often causes a crash.

This patchs adds a omap_crtc_flush() function which waits until the crtc has finished with its apply queue and page flips.

The function utilizes a simple polling while-loop, as the performance is not an issue here.

Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com --- drivers/gpu/drm/omapdrm/omap_crtc.c | 19 +++++++++++++++++++ drivers/gpu/drm/omapdrm/omap_drv.c | 6 ++++++ drivers/gpu/drm/omapdrm/omap_drv.h | 1 + 3 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index e7b643c178a6..4f624c59a660 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -620,6 +620,25 @@ static void omap_crtc_post_apply(struct omap_drm_apply *apply) /* nothing needed for post-apply */ }

+void omap_crtc_flush(struct drm_crtc *crtc) +{ + struct omap_crtc *omap_crtc = to_omap_crtc(crtc); + int loops = 0; + + while (!list_empty(&omap_crtc->pending_applies) || + !list_empty(&omap_crtc->queued_applies) || + omap_crtc->event || omap_crtc->old_fb) { + + if (++loops > 10) { + dev_err(crtc->dev->dev, + "omap_crtc_flush() timeout\n"); + break; + } + + schedule_timeout_uninterruptible(msecs_to_jiffies(20)); + } +} + static const char *channel_names[] = { [OMAP_DSS_CHANNEL_LCD] = "lcd", [OMAP_DSS_CHANNEL_DIGIT] = "tv", diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index f16a07d1668d..c8270e4b26f3 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -513,12 +513,18 @@ static int dev_load(struct drm_device *dev, unsigned long flags) static int dev_unload(struct drm_device *dev) { struct omap_drm_private *priv = dev->dev_private; + int i;

DBG("unload: dev=%p", dev);

drm_kms_helper_poll_fini(dev);

omap_fbdev_free(dev); + + /* flush crtcs so the fbs get released */ + for (i = 0; i < priv->num_crtcs; i++) + omap_crtc_flush(priv->crtcs[i]); + omap_modeset_free(dev); omap_gem_deinit(dev);

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h index 428b2981fd68..284b80fc3c54 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.h +++ b/drivers/gpu/drm/omapdrm/omap_drv.h @@ -163,6 +163,7 @@ void omap_crtc_pre_init(void); void omap_crtc_pre_uninit(void); struct drm_crtc *omap_crtc_init(struct drm_device *dev, struct drm_plane *plane, enum omap_channel channel, int id); +void omap_crtc_flush(struct drm_crtc *crtc);

struct drm_plane *omap_plane_init(struct drm_device *dev, int plane_id, bool private_plane);

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