| Message ID | 1383063198-10526-26-git-send-email-seanpaul@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
As I mentioned already, I'll not merge eDP related codes yet. So can you rebase patche 25 through 26 to top of patch 20? Thanks, Inki Dae > -----Original Message----- > From: Sean Paul [mailto:seanpaul@chromium.org] > Sent: Wednesday, October 30, 2013 1:13 AM > To: dri-devel@lists.freedesktop.org; inki.dae@samsung.com > Cc: airlied@linux.ie; tomasz.figa@gmail.com; marcheu@chromium.org; Sean > Paul > Subject: [PATCH v3 25/32] drm/exynos: Clean up FIMD power on/off routines > > This patch separates the fimd_activate function into poweron/poweroff > functions to be more consistent with the other drivers in exynos drm. It > also properly cleans up after failures in poweron. The functions have > also been shuffled around such that they are all in the same > spot in the file and poweron/poweroff can be called from the dpms function. > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > > Changes in v2: > - Added to the patchset > Changes in v3: None > > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 247 +++++++++++++++++--------- > ----- > 1 file changed, 135 insertions(+), 112 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index c6a05f6..ba12916 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -324,6 +324,12 @@ static void fimd_win_commit(struct exynos_drm_manager > *mgr, int zpos) > > win_data = &ctx->win_data[win]; > > + /* If suspended, enable this on resume */ > + if (ctx->suspended) { > + win_data->resume = true; > + return; > + } > + > /* > * SHADOWCON/PRTCON register is used for enabling timing. > * > @@ -505,35 +511,6 @@ static void fimd_mgr_remove(struct exynos_drm_manager > *mgr) > drm_iommu_detach_device(ctx->drm_dev, ctx->dev); > } > > -static void fimd_dpms(struct exynos_drm_manager *mgr, int mode) > -{ > - struct fimd_context *ctx = mgr->ctx; > - > - DRM_DEBUG_KMS("%d\n", mode); > - > - switch (mode) { > - case DRM_MODE_DPMS_ON: > - /* > - * enable fimd hardware only if suspended status. > - * > - * P.S. fimd_dpms function would be called at booting time so > - * clk_enable could be called double time. > - */ > - if (ctx->suspended) > - pm_runtime_get_sync(ctx->dev); > - break; > - case DRM_MODE_DPMS_STANDBY: > - case DRM_MODE_DPMS_SUSPEND: > - case DRM_MODE_DPMS_OFF: > - if (!ctx->suspended) > - pm_runtime_put_sync(ctx->dev); > - break; > - default: > - DRM_DEBUG_KMS("unspecified mode %d\n", mode); > - break; > - } > -} > - > static u32 fimd_calc_clkdiv(struct fimd_context *ctx, > const struct drm_display_mode *mode) > { > @@ -726,6 +703,130 @@ static void fimd_wait_for_vblank(struct > exynos_drm_manager *mgr) > DRM_DEBUG_KMS("vblank wait timed out.\n"); > } > > +static void fimd_window_suspend(struct exynos_drm_manager *mgr) > +{ > + struct fimd_context *ctx = mgr->ctx; > + 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; > + if (win_data->enabled) > + fimd_win_disable(mgr, i); > + } > + fimd_wait_for_vblank(mgr); > +} > + > +static void fimd_window_resume(struct exynos_drm_manager *mgr) > +{ > + struct fimd_context *ctx = mgr->ctx; > + 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_poweron(struct exynos_drm_manager *mgr) > +{ > + struct fimd_context *ctx = mgr->ctx; > + int ret; > + > + if (!ctx->suspended) > + return 0; > + > + ctx->suspended = false; > + > + ret = clk_prepare_enable(ctx->bus_clk); > + if (ret < 0) { > + DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", > ret); > + goto bus_clk_err; > + } > + > + ret = clk_prepare_enable(ctx->lcd_clk); > + if (ret < 0) { > + DRM_ERROR("Failed to prepare_enable the lcd clk [%d]\n", > ret); > + goto lcd_clk_err; > + } > + > + /* if vblank was enabled status, enable it again. */ > + if (test_and_clear_bit(0, &ctx->irq_flags)) { > + ret = fimd_enable_vblank(mgr); > + if (ret) { > + DRM_ERROR("Failed to re-enable vblank [%d]\n", ret); > + goto enable_vblank_err; > + } > + } > + > + fimd_window_resume(mgr); > + > + fimd_apply(mgr); > + > + return 0; > + > +enable_vblank_err: > + clk_disable_unprepare(ctx->lcd_clk); > +lcd_clk_err: > + clk_disable_unprepare(ctx->bus_clk); > +bus_clk_err: > + ctx->suspended = true; > + return ret; > +} > + > +static int fimd_poweroff(struct exynos_drm_manager *mgr) > +{ > + struct fimd_context *ctx = mgr->ctx; > + > + if (ctx->suspended) > + return 0; > + > + /* > + * We need to make sure that all windows are disabled before we > + * suspend that connector. Otherwise we might try to scan from > + * a destroyed buffer later. > + */ > + fimd_window_suspend(mgr); > + > + clk_disable_unprepare(ctx->lcd_clk); > + clk_disable_unprepare(ctx->bus_clk); > + > + ctx->suspended = true; > + return 0; > +} > + > +static void fimd_dpms(struct exynos_drm_manager *mgr, int mode) > +{ > + struct fimd_context *ctx = mgr->ctx; > + > + DRM_DEBUG_KMS("%s, %d\n", __FILE__, mode); > + > + switch (mode) { > + case DRM_MODE_DPMS_ON: > + /* > + * enable fimd hardware only if suspended status. > + * > + * P.S. fimd_dpms function would be called at booting time so > + * clk_enable could be called double time. > + */ > + if (ctx->suspended) > + pm_runtime_get_sync(ctx->dev); > + break; > + case DRM_MODE_DPMS_STANDBY: > + case DRM_MODE_DPMS_SUSPEND: > + case DRM_MODE_DPMS_OFF: > + if (!ctx->suspended) > + pm_runtime_put_sync(ctx->dev); > + break; > + default: > + DRM_DEBUG_KMS("unspecified mode %d\n", mode); > + break; > + } > +} > + > static struct exynos_drm_manager_ops fimd_manager_ops = { > .initialize = fimd_mgr_initialize, > .remove = fimd_mgr_remove, > @@ -786,85 +887,6 @@ static void fimd_clear_win(struct fimd_context *ctx, > int win) > fimd_shadow_protect_win(ctx, win, false); > } > > -static int fimd_clock(struct fimd_context *ctx, bool enable) > -{ > - if (enable) { > - int ret; > - > - ret = clk_prepare_enable(ctx->bus_clk); > - if (ret < 0) > - return ret; > - > - ret = clk_prepare_enable(ctx->lcd_clk); > - if (ret < 0) { > - clk_disable_unprepare(ctx->bus_clk); > - return ret; > - } > - } else { > - clk_disable_unprepare(ctx->lcd_clk); > - clk_disable_unprepare(ctx->bus_clk); > - } > - > - return 0; > -} > - > -static void fimd_window_suspend(struct exynos_drm_manager *mgr) > -{ > - struct fimd_context *ctx = mgr->ctx; > - 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(mgr, i); > - } > - fimd_wait_for_vblank(mgr); > -} > - > -static void fimd_window_resume(struct exynos_drm_manager *mgr) > -{ > - struct fimd_context *ctx = mgr->ctx; > - 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 exynos_drm_manager *mgr, bool enable) > -{ > - struct fimd_context *ctx = mgr->ctx; > - > - if (enable) { > - int ret; > - > - ret = fimd_clock(ctx, true); > - if (ret < 0) > - return ret; > - > - ctx->suspended = false; > - > - /* if vblank was enabled status, enable it again. */ > - if (test_and_clear_bit(0, &ctx->irq_flags)) > - fimd_enable_vblank(mgr); > - > - fimd_window_resume(mgr); > - > - fimd_apply(mgr); > - } else { > - fimd_window_suspend(mgr); > - > - fimd_clock(ctx, false); > - ctx->suspended = true; > - } > - > - return 0; > -} > - > static int fimd_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -881,6 +903,7 @@ static int fimd_probe(struct platform_device *pdev) > return -ENOMEM; > > ctx->dev = dev; > + ctx->suspended = true; > > if (of_property_read_bool(dev->of_node, "samsung,invert-vden")) > ctx->vidcon1 |= VIDCON1_INV_VDEN; > @@ -967,7 +990,7 @@ static int fimd_suspend(struct device *dev) > * because the usage_count of pm runtime is more than 1. > */ > if (!pm_runtime_suspended(dev)) > - return fimd_activate(mgr, false); > + return fimd_poweroff(mgr); > > return 0; > } > @@ -984,7 +1007,7 @@ static int fimd_resume(struct device *dev) > if (pm_runtime_suspended(dev)) > return 0; > > - return fimd_activate(mgr, true); > + return fimd_poweron(mgr); > } > #endif > > @@ -993,14 +1016,14 @@ static int fimd_runtime_suspend(struct device *dev) > { > struct exynos_drm_manager *mgr = get_fimd_manager(dev); > > - return fimd_activate(mgr, false); > + return fimd_poweroff(mgr); > } > > static int fimd_runtime_resume(struct device *dev) > { > struct exynos_drm_manager *mgr = get_fimd_manager(dev); > > - return fimd_activate(mgr, true); > + return fimd_poweron(mgr); > } > #endif > > -- > 1.8.4
Hi Sean, On Tuesday 29 of October 2013 12:13:11 Sean Paul wrote: > This patch separates the fimd_activate function into poweron/poweroff > functions to be more consistent with the other drivers in exynos drm. It > also properly cleans up after failures in poweron. The functions have > also been shuffled around such that they are all in the same > spot in the file and poweron/poweroff can be called from the dpms function. > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > > Changes in v2: > - Added to the patchset > Changes in v3: None > > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 247 +++++++++++++++++-------------- > 1 file changed, 135 insertions(+), 112 deletions(-) Reviewed-by: Tomasz Figa <t.figa@samsung.com> Best regards, Tomasz
> -----Original Message----- > From: Tomasz Figa [mailto:tomasz.figa@gmail.com] > Sent: Monday, November 11, 2013 11:09 AM > To: Inki Dae > Cc: 'Sean Paul'; dri-devel@lists.freedesktop.org; airlied@linux.ie; > marcheu@chromium.org > Subject: Re: [PATCH v3 25/32] drm/exynos: Clean up FIMD power on/off > routines > > On Thursday 31 of October 2013 19:54:18 Inki Dae wrote: > > > > As I mentioned already, I'll not merge eDP related codes yet. So can you > > rebase patche 25 through 26 to top of patch 20? > > Why not? IMHO Exynos DRM is already the only way to support Exynos DP > and it should simplify things. Of course, only if done properly and not > breaking other things. > Your late comment. I wanted to merge only the re-factoring patch series because Sean missed CCing proper person and I was not sure that moving eDP driver into Exynos drm is good way or not _for the moment_. So I just wanted to have time enough about merging this patch and watch out how other maintainers will handle the similar thing. However, s3c-fb driver wouldn't be used anymore and Jingoo commented enough - for this, see the original email thread - so the moving is reasonable to me. Thanks, Inki Dae > Best regards, > Tomasz
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index c6a05f6..ba12916 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -324,6 +324,12 @@ static void fimd_win_commit(struct exynos_drm_manager *mgr, int zpos) win_data = &ctx->win_data[win]; + /* If suspended, enable this on resume */ + if (ctx->suspended) { + win_data->resume = true; + return; + } + /* * SHADOWCON/PRTCON register is used for enabling timing. * @@ -505,35 +511,6 @@ static void fimd_mgr_remove(struct exynos_drm_manager *mgr) drm_iommu_detach_device(ctx->drm_dev, ctx->dev); } -static void fimd_dpms(struct exynos_drm_manager *mgr, int mode) -{ - struct fimd_context *ctx = mgr->ctx; - - DRM_DEBUG_KMS("%d\n", mode); - - switch (mode) { - case DRM_MODE_DPMS_ON: - /* - * enable fimd hardware only if suspended status. - * - * P.S. fimd_dpms function would be called at booting time so - * clk_enable could be called double time. - */ - if (ctx->suspended) - pm_runtime_get_sync(ctx->dev); - break; - case DRM_MODE_DPMS_STANDBY: - case DRM_MODE_DPMS_SUSPEND: - case DRM_MODE_DPMS_OFF: - if (!ctx->suspended) - pm_runtime_put_sync(ctx->dev); - break; - default: - DRM_DEBUG_KMS("unspecified mode %d\n", mode); - break; - } -} - static u32 fimd_calc_clkdiv(struct fimd_context *ctx, const struct drm_display_mode *mode) { @@ -726,6 +703,130 @@ static void fimd_wait_for_vblank(struct exynos_drm_manager *mgr) DRM_DEBUG_KMS("vblank wait timed out.\n"); } +static void fimd_window_suspend(struct exynos_drm_manager *mgr) +{ + struct fimd_context *ctx = mgr->ctx; + 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; + if (win_data->enabled) + fimd_win_disable(mgr, i); + } + fimd_wait_for_vblank(mgr); +} + +static void fimd_window_resume(struct exynos_drm_manager *mgr) +{ + struct fimd_context *ctx = mgr->ctx; + 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_poweron(struct exynos_drm_manager *mgr) +{ + struct fimd_context *ctx = mgr->ctx; + int ret; + + if (!ctx->suspended) + return 0; + + ctx->suspended = false; + + ret = clk_prepare_enable(ctx->bus_clk); + if (ret < 0) { + DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret); + goto bus_clk_err; + } + + ret = clk_prepare_enable(ctx->lcd_clk); + if (ret < 0) { + DRM_ERROR("Failed to prepare_enable the lcd clk [%d]\n", ret); + goto lcd_clk_err; + } + + /* if vblank was enabled status, enable it again. */ + if (test_and_clear_bit(0, &ctx->irq_flags)) { + ret = fimd_enable_vblank(mgr); + if (ret) { + DRM_ERROR("Failed to re-enable vblank [%d]\n", ret); + goto enable_vblank_err; + } + } + + fimd_window_resume(mgr); + + fimd_apply(mgr); + + return 0; + +enable_vblank_err: + clk_disable_unprepare(ctx->lcd_clk); +lcd_clk_err: + clk_disable_unprepare(ctx->bus_clk); +bus_clk_err: + ctx->suspended = true; + return ret; +} + +static int fimd_poweroff(struct exynos_drm_manager *mgr) +{ + struct fimd_context *ctx = mgr->ctx; + + if (ctx->suspended) + return 0; + + /* + * We need to make sure that all windows are disabled before we + * suspend that connector. Otherwise we might try to scan from + * a destroyed buffer later. + */ + fimd_window_suspend(mgr); + + clk_disable_unprepare(ctx->lcd_clk); + clk_disable_unprepare(ctx->bus_clk); + + ctx->suspended = true; + return 0; +} + +static void fimd_dpms(struct exynos_drm_manager *mgr, int mode) +{ + struct fimd_context *ctx = mgr->ctx; + + DRM_DEBUG_KMS("%s, %d\n", __FILE__, mode); + + switch (mode) { + case DRM_MODE_DPMS_ON: + /* + * enable fimd hardware only if suspended status. + * + * P.S. fimd_dpms function would be called at booting time so + * clk_enable could be called double time. + */ + if (ctx->suspended) + pm_runtime_get_sync(ctx->dev); + break; + case DRM_MODE_DPMS_STANDBY: + case DRM_MODE_DPMS_SUSPEND: + case DRM_MODE_DPMS_OFF: + if (!ctx->suspended) + pm_runtime_put_sync(ctx->dev); + break; + default: + DRM_DEBUG_KMS("unspecified mode %d\n", mode); + break; + } +} + static struct exynos_drm_manager_ops fimd_manager_ops = { .initialize = fimd_mgr_initialize, .remove = fimd_mgr_remove, @@ -786,85 +887,6 @@ static void fimd_clear_win(struct fimd_context *ctx, int win) fimd_shadow_protect_win(ctx, win, false); } -static int fimd_clock(struct fimd_context *ctx, bool enable) -{ - if (enable) { - int ret; - - ret = clk_prepare_enable(ctx->bus_clk); - if (ret < 0) - return ret; - - ret = clk_prepare_enable(ctx->lcd_clk); - if (ret < 0) { - clk_disable_unprepare(ctx->bus_clk); - return ret; - } - } else { - clk_disable_unprepare(ctx->lcd_clk); - clk_disable_unprepare(ctx->bus_clk); - } - - return 0; -} - -static void fimd_window_suspend(struct exynos_drm_manager *mgr) -{ - struct fimd_context *ctx = mgr->ctx; - 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(mgr, i); - } - fimd_wait_for_vblank(mgr); -} - -static void fimd_window_resume(struct exynos_drm_manager *mgr) -{ - struct fimd_context *ctx = mgr->ctx; - 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 exynos_drm_manager *mgr, bool enable) -{ - struct fimd_context *ctx = mgr->ctx; - - if (enable) { - int ret; - - ret = fimd_clock(ctx, true); - if (ret < 0) - return ret; - - ctx->suspended = false; - - /* if vblank was enabled status, enable it again. */ - if (test_and_clear_bit(0, &ctx->irq_flags)) - fimd_enable_vblank(mgr); - - fimd_window_resume(mgr); - - fimd_apply(mgr); - } else { - fimd_window_suspend(mgr); - - fimd_clock(ctx, false); - ctx->suspended = true; - } - - return 0; -} - static int fimd_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -881,6 +903,7 @@ static int fimd_probe(struct platform_device *pdev) return -ENOMEM; ctx->dev = dev; + ctx->suspended = true; if (of_property_read_bool(dev->of_node, "samsung,invert-vden")) ctx->vidcon1 |= VIDCON1_INV_VDEN; @@ -967,7 +990,7 @@ static int fimd_suspend(struct device *dev) * because the usage_count of pm runtime is more than 1. */ if (!pm_runtime_suspended(dev)) - return fimd_activate(mgr, false); + return fimd_poweroff(mgr); return 0; } @@ -984,7 +1007,7 @@ static int fimd_resume(struct device *dev) if (pm_runtime_suspended(dev)) return 0; - return fimd_activate(mgr, true); + return fimd_poweron(mgr); } #endif @@ -993,14 +1016,14 @@ static int fimd_runtime_suspend(struct device *dev) { struct exynos_drm_manager *mgr = get_fimd_manager(dev); - return fimd_activate(mgr, false); + return fimd_poweroff(mgr); } static int fimd_runtime_resume(struct device *dev) { struct exynos_drm_manager *mgr = get_fimd_manager(dev); - return fimd_activate(mgr, true); + return fimd_poweron(mgr); } #endif
This patch separates the fimd_activate function into poweron/poweroff functions to be more consistent with the other drivers in exynos drm. It also properly cleans up after failures in poweron. The functions have also been shuffled around such that they are all in the same spot in the file and poweron/poweroff can be called from the dpms function. Signed-off-by: Sean Paul <seanpaul@chromium.org> --- Changes in v2: - Added to the patchset Changes in v3: None drivers/gpu/drm/exynos/exynos_drm_fimd.c | 247 +++++++++++++++++-------------- 1 file changed, 135 insertions(+), 112 deletions(-)