[PATCH 7/8] drm/exynos: hdmi: add complete_scanout function

[Sean Paul]

On Wed, Dec 26, 2012 at 6:27 AM, Prathyush K <prathyush.k at samsung.com> wrote:
> The wait_for_vblank interface is modified to the complete_scanout
> function in hdmi. This inturn calls the complete_scanout mixer op.
> This patch also adds the mixer_complete_scanout function.
>
> Inside mixer_complete_scanout, we read the base register and shadow
> register for each window and compare it with the dma address of the
> framebuffer. If the dma_address is in the base register, the mixer
> window is disabled. If the dma_address is in the shadow register,
> then the function waits for the next vblank and returns.
>
> Signed-off-by: Prathyush K <prathyush.k at samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 11 ++++++----
>  drivers/gpu/drm/exynos/exynos_drm_hdmi.h |  3 ++-
>  drivers/gpu/drm/exynos/exynos_mixer.c    | 37 +++++++++++++++++++++++++++++++-
>  3 files changed, 45 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
> index d8ae47e..e32eb1c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
> @@ -177,14 +177,17 @@ 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)
> +static void drm_hdmi_complete_scanout(struct device *subdrv_dev,
> +                                       dma_addr_t dma_addr,
> +                                       unsigned long size)
>  {
>         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);
> +       if (mixer_ops && mixer_ops->complete_scanout)
> +               mixer_ops->complete_scanout(ctx->mixer_ctx->ctx,
> +                                               dma_addr, size);
>  }
>
>  static void drm_hdmi_mode_fixup(struct device *subdrv_dev,
> @@ -263,7 +266,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,
> +       .complete_scanout = drm_hdmi_complete_scanout,
>         .mode_fixup = drm_hdmi_mode_fixup,
>         .mode_set = drm_hdmi_mode_set,
>         .get_max_resol = drm_hdmi_get_max_resol,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
> index 4fad00c..7d5bf00 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
> @@ -51,7 +51,8 @@ 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 (*complete_scanout)(void *ctx, dma_addr_t dma_addr,
> +                                       unsigned long size);
>         void (*dpms)(void *ctx, int mode);
>
>         /* overlay */
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 3369d57..151e13f 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -861,6 +861,41 @@ static void mixer_wait_for_vblank(void *ctx)
>                 DRM_DEBUG_KMS("vblank wait timed out.\n");
>  }
>
> +static void mixer_complete_scanout(void *ctx, dma_addr_t dma_addr,
> +                                       unsigned long size)
> +{
> +       struct mixer_context *mixer_ctx = ctx;
> +       struct mixer_resources *res = &mixer_ctx->mixer_res;
> +       int win;
> +       bool in_use = false;
> +       bool in_use_s = false;
> +
> +       if (!mixer_ctx->powered)

Accesses to mixer_ctx->powered are always protected by mixer_mutex,
this should probably stay consistent (either way).

> +               return;
> +
> +       for (win = 0; win < MIXER_WIN_NR; win++) {
> +               dma_addr_t dma_addr_in_use;
> +
> +               if (!mixer_ctx->win_data[win].enabled)
> +                       continue;
> +
> +               dma_addr_in_use = mixer_reg_read(res, MXR_GRAPHIC_BASE(win));
> +               if (dma_addr_in_use >= dma_addr &&
> +                       dma_addr_in_use < (dma_addr + size)) {
> +                               mixer_win_disable(ctx, win);
> +                               in_use = true;
> +               }
> +
> +               dma_addr_in_use = mixer_reg_read(res, MXR_GRAPHIC_BASE_S(win));
> +               if (dma_addr_in_use >= dma_addr &&
> +                       dma_addr_in_use < (dma_addr + size))
> +                               in_use_s = true;

You don't use this anywhere in the code. I think bad things will
happen if the framebuffer is in a shadow register and we free it.

I also wonder if you should wrap this all with mixer_vsync_set_update
so things don't change while you're doing this.

Sean


> +       }
> +
> +       if (in_use)
> +               mixer_wait_for_vblank(ctx);
> +}
> +
>  static void mixer_window_suspend(struct mixer_context *ctx)
>  {
>         struct hdmi_win_data *win_data;
> @@ -969,7 +1004,7 @@ static struct exynos_mixer_ops mixer_ops = {
>         .iommu_on               = mixer_iommu_on,
>         .enable_vblank          = mixer_enable_vblank,
>         .disable_vblank         = mixer_disable_vblank,
> -       .wait_for_vblank        = mixer_wait_for_vblank,
> +       .complete_scanout       = mixer_complete_scanout,
>         .dpms                   = mixer_dpms,
>
>         /* overlay */
> --
> 1.8.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel