[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 
> >
> > For DCU support atmost 16 layers(on ls1021a) or 64 layers(on vf610),
> > add (total_layer - 1) overlay planes.
> >
> > Signed-off-by: Jianwei Wang 
> > Signed-off-by: Yi Meng 
> > Signed-off-by: Wang Dongsheng 
> >
> > 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,
> >_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, _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,
> > +  _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, _dcu_drm_plane_helper_funcs);
> > +   }
> > +
> > +out:
> > return primary;
> >  }

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

[Stefan Agner]

Hi Dongsheng,

On 2015-12-01 00:16, Dongsheng Wang wrote:
> From: Jianwei Wang 
> 
> For DCU support atmost 16 layers(on ls1021a) or 64 layers(on vf610),
> add (total_layer - 1) overlay planes.
> 
> Signed-off-by: Jianwei Wang 
> Signed-off-by: Yi Meng 
> Signed-off-by: Wang Dongsheng 
> 
> 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,
>  _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, _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,
> +_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, _dcu_drm_plane_helper_funcs);
> + }
> +
> +out:
>   return primary;
>  }

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

[Dongsheng Wang]

From: Jianwei Wang 

For DCU support atmost 16 layers(on ls1021a) or 64 layers(on vf610),
add (total_layer - 1) overlay planes.

Signed-off-by: Jianwei Wang 
Signed-off-by: Yi Meng 
Signed-off-by: Wang Dongsheng 

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)
 {
+   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,
   _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, _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,
+  _dcu_drm_plane_funcs,
+  fsl_dcu_drm_plane_formats,
+  formats_size,
+  DRM_PLANE_TYPE_OVERLAY);
+   if (ret) {
+   kfree(overlay);
+   goto out;
+   }
+   drm_plane_helper_add(overlay, _dcu_drm_plane_helper_funcs);
+   }
+
+out:
return primary;
 }
-- 
2.1.0.27.g96db324