All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] omap2+: add drm device
@ 2012-03-13 20:34 Rob Clark
  2012-03-14 12:38 ` Tomi Valkeinen
  0 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-03-13 20:34 UTC (permalink / raw)
  To: dri-devel, linux-omap
  Cc: patches, Greg KH, Tomi Valkeinen, Andy Gross, Rob Clark

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/mach-omap2/Makefile          |    4 ++
 arch/arm/mach-omap2/drm.c             |   83 +++++++++++++++++++++++++++++++++
 arch/arm/plat-omap/common.c           |    3 +-
 arch/arm/plat-omap/include/plat/drm.h |   64 +++++++++++++++++++++++++
 4 files changed, 153 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-omap2/drm.c
 create mode 100644 arch/arm/plat-omap/include/plat/drm.h

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index bd76394..9e6065b 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -189,6 +189,10 @@ ifneq ($(CONFIG_TIDSPBRIDGE),)
 obj-y					+= dsp.o
 endif
 
+ifneq ($(CONFIG_DRM_OMAP),)
+obj-y					+= drm.o
+endif
+
 # Specific board support
 obj-$(CONFIG_MACH_OMAP_GENERIC)		+= board-generic.o
 obj-$(CONFIG_MACH_OMAP_H4)		+= board-h4.o
diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c
new file mode 100644
index 0000000..779ae02
--- /dev/null
+++ b/arch/arm/mach-omap2/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_drm(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_drm);
+
+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/common.c b/arch/arm/plat-omap/common.c
index 4de7d1e..e027cc7 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/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__ */
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-13 20:34 [PATCH] omap2+: add drm device Rob Clark
@ 2012-03-14 12:38 ` Tomi Valkeinen
  2012-03-14 12:55   ` Rob Clark
  0 siblings, 1 reply; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-14 12:38 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, linux-omap, patches, Greg KH, Andy Gross, Rob Clark

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

Hi,

On Tue, 2012-03-13 at 15:34 -0500, 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.

What's the diff with this and the previous one?

I see you still have the platform data there. You didn't comment on
that. Where is it used after the board files are gone when we use DT?

And how about the size of the contiguous memory area, it was left a bit
unclear to me why it cannot be dynamic.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-14 12:38 ` Tomi Valkeinen
@ 2012-03-14 12:55   ` Rob Clark
  2012-03-14 13:07     ` Tomi Valkeinen
  0 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-03-14 12:55 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: dri-devel, linux-omap, patches, Greg KH, Andy Gross

On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi,
>
> On Tue, 2012-03-13 at 15:34 -0500, 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.
>
> What's the diff with this and the previous one?

Moving drm.c to mach-omap2 (header could not move because
omap_reserve() is in plat-omap.. but this seems to be the same as what
is done for dspbridge).

> I see you still have the platform data there. You didn't comment on
> that. Where is it used after the board files are gone when we use DT?

I was kind-of thinking that there would be some DT<->data-structure
glue somewhere.. not sure if this goes in mach-omap2 or in the driver
itself, but presumably it is needed somewhere.

It is only really special cases where some board specific device-tree
data is needed, so it seems like this is ok to handle later.

> And how about the size of the contiguous memory area, it was left a bit
> unclear to me why it cannot be dynamic.

I don't think there is anything preventing adding a bootarg, but I
think it is not essential so ok to add later

BR,
-R

>  Tomi
>
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-14 12:55   ` Rob Clark
@ 2012-03-14 13:07     ` Tomi Valkeinen
  2012-03-14 13:16       ` Rob Clark
  0 siblings, 1 reply; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-14 13:07 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, linux-omap, patches, Greg KH, Andy Gross

On Wed, 2012-03-14 at 07:55 -0500, Rob Clark wrote:
> On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > Hi,
> >
> > On Tue, 2012-03-13 at 15:34 -0500, 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.
> >
> > What's the diff with this and the previous one?
> 
> Moving drm.c to mach-omap2 (header could not move because
> omap_reserve() is in plat-omap.. but this seems to be the same as what
> is done for dspbridge).
> 
> > I see you still have the platform data there. You didn't comment on
> > that. Where is it used after the board files are gone when we use DT?
> 
> I was kind-of thinking that there would be some DT<->data-structure
> glue somewhere.. not sure if this goes in mach-omap2 or in the driver
> itself, but presumably it is needed somewhere.
> 
> It is only really special cases where some board specific device-tree
> data is needed, so it seems like this is ok to handle later.

Afaik DT data should only contain information about the hardware. This
is SW configuration, so I think DT people won't accept things like that.

> > And how about the size of the contiguous memory area, it was left a bit
> > unclear to me why it cannot be dynamic.
> 
> I don't think there is anything preventing adding a bootarg, but I
> think it is not essential so ok to add later

Well, maybe not essential to you =). But you are reserving 32MB memory,
which is quite a big amount. Even if the reserved memory can be used for
some other purposes, it's still a big chunk of "special" memory being
reserved even if the user doesn't use or have a display at all.

Well, it's not an issue for me either, but I just feel that doing things
like that without allowing the user to avoid it is a bit bad thing.

Btw, do you know why the dma_declare_contiguous() takes a dev pointer as
an argument, if the memory is not private to that device? (at least I
understood from you that the memory can be used for other purposes).

 Tomi



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-14 13:07     ` Tomi Valkeinen
@ 2012-03-14 13:16       ` Rob Clark
  2012-03-14 13:43         ` Tomi Valkeinen
  0 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-03-14 13:16 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Greg KH, linux-omap, dri-devel, patches

On Wed, Mar 14, 2012 at 8:07 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2012-03-14 at 07:55 -0500, Rob Clark wrote:
>> On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > Hi,
>> >
>> > On Tue, 2012-03-13 at 15:34 -0500, 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.
>> >
>> > What's the diff with this and the previous one?
>>
>> Moving drm.c to mach-omap2 (header could not move because
>> omap_reserve() is in plat-omap.. but this seems to be the same as what
>> is done for dspbridge).
>>
>> > I see you still have the platform data there. You didn't comment on
>> > that. Where is it used after the board files are gone when we use DT?
>>
>> I was kind-of thinking that there would be some DT<->data-structure
>> glue somewhere.. not sure if this goes in mach-omap2 or in the driver
>> itself, but presumably it is needed somewhere.
>>
>> It is only really special cases where some board specific device-tree
>> data is needed, so it seems like this is ok to handle later.
>
> Afaik DT data should only contain information about the hardware. This
> is SW configuration, so I think DT people won't accept things like that.

hmm, then where *does* it go.. it is a bit too much for bootargs..

>> > And how about the size of the contiguous memory area, it was left a bit
>> > unclear to me why it cannot be dynamic.
>>
>> I don't think there is anything preventing adding a bootarg, but I
>> think it is not essential so ok to add later
>
> Well, maybe not essential to you =). But you are reserving 32MB memory,
> which is quite a big amount. Even if the reserved memory can be used for
> some other purposes, it's still a big chunk of "special" memory being
> reserved even if the user doesn't use or have a display at all.

If you didn't have display, and were tight on memory, wouldn't you
just disable the display device in your kernel config?

> Well, it's not an issue for me either, but I just feel that doing things
> like that without allowing the user to avoid it is a bit bad thing.
>
> Btw, do you know why the dma_declare_contiguous() takes a dev pointer as
> an argument, if the memory is not private to that device? (at least I
> understood from you that the memory can be used for other purposes).

contiguous use of the memory is private to the device.  There is also
a global CMA pool, from which all dma_alloc_xyz() which is not
associated w/ a per-device pool comes from.. but the advantage of a
per-device-pool is it puts an upper limit on how much dma memory that
device can allocate so that it cannot starve other devices.

BR,
-R

>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-14 13:16       ` Rob Clark
@ 2012-03-14 13:43         ` Tomi Valkeinen
  2012-03-14 15:06           ` Rob Clark
  0 siblings, 1 reply; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-14 13:43 UTC (permalink / raw)
  To: Rob Clark; +Cc: Greg KH, linux-omap, dri-devel, patches

On Wed, 2012-03-14 at 08:16 -0500, Rob Clark wrote:
> On Wed, Mar 14, 2012 at 8:07 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Wed, 2012-03-14 at 07:55 -0500, Rob Clark wrote:
> >> On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> > Hi,
> >> >
> >> > On Tue, 2012-03-13 at 15:34 -0500, 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.
> >> >
> >> > What's the diff with this and the previous one?
> >>
> >> Moving drm.c to mach-omap2 (header could not move because
> >> omap_reserve() is in plat-omap.. but this seems to be the same as what
> >> is done for dspbridge).
> >>
> >> > I see you still have the platform data there. You didn't comment on
> >> > that. Where is it used after the board files are gone when we use DT?
> >>
> >> I was kind-of thinking that there would be some DT<->data-structure
> >> glue somewhere.. not sure if this goes in mach-omap2 or in the driver
> >> itself, but presumably it is needed somewhere.
> >>
> >> It is only really special cases where some board specific device-tree
> >> data is needed, so it seems like this is ok to handle later.
> >
> > Afaik DT data should only contain information about the hardware. This
> > is SW configuration, so I think DT people won't accept things like that.
> 
> hmm, then where *does* it go.. it is a bit too much for bootargs..

I have no idea =). And I may be wrong, but my understanding is the the
only thing DT data should have is information about the HW
configuration.

But is that kind of configuration needed at boot time? Why cannot the
userspace do the config? What configs are actually needed at boot time,
before userspace takes control? The only thing coming to my mind is to
define the display used for the console.

> >> > And how about the size of the contiguous memory area, it was left a bit
> >> > unclear to me why it cannot be dynamic.
> >>
> >> I don't think there is anything preventing adding a bootarg, but I
> >> think it is not essential so ok to add later
> >
> > Well, maybe not essential to you =). But you are reserving 32MB memory,
> > which is quite a big amount. Even if the reserved memory can be used for
> > some other purposes, it's still a big chunk of "special" memory being
> > reserved even if the user doesn't use or have a display at all.
> 
> If you didn't have display, and were tight on memory, wouldn't you
> just disable the display device in your kernel config?

Sure, if you want to modify your kernel. But you could as well use the
same kernel binary, and just say vram=0M which disables the vram
allocation. For DRM you would _have_ to modify your kernel.

Actually, I just realized vram=0M doesn't work, as the code checks for !
= 0, and just skips the vram argument when it's 0 =). So my code sucks
also...

> > Well, it's not an issue for me either, but I just feel that doing things
> > like that without allowing the user to avoid it is a bit bad thing.
> >
> > Btw, do you know why the dma_declare_contiguous() takes a dev pointer as
> > an argument, if the memory is not private to that device? (at least I
> > understood from you that the memory can be used for other purposes).
> 
> contiguous use of the memory is private to the device.  There is also
> a global CMA pool, from which all dma_alloc_xyz() which is not
> associated w/ a per-device pool comes from.. but the advantage of a
> per-device-pool is it puts an upper limit on how much dma memory that
> device can allocate so that it cannot starve other devices.

Ah, I see. So the 32MB you reserve there is not visible as contiguous
memory to anyone else than omapdrm, but userspace can still use the 32MB
area for pages that can be moved out as needed.

But then, if it's private, it's also rather bad. If I have a kernel with
omapfb and omapdrm enabled as modules, but I never use omapdrm, the 32MB
is still always reserver and away from other contiguous memory use.

Also, I just realized that besides the memory reservation being fixed,
it's also hidden. If I enable omapdrm in the kernel config, I get no
indication that 32MB of mem is being reserved. Perhaps a Kconfig option
for it, like with OMAP VRAM, and then a boot arg which will override the
Kconfig value.

Well, as I said, it's not an issue for me and from my side it can be
improved later.

 Tomi



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-14 13:43         ` Tomi Valkeinen
@ 2012-03-14 15:06           ` Rob Clark
  2012-03-15  8:46             ` Tomi Valkeinen
  0 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-03-14 15:06 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Greg KH, linux-omap, dri-devel, patches

