This patchset fixes the various issues which result in getting a PAGE FAULT while using fimd and hdmi in exynos drm.
Changelog v2: Seperated patches into drm framework and fimd/mixer patches. Not using patch drm/exynos: do not disable crtc if already off Added two new patches for clearing windows during dpms off
Patch 1: make wait_for_vblank a manager op Patch 2: move hdmi's wait_for_vblank to manager_ops Patch 3: move fimd wait_for_vblank to manager_ops Currently, we do not call wait for vblank if encoder is in DPMS_OFF state inside exynos_drm_encoder_complete_scanout function. This is because wait for vblank is treated as an overlay op. This can be modified to a manager_op thus removing the above check for DPMS_OFF. This ensures that the hardware overlay actually gets disabled while removing the current framebuffer.
Patch 4: modify wait_for_vblank of mixer Patch 5: modify wait_for_vblank of fimd This modifies the wait_for_vblank functions to use wait queues thus ensuring that the current task goes to sleep without taking up CPU while waiting. Also, if crtc is off, then it returns without waiting.
Patch 6: clear windows in mixer dpms off Patch 7: clear windows in fimd dpms off When a crtc is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed. When fimd/mixer resumes, the dma will continue from where it left off and will throw a PAGE FAULT since the memory was freed. This can be fixed by disabling the windows before disabling the clocks. We should also keep track of which windows were active. This is done by setting the resume flag in the window properties. Now if a current fb is removed when the crtc is off, win_disable will set the 'resume' flag of that window to zero and return. So when fimd/mixer resumes, that window will not be resumed.
Prathyush K (7): drm/exynos: make wait_for_vblank a manager op drm/exynos: move hdmi's wait_for_vblank to manager_ops drm/exynos: move fimd wait_for_vblank to manager_ops drm/exynos: modify wait_for_vblank of mixer drm/exynos: modify wait_for_vblank of fimd drm/exynos: clear windows in mixer dpms off drm/exynos: clear windows in fimd dpms off
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 | 81 ++++++++-- 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 | 240 +++++++++++++++++---------- 6 files changed, 239 insertions(+), 127 deletions(-)
The wait_for_vblank callback is moved from overlay ops to manager ops of exynos drm driver. Also, the check for DPMS OFF of encoder is removed before calling wait_for_vblank.
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 ++++----------- 2 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index 9c9c2dc..398210d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -74,8 +74,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, @@ -83,7 +81,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); };
/* @@ -186,6 +183,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); @@ -200,6 +199,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); } }
From: Prathyush K prathyush.k@samsung.com
Changelog v2: remove unnecessay wait_for_vblank call. - with this patch, wait_for_vblank callback is moved from overlay ops to manager ops so it should be removed and it doesn't need to wait vblank signal at plane disable.
Changelog v1: The wait_for_vblank callback is moved from overlay ops to manager ops of exynos drm driver. Also, the check for DPMS OFF of encoder is removed before calling wait_for_vblank.
Signed-off-by: Prathyush K prathyush.k@samsung.com Signed-off-by: Inki Dae inki.dae@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 +++--- drivers/gpu/drm/exynos/exynos_drm_encoder.c | 25 ++++--------------------- 2 files changed, 7 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index a4702a8..5a8c1f2 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -74,8 +74,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, @@ -83,7 +81,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); };
/* @@ -186,6 +183,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); @@ -200,6 +199,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 e5001dd..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); } }
@@ -538,14 +531,4 @@ void exynos_drm_encoder_plane_disable(struct drm_encoder *encoder, void *data)
if (overlay_ops && overlay_ops->disable) overlay_ops->disable(manager->dev, zpos); - - /* - * wait for vblank interrupt - * - this makes sure that hardware overlay is disabled to avoid - * for the dma accesses to memory after gem buffer was released - * because the setting for disabling the overlay will be updated - * at vsync. - */ - if (overlay_ops && overlay_ops->wait_for_vblank) - overlay_ops->wait_for_vblank(manager->dev); }
The wait_for_vblank callback of hdmi and mixer is now moved from overlay_ops to manager_ops.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- 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 | 26 +++++++++++++------------- 3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c index 2d11e70..17d26e2 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c @@ -157,6 +157,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, @@ -238,6 +248,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, @@ -291,21 +302,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 40a6e19..b6f12fb 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -788,18 +788,6 @@ 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) - DRM_DEBUG_KMS("vblank wait timed out.\n"); -} - static void mixer_win_mode_set(void *ctx, struct exynos_drm_overlay *overlay) { @@ -885,15 +873,27 @@ static void mixer_win_disable(void *ctx, int win) spin_unlock_irqrestore(&res->reg_slock, flags); }
+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) + DRM_DEBUG_KMS("vblank wait timed out.\n"); +} + static struct exynos_mixer_ops mixer_ops = { /* manager */ .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 wait for vblank callback is moved from overlay_ops to manager_ops for fimd.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 00bd266..1d46286 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -308,12 +308,24 @@ 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); + int ret; + + ret = wait_for((__raw_readl(ctx->regs + VIDCON1) & + VIDCON1_VSTATUS_VSYNC), 50); + if (ret < 0) + 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, @@ -593,22 +605,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); - int ret; - - ret = wait_for((__raw_readl(ctx->regs + VIDCON1) & - VIDCON1_VSTATUS_VSYNC), 50); - if (ret < 0) - 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 = {
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 function of mixer.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 31 ++++++++++++++++++++++++++----- 1 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index b6f12fb..b20c063 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 { @@ -876,12 +878,23 @@ static void mixer_win_disable(void *ctx, int win) 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) + mutex_lock(&mixer_ctx->mixer_mutex); + if (!mixer_ctx->powered) { + mutex_unlock(&mixer_ctx->mixer_mutex); + return; + } + mutex_unlock(&mixer_ctx->mixer_mutex); + + 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 +970,12 @@ 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); + + /* set wait vsync event to zero and wake up queue. */ + if (atomic_read(&ctx->wait_vsync_event)) { + atomic_set(&ctx->wait_vsync_event, 0); + DRM_WAKEUP(&ctx->wait_vsync_queue); + } }
out: @@ -1139,6 +1158,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);
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 function of fimd.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 25 +++++++++++++++++++++---- 1 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 1d46286..1517d15 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -100,6 +100,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; }; @@ -311,11 +313,19 @@ 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); - int ret;
- ret = wait_for((__raw_readl(ctx->regs + VIDCON1) & - VIDCON1_VSTATUS_VSYNC), 50); - if (ret < 0) + if (ctx->suspended) + return; + + 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"); }
@@ -667,6 +677,11 @@ 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. */ + if (atomic_read(&ctx->wait_vsync_event)) { + atomic_set(&ctx->wait_vsync_event, 0); + DRM_WAKEUP(&ctx->wait_vsync_queue); + } out: return IRQ_HANDLED; } @@ -885,6 +900,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;
When mixer is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed. When mixer resumes, the dma will continue from where it left off and will throw a PAGE FAULT since the memory was freed.
This patch fixes the above problem by disabling the mixer windows before disabling the mixer clocks. It also keeps track of which windows were currently active by setting the 'resume' flag. When mixer resumes, the window with a resume flag set is enabled again.
Now if a current fb is removed when mixer is off, mixer_win_disable will set the 'resume' flag of that window to zero and return. So when mixer resumes, that window will not be resumed.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 193 ++++++++++++++++++++------------- 1 files changed, 118 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index b20c063..6c2c499 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -60,6 +60,8 @@ struct hdmi_win_data { unsigned int mode_width; unsigned int mode_height; unsigned int scan_flags; + bool enabled; + bool resume; };
struct mixer_resources { @@ -688,60 +690,6 @@ static int mixer_iommu_on(void *ctx, bool enable) return 0; }
-static void mixer_poweron(struct mixer_context *ctx) -{ - struct mixer_resources *res = &ctx->mixer_res; - - DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); - - mutex_lock(&ctx->mixer_mutex); - if (ctx->powered) { - mutex_unlock(&ctx->mixer_mutex); - return; - } - ctx->powered = true; - mutex_unlock(&ctx->mixer_mutex); - - pm_runtime_get_sync(ctx->dev); - - clk_enable(res->mixer); - if (ctx->vp_enabled) { - clk_enable(res->vp); - clk_enable(res->sclk_mixer); - } - - mixer_reg_write(res, MXR_INT_EN, ctx->int_en); - mixer_win_reset(ctx); -} - -static void mixer_poweroff(struct mixer_context *ctx) -{ - struct mixer_resources *res = &ctx->mixer_res; - - DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); - - mutex_lock(&ctx->mixer_mutex); - if (!ctx->powered) - goto out; - mutex_unlock(&ctx->mixer_mutex); - - ctx->int_en = mixer_reg_read(res, MXR_INT_EN); - - clk_disable(res->mixer); - if (ctx->vp_enabled) { - clk_disable(res->vp); - clk_disable(res->sclk_mixer); - } - - pm_runtime_put_sync(ctx->dev); - - mutex_lock(&ctx->mixer_mutex); - ctx->powered = false; - -out: - mutex_unlock(&ctx->mixer_mutex); -} - static int mixer_enable_vblank(void *ctx, int pipe) { struct mixer_context *mixer_ctx = ctx; @@ -769,27 +717,6 @@ static void mixer_disable_vblank(void *ctx) mixer_reg_writemask(res, MXR_INT_EN, 0, MXR_INT_EN_VSYNC); }
-static void mixer_dpms(void *ctx, int mode) -{ - struct mixer_context *mixer_ctx = ctx; - - DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); - - switch (mode) { - case DRM_MODE_DPMS_ON: - mixer_poweron(mixer_ctx); - break; - case DRM_MODE_DPMS_STANDBY: - case DRM_MODE_DPMS_SUSPEND: - case DRM_MODE_DPMS_OFF: - mixer_poweroff(mixer_ctx); - break; - default: - DRM_DEBUG_KMS("unknown dpms mode: %d\n", mode); - break; - } -} - static void mixer_win_mode_set(void *ctx, struct exynos_drm_overlay *overlay) { @@ -856,6 +783,8 @@ static void mixer_win_commit(void *ctx, int win) vp_video_buffer(mixer_ctx, win); else mixer_graph_buffer(mixer_ctx, win); + + mixer_ctx->win_data[win].enabled = true; }
static void mixer_win_disable(void *ctx, int win) @@ -866,6 +795,14 @@ static void mixer_win_disable(void *ctx, int win)
DRM_DEBUG_KMS("[%d] %s, win: %d\n", __LINE__, __func__, win);
+ mutex_lock(&mixer_ctx->mixer_mutex); + if (!mixer_ctx->powered) { + mutex_unlock(&mixer_ctx->mixer_mutex); + mixer_ctx->win_data[win].resume = false; + return; + } + mutex_unlock(&mixer_ctx->mixer_mutex); + spin_lock_irqsave(&res->reg_slock, flags); mixer_vsync_set_update(mixer_ctx, false);
@@ -873,6 +810,8 @@ static void mixer_win_disable(void *ctx, int win)
mixer_vsync_set_update(mixer_ctx, true); spin_unlock_irqrestore(&res->reg_slock, flags); + + mixer_ctx->win_data[win].enabled = false; }
static void mixer_wait_for_vblank(void *ctx) @@ -898,6 +837,110 @@ static void mixer_wait_for_vblank(void *ctx) DRM_DEBUG_KMS("vblank wait timed out.\n"); }
+static void mixer_window_suspend(struct mixer_context *ctx) +{ + struct hdmi_win_data *win_data; + int i; + + for (i = 0; i < MIXER_WIN_NR; i++) { + win_data = &ctx->win_data[i]; + win_data->resume = win_data->enabled; + mixer_win_disable(ctx, i); + } + mixer_wait_for_vblank(ctx); +} + +static void mixer_window_resume(struct mixer_context *ctx) +{ + struct hdmi_win_data *win_data; + int i; + + for (i = 0; i < MIXER_WIN_NR; i++) { + win_data = &ctx->win_data[i]; + win_data->enabled = win_data->resume; + win_data->resume = false; + } +} + +static void mixer_poweron(struct mixer_context *ctx) +{ + struct mixer_resources *res = &ctx->mixer_res; + + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); + + mutex_lock(&ctx->mixer_mutex); + if (ctx->powered) { + mutex_unlock(&ctx->mixer_mutex); + return; + } + ctx->powered = true; + mutex_unlock(&ctx->mixer_mutex); + + pm_runtime_get_sync(ctx->dev); + + clk_enable(res->mixer); + if (ctx->vp_enabled) { + clk_enable(res->vp); + clk_enable(res->sclk_mixer); + } + + mixer_reg_write(res, MXR_INT_EN, ctx->int_en); + mixer_win_reset(ctx); + + mixer_window_resume(ctx); +} + +static void mixer_poweroff(struct mixer_context *ctx) +{ + struct mixer_resources *res = &ctx->mixer_res; + + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); + + mutex_lock(&ctx->mixer_mutex); + if (!ctx->powered) + goto out; + mutex_unlock(&ctx->mixer_mutex); + + mixer_window_suspend(ctx); + + ctx->int_en = mixer_reg_read(res, MXR_INT_EN); + + clk_disable(res->mixer); + if (ctx->vp_enabled) { + clk_disable(res->vp); + clk_disable(res->sclk_mixer); + } + + pm_runtime_put_sync(ctx->dev); + + mutex_lock(&ctx->mixer_mutex); + ctx->powered = false; + +out: + mutex_unlock(&ctx->mixer_mutex); +} + +static void mixer_dpms(void *ctx, int mode) +{ + struct mixer_context *mixer_ctx = ctx; + + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); + + switch (mode) { + case DRM_MODE_DPMS_ON: + mixer_poweron(mixer_ctx); + break; + case DRM_MODE_DPMS_STANDBY: + case DRM_MODE_DPMS_SUSPEND: + case DRM_MODE_DPMS_OFF: + mixer_poweroff(mixer_ctx); + break; + default: + DRM_DEBUG_KMS("unknown dpms mode: %d\n", mode); + break; + } +} + static struct exynos_mixer_ops mixer_ops = { /* manager */ .iommu_on = mixer_iommu_on,
When fimd is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed. When fimd resumes, the dma will continue from where it left off and will throw a PAGE FAULT since the memory was freed.
This patch fixes the above problem by disabling the fimd windows before disabling the fimd clocks. It also keeps track of which windows were currently active by setting the 'resume' flag. When fimd resumes, the window with a resume flag set is enabled again.
Now if a current fb is removed when fimd is off, fimd_win_disable will set the 'resume' flag of that window to zero and return. So when fimd resumes, that window will not be resumed.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 40 +++++++++++++++++++++++++++++- 1 files changed, 39 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 1517d15..7e66032 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -83,6 +83,7 @@ struct fimd_win_data { unsigned int buf_offsize; unsigned int line_size; /* bytes */ bool enabled; + bool resume; };
struct fimd_context { @@ -596,6 +597,12 @@ static void fimd_win_disable(struct device *dev, int zpos)
win_data = &ctx->win_data[win];
+ if (ctx->suspended) { + /* do not resume this window*/ + win_data->resume = false; + return; + } + /* protect windows */ val = readl(ctx->regs + SHADOWCON); val |= SHADOWCON_WINx_PROTECT(win); @@ -809,11 +816,38 @@ static int fimd_clock(struct fimd_context *ctx, bool enable) return 0; }
+static void fimd_window_suspend(struct device *dev) +{ + struct fimd_context *ctx = get_fimd_context(dev); + struct fimd_win_data *win_data; + int i; + + for (i = 0; i < WINDOWS_NR; i++) { + win_data = &ctx->win_data[i]; + win_data->resume = win_data->enabled; + fimd_win_disable(dev, i); + } + fimd_wait_for_vblank(dev); +} + +static void fimd_window_resume(struct device *dev) +{ + struct fimd_context *ctx = get_fimd_context(dev); + struct fimd_win_data *win_data; + int i; + + for (i = 0; i < WINDOWS_NR; i++) { + win_data = &ctx->win_data[i]; + win_data->enabled = win_data->resume; + win_data->resume = false; + } +} + static int fimd_activate(struct fimd_context *ctx, bool enable) { + struct device *dev = ctx->subdrv.dev; if (enable) { int ret; - struct device *dev = ctx->subdrv.dev;
ret = fimd_clock(ctx, true); if (ret < 0) @@ -824,7 +858,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable) /* if vblank was enabled status, enable it again. */ if (test_and_clear_bit(0, &ctx->irq_flags)) fimd_enable_vblank(dev); + + fimd_window_resume(dev); } else { + fimd_window_suspend(dev); + fimd_clock(ctx, false); ctx->suspended = true; }
2012/12/6 Prathyush K prathyush.k@samsung.com
When fimd is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed. When fimd resumes, the dma will continue from where it left off and will throw a PAGE FAULT since the memory was freed.
This patch fixes the above problem by disabling the fimd windows before disabling the fimd clocks. It also keeps track of which windows were currently active by setting the 'resume' flag. When fimd resumes, the window with a resume flag set is enabled again.
Now if a current fb is removed when fimd is off, fimd_win_disable will set the 'resume' flag of that window to zero and return. So when fimd resumes, that window will not be resumed.
Signed-off-by: Prathyush K prathyush.k@samsung.com
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 40 +++++++++++++++++++++++++++++- 1 files changed, 39 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 1517d15..7e66032 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -83,6 +83,7 @@ struct fimd_win_data { unsigned int buf_offsize; unsigned int line_size; /* bytes */ bool enabled;
bool resume;};
struct fimd_context { @@ -596,6 +597,12 @@ static void fimd_win_disable(struct device *dev, int zpos)
win_data = &ctx->win_data[win];
+ if (ctx->suspended) {
Ok, add the below comment, "if disabling hw overlay is tried when fimd was disabled already then do not access the hardware."
/* do not resume this window*/win_data->resume = false;return;}/* protect windows */ val = readl(ctx->regs + SHADOWCON); val |= SHADOWCON_WINx_PROTECT(win);@@ -809,11 +816,38 @@ static int fimd_clock(struct fimd_context *ctx, bool enable) return 0; }
+static void fimd_window_suspend(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->resume = win_data->enabled;fimd_win_disable(dev, i);}fimd_wait_for_vblank(dev);+}
+static void fimd_window_resume(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->enabled = win_data->resume;win_data->resume = false;
shouldn't the below code be added? fimd_win_enable(dev, i);
you backed up overlay status before suspended but this patch doesn't enable the overlays again. you supposed that the registers would be restored by SoC specific pm framework?
+ }
+}
static int fimd_activate(struct fimd_context *ctx, bool enable) {
struct device *dev = ctx->subdrv.dev; if (enable) { int ret;
struct device *dev = ctx->subdrv.dev; ret = fimd_clock(ctx, true); if (ret < 0)@@ -824,7 +858,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable) /* if vblank was enabled status, enable it again. */ if (test_and_clear_bit(0, &ctx->irq_flags)) fimd_enable_vblank(dev);
fimd_window_resume(dev); } else {fimd_window_suspend(dev);fimd_clock(ctx, false); ctx->suspended = true; }-- 1.7.0.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
2012/12/6 Prathyush K prathyush.k@samsung.com
When fimd is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed.
I think that if fimd was turned off by dpms operation (off only clock and display power) then rmfb request from user should be failed. Otherwise, someone(drm core or specific driver) should turn hw power on and disable hardware overlay and then turn hw power off again so that such exception codes aren't added to specific driver. I'm not sure but there is something to should be considered in drm core.
When fimd resumes, the dma will continue from where it left off and will throw a PAGE FAULT since the memory was freed.
This patch fixes the above problem by disabling the fimd windows before disabling the fimd clocks. It also keeps track of which windows were currently active by setting the 'resume' flag. When fimd resumes, the window with a resume flag set is enabled again.
Now if a current fb is removed when fimd is off, fimd_win_disable will set the 'resume' flag of that window to zero and return. So when fimd resumes, that window will not be resumed.
Signed-off-by: Prathyush K prathyush.k@samsung.com
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 40 +++++++++++++++++++++++++++++- 1 files changed, 39 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 1517d15..7e66032 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -83,6 +83,7 @@ struct fimd_win_data { unsigned int buf_offsize; unsigned int line_size; /* bytes */ bool enabled;
bool resume;};
struct fimd_context { @@ -596,6 +597,12 @@ static void fimd_win_disable(struct device *dev, int zpos)
win_data = &ctx->win_data[win];
if (ctx->suspended) {/* do not resume this window*/win_data->resume = false;return;}/* protect windows */ val = readl(ctx->regs + SHADOWCON); val |= SHADOWCON_WINx_PROTECT(win);@@ -809,11 +816,38 @@ static int fimd_clock(struct fimd_context *ctx, bool enable) return 0; }
+static void fimd_window_suspend(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->resume = win_data->enabled;fimd_win_disable(dev, i);}fimd_wait_for_vblank(dev);+}
+static void fimd_window_resume(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->enabled = win_data->resume;win_data->resume = false;}+}
static int fimd_activate(struct fimd_context *ctx, bool enable) {
struct device *dev = ctx->subdrv.dev; if (enable) { int ret;
struct device *dev = ctx->subdrv.dev; ret = fimd_clock(ctx, true); if (ret < 0)@@ -824,7 +858,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable) /* if vblank was enabled status, enable it again. */ if (test_and_clear_bit(0, &ctx->irq_flags)) fimd_enable_vblank(dev);
fimd_window_resume(dev); } else {fimd_window_suspend(dev);fimd_clock(ctx, false); ctx->suspended = true; }-- 1.7.0.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Dec 7, 2012 at 10:37 AM, Inki Dae inki.dae@samsung.com wrote:
2012/12/6 Prathyush K prathyush.k@samsung.com
When fimd is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed.
I think that if fimd was turned off by dpms operation (off only clock and display power) then rmfb request from user should be failed. Otherwise, someone(drm core or specific driver) should turn hw power on and disable hardware overlay and then turn hw power off again so that such exception codes aren't added to specific driver. I'm not sure but there is something to should be considered in drm core.
Ideally that should true. But we never check for return value of rmFB in our application. If we do not free our framebuffer, application will not try to free it again. So that memory will be locked till drm gets released.
As for your previous question:
The overlays are enabled again by the encoder DPMS function. Initially I was resuming the windows by calling fimd_win_commit inside fimd_window_resume. But I saw that the overlays were getting updated twice. So I removed that from fimd_window_resume.
In case of suspend/resume, the fimd_resume function is calling fimd_apply after it has called fimd_activate. This ensures that the overlays get updated upon fimd_resume as well as DPMS_ON.
Regards, Prathyush
When fimd resumes, the dma will continue from where it left off
and will throw a PAGE FAULT since the memory was freed.
This patch fixes the above problem by disabling the fimd windows before disabling the fimd clocks. It also keeps track of which windows were currently active by setting the 'resume' flag. When fimd resumes, the window with a resume flag set is enabled again.
Now if a current fb is removed when fimd is off, fimd_win_disable will set the 'resume' flag of that window to zero and return. So when fimd resumes, that window will not be resumed.
Signed-off-by: Prathyush K prathyush.k@samsung.com
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 40 +++++++++++++++++++++++++++++- 1 files changed, 39 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 1517d15..7e66032 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -83,6 +83,7 @@ struct fimd_win_data { unsigned int buf_offsize; unsigned int line_size; /* bytes */ bool enabled;
bool resume;};
struct fimd_context { @@ -596,6 +597,12 @@ static void fimd_win_disable(struct device *dev, int zpos)
win_data = &ctx->win_data[win];
if (ctx->suspended) {/* do not resume this window*/win_data->resume = false;return;}/* protect windows */ val = readl(ctx->regs + SHADOWCON); val |= SHADOWCON_WINx_PROTECT(win);@@ -809,11 +816,38 @@ static int fimd_clock(struct fimd_context *ctx, bool enable) return 0; }
+static void fimd_window_suspend(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->resume = win_data->enabled;fimd_win_disable(dev, i);}fimd_wait_for_vblank(dev);+}
+static void fimd_window_resume(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->enabled = win_data->resume;win_data->resume = false;}+}
static int fimd_activate(struct fimd_context *ctx, bool enable) {
struct device *dev = ctx->subdrv.dev; if (enable) { int ret;
struct device *dev = ctx->subdrv.dev; ret = fimd_clock(ctx, true); if (ret < 0)@@ -824,7 +858,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable) /* if vblank was enabled status, enable it again. */ if (test_and_clear_bit(0, &ctx->irq_flags)) fimd_enable_vblank(dev);
fimd_window_resume(dev); } else {fimd_window_suspend(dev);fimd_clock(ctx, false); ctx->suspended = true; }-- 1.7.0.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Dec 7, 2012 at 1:04 PM, Prathyush K prathyush@chromium.org wrote:
On Fri, Dec 7, 2012 at 10:37 AM, Inki Dae inki.dae@samsung.com wrote:
2012/12/6 Prathyush K prathyush.k@samsung.com
When fimd is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed.
I think that if fimd was turned off by dpms operation (off only clock and display power) then rmfb request from user should be failed. Otherwise, someone(drm core or specific driver) should turn hw power on and disable hardware overlay and then turn hw power off again so that such exception codes aren't added to specific driver. I'm not sure but there is something to should be considered in drm core.
Yes I thought of that option i.e. to power on fimd, disable overlay, wait for vblank and then power off fimd.
But this will cause a flicker on the screen as it'll come on for a brief instant and then go off again. I don't think that is correct.
Ideally that should true. But we never check for return value of rmFB in our application. If we do not free our framebuffer, application will not try to free it again. So that memory will be locked till drm gets released.
As for your previous question:
The overlays are enabled again by the encoder DPMS function. Initially I was resuming the windows by calling fimd_win_commit inside fimd_window_resume. But I saw that the overlays were getting updated twice. So I removed that from fimd_window_resume.
In case of suspend/resume, the fimd_resume function is calling fimd_apply after it has called fimd_activate. This ensures that the overlays get updated upon fimd_resume as well as DPMS_ON.
Regards, Prathyush
When fimd resumes, the dma will continue from where it left off
and will throw a PAGE FAULT since the memory was freed.
This patch fixes the above problem by disabling the fimd windows before disabling the fimd clocks. It also keeps track of which windows were currently active by setting the 'resume' flag. When fimd resumes, the window with a resume flag set is enabled again.
Now if a current fb is removed when fimd is off, fimd_win_disable will set the 'resume' flag of that window to zero and return. So when fimd resumes, that window will not be resumed.
Signed-off-by: Prathyush K prathyush.k@samsung.com
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 40 +++++++++++++++++++++++++++++- 1 files changed, 39 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 1517d15..7e66032 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -83,6 +83,7 @@ struct fimd_win_data { unsigned int buf_offsize; unsigned int line_size; /* bytes */ bool enabled;
bool resume;};
struct fimd_context { @@ -596,6 +597,12 @@ static void fimd_win_disable(struct device *dev, int zpos)
win_data = &ctx->win_data[win];
if (ctx->suspended) {/* do not resume this window*/win_data->resume = false;return;}/* protect windows */ val = readl(ctx->regs + SHADOWCON); val |= SHADOWCON_WINx_PROTECT(win);@@ -809,11 +816,38 @@ static int fimd_clock(struct fimd_context *ctx, bool enable) return 0; }
+static void fimd_window_suspend(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->resume = win_data->enabled;fimd_win_disable(dev, i);}fimd_wait_for_vblank(dev);+}
+static void fimd_window_resume(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->enabled = win_data->resume;win_data->resume = false;}+}
static int fimd_activate(struct fimd_context *ctx, bool enable) {
struct device *dev = ctx->subdrv.dev; if (enable) { int ret;
struct device *dev = ctx->subdrv.dev; ret = fimd_clock(ctx, true); if (ret < 0)@@ -824,7 +858,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable) /* if vblank was enabled status, enable it again. */ if (test_and_clear_bit(0, &ctx->irq_flags)) fimd_enable_vblank(dev);
fimd_window_resume(dev); } else {fimd_window_suspend(dev);fimd_clock(ctx, false); ctx->suspended = true; }-- 1.7.0.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
2012/12/7 Prathyush K prathyush@chromium.org
On Fri, Dec 7, 2012 at 1:04 PM, Prathyush K prathyush@chromium.orgwrote:
On Fri, Dec 7, 2012 at 10:37 AM, Inki Dae inki.dae@samsung.com wrote:
2012/12/6 Prathyush K prathyush.k@samsung.com
When fimd is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed.
I think that if fimd was turned off by dpms operation (off only clock and display power) then rmfb request from user should be failed. Otherwise, someone(drm core or specific driver) should turn hw power on and disable hardware overlay and then turn hw power off again so that such exception codes aren't added to specific driver. I'm not sure but there is something to should be considered in drm core.
Yes I thought of that option i.e. to power on fimd, disable overlay, wait for vblank and then power off fimd.
But this will cause a flicker on the screen as it'll come on for a brief instant and then go off again. I don't think that is correct.
Right. but at least, we could avoid the flicker on the screen in case of Exynos drm framework because we could control lcd and display controller power respectively. i.e. it turns lcd power off and display controller power on. Thus, Exynos framework can do it instead of specific drivers. Anyway it's a good as is. But I hope more generic way would come up later.
Now your patch set are being tested and will be merged if no problem because it's best for now.
Thanks, Inki Dae
Ideally that should true. But we never check for return value of rmFB in our application. If we do not free our framebuffer, application will not try to free it again. So that memory will be locked till drm gets released.
As for your previous question:
The overlays are enabled again by the encoder DPMS function. Initially I was resuming the windows by calling fimd_win_commit inside fimd_window_resume. But I saw that the overlays were getting updated twice. So I removed that from fimd_window_resume.
In case of suspend/resume, the fimd_resume function is calling fimd_apply after it has called fimd_activate. This ensures that the overlays get updated upon fimd_resume as well as DPMS_ON.
Regards, Prathyush
When fimd resumes, the dma will continue from where it left off
and will throw a PAGE FAULT since the memory was freed.
This patch fixes the above problem by disabling the fimd windows before disabling the fimd clocks. It also keeps track of which windows were currently active by setting the 'resume' flag. When fimd resumes, the window with a resume flag set is enabled again.
Now if a current fb is removed when fimd is off, fimd_win_disable will set the 'resume' flag of that window to zero and return. So when fimd resumes, that window will not be resumed.
Signed-off-by: Prathyush K prathyush.k@samsung.com
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 40 +++++++++++++++++++++++++++++- 1 files changed, 39 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 1517d15..7e66032 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -83,6 +83,7 @@ struct fimd_win_data { unsigned int buf_offsize; unsigned int line_size; /* bytes */ bool enabled;
bool resume;};
struct fimd_context { @@ -596,6 +597,12 @@ static void fimd_win_disable(struct device *dev, int zpos)
win_data = &ctx->win_data[win];
if (ctx->suspended) {/* do not resume this window*/win_data->resume = false;return;}/* protect windows */ val = readl(ctx->regs + SHADOWCON); val |= SHADOWCON_WINx_PROTECT(win);@@ -809,11 +816,38 @@ static int fimd_clock(struct fimd_context *ctx, bool enable) return 0; }
+static void fimd_window_suspend(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->resume = win_data->enabled;fimd_win_disable(dev, i);}fimd_wait_for_vblank(dev);+}
+static void fimd_window_resume(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->enabled = win_data->resume;win_data->resume = false;}+}
static int fimd_activate(struct fimd_context *ctx, bool enable) {
struct device *dev = ctx->subdrv.dev; if (enable) { int ret;
struct device *dev = ctx->subdrv.dev; ret = fimd_clock(ctx, true); if (ret < 0)@@ -824,7 +858,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable) /* if vblank was enabled status, enable it again. */ if (test_and_clear_bit(0, &ctx->irq_flags)) fimd_enable_vblank(dev);
fimd_window_resume(dev); } else {fimd_window_suspend(dev);fimd_clock(ctx, false); ctx->suspended = true; }-- 1.7.0.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Dec 6, 2012 at 6:46 AM, Prathyush K prathyush.k@samsung.com wrote:
When fimd is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed. When fimd resumes, the dma will continue from where it left off and will throw a PAGE FAULT since the memory was freed.
This patch fixes the above problem by disabling the fimd windows before disabling the fimd clocks. It also keeps track of which windows were currently active by setting the 'resume' flag. When fimd resumes, the window with a resume flag set is enabled again.
Now if a current fb is removed when fimd is off, fimd_win_disable will set the 'resume' flag of that window to zero and return. So when fimd resumes, that window will not be resumed.
Signed-off-by: Prathyush K prathyush.k@samsung.com
Hi Prathyush,
I think you removed my signed-off/ownership on this patch. See: http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel.git;a=commit...
Stephane
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 40 +++++++++++++++++++++++++++++- 1 files changed, 39 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 1517d15..7e66032 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -83,6 +83,7 @@ struct fimd_win_data { unsigned int buf_offsize; unsigned int line_size; /* bytes */ bool enabled;
bool resume;};
struct fimd_context { @@ -596,6 +597,12 @@ static void fimd_win_disable(struct device *dev, int zpos)
win_data = &ctx->win_data[win];
if (ctx->suspended) {/* do not resume this window*/win_data->resume = false;return;}/* protect windows */ val = readl(ctx->regs + SHADOWCON); val |= SHADOWCON_WINx_PROTECT(win);@@ -809,11 +816,38 @@ static int fimd_clock(struct fimd_context *ctx, bool enable) return 0; }
+static void fimd_window_suspend(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->resume = win_data->enabled;fimd_win_disable(dev, i);}fimd_wait_for_vblank(dev);+}
+static void fimd_window_resume(struct device *dev) +{
struct fimd_context *ctx = get_fimd_context(dev);struct fimd_win_data *win_data;int i;for (i = 0; i < WINDOWS_NR; i++) {win_data = &ctx->win_data[i];win_data->enabled = win_data->resume;win_data->resume = false;}+}
static int fimd_activate(struct fimd_context *ctx, bool enable) {
struct device *dev = ctx->subdrv.dev; if (enable) { int ret;
struct device *dev = ctx->subdrv.dev; ret = fimd_clock(ctx, true); if (ret < 0)@@ -824,7 +858,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable) /* if vblank was enabled status, enable it again. */ if (test_and_clear_bit(0, &ctx->irq_flags)) fimd_enable_vblank(dev);
fimd_window_resume(dev); } else {fimd_window_suspend(dev);fimd_clock(ctx, false); ctx->suspended = true; }-- 1.7.0.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
2012/12/6 Prathyush K prathyush.k@samsung.com
This patchset fixes the various issues which result in getting a PAGE FAULT while using fimd and hdmi in exynos drm.
Changelog v2: Seperated patches into drm framework and fimd/mixer patches. Not using patch drm/exynos: do not disable crtc if already off Added two new patches for clearing windows during dpms off
Patch 1: make wait_for_vblank a manager op Patch 2: move hdmi's wait_for_vblank to manager_ops Patch 3: move fimd wait_for_vblank to manager_ops Currently, we do not call wait for vblank if encoder is in DPMS_OFF state inside exynos_drm_encoder_complete_scanout function. This is because wait for vblank is treated as an overlay op. This can be modified to a manager_op thus removing the above check for DPMS_OFF. This ensures that the hardware overlay actually gets disabled while removing the current framebuffer.
Patch 4: modify wait_for_vblank of mixer Patch 5: modify wait_for_vblank of fimd This modifies the wait_for_vblank functions to use wait queues thus ensuring that the current task goes to sleep without taking up CPU while waiting. Also, if crtc is off, then it returns without waiting.
Patch 6: clear windows in mixer dpms off Patch 7: clear windows in fimd dpms off When a crtc is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed. When fimd/mixer resumes, the dma will continue from where it left off and will throw a PAGE FAULT since the memory was freed. This can be fixed by disabling the windows before disabling the clocks. We should also keep track of which windows were active. This is done by setting the resume flag in the window properties. Now if a current fb is removed when the crtc is off, win_disable will set the 'resume' flag of that window to zero and return. So when fimd/mixer resumes, that window will not be resumed.
And "drm/exynos: do not disable crtc if already off"? otherwise isn't there any problem without this patch?
Prathyush K (7): drm/exynos: make wait_for_vblank a manager op drm/exynos: move hdmi's wait_for_vblank to manager_ops drm/exynos: move fimd wait_for_vblank to manager_ops drm/exynos: modify wait_for_vblank of mixer drm/exynos: modify wait_for_vblank of fimd drm/exynos: clear windows in mixer dpms off drm/exynos: clear windows in fimd dpms off
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 | 81 ++++++++-- 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 | 240 +++++++++++++++++---------- 6 files changed, 239 insertions(+), 127 deletions(-)
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
This is taken care inside fimd and mixer with the clearing windows in dpms off patch itself.
I've added a check inside win_disable function (for both fimd/mixer) where we set the 'resume' flag to zero and return.
So the "drm/exynos: do not disable crtc if already off" patch is not required.
Regards, Prathyush
On Fri, Dec 7, 2012 at 9:43 AM, Inki Dae inki.dae@samsung.com wrote:
2012/12/6 Prathyush K prathyush.k@samsung.com
This patchset fixes the various issues which result in getting a PAGE FAULT while using fimd and hdmi in exynos drm.
Changelog v2: Seperated patches into drm framework and fimd/mixer patches. Not using patch drm/exynos: do not disable crtc if already off Added two new patches for clearing windows during dpms off
Patch 1: make wait_for_vblank a manager op Patch 2: move hdmi's wait_for_vblank to manager_ops Patch 3: move fimd wait_for_vblank to manager_ops Currently, we do not call wait for vblank if encoder is in DPMS_OFF state inside exynos_drm_encoder_complete_scanout function. This is because wait for vblank is treated as an overlay op. This can be modified to a manager_op thus removing the above check for DPMS_OFF. This ensures that the hardware overlay actually gets disabled while removing the current framebuffer.
Patch 4: modify wait_for_vblank of mixer Patch 5: modify wait_for_vblank of fimd This modifies the wait_for_vblank functions to use wait queues thus ensuring that the current task goes to sleep without taking up CPU while waiting. Also, if crtc is off, then it returns without waiting.
Patch 6: clear windows in mixer dpms off Patch 7: clear windows in fimd dpms off When a crtc is turned off, we disable the clocks which will stop the dma. Now if we remove the current framebuffer, we cannot disable the overlay but the current framebuffer will still be freed. When fimd/mixer resumes, the dma will continue from where it left off and will throw a PAGE FAULT since the memory was freed. This can be fixed by disabling the windows before disabling the clocks. We should also keep track of which windows were active. This is done by setting the resume flag in the window properties. Now if a current fb is removed when the crtc is off, win_disable will set the 'resume' flag of that window to zero and return. So when fimd/mixer resumes, that window will not be resumed.
And "drm/exynos: do not disable crtc if already off"? otherwise isn't there any problem without this patch?
Prathyush K (7): drm/exynos: make wait_for_vblank a manager op drm/exynos: move hdmi's wait_for_vblank to manager_ops drm/exynos: move fimd wait_for_vblank to manager_ops drm/exynos: modify wait_for_vblank of mixer drm/exynos: modify wait_for_vblank of fimd drm/exynos: clear windows in mixer dpms off drm/exynos: clear windows in fimd dpms off
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 | 81 ++++++++-- 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 | 240 +++++++++++++++++---------- 6 files changed, 239 insertions(+), 127 deletions(-)
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Inki Dae -
Prathyush K -
Prathyush K -
Stéphane Marchesin