[RESEND 3/3] drm :fsl-dcu: Add multi layers support

[Wang Dongsheng]

Hi Stefan,

Thanks, now I'm working on PSCIv1.0 for u-boot, and PCIe PM patches.
Will reply your comments later(Maybe next week).

Regards,
-Dongsheng

> 
> Hi Dongsheng,
> 
> On 2015-12-01 00:16, Dongsheng Wang wrote:
> > From: Jianwei Wang <jianwei.wang.chn at gmail.com>
> >
> > For DCU support atmost 16 layers(on ls1021a) or 64 layers(on vf610),
> > add (total_layer - 1) overlay planes.
> >
> > Signed-off-by: Jianwei Wang <jianwei.wang.chn at gmail.com>
> > Signed-off-by: Yi Meng <b56799 at freescale.com>
> > Signed-off-by: Wang Dongsheng <dongsheng.wang at freescale.com>
> >
> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> > b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> > index a8932a8..7ed1a7e 100644
> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> > @@ -235,7 +235,10 @@ static const u32 fsl_dcu_drm_plane_formats[] = {
> >
> >  struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device
> > *dev)  {
> 
> It feels wrong to me to add multi layer support in a function called
> fsl_dcu_drm_primary_create_plane...
> 
> I suggest to either rename the function, or better create a new function for the
> overlay planes. The only shared variable is formats_size, which is anyway
> statically determined by the compiler.
> 
> --
> Stefan
> 
> > +	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> >  	struct drm_plane *primary;
> > +	struct drm_plane *overlay;
> > +	unsigned int total_layer, formats_size, i;
> >  	int ret;
> >
> >  	primary = kzalloc(sizeof(*primary), GFP_KERNEL); @@ -244,11 +247,12
> > @@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct
> > drm_device *dev)
> >  		return NULL;
> >  	}
> >
> > +	formats_size = ARRAY_SIZE(fsl_dcu_drm_plane_formats);
> >  	/* possible_crtc's will be filled in later by crtc_init */
> >  	ret = drm_universal_plane_init(dev, primary, 0,
> >  				       &fsl_dcu_drm_plane_funcs,
> >  				       fsl_dcu_drm_plane_formats,
> > -				       ARRAY_SIZE(fsl_dcu_drm_plane_formats),
> > +				       formats_size,
> >  				       DRM_PLANE_TYPE_PRIMARY);
> >  	if (ret) {
> >  		kfree(primary);
> > @@ -256,5 +260,26 @@ struct drm_plane
> > *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
> >  	}
> >  	drm_plane_helper_add(primary, &fsl_dcu_drm_plane_helper_funcs);
> >
> > +	total_layer = fsl_dev->soc->total_layer;
> > +	for (i = 0; i < total_layer - 1; i++) {
> > +		overlay = kzalloc(sizeof(*overlay), GFP_KERNEL);
> > +		if (!overlay) {
> > +			DRM_DEBUG_KMS("Failed to allocate overlay plane\n");
> > +			goto out;
> > +		}
> > +
> > +		ret = drm_universal_plane_init(dev, overlay, 1,
> > +					       &fsl_dcu_drm_plane_funcs,
> > +					       fsl_dcu_drm_plane_formats,
> > +					       formats_size,
> > +					       DRM_PLANE_TYPE_OVERLAY);
> > +		if (ret) {
> > +			kfree(overlay);
> 
> You basically allow to initialize only some layers, which is probably fine. But
> I think we should add at least an error message here...
> 
> --
> Stefan
> 
> > +			goto out;
> > +		}
> > +		drm_plane_helper_add(overlay, &fsl_dcu_drm_plane_helper_funcs);
> > +	}
> > +
> > +out:
> >  	return primary;
> >  }