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

[Joonyoung Shim]

On 05/22/2015 05:02 AM, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> 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 
> ---
>  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^Istruct 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.

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

[Gustavo Padovan]

Hi Joonyoung,

2015-05-22 Joonyoung Shim :

> On 05/22/2015 05:02 AM, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > 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 
> > ---
> >  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.

Yeah, this makes more sense. I fixed this and checkpatch error.

Gustavo

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

[Gustavo Padovan]

From: Gustavo Padovan 

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 
---
 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;
 }

 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)
+{
+   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,
-- 
2.1.0