On Wed, Mar 14, 2012 at 8:43 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2012-03-14 at 08:16 -0500, Rob Clark wrote:
>> On Wed, Mar 14, 2012 at 8:07 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > On Wed, 2012-03-14 at 07:55 -0500, Rob Clark wrote:
>> >> On Wed, Mar 14, 2012 at 7:38 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> >> > Hi,
>> >> >
>> >> > On Tue, 2012-03-13 at 15:34 -0500, 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.
>> >> >
>> >> > What's the diff with this and the previous one?
>> >>
>> >> Moving drm.c to mach-omap2 (header could not move because
>> >> omap_reserve() is in plat-omap.. but this seems to be the same as what
>> >> is done for dspbridge).
>> >>
>> >> > I see you still have the platform data there. You didn't comment on
>> >> > that. Where is it used after the board files are gone when we use DT?
>> >>
>> >> I was kind-of thinking that there would be some DT<->data-structure
>> >> glue somewhere.. not sure if this goes in mach-omap2 or in the driver
>> >> itself, but presumably it is needed somewhere.
>> >>
>> >> It is only really special cases where some board specific device-tree
>> >> data is needed, so it seems like this is ok to handle later.
>> >
>> > Afaik DT data should only contain information about the hardware. This
>> > is SW configuration, so I think DT people won't accept things like that.
>>
>> hmm, then where *does* it go.. it is a bit too much for bootargs..
>
> I have no idea =). And I may be wrong, but my understanding is the the
> only thing DT data should have is information about the HW
> configuration.
>
> But is that kind of configuration needed at boot time? Why cannot the
> userspace do the config? What configs are actually needed at boot time,
> before userspace takes control? The only thing coming to my mind is to
> define the display used for the console.

drm builds up list of resources at startup, and attempts to light up
any connected displays at boot..  the decision about what resources to
use must be taken before userspace starts.

Anyways, if it is too controversial, maybe I just remove it.  It is
really only very special cases (possibly multi-seat setups) where you
might want to do something other than the default (which is for a
single omapdrm instance to use all dss video pipes).  It is just code
I had written previously for a certain product, and it seemed
potentially useful enough to not strip out of the upstream driver.

>> >> > And how about the size of the contiguous memory area, it was left a bit
>> >> > unclear to me why it cannot be dynamic.
>> >>
>> >> I don't think there is anything preventing adding a bootarg, but I
>> >> think it is not essential so ok to add later
>> >
>> > Well, maybe not essential to you =). But you are reserving 32MB memory,
>> > which is quite a big amount. Even if the reserved memory can be used for
>> > some other purposes, it's still a big chunk of "special" memory being
>> > reserved even if the user doesn't use or have a display at all.
>>
>> If you didn't have display, and were tight on memory, wouldn't you
>> just disable the display device in your kernel config?
>
> Sure, if you want to modify your kernel. But you could as well use the
> same kernel binary, and just say vram=0M which disables the vram
> allocation. For DRM you would _have_ to modify your kernel.
>
> Actually, I just realized vram=0M doesn't work, as the code checks for !
> = 0, and just skips the vram argument when it's 0 =). So my code sucks
> also...

well, I suppose there is always something somewhere to improve..

anyways, at this point omapdrm isn't enabled by default in
omap2plus_defconfig.. I just want to get the platform device
registration merged in some form so folks don't have to go poking
around to find some out of tree patch in order to enable it.

>> > Well, it's not an issue for me either, but I just feel that doing things
>> > like that without allowing the user to avoid it is a bit bad thing.
>> >
>> > Btw, do you know why the dma_declare_contiguous() takes a dev pointer as
>> > an argument, if the memory is not private to that device? (at least I
>> > understood from you that the memory can be used for other purposes).
>>
>> contiguous use of the memory is private to the device.  There is also
>> a global CMA pool, from which all dma_alloc_xyz() which is not
>> associated w/ a per-device pool comes from.. but the advantage of a
>> per-device-pool is it puts an upper limit on how much dma memory that
>> device can allocate so that it cannot starve other devices.
>
> Ah, I see. So the 32MB you reserve there is not visible as contiguous
> memory to anyone else than omapdrm, but userspace can still use the 32MB
> area for pages that can be moved out as needed.
>
> But then, if it's private, it's also rather bad. If I have a kernel with
> omapfb and omapdrm enabled as modules, but I never use omapdrm, the 32MB
> is still always reserver and away from other contiguous memory use.
>
> Also, I just realized that besides the memory reservation being fixed,
> it's also hidden. If I enable omapdrm in the kernel config, I get no
> indication that 32MB of mem is being reserved. Perhaps a Kconfig option
> for it, like with OMAP VRAM, and then a boot arg which will override the
> Kconfig value.

well, there is a useful CMA debug option which will print some traces
when CMA pools are created, etc.. (but probably not what you meant)

> Well, as I said, it's not an issue for me and from my side it can be
> improved later.

yeah, when CMA is actually merged, there are a few other things I'd
like to do to, incl converting omapfb over to use CMA and remove
omap_vram.. but I guess those will be other patches.

BR,
-R

>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-14 15:06           ` Rob Clark
@ 2012-03-15  8:46             ` Tomi Valkeinen
  2012-03-15 12:32               ` Rob Clark
  0 siblings, 1 reply; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-15  8:46 UTC (permalink / raw)
  To: Rob Clark; +Cc: Greg KH, linux-omap, dri-devel, patches

[-- Attachment #1: Type: text/plain, Size: 621 bytes --]

On Wed, 2012-03-14 at 10:06 -0500, Rob Clark wrote:

> > Well, as I said, it's not an issue for me and from my side it can be
> > improved later.
> 
> yeah, when CMA is actually merged, there are a few other things I'd
> like to do to, incl converting omapfb over to use CMA and remove
> omap_vram.. but I guess those will be other patches.

Right, I just realized CMA is not in the kernel, nor does it seem to be
in the linux-next. Is there a reason why you want it already merged?
Wouldn't it be easier to get it in only when it can actually be used.
Especially if there's room for improvement.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-15  8:46             ` Tomi Valkeinen
@ 2012-03-15 12:32               ` Rob Clark
  2012-03-16 11:03                 ` Tomi Valkeinen
  0 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-03-15 12:32 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Greg KH, linux-omap, dri-devel, patches

On Thu, Mar 15, 2012 at 3:46 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2012-03-14 at 10:06 -0500, Rob Clark wrote:
>
>> > Well, as I said, it's not an issue for me and from my side it can be
>> > improved later.
>>
>> yeah, when CMA is actually merged, there are a few other things I'd
>> like to do to, incl converting omapfb over to use CMA and remove
>> omap_vram.. but I guess those will be other patches.
>
> Right, I just realized CMA is not in the kernel, nor does it seem to be
> in the linux-next. Is there a reason why you want it already merged?
> Wouldn't it be easier to get it in only when it can actually be used.
> Especially if there's room for improvement.

Some folks are already pulling CMA into product kernels for various
reasons.. keeping this w/ #ifdef CONFIG_CMA guards seemed like it
would make their life a bit easier.

But if people feel strongly about it, I can strip that out.

BR,
-R


>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-15 12:32               ` Rob Clark
@ 2012-03-16 11:03                 ` Tomi Valkeinen
  0 siblings, 0 replies; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-16 11:03 UTC (permalink / raw)
  To: Rob Clark; +Cc: Greg KH, linux-omap, dri-devel, patches


