diff mbox

omap2+: add drm device

Message ID 1330966464-28662-1-git-send-email-rob.clark@linaro.org
State Superseded
Headers show

Commit Message

Rob Clark March 5, 2012, 4:54 p.m. UTC
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

Comments

Tony Lindgren March 6, 2012, 12:10 a.m. UTC | #1
* Rob Clark <rob.clark@linaro.org> [120305 08:24]:
> 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.
> --- 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();

Tomi needs to look at the vram changes here.

> --- /dev/null
> +++ b/arch/arm/plat-omap/drm.c

This file would be better in mach-omap2/drm.c. I doubt that this will
ever be usable for omap1. But other than that, up to Tomi.

Tony
Rob Clark March 6, 2012, 1:42 a.m. UTC | #2
On Mon, Mar 5, 2012 at 6:10 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Rob Clark <rob.clark@linaro.org> [120305 08:24]:
>> 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.
>> --- 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();
>
> Tomi needs to look at the vram changes here.

it is just setting up a CMA region (if CMA is enabled.. basically for
once CMA is merged)

after CMA is merged, other drivers should be migrated to use CMA, and
omap_vram removed.. but that is a topic for another patch

>> --- /dev/null
>> +++ b/arch/arm/plat-omap/drm.c
>
> This file would be better in mach-omap2/drm.c. I doubt that this will
> ever be usable for omap1. But other than that, up to Tomi.

I was attempting to copy what omapfb was doing (plat-omap/fb.c)
because I wasn't really sure about plat-omap vs mach-omap2, although
on closer inspection I think the file I was looking at is actually for
the legacy omapdss1..  So I'll move this stuff under mach-omap2 and
resubmit this patch.

BR,
-R

> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen March 6, 2012, 1:26 p.m. UTC | #3
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
Rob Clark March 6, 2012, 2:01 p.m. UTC | #4
On Tue, Mar 6, 2012 at 7:26 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 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.

in the past, we have used multiple devices (using the platform-data to
divide up the dss resources), although this was sort of a
special-case.  But in theory you could have multiple.  (Think of a
multi-seat scenario, where multiple compositors need to run and each
need to be drm-master of their own device to do mode-setting on their
corresponding display.)

I think if we use .id = -1, then we'd end up with a funny looking
bus-id for the device: "platform:omapdrm:-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?

only legacy.. product branches also have sgx initialization here.  But
I can change that to omap_init_drm()

DMM is managed by the drm driver (and exposed to userspace as drm/gem
buffers, shared with other devices via dma-buf, etc).  It is only
split out to a separate device to play along with hwmod.

>> +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?

It can be re-used by others.. see http://lwn.net/Articles/479297/

BR,
-R

>> +#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
>
Tomi Valkeinen March 6, 2012, 2:35 p.m. UTC | #5
On Tue, 2012-03-06 at 08:01 -0600, Rob Clark wrote:
> On Tue, Mar 6, 2012 at 7:26 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> > Can there be more than one omapdrm device? I guess not. If so, the id
> > should be -1.
> 
> in the past, we have used multiple devices (using the platform-data to
> divide up the dss resources), although this was sort of a
> special-case.  But in theory you could have multiple.  (Think of a
> multi-seat scenario, where multiple compositors need to run and each
> need to be drm-master of their own device to do mode-setting on their
> corresponding display.)
> 
> I think if we use .id = -1, then we'd end up with a funny looking
> bus-id for the device: "platform:omapdrm:-1"

I don't know about that, but it's the way platform devices are meant to
be used if there can be only one instance. If the case where there are
multiple omapdrm devices is rather theoretical, or only used for certain
debugging scenarios or such, I think having the id as -1 in the mainline
is cleaner.

> >> +};
> >> +
> >> +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?
> 
> only legacy.. product branches also have sgx initialization here.  But
> I can change that to omap_init_drm()
> 
> DMM is managed by the drm driver (and exposed to userspace as drm/gem
> buffers, shared with other devices via dma-buf, etc).  It is only
> split out to a separate device to play along with hwmod.

I have to say I don't know much about DMM, but my understanding is that
DMM is a bigger entity, of which TILER is only a small part, and DMM
manages all memory accesses.

Can there be other users for the DMM than DRM? I know there could be
other users for the TILER, and I know you want to combine that with the
DRM driver, but I'm wondering about the other parts of DMM than the
TILER.

Somehow having a DMM driver inside omapdrm sounds strange.

> >> +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?
> 
> It can be re-used by others.. see http://lwn.net/Articles/479297/

The link didn't tell much. I looked at the patches, and
dma_declare_contiguous allocates the memory with memblock_alloc. So is
that allocated memory available for any users? If so, why does the
function want a device pointer as an argument...

Is the memory available for normal kmalloc, or only available via the
CMA functions? Surely there must be some downside to the above? =) And
if so, it should be configurable. You could have a board with no display
at all, and you probably don't want to have 32MB allocated for DRM in
that case.

 Tomi
