[PATCH v2] DRM: Add DRM kms/fb cma helper

2012-06-04 Thread Lars-Peter Clausen 
On 05/31/2012 08:11 PM, Sascha Hauer wrote:
> On Thu, May 31, 2012 at 11:34:37AM +0200, Lars-Peter Clausen wrote:
 +  drm_helper_mode_fill_fb_struct(_cma->fb, mode_cmd);
 +
 +  for (i = 0; i < num_planes; i++)
 +  fb_cma->obj[i] = obj[i];
>>>
>>> Check for valid num_planes before this loop?
>>>
>>
>> Hm, I think the callers already take care of this. drm_format_num_planes will
>> always return a valid number and the other caller passes 1 unconditionally.
> 
> As long as the array always is big enough to hold the maximum number of
> planes I think it's fine. However, if there is some format with 5 planes
> (don't know if there is, I only know yuv with multiple planes) it's not
> obvious that this code does not support this.

The DRM ABI limits the number of planes to 4. But adding a global
DRM_MAX_PLANES is probably a good idea to avoid surprises if this ever changes.

Re: [PATCH v2] DRM: Add DRM kms/fb cma helper

2012-06-04 Thread Lars-Peter Clausen 
On 05/31/2012 08:11 PM, Sascha Hauer wrote:
 On Thu, May 31, 2012 at 11:34:37AM +0200, Lars-Peter Clausen wrote:
 +  drm_helper_mode_fill_fb_struct(fb_cma-fb, mode_cmd);
 +
 +  for (i = 0; i  num_planes; i++)
 +  fb_cma-obj[i] = obj[i];

 Check for valid num_planes before this loop?


 Hm, I think the callers already take care of this. drm_format_num_planes will
 always return a valid number and the other caller passes 1 unconditionally.
 
 As long as the array always is big enough to hold the maximum number of
 planes I think it's fine. However, if there is some format with 5 planes
 (don't know if there is, I only know yuv with multiple planes) it's not
 obvious that this code does not support this.

The DRM ABI limits the number of planes to 4. But adding a global
DRM_MAX_PLANES is probably a good idea to avoid surprises if this ever changes.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] DRM: Add DRM kms/fb cma helper

2012-05-31 Thread Sascha Hauer 
On Thu, May 31, 2012 at 11:34:37AM +0200, Lars-Peter Clausen wrote:
> >> +  drm_helper_mode_fill_fb_struct(_cma->fb, mode_cmd);
> >> +
> >> +  for (i = 0; i < num_planes; i++)
> >> +  fb_cma->obj[i] = obj[i];
> > 
> > Check for valid num_planes before this loop?
> > 
> 
> Hm, I think the callers already take care of this. drm_format_num_planes will
> always return a valid number and the other caller passes 1 unconditionally.

As long as the array always is big enough to hold the maximum number of
planes I think it's fine. However, if there is some format with 5 planes
(don't know if there is, I only know yuv with multiple planes) it's not
obvious that this code does not support this.

> 
> > 
> > Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object
> > in the patch I just sent.
> > 
> 
> Do you think it makes sense for you to carry this patch as part of your iMX 
> DRM
> series?

I can carry this, but it could be that Laurent has his driver in an
acceptable state earlier. Let's see.

Sascha


-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

[PATCH v2] DRM: Add DRM kms/fb cma helper

2012-05-31 Thread Lars-Peter Clausen 
On 05/31/2012 10:13 AM, Sascha Hauer wrote:
> On Wed, May 30, 2012 at 05:30:15PM +0200, Lars-Peter Clausen wrote:
>> This patch introduces a set of helper function for implementing the KMS
>> framebuffer layer for drivers which use the drm gem CMA helper function.
>>
>> Signed-off-by: Lars-Peter Clausen 
>>
>> ---
>> Changes since v1:
>>  * Some spelling fixes
>>  * Add missing kfree in drm_fb_cma_alloc error path
>>  * Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second
>>parameter to specify the plane.
>> ---
>>  drivers/gpu/drm/Kconfig |   11 +
>>  drivers/gpu/drm/Makefile|1 +
>>  drivers/gpu/drm/drm_fb_cma_helper.c |  384 
>> +++
>>  include/drm/drm_fb_cma_helper.h |   27 +++
>>  4 files changed, 423 insertions(+)
>>  create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c
>>  create mode 100644 include/drm/drm_fb_cma_helper.h
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index a78dfbe..e7c0a3d 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER
>>  help
>>Choose this if you need the GEM cma helper functions
>>  
>> +config DRM_KMS_CMA_HELPER
>> +tristate
>> +select DRM_GEM_CMA_HELPER
>> +select DRM_KMS_HELPER
>> +select FB_SYS_FILLRECT
>> +select FB_SYS_COPYAREA
>> +select FB_SYS_IMAGEBLIT
>> +help
>> +  Choose this if you need the KMS cma helper functions
>> +
>> +
>>  config DRM_TDFX
>>  tristate "3dfx Banshee/Voodoo3+"
>>  depends on DRM && PCI
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 6e9e948..5dcb1a5 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o
>>  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
>>  
>>  drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o
>> +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>>  
>>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>>  
>> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
>> b/drivers/gpu/drm/drm_fb_cma_helper.c
>> new file mode 100644
>> index 000..8821a98
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
>> @@ -0,0 +1,384 @@
>> +/*
>> + * drm kms/fb cma (contiguous memory allocator) helper functions
>> + *
>> + * Copyright (C) 2012 Analog Device Inc.
>> + *   Author: Lars-Peter Clausen 
>> + *
>> + * Based on udl_fbdev.c
>> + *  Copyright (C) 2012 Red Hat
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +struct drm_fb_cma {
>> +struct drm_framebuffer  fb;
>> +struct drm_gem_cma_obj  *obj[4];
> 
> Can we have a define for this magic '4'? It is used several times in
> this patch.

Yes, had the same though. The magic 4 is actually used all through all of DRM.
Something like '#define DRM_MAX_PLANES 4' probably makes sense.

> 
>> +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
>> +struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj,
>> +unsigned int num_planes)
>> +{
>> +struct drm_fb_cma *fb_cma;
>> +int ret;
>> +int i;
>> +
>> +fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
>> +if (!fb_cma)
>> +return ERR_PTR(-ENOMEM);
>> +
>> +ret = drm_framebuffer_init(dev, _cma->fb, _fb_cma_funcs);
>> +if (ret) {
>> +dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret);
>> +kfree(fb_cma);
>> +return ERR_PTR(ret);
>> +}
>> +
>> +drm_helper_mode_fill_fb_struct(_cma->fb, mode_cmd);
>> +
>> +for (i = 0; i < num_planes; i++)
>> +fb_cma->obj[i] = obj[i];
> 
> Check for valid num_planes before this loop?
> 

Hm, I think the callers already take care of this. drm_format_num_planes will
always return a valid number and the other caller passes 1 unconditionally.

> 
> Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object
> in the patch I just sent.
> 

Do you think it makes sense for you to carry this patch as part of your iMX DRM
series?

Thanks,
- Lars

[PATCH v2] DRM: Add DRM kms/fb cma helper

2012-05-31 Thread Sascha Hauer 
On Wed, May 30, 2012 at 05:30:15PM +0200, Lars-Peter Clausen wrote:
> This patch introduces a set of helper function for implementing the KMS
> framebuffer layer for drivers which use the drm gem CMA helper function.
> 
> Signed-off-by: Lars-Peter Clausen 
> 
> ---
> Changes since v1:
>   * Some spelling fixes
>   * Add missing kfree in drm_fb_cma_alloc error path
>   * Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second
> parameter to specify the plane.
> ---
>  drivers/gpu/drm/Kconfig |   11 +
>  drivers/gpu/drm/Makefile|1 +
>  drivers/gpu/drm/drm_fb_cma_helper.c |  384 
> +++
>  include/drm/drm_fb_cma_helper.h |   27 +++
>  4 files changed, 423 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c
>  create mode 100644 include/drm/drm_fb_cma_helper.h
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index a78dfbe..e7c0a3d 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER
>   help
> Choose this if you need the GEM cma helper functions
>  
> +config DRM_KMS_CMA_HELPER
> + tristate
> + select DRM_GEM_CMA_HELPER
> + select DRM_KMS_HELPER
> + select FB_SYS_FILLRECT
> + select FB_SYS_COPYAREA
> + select FB_SYS_IMAGEBLIT
> + help
> +   Choose this if you need the KMS cma helper functions
> +
> +
>  config DRM_TDFX
>   tristate "3dfx Banshee/Voodoo3+"
>   depends on DRM && PCI
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6e9e948..5dcb1a5 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o
>  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
>  
>  drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o
> +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>  
>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>  
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> new file mode 100644
> index 000..8821a98
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -0,0 +1,384 @@
> +/*
> + * drm kms/fb cma (contiguous memory allocator) helper functions
> + *
> + * Copyright (C) 2012 Analog Device Inc.
> + *   Author: Lars-Peter Clausen 
> + *
> + * Based on udl_fbdev.c
> + *  Copyright (C) 2012 Red Hat
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct drm_fb_cma {
> + struct drm_framebuffer  fb;
> + struct drm_gem_cma_obj  *obj[4];

Can we have a define for this magic '4'? It is used several times in
this patch.

> +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
> + struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj,
> + unsigned int num_planes)
> +{
> + struct drm_fb_cma *fb_cma;
> + int ret;
> + int i;
> +
> + fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
> + if (!fb_cma)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = drm_framebuffer_init(dev, _cma->fb, _fb_cma_funcs);
> + if (ret) {
> + dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret);
> + kfree(fb_cma);
> + return ERR_PTR(ret);
> + }
> +
> + drm_helper_mode_fill_fb_struct(_cma->fb, mode_cmd);
> +
> + for (i = 0; i < num_planes; i++)
> + fb_cma->obj[i] = obj[i];

Check for valid num_planes before this loop?


Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object
in the patch I just sent.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

Re: [PATCH v2] DRM: Add DRM kms/fb cma helper

2012-05-31 Thread Sascha Hauer 
On Wed, May 30, 2012 at 05:30:15PM +0200, Lars-Peter Clausen wrote:
 This patch introduces a set of helper function for implementing the KMS
 framebuffer layer for drivers which use the drm gem CMA helper function.
 
 Signed-off-by: Lars-Peter Clausen l...@metafoo.de
 
 ---
 Changes since v1:
   * Some spelling fixes
   * Add missing kfree in drm_fb_cma_alloc error path
   * Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second
 parameter to specify the plane.
 ---
  drivers/gpu/drm/Kconfig |   11 +
  drivers/gpu/drm/Makefile|1 +
  drivers/gpu/drm/drm_fb_cma_helper.c |  384 
 +++
  include/drm/drm_fb_cma_helper.h |   27 +++
  4 files changed, 423 insertions(+)
  create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c
  create mode 100644 include/drm/drm_fb_cma_helper.h
 
 diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
 index a78dfbe..e7c0a3d 100644
 --- a/drivers/gpu/drm/Kconfig
 +++ b/drivers/gpu/drm/Kconfig
 @@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER
   help
 Choose this if you need the GEM cma helper functions
  
 +config DRM_KMS_CMA_HELPER
 + tristate
 + select DRM_GEM_CMA_HELPER
 + select DRM_KMS_HELPER
 + select FB_SYS_FILLRECT
 + select FB_SYS_COPYAREA
 + select FB_SYS_IMAGEBLIT
 + help
 +   Choose this if you need the KMS cma helper functions
 +
 +
  config DRM_TDFX
   tristate 3dfx Banshee/Voodoo3+
   depends on DRM  PCI
 diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 index 6e9e948..5dcb1a5 100644
 --- a/drivers/gpu/drm/Makefile
 +++ b/drivers/gpu/drm/Makefile
 @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o
  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
  
  drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o
 +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
  
  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
  
 diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
 b/drivers/gpu/drm/drm_fb_cma_helper.c
 new file mode 100644
 index 000..8821a98
 --- /dev/null
 +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
 @@ -0,0 +1,384 @@
 +/*
 + * drm kms/fb cma (contiguous memory allocator) helper functions
 + *
 + * Copyright (C) 2012 Analog Device Inc.
 + *   Author: Lars-Peter Clausen l...@metafoo.de
 + *
 + * Based on udl_fbdev.c
 + *  Copyright (C) 2012 Red Hat
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License
 + * as published by the Free Software Foundation; either version 2
 + * of the License, or (at your option) any later version.
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include drm/drmP.h
 +#include drm/drm_crtc.h
 +#include drm/drm_fb_helper.h
 +#include drm/drm_crtc_helper.h
 +#include drm/drm_gem_cma_helper.h
 +#include drm/drm_fb_cma_helper.h
 +#include linux/module.h
 +
 +struct drm_fb_cma {
 + struct drm_framebuffer  fb;
 + struct drm_gem_cma_obj  *obj[4];

Can we have a define for this magic '4'? It is used several times in
this patch.

 +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
 + struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj,
 + unsigned int num_planes)
 +{
 + struct drm_fb_cma *fb_cma;
 + int ret;
 + int i;
 +
 + fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
 + if (!fb_cma)
 + return ERR_PTR(-ENOMEM);
 +
 + ret = drm_framebuffer_init(dev, fb_cma-fb, drm_fb_cma_funcs);
 + if (ret) {
 + dev_err(dev-dev, Failed to initalize framebuffer: %d\n, ret);
 + kfree(fb_cma);
 + return ERR_PTR(ret);
 + }
 +
 + drm_helper_mode_fill_fb_struct(fb_cma-fb, mode_cmd);
 +
 + for (i = 0; i  num_planes; i++)
 + fb_cma-obj[i] = obj[i];

Check for valid num_planes before this loop?


Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object
in the patch I just sent.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] DRM: Add DRM kms/fb cma helper

2012-05-31 Thread Lars-Peter Clausen 
On 05/31/2012 10:13 AM, Sascha Hauer wrote:
 On Wed, May 30, 2012 at 05:30:15PM +0200, Lars-Peter Clausen wrote:
 This patch introduces a set of helper function for implementing the KMS
 framebuffer layer for drivers which use the drm gem CMA helper function.

 Signed-off-by: Lars-Peter Clausen l...@metafoo.de

 ---
 Changes since v1:
  * Some spelling fixes
  * Add missing kfree in drm_fb_cma_alloc error path
  * Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second
parameter to specify the plane.
 ---
  drivers/gpu/drm/Kconfig |   11 +
  drivers/gpu/drm/Makefile|1 +
  drivers/gpu/drm/drm_fb_cma_helper.c |  384 
 +++
  include/drm/drm_fb_cma_helper.h |   27 +++
  4 files changed, 423 insertions(+)
  create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c
  create mode 100644 include/drm/drm_fb_cma_helper.h

 diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
 index a78dfbe..e7c0a3d 100644
 --- a/drivers/gpu/drm/Kconfig
 +++ b/drivers/gpu/drm/Kconfig
 @@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER
  help
Choose this if you need the GEM cma helper functions
  
 +config DRM_KMS_CMA_HELPER
 +tristate
 +select DRM_GEM_CMA_HELPER
 +select DRM_KMS_HELPER
 +select FB_SYS_FILLRECT
 +select FB_SYS_COPYAREA
 +select FB_SYS_IMAGEBLIT
 +help
 +  Choose this if you need the KMS cma helper functions
 +
 +
  config DRM_TDFX
  tristate 3dfx Banshee/Voodoo3+
  depends on DRM  PCI
 diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 index 6e9e948..5dcb1a5 100644
 --- a/drivers/gpu/drm/Makefile
 +++ b/drivers/gpu/drm/Makefile
 @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o
  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
  
  drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o
 +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
  
  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
  
 diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
 b/drivers/gpu/drm/drm_fb_cma_helper.c
 new file mode 100644
 index 000..8821a98
 --- /dev/null
 +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
 @@ -0,0 +1,384 @@
 +/*
 + * drm kms/fb cma (contiguous memory allocator) helper functions
 + *
 + * Copyright (C) 2012 Analog Device Inc.
 + *   Author: Lars-Peter Clausen l...@metafoo.de
 + *
 + * Based on udl_fbdev.c
 + *  Copyright (C) 2012 Red Hat
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License
 + * as published by the Free Software Foundation; either version 2
 + * of the License, or (at your option) any later version.
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include drm/drmP.h
 +#include drm/drm_crtc.h
 +#include drm/drm_fb_helper.h
 +#include drm/drm_crtc_helper.h
 +#include drm/drm_gem_cma_helper.h
 +#include drm/drm_fb_cma_helper.h
 +#include linux/module.h
 +
 +struct drm_fb_cma {
 +struct drm_framebuffer  fb;
 +struct drm_gem_cma_obj  *obj[4];
 
 Can we have a define for this magic '4'? It is used several times in
 this patch.

Yes, had the same though. The magic 4 is actually used all through all of DRM.
Something like '#define DRM_MAX_PLANES 4' probably makes sense.

 
 +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
 +struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj,
 +unsigned int num_planes)
 +{
 +struct drm_fb_cma *fb_cma;
 +int ret;
 +int i;
 +
 +fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
 +if (!fb_cma)
 +return ERR_PTR(-ENOMEM);
 +
 +ret = drm_framebuffer_init(dev, fb_cma-fb, drm_fb_cma_funcs);
 +if (ret) {
 +dev_err(dev-dev, Failed to initalize framebuffer: %d\n, ret);
 +kfree(fb_cma);
 +return ERR_PTR(ret);
 +}
 +
 +drm_helper_mode_fill_fb_struct(fb_cma-fb, mode_cmd);
 +
 +for (i = 0; i  num_planes; i++)
 +fb_cma-obj[i] = obj[i];
 
 Check for valid num_planes before this loop?
 

Hm, I think the callers already take care of this. drm_format_num_planes will
always return a valid number and the other caller passes 1 unconditionally.

 
 Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object
 in the patch I just sent.
 

Do you think it makes sense for you to carry this patch as part of your iMX DRM
series?

Thanks,
- Lars
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] DRM: Add DRM kms/fb cma helper

2012-05-31 Thread Sascha Hauer 
On Thu, May 31, 2012 at 11:34:37AM +0200, Lars-Peter Clausen wrote:
  +  drm_helper_mode_fill_fb_struct(fb_cma-fb, mode_cmd);
  +
  +  for (i = 0; i  num_planes; i++)
  +  fb_cma-obj[i] = obj[i];
  
  Check for valid num_planes before this loop?
  
 
 Hm, I think the callers already take care of this. drm_format_num_planes will
 always return a valid number and the other caller passes 1 unconditionally.

As long as the array always is big enough to hold the maximum number of
planes I think it's fine. However, if there is some format with 5 planes
(don't know if there is, I only know yuv with multiple planes) it's not
obvious that this code does not support this.

 
  
  Note I have renamed struct drm_gem_cma_obj to struct drm_gem_cma_object
  in the patch I just sent.
  
 
 Do you think it makes sense for you to carry this patch as part of your iMX 
 DRM
 series?

I can carry this, but it could be that Laurent has his driver in an
acceptable state earlier. Let's see.

Sascha


-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] DRM: Add DRM kms/fb cma helper

2012-05-30 Thread Lars-Peter Clausen 
This patch introduces a set of helper function for implementing the KMS
framebuffer layer for drivers which use the drm gem CMA helper function.

Signed-off-by: Lars-Peter Clausen 

---
Changes since v1:
* Some spelling fixes
* Add missing kfree in drm_fb_cma_alloc error path
* Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second
  parameter to specify the plane.
---
 drivers/gpu/drm/Kconfig |   11 +
 drivers/gpu/drm/Makefile|1 +
 drivers/gpu/drm/drm_fb_cma_helper.c |  384 +++
 include/drm/drm_fb_cma_helper.h |   27 +++
 4 files changed, 423 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c
 create mode 100644 include/drm/drm_fb_cma_helper.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index a78dfbe..e7c0a3d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER
help
  Choose this if you need the GEM cma helper functions

+config DRM_KMS_CMA_HELPER
+   tristate
+   select DRM_GEM_CMA_HELPER
+   select DRM_KMS_HELPER
+   select FB_SYS_FILLRECT
+   select FB_SYS_COPYAREA
+   select FB_SYS_IMAGEBLIT
+   help
+ Choose this if you need the KMS cma helper functions
+
+
 config DRM_TDFX
tristate "3dfx Banshee/Voodoo3+"
depends on DRM && PCI
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6e9e948..5dcb1a5 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o

 drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o
+drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o

 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
b/drivers/gpu/drm/drm_fb_cma_helper.c
new file mode 100644
index 000..8821a98
--- /dev/null
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -0,0 +1,384 @@
+/*
+ * drm kms/fb cma (contiguous memory allocator) helper functions
+ *
+ * Copyright (C) 2012 Analog Device Inc.
+ *   Author: Lars-Peter Clausen 
+ *
+ * Based on udl_fbdev.c
+ *  Copyright (C) 2012 Red Hat
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct drm_fb_cma {
+   struct drm_framebuffer  fb;
+   struct drm_gem_cma_obj  *obj[4];
+};
+
+struct drm_fbdev_cma {
+   struct drm_fb_helperfb_helper;
+   struct drm_fb_cma   *fb;
+};
+
+static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
+{
+   return container_of(helper, struct drm_fbdev_cma, fb_helper);
+}
+
+static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)
+{
+   return container_of(fb, struct drm_fb_cma, fb);
+}
+
+static void drm_fb_cma_destroy(struct drm_framebuffer *fb)
+{
+   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+   int i;
+
+   for (i = 0; i < 4; i++) {
+   if (fb_cma->obj[i])
+   
drm_gem_object_unreference_unlocked(_cma->obj[i]->base);
+   }
+
+   drm_framebuffer_cleanup(fb);
+   kfree(fb_cma);
+}
+
+static int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
+   struct drm_file *file_priv, unsigned int *handle)
+{
+   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+
+   return drm_gem_handle_create(file_priv,
+   _cma->obj[0]->base, handle);
+}
+
+static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
+   .destroy= drm_fb_cma_destroy,
+   .create_handle  = drm_fb_cma_create_handle,
+};
+
+static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
+   struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj,
+   unsigned int num_planes)
+{
+   struct drm_fb_cma *fb_cma;
+   int ret;
+   int i;
+
+   fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
+   if (!fb_cma)
+   return ERR_PTR(-ENOMEM);
+
+   ret = drm_framebuffer_init(dev, _cma->fb, _fb_cma_funcs);
+   if (ret) {
+   dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret);
+   kfree(fb_cma);
+   return ERR_PTR(ret);
+   }
+
+   drm_helper_mode_fill_fb_struct(_cma->fb, mode_cmd);
+
+   for (i = 0; i < num_planes; i++)
+   fb_cma->obj[i] = obj[i];
+
+   return fb_cma;

[PATCH v2] DRM: Add DRM kms/fb cma helper

2012-05-30 Thread Lars-Peter Clausen 
This patch introduces a set of helper function for implementing the KMS
framebuffer layer for drivers which use the drm gem CMA helper function.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de

---
Changes since v1:
* Some spelling fixes
* Add missing kfree in drm_fb_cma_alloc error path
* Add multi-plane support. drm_fb_cma_get_gem_obj now takes a second
  parameter to specify the plane.
---
 drivers/gpu/drm/Kconfig |   11 +
 drivers/gpu/drm/Makefile|1 +
 drivers/gpu/drm/drm_fb_cma_helper.c |  384 +++
 include/drm/drm_fb_cma_helper.h |   27 +++
 4 files changed, 423 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c
 create mode 100644 include/drm/drm_fb_cma_helper.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index a78dfbe..e7c0a3d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -41,6 +41,17 @@ config DRM_GEM_CMA_HELPER
help
  Choose this if you need the GEM cma helper functions
 
+config DRM_KMS_CMA_HELPER
+   tristate
+   select DRM_GEM_CMA_HELPER
+   select DRM_KMS_HELPER
+   select FB_SYS_FILLRECT
+   select FB_SYS_COPYAREA
+   select FB_SYS_IMAGEBLIT
+   help
+ Choose this if you need the KMS cma helper functions
+
+
 config DRM_TDFX
tristate 3dfx Banshee/Voodoo3+
depends on DRM  PCI
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6e9e948..5dcb1a5 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
 
 drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o
+drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
 
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
 
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
b/drivers/gpu/drm/drm_fb_cma_helper.c
new file mode 100644
index 000..8821a98
--- /dev/null
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -0,0 +1,384 @@
+/*
+ * drm kms/fb cma (contiguous memory allocator) helper functions
+ *
+ * Copyright (C) 2012 Analog Device Inc.
+ *   Author: Lars-Peter Clausen l...@metafoo.de
+ *
+ * Based on udl_fbdev.c
+ *  Copyright (C) 2012 Red Hat
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include drm/drmP.h
+#include drm/drm_crtc.h
+#include drm/drm_fb_helper.h
+#include drm/drm_crtc_helper.h
+#include drm/drm_gem_cma_helper.h
+#include drm/drm_fb_cma_helper.h
+#include linux/module.h
+
+struct drm_fb_cma {
+   struct drm_framebuffer  fb;
+   struct drm_gem_cma_obj  *obj[4];
+};
+
+struct drm_fbdev_cma {
+   struct drm_fb_helperfb_helper;
+   struct drm_fb_cma   *fb;
+};
+
+static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
+{
+   return container_of(helper, struct drm_fbdev_cma, fb_helper);
+}
+
+static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)
+{
+   return container_of(fb, struct drm_fb_cma, fb);
+}
+
+static void drm_fb_cma_destroy(struct drm_framebuffer *fb)
+{
+   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+   int i;
+
+   for (i = 0; i  4; i++) {
+   if (fb_cma-obj[i])
+   
drm_gem_object_unreference_unlocked(fb_cma-obj[i]-base);
+   }
+
+   drm_framebuffer_cleanup(fb);
+   kfree(fb_cma);
+}
+
+static int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
+   struct drm_file *file_priv, unsigned int *handle)
+{
+   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+
+   return drm_gem_handle_create(file_priv,
+   fb_cma-obj[0]-base, handle);
+}
+
+static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
+   .destroy= drm_fb_cma_destroy,
+   .create_handle  = drm_fb_cma_create_handle,
+};
+
+static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
+   struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_obj **obj,
+   unsigned int num_planes)
+{
+   struct drm_fb_cma *fb_cma;
+   int ret;
+   int i;
+
+   fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
+   if (!fb_cma)
+   return ERR_PTR(-ENOMEM);
+
+   ret = drm_framebuffer_init(dev, fb_cma-fb, drm_fb_cma_funcs);
+   if (ret) {
+   dev_err(dev-dev, Failed to initalize framebuffer: %d\n, ret);
+   kfree(fb_cma);
+   return ERR_PTR(ret);
+   }
+
+