Re: [PATCH] omap2+: add drm device

On Mon, 2012-03-05 at 10:54 -0600, Rob Clark wrote:

From: Andy Gross andy.gross@ti.com

Register OMAP DRM/KMS platform device, and reserve a CMA region for the device to use for buffer allocation. DMM is split into a separate device using hwmod.

Signed-off-by: Andy Gross andy.gross@ti.com Signed-off-by: Rob Clark rob@ti.com

---

arch/arm/plat-omap/Makefile | 2 +- arch/arm/plat-omap/common.c | 3 +- arch/arm/plat-omap/drm.c | 83 +++++++++++++++++++++++++++++++++ arch/arm/plat-omap/include/plat/drm.h | 64 +++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 arch/arm/plat-omap/drm.c create mode 100644 arch/arm/plat-omap/include/plat/drm.h

diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile index 9a58461..b86e6cb 100644 --- a/arch/arm/plat-omap/Makefile +++ b/arch/arm/plat-omap/Makefile @@ -4,7 +4,7 @@

# Common support obj-y := common.o sram.o clock.o devices.o dma.o mux.o \

• usb.o fb.o counter_32k.o
  

• usb.o fb.o counter_32k.o drm.o
  

obj-m := obj-n := obj- := diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c index 06383b5..0d87dab 100644 --- a/arch/arm/plat-omap/common.c +++ b/arch/arm/plat-omap/common.c @@ -21,10 +21,10 @@ #include <plat/board.h> #include <plat/vram.h> #include <plat/dsp.h> +#include <plat/drm.h>

#include <plat/omap-secure.h>

#define NO_LENGTH_CHECK 0xffffffff

struct omap_board_config_kernel *omap_board_config __initdata; @@ -65,6 +65,7 @@ const void *__init omap_get_var_config(u16 tag, size_t *len)

void __init omap_reserve(void) {

• omapdrm_reserve_vram(); omapfb_reserve_sdram_memblock(); omap_vram_reserve_sdram_memblock(); omap_dsp_reserve_sdram_memblock();

diff --git a/arch/arm/plat-omap/drm.c b/arch/arm/plat-omap/drm.c

As Tony said, mach-omap2 is probably a better place. fb.c is in plat-omap because it supports both OMAP1 and OMAP2+.

new file mode 100644 index 0000000..28279df --- /dev/null +++ b/arch/arm/plat-omap/drm.c @@ -0,0 +1,83 @@ +/*

• • DRM/KMS device registration for TI OMAP platforms
• • Copyright (C) 2012 Texas Instruments
• • Author: Rob Clark rob.clark@linaro.org
• • This program is free software; you can redistribute it and/or modify it
• • under the terms of the GNU General Public License version 2 as published by
• • the Free Software Foundation.
• • 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.
• • You should have received a copy of the GNU General Public License along with
• • this program. If not, see http://www.gnu.org/licenses/.
• */

+#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/mm.h> +#include <linux/init.h> +#include <linux/platform_device.h> +#include <linux/dma-mapping.h> +#ifdef CONFIG_CMA +# include <linux/dma-contiguous.h> +#endif

+#include <plat/omap_device.h> +#include <plat/omap_hwmod.h>

+#include <plat/drm.h>

+#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)

+static struct omap_drm_platform_data omapdrm_platdata;

+static struct platform_device omap_drm_device = {

• .dev = {
  

• 	.coherent_dma_mask = DMA_BIT_MASK(32),
  

• 	.platform_data = &omapdrm_platdata,
  

• },
  

• .name = "omapdrm",
  

• .id = 0,
  

Can there be more than one omapdrm device? I guess not. If so, the id should be -1.

+};

+static int __init omap_init_gpu(void) +{

• struct omap_hwmod *oh = NULL;
• struct platform_device *pdev;
• /* lookup and populate the DMM information, if present - OMAP4+ */
• oh = omap_hwmod_lookup("dmm");
• if (oh) {

• pdev = omap_device_build(oh->name, -1, oh, NULL, 0, NULL, 0,
  

• 			false);
  

• WARN(IS_ERR(pdev), "Could not build omap_device for %s\n",
  

• 	oh->name);
  

• }
• return platform_device_register(&omap_drm_device);

+}

The function is called omap_init_gpu(), but it doesn't do anything related to the gpu... And aren't DMM and DRM totally separate things, why are they bunched together like that?

+arch_initcall(omap_init_gpu);

+void omapdrm_reserve_vram(void) +{ +#ifdef CONFIG_CMA

• /*

• * Create private 32MiB contiguous memory area for omapdrm.0 device
  

• * TODO revisit size.. if uc/wc buffers are allocated from CMA pages
  

• * then the amount of memory we need goes up..
  

• */
  

• dma_declare_contiguous(&omap_drm_device.dev, 32 * SZ_1M, 0, 0);

What does this actually do? Does it reserve the memory, i.e. it's not usable for others? If so, shouldn't there be a way for the user to configure it?

+#else +# warning "CMA is not enabled, there may be limitations about scanout buffer allocations on OMAP3 and earlier" +#endif +}

+#endif diff --git a/arch/arm/plat-omap/include/plat/drm.h b/arch/arm/plat-omap/include/plat/drm.h new file mode 100644 index 0000000..df9bc41 --- /dev/null +++ b/arch/arm/plat-omap/include/plat/drm.h @@ -0,0 +1,64 @@ +/*

• • DRM/KMS device registration for TI OMAP platforms
• • Copyright (C) 2012 Texas Instruments
• • Author: Rob Clark rob.clark@linaro.org
• • This program is free software; you can redistribute it and/or modify it
• • under the terms of the GNU General Public License version 2 as published by
• • the Free Software Foundation.
• • 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.
• • You should have received a copy of the GNU General Public License along with
• • this program. If not, see http://www.gnu.org/licenses/.
• */

+#ifndef __PLAT_OMAP_DRM_H__ +#define __PLAT_OMAP_DRM_H__

+/*

• • Optional platform data to configure the default configuration of which
• • pipes/overlays/CRTCs are used.. if this is not provided, then instead the
• • first CONFIG_DRM_OMAP_NUM_CRTCS are used, and they are each connected to
• • one manager, with priority given to managers that are connected to
• • detected devices. Remaining overlays are used as video planes. This
• • should be a good default behavior for most cases, but yet there still
• • might be times when you wish to do something different.
• */

+struct omap_kms_platform_data {

• /* overlays to use as CRTCs: */
• int ovl_cnt;
• const int *ovl_ids;
• /* overlays to use as video planes: */
• int pln_cnt;
• const int *pln_ids;
• int mgr_cnt;
• const int *mgr_ids;
• int dev_cnt;
• const char **dev_names;

+};

+struct omap_drm_platform_data {

• struct omap_kms_platform_data *kms_pdata;

+};

I don't know if there's need to add these... With device tree the plaform data will not be usable anyway.

Tomi