[-- Attachment #1.1: Type: text/plain, Size: 1462 bytes --]

On Thu, 2012-03-15 at 07:32 -0500, Rob Clark wrote:
> On Thu, Mar 15, 2012 at 3:46 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Wed, 2012-03-14 at 10:06 -0500, Rob Clark wrote:
> >
> >> > Well, as I said, it's not an issue for me and from my side it can be
> >> > improved later.
> >>
> >> yeah, when CMA is actually merged, there are a few other things I'd
> >> like to do to, incl converting omapfb over to use CMA and remove
> >> omap_vram.. but I guess those will be other patches.
> >
> > Right, I just realized CMA is not in the kernel, nor does it seem to be
> > in the linux-next. Is there a reason why you want it already merged?
> > Wouldn't it be easier to get it in only when it can actually be used.
> > Especially if there's room for improvement.
> 
> Some folks are already pulling CMA into product kernels for various
> reasons.. keeping this w/ #ifdef CONFIG_CMA guards seemed like it
> would make their life a bit easier.
> 
> But if people feel strongly about it, I can strip that out.

Well, I wouldn't say "feel strongly", but... I think the mainline kernel
should have code only for the mainline kernel, not for some custom
kernels. And the code is not testable in any way, not even compilable. I
think all code going in should be tested and compiled. Also, if the CMA
code is not in, who says it won't change. Perhaps the CMA function won't
even exist in the version going into mainline.

 Tomi


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-06-19 21:12   ` Gross, Andy
@ 2012-07-03  7:09     ` Tony Lindgren
  0 siblings, 0 replies; 52+ messages in thread
From: Tony Lindgren @ 2012-07-03  7:09 UTC (permalink / raw)
  To: Gross, Andy; +Cc: Rob Clark, dri-devel, linux-omap, greg, tomi.valkeinen

* Gross, Andy <andy.gross@ti.com> [120619 14:17]:
> Tony,
> 
> Please queue this patch at your earliest convenience.
> 
> We had some discussion on the splitting out of the DMM/Tiler driver
> from the omapdrm driver.  There might be some interest in leveraging
> the Tiler for omapfb.  However, we agreed this can be deferred until
> some other device (omapfb or otherwise) needs to use the Tiler.

OK I'll apply this into devel-board as that's seems to fit the platform
data area.

Regards,

Tony

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-06-11 15:51 ` Rob Clark
@ 2012-06-19 21:12   ` Gross, Andy
  2012-07-03  7:09     ` Tony Lindgren
  0 siblings, 1 reply; 52+ messages in thread
From: Gross, Andy @ 2012-06-19 21:12 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, linux-omap, greg, tomi.valkeinen, tony

Tony,

Please queue this patch at your earliest convenience.

We had some discussion on the splitting out of the DMM/Tiler driver
from the omapdrm driver.  There might be some interest in leveraging
the Tiler for omapfb.  However, we agreed this can be deferred until
some other device (omapfb or otherwise) needs to use the Tiler.

Regards,

Andy Gross


On Mon, Jun 11, 2012 at 10:51 AM, Rob Clark <rob.clark@linaro.org> wrote:
>
> On Wed, May 23, 2012 at 3:08 PM, Andy Gross <andy.gross@ti.com> wrote:
> > Register OMAP DRM/KMS platform device.  DMM is split into a
> > separate device using hwmod.
> >
> > Signed-off-by: Andy Gross <andy.gross@ti.com>
>
> Signed-off-by: Rob Clark <rob.clark@linaro.org>
>
> > ---
> >  arch/arm/mach-omap2/Makefile           |    4 ++
> >  arch/arm/mach-omap2/drm.c              |   61 ++++++++++++++++++++++++++++++++
> >  drivers/staging/omapdrm/omap_drv.h     |    2 +-
> >  drivers/staging/omapdrm/omap_priv.h    |   55 ----------------------------
> >  include/linux/platform_data/omap_drm.h |   52 +++++++++++++++++++++++++++
> >  5 files changed, 118 insertions(+), 56 deletions(-)
> >  create mode 100644 arch/arm/mach-omap2/drm.c
> >  delete mode 100644 drivers/staging/omapdrm/omap_priv.h
> >  create mode 100644 include/linux/platform_data/omap_drm.h
> >
> > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> > index 49f92bc..c301ab7 100644
> > --- a/arch/arm/mach-omap2/Makefile
> > +++ b/arch/arm/mach-omap2/Makefile
> > @@ -187,6 +187,10 @@ ifneq ($(CONFIG_TIDSPBRIDGE),)
> >  obj-y                                  += dsp.o
> >  endif
> >
> > +ifneq ($(CONFIG_DRM_OMAP),)
> > +obj-y                                  += drm.o
> > +endif
> > +
> >  # Specific board support
> >  obj-$(CONFIG_MACH_OMAP_GENERIC)                += board-generic.o
> >  obj-$(CONFIG_MACH_OMAP_H4)             += board-h4.o
> > diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c
> > new file mode 100644
> > index 0000000..72e0f01b
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/drm.c
> > @@ -0,0 +1,61 @@
> > +/*
> > + * 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>
> > +
> > +#include <plat/omap_device.h>
> > +#include <plat/omap_hwmod.h>
> > +
> > +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
> > +
> > +static struct platform_device omap_drm_device = {
> > +       .dev = {
> > +               .coherent_dma_mask = DMA_BIT_MASK(32),
> > +       },
> > +       .name = "omapdrm",
> > +       .id = 0,
> > +};
> > +
> > +static int __init omap_init_drm(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_drm);
> > +
> > +#endif
> > diff --git a/drivers/staging/omapdrm/omap_drv.h b/drivers/staging/omapdrm/omap_drv.h
> > index b7e0f07..96296e0 100644
> > --- a/drivers/staging/omapdrm/omap_drv.h
> > +++ b/drivers/staging/omapdrm/omap_drv.h
> > @@ -25,8 +25,8 @@
> >  #include <linux/types.h>
> >  #include <drm/drmP.h>
> >  #include <drm/drm_crtc_helper.h>
> > +#include <linux/platform_data/omap_drm.h>
> >  #include "omap_drm.h"
> > -#include "omap_priv.h"
> >
> >  #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
> >  #define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug */
> > diff --git a/drivers/staging/omapdrm/omap_priv.h b/drivers/staging/omapdrm/omap_priv.h
> > deleted file mode 100644
> > index ef64414..0000000
> > --- a/drivers/staging/omapdrm/omap_priv.h
> > +++ /dev/null
> > @@ -1,55 +0,0 @@
> > -/*
> > - * include/drm/omap_priv.h
> > - *
> > - * Copyright (C) 2011 Texas Instruments
> > - * Author: Rob Clark <rob@ti.com>
> > - *
> > - * 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 __OMAP_PRIV_H__
> > -#define __OMAP_PRIV_H__
> > -
> > -/* Non-userspace facing APIs
> > - */
> > -
> > -/* 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;
> > -       struct omap_dmm_platform_data *dmm_pdata;
> > -};
> > -
> > -#endif /* __OMAP_DRM_H__ */
> > diff --git a/include/linux/platform_data/omap_drm.h b/include/linux/platform_data/omap_drm.h
> > new file mode 100644
> > index 0000000..3da73bd
> > --- /dev/null
> > +++ b/include/linux/platform_data/omap_drm.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * DRM/KMS platform data 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 __PLATFORM_DATA_OMAP_DRM_H__
> > +#define __PLATFORM_DATA_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;
> > +};
> > +
> > +#endif /* __PLATFORM_DATA_OMAP_DRM_H__ */
> > --
> > 1.7.5.4
> >
> > --
> > 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
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-23 20:08 Andy Gross
  2012-05-24  6:01 ` Tomi Valkeinen
@ 2012-06-11 15:51 ` Rob Clark
  2012-06-19 21:12   ` Gross, Andy
  1 sibling, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-06-11 15:51 UTC (permalink / raw)
  To: Andy Gross; +Cc: dri-devel, linux-omap, greg, tomi.valkeinen

On Wed, May 23, 2012 at 3:08 PM, Andy Gross <andy.gross@ti.com> wrote:
> Register OMAP DRM/KMS platform device.  DMM is split into a
> separate device using hwmod.
>
> Signed-off-by: Andy Gross <andy.gross@ti.com>

Signed-off-by: Rob Clark <rob.clark@linaro.org>

> ---
>  arch/arm/mach-omap2/Makefile           |    4 ++
>  arch/arm/mach-omap2/drm.c              |   61 ++++++++++++++++++++++++++++++++
>  drivers/staging/omapdrm/omap_drv.h     |    2 +-
>  drivers/staging/omapdrm/omap_priv.h    |   55 ----------------------------
>  include/linux/platform_data/omap_drm.h |   52 +++++++++++++++++++++++++++
>  5 files changed, 118 insertions(+), 56 deletions(-)
>  create mode 100644 arch/arm/mach-omap2/drm.c
>  delete mode 100644 drivers/staging/omapdrm/omap_priv.h
>  create mode 100644 include/linux/platform_data/omap_drm.h
>
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> index 49f92bc..c301ab7 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -187,6 +187,10 @@ ifneq ($(CONFIG_TIDSPBRIDGE),)
>  obj-y                                  += dsp.o
>  endif
>
> +ifneq ($(CONFIG_DRM_OMAP),)
> +obj-y                                  += drm.o
> +endif
> +
>  # Specific board support
>  obj-$(CONFIG_MACH_OMAP_GENERIC)                += board-generic.o
>  obj-$(CONFIG_MACH_OMAP_H4)             += board-h4.o
> diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c
> new file mode 100644
> index 0000000..72e0f01b
> --- /dev/null
> +++ b/arch/arm/mach-omap2/drm.c
> @@ -0,0 +1,61 @@
> +/*
> + * 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>
> +
> +#include <plat/omap_device.h>
> +#include <plat/omap_hwmod.h>
> +
> +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
> +
> +static struct platform_device omap_drm_device = {
> +       .dev = {
> +               .coherent_dma_mask = DMA_BIT_MASK(32),
> +       },
> +       .name = "omapdrm",
> +       .id = 0,
> +};
> +
> +static int __init omap_init_drm(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_drm);
> +
> +#endif
> diff --git a/drivers/staging/omapdrm/omap_drv.h b/drivers/staging/omapdrm/omap_drv.h
> index b7e0f07..96296e0 100644
> --- a/drivers/staging/omapdrm/omap_drv.h
> +++ b/drivers/staging/omapdrm/omap_drv.h
> @@ -25,8 +25,8 @@
>  #include <linux/types.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <linux/platform_data/omap_drm.h>
>  #include "omap_drm.h"
> -#include "omap_priv.h"
>
>  #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
>  #define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug */
> diff --git a/drivers/staging/omapdrm/omap_priv.h b/drivers/staging/omapdrm/omap_priv.h
> deleted file mode 100644
> index ef64414..0000000
> --- a/drivers/staging/omapdrm/omap_priv.h
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -/*
> - * include/drm/omap_priv.h
> - *
> - * Copyright (C) 2011 Texas Instruments
> - * Author: Rob Clark <rob@ti.com>
> - *
> - * 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 __OMAP_PRIV_H__
> -#define __OMAP_PRIV_H__
> -
> -/* Non-userspace facing APIs
> - */
> -
> -/* 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;
> -       struct omap_dmm_platform_data *dmm_pdata;
> -};
> -
> -#endif /* __OMAP_DRM_H__ */
> diff --git a/include/linux/platform_data/omap_drm.h b/include/linux/platform_data/omap_drm.h
> new file mode 100644
> index 0000000..3da73bd
> --- /dev/null
> +++ b/include/linux/platform_data/omap_drm.h
> @@ -0,0 +1,52 @@
> +/*
> + * DRM/KMS platform data 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 __PLATFORM_DATA_OMAP_DRM_H__
> +#define __PLATFORM_DATA_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;
> +};
> +
> +#endif /* __PLATFORM_DATA_OMAP_DRM_H__ */
> --
> 1.7.5.4
>
> --
> 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
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-06-11 14:51                 ` Gross, Andy
  2012-06-11 14:54                   ` Gross, Andy
@ 2012-06-11 15:05                   ` Tomi Valkeinen
  1 sibling, 0 replies; 52+ messages in thread
From: Tomi Valkeinen @ 2012-06-11 15:05 UTC (permalink / raw)
  To: Gross, Andy; +Cc: greg, linux-omap, dri-devel, Rob Clark


[-- Attachment #1.1: Type: text/plain, Size: 334 bytes --]

On Mon, 2012-06-11 at 09:51 -0500, Gross, Andy wrote:
> Tomi,
> 
> 
> So at this point, are you OK with deferring a split of the DMM until
> it necessary to do so (if ever)?  I'd like to get this patch in so
> that people have a working omapdrm device when they enable the config
> options.

Yes, I'm ok with it.

 Tomi


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-06-11 14:51                 ` Gross, Andy
@ 2012-06-11 14:54                   ` Gross, Andy
  2012-06-11 15:05                   ` Tomi Valkeinen
  1 sibling, 0 replies; 52+ messages in thread
From: Gross, Andy @ 2012-06-11 14:54 UTC (permalink / raw)
  To: linux-omap

Tomi,

So at this point, are you OK with deferring a split of the DMM until it
necessary to do so (if ever)?  I'd like to get this patch in so that people
have a working omapdrm device when they enable the config options.

Regards,

Andy
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24 15:26               ` Tomi Valkeinen
@ 2012-06-11 14:51                 ` Gross, Andy
  2012-06-11 14:54                   ` Gross, Andy
  2012-06-11 15:05                   ` Tomi Valkeinen
  0 siblings, 2 replies; 52+ messages in thread
From: Gross, Andy @ 2012-06-11 14:51 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: greg, linux-omap, dri-devel, Rob Clark


[-- Attachment #1.1: Type: text/plain, Size: 238 bytes --]

Tomi,

So at this point, are you OK with deferring a split of the DMM until it
necessary to do so (if ever)?  I'd like to get this patch in so that people
have a working omapdrm device when they enable the config options.

Regards,

Andy

[-- Attachment #1.2: Type: text/html, Size: 321 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24 15:09             ` Gross, Andy
@ 2012-05-24 15:26               ` Tomi Valkeinen
  2012-06-11 14:51                 ` Gross, Andy
  0 siblings, 1 reply; 52+ messages in thread
From: Tomi Valkeinen @ 2012-05-24 15:26 UTC (permalink / raw)
  To: Gross, Andy; +Cc: Rob Clark, greg, linux-omap, dri-devel

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

On Thu, 2012-05-24 at 10:09 -0500, Gross, Andy wrote:
> On Thu, May 24, 2012 at 7:13 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Thu, 2012-05-24 at 02:44 -0600, Rob Clark wrote:
> >
> >> but other drivers *can* use tiler, thanks to dmabuf.. I have omap4iss
> >> v4l2 camera working w/ tiler buffers on my pandaboard, for example.
> >>
> >> Maybe fbdev is an exception to the rule because it has no way for
> >> userspace to pass it a buffer to use.  But on the other hand it is a
> >> legacy API so I'm not sure if it is worth loosing too much sleep over
> >> that.
> >
> > I'm not that familiar with dmabuf, but are you saying it's not possible
> > for a kernel driver to request the buffers? They _must_ come from the
> > userspace?
> >
> > Anyway, even if it would be possible, if the tiler is a part of omapdrm
> > we need omapdrm to get and use the tiler buffers. And we can't have
> > omapdrm running when using omapfb, because they both use omapdss.
> 
> And that is a good point.  The DSS is kind of a sticking point to anyone
> having to enable DRM to get Tiler.  However, omapfb doesn't currently utilize
> DMM/Tiler features.  Can't we defer generalizing until there is a
> requirement for it?

Sure. I just brought it up because it'd be nice and it'd be better
architecturally. However, as I said, I'm not familiar with the related
problems, so I take your word that it's not simple =).

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24 12:13           ` Tomi Valkeinen
@ 2012-05-24 15:09             ` Gross, Andy
  2012-05-24 15:26               ` Tomi Valkeinen
  0 siblings, 1 reply; 52+ messages in thread
From: Gross, Andy @ 2012-05-24 15:09 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: greg, linux-omap, dri-devel, Rob Clark

On Thu, May 24, 2012 at 7:13 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-05-24 at 02:44 -0600, Rob Clark wrote:
>
>> but other drivers *can* use tiler, thanks to dmabuf.. I have omap4iss
>> v4l2 camera working w/ tiler buffers on my pandaboard, for example.
>>
>> Maybe fbdev is an exception to the rule because it has no way for
>> userspace to pass it a buffer to use.  But on the other hand it is a
>> legacy API so I'm not sure if it is worth loosing too much sleep over
>> that.
>
> I'm not that familiar with dmabuf, but are you saying it's not possible
> for a kernel driver to request the buffers? They _must_ come from the
> userspace?
>
> Anyway, even if it would be possible, if the tiler is a part of omapdrm
> we need omapdrm to get and use the tiler buffers. And we can't have
> omapdrm running when using omapfb, because they both use omapdss.

And that is a good point.  The DSS is kind of a sticking point to anyone
having to enable DRM to get Tiler.  However, omapfb doesn't currently utilize
DMM/Tiler features.  Can't we defer generalizing until there is a
requirement for it?

Andy

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24  6:01 ` Tomi Valkeinen
  2012-05-24  6:27   ` Clark, Rob
@ 2012-05-24 14:22   ` Gross, Andy
  1 sibling, 0 replies; 52+ messages in thread
From: Gross, Andy @ 2012-05-24 14:22 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: dri-devel, linux-omap, greg, rob

On Thu, May 24, 2012 at 1:01 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> +struct omap_drm_platform_data {
>> +     struct omap_kms_platform_data *kms_pdata;
>> +};
>
> This one is missing struct omap_dmm_platform_data *dmm_pdata, so you
> didn't just move the struct. Is that on purpose?

Good point.  I can clean that up.


Andy
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24  8:44         ` Rob Clark
@ 2012-05-24 12:13           ` Tomi Valkeinen
  2012-05-24 15:09             ` Gross, Andy
  0 siblings, 1 reply; 52+ messages in thread
From: Tomi Valkeinen @ 2012-05-24 12:13 UTC (permalink / raw)
  To: Rob Clark; +Cc: greg, linux-omap, dri-devel

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On Thu, 2012-05-24 at 02:44 -0600, Rob Clark wrote:

> but other drivers *can* use tiler, thanks to dmabuf.. I have omap4iss
> v4l2 camera working w/ tiler buffers on my pandaboard, for example.
> 
> Maybe fbdev is an exception to the rule because it has no way for
> userspace to pass it a buffer to use.  But on the other hand it is a
> legacy API so I'm not sure if it is worth loosing too much sleep over
> that.

I'm not that familiar with dmabuf, but are you saying it's not possible
for a kernel driver to request the buffers? They _must_ come from the
userspace?

Anyway, even if it would be possible, if the tiler is a part of omapdrm
we need omapdrm to get and use the tiler buffers. And we can't have
omapdrm running when using omapfb, because they both use omapdss.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24  8:35       ` Rob Clark
@ 2012-05-24 12:10         ` Tomi Valkeinen
  0 siblings, 0 replies; 52+ messages in thread
From: Tomi Valkeinen @ 2012-05-24 12:10 UTC (permalink / raw)
  To: Rob Clark; +Cc: greg, linux-omap, dri-devel

[-- Attachment #1: Type: text/plain, Size: 5178 bytes --]

On Thu, 2012-05-24 at 02:35 -0600, Rob Clark wrote:
> On Thu, May 24, 2012 at 1:05 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
> >> On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> > Hi,
> >> >
> >> > On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
> >> >> Register OMAP DRM/KMS platform device.  DMM is split into a
> >> >> separate device using hwmod.
> >> >>
> >> >> Signed-off-by: Andy Gross <andy.gross@ti.com>
> >> >
> >> > <snip>
> >> >
> >> >> +static int __init omap_init_drm(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);
> >> >> +
> >> >> +}
> >> >
> >> > I still don't like fixing the tiler to drm. I would like to have basic
> >> > tiler support in omapfb also, but with this approach I'll need to
> >> > duplicate the code. And even if we disregard omapfb, wouldn't it be
> >> > architecturally better to have the tiler as a separate independent
> >> > library/driver?
> >>
> >> Not easily, at least not if we want to manage to use tiler/dmm in a
> >> more dynamic way, or to enable some additional features which are
> >> still on the roadmap (like reprogramming dmm synchronized w/ scanout,
> >> or some things which are coming if future hw generations).  We need
> >> one place to keep track of which buffers are potentially evictable to
> >> make room for mapping a new buffer.  And if you look at the tricks
> >> that go on with mmap'ing tiled buffers to userspace, you *really*
> >> don't want to duplicate that in N different drivers.
> >
> > So why can't all that code be in a tiler library/driver?
> 
> Possibly the placement stuff could be in a library..  the
> mmap/faulting stuff I think would be harder to split.  With it split
> out in a separate lib, it becomes logistically a bit more of a
> headache to change APIs, etc.  Basically it just makes more work and
> is unnecessary.

Unnecessary for you, but maybe not for those who want to use omapfb.

> >> Fortunately with dmabuf there is not really a need for N different
> >> drivers to need to use tiler/dmm directly.  The dmabuf mechanism
> >> provides what they need to import GEM buffers from omapdrm.  That may
> >> not really help omapfb because fbdev doesn't have a concept of
> >> importing buffers.  But OTOH this is unnecessary, because drm provides
> >> an fbdev interface for legacy apps.  The best thing I'd recommend is,
> >> if you miss some features of omapfb in the drm fbdev implementation,
> >> is to send some patches to add this missing features.
> >
> > Well, at least currently omapfb and omapdrm work quite differently, if
> > I've understood right. Can we make a full omapfb layer on top of
> > omapdrm? With multiple framebuffers mapped to one or more overlays,
> > support for all the ioctls, etc?
> 
> Well, there is still room to add your own fb_ioctl() fxn, so I guess
> in principle it should be possible to add whatever custom ioctls are
> required.
> 
> Typically you have a single fbdev device for a single drm device,
> although I suppose nothing prevents creating more.  I'd probably want
> to encourage users more towards using KMS directly for multi-display
> cases because you have a lot more options/flexibility that way.

Sure, but we can't force people to use omapdrm instead of omapfb. And
omapfb is not going to disappear. So obviously we should recommend using
omapdrm, but on the other hand, I don't see any problem in adding new
features to omapfb if they are easily implemented (using, for example, a
separate tiler driver).

> > I guess we'd still need to have omapfb driver to keep the module
> > parameters and behavior the same. Can omapdrm be used from inside the
> > kernel by another driver?
> 
> Hmm, I'm not quite sure what you have in mind, but it sounds a bit
> hacky..  I'd guess if you need 100% backwards compatibility even down
> to kernel cmdline / module params, then you probably want to use
> omapfb.  But there isn't really need to add new features to omapfb in
> that case.

I was thinking of making omapfb use omapdrm, instead of omapdss. I mean,
not planning to do that, just wondering if that would be possible.

> Off the top of my head, I guess that 80-90% compatibility would
> probably be reasonable to add to omapdrm's fbdev..  and that the last
> 10-20% would be too hacky/invasive to justify adding to omapdrm.

I think it should be 99.9% - 100% or nothing. If it's only 80-90%
compatible, then it's not compatible =).

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24  7:21       ` Tomi Valkeinen
@ 2012-05-24  8:44         ` Rob Clark
  2012-05-24 12:13           ` Tomi Valkeinen
  0 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-05-24  8:44 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: greg, linux-omap, dri-devel

On Thu, May 24, 2012 at 1:21 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-05-24 at 10:05 +0300, Tomi Valkeinen wrote:
>> On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
>> > On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > > Hi,
>> > >
>> > > On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
>> > >> Register OMAP DRM/KMS platform device.  DMM is split into a
>> > >> separate device using hwmod.
>> > >>
>> > >> Signed-off-by: Andy Gross <andy.gross@ti.com>
>> > >
>> > > <snip>
>> > >
>> > >> +static int __init omap_init_drm(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);
>> > >> +
>> > >> +}
>> > >
>> > > I still don't like fixing the tiler to drm. I would like to have basic
>> > > tiler support in omapfb also, but with this approach I'll need to
>> > > duplicate the code. And even if we disregard omapfb, wouldn't it be
>> > > architecturally better to have the tiler as a separate independent
>> > > library/driver?
>> >
>> > Not easily, at least not if we want to manage to use tiler/dmm in a
>> > more dynamic way, or to enable some additional features which are
>> > still on the roadmap (like reprogramming dmm synchronized w/ scanout,
>> > or some things which are coming if future hw generations).  We need
>> > one place to keep track of which buffers are potentially evictable to
>> > make room for mapping a new buffer.  And if you look at the tricks
>> > that go on with mmap'ing tiled buffers to userspace, you *really*
>> > don't want to duplicate that in N different drivers.
>>
>> So why can't all that code be in a tiler library/driver?
>
> And I think we've discussed about this before, so sorry if I'm repeating
> myself. I just find it odd that we are not able to create a nice
> separate lib/driver for the tiler, which is a separate piece of HW that
> multiple drivers might want to use.

but other drivers *can* use tiler, thanks to dmabuf.. I have omap4iss
v4l2 camera working w/ tiler buffers on my pandaboard, for example.

Maybe fbdev is an exception to the rule because it has no way for
userspace to pass it a buffer to use.  But on the other hand it is a
legacy API so I'm not sure if it is worth loosing too much sleep over
that.

BR,
-R

>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24  7:05     ` Tomi Valkeinen
  2012-05-24  7:21       ` Tomi Valkeinen
@ 2012-05-24  8:35       ` Rob Clark
  2012-05-24 12:10         ` Tomi Valkeinen
  1 sibling, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-05-24  8:35 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: greg, linux-omap, dri-devel

On Thu, May 24, 2012 at 1:05 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
>> On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > Hi,
>> >
>> > On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
>> >> Register OMAP DRM/KMS platform device.  DMM is split into a
>> >> separate device using hwmod.
>> >>
>> >> Signed-off-by: Andy Gross <andy.gross@ti.com>
>> >
>> > <snip>
>> >
>> >> +static int __init omap_init_drm(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);
>> >> +
>> >> +}
>> >
>> > I still don't like fixing the tiler to drm. I would like to have basic
>> > tiler support in omapfb also, but with this approach I'll need to
>> > duplicate the code. And even if we disregard omapfb, wouldn't it be
>> > architecturally better to have the tiler as a separate independent
>> > library/driver?
>>
>> Not easily, at least not if we want to manage to use tiler/dmm in a
>> more dynamic way, or to enable some additional features which are
>> still on the roadmap (like reprogramming dmm synchronized w/ scanout,
>> or some things which are coming if future hw generations).  We need
>> one place to keep track of which buffers are potentially evictable to
>> make room for mapping a new buffer.  And if you look at the tricks
>> that go on with mmap'ing tiled buffers to userspace, you *really*
>> don't want to duplicate that in N different drivers.
>
> So why can't all that code be in a tiler library/driver?

Possibly the placement stuff could be in a library..  the
mmap/faulting stuff I think would be harder to split.  With it split
out in a separate lib, it becomes logistically a bit more of a
headache to change APIs, etc.  Basically it just makes more work and
is unnecessary.

>> Fortunately with dmabuf there is not really a need for N different
>> drivers to need to use tiler/dmm directly.  The dmabuf mechanism
>> provides what they need to import GEM buffers from omapdrm.  That may
>> not really help omapfb because fbdev doesn't have a concept of
>> importing buffers.  But OTOH this is unnecessary, because drm provides
>> an fbdev interface for legacy apps.  The best thing I'd recommend is,
>> if you miss some features of omapfb in the drm fbdev implementation,
>> is to send some patches to add this missing features.
>
> Well, at least currently omapfb and omapdrm work quite differently, if
> I've understood right. Can we make a full omapfb layer on top of
> omapdrm? With multiple framebuffers mapped to one or more overlays,
> support for all the ioctls, etc?

Well, there is still room to add your own fb_ioctl() fxn, so I guess
in principle it should be possible to add whatever custom ioctls are
required.

Typically you have a single fbdev device for a single drm device,
although I suppose nothing prevents creating more.  I'd probably want
to encourage users more towards using KMS directly for multi-display
cases because you have a lot more options/flexibility that way.

> I guess we'd still need to have omapfb driver to keep the module
> parameters and behavior the same. Can omapdrm be used from inside the
> kernel by another driver?

Hmm, I'm not quite sure what you have in mind, but it sounds a bit
hacky..  I'd guess if you need 100% backwards compatibility even down
to kernel cmdline / module params, then you probably want to use
omapfb.  But there isn't really need to add new features to omapfb in
that case.

Off the top of my head, I guess that 80-90% compatibility would
probably be reasonable to add to omapdrm's fbdev..  and that the last
10-20% would be too hacky/invasive to justify adding to omapdrm.

BR,
-R

>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24  7:05     ` Tomi Valkeinen
@ 2012-05-24  7:21       ` Tomi Valkeinen
  2012-05-24  8:44         ` Rob Clark
  2012-05-24  8:35       ` Rob Clark
  1 sibling, 1 reply; 52+ messages in thread
From: Tomi Valkeinen @ 2012-05-24  7:21 UTC (permalink / raw)
  To: Clark, Rob; +Cc: greg, linux-omap, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2390 bytes --]

