| Message ID | 1379063679-4869-2-git-send-email-archit@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/09/13 12:14, Archit Taneja wrote: > Move omapdrm device creation inside the omap_display_init so that we can > correctly create the device based on the presence of omapdss. > > Originally worked on by Andy Gross. If the dmm device is present in the DT data, there is no need to create the dmm device. It's created automatically. Also, omapfb device is currently created the same way as omapdrm, independently of omapdss. Why is it a problem to have them like that? Tomi
On Friday 13 September 2013 02:54 PM, Tomi Valkeinen wrote: > On 13/09/13 12:14, Archit Taneja wrote: >> Move omapdrm device creation inside the omap_display_init so that we can >> correctly create the device based on the presence of omapdss. >> >> Originally worked on by Andy Gross. > > If the dmm device is present in the DT data, there is no need to create > the dmm device. It's created automatically. Yes, that is done in a later patch. > > Also, omapfb device is currently created the same way as omapdrm, > independently of omapdss. Why is it a problem to have them like that? In a multiplatform config, we might have CONFIG_DRM_OMAP and CONFIG_DRM_OMAP_MODULE selected. This would cause the call omap_init_drm() in arch/arm/mach-omap2/drm.c to occur. Creating a platform device for omapdrm is unnecessary here isn't it? Tying it with the initialisation of omapdss devices prevents it. Archit -- 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
On Friday 13 September 2013 03:08 PM, Archit Taneja wrote: > On Friday 13 September 2013 02:54 PM, Tomi Valkeinen wrote: >> On 13/09/13 12:14, Archit Taneja wrote: >>> Move omapdrm device creation inside the omap_display_init so that we can >>> correctly create the device based on the presence of omapdss. >>> >>> Originally worked on by Andy Gross. >> >> If the dmm device is present in the DT data, there is no need to create >> the dmm device. It's created automatically. > > Yes, that is done in a later patch. > >> >> Also, omapfb device is currently created the same way as omapdrm, >> independently of omapdss. Why is it a problem to have them like that? > > In a multiplatform config, we might have CONFIG_DRM_OMAP and > CONFIG_DRM_OMAP_MODULE selected. I meant these configs might be selected even if the image is booted on am3xx platform. Archit > > This would cause the call omap_init_drm() in arch/arm/mach-omap2/drm.c > to occur. Creating a platform device for omapdrm is unnecessary here > isn't it? Tying it with the initialisation of omapdss devices prevents it. > > Archit > -- > 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
On 13/09/13 12:38, Archit Taneja wrote: >> Also, omapfb device is currently created the same way as omapdrm, >> independently of omapdss. Why is it a problem to have them like that? > > In a multiplatform config, we might have CONFIG_DRM_OMAP and > CONFIG_DRM_OMAP_MODULE selected. > > This would cause the call omap_init_drm() in arch/arm/mach-omap2/drm.c > to occur. Creating a platform device for omapdrm is unnecessary here > isn't it? Tying it with the initialisation of omapdss devices prevents it. Well, omapdrm depends on omapdss in Kconfig. So if you have DRM enabled, you also have DSS enabled. Tomi
On 13/09/13 12:39, Archit Taneja wrote: > On Friday 13 September 2013 03:08 PM, Archit Taneja wrote: >> On Friday 13 September 2013 02:54 PM, Tomi Valkeinen wrote: >>> On 13/09/13 12:14, Archit Taneja wrote: >>>> Move omapdrm device creation inside the omap_display_init so that we >>>> can >>>> correctly create the device based on the presence of omapdss. >>>> >>>> Originally worked on by Andy Gross. >>> >>> If the dmm device is present in the DT data, there is no need to create >>> the dmm device. It's created automatically. >> >> Yes, that is done in a later patch. >> >>> >>> Also, omapfb device is currently created the same way as omapdrm, >>> independently of omapdss. Why is it a problem to have them like that? >> >> In a multiplatform config, we might have CONFIG_DRM_OMAP and >> CONFIG_DRM_OMAP_MODULE selected. > > I meant these configs might be selected even if the image is booted on > am3xx platform. Ah, I see. And the same omap_arch_initcall() is used for am3xxx also, even if the DSS (and thus DRM) doesn't exist in the HW. We have the same problem with omapfb, so it'd be good to include that in the same series. Hmm. If omap_generic_init() is called on am3xxx, it means we try to create the dss stuff there also. It should fail to the DSS version check, printing an error (at least I hope), but we really shouldn't even call the dss init code on am3xxx. I wonder how this should be fixed... Tomi
On Friday 13 September 2013 03:18 PM, Tomi Valkeinen wrote: > On 13/09/13 12:39, Archit Taneja wrote: >> On Friday 13 September 2013 03:08 PM, Archit Taneja wrote: >>> On Friday 13 September 2013 02:54 PM, Tomi Valkeinen wrote: >>>> On 13/09/13 12:14, Archit Taneja wrote: >>>>> Move omapdrm device creation inside the omap_display_init so that we >>>>> can >>>>> correctly create the device based on the presence of omapdss. >>>>> >>>>> Originally worked on by Andy Gross. >>>> >>>> If the dmm device is present in the DT data, there is no need to create >>>> the dmm device. It's created automatically. >>> >>> Yes, that is done in a later patch. >>> >>>> >>>> Also, omapfb device is currently created the same way as omapdrm, >>>> independently of omapdss. Why is it a problem to have them like that? >>> >>> In a multiplatform config, we might have CONFIG_DRM_OMAP and >>> CONFIG_DRM_OMAP_MODULE selected. >> >> I meant these configs might be selected even if the image is booted on >> am3xx platform. > > Ah, I see. And the same omap_arch_initcall() is used for am3xxx also, > even if the DSS (and thus DRM) doesn't exist in the HW. > > We have the same problem with omapfb, so it'd be good to include that in > the same series. > > Hmm. If omap_generic_init() is called on am3xxx, it means we try to > create the dss stuff there also. It should fail to the DSS version > check, printing an error (at least I hope), but we really shouldn't even > call the dss init code on am3xxx. > > I wonder how this should be fixed... The calls in omap_generic_init() check the machine type via of_machine_is_compatible(), even if it's a multiplatform image, the dtb should be only of one platform. So it wouldn't be called at all. Archit -- 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
On 13/09/13 12:51, Archit Taneja wrote: > The calls in omap_generic_init() check the machine type via > of_machine_is_compatible(), even if it's a multiplatform image, the dtb > should be only of one platform. So it wouldn't be called at all. Hmm. BeagleBone is "ti,am33xx". The "Generic AM33XX (Flattened Device Tree)" machine definition uses omap_generic_init(). So if I'm not missing something here, omap_generic_init() is called for BeagleBone. Tomi
On Friday 13 September 2013 03:32 PM, Tomi Valkeinen wrote: > On 13/09/13 12:51, Archit Taneja wrote: > >> The calls in omap_generic_init() check the machine type via >> of_machine_is_compatible(), even if it's a multiplatform image, the dtb >> should be only of one platform. So it wouldn't be called at all. > > Hmm. BeagleBone is "ti,am33xx". The "Generic AM33XX (Flattened Device > Tree)" machine definition uses omap_generic_init(). So if I'm not > missing something here, omap_generic_init() is called for BeagleBone. I was talking about the calls within omap_generic_init() : omap_generic_init(void) { ... if (of_machine_is_compatible("ti,omap4-panda")) { omap4_panda_display_init_of(); legacy_init_ehci_clk("auxclk3_ck"); } } omap4_panda_display_init_of() would be called only if a panda board dtb was used. Are you talking about these display calls, or something else? Archit -- 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
On 13/09/13 13:17, Archit Taneja wrote: > On Friday 13 September 2013 03:32 PM, Tomi Valkeinen wrote: >> On 13/09/13 12:51, Archit Taneja wrote: >> >>> The calls in omap_generic_init() check the machine type via >>> of_machine_is_compatible(), even if it's a multiplatform image, the dtb >>> should be only of one platform. So it wouldn't be called at all. >> >> Hmm. BeagleBone is "ti,am33xx". The "Generic AM33XX (Flattened Device >> Tree)" machine definition uses omap_generic_init(). So if I'm not >> missing something here, omap_generic_init() is called for BeagleBone. > > I was talking about the calls within omap_generic_init() : > > omap_generic_init(void) > { > ... > if (of_machine_is_compatible("ti,omap4-panda")) { > omap4_panda_display_init_of(); > legacy_init_ehci_clk("auxclk3_ck"); > > } > } > > omap4_panda_display_init_of() would be called only if a panda board dtb > was used. Are you talking about these display calls, or something else? Ah, right. I was looking at the DSS DT branch. There we have omapdss_init_of() call from omap_generic_init(). So that is a problem, but not in the mainline. You're right, the current mainline doesn't call the DSS init function on am33xx. But it does create omapfb and omapdrm devices, as you noted. Well, those devices don't do any actual harm, but I agree that they shouldn't be there. It's probably best to move the device creation into display.c, as you did. Just include omapfb also, and maybe remove the DMM creation as a first patch. Tomi
On Friday 13 September 2013 03:54 PM, Tomi Valkeinen wrote: > On 13/09/13 13:17, Archit Taneja wrote: >> On Friday 13 September 2013 03:32 PM, Tomi Valkeinen wrote: >>> On 13/09/13 12:51, Archit Taneja wrote: >>> >>>> The calls in omap_generic_init() check the machine type via >>>> of_machine_is_compatible(), even if it's a multiplatform image, the dtb >>>> should be only of one platform. So it wouldn't be called at all. >>> >>> Hmm. BeagleBone is "ti,am33xx". The "Generic AM33XX (Flattened Device >>> Tree)" machine definition uses omap_generic_init(). So if I'm not >>> missing something here, omap_generic_init() is called for BeagleBone. >> >> I was talking about the calls within omap_generic_init() : >> >> omap_generic_init(void) >> { >> ... >> if (of_machine_is_compatible("ti,omap4-panda")) { >> omap4_panda_display_init_of(); >> legacy_init_ehci_clk("auxclk3_ck"); >> >> } >> } >> >> omap4_panda_display_init_of() would be called only if a panda board dtb >> was used. Are you talking about these display calls, or something else? > > Ah, right. I was looking at the DSS DT branch. There we have > omapdss_init_of() call from omap_generic_init(). So that is a problem, > but not in the mainline. > > You're right, the current mainline doesn't call the DSS init function on > am33xx. But it does create omapfb and omapdrm devices, as you noted. > > Well, those devices don't do any actual harm, but I agree that they > shouldn't be there. It's probably best to move the device creation into > display.c, as you did. Just include omapfb also, and maybe remove the > DMM creation as a first patch. Sure, I'll do that. I'm not sure either about how to deal with the direct call to omapdss_init_of(). Archit -- 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
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index ff2c162..f73b6a5 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -229,10 +229,6 @@ endif # OMAP2420 MSDI controller integration support ("MMC") obj-$(CONFIG_SOC_OMAP2420) += msdi.o -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/display.c b/arch/arm/mach-omap2/display.c index 03a0516..d097d23 100644 --- a/arch/arm/mach-omap2/display.c +++ b/arch/arm/mach-omap2/display.c @@ -18,11 +18,13 @@ #include <linux/string.h> #include <linux/kernel.h> #include <linux/init.h> +#include <linux/dma-mapping.h> #include <linux/platform_device.h> #include <linux/io.h> #include <linux/clk.h> #include <linux/err.h> #include <linux/delay.h> +#include <linux/platform_data/omap_drm.h> #include <video/omapdss.h> #include "omap_hwmod.h" @@ -102,6 +104,52 @@ static const struct omap_dss_hwmod_data omap4_dss_hwmod_data[] __initconst = { { "dss_hdmi", "omapdss_hdmi", -1 }, }; +#if defined(CONFIG_DRM_OMAP) || defined(CONFIG_DRM_OMAP_MODULE) + +static struct omap_drm_platform_data platform_drm_data; + +static struct platform_device drm_device = { + .dev = { + .coherent_dma_mask = DMA_BIT_MASK(32), + .platform_data = &platform_drm_data, + }, + .name = "omapdrm", + .id = 0, +}; + +static struct platform_device *omap_drm_device = &drm_device; +#else +static struct platform_device *omap_drm_device; +#endif + +static int __init omapdrm_init(void) +{ + int r = 0; + + /* create DRM and DMM device */ + if (omap_drm_device != NULL) { + 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); + WARN(IS_ERR(pdev), + "Could not build omap_device for %s\n", + oh->name); + } + + platform_drm_data.omaprev = GET_OMAP_TYPE; + + r = platform_device_register(omap_drm_device); + if (r < 0) + pr_err("Unable to register omapdrm device\n"); + } + + return r; +} + static void __init omap4_tpd12s015_mux_pads(void) { omap_mux_init_signal("hdmi_cec", @@ -416,6 +464,13 @@ int __init omap_display_init(struct omap_dss_board_info *board_data) } } + /* create DRM and DMM device */ + r = omapdrm_init(); + if (r < 0) { + pr_err("Unable to register omapdrm device\n"); + return r; + } + return 0; } diff --git a/arch/arm/mach-omap2/drm.c b/arch/arm/mach-omap2/drm.c deleted file mode 100644 index 59a4af7..0000000 --- a/arch/arm/mach-omap2/drm.c +++ /dev/null @@ -1,67 +0,0 @@ -/* - * 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 <linux/platform_data/omap_drm.h> - -#include "soc.h" -#include "omap_device.h" -#include "omap_hwmod.h" - -#if defined(CONFIG_DRM_OMAP) || (CONFIG_DRM_OMAP_MODULE) - -static struct omap_drm_platform_data platform_data; - -static struct platform_device omap_drm_device = { - .dev = { - .coherent_dma_mask = DMA_BIT_MASK(32), - .platform_data = &platform_data, - }, - .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); - WARN(IS_ERR(pdev), "Could not build omap_device for %s\n", - oh->name); - } - - platform_data.omaprev = GET_OMAP_TYPE; - - return platform_device_register(&omap_drm_device); - -} - -omap_arch_initcall(omap_init_drm); - -#endif
Move omapdrm device creation inside the omap_display_init so that we can correctly create the device based on the presence of omapdss. Originally worked on by Andy Gross. Cc: Andy Gross <andygro@gmail.com> Signed-off-by: Archit Taneja <archit@ti.com> --- arch/arm/mach-omap2/Makefile | 4 --- arch/arm/mach-omap2/display.c | 55 +++++++++++++++++++++++++++++++++++ arch/arm/mach-omap2/drm.c | 67 ------------------------------------------- 3 files changed, 55 insertions(+), 71 deletions(-) delete mode 100644 arch/arm/mach-omap2/drm.c