From 2ec8e3787ae6957f738bb133e755213b9d7c066e Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Wed, 2 Apr 2014 11:37:06 +0300 Subject: drm/omap: fix output enable/disable sequence 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 Reported-by: Sanjay Singh Rawat Reviewed-by: Rob Clark --- drivers/gpu/drm/omapdrm/omap_crtc.c | 46 ++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 19 deletions(-) (limited to 'drivers/gpu/drm/omapdrm/omap_crtc.c') diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 355157e4f78d..0acbe62901d9 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -528,38 +528,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); -- cgit From e2f8fd74ec1bf15cb2abc1b11f7d9fa09581024e Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Wed, 2 Apr 2014 14:31:57 +0300 Subject: drm/omap: fix race issue when unloading omapdrm 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 Reviewed-by: Rob Clark --- 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(+) (limited to 'drivers/gpu/drm/omapdrm/omap_crtc.c') diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 0acbe62901d9..161a74a3ac5e 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -621,6 +621,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); -- cgit From c7aef12f344459961eb1e0ba10d184816ed42d99 Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Thu, 3 Apr 2014 16:30:03 +0300 Subject: drm/omap: fix missing disable for unused encoder When an encoder is no longer connected to a crtc, the driver will leave the encoder enabled. This patch adds code to track the encoder used for a crtc, and when the encoder changes, the old one is disabled. Signed-off-by: Tomi Valkeinen Reviewed-by: Rob Clark --- drivers/gpu/drm/omapdrm/omap_crtc.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/gpu/drm/omapdrm/omap_crtc.c') diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 161a74a3ac5e..61d1c4897a45 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -33,6 +33,7 @@ struct omap_crtc { int pipe; enum omap_channel channel; struct omap_overlay_manager_info info; + struct drm_encoder *current_encoder; /* * Temporary: eventually this will go away, but it is needed @@ -594,6 +595,11 @@ static void omap_crtc_pre_apply(struct omap_drm_apply *apply) } } + if (omap_crtc->current_encoder && encoder != omap_crtc->current_encoder) + omap_encoder_set_enabled(omap_crtc->current_encoder, false); + + omap_crtc->current_encoder = encoder; + if (!omap_crtc->enabled) { set_enabled(&omap_crtc->base, false); if (encoder) -- cgit From 506096a113832239ce763d20fab8e94f76d56266 Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Thu, 3 Apr 2014 13:11:54 +0300 Subject: drm/omap: fix enabling/disabling of video pipeline At the moment the omap_crtc_pre_apply() handles the enabling, disabling and configuring of encoders and panels separately from the CRTC (i.e. the overlay manager). However, this doesn't work correctly. The encoder driver has to be in control of its video input (i.e. the crtc) for correct operation. This problem causes bugs with (at least) HDMI: the HDMI encoder supplies pixel clock for DISPC, and DISPC supplies video stream for HDMI. The current code first enables the HDMI encoder, and CRTC after that. However, the encoder expects the video stream to start during the encoder's enable, and if it doesn't, there will be sync lost errors. The encoder enables its video source by calling src->enable(), and this call goes to omapdrm (omap_crtc_enable), but omapdrm doesn't do anything in that function. Similarly for disable, which goes to omap_crtc_disable(). This patch moves the code to setup and enable/disable the crtc to omap_crtc_enable. and omap_crtc_disable(). Signed-off-by: Tomi Valkeinen Reviewed-by: Rob Clark --- drivers/gpu/drm/omapdrm/omap_crtc.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/omapdrm/omap_crtc.c') diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 61d1c4897a45..f59ef9359e66 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -121,13 +121,25 @@ static void omap_crtc_start_update(struct omap_overlay_manager *mgr) { } +static void set_enabled(struct drm_crtc *crtc, bool enable); + static int omap_crtc_enable(struct omap_overlay_manager *mgr) { + struct omap_crtc *omap_crtc = omap_crtcs[mgr->id]; + + dispc_mgr_setup(omap_crtc->channel, &omap_crtc->info); + dispc_mgr_set_timings(omap_crtc->channel, + &omap_crtc->timings); + set_enabled(&omap_crtc->base, true); + return 0; } static void omap_crtc_disable(struct omap_overlay_manager *mgr) { + struct omap_crtc *omap_crtc = omap_crtcs[mgr->id]; + + set_enabled(&omap_crtc->base, false); } static void omap_crtc_set_timings(struct omap_overlay_manager *mgr, @@ -601,7 +613,6 @@ static void omap_crtc_pre_apply(struct omap_drm_apply *apply) omap_crtc->current_encoder = encoder; if (!omap_crtc->enabled) { - set_enabled(&omap_crtc->base, false); if (encoder) omap_encoder_set_enabled(encoder, false); } else { @@ -610,13 +621,7 @@ static void omap_crtc_pre_apply(struct omap_drm_apply *apply) omap_encoder_update(encoder, omap_crtc->mgr, &omap_crtc->timings); omap_encoder_set_enabled(encoder, true); - omap_crtc->full_update = false; } - - dispc_mgr_setup(omap_crtc->channel, &omap_crtc->info); - dispc_mgr_set_timings(omap_crtc->channel, - &omap_crtc->timings); - set_enabled(&omap_crtc->base, true); } omap_crtc->full_update = false; -- cgit From 772cdc9777403f10c4229c49645024a502dfd783 Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Wed, 9 Apr 2014 14:51:01 +0300 Subject: drm/omap: remove extra plane->destroy from crtc destroy All the planes, including primary planes, are now destroyed by the drm framework. Thus we no longer need the explicit call to plane->destroy from the crtc's destroy function. This patch removes the call, thus fixing the crash caused by double freeing the plane. remove omap_crtc->plane->funcs->destroy(omap_crtc->plane) Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/omapdrm/omap_crtc.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/gpu/drm/omapdrm/omap_crtc.c') diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index f59ef9359e66..b3a7529845b9 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -197,7 +197,6 @@ static void omap_crtc_destroy(struct drm_crtc *crtc) WARN_ON(omap_crtc->apply_irq.registered); omap_irq_unregister(crtc->dev, &omap_crtc->error_irq); - omap_crtc->plane->funcs->destroy(omap_crtc->plane); drm_crtc_cleanup(crtc); kfree(omap_crtc); -- cgit From 71b6667765c7019f3fd5ea5e0c02f65f7331f3e1 Mon Sep 17 00:00:00 2001 From: Archit Taneja Date: Fri, 11 Apr 2014 12:53:32 +0530 Subject: drm/omap: Fix crash when using LCD3 overlay manager The channel_names list didn't have a string populated for LCD3 manager, this results in a crash when the display's output is connected to LCD3. Add an entry for LCD3. Reported-by: Somnath Mukherjee Signed-off-by: Archit Taneja Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/omapdrm/omap_crtc.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/gpu/drm/omapdrm/omap_crtc.c') diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index b3a7529845b9..46f8e1e40e88 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -654,6 +654,7 @@ static const char *channel_names[] = { [OMAP_DSS_CHANNEL_LCD] = "lcd", [OMAP_DSS_CHANNEL_DIGIT] = "tv", [OMAP_DSS_CHANNEL_LCD2] = "lcd2", + [OMAP_DSS_CHANNEL_LCD3] = "lcd3", }; void omap_crtc_pre_init(void) -- cgit From bc905aced30e48a39af7c452bf46228d7c6188b9 Mon Sep 17 00:00:00 2001 From: Archit Taneja Date: Fri, 11 Apr 2014 12:53:34 +0530 Subject: drm/omap: Use old_fb to synchronize between successive page flips omap_crtc->old_fb is used to check whether the previous page flip has completed or not. However, it's never initialized to anything, so it's always NULL. This results in the check to always succeed, and the page_flip to proceed. Initialize old_fb to the fb that we intend to flip to through page_flip, and therefore prevent a future page flip to proceed if the last one didn't complete. Signed-off-by: Archit Taneja Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/omapdrm/omap_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/omapdrm/omap_crtc.c') diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 46f8e1e40e88..00798247190b 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -360,7 +360,7 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc, } omap_crtc->event = event; - primary->fb = fb; + omap_crtc->old_fb = primary->fb = fb; /* * Hold a reference temporarily until the crtc is updated -- cgit From 38e5597a03d2d1499a785230031c4f48e1d9c6b7 Mon Sep 17 00:00:00 2001 From: Archit Taneja Date: Fri, 11 Apr 2014 12:53:35 +0530 Subject: drm/omap: protect omap_crtc's event with event_lock spinlock The vblank_cb callback and the page_flip ioctl can occur together in different CPU contexts. vblank_cb uses takes tje drm device's event_lock spinlock when sending the vblank event and updating omap_crtc->event and omap_crtc->od_fb. Use the same spinlock in page_flip, to make sure the above omap_crtc parameters are configured sequentially. Signed-off-by: Archit Taneja Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/omapdrm/omap_crtc.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/gpu/drm/omapdrm/omap_crtc.c') diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 00798247190b..e3c47a8005ff 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -350,11 +350,15 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc, struct omap_crtc *omap_crtc = to_omap_crtc(crtc); struct drm_plane *primary = crtc->primary; struct drm_gem_object *bo; + unsigned long flags; DBG("%d -> %d (event=%p)", primary->fb ? primary->fb->base.id : -1, fb->base.id, event); + spin_lock_irqsave(&dev->event_lock, flags); + if (omap_crtc->old_fb) { + spin_unlock_irqrestore(&dev->event_lock, flags); dev_err(dev->dev, "already a pending flip\n"); return -EINVAL; } @@ -362,6 +366,8 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc, omap_crtc->event = event; omap_crtc->old_fb = primary->fb = fb; + spin_unlock_irqrestore(&dev->event_lock, flags); + /* * Hold a reference temporarily until the crtc is updated * and takes the reference to the bo. This avoids it -- cgit