On Thu, 2012-05-24 at 10:05 +0300, Tomi Valkeinen wrote:
> On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
> > On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > > Hi,
> > >
> > > On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
> > >> Register OMAP DRM/KMS platform device.  DMM is split into a
> > >> separate device using hwmod.
> > >>
> > >> Signed-off-by: Andy Gross <andy.gross@ti.com>
> > >
> > > <snip>
> > >
> > >> +static int __init omap_init_drm(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);
> > >> +
> > >> +}
> > >
> > > I still don't like fixing the tiler to drm. I would like to have basic
> > > tiler support in omapfb also, but with this approach I'll need to
> > > duplicate the code. And even if we disregard omapfb, wouldn't it be
> > > architecturally better to have the tiler as a separate independent
> > > library/driver?
> > 
> > Not easily, at least not if we want to manage to use tiler/dmm in a
> > more dynamic way, or to enable some additional features which are
> > still on the roadmap (like reprogramming dmm synchronized w/ scanout,
> > or some things which are coming if future hw generations).  We need
> > one place to keep track of which buffers are potentially evictable to
> > make room for mapping a new buffer.  And if you look at the tricks
> > that go on with mmap'ing tiled buffers to userspace, you *really*
> > don't want to duplicate that in N different drivers.
> 
> So why can't all that code be in a tiler library/driver?

