[PATCH v10 17/17] drm/exynos: split exynos_crtc->dpms in enable() and disable()
Inki Dae
inki.dae at samsung.com
Wed Jun 3 00:53:23 PDT 2015
Hi,
On 2015년 06월 02일 00:04, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>
> To follow more closely the new atomic API we split the dpms()
> helper into the enable() and disable() helper to get exactly the
> same semantics.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> ---
> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 87 ++++++------------------------
> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 8 +--
> drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 ++-
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 69 +++++-------------------
> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 53 +++++++-----------
> drivers/gpu/drm/exynos/exynos_mixer.c | 26 +++------
> 6 files changed, 62 insertions(+), 187 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index f29e4be..d659ba2 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -603,75 +603,39 @@ static void decon_init(struct decon_context *ctx)
> writel(VIDCON1_VCLK_HOLD, ctx->regs + VIDCON1(0));
> }
>
> -static int decon_poweron(struct decon_context *ctx)
> +static void decon_enable(struct exynos_drm_crtc *crtc)
> {
> - int ret;
> + struct decon_context *ctx = crtc->ctx;
>
> if (!ctx->suspended)
> - return 0;
> + return;
>
> ctx->suspended = false;
>
> pm_runtime_get_sync(ctx->dev);
>
> - ret = clk_prepare_enable(ctx->pclk);
> - if (ret < 0) {
> - DRM_ERROR("Failed to prepare_enable the pclk [%d]\n", ret);
> - goto pclk_err;
> - }
> -
> - ret = clk_prepare_enable(ctx->aclk);
> - if (ret < 0) {
> - DRM_ERROR("Failed to prepare_enable the aclk [%d]\n", ret);
> - goto aclk_err;
> - }
> -
> - ret = clk_prepare_enable(ctx->eclk);
> - if (ret < 0) {
> - DRM_ERROR("Failed to prepare_enable the eclk [%d]\n", ret);
> - goto eclk_err;
> - }
> -
> - ret = clk_prepare_enable(ctx->vclk);
> - if (ret < 0) {
> - DRM_ERROR("Failed to prepare_enable the vclk [%d]\n", ret);
> - goto vclk_err;
> - }
> + clk_prepare_enable(ctx->pclk);
> + clk_prepare_enable(ctx->aclk);
> + clk_prepare_enable(ctx->eclk);
> + clk_prepare_enable(ctx->vclk);
Merged this patch series to exynos-drm-next. However, this patch
especially above codes is required for more clean-up. Even though
decon_enable function never return error number, I think its internal
codes should be considered for some exception cases to check where an
error occurred at. So could you post the clean-up patch?
Thanks,
Inki Dae
>
> decon_init(ctx);
>
> /* if vblank was enabled status, enable it again. */
> - if (test_and_clear_bit(0, &ctx->irq_flags)) {
> - ret = decon_enable_vblank(ctx->crtc);
> - if (ret) {
> - DRM_ERROR("Failed to re-enable vblank [%d]\n", ret);
> - goto err;
> - }
> - }
> + if (test_and_clear_bit(0, &ctx->irq_flags))
> + decon_enable_vblank(ctx->crtc);
>
> decon_window_resume(ctx);
>
> decon_apply(ctx);
> -
> - return 0;
> -
> -err:
> - clk_disable_unprepare(ctx->vclk);
> -vclk_err:
> - clk_disable_unprepare(ctx->eclk);
> -eclk_err:
> - clk_disable_unprepare(ctx->aclk);
> -aclk_err:
> - clk_disable_unprepare(ctx->pclk);
> -pclk_err:
> - ctx->suspended = true;
> - return ret;
> }
>
> -static int decon_poweroff(struct decon_context *ctx)
> +static void decon_disable(struct exynos_drm_crtc *crtc)
> {
> + struct decon_context *ctx = crtc->ctx;
> +
> if (ctx->suspended)
> - return 0;
> + return;
>
> /*
> * We need to make sure that all windows are disabled before we
> @@ -688,30 +652,11 @@ static int decon_poweroff(struct decon_context *ctx)
> pm_runtime_put_sync(ctx->dev);
>
> ctx->suspended = true;
> - return 0;
> -}
> -
> -static void decon_dpms(struct exynos_drm_crtc *crtc, int mode)
> -{
> - DRM_DEBUG_KMS("%s, %d\n", __FILE__, mode);
> -
> - switch (mode) {
> - case DRM_MODE_DPMS_ON:
> - decon_poweron(crtc->ctx);
> - break;
> - case DRM_MODE_DPMS_STANDBY:
> - case DRM_MODE_DPMS_SUSPEND:
> - case DRM_MODE_DPMS_OFF:
> - decon_poweroff(crtc->ctx);
> - break;
> - default:
> - DRM_DEBUG_KMS("unspecified mode %d\n", mode);
> - break;
> - }
> }
>
> static const struct exynos_drm_crtc_ops decon_crtc_ops = {
> - .dpms = decon_dpms,
> + .enable = decon_enable,
> + .disable = decon_disable,
> .mode_fixup = decon_mode_fixup,
> .commit = decon_commit,
> .enable_vblank = decon_enable_vblank,
> @@ -796,7 +741,7 @@ static void decon_unbind(struct device *dev, struct device *master,
> {
> struct decon_context *ctx = dev_get_drvdata(dev);
>
> - decon_dpms(ctx->crtc, DRM_MODE_DPMS_OFF);
> + decon_disable(ctx->crtc);
>
> if (ctx->display)
> exynos_dpi_remove(ctx->display);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index b7c6d51..644b4b7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -29,8 +29,8 @@ static void exynos_drm_crtc_enable(struct drm_crtc *crtc)
> if (exynos_crtc->enabled)
> return;
>
> - if (exynos_crtc->ops->dpms)
> - exynos_crtc->ops->dpms(exynos_crtc, DRM_MODE_DPMS_ON);
> + if (exynos_crtc->ops->enable)
> + exynos_crtc->ops->enable(exynos_crtc);
>
> exynos_crtc->enabled = true;
>
> @@ -51,8 +51,8 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
>
> drm_crtc_vblank_off(crtc);
>
> - if (exynos_crtc->ops->dpms)
> - exynos_crtc->ops->dpms(exynos_crtc, DRM_MODE_DPMS_OFF);
> + if (exynos_crtc->ops->disable)
> + exynos_crtc->ops->disable(exynos_crtc);
>
> exynos_crtc->enabled = false;
> }
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index 86d6894..1c66f65 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -157,7 +157,8 @@ struct exynos_drm_display {
> /*
> * Exynos drm crtc ops
> *
> - * @dpms: control device power.
> + * @enable: enable the device
> + * @disable: disable the device
> * @mode_fixup: fix mode data before applying it
> * @commit: set current hw specific display mode to hw.
> * @enable_vblank: specific driver callback for enabling vblank interrupt.
> @@ -175,7 +176,8 @@ struct exynos_drm_display {
> */
> struct exynos_drm_crtc;
> struct exynos_drm_crtc_ops {
> - void (*dpms)(struct exynos_drm_crtc *crtc, int mode);
> + void (*enable)(struct exynos_drm_crtc *crtc);
> + void (*disable)(struct exynos_drm_crtc *crtc);
> bool (*mode_fixup)(struct exynos_drm_crtc *crtc,
> const struct drm_display_mode *mode,
> struct drm_display_mode *adjusted_mode);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index b326b31..9661853 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -805,57 +805,35 @@ static void fimd_apply(struct fimd_context *ctx)
> fimd_commit(ctx->crtc);
> }
>
> -static int fimd_poweron(struct fimd_context *ctx)
> +static void fimd_enable(struct exynos_drm_crtc *crtc)
> {
> - int ret;
> + struct fimd_context *ctx = crtc->ctx;
>
> if (!ctx->suspended)
> - return 0;
> + return;
>
> ctx->suspended = false;
>
> pm_runtime_get_sync(ctx->dev);
>
> - 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;
> - }
> + clk_prepare_enable(ctx->bus_clk);
> + clk_prepare_enable(ctx->lcd_clk);
>
> /* if vblank was enabled status, enable it again. */
> - if (test_and_clear_bit(0, &ctx->irq_flags)) {
> - ret = fimd_enable_vblank(ctx->crtc);
> - if (ret) {
> - DRM_ERROR("Failed to re-enable vblank [%d]\n", ret);
> - goto enable_vblank_err;
> - }
> - }
> + if (test_and_clear_bit(0, &ctx->irq_flags))
> + fimd_enable_vblank(ctx->crtc);
>
> fimd_window_resume(ctx);
>
> fimd_apply(ctx);
> -
> - 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 fimd_context *ctx)
> +static void fimd_disable(struct exynos_drm_crtc *crtc)
> {
> + struct fimd_context *ctx = crtc->ctx;
> +
> if (ctx->suspended)
> - return 0;
> + return;
>
> /*
> * We need to make sure that all windows are disabled before we
> @@ -870,26 +848,6 @@ static int fimd_poweroff(struct fimd_context *ctx)
> pm_runtime_put_sync(ctx->dev);
>
> ctx->suspended = true;
> - return 0;
> -}
> -
> -static void fimd_dpms(struct exynos_drm_crtc *crtc, int mode)
> -{
> - DRM_DEBUG_KMS("%s, %d\n", __FILE__, mode);
> -
> - switch (mode) {
> - case DRM_MODE_DPMS_ON:
> - fimd_poweron(crtc->ctx);
> - break;
> - case DRM_MODE_DPMS_STANDBY:
> - case DRM_MODE_DPMS_SUSPEND:
> - case DRM_MODE_DPMS_OFF:
> - fimd_poweroff(crtc->ctx);
> - break;
> - default:
> - DRM_DEBUG_KMS("unspecified mode %d\n", mode);
> - break;
> - }
> }
>
> static void fimd_trigger(struct device *dev)
> @@ -964,7 +922,8 @@ static void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
> }
>
> static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
> - .dpms = fimd_dpms,
> + .enable = fimd_enable,
> + .disable = fimd_disable,
> .mode_fixup = fimd_mode_fixup,
> .commit = fimd_commit,
> .enable_vblank = fimd_enable_vblank,
> @@ -1051,7 +1010,7 @@ static void fimd_unbind(struct device *dev, struct device *master,
> {
> struct fimd_context *ctx = dev_get_drvdata(dev);
>
> - fimd_dpms(ctx->crtc, DRM_MODE_DPMS_OFF);
> + fimd_disable(ctx->crtc);
>
> fimd_iommu_detach_devices(ctx);
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index 63c1536..abe4ee0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -153,56 +153,38 @@ static void vidi_win_disable(struct exynos_drm_crtc *crtc, unsigned int win)
> /* TODO. */
> }
>
> -static int vidi_power_on(struct vidi_context *ctx, bool enable)
> +static void vidi_enable(struct exynos_drm_crtc *crtc)
> {
> + struct vidi_context *ctx = crtc->ctx;
> struct exynos_drm_plane *plane;
> int i;
>
> - DRM_DEBUG_KMS("%s\n", __FILE__);
> -
> - if (enable != false && enable != true)
> - return -EINVAL;
> + mutex_lock(&ctx->lock);
>
> - if (enable) {
> - ctx->suspended = false;
> + ctx->suspended = false;
>
> - /* if vblank was enabled status, enable it again. */
> - if (test_and_clear_bit(0, &ctx->irq_flags))
> - vidi_enable_vblank(ctx->crtc);
> + /* if vblank was enabled status, enable it again. */
> + if (test_and_clear_bit(0, &ctx->irq_flags))
> + vidi_enable_vblank(ctx->crtc);
>
> - for (i = 0; i < WINDOWS_NR; i++) {
> - plane = &ctx->planes[i];
> - if (plane->enabled)
> - vidi_win_commit(ctx->crtc, i);
> - }
> - } else {
> - ctx->suspended = true;
> + for (i = 0; i < WINDOWS_NR; i++) {
> + plane = &ctx->planes[i];
> + if (plane->enabled)
> + vidi_win_commit(ctx->crtc, i);
> }
>
> - return 0;
> + mutex_unlock(&ctx->lock);
> }
>
> -static void vidi_dpms(struct exynos_drm_crtc *crtc, int mode)
> +static void vidi_disable(struct exynos_drm_crtc *crtc)
> {
> struct vidi_context *ctx = crtc->ctx;
> -
> - DRM_DEBUG_KMS("%d\n", mode);
> + struct exynos_drm_plane *plane;
> + int i;
>
> mutex_lock(&ctx->lock);
>
> - switch (mode) {
> - case DRM_MODE_DPMS_ON:
> - vidi_power_on(ctx, true);
> - break;
> - case DRM_MODE_DPMS_STANDBY:
> - case DRM_MODE_DPMS_SUSPEND:
> - case DRM_MODE_DPMS_OFF:
> - vidi_power_on(ctx, false);
> - break;
> - default:
> - DRM_DEBUG_KMS("unspecified mode %d\n", mode);
> - break;
> - }
> + ctx->suspended = true;
>
> mutex_unlock(&ctx->lock);
> }
> @@ -219,7 +201,8 @@ static int vidi_ctx_initialize(struct vidi_context *ctx,
> }
>
> static const struct exynos_drm_crtc_ops vidi_crtc_ops = {
> - .dpms = vidi_dpms,
> + .enable = vidi_enable,
> + .disable = vidi_disable,
> .enable_vblank = vidi_enable_vblank,
> .disable_vblank = vidi_disable_vblank,
> .win_commit = vidi_win_commit,
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 8874c1f..6bab717 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -1027,8 +1027,9 @@ static void mixer_window_resume(struct mixer_context *ctx)
> }
> }
>
> -static void mixer_poweron(struct mixer_context *ctx)
> +static void mixer_enable(struct exynos_drm_crtc *crtc)
> {
> + struct mixer_context *ctx = crtc->ctx;
> struct mixer_resources *res = &ctx->mixer_res;
>
> mutex_lock(&ctx->mixer_mutex);
> @@ -1061,8 +1062,9 @@ static void mixer_poweron(struct mixer_context *ctx)
> mixer_window_resume(ctx);
> }
>
> -static void mixer_poweroff(struct mixer_context *ctx)
> +static void mixer_disable(struct exynos_drm_crtc *crtc)
> {
> + struct mixer_context *ctx = crtc->ctx;
> struct mixer_resources *res = &ctx->mixer_res;
>
> mutex_lock(&ctx->mixer_mutex);
> @@ -1093,23 +1095,6 @@ static void mixer_poweroff(struct mixer_context *ctx)
> pm_runtime_put_sync(ctx->dev);
> }
>
> -static void mixer_dpms(struct exynos_drm_crtc *crtc, int mode)
> -{
> - switch (mode) {
> - case DRM_MODE_DPMS_ON:
> - mixer_poweron(crtc->ctx);
> - break;
> - case DRM_MODE_DPMS_STANDBY:
> - case DRM_MODE_DPMS_SUSPEND:
> - case DRM_MODE_DPMS_OFF:
> - mixer_poweroff(crtc->ctx);
> - break;
> - default:
> - DRM_DEBUG_KMS("unknown dpms mode: %d\n", mode);
> - break;
> - }
> -}
> -
> /* Only valid for Mixer version 16.0.33.0 */
> int mixer_check_mode(struct drm_display_mode *mode)
> {
> @@ -1131,7 +1116,8 @@ int mixer_check_mode(struct drm_display_mode *mode)
> }
>
> static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
> - .dpms = mixer_dpms,
> + .enable = mixer_enable,
> + .disable = mixer_disable,
> .enable_vblank = mixer_enable_vblank,
> .disable_vblank = mixer_disable_vblank,
> .wait_for_vblank = mixer_wait_for_vblank,
>
More information about the dri-devel
mailing list