<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jan 2, 2013 at 10:12 PM, Sean Paul <span dir="ltr"><<a href="mailto:seanpaul@chromium.org" target="_blank">seanpaul@chromium.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On Wed, Dec 26, 2012 at 6:27 AM, Prathyush K <<a href="mailto:prathyush.k@samsung.com" target="_blank">prathyush.k@samsung.com</a>> wrote:<br>
> The wait_for_vblank interface is modified to the complete_scanout<br>
> function in hdmi. This inturn calls the complete_scanout mixer op.<br>
> This patch also adds the mixer_complete_scanout function.<br>
><br>
> Inside mixer_complete_scanout, we read the base register and shadow<br>
> register for each window and compare it with the dma address of the<br>
> framebuffer. If the dma_address is in the base register, the mixer<br>
> window is disabled. If the dma_address is in the shadow register,<br>
> then the function waits for the next vblank and returns.<br>
><br>
> Signed-off-by: Prathyush K <<a href="mailto:prathyush.k@samsung.com" target="_blank">prathyush.k@samsung.com</a>><br>
> ---<br>
> drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 11 ++++++----<br>
> drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 3 ++-<br>
> drivers/gpu/drm/exynos/exynos_mixer.c | 37 +++++++++++++++++++++++++++++++-<br>
> 3 files changed, 45 insertions(+), 6 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c<br>
> index d8ae47e..e32eb1c 100644<br>
> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c<br>
> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c<br>
> @@ -177,14 +177,17 @@ static void drm_hdmi_disable_vblank(struct device *subdrv_dev)<br>
> return mixer_ops->disable_vblank(ctx->mixer_ctx->ctx);<br>
> }<br>
><br>
> -static void drm_hdmi_wait_for_vblank(struct device *subdrv_dev)<br>
> +static void drm_hdmi_complete_scanout(struct device *subdrv_dev,<br>
> + dma_addr_t dma_addr,<br>
> + unsigned long size)<br>
> {<br>
> struct drm_hdmi_context *ctx = to_context(subdrv_dev);<br>
><br>
> DRM_DEBUG_KMS("%s\n", __FILE__);<br>
><br>
> - if (mixer_ops && mixer_ops->wait_for_vblank)<br>
> - mixer_ops->wait_for_vblank(ctx->mixer_ctx->ctx);<br>
> + if (mixer_ops && mixer_ops->complete_scanout)<br>
> + mixer_ops->complete_scanout(ctx->mixer_ctx->ctx,<br>
> + dma_addr, size);<br>
> }<br>
><br>
> static void drm_hdmi_mode_fixup(struct device *subdrv_dev,<br>
> @@ -263,7 +266,7 @@ static struct exynos_drm_manager_ops drm_hdmi_manager_ops = {<br>
> .apply = drm_hdmi_apply,<br>
> .enable_vblank = drm_hdmi_enable_vblank,<br>
> .disable_vblank = drm_hdmi_disable_vblank,<br>
> - .wait_for_vblank = drm_hdmi_wait_for_vblank,<br>
> + .complete_scanout = drm_hdmi_complete_scanout,<br>
> .mode_fixup = drm_hdmi_mode_fixup,<br>
> .mode_set = drm_hdmi_mode_set,<br>
> .get_max_resol = drm_hdmi_get_max_resol,<br>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h<br>
> index 4fad00c..7d5bf00 100644<br>
> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h<br>
> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h<br>
> @@ -51,7 +51,8 @@ struct exynos_mixer_ops {<br>
> int (*iommu_on)(void *ctx, bool enable);<br>
> int (*enable_vblank)(void *ctx, int pipe);<br>
> void (*disable_vblank)(void *ctx);<br>
> - void (*wait_for_vblank)(void *ctx);<br>
> + void (*complete_scanout)(void *ctx, dma_addr_t dma_addr,<br>
> + unsigned long size);<br>
> void (*dpms)(void *ctx, int mode);<br>
><br>
> /* overlay */<br>
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c<br>
> index 3369d57..151e13f 100644<br>
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c<br>
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c<br>
> @@ -861,6 +861,41 @@ static void mixer_wait_for_vblank(void *ctx)<br>
> DRM_DEBUG_KMS("vblank wait timed out.\n");<br>
> }<br>
><br>
> +static void mixer_complete_scanout(void *ctx, dma_addr_t dma_addr,<br>
> + unsigned long size)<br>
> +{<br>
> + struct mixer_context *mixer_ctx = ctx;<br>
> + struct mixer_resources *res = &mixer_ctx->mixer_res;<br>
> + int win;<br>
> + bool in_use = false;<br>
> + bool in_use_s = false;<br>
> +<br>
> + if (!mixer_ctx->powered)<br>
<br>
</div></div>Accesses to mixer_ctx->powered are always protected by mixer_mutex,<br>
this should probably stay consistent (either way).<br>
<div><br></div></blockquote><div><br></div><div>Hi Sean,</div><div><br></div><div>Right, thanks for pointing that out. I'll update in next patch set.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
> + return;<br>
> +<br>
> + for (win = 0; win < MIXER_WIN_NR; win++) {<br>
> + dma_addr_t dma_addr_in_use;<br>
> +<br>
> + if (!mixer_ctx->win_data[win].enabled)<br>
> + continue;<br>
> +<br>
> + dma_addr_in_use = mixer_reg_read(res, MXR_GRAPHIC_BASE(win));<br>
> + if (dma_addr_in_use >= dma_addr &&<br>
> + dma_addr_in_use < (dma_addr + size)) {<br>
> + mixer_win_disable(ctx, win);<br>
> + in_use = true;<br>
> + }<br>
> +<br>
> + dma_addr_in_use = mixer_reg_read(res, MXR_GRAPHIC_BASE_S(win));<br>
> + if (dma_addr_in_use >= dma_addr &&<br>
> + dma_addr_in_use < (dma_addr + size))<br>
> + in_use_s = true;<br>
<br>
</div>You don't use this anywhere in the code. I think bad things will<br>
happen if the framebuffer is in a shadow register and we free it.<br>
<br></blockquote><div>I don't get your point here. What is not used anywhere in the code?</div><div><br></div><div style>Please check the comments on the fimd patch.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I also wonder if you should wrap this all with mixer_vsync_set_update<br>
so things don't change while you're doing this.<br>
<span><font color="#888888"><br></font></span></blockquote><div style>Haven't thought of that. Will definitely consider that for next patch set.</div><div style><br></div><div style>Thanks for reviewing,</div><div style>
<br></div><div style>Regards,</div><div style>Prathyush</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><font color="#888888">
Sean<br>
</font></span><div><div><br>
<br>
> + }<br>
> +<br>
> + if (in_use)<br>
> + mixer_wait_for_vblank(ctx);<br>
> +}<br>
> +<br>
> static void mixer_window_suspend(struct mixer_context *ctx)<br>
> {<br>
> struct hdmi_win_data *win_data;<br>
> @@ -969,7 +1004,7 @@ static struct exynos_mixer_ops mixer_ops = {<br>
> .iommu_on = mixer_iommu_on,<br>
> .enable_vblank = mixer_enable_vblank,<br>
> .disable_vblank = mixer_disable_vblank,<br>
> - .wait_for_vblank = mixer_wait_for_vblank,<br>
> + .complete_scanout = mixer_complete_scanout,<br>
> .dpms = mixer_dpms,<br>
><br>
> /* overlay */<br>
> --<br>
> 1.8.0<br>
><br>
> _______________________________________________<br>
> dri-devel mailing list<br>
> <a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a><br>
> <a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
_______________________________________________<br>
dri-devel mailing list<br>
<a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
</div></div></blockquote></div><br></div></div>