And I think we've discussed about this before, so sorry if I'm repeating
myself. I just find it odd that we are not able to create a nice
separate lib/driver for the tiler, which is a separate piece of HW that
multiple drivers might want to use.

 Tomi


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24  6:27   ` Clark, Rob
@ 2012-05-24  7:05     ` Tomi Valkeinen
  2012-05-24  7:21       ` Tomi Valkeinen
  2012-05-24  8:35       ` Rob Clark
  0 siblings, 2 replies; 52+ messages in thread
From: Tomi Valkeinen @ 2012-05-24  7:05 UTC (permalink / raw)
  To: Clark, Rob; +Cc: Andy Gross, dri-devel, linux-omap, greg

[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]

On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
> On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > Hi,
> >
> > On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
> >> Register OMAP DRM/KMS platform device.  DMM is split into a
> >> separate device using hwmod.
> >>
> >> Signed-off-by: Andy Gross <andy.gross@ti.com>
> >
> > <snip>
> >
> >> +static int __init omap_init_drm(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);
> >> +
> >> +}
> >
> > I still don't like fixing the tiler to drm. I would like to have basic
> > tiler support in omapfb also, but with this approach I'll need to
> > duplicate the code. And even if we disregard omapfb, wouldn't it be
> > architecturally better to have the tiler as a separate independent
> > library/driver?
> 
> Not easily, at least not if we want to manage to use tiler/dmm in a
> more dynamic way, or to enable some additional features which are
> still on the roadmap (like reprogramming dmm synchronized w/ scanout,
> or some things which are coming if future hw generations).  We need
> one place to keep track of which buffers are potentially evictable to
> make room for mapping a new buffer.  And if you look at the tricks
> that go on with mmap'ing tiled buffers to userspace, you *really*
> don't want to duplicate that in N different drivers.

So why can't all that code be in a tiler library/driver?

> Fortunately with dmabuf there is not really a need for N different
> drivers to need to use tiler/dmm directly.  The dmabuf mechanism
> provides what they need to import GEM buffers from omapdrm.  That may
> not really help omapfb because fbdev doesn't have a concept of
> importing buffers.  But OTOH this is unnecessary, because drm provides
> an fbdev interface for legacy apps.  The best thing I'd recommend is,
> if you miss some features of omapfb in the drm fbdev implementation,
> is to send some patches to add this missing features.

Well, at least currently omapfb and omapdrm work quite differently, if
I've understood right. Can we make a full omapfb layer on top of
omapdrm? With multiple framebuffers mapped to one or more overlays,
support for all the ioctls, etc?

I guess we'd still need to have omapfb driver to keep the module
parameters and behavior the same. Can omapdrm be used from inside the
kernel by another driver?

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-24  6:01 ` Tomi Valkeinen
@ 2012-05-24  6:27   ` Clark, Rob
  2012-05-24  7:05     ` Tomi Valkeinen
  2012-05-24 14:22   ` Gross, Andy
  1 sibling, 1 reply; 52+ messages in thread
From: Clark, Rob @ 2012-05-24  6:27 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Andy Gross, dri-devel, linux-omap, greg

On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi,
>
> On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
>> Register OMAP DRM/KMS platform device.  DMM is split into a
>> separate device using hwmod.
>>
>> Signed-off-by: Andy Gross <andy.gross@ti.com>
>
> <snip>
>
>> +static int __init omap_init_drm(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);
>> +
>> +}
>
> I still don't like fixing the tiler to drm. I would like to have basic
> tiler support in omapfb also, but with this approach I'll need to
> duplicate the code. And even if we disregard omapfb, wouldn't it be
> architecturally better to have the tiler as a separate independent
> library/driver?

Not easily, at least not if we want to manage to use tiler/dmm in a
more dynamic way, or to enable some additional features which are
still on the roadmap (like reprogramming dmm synchronized w/ scanout,
or some things which are coming if future hw generations).  We need
one place to keep track of which buffers are potentially evictable to
make room for mapping a new buffer.  And if you look at the tricks
that go on with mmap'ing tiled buffers to userspace, you *really*
don't want to duplicate that in N different drivers.

Fortunately with dmabuf there is not really a need for N different
drivers to need to use tiler/dmm directly.  The dmabuf mechanism
provides what they need to import GEM buffers from omapdrm.  That may
not really help omapfb because fbdev doesn't have a concept of
importing buffers.  But OTOH this is unnecessary, because drm provides
an fbdev interface for legacy apps.  The best thing I'd recommend is,
if you miss some features of omapfb in the drm fbdev implementation,
is to send some patches to add this missing features.

>> +struct omap_drm_platform_data {
>> +     struct omap_kms_platform_data *kms_pdata;
>> +};
>
> This one is missing struct omap_dmm_platform_data *dmm_pdata, so you
> didn't just move the struct. Is that on purpose?

the dmm pdata is no longer needed because we get what we need from
hwmod via platform_get_resource()

BR,
-R

>  Tomi
>
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-05-23 20:08 Andy Gross
@ 2012-05-24  6:01 ` Tomi Valkeinen
  2012-05-24  6:27   ` Clark, Rob
  2012-05-24 14:22   ` Gross, Andy
  2012-06-11 15:51 ` Rob Clark
  1 sibling, 2 replies; 52+ messages in thread
From: Tomi Valkeinen @ 2012-05-24  6:01 UTC (permalink / raw)
  To: Andy Gross; +Cc: dri-devel, linux-omap, greg, rob

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

Hi,

On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
> Register OMAP DRM/KMS platform device.  DMM is split into a
> separate device using hwmod.
> 
> Signed-off-by: Andy Gross <andy.gross@ti.com>

<snip>

> +static int __init omap_init_drm(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);
> +
> +}

I still don't like fixing the tiler to drm. I would like to have basic
tiler support in omapfb also, but with this approach I'll need to
duplicate the code. And even if we disregard omapfb, wouldn't it be
architecturally better to have the tiler as a separate independent
library/driver?

> +struct omap_drm_platform_data {
> +	struct omap_kms_platform_data *kms_pdata;
> +};

This one is missing struct omap_dmm_platform_data *dmm_pdata, so you
didn't just move the struct. Is that on purpose?

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH] omap2+: add drm device
@ 2012-05-23 20:08 Andy Gross
  2012-05-24  6:01 ` Tomi Valkeinen
  2012-06-11 15:51 ` Rob Clark
  0 siblings, 2 replies; 52+ messages in thread
From: Andy Gross @ 2012-05-23 20:08 UTC (permalink / raw)
  To: dri-devel, linux-omap; +Cc: greg, tomi.valkeinen, rob, Andy Gross

Register OMAP DRM/KMS platform device.  DMM is split into a
separate device using hwmod.

Signed-off-by: Andy Gross <andy.gross@ti.com>
---
 arch/arm/mach-omap2/Makefile           |    4 ++
 arch/arm/mach-omap2/drm.c              |   61 ++++++++++++++++++++++++++++++++
 drivers/staging/omapdrm/omap_drv.h     |    2 +-
 drivers/staging/omapdrm/omap_priv.h    |   55 ----------------------------
 include/linux/platform_data/omap_drm.h |   52 +++++++++++++++++++++++++++
 5 files changed, 118 insertions(+), 56 deletions(-)
 create mode 100644 arch/arm/mach-omap2/drm.c
 delete mode 100644 drivers/staging/omapdrm/omap_priv.h
 create mode 100644 include/linux/platform_data/omap_drm.h

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 49f92bc..c301ab7 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -187,6 +187,10 @@ ifneq ($(CONFIG_TIDSPBRIDGE),)
 obj-y					+= dsp.o
 endif
 