Andy Gross March 6, 2012, 3:50 p.m. UTC | #6
On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen <tomi.valkeinen@ti.com>wrote:
>
>
> I have to say I don't know much about DMM, but my understanding is that
> DMM is a bigger entity, of which TILER is only a small part, and DMM
> manages all memory accesses.
>
> Can there be other users for the DMM than DRM? I know there could be
> other users for the TILER, and I know you want to combine that with the
> DRM driver, but I'm wondering about the other parts of DMM than the
> TILER.
>
> Somehow having a DMM driver inside omapdrm sounds strange.


The DMM does indeed contain a number of entities.  However, the TILER
portion is the only part that requires a driver.  All other register
modifications (LISA map settings, EMIF, etc) are done statically in the
loader or u-boot and never changed again.  As such, DMM has become
synonymous with TILER.

Regards,

Andy
Tomi Valkeinen March 7, 2012, 12:05 p.m. UTC | #7
On Tue, 2012-03-06 at 09:50 -0600, Gross, Andy wrote:
> 
> 
> On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen <tomi.valkeinen@ti.com>
> wrote:
>         
>         
>         I have to say I don't know much about DMM, but my
>         understanding is that
>         DMM is a bigger entity, of which TILER is only a small part,
>         and DMM
>         manages all memory accesses.
>         
>         Can there be other users for the DMM than DRM? I know there
>         could be
>         other users for the TILER, and I know you want to combine that
>         with the
>         DRM driver, but I'm wondering about the other parts of DMM
>         than the
>         TILER.
>         
>         Somehow having a DMM driver inside omapdrm sounds strange.
> 
> 
> The DMM does indeed contain a number of entities.  However, the TILER
> portion is the only part that requires a driver.  All other register
> modifications (LISA map settings, EMIF, etc) are done statically in
> the loader or u-boot and never changed again.  As such, DMM has become
> synonymous with TILER.

Ok. Well, as I said, I don't know much about that, just sounds rather
strange to me =).

Does this "DMM has become synonymous" mean that people just started
calling TILER DMM, and thus the name has stuck, or are there technical
reasons to handle it as DMM in the kernel? If the former, and if TILER
is the technically exact name for it, perhaps it would make sense to
call it TILER, as that's what (at least I) would be looking for if I
read the TRM and try to find the code for the TILER.

 Tomi
Andy Gross March 7, 2012, 3:59 p.m. UTC | #8
On Wed, Mar 7, 2012 at 6:05 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> Does this "DMM has become synonymous" mean that people just started
> calling TILER DMM, and thus the name has stuck, or are there technical
> reasons to handle it as DMM in the kernel? If the former, and if TILER
> is the technically exact name for it, perhaps it would make sense to
> call it TILER, as that's what (at least I) would be looking for if I
> read the TRM and try to find the code for the TILER.
>
>  Tomi

Well the breakdown of the DMM/Tiler kind of goes like this.  The DMM
defines a set of registers used to manipulate the PAT table that backs
the address space with physical memory.  The Tiler portion of the DMM
really just describes the address space that is exposed.  Depending on
what address range you access, you'll get a different mode of access
and rotation.  Management of the address space is attributed to the
Tiler as well and that is where we have managed the address space and
provided APIs to reserve address space and pin/unpin memory to those
regions.

From a hardware perspective, we need access to the resources provided
by the DMM so that we can do the PAT programming and that can only be
gotten from the hwmod entry.  And if you use the hwmod device entry,
you have to match the name used for that entry.

Regards,

Andy
Tomi Valkeinen March 8, 2012, 7:47 a.m. UTC | #9
On Wed, 2012-03-07 at 09:59 -0600, Gross, Andy wrote:
> On Wed, Mar 7, 2012 at 6:05 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> > Does this "DMM has become synonymous" mean that people just started
> > calling TILER DMM, and thus the name has stuck, or are there technical
> > reasons to handle it as DMM in the kernel? If the former, and if TILER
> > is the technically exact name for it, perhaps it would make sense to
> > call it TILER, as that's what (at least I) would be looking for if I
> > read the TRM and try to find the code for the TILER.
> >
> >  Tomi
> 
> Well the breakdown of the DMM/Tiler kind of goes like this.  The DMM
> defines a set of registers used to manipulate the PAT table that backs
> the address space with physical memory.  The Tiler portion of the DMM
> really just describes the address space that is exposed.  Depending on
> what address range you access, you'll get a different mode of access
> and rotation.  Management of the address space is attributed to the
> Tiler as well and that is where we have managed the address space and
> provided APIs to reserve address space and pin/unpin memory to those
> regions.
> 
> From a hardware perspective, we need access to the resources provided
> by the DMM so that we can do the PAT programming and that can only be
> gotten from the hwmod entry.  And if you use the hwmod device entry,
> you have to match the name used for that entry.

Ok. Thanks for the detailed explanation =).

 Tomi
diff mbox

Patch

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
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,
+};
+
+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);
+
+}
+
+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);
+#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;
+};
+
+#if defined(CONFIG_DRM_OMAP) || defined(CONFIG_DRM_OMAP_MODULE)
+
+void omapdrm_reserve_vram(void);
+
+#else
+
+static inline void omapdrm_reserve_vram(void)
+{
+}
+
+#endif
+
+#endif /* __PLAT_OMAP_DRM_H__ */