diff mbox

[14/23] drm/atmel-hlcdc: Stop using plane->fb

Message ID 20180322152313.6561-15-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä March 22, 2018, 3:23 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We want to get rid of plane->fb on atomic drivers. Stop looking at it.

Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

Comments

Maarten Lankhorst March 22, 2018, 4:14 p.m. UTC | #1 
Op 22-03-18 om 16:23 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We want to get rid of plane->fb on atomic drivers. Stop looking at it.
>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index e18800ed7cd1..0dd9fb617c28 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -819,16 +819,6 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>  	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>  }
>  
> -static void atmel_hlcdc_plane_destroy(struct drm_plane *p)
> -{
> -	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> -
> -	if (plane->base.fb)
> -		drm_framebuffer_put(plane->base.fb);
> -
> -	drm_plane_cleanup(p);
> -}
> -
>  static int atmel_hlcdc_plane_atomic_set_property(struct drm_plane *p,
>  						 struct drm_plane_state *s,
>  						 struct drm_property *property,
> @@ -1038,7 +1028,7 @@ static void atmel_hlcdc_plane_atomic_destroy_state(struct drm_plane *p,
>  static const struct drm_plane_funcs layer_plane_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
> -	.destroy = atmel_hlcdc_plane_destroy,
> +	.destroy = drm_plane_cleanup,
>  	.reset = atmel_hlcdc_plane_reset,
>  	.atomic_duplicate_state = atmel_hlcdc_plane_atomic_duplicate_state,
>  	.atomic_destroy_state = atmel_hlcdc_plane_atomic_destroy_state,

This patch should probably be after 'drm: Stop updating plane->crtc/fb/old_fb on atomic drivers'?
Ville Syrjälä March 22, 2018, 5:30 p.m. UTC | #2 
On Thu, Mar 22, 2018 at 05:14:08PM +0100, Maarten Lankhorst wrote:
> Op 22-03-18 om 16:23 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We want to get rid of plane->fb on atomic drivers. Stop looking at it.
> >
> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 12 +-----------
> >  1 file changed, 1 insertion(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > index e18800ed7cd1..0dd9fb617c28 100644
> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > @@ -819,16 +819,6 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
> >  	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> >  }
> >  
> > -static void atmel_hlcdc_plane_destroy(struct drm_plane *p)
> > -{
> > -	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> > -
> > -	if (plane->base.fb)
> > -		drm_framebuffer_put(plane->base.fb);
> > -
> > -	drm_plane_cleanup(p);
> > -}
> > -
> >  static int atmel_hlcdc_plane_atomic_set_property(struct drm_plane *p,
> >  						 struct drm_plane_state *s,
> >  						 struct drm_property *property,
> > @@ -1038,7 +1028,7 @@ static void atmel_hlcdc_plane_atomic_destroy_state(struct drm_plane *p,
> >  static const struct drm_plane_funcs layer_plane_funcs = {
> >  	.update_plane = drm_atomic_helper_update_plane,
> >  	.disable_plane = drm_atomic_helper_disable_plane,
> > -	.destroy = atmel_hlcdc_plane_destroy,
> > +	.destroy = drm_plane_cleanup,
> >  	.reset = atmel_hlcdc_plane_reset,
> >  	.atomic_duplicate_state = atmel_hlcdc_plane_atomic_duplicate_state,
> >  	.atomic_destroy_state = atmel_hlcdc_plane_atomic_destroy_state,
> 
> This patch should probably be after 'drm: Stop updating plane->crtc/fb/old_fb on atomic drivers'?

Yeah, that does seem like a better order.
Daniel Vetter March 27, 2018, 8:13 a.m. UTC | #3 
On Thu, Mar 22, 2018 at 05:23:04PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We want to get rid of plane->fb on atomic drivers. Stop looking at it.
> 
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index e18800ed7cd1..0dd9fb617c28 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -819,16 +819,6 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>  	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>  }
>  
> -static void atmel_hlcdc_plane_destroy(struct drm_plane *p)
> -{
> -	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> -
> -	if (plane->base.fb)
> -		drm_framebuffer_put(plane->base.fb);
> -
> -	drm_plane_cleanup(p);
> -}

This looks like refcount leak duct-tape due to lack of
drm_atomic_helper_shutdown() in atmel. If you just apply this I think
you'll start leaking.

We need a patch here to add the shutdown call instead.
-Daniel

> -
>  static int atmel_hlcdc_plane_atomic_set_property(struct drm_plane *p,
>  						 struct drm_plane_state *s,
>  						 struct drm_property *property,
> @@ -1038,7 +1028,7 @@ static void atmel_hlcdc_plane_atomic_destroy_state(struct drm_plane *p,
>  static const struct drm_plane_funcs layer_plane_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
> -	.destroy = atmel_hlcdc_plane_destroy,
> +	.destroy = drm_plane_cleanup,
>  	.reset = atmel_hlcdc_plane_reset,
>  	.atomic_duplicate_state = atmel_hlcdc_plane_atomic_duplicate_state,
>  	.atomic_destroy_state = atmel_hlcdc_plane_atomic_destroy_state,
> -- 
> 2.16.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index e18800ed7cd1..0dd9fb617c28 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -819,16 +819,6 @@  static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
 	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
 }
 
-static void atmel_hlcdc_plane_destroy(struct drm_plane *p)
-{
-	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
-
-	if (plane->base.fb)
-		drm_framebuffer_put(plane->base.fb);
-
-	drm_plane_cleanup(p);
-}
-
 static int atmel_hlcdc_plane_atomic_set_property(struct drm_plane *p,
 						 struct drm_plane_state *s,
 						 struct drm_property *property,
@@ -1038,7 +1028,7 @@  static void atmel_hlcdc_plane_atomic_destroy_state(struct drm_plane *p,
 static const struct drm_plane_funcs layer_plane_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
-	.destroy = atmel_hlcdc_plane_destroy,
+	.destroy = drm_plane_cleanup,
 	.reset = atmel_hlcdc_plane_reset,
 	.atomic_duplicate_state = atmel_hlcdc_plane_atomic_duplicate_state,
 	.atomic_destroy_state = atmel_hlcdc_plane_atomic_destroy_state,