+ifneq ($(CONFIG_DRM_OMAP),)
+obj-y					+= drm.o
+endif
+
 # Specific board support
 obj-$(CONFIG_MACH_OMAP_GENERIC)		+= board-generic.o
 obj-$(CONFIG_MACH_OMAP_H4)		+= board-h4.o
diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c
new file mode 100644
index 0000000..72e0f01b
--- /dev/null
+++ b/arch/arm/mach-omap2/drm.c
@@ -0,0 +1,61 @@
+/*
+ * 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>
+
+#include <plat/omap_device.h>
+#include <plat/omap_hwmod.h>
+
+#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
+
+static struct platform_device omap_drm_device = {
+	.dev = {
+		.coherent_dma_mask = DMA_BIT_MASK(32),
+	},
+	.name = "omapdrm",
+	.id = 0,
+};
+
+static int __init omap_init_drm(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_drm);
+
+#endif
diff --git a/drivers/staging/omapdrm/omap_drv.h b/drivers/staging/omapdrm/omap_drv.h
index b7e0f07..96296e0 100644
--- a/drivers/staging/omapdrm/omap_drv.h
+++ b/drivers/staging/omapdrm/omap_drv.h
@@ -25,8 +25,8 @@
 #include <linux/types.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
+#include <linux/platform_data/omap_drm.h>
 #include "omap_drm.h"
-#include "omap_priv.h"
 
 #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 #define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug */
diff --git a/drivers/staging/omapdrm/omap_priv.h b/drivers/staging/omapdrm/omap_priv.h
deleted file mode 100644
index ef64414..0000000
--- a/drivers/staging/omapdrm/omap_priv.h
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * include/drm/omap_priv.h
- *
- * Copyright (C) 2011 Texas Instruments
- * Author: Rob Clark <rob@ti.com>
- *
- * 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 __OMAP_PRIV_H__
-#define __OMAP_PRIV_H__
-
-/* Non-userspace facing APIs
- */
-
-/* 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;
-	struct omap_dmm_platform_data *dmm_pdata;
-};
-
-#endif /* __OMAP_DRM_H__ */
diff --git a/include/linux/platform_data/omap_drm.h b/include/linux/platform_data/omap_drm.h
new file mode 100644
index 0000000..3da73bd
--- /dev/null
+++ b/include/linux/platform_data/omap_drm.h
@@ -0,0 +1,52 @@
+/*
+ * DRM/KMS platform data 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 __PLATFORM_DATA_OMAP_DRM_H__
+#define __PLATFORM_DATA_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;
+};
+
+#endif /* __PLATFORM_DATA_OMAP_DRM_H__ */
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-07 15:59           ` Gross, Andy
@ 2012-03-08  7:47             ` Tomi Valkeinen
  0 siblings, 0 replies; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-08  7:47 UTC (permalink / raw)
  To: Gross, Andy; +Cc: Rob Clark, dri-devel, linux-omap, patches, Greg KH

[-- Attachment #1: Type: text/plain, Size: 1516 bytes --]

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


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-07 12:05         ` Tomi Valkeinen
  2012-03-07 13:27           ` Rob Clark
@ 2012-03-07 15:59           ` Gross, Andy
  2012-03-08  7:47             ` Tomi Valkeinen
  1 sibling, 1 reply; 52+ messages in thread
From: Gross, Andy @ 2012-03-07 15:59 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Rob Clark, dri-devel, linux-omap, patches, Greg KH

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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-07 12:05         ` Tomi Valkeinen
@ 2012-03-07 13:27           ` Rob Clark
  2012-03-07 15:59           ` Gross, Andy
  1 sibling, 0 replies; 52+ messages in thread
From: Rob Clark @ 2012-03-07 13:27 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Gross, Andy, Greg KH, linux-omap, patches, dri-devel

On Wed, Mar 7, 2012 at 6:05 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 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.

Well, "dmm" is the name in hwmod, so either way we are sort of "stuck"
with that.. but if you look in the TRM, you'd be looking for "dynamic
memory manager", so I tend to think "dmm" is the better name.  But for
some reason the old driver (in some product kernels but never made it
upstream) was called "tiler", which might be part of the reason that
people have started using the names interchangeably.

At any rate, neither "tiler" nor "dmm" appear in any userspace facing
API (instead you just have some bits you can set in the flags you
specify if you want tiled buffer or not when allocating a gem buffer
object).  So I guess it isn't as critical.

BR,
-R

>  Tomi
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-07 13:06           ` Rob Clark
@ 2012-03-07 13:11             ` Tomi Valkeinen
  0 siblings, 0 replies; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-07 13:11 UTC (permalink / raw)
  To: Rob Clark; +Cc: Greg KH, linux-omap, dri-devel, patches

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

On Wed, 2012-03-07 at 07:06 -0600, Rob Clark wrote:
> On Wed, Mar 7, 2012 at 5:59 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> > Hmm, why does the drm core care about it?
> 
> Because it is the one generating the bus-id.. see
> drivers/gpu/drm/drm_platform.c drm_platform_set_busid()
> 
> Anyways, it's just a detail about how libdrm/drmOpen() can
> differentiate between multiple instances of the same driver, similar
> to how PCI bus-id is used in the desktop world.  It is not difficult
> to change in drm_platform_set_busid(), although not sure if that would
> be considered an ABI change to userspace.  (Maybe it is less critical,
> I'm under the impression that other platform-drm users didn't even
> realize we had a bus-id.)

Ok. Well, I'm fine with id 0 also, if it makes sense in the DRM side. It
was just something that caught my eye.

> > Okay, let me ask the other way. Is 32MB enough for everyone? Hardcoding
> > a value like that without any possibility to adjust it just sounds like
> > a rather bad thing.
> 
> The main requirement is that, on omap3 or before (platforms without
> DMM) you need enough to allocate all of your scanout buffers.
> 
> I'm not against having a bootarg to adjust, although I strongly prefer
> sane defaults and not requiring a million bootargs just to boot in
> some usable fashion.

Well, those are two different things. I'm fine with a default of 32MB.
My point was, what if I need more, or I don't need any.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-07 11:59         ` Tomi Valkeinen
@ 2012-03-07 13:06           ` Rob Clark
  2012-03-07 13:11             ` Tomi Valkeinen
  0 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-03-07 13:06 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Greg KH, linux-omap, dri-devel, patches

On Wed, Mar 7, 2012 at 5:59 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Tue, 2012-03-06 at 09:29 -0600, Rob Clark wrote:
>> On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > 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.
>>
>> well, I don't care much one way or another, but need to check if there
>> is a small patch needed in drm core code that generates the bus-id for
>> .id = -1..
>
> Hmm, why does the drm core care about it?

Because it is the one generating the bus-id.. see
drivers/gpu/drm/drm_platform.c drm_platform_set_busid()

Anyways, it's just a detail about how libdrm/drmOpen() can
differentiate between multiple instances of the same driver, similar
to how PCI bus-id is used in the desktop world.  It is not difficult
to change in drm_platform_set_busid(), although not sure if that would
be considered an ABI change to userspace.  (Maybe it is less critical,
I'm under the impression that other platform-drm users didn't even
realize we had a bus-id.)

> And generally, I think if the bus id is -1, there is no bus id in a
> sense. For example, with bus id 0 you'll get a device "omapdrm.0". With
> bus id -1 you'll get a device "omapdrm".
>
>> >> >> +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.
>>
>> Basically the short version is memory from a CMA carveout can be
>> re-used for unpinnable memory.  Not kmalloc, but it can be used for
>> anon userspace pages, for example.  Userspace needs memory too.
>
> Okay, let me ask the other way. Is 32MB enough for everyone? Hardcoding
> a value like that without any possibility to adjust it just sounds like
> a rather bad thing.

The main requirement is that, on omap3 or before (platforms without
DMM) you need enough to allocate all of your scanout buffers.

I'm not against having a bootarg to adjust, although I strongly prefer
sane defaults and not requiring a million bootargs just to boot in
some usable fashion.

BR,
-R

>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-06 15:50       ` Gross, Andy
@ 2012-03-07 12:05         ` Tomi Valkeinen
  2012-03-07 13:27           ` Rob Clark
  2012-03-07 15:59           ` Gross, Andy
  0 siblings, 2 replies; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-07 12:05 UTC (permalink / raw)
  To: Gross, Andy; +Cc: Rob Clark, dri-devel, linux-omap, patches, Greg KH

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

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



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-06 15:29       ` Rob Clark
@ 2012-03-07 11:59         ` Tomi Valkeinen
  2012-03-07 13:06           ` Rob Clark
  0 siblings, 1 reply; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-07 11:59 UTC (permalink / raw)
  To: Rob Clark; +Cc: Greg KH, linux-omap, dri-devel, patches

[-- Attachment #1: Type: text/plain, Size: 3362 bytes --]

On Tue, 2012-03-06 at 09:29 -0600, Rob Clark wrote:
> On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > 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.
> 
> well, I don't care much one way or another, but need to check if there
> is a small patch needed in drm core code that generates the bus-id for
> .id = -1..

Hmm, why does the drm core care about it?

And generally, I think if the bus id is -1, there is no bus id in a
sense. For example, with bus id 0 you'll get a device "omapdrm.0". With
bus id -1 you'll get a device "omapdrm".

> >> >> +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.
> 
> Basically the short version is memory from a CMA carveout can be
> re-used for unpinnable memory.  Not kmalloc, but it can be used for
> anon userspace pages, for example.  Userspace needs memory too.

Okay, let me ask the other way. Is 32MB enough for everyone? Hardcoding
a value like that without any possibility to adjust it just sounds like
a rather bad thing.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-06 14:35     ` Tomi Valkeinen
  2012-03-06 15:29       ` Rob Clark
@ 2012-03-06 15:50       ` Gross, Andy
  2012-03-07 12:05         ` Tomi Valkeinen
  1 sibling, 1 reply; 52+ messages in thread
From: Gross, Andy @ 2012-03-06 15:50 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Greg KH, linux-omap, patches, dri-devel, Rob Clark


[-- Attachment #1.1: Type: text/plain, Size: 865 bytes --]

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

[-- Attachment #1.2: Type: text/html, Size: 1224 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-06 14:35     ` Tomi Valkeinen
@ 2012-03-06 15:29       ` Rob Clark
  2012-03-07 11:59         ` Tomi Valkeinen
  2012-03-06 15:50       ` Gross, Andy
  1 sibling, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-03-06 15:29 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Greg KH, linux-omap, dri-devel, patches

On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 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.

well, I don't care much one way or another, but need to check if there
is a small patch needed in drm core code that generates the bus-id for
.id = -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.
>
> 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.

when most people refer to TILER they actually refer to DMM..

DMM is the piece which is basically a GART, it is what omapdrm is
programming and managing.

> 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.

yes, clearly there are other users.. we pass gem buffers allocated
from omapdrm to (for example, video decoder).  But it is omapdrm which
is managing the allocation, keeping track of which buffers are pinned
and which can be evicted, dealing with the complications of userspace
mmap'ing of tiled buffers, etc.  That stuff you don't want littered
throughout the kernel.

> 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.

Basically the short version is memory from a CMA carveout can be
re-used for unpinnable memory.  Not kmalloc, but it can be used for
anon userspace pages, for example.  Userspace needs memory too.

BR,
-R

>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-06 14:01   ` Rob Clark
@ 2012-03-06 14:35     ` Tomi Valkeinen
  2012-03-06 15:29       ` Rob Clark
  2012-03-06 15:50       ` Gross, Andy
  0 siblings, 2 replies; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-06 14:35 UTC (permalink / raw)
  To: Rob Clark; +Cc: Greg KH, linux-omap, dri-devel, patches


[-- Attachment #1.1: Type: text/plain, Size: 3945 bytes --]

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


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-06 13:26 ` Tomi Valkeinen
@ 2012-03-06 14:01   ` Rob Clark
  2012-03-06 14:35     ` Tomi Valkeinen
  0 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-03-06 14:01 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: dri-devel, linux-omap, patches, Greg KH, Andy Gross

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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-05 16:54 Rob Clark
  2012-03-06  0:10 ` Tony Lindgren
@ 2012-03-06 13:26 ` Tomi Valkeinen
  2012-03-06 14:01   ` Rob Clark
  1 sibling, 1 reply; 52+ messages in thread
From: Tomi Valkeinen @ 2012-03-06 13:26 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, linux-omap, patches, Greg KH, Andy Gross, Rob Clark

[-- Attachment #1: Type: text/plain, Size: 7433 bytes --]

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


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-06  0:10 ` Tony Lindgren
@ 2012-03-06  1:42   ` Rob Clark
  0 siblings, 0 replies; 52+ messages in thread
From: Rob Clark @ 2012-03-06  1:42 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: dri-devel, linux-omap, patches, Greg KH, Tomi Valkeinen, Andy Gross

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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-03-05 16:54 Rob Clark
@ 2012-03-06  0:10 ` Tony Lindgren
  2012-03-06  1:42   ` Rob Clark
  2012-03-06 13:26 ` Tomi Valkeinen
  1 sibling, 1 reply; 52+ messages in thread
From: Tony Lindgren @ 2012-03-06  0:10 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, linux-omap, patches, Greg KH, Tomi Valkeinen,
	Andy Gross, Rob Clark

* 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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH] omap2+: add drm device
@ 2012-03-05 16:54 Rob Clark
  2012-03-06  0:10 ` Tony Lindgren
  2012-03-06 13:26 ` Tomi Valkeinen
  0 siblings, 2 replies; 52+ messages in thread
