Re: [PATCH v6 02/12] drm/exynos: atomic phase 1: use drm_plane_helper_disable()

[Joonyoung Shim <jy0922.shim@samsung.com>]

On 05/22/2015 05:02 AM, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> The atomic helper to disable planes also uses the optional
> .atomic_disable() helper. The unique operation it does is calling
> .win_disable()
> 
> exynos_drm_fb_get_buf_cnt() needs a fb check too to avoid a null pointer.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fb.c    |  2 +-
>  drivers/gpu/drm/exynos/exynos_drm_plane.c | 29 ++++++++++++++++-------------
>  2 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index 142eb4e..bf814c8 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -136,7 +136,7 @@ unsigned int exynos_drm_fb_get_buf_cnt(struct drm_framebuffer *fb)
>  
>  	exynos_fb = to_exynos_fb(fb);
>  
> -	return exynos_fb->buf_cnt;
> +	return exynos_fb ? exynos_fb->buf_cnt : 0;

I think this should be checked by exynos_check_plane on plane disabling
routine whether fb is NULL or not.

>  }
>  
>  struct drm_framebuffer *
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> index b218b7a..87078db 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> @@ -162,21 +162,9 @@ exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		exynos_crtc->ops->win_commit(exynos_crtc, exynos_plane->zpos);
>  }
>  
> -static int exynos_disable_plane(struct drm_plane *plane)
> -{
> -	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
> -	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(plane->crtc);
> -
> -	if (exynos_crtc && exynos_crtc->ops->win_disable)
> -		exynos_crtc->ops->win_disable(exynos_crtc,
> -					      exynos_plane->zpos);
> -
> -	return 0;
> -}
> -
>  static struct drm_plane_funcs exynos_plane_funcs = {
>  	.update_plane	= drm_plane_helper_update,
> -	.disable_plane	= exynos_disable_plane,
> +	.disable_plane	= drm_plane_helper_disable,
>  	.destroy	= drm_plane_cleanup,
>  };
>  
> @@ -201,9 +189,24 @@ static void exynos_plane_atomic_update(struct drm_plane *plane,
>  			    state->src_w >> 16, state->src_h >> 16);
>  }
>  
> +static void exynos_plane_atomic_disable(struct drm_plane *plane,
> +				        struct drm_plane_state *old_state)

checkpatch error:

ERROR: code indent should use tabs where possible
#64: FILE: drivers/gpu/drm/exynos/exynos_drm_plane.c:193:
+^I^I^I^I        struct drm_plane_state *old_state)$

total: 1 errors, 0 warnings, 54 lines checked

> +{
> +	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
> +	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(old_state->crtc);
> +
> +	if (!old_state->crtc)
> +		return;
> +
> +	if (exynos_crtc->ops->win_disable)
> +		exynos_crtc->ops->win_disable(exynos_crtc,
> +					      exynos_plane->zpos);
> +}
> +
>  static const struct drm_plane_helper_funcs plane_helper_funcs = {
>  	.atomic_check = exynos_plane_atomic_check,
>  	.atomic_update = exynos_plane_atomic_update,
> +	.atomic_disable = exynos_plane_atomic_disable,
>  };
>  
>  static void exynos_plane_attach_zpos_property(struct drm_plane *plane,
> 

Thanks.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel