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__ */
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
* 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
dri-devel@lists.freedesktop.org
-
Andy Gross -
Clark, Rob -
Gross, Andy
-
Rob Clark -
Tomi Valkeinen -
Tony Lindgren