From: Rob Clark @ 2012-03-05 16:54 UTC (permalink / raw)
  To: dri-devel, linux-omap
  Cc: patches, Greg KH, Tomi Valkeinen, Andy Gross, Rob Clark

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
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__ */
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-01-13 19:41 Rob Clark
  2012-01-13 19:46 ` Rob Clark
  2012-01-13 19:51 ` Aguirre, Sergio
@ 2012-01-13 20:29 ` Felipe Contreras
  2 siblings, 0 replies; 52+ messages in thread
From: Felipe Contreras @ 2012-01-13 20:29 UTC (permalink / raw)
  To: Rob Clark; +Cc: linux-omap, patches, Tomi Valkeinen, Andy Gross, Rob Clark

On Fri, Jan 13, 2012 at 9:41 PM, Rob Clark <rob.clark@linaro.org> wrote:
> +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..
> +        */

/*
 * Foo.
 */

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

32 * SZ1_M

> +#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/drm.h b/arch/arm/plat-omap/drm.h
> new file mode 100644
> index 0000000..56e0c0e
> --- /dev/null
> +++ b/arch/arm/plat-omap/drm.h

Maybe this should go to include/plat

> +#ifndef __PLAT_OMAP_DRM_H__
> +#define __PLAT_OMAP_DRM_H__

I see a lot of headers using this form:
__ARCH_OMAP_FOO_H

Cheers.

-- 
Felipe Contreras
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-01-13 20:23       ` Felipe Contreras
@ 2012-01-13 20:25         ` Rob Clark
  0 siblings, 0 replies; 52+ messages in thread
From: Rob Clark @ 2012-01-13 20:25 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: linux-omap, patches, Tomi Valkeinen, Andy Gross, Greg KH

On Fri, Jan 13, 2012 at 2:23 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Fri, Jan 13, 2012 at 9:53 PM, Rob Clark <rob@ti.com> wrote:
>> On Fri, Jan 13, 2012 at 1:49 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark <rob@ti.com> wrote:
>>>> On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark <rob.clark@linaro.org> wrote:
>>>>> +/* files from staging that contain platform data structure definitions */
>>>>> +#include "../../../drivers/staging/omapdrm/omap_priv.h"
>>>>> +#include "../../../drivers/staging/omapdrm/omap_dmm_tiler.h"
>>>>
>>>> btw, I'm not a huge fan of doing #includes this way, so if someone has
>>>> a better suggestion (given that staging drivers should not have
>>>> headers outside of drivers/staging) please let me know
>>>
>>> Move those structs to /arch/arm/plat-omap/drm.h?
>>
>> Can I do that?  Maybe it depends of if you consider those structs as
>> part of the driver, or part of the platform?
>
> Why not? The platform is using them in arch/arm/plat-omap/drm.c.
>
>> I guess that looks like how it was handled for dspbridge, so maybe
>> that means it is a suitable precedent..
>
> Indeed, the way I see it is this: imagine there's another driver in
> staging (or maybe even out of tree), that requires this data. It would
> simply include plat-omap/drm.h to fill this. OK, this is pretty
> far-fetched, but demonstrates the point: platform data should be
> completely independent of the drivers.

yeah, makes sense.. I'm about to send v2 of this patch.

BR,
-R

> Cheers.
>
> --
> Felipe Contreras
> --
> 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
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-01-13 19:53     ` Rob Clark
@ 2012-01-13 20:23       ` Felipe Contreras
  2012-01-13 20:25         ` Rob Clark
  0 siblings, 1 reply; 52+ messages in thread
From: Felipe Contreras @ 2012-01-13 20:23 UTC (permalink / raw)
  To: Rob Clark; +Cc: linux-omap, patches, Tomi Valkeinen, Andy Gross, Greg KH

On Fri, Jan 13, 2012 at 9:53 PM, Rob Clark <rob@ti.com> wrote:
> On Fri, Jan 13, 2012 at 1:49 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark <rob@ti.com> wrote:
>>> On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark <rob.clark@linaro.org> wrote:
>>>> +/* files from staging that contain platform data structure definitions */
>>>> +#include "../../../drivers/staging/omapdrm/omap_priv.h"
>>>> +#include "../../../drivers/staging/omapdrm/omap_dmm_tiler.h"
>>>
>>> btw, I'm not a huge fan of doing #includes this way, so if someone has
>>> a better suggestion (given that staging drivers should not have
>>> headers outside of drivers/staging) please let me know
>>
>> Move those structs to /arch/arm/plat-omap/drm.h?
>
> Can I do that?  Maybe it depends of if you consider those structs as
> part of the driver, or part of the platform?

Why not? The platform is using them in arch/arm/plat-omap/drm.c.

> I guess that looks like how it was handled for dspbridge, so maybe
> that means it is a suitable precedent..

Indeed, the way I see it is this: imagine there's another driver in
staging (or maybe even out of tree), that requires this data. It would
simply include plat-omap/drm.h to fill this. OK, this is pretty
far-fetched, but demonstrates the point: platform data should be
completely independent of the drivers.

Cheers.

-- 
Felipe Contreras
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-01-13 19:51 ` Aguirre, Sergio
@ 2012-01-13 19:54   ` Rob Clark
  0 siblings, 0 replies; 52+ messages in thread
From: Rob Clark @ 2012-01-13 19:54 UTC (permalink / raw)
  To: Aguirre, Sergio; +Cc: linux-omap, patches, Tomi Valkeinen, Andy Gross

On Fri, Jan 13, 2012 at 1:51 PM, Aguirre, Sergio <saaguirre@ti.com> wrote:
>> + *
>> + * DRM/KMS device registration for TI OMAP platforms
>> + *
>> + * Copyright (C) 2011 Texas Instruments
>
> Happy new year! (2012?) :)

well, the patch did start life last year.. I can update these ;-)

BR,
-R

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-01-13 19:49   ` Felipe Contreras
@ 2012-01-13 19:53     ` Rob Clark
  2012-01-13 20:23       ` Felipe Contreras
  0 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-01-13 19:53 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: linux-omap, patches, Tomi Valkeinen, Andy Gross, Greg KH

On Fri, Jan 13, 2012 at 1:49 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark <rob@ti.com> wrote:
>> On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark <rob.clark@linaro.org> wrote:
>>> +/* files from staging that contain platform data structure definitions */
>>> +#include "../../../drivers/staging/omapdrm/omap_priv.h"
>>> +#include "../../../drivers/staging/omapdrm/omap_dmm_tiler.h"
>>
>> btw, I'm not a huge fan of doing #includes this way, so if someone has
>> a better suggestion (given that staging drivers should not have
>> headers outside of drivers/staging) please let me know
>
> Move those structs to /arch/arm/plat-omap/drm.h?

Can I do that?  Maybe it depends of if you consider those structs as
part of the driver, or part of the platform?

I guess that looks like how it was handled for dspbridge, so maybe
that means it is a suitable precedent..

BR,
-R

> --
> Felipe Contreras
> --
> 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
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-01-13 19:41 Rob Clark
  2012-01-13 19:46 ` Rob Clark
@ 2012-01-13 19:51 ` Aguirre, Sergio
  2012-01-13 19:54   ` Rob Clark
  2012-01-13 20:29 ` Felipe Contreras
  2 siblings, 1 reply; 52+ messages in thread
From: Aguirre, Sergio @ 2012-01-13 19:51 UTC (permalink / raw)
  To: Rob Clark; +Cc: linux-omap, patches, Tomi Valkeinen, Andy Gross, Rob Clark

Hi Rob,

Minor nitpicks.

On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark <rob.clark@linaro.org> wrote:
> From: Rob Clark <rob@ti.com>
>
> Register OMAP DRM/KMS platform device, and reserve a CMA region for
> the device to use for buffer allocation.
>
> Signed-off-by: Rob Clark <rob@ti.com>
> ---
>  arch/arm/plat-omap/Makefile |    2 +-
>  arch/arm/plat-omap/common.c |    2 +
>  arch/arm/plat-omap/drm.c    |   88 +++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/plat-omap/drm.h    |   37 ++++++++++++++++++
>  4 files changed, 128 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/plat-omap/drm.c
>  create mode 100644 arch/arm/plat-omap/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..caf6082 100644
> --- a/arch/arm/plat-omap/common.c
> +++ b/arch/arm/plat-omap/common.c
> @@ -24,6 +24,7 @@
>
>  #include <plat/omap-secure.h>
>
> +#include "drm.h"
>
>  #define NO_LENGTH_CHECK 0xffffffff
>
> @@ -65,6 +66,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..5d8588f
> --- /dev/null
> +++ b/arch/arm/plat-omap/drm.c
> @@ -0,0 +1,88 @@
> +/*
> + * File: arch/arm/plat-omap/drm.c

I believe keeping a file path is frowned upon.

> + *
> + * DRM/KMS device registration for TI OMAP platforms
> + *
> + * Copyright (C) 2011 Texas Instruments

Happy new year! (2012?) :)

> + * 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 "drm.h"
> +
> +/* files from staging that contain platform data structure definitions */
> +#include "../../../drivers/staging/omapdrm/omap_priv.h"
> +#include "../../../drivers/staging/omapdrm/omap_dmm_tiler.h"
> +
> +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
> +
> +static struct omap_drm_platform_data omapdrm_platdata;
> +static struct omap_dmm_platform_data dmm_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;
> +
> +       /* lookup and populate the DMM information, if present - OMAP4+ */
> +       oh = omap_hwmod_lookup("dmm");
> +
> +       if (oh) {
> +               dmm_platdata.base = omap_hwmod_get_mpu_rt_va(oh);
> +               dmm_platdata.irq = oh->mpu_irqs[0].irq;
> +
> +               if (dmm_platdata.base)
> +                       omapdrm_platdata.dmm_pdata = &dmm_platdata;
> +       }
> +
> +       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/drm.h b/arch/arm/plat-omap/drm.h
> new file mode 100644
> index 0000000..56e0c0e
> --- /dev/null
> +++ b/arch/arm/plat-omap/drm.h
> @@ -0,0 +1,37 @@
> +/*
> + * File: arch/arm/plat-omap/drm.c

Again here. (The path is incorrect anyways)

> + *
> + * DRM/KMS device registration for TI OMAP platforms
> + *
> + * Copyright (C) 2011 Texas Instruments

2012 also here.

> + * 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__
> +
> +#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__ */
> --
> 1.7.5.4
>
> --
> 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
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-01-13 19:46 ` Rob Clark
@ 2012-01-13 19:49   ` Felipe Contreras
  2012-01-13 19:53     ` Rob Clark
  0 siblings, 1 reply; 52+ messages in thread
