It is more optimium to use wait queues while waiting for vsync so that the current task is put to sleep. This way, the task wont hog the CPU while waiting. We use wait_event_timeout and not an interruptible function since we dont want the function to exit when a signal is pending (e.g. drm release). This patch modifies the wait for vblank functions of both fimd and mixer.
Also, the current fimd wait_for_vblank did not work in exynos5 since VIDCON1 has to be used in addition to timing base offset (0x20000). This patch also eliminates this problem.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_drv.h | 14 -------------- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 21 +++++++++++++++++---- drivers/gpu/drm/exynos/exynos_mixer.c | 21 ++++++++++++++++----- 3 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index a4702a8..6bb8644 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -36,20 +36,6 @@ #define MAX_FB_BUFFER 4 #define DEFAULT_ZPOS -1
-#define _wait_for(COND, MS) ({ \ - unsigned long timeout__ = jiffies + msecs_to_jiffies(MS); \ - int ret__ = 0; \ - while (!(COND)) { \ - if (time_after(jiffies, timeout__)) { \ - ret__ = -ETIMEDOUT; \ - break; \ - } \ - } \ - ret__; \ -}) - -#define wait_for(COND, MS) _wait_for(COND, MS) - struct drm_device; struct exynos_drm_overlay; struct drm_connector; diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index e2abae6..63f0d0f 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -101,6 +101,8 @@ struct fimd_context { u32 vidcon1; bool suspended; struct mutex lock; + wait_queue_head_t wait_vsync_queue; + atomic_t wait_vsync_event;
struct exynos_drm_panel_info *panel; }; @@ -606,11 +608,16 @@ static void fimd_win_disable(struct device *dev, int zpos) static void fimd_wait_for_vblank(struct device *dev) { struct fimd_context *ctx = get_fimd_context(dev); - int ret;
- ret = wait_for((__raw_readl(ctx->regs + VIDCON1) & - VIDCON1_VSTATUS_VSYNC), 50); - if (ret < 0) + atomic_set(&ctx->wait_vsync_event, 1); + + /* + * wait for FIMD to signal VSYNC interrupt or return after + * timeout which is set to 50ms (refresh rate of 20). + */ + if (!wait_event_timeout(ctx->wait_vsync_queue, + !atomic_read(&ctx->wait_vsync_event), + DRM_HZ/20)) DRM_DEBUG_KMS("vblank wait timed out.\n"); }
@@ -677,6 +684,10 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id) drm_handle_vblank(drm_dev, manager->pipe); fimd_finish_pageflip(drm_dev, manager->pipe);
+ /* set wait vsync event to zero and wake up queue. */ + atomic_set(&ctx->wait_vsync_event, 0); + DRM_WAKEUP(&ctx->wait_vsync_queue); + out: return IRQ_HANDLED; } @@ -967,6 +978,8 @@ static int __devinit fimd_probe(struct platform_device *pdev) ctx->vidcon1 = pdata->vidcon1; ctx->default_win = pdata->default_win; ctx->panel = panel; + DRM_INIT_WAITQUEUE(&ctx->wait_vsync_queue); + atomic_set(&ctx->wait_vsync_event, 0);
subdrv = &ctx->subdrv;
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 50100cf..972281e 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -93,6 +93,8 @@ struct mixer_context { struct hdmi_win_data win_data[MIXER_WIN_NR]; enum mixer_version_id mxr_ver; void *parent_ctx; + wait_queue_head_t wait_vsync_queue; + atomic_t wait_vsync_event; };
struct mixer_drv_data { @@ -791,12 +793,16 @@ static void mixer_dpms(void *ctx, int mode) static void mixer_wait_for_vblank(void *ctx) { struct mixer_context *mixer_ctx = ctx; - struct mixer_resources *res = &mixer_ctx->mixer_res; - int ret;
- ret = wait_for((mixer_reg_read(res, MXR_INT_STATUS) & - MXR_INT_STATUS_VSYNC), 50); - if (ret < 0) + atomic_set(&mixer_ctx->wait_vsync_event, 1); + + /* + * wait for MIXER to signal VSYNC interrupt or return after + * timeout which is set to 50ms (refresh rate of 20). + */ + if (!wait_event_timeout(mixer_ctx->wait_vsync_queue, + !atomic_read(&mixer_ctx->wait_vsync_event), + DRM_HZ/20)) DRM_DEBUG_KMS("vblank wait timed out.\n"); }
@@ -957,6 +963,9 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
drm_handle_vblank(drm_hdmi_ctx->drm_dev, ctx->pipe); mixer_finish_pageflip(drm_hdmi_ctx->drm_dev, ctx->pipe); + + atomic_set(&ctx->wait_vsync_event, 0); + DRM_WAKEUP(&ctx->wait_vsync_queue); }
out: @@ -1166,6 +1175,8 @@ static int __devinit mixer_probe(struct platform_device *pdev) drm_hdmi_ctx->ctx = (void *)ctx; ctx->vp_enabled = drv->is_vp_enabled; ctx->mxr_ver = drv->version; + DRM_INIT_WAITQUEUE(&ctx->wait_vsync_queue); + atomic_set(&ctx->wait_vsync_event, 0);
platform_set_drvdata(pdev, drm_hdmi_ctx);
Currently wait_for_vblank is set as an overlay_op which is not correct since wait for vblank is not dependant on any particular overlay. It should be grouped with enable/disable vblank calls inside manager_ops. Also if wait for vblank is a manager op, the check for DPMS_OFF of encoder can be removed inside exynos_drm_encoder_complete_scanout. This fixes the following issue while removing a fb. While removing the current fb (crtc->fb == fb) the encoder dpms off is called and then the framebuffer is removed. So in this case, the wait for vblank is not called thus leading to a crash (since fb might get removed before dma is finished).
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 ++-- drivers/gpu/drm/exynos/exynos_drm_encoder.c | 15 +++-------- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 34 +++++++++++++------------- drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 22 ++++++++-------- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 2 +- drivers/gpu/drm/exynos/exynos_mixer.c | 34 +++++++++++++------------- 6 files changed, 53 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index 6bb8644..98597d7 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -60,8 +60,6 @@ enum exynos_drm_output_type { * @commit: apply hardware specific overlay data to registers. * @enable: enable hardware specific overlay. * @disable: disable hardware specific overlay. - * @wait_for_vblank: wait for vblank interrupt to make sure that - * hardware overlay is disabled. */ struct exynos_drm_overlay_ops { void (*mode_set)(struct device *subdrv_dev, @@ -69,7 +67,6 @@ struct exynos_drm_overlay_ops { void (*commit)(struct device *subdrv_dev, int zpos); void (*enable)(struct device *subdrv_dev, int zpos); void (*disable)(struct device *subdrv_dev, int zpos); - void (*wait_for_vblank)(struct device *subdrv_dev); };
/* @@ -172,6 +169,8 @@ struct exynos_drm_display_ops { * @commit: set current hw specific display mode to hw. * @enable_vblank: specific driver callback for enabling vblank interrupt. * @disable_vblank: specific driver callback for disabling vblank interrupt. + * @wait_for_vblank: wait for vblank interrupt to make sure that + * hardware overlay is updated. */ struct exynos_drm_manager_ops { void (*dpms)(struct device *subdrv_dev, int mode); @@ -186,6 +185,7 @@ struct exynos_drm_manager_ops { void (*commit)(struct device *subdrv_dev); int (*enable_vblank)(struct device *subdrv_dev); void (*disable_vblank)(struct device *subdrv_dev); + void (*wait_for_vblank)(struct device *subdrv_dev); };
/* diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c index d9afb11..3014852 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c @@ -237,8 +237,7 @@ static void exynos_drm_encoder_commit(struct drm_encoder *encoder) void exynos_drm_encoder_complete_scanout(struct drm_framebuffer *fb) { struct exynos_drm_encoder *exynos_encoder; - struct exynos_drm_overlay_ops *overlay_ops; - struct exynos_drm_manager *manager; + struct exynos_drm_manager_ops *ops; struct drm_device *dev = fb->dev; struct drm_encoder *encoder;
@@ -248,21 +247,15 @@ void exynos_drm_encoder_complete_scanout(struct drm_framebuffer *fb) */ list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { exynos_encoder = to_exynos_encoder(encoder); - - /* if exynos was disabled, just ignor it. */ - if (exynos_encoder->dpms > DRM_MODE_DPMS_ON) - continue; - - manager = exynos_encoder->manager; - overlay_ops = manager->overlay_ops; + ops = exynos_encoder->manager->ops;
/* * wait for vblank interrupt * - this makes sure that overlay data are updated to * real hardware. */ - if (overlay_ops->wait_for_vblank) - overlay_ops->wait_for_vblank(manager->dev); + if (ops->wait_for_vblank) + ops->wait_for_vblank(exynos_encoder->manager->dev); } }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 63f0d0f..020eb86 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -320,12 +320,29 @@ static void fimd_disable_vblank(struct device *dev) } }
+static void fimd_wait_for_vblank(struct device *dev) +{ + struct fimd_context *ctx = get_fimd_context(dev); + + atomic_set(&ctx->wait_vsync_event, 1); + + /* + * wait for FIMD to signal VSYNC interrupt or return after + * timeout which is set to 50ms (refresh rate of 20). + */ + if (!wait_event_timeout(ctx->wait_vsync_queue, + !atomic_read(&ctx->wait_vsync_event), + DRM_HZ/20)) + DRM_DEBUG_KMS("vblank wait timed out.\n"); +} + static struct exynos_drm_manager_ops fimd_manager_ops = { .dpms = fimd_dpms, .apply = fimd_apply, .commit = fimd_commit, .enable_vblank = fimd_enable_vblank, .disable_vblank = fimd_disable_vblank, + .wait_for_vblank = fimd_wait_for_vblank, };
static void fimd_win_mode_set(struct device *dev, @@ -605,27 +622,10 @@ static void fimd_win_disable(struct device *dev, int zpos) win_data->enabled = false; }
-static void fimd_wait_for_vblank(struct device *dev) -{ - struct fimd_context *ctx = get_fimd_context(dev); - - atomic_set(&ctx->wait_vsync_event, 1); - - /* - * wait for FIMD to signal VSYNC interrupt or return after - * timeout which is set to 50ms (refresh rate of 20). - */ - if (!wait_event_timeout(ctx->wait_vsync_queue, - !atomic_read(&ctx->wait_vsync_event), - DRM_HZ/20)) - DRM_DEBUG_KMS("vblank wait timed out.\n"); -} - static struct exynos_drm_overlay_ops fimd_overlay_ops = { .mode_set = fimd_win_mode_set, .commit = fimd_win_commit, .disable = fimd_win_disable, - .wait_for_vblank = fimd_wait_for_vblank, };
static struct exynos_drm_manager fimd_manager = { diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c index 8b771a3..55793c4 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c @@ -179,6 +179,16 @@ static void drm_hdmi_disable_vblank(struct device *subdrv_dev) return mixer_ops->disable_vblank(ctx->mixer_ctx->ctx); }
+static void drm_hdmi_wait_for_vblank(struct device *subdrv_dev) +{ + struct drm_hdmi_context *ctx = to_context(subdrv_dev); + + DRM_DEBUG_KMS("%s\n", __FILE__); + + if (mixer_ops && mixer_ops->wait_for_vblank) + mixer_ops->wait_for_vblank(ctx->mixer_ctx->ctx); +} + static void drm_hdmi_mode_fixup(struct device *subdrv_dev, struct drm_connector *connector, const struct drm_display_mode *mode, @@ -260,6 +270,7 @@ static struct exynos_drm_manager_ops drm_hdmi_manager_ops = { .apply = drm_hdmi_apply, .enable_vblank = drm_hdmi_enable_vblank, .disable_vblank = drm_hdmi_disable_vblank, + .wait_for_vblank = drm_hdmi_wait_for_vblank, .mode_fixup = drm_hdmi_mode_fixup, .mode_set = drm_hdmi_mode_set, .get_max_resol = drm_hdmi_get_max_resol, @@ -313,21 +324,10 @@ static void drm_mixer_disable(struct device *subdrv_dev, int zpos) ctx->enabled[win] = false; }
-static void drm_mixer_wait_for_vblank(struct device *subdrv_dev) -{ - struct drm_hdmi_context *ctx = to_context(subdrv_dev); - - DRM_DEBUG_KMS("%s\n", __FILE__); - - if (mixer_ops && mixer_ops->wait_for_vblank) - mixer_ops->wait_for_vblank(ctx->mixer_ctx->ctx); -} - static struct exynos_drm_overlay_ops drm_hdmi_overlay_ops = { .mode_set = drm_mixer_mode_set, .commit = drm_mixer_commit, .disable = drm_mixer_disable, - .wait_for_vblank = drm_mixer_wait_for_vblank, };
static struct exynos_drm_manager hdmi_manager = { diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h index 54b5223..fcc3093 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h @@ -65,10 +65,10 @@ struct exynos_mixer_ops { int (*iommu_on)(void *ctx, bool enable); int (*enable_vblank)(void *ctx, int pipe); void (*disable_vblank)(void *ctx); + void (*wait_for_vblank)(void *ctx); void (*dpms)(void *ctx, int mode);
/* overlay */ - void (*wait_for_vblank)(void *ctx); void (*win_mode_set)(void *ctx, struct exynos_drm_overlay *overlay); void (*win_commit)(void *ctx, int zpos); void (*win_disable)(void *ctx, int zpos); diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 972281e..e2083a9 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -769,6 +769,22 @@ static void mixer_disable_vblank(void *ctx) mixer_reg_writemask(res, MXR_INT_EN, 0, MXR_INT_EN_VSYNC); }
+static void mixer_wait_for_vblank(void *ctx) +{ + struct mixer_context *mixer_ctx = ctx; + + atomic_set(&mixer_ctx->wait_vsync_event, 1); + + /* + * wait for MIXER to signal VSYNC interrupt or return after + * timeout which is set to 50ms (refresh rate of 20). + */ + if (!wait_event_timeout(mixer_ctx->wait_vsync_queue, + !atomic_read(&mixer_ctx->wait_vsync_event), + DRM_HZ/20)) + DRM_DEBUG_KMS("vblank wait timed out.\n"); +} + static void mixer_dpms(void *ctx, int mode) { struct mixer_context *mixer_ctx = ctx; @@ -790,22 +806,6 @@ static void mixer_dpms(void *ctx, int mode) } }
-static void mixer_wait_for_vblank(void *ctx) -{ - struct mixer_context *mixer_ctx = ctx; - - atomic_set(&mixer_ctx->wait_vsync_event, 1); - - /* - * wait for MIXER to signal VSYNC interrupt or return after - * timeout which is set to 50ms (refresh rate of 20). - */ - if (!wait_event_timeout(mixer_ctx->wait_vsync_queue, - !atomic_read(&mixer_ctx->wait_vsync_event), - DRM_HZ/20)) - DRM_DEBUG_KMS("vblank wait timed out.\n"); -} - static void mixer_win_mode_set(void *ctx, struct exynos_drm_overlay *overlay) { @@ -896,10 +896,10 @@ static struct exynos_mixer_ops mixer_ops = { .iommu_on = mixer_iommu_on, .enable_vblank = mixer_enable_vblank, .disable_vblank = mixer_disable_vblank, + .wait_for_vblank = mixer_wait_for_vblank, .dpms = mixer_dpms,
/* overlay */ - .wait_for_vblank = mixer_wait_for_vblank, .win_mode_set = mixer_win_mode_set, .win_commit = mixer_win_commit, .win_disable = mixer_win_disable,
The crtc disable function should not disable the overlays if the crtc is already in DPMS_OFF as this will lead to register access when clock is off. Also the crtc disable function should not call DPMS OFF of the crtc. This is required to ensure we are able to wait for vblank before freeing any framebuffers after disabling the crtc.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index 2efa4b0..faa6ee0 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -186,8 +186,12 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
DRM_DEBUG_KMS("%s\n", __FILE__);
+ if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) { + DRM_DEBUG_KMS("crtc is already off.\n"); + return; + } + exynos_plane_dpms(exynos_crtc->plane, DRM_MODE_DPMS_OFF); - exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); }
static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
dri-devel@lists.freedesktop.org
-
Prathyush K