[PATCH v2 05/15] drm/atmel-hlcdc: Use per-plane rotation property

[Boris Brezillon]

On Wed, 10 Aug 2016 12:09:41 +0300
Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:

> On Wed, Aug 10, 2016 at 10:35:22AM +0200, Boris Brezillon wrote:
> > Hi Ville,
> > 
> > On Fri, 22 Jul 2016 18:47:01 +0300
> > ville.syrjala at linux.intel.com wrote:
> >   
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > The global mode_config.rotation_property is going away, switch over to
> > > per-plane rotation_property.
> > > 
> > > v2: Propagate error upwards (Boris)
> > > 
> > > Cc: Boris Brezillon <boris.brezillon at free-electrons.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 37 +++++++++++++------------
> > >  1 file changed, 20 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > index f3350c80704d..ee9bd7a938c3 100644
> > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> > > @@ -883,9 +883,9 @@ static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
> > >  	return 0;
> > >  }
> > >  
> > > -static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > -				const struct atmel_hlcdc_layer_desc *desc,
> > > -				struct atmel_hlcdc_plane_properties *props)
> > > +static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > > +					     const struct atmel_hlcdc_layer_desc *desc,
> > > +					     struct atmel_hlcdc_plane_properties *props)
> > >  {
> > >  	struct regmap *regmap = plane->layer.hlcdc->regmap;
> > >  
> > > @@ -902,10 +902,18 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > >  				ATMEL_HLCDC_LAYER_GA_MASK);
> > >  	}
> > >  
> > > -	if (desc->layout.xstride && desc->layout.pstride)
> > > -		drm_object_attach_property(&plane->base.base,
> > > -				plane->base.dev->mode_config.rotation_property,
> > > -				BIT(DRM_ROTATE_0));
> > > +	if (desc->layout.xstride && desc->layout.pstride) {
> > > +		int ret;
> > > +
> > > +		ret = drm_plane_create_rotation_property(&plane->base,
> > > +							 BIT(DRM_ROTATE_0),
> > > +							 BIT(DRM_ROTATE_0) |
> > > +							 BIT(DRM_ROTATE_90) |
> > > +							 BIT(DRM_ROTATE_180) |
> > > +							 BIT(DRM_ROTATE_270));
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > >  
> > >  	if (desc->layout.csc) {
> > >  		/*
> > > @@ -925,6 +933,8 @@ static void atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
> > >  			     ATMEL_HLCDC_LAYER_CSC_CFG(&plane->layer, 2),
> > >  			     0x40040890);
> > >  	}
> > > +
> > > +	return 0;
> > >  }
> > >  
> > >  static struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> > > @@ -1036,7 +1046,9 @@ atmel_hlcdc_plane_create(struct drm_device *dev,
> > >  			     &atmel_hlcdc_layer_plane_helper_funcs);
> > >  
> > >  	/* Set default property values*/
> > > -	atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > +	ret = atmel_hlcdc_plane_init_properties(plane, desc, props);
> > > +	if (ret)
> > > +		return ERR_PTR(ret);  
> > 
> > Shouldn't we call drm_plane_cleanup() here?  
> 
> The other error path doesn't call it either, so I figured no really
> cares anyway.

Because the other error path is before plane initialization.
Now, it's true that atmel_hlcdc_create_planes() does not take care of
releasing previously registered planes in case we fail to register one
of them, but that's something we should fix instead of introducing new
potential sources of memory leaks ;).

> 
> >   
> > >  
> > >  	return plane;
> > >  }
> > > @@ -1054,15 +1066,6 @@ atmel_hlcdc_plane_create_properties(struct drm_device *dev)
> > >  	if (!props->alpha)
> > >  		return ERR_PTR(-ENOMEM);
> > >  
> > > -	dev->mode_config.rotation_property =
> > > -			drm_mode_create_rotation_property(dev,
> > > -							  BIT(DRM_ROTATE_0) |
> > > -							  BIT(DRM_ROTATE_90) |
> > > -							  BIT(DRM_ROTATE_180) |
> > > -							  BIT(DRM_ROTATE_270));
> > > -	if (!dev->mode_config.rotation_property)
> > > -		return ERR_PTR(-ENOMEM);
> > > -
> > >  	return props;
> > >  }
> > >    
>