From: Felipe Contreras @ 2012-01-13 19:49 UTC (permalink / raw)
  To: Rob Clark; +Cc: linux-omap, patches, Tomi Valkeinen, Andy Gross

On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark <rob@ti.com> wrote:
> On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark <rob.clark@linaro.org> wrote:
>> +/* files from staging that contain platform data structure definitions */
>> +#include "../../../drivers/staging/omapdrm/omap_priv.h"
>> +#include "../../../drivers/staging/omapdrm/omap_dmm_tiler.h"
>
> btw, I'm not a huge fan of doing #includes this way, so if someone has
> a better suggestion (given that staging drivers should not have
> headers outside of drivers/staging) please let me know

Move those structs to /arch/arm/plat-omap/drm.h?

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH] omap2+: add drm device
  2012-01-13 19:41 Rob Clark
@ 2012-01-13 19:46 ` Rob Clark
  2012-01-13 19:49   ` Felipe Contreras
  2012-01-13 19:51 ` Aguirre, Sergio
  2012-01-13 20:29 ` Felipe Contreras
  2 siblings, 1 reply; 52+ messages in thread
From: Rob Clark @ 2012-01-13 19:46 UTC (permalink / raw)
  To: linux-omap; +Cc: patches, Tomi Valkeinen, Andy Gross, Rob Clark

On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark <rob.clark@linaro.org> wrote:
> From: Rob Clark <rob@ti.com>
>
> Register OMAP DRM/KMS platform device, and reserve a CMA region for
> the device to use for buffer allocation.
>
> Signed-off-by: Rob Clark <rob@ti.com>
> ---
>  arch/arm/plat-omap/Makefile |    2 +-
>  arch/arm/plat-omap/common.c |    2 +
>  arch/arm/plat-omap/drm.c    |   88 +++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/plat-omap/drm.h    |   37 ++++++++++++++++++
>  4 files changed, 128 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/plat-omap/drm.c
>  create mode 100644 arch/arm/plat-omap/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..caf6082 100644
> --- a/arch/arm/plat-omap/common.c
> +++ b/arch/arm/plat-omap/common.c
> @@ -24,6 +24,7 @@
>
>  #include <plat/omap-secure.h>
>
> +#include "drm.h"
>
>  #define NO_LENGTH_CHECK 0xffffffff
>
> @@ -65,6 +66,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..5d8588f
> --- /dev/null
> +++ b/arch/arm/plat-omap/drm.c
> @@ -0,0 +1,88 @@
> +/*
> + * File: arch/arm/plat-omap/drm.c
> + *
> + * DRM/KMS device registration for TI OMAP platforms
> + *
> + * Copyright (C) 2011 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 "drm.h"
> +
> +/* files from staging that contain platform data structure definitions */
> +#include "../../../drivers/staging/omapdrm/omap_priv.h"
> +#include "../../../drivers/staging/omapdrm/omap_dmm_tiler.h"

btw, I'm not a huge fan of doing #includes this way, so if someone has
a better suggestion (given that staging drivers should not have
headers outside of drivers/staging) please let me know

BR,
-R

> +#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
> +
> +static struct omap_drm_platform_data omapdrm_platdata;
> +static struct omap_dmm_platform_data dmm_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;
> +
> +       /* lookup and populate the DMM information, if present - OMAP4+ */
> +       oh = omap_hwmod_lookup("dmm");
> +
> +       if (oh) {
> +               dmm_platdata.base = omap_hwmod_get_mpu_rt_va(oh);
> +               dmm_platdata.irq = oh->mpu_irqs[0].irq;
> +
> +               if (dmm_platdata.base)
> +                       omapdrm_platdata.dmm_pdata = &dmm_platdata;
> +       }
> +
> +       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/drm.h b/arch/arm/plat-omap/drm.h
> new file mode 100644
> index 0000000..56e0c0e
> --- /dev/null
> +++ b/arch/arm/plat-omap/drm.h
> @@ -0,0 +1,37 @@
> +/*
> + * File: arch/arm/plat-omap/drm.c
> + *
> + * DRM/KMS device registration for TI OMAP platforms
> + *
> + * Copyright (C) 2011 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__
> +
> +#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__ */
> --
> 1.7.5.4
>
--
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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH] omap2+: add drm device
@ 2012-01-13 19:41 Rob Clark
  2012-01-13 19:46 ` Rob Clark
                   ` (2 more replies)
  0 siblings, 3 replies; 52+ messages in thread
From: Rob Clark @ 2012-01-13 19:41 UTC (permalink / raw)
  To: linux-omap; +Cc: patches, Tomi Valkeinen, Andy Gross, Rob Clark

From: Rob Clark <rob@ti.com>

Register OMAP DRM/KMS platform device, and reserve a CMA region for
the device to use for buffer allocation.

Signed-off-by: Rob Clark <rob@ti.com>
---
 arch/arm/plat-omap/Makefile |    2 +-
 arch/arm/plat-omap/common.c |    2 +
 arch/arm/plat-omap/drm.c    |   88 +++++++++++++++++++++++++++++++++++++++++++
 arch/arm/plat-omap/drm.h    |   37 ++++++++++++++++++
 4 files changed, 128 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/plat-omap/drm.c
 create mode 100644 arch/arm/plat-omap/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..caf6082 100644
--- a/arch/arm/plat-omap/common.c
+++ b/arch/arm/plat-omap/common.c
@@ -24,6 +24,7 @@
 
 #include <plat/omap-secure.h>
 
+#include "drm.h"
 
 #define NO_LENGTH_CHECK 0xffffffff
 
@@ -65,6 +66,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..5d8588f
--- /dev/null
+++ b/arch/arm/plat-omap/drm.c
@@ -0,0 +1,88 @@
+/*
+ * File: arch/arm/plat-omap/drm.c
+ *
+ * DRM/KMS device registration for TI OMAP platforms
+ *
+ * Copyright (C) 2011 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 "drm.h"
+
+/* files from staging that contain platform data structure definitions */
+#include "../../../drivers/staging/omapdrm/omap_priv.h"
+#include "../../../drivers/staging/omapdrm/omap_dmm_tiler.h"
+
+#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE)
+
+static struct omap_drm_platform_data omapdrm_platdata;
+static struct omap_dmm_platform_data dmm_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;
+
+	/* lookup and populate the DMM information, if present - OMAP4+ */
+	oh = omap_hwmod_lookup("dmm");
+
+	if (oh) {
+		dmm_platdata.base = omap_hwmod_get_mpu_rt_va(oh);
+		dmm_platdata.irq = oh->mpu_irqs[0].irq;
+
+		if (dmm_platdata.base)
+			omapdrm_platdata.dmm_pdata = &dmm_platdata;
+	}
+
+	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/drm.h b/arch/arm/plat-omap/drm.h
new file mode 100644
index 0000000..56e0c0e
--- /dev/null
+++ b/arch/arm/plat-omap/drm.h
@@ -0,0 +1,37 @@
+/*
+ * File: arch/arm/plat-omap/drm.c
+ *
+ * DRM/KMS device registration for TI OMAP platforms
+ *
+ * Copyright (C) 2011 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__
+
+#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__ */
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 52+ messages in thread

end of thread, other threads:[~2012-07-03  7:09 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-13 20:34 [PATCH] omap2+: add drm device Rob Clark
2012-03-14 12:38 ` Tomi Valkeinen
2012-03-14 12:55   ` Rob Clark
2012-03-14 13:07     ` Tomi Valkeinen
2012-03-14 13:16       ` Rob Clark
2012-03-14 13:43         ` Tomi Valkeinen
2012-03-14 15:06           ` Rob Clark
2012-03-15  8:46             ` Tomi Valkeinen
2012-03-15 12:32               ` Rob Clark
2012-03-16 11:03                 ` Tomi Valkeinen
  -- strict thread matches above, loose matches on Subject: below --
2012-05-23 20:08 Andy Gross
2012-05-24  6:01 ` Tomi Valkeinen
2012-05-24  6:27   ` Clark, Rob
2012-05-24  7:05     ` Tomi Valkeinen
2012-05-24  7:21       ` Tomi Valkeinen
2012-05-24  8:44         ` Rob Clark
2012-05-24 12:13           ` Tomi Valkeinen
2012-05-24 15:09             ` Gross, Andy
2012-05-24 15:26               ` Tomi Valkeinen
2012-06-11 14:51                 ` Gross, Andy
2012-06-11 14:54                   ` Gross, Andy
2012-06-11 15:05                   ` Tomi Valkeinen
2012-05-24  8:35       ` Rob Clark
2012-05-24 12:10         ` Tomi Valkeinen
2012-05-24 14:22   ` Gross, Andy
2012-06-11 15:51 ` Rob Clark
2012-06-19 21:12   ` Gross, Andy
2012-07-03  7:09     ` Tony Lindgren
2012-03-05 16:54 Rob Clark
2012-03-06  0:10 ` Tony Lindgren
2012-03-06  1:42   ` Rob Clark
2012-03-06 13:26 ` Tomi Valkeinen
2012-03-06 14:01   ` Rob Clark
2012-03-06 14:35     ` Tomi Valkeinen
2012-03-06 15:29       ` Rob Clark
2012-03-07 11:59         ` Tomi Valkeinen
2012-03-07 13:06           ` Rob Clark
2012-03-07 13:11             ` Tomi Valkeinen
2012-03-06 15:50       ` Gross, Andy
2012-03-07 12:05         ` Tomi Valkeinen
2012-03-07 13:27           ` Rob Clark
2012-03-07 15:59           ` Gross, Andy
2012-03-08  7:47             ` Tomi Valkeinen
2012-01-13 19:41 Rob Clark
2012-01-13 19:46 ` Rob Clark
2012-01-13 19:49   ` Felipe Contreras
2012-01-13 19:53     ` Rob Clark
2012-01-13 20:23       ` Felipe Contreras
2012-01-13 20:25         ` Rob Clark
2012-01-13 19:51 ` Aguirre, Sergio
2012-01-13 19:54   ` Rob Clark
2012-01-13 20:29 ` Felipe Contreras

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.