| Message ID | 1368897139-25485-4-git-send-email-sebastian.hesselbarth@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 18 May 2013 19:12:18 +0200 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote: > This adds OF support for the Dove DRM driver recently posted as RFC by > Russell King. > > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> > --- > Cc: Russell King <linux@arm.linux.org.uk> > Cc: linux-arm-kernel@lists.infradead.org > Cc: dri-devel@lists.freedesktop.org > Cc: Jason Cooper <jason@lakedaemon.net> > Cc: Jean-Francois Moine <moinejf@free.fr> > --- > drivers/gpu/drm/dove/Kconfig | 4 ++ > drivers/gpu/drm/dove/Makefile | 1 + > drivers/gpu/drm/dove/dove_card.c | 110 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 115 insertions(+) > create mode 100644 drivers/gpu/drm/dove/dove_card.c > > diff --git a/drivers/gpu/drm/dove/Kconfig b/drivers/gpu/drm/dove/Kconfig > index 718d3c5..a943ea5 100644 > --- a/drivers/gpu/drm/dove/Kconfig > +++ b/drivers/gpu/drm/dove/Kconfig > @@ -28,4 +28,8 @@ config DRM_DOVE_TDA1998X > config DRM_DOVE_CURSOR > bool "Enable Dove DRM hardware cursor support" > > +config DRM_DOVE_OF > + bool "Enable Dove DRM OF video card" > + depends on OF > + > endif > diff --git a/drivers/gpu/drm/dove/Makefile b/drivers/gpu/drm/dove/Makefile > index 65c701e..f0b6eed 100644 > --- a/drivers/gpu/drm/dove/Makefile > +++ b/drivers/gpu/drm/dove/Makefile > @@ -5,5 +5,6 @@ dove-y := dove_crtc.o dove_drv.o dove_fb.o dove_fbdev.o \ > dove-$(CONFIG_DEBUG_FS) += dove_debugfs.o > > dove-$(CONFIG_DRM_DOVE_TDA1998X) += dove_tda19988.o > +dove-$(CONFIG_DRM_DOVE_OF) += dove_card.o > > obj-$(CONFIG_DRM_DOVE) := dove.o > diff --git a/drivers/gpu/drm/dove/dove_card.c b/drivers/gpu/drm/dove/dove_card.c > new file mode 100644 > index 0000000..e4bcb5b > --- /dev/null > +++ b/drivers/gpu/drm/dove/dove_card.c > @@ -0,0 +1,110 @@ > +/* > + * Copyright (C) 2013 > + * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.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. > + */ > +#include <linux/clk.h> > +#include <linux/clkdev.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> > + > +#define DOVE_LCD0_BASE 0x20000 > +#define DOVE_LCD1_BASE 0x10000 > + > +static struct resource dove_drm_resources[5]; > +static struct platform_device dove_drm_platform_device = { > + .name = "dove-drm", > + .id = 0, > + .dev = { .coherent_dma_mask = ~0, }, > + .resource = dove_drm_resources, > +}; > + > +static int dove_card_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct device_node *lcdnp; > + struct resource *res = dove_drm_resources; > + int ret, n = 0, crtcs = 0; > + > + /* get video memory resource */ > + if (of_address_to_resource(np, 0, &res[n++])) { > + dev_err(&pdev->dev, "invalid or missing video memory\n"); > + return -EINVAL; > + } > + > + /* get reg and irq resource from each enabled lcdc */ > + for_each_compatible_node(lcdnp, NULL, "marvell,dove-lcd") { > + struct clk_lookup *cl; > + struct clk *clk; > + int lcd; > + > + if (!of_device_is_available(lcdnp)) > + continue; > + > + ret = of_address_to_resource(lcdnp, 0, &res[n]); > + if (ret) > + return ret; > + lcd = ((res[n].start & 0xfffff) == DOVE_LCD1_BASE); > + n++; > + > + ret = of_irq_to_resource(lcdnp, 0, &res[n]); > + if (ret < 0) > + return ret; > + n++; > + > + crtcs++; > + > + clk = clk_get(&pdev->dev, NULL); > + if (IS_ERR(clk)) { > + ret = PTR_ERR(clk); > + if (ret == -ENOENT) > + return -EPROBE_DEFER; > + return ret; > + } > + > + /* add clock alias for dovefb.0 */ > + cl = clkdev_alloc(clk, "extclk", "dovefb.0"); > + if (cl) > + clkdev_add(cl); > + clk_put(clk); > + } > + > + if (!crtcs) > + return -ENODEV; > + > + dove_drm_platform_device.num_resources = n; > + ret = platform_device_register(&dove_drm_platform_device); > + if (ret) { > + dev_err(&pdev->dev, "unable to register drm device\n"); > + return ret; > + } > + > + return 0; > +} > + > +static const struct of_device_id dove_card_of_ids[] = { > + { .compatible = "marvell,dove-video-card", }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, dove_card_of_ids); > + > +static struct platform_driver dove_card_driver = { > + .probe = dove_card_probe, > + .driver = { > + .owner = THIS_MODULE, > + .name = "dove-drm-card", > + .of_match_table = of_match_ptr(dove_card_of_ids), > + }, > +}; > +module_platform_driver(dove_card_driver); > + > +MODULE_AUTHOR("Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>"); > +MODULE_DESCRIPTION("Dove DRM Graphics Card"); > +MODULE_LICENSE("GPL"); It seems we are moving backwards: - what about the display controller? - how do you clone the lcd 0 output to the port B? - what occurs when the si5351 and the tda998x are modules? My driver had the same layout as Russell's when I proposed it to you and when you insisted to handle the 2 LCDs and the 2 ports as one card. I spent 2 months to have a nice design and you put it to garbage! I am not happy...
On 05/18/2013 07:45 PM, Jean-Francois Moine wrote: > On Sat, 18 May 2013 19:12:18 +0200 > Sebastian Hesselbarth<sebastian.hesselbarth@gmail.com> wrote: >> This adds OF support for the Dove DRM driver recently posted as RFC by >> Russell King. >> ... Jean-Francois, one thing first: It is an RFC! It is to allow you to _test_ rmk's driver on DT. Nothing more, nothing less. I will comment on your questions but that all can change for a full patch set of rmk, you, or me. The "video-card" node combines all devices that will be available and active on a specific Dove board. As you may have noticed about rmk's driver, it is registering crtcs from IORESOURCE_MEM passed with the platform_device. To match with this approach we _have to_ recreate that platform_device from what we see on DT. On DT each bus node gets registered as its own platform_device. So in the video-card driver we look for node we know of and put together a platform_device for rmk's driver. We cannot hook DT upon either an lcd node nor dcon node as they might be disabled and the driver will get called multiple times. > It seems we are moving backwards: > - what about the display controller? That would be part of probing DT nodes above. I did not take care of that because rmk doesn't support dcon for now. > - how do you clone the lcd 0 output to the port B? Pass properties on video-card node or even better let dcon driver take care of it when it sees a video-card with more than one crtc. > - what occurs when the si5351 and the tda998x are modules? Touche, forgot that part. Feel free to add module support to the RFC. > My driver had the same layout as Russell's when I proposed it to you > and when you insisted to handle the 2 LCDs and the 2 ports as one card. I still insist to handle 2 LCDs and DCON. > I spent 2 months to have a nice design and you put it to garbage! > I am not happy... I put nothing to garbage. _You_ also agreed to merge with rmk's driver! We can now put in all features we implemented differently _step-by-step_. Merging the drivers starts with adding support for DT - that is what I provided. You know the HW better than me, why don't you start picking features from your driver and add them in rmk's driver? Sebastian
On Sat, 18 May 2013 20:20:00 +0200 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote: > I put nothing to garbage. _You_ also agreed to merge with rmk's driver! > We can now put in all features we implemented differently > _step-by-step_. > > Merging the drivers starts with adding support for DT - that is what > I provided. You know the HW better than me, why don't you start picking > features from your driver and add them in rmk's driver? The general hardware code of both drivers is close enough for merging may be done starting from anyone. But the general layout is not: - my driver is DT driven and has one card with 2 CTRC's and 2 connectors. - Russell's is non-DT, so, with some extra code I am not aware of, with any number of cards each one with one CRTC and one connector (no, I tried it, you cannot clone a connector of one card to the connector of another card). So, for me, merging means enhance my code from Russell's, but I will not go to a non-DT kernel.
On Sat, May 18, 2013 at 07:45:02PM +0200, Jean-Francois Moine wrote: > It seems we are moving backwards: > - what about the display controller? > - how do you clone the lcd 0 output to the port B? > - what occurs when the si5351 and the tda998x are modules? I've no idea why you keep bringing that last point up. I've already told you what happens when the SI5351 is a module. It is already not a problem as I have already evidenced by my boot log. So please get that and stop repeating this same point which I've already answered, or I will start ranting at you and we will have a massive falling out. As for cloning output to the VGA port, that's just a matter of dealing with the display controller. That's _not_ difficult. > My driver had the same layout as Russell's when I proposed it to you > and when you insisted to handle the 2 LCDs and the 2 ports as one card. This seems to be a misrepresentation. So what you're saying is that your driver originally handled the two LCDs as two separate cards. That is _not_ the same as my driver. My driver handles the two LCDs as two separate CRTs of the same DRM device. This allows X to drive the two CRTs together in any manner it desires depending on the capabilities of the "connectors" associated with each CRTC and the user preferences. > I spent 2 months to have a nice design and you put it to garbage! > I am not happy... Stop that right now. If you want to start whinging about the amount of time you've spent on this, then I can tell you now that if you have only spent two months on this, you are a total newbie to this and your effort is utterly insignificant compared to mine. And you *have* touched a nerve here by making that statement.
On Sat, May 18, 2013 at 09:18:46PM +0200, Jean-Francois Moine wrote: > The general hardware code of both drivers is close enough for merging > may be done starting from anyone. But the general layout is not: > > - my driver is DT driven and has one card with 2 CTRC's and 2 connectors. > > - Russell's is non-DT, so, with some extra code I am not aware of, with > any number of cards each one with one CRTC and one connector (no, I Utter shit, and I've told you before. I'm not going to repeat it again, and I warned you that we would have a falling out. As you seem to be immune to my comments provided in a sane manner, I'm now just going to ignore you in the future because you're clearly not bothering to read anything I'm telling you. That makes further discussion with you utterly pointless. > tried it, you cannot clone a connector of one card to the connector > of another card). > > So, for me, merging means enhance my code from Russell's, but I will > not go to a non-DT kernel. You seem to see DT setups vs non-DT setups as two entirely separate things. They are not. We have plenty of drivers which work in both setups just fine. So really all of your points above are utter trash to me - really. As to illustrate the point, my TDA19988 backend is now _just_ a slave encoder backend, it has nothing TDA19988 specific, nor does it have anything cubox specific there; the cubox specific part has been moved up into dove_drv.c as pure data, and that data can be constructed either from platform data or from OF properties parsed by dove_drv.c. But why did I just bother to type that. You're just going to ignore it and keep repeating your same old claptrap.
diff --git a/drivers/gpu/drm/dove/Kconfig b/drivers/gpu/drm/dove/Kconfig index 718d3c5..a943ea5 100644 --- a/drivers/gpu/drm/dove/Kconfig +++ b/drivers/gpu/drm/dove/Kconfig @@ -28,4 +28,8 @@ config DRM_DOVE_TDA1998X config DRM_DOVE_CURSOR bool "Enable Dove DRM hardware cursor support" +config DRM_DOVE_OF + bool "Enable Dove DRM OF video card" + depends on OF + endif diff --git a/drivers/gpu/drm/dove/Makefile b/drivers/gpu/drm/dove/Makefile index 65c701e..f0b6eed 100644 --- a/drivers/gpu/drm/dove/Makefile +++ b/drivers/gpu/drm/dove/Makefile @@ -5,5 +5,6 @@ dove-y := dove_crtc.o dove_drv.o dove_fb.o dove_fbdev.o \ dove-$(CONFIG_DEBUG_FS) += dove_debugfs.o dove-$(CONFIG_DRM_DOVE_TDA1998X) += dove_tda19988.o +dove-$(CONFIG_DRM_DOVE_OF) += dove_card.o obj-$(CONFIG_DRM_DOVE) := dove.o diff --git a/drivers/gpu/drm/dove/dove_card.c b/drivers/gpu/drm/dove/dove_card.c new file mode 100644 index 0000000..e4bcb5b --- /dev/null +++ b/drivers/gpu/drm/dove/dove_card.c @@ -0,0 +1,110 @@ +/* + * Copyright (C) 2013 + * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.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. + */ +#include <linux/clk.h> +#include <linux/clkdev.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> + +#define DOVE_LCD0_BASE 0x20000 +#define DOVE_LCD1_BASE 0x10000 + +static struct resource dove_drm_resources[5]; +static struct platform_device dove_drm_platform_device = { + .name = "dove-drm", + .id = 0, + .dev = { .coherent_dma_mask = ~0, }, + .resource = dove_drm_resources, +}; + +static int dove_card_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct device_node *lcdnp; + struct resource *res = dove_drm_resources; + int ret, n = 0, crtcs = 0; + + /* get video memory resource */ + if (of_address_to_resource(np, 0, &res[n++])) { + dev_err(&pdev->dev, "invalid or missing video memory\n"); + return -EINVAL; + } + + /* get reg and irq resource from each enabled lcdc */ + for_each_compatible_node(lcdnp, NULL, "marvell,dove-lcd") { + struct clk_lookup *cl; + struct clk *clk; + int lcd; + + if (!of_device_is_available(lcdnp)) + continue; + + ret = of_address_to_resource(lcdnp, 0, &res[n]); + if (ret) + return ret; + lcd = ((res[n].start & 0xfffff) == DOVE_LCD1_BASE); + n++; + + ret = of_irq_to_resource(lcdnp, 0, &res[n]); + if (ret < 0) + return ret; + n++; + + crtcs++; + + clk = clk_get(&pdev->dev, NULL); + if (IS_ERR(clk)) { + ret = PTR_ERR(clk); + if (ret == -ENOENT) + return -EPROBE_DEFER; + return ret; + } + + /* add clock alias for dovefb.0 */ + cl = clkdev_alloc(clk, "extclk", "dovefb.0"); + if (cl) + clkdev_add(cl); + clk_put(clk); + } + + if (!crtcs) + return -ENODEV; + + dove_drm_platform_device.num_resources = n; + ret = platform_device_register(&dove_drm_platform_device); + if (ret) { + dev_err(&pdev->dev, "unable to register drm device\n"); + return ret; + } + + return 0; +} + +static const struct of_device_id dove_card_of_ids[] = { + { .compatible = "marvell,dove-video-card", }, + { } +}; +MODULE_DEVICE_TABLE(of, dove_card_of_ids); + +static struct platform_driver dove_card_driver = { + .probe = dove_card_probe, + .driver = { + .owner = THIS_MODULE, + .name = "dove-drm-card", + .of_match_table = of_match_ptr(dove_card_of_ids), + }, +}; +module_platform_driver(dove_card_driver); + +MODULE_AUTHOR("Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>"); +MODULE_DESCRIPTION("Dove DRM Graphics Card"); +MODULE_LICENSE("GPL");
This adds OF support for the Dove DRM driver recently posted as RFC by Russell King. Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> --- Cc: Russell King <linux@arm.linux.org.uk> Cc: linux-arm-kernel@lists.infradead.org Cc: dri-devel@lists.freedesktop.org Cc: Jason Cooper <jason@lakedaemon.net> Cc: Jean-Francois Moine <moinejf@free.fr> --- drivers/gpu/drm/dove/Kconfig | 4 ++ drivers/gpu/drm/dove/Makefile | 1 + drivers/gpu/drm/dove/dove_card.c | 110 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+) create mode 100644 drivers/gpu/drm/dove/dove_card.c