Re: [PATCH 03/11] drm/exynos: add prepare and cleanup phases for planes

[Inki Dae <inki.dae@samsung.com>]

On 2015년 08월 16일 01:26, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> .prepare_plane() and .cleanup_plane() allows to perform extra operations
> before and after the update of planes. For FIMD for example this will
> be used to enable disable the shadow protection bit.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 19 +++++++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  6 ++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index 5a19e16..3a89fc9 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -72,15 +72,34 @@ exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  static void exynos_crtc_atomic_begin(struct drm_crtc *crtc)
>  {
>  	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> +	struct drm_plane *plane;
>  
>  	if (crtc->state->event) {
>  		WARN_ON(drm_crtc_vblank_get(crtc) != 0);
>  		exynos_crtc->event = crtc->state->event;
>  	}
> +
> +	drm_atomic_crtc_for_each_plane(plane, crtc) {
> +		struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
> +
> +		if (exynos_crtc->ops->prepare_plane)
> +			exynos_crtc->ops->prepare_plane(exynos_crtc,
> +							exynos_plane);

There is no any reason to use prepare_plane/cleanup_plane callback
names. How about using atomic_begin/atomic_flush callback names instead
for consistency between framework and device drivers?

Thanks,
Inki Dae

> +	}
>  }
>  
>  static void exynos_crtc_atomic_flush(struct drm_crtc *crtc)
>  {
> +	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> +	struct drm_plane *plane;
> +
> +	drm_atomic_crtc_for_each_plane(plane, crtc) {
> +		struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
> +
> +		if (exynos_crtc->ops->cleanup_plane)
> +			exynos_crtc->ops->cleanup_plane(exynos_crtc,
> +							exynos_plane);
> +	}
>  }
>  
>  static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index a993aac..9f2b5c9 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -87,6 +87,8 @@ struct exynos_drm_plane {
>   * @disable_vblank: specific driver callback for disabling vblank interrupt.
>   * @wait_for_vblank: wait for vblank interrupt to make sure that
>   *	hardware overlay is updated.
> + * @prepare_plane: prepare a window to receive a update
> + * @cleanup_plane: mark the end of a window update
>   * @update_plane: apply hardware specific overlay data to registers.
>   * @disable_plane: disable hardware specific overlay.
>   * @te_handler: trigger to transfer video image at the tearing effect
> @@ -107,10 +109,14 @@ struct exynos_drm_crtc_ops {
>  	int (*enable_vblank)(struct exynos_drm_crtc *crtc);
>  	void (*disable_vblank)(struct exynos_drm_crtc *crtc);
>  	void (*wait_for_vblank)(struct exynos_drm_crtc *crtc);
> +	void (*prepare_plane)(struct exynos_drm_crtc *crtc,
> +			      struct exynos_drm_plane *plane);
>  	void (*update_plane)(struct exynos_drm_crtc *crtc,
>  			     struct exynos_drm_plane *plane);
>  	void (*disable_plane)(struct exynos_drm_crtc *crtc,
>  			      struct exynos_drm_plane *plane);
> +	void (*cleanup_plane)(struct exynos_drm_crtc *crtc,
> +			      struct exynos_drm_plane *plane);
>  	void (*te_handler)(struct exynos_drm_crtc *crtc);
>  	void (*clock_enable)(struct exynos_drm_crtc *crtc, bool enable);
>  };
>