Re: [PATCH 0/2] make imx hdmi publicly used by dw hdmi compatible platform
On Tue, Nov 04, 2014 at 09:33:10PM +0800, Andy Yan wrote: From: Andy yan andy@rock-chips.com We found freescale imx6 and rockchip rk3288 and Ingenic JZ4780 (Xburst/MIPS) use the interface compatible Designware HDMI IP, but they also have some lightly difference, such as phy pll configuration, register width(imx hdmi register is one byte, but rk3288 is 4 bytes width and can only access by word), 4K support(imx6 doesn't support 4k, but rk3288 does). To reuse the imx-hdmi driver, we do this patch set: patch (1): split out imx-soc code from imx-hdmi to dw_hdmi-imx.c patch (2): move imx-hdmi to bridge/, and rename to dw-hdmi to make this driver indepent of drm-imx . And we will add rockchip platform specific code dw_hdmi-rockchip.c later, this is depend on drm-rockchip. Great - I fully agree that this needs to be renamed as it is a Designware IP. Should it be moved into bridge/ ? It isn't implemented as a DRM bridge driver at present, so this seems illogical. Is the longer term plan to convert it to be a DRM bridge? Secondly, if you want HDMI audio support, you may find the patches I maintain for the SolidRun devices useful, which add this as a standard ALSA device. I also have CEC support for it as well. If you're interested, I'll email those. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] make imx hdmi publicly used by dw hdmi compatible platform
On Thu, Nov 06, 2014 at 05:35:40PM +0800, Kuankuan.Yang wrote: I'm working on Designware hdmi-audio, also add it as a standard ALSA device. Before I saw this email, I also planed to submit my patchs to upsteam. I'm very grateful if you can email those patchs to us. I've attached the set of patches - they're part of a much bigger patch set for supporting the Cubox-i in mainline, but should apply cleanly. They're against a 3.17 base rather than 3.18-rc, but I do have these rebased as 3.18-rc2 based patches too. I've been waiting for imx-drm to move out of drivers/staging before deciding where they should live - there seems to be no good place in the sound/ subtree to place these (sound/soc is not the right place.) They're being used not only with the SolidRun Cubox-i and Hummingboard, but also the Novena project too. One thing the audio part does not yet support is using SDMA on iMX6 to feed updates to the audio DMA, so this driver should work with other non-iMX6 SDMA. Patches 21 to 23 are various attempts to try and fix a problem I've noticed only on certain iMX6 SoCs (two different revisions of the DW IP are used in iMX6 depending on whether it's the solo/dual-lite parts, or the dual/quad parts.) Inspite of the published errata, I found that the given workarounds had no useful effect on the problem, and like most bought-in IP, it's extremely difficult to resolve these kinds of issues from an open source perspective without having commercial links with manufacturers to gain access to internal documentation and/or support. The CEC bits provide a mostly compatible interface with the current FSL iMX BSP trees, but with a different structure to it, and hopefully less buggily than the FSL driver. There has been an effort to add support for the FSL interface to libcec, but when I've looked at the library, I've never been impressed by the code, nor by the authors handling of anything but a clean transmission (which IMHO makes the whole thing unsafe, especially if you have multiple devices on the CEC bus, you need the logical ID arbitration to work properly.) -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. From: Russell King rmk+ker...@arm.linux.org.uk Subject: [PATCH 018/107] dw-hdmi-audio: add audio driver MIME-Version: 1.0 Content-Disposition: inline Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=utf-8 Add ALSA based HDMI audio driver for imx-hdmi. The imx-hdmi is a Synopsis DesignWare module, so let's name it after that. The only buffer format supported is its own special IEC958 based format, which is not compatible with any ALSA format. To avoid doing too much data manipulation within the driver, we support only ALSAs IEC958 LE, and 24-bit PCM formats for 2 to 6 channels. This allows us to modify the buffer in place as each period is passed for DMA without needing a separate buffer. A more desirable solution would be to have this conversion in userspace, but ALSA does not appear to allow such transformations outside of libasound itself. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk --- drivers/staging/imx-drm/Kconfig | 8 + drivers/staging/imx-drm/Makefile| 1 + drivers/staging/imx-drm/dw-hdmi-audio.c | 547 drivers/staging/imx-drm/dw-hdmi-audio.h | 14 + drivers/staging/imx-drm/imx-hdmi.c | 29 ++ 5 files changed, 599 insertions(+) create mode 100644 drivers/staging/imx-drm/dw-hdmi-audio.c create mode 100644 drivers/staging/imx-drm/dw-hdmi-audio.h diff --git a/drivers/staging/imx-drm/Kconfig b/drivers/staging/imx-drm/Kconfig index 82fb758a29bc..008b544b9911 100644 --- a/drivers/staging/imx-drm/Kconfig +++ b/drivers/staging/imx-drm/Kconfig @@ -51,3 +51,11 @@ config DRM_IMX_HDMI depends on DRM_IMX help Choose this if you want to use HDMI on i.MX6. + +config DRM_DW_HDMI_AUDIO + tristate Synopsis Designware Audio interface + depends on DRM_IMX_HDMI != n + help + Support the Audio interface which is part of the Synopsis + Designware HDMI block. This is used in conjunction with + the i.MX HDMI driver. diff --git a/drivers/staging/imx-drm/Makefile b/drivers/staging/imx-drm/Makefile index 582c438d8cbd..07e65a410f8f 100644 --- a/drivers/staging/imx-drm/Makefile +++ b/drivers/staging/imx-drm/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o imx-ipuv3-crtc-objs := ipuv3-crtc.o ipuv3-plane.o obj-$(CONFIG_DRM_IMX_IPUV3)+= imx-ipuv3-crtc.o obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o +obj-$(CONFIG_DRM_DW_HDMI_AUDIO) += dw-hdmi-audio.o diff --git a/drivers/staging/imx-drm/dw-hdmi-audio.c b/drivers/staging/imx-drm/dw-hdmi-audio.c new file mode 100644 index ..4f9790dea6db --- /dev/null +++ b/drivers/staging/imx-drm/dw-hdmi-audio.c @@ -0,0 +1,547 @@ +/* + * DesignWare HDMI audio driver + * + * This program is free software; you can redistribute it and/or modify +
Re: [PATCH v10 05/11] drm: bridge/dw_hdmi:split some phy configuration to platform driver
On Sat, Nov 15, 2014 at 06:07:50PM +0800, Daniel Kurtz wrote: On Fri, Nov 14, 2014 at 7:13 PM, Zubair Lutfullah Kakakhel zubair.kakak...@imgtec.com wrote: On 14/11/14 11:08, Andy Yan wrote: On 2014年11月14日 18:55, Zubair Lutfullah Kakakhel wrote: On 14/11/14 10:53, Andy Yan wrote: Hi ZubairLK: Thanks for your review. On 2014年11月14日 18:19, Zubair Lutfullah Kakakhel wrote: Hi Andy, Nice work on this patch series. Its getting better and better :). On 14/11/14 03:27, Andy Yan wrote: hdmi phy clock symbol and transmission termination value can adjust platform specific to get the best SI ^Is this signal integrity? yes , SI is signal integrity, such as eye diagram measurement Are these two disjoint features in separate patches? also add mode_valid interface for some platform may not support all the display mode Sounds like another separate patch to me. :) they can seperate Also, This series is becoming quite large. With major changes and fixes mixed together. Patch 3 splits imx-drm. Patch 4 moves dw-drm out of imx-drm folder. Patch 7 adds binding Patch 9 converts to drm bridge. Can these be placed together easily? And in the start. i.e. patch 1, 2, 3, 4, Then all fixes etc can come afterwards? It helps when checking histories later as to how a driver was made and how fixes happened. Especially when file moves happen.. Do you mean we can rearrange the patch series? put patch 3, 4 ,7, 9 together one bye one than followed by the fixes patches 5 ,6, 8, 11 ? Yes. Rearrange so that the split imx-drm/imx-hdmi and conversion to drm-bridge is at the start of the series. Then the rest are bug fixes and feature additions. Can I put patch#1(make checkpatch happy) and patch#2 (defer probe) as the first two patch. Daniel from Google chromium think it's better to put the two slightly changes in the front for easy review. Sorry, I didn't see this conversation before my earlier emails. I was suggesting to Andy to arrange his patches like this: (1) fixes that directly apply to imx-drm as it is today. (2) split out the generic dw_hdmi parts from imx-hdmi into dw_hdmi.c (3) convert dw_hdmi.c to a drm_bridge (4) move the dw_hdmi.c bridge implementation to drm/bridge (5) modifications to dw_hdmi.c required to support hdmi on rk3288 (5) add rk3288-hdmi.c to drm/rockchip (at least whenever drm/rockchip lands) The idea being that we can start landing the patches for (1) even while we are still debating / reviewing (2)+ upstream. Sure. I am not the maintainer. They have to make the final decision. imx-drm is still in staging. I do not see anyone listed explicitly under MAINTAINERS for drivers/staging/imx-drm, so by default I guess it falls back to gregkh (drivers/staging/)? The Commit field in git log seems to back this up. Although, based on Author, I think we also want the opinions of Philipp Zabel and Russel King. Once the wranglings on the patch series are complete, I do intend to test it on the platforms I have - and remember that I do have the ALSA based audio and CEC bits as well, some of which will probably need a little bit of re-work. All in all, I welcome the renaming of this to include a reference to DesignWare - I've always thought it's a mistake that the HDMI interface in iMX6 was not named with a dw prefix as the docs contain references to it being a DesignWare IP module. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v10 05/11] drm: bridge/dw_hdmi:split some phy configuration to platform driver
On Sat, Nov 15, 2014 at 10:12:18AM +, Russell King - ARM Linux wrote: Once the wranglings on the patch series are complete, I do intend to test it on the platforms I have - and remember that I do have the ALSA based audio and CEC bits as well, some of which will probably need a little bit of re-work. All in all, I welcome the renaming of this to include a reference to DesignWare - I've always thought it's a mistake that the HDMI interface in iMX6 was not named with a dw prefix as the docs contain references to it being a DesignWare IP module. One thing I would ask is that the subsequent submissions do not thread onto the previous submission. It may seem a good idea (people claim that it allows the previous reviews to be trivially found) but these people forget an important side effect from this behaviour - when looking at the message index in a threaded mail reader (like mutt), each reply to a thread moves the subject line by three characters to the right. What this means is that after about five or six iterations of the submission, there is no longer any subject line visible. Moreover, it means that with lesser iterations, it becomes much more difficult to see /any/ of the review thread structure. I would suggest that if you do want to connect the subsequent submissions, please use the same reference message for each submission. In other words, rather than: v1 0/2 +- v1 1/2 +- v1 2/2 +- v2 0/2 +- v2 1/2 +- v2 2/2 +- v3 0/2 +- v3 1/2 +- v3 2/2 ... This is done instead: v1 0/2 +- v1 1/2 +- v1 2/2 +- v2 0/2 | +- v2 1/2 | +- v2 2/2 +- v3 0/2 | +- v3 1/2 | +- v3 2/2 ... which is a compromise between threading the messages together, and keeping stopping the thread pushing the subject line completely off the right hand side of the screen. In this case, I'd suggest a reference of: 1415793593-5075-1-git-send-email-andy@rock-chips.com which is the v8 covering message which started this big thread. Thanks. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
On Wed, Dec 03, 2014 at 11:29:26PM +0800, Andy Yan wrote:
+int imx_hdmi_bind(struct device *dev, struct device *master,
+ void *data, struct drm_encoder *encoder,
+ const struct imx_hdmi_plat_data *plat_data)
{
struct platform_device *pdev = to_platform_device(dev);
- const struct of_device_id *of_id =
- of_match_device(imx_hdmi_dt_ids, dev);
struct drm_device *drm = data;
struct device_node *np = dev-of_node;
struct device_node *ddc_node;
@@ -1594,19 +1566,16 @@ static int imx_hdmi_bind(struct device *dev, struct
device *master, void *data)
struct resource *iores;
int ret, irq;
- hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
+ hdmi = devm_kzalloc(pdev-dev, sizeof(*hdmi), GFP_KERNEL);
if (!hdmi)
return -ENOMEM;
- hdmi-dev = dev;
+ hdmi-plat_data = plat_data;
+ hdmi-dev = pdev-dev;
+ hdmi-dev_type = plat_data-dev_type;
hdmi-sample_rate = 48000;
hdmi-ratio = 100;
-
- if (of_id) {
- const struct platform_device_id *device_id = of_id-data;
-
- hdmi-dev_type = device_id-driver_data;
- }
+ hdmi-encoder = encoder;
I'd suggest changing imx_hdmi_bind() to take the struct resource and irq
number, and avoiding the platform device stuff altogether in here.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v16 05/12] drm: imx: imx-hdmi: move imx-hdmi to bridge/dw_hdmi
On Wed, Dec 03, 2014 at 11:32:12PM +0800, Andy Yan wrote: diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 884923f..26162ef 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -3,3 +3,8 @@ config DRM_PTN3460 depends on DRM select DRM_KMS_HELPER ---help--- + +config DRM_DW_HDMI + bool Synopsys DesignWare High-Definition Multimedia Interface + depends on DRM + select DRM_KMS_HELPER ... diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig index 82fb758..7070077 100644 --- a/drivers/gpu/drm/imx/Kconfig +++ b/drivers/gpu/drm/imx/Kconfig @@ -48,6 +48,7 @@ config DRM_IMX_IPUV3 config DRM_IMX_HDMI tristate Freescale i.MX DRM HDMI + select DRM_DW_HDMI depends on DRM_IMX help Choose this if you want to use HDMI on i.MX6. I'd recommend that if you want to select DRM_DW_HDMI, then don't give DRM_DW_HDMI a prompt message. I assume you're going to do something similar with your Rockchip driver too - in which case DRM_DW_HDMI is really about building a library module. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v16 05/12] drm: imx: imx-hdmi: move imx-hdmi to bridge/dw_hdmi
On Thu, Dec 04, 2014 at 12:01:25AM +0800, Andy Yan wrote: Hi Russell: Do you mean I just neet to do like bellow? + +config DRM_DW_HDMI + bool + depends on DRM + select DRM_KMS_HELPER Yep. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
On Thu, Dec 04, 2014 at 12:04:37AM +0800, Andy Yan wrote:
Hi Russell:
On 2014年12月03日 23:38, Russell King - ARM Linux wrote:
On Wed, Dec 03, 2014 at 11:29:26PM +0800, Andy Yan wrote:
+int imx_hdmi_bind(struct device *dev, struct device *master,
+ void *data, struct drm_encoder *encoder,
+ const struct imx_hdmi_plat_data *plat_data)
{
struct platform_device *pdev = to_platform_device(dev);
- const struct of_device_id *of_id =
- of_match_device(imx_hdmi_dt_ids, dev);
struct drm_device *drm = data;
struct device_node *np = dev-of_node;
struct device_node *ddc_node;
@@ -1594,19 +1566,16 @@ static int imx_hdmi_bind(struct device *dev, struct
device *master, void *data)
struct resource *iores;
int ret, irq;
- hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
+ hdmi = devm_kzalloc(pdev-dev, sizeof(*hdmi), GFP_KERNEL);
if (!hdmi)
return -ENOMEM;
- hdmi-dev = dev;
+ hdmi-plat_data = plat_data;
+ hdmi-dev = pdev-dev;
+ hdmi-dev_type = plat_data-dev_type;
hdmi-sample_rate = 48000;
hdmi-ratio = 100;
-
- if (of_id) {
- const struct platform_device_id *device_id = of_id-data;
-
- hdmi-dev_type = device_id-driver_data;
- }
+ hdmi-encoder = encoder;
I'd suggest changing imx_hdmi_bind() to take the struct resource and irq
number, and avoiding the platform device stuff altogether in here.
Actually this is what the current code do: the resource and irq number
are all handled in imx_hdmi_bind
I meant that imx_hdmi_bind should be passed these, so that it needs to
know nothing about the struct device beyond the generic device structure.
In other words, the dw-hdmi core should not assume that the struct device
is part of a platform device.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
On Wed, Dec 03, 2014 at 05:20:15PM +0100, Philipp Zabel wrote: Hi Andy, It would be better if the bind function would not have to care about platform resources, that should be handled in the probe function. I had a patch to move them: http://lists.freedesktop.org/archives/dri-devel/2014-May/059630.html Maybe you could incorporate something like this? Personally, I hate this idea. Having a two-layered setup means that the when the bind() method is called, the state of struct imx_hdmi is indeterminant. If it's called immediately from probe, most of the structure will be zeroed, and only those members initialised by the probe function will be set to non-zero values. However, if the HDMI interface has been previously bound, and is subsequently re-bound, then the structure will most definitely no longer be in a known state on the second bind() call. This is fragile. Now, people have tried to tell me that this isn't fragile, but, I now have proof that it is as fragile as I fear. The component helper doesn't yet have that many users, and already we have one user (okay, it's not part of the mainline kernel - it's etnaviv) which contained exactly this kind of bug: it expected its private structures to be zeroed on the bind() call. So, I /really/ hate this idea. If you really want to do this, then please ensure that the bind() call explicitly zeros the bits of the struct which aren't initialised by the probe() call, so we know that the driver will always start off with a known initial state. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
On Thu, Dec 04, 2014 at 12:30:23AM +0800, Andy Yan wrote: On 2014年12月04日 00:11, Russell King - ARM Linux wrote: I meant that imx_hdmi_bind should be passed these, so that it needs to know nothing about the struct device beyond the generic device structure. In other words, the dw-hdmi core should not assume that the struct device is part of a platform device. if so, how about the device tree properties ddc-i2c-bus, reg-io-width, iahb, isfr, they are all found by device? If the device has a device tree node associated with it, it will have a non-NULL dev-of_node - which is part of the generic device structure. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
On Thu, Dec 04, 2014 at 12:56:24AM +0800, Andy Yan wrote:
Hi Russell:
On 2014年12月04日 00:33, Russell King - ARM Linux wrote:
On Thu, Dec 04, 2014 at 12:30:23AM +0800, Andy Yan wrote:
On 2014年12月04日 00:11, Russell King - ARM Linux wrote:
I meant that imx_hdmi_bind should be passed these, so that it needs to
know nothing about the struct device beyond the generic device structure.
In other words, the dw-hdmi core should not assume that the struct device
is part of a platform device.
if so, how about the device tree properties ddc-i2c-bus, reg-io-width,
iahb, isfr,
they are all found by device?
If the device has a device tree node associated with it, it will have a
non-NULL dev-of_node - which is part of the generic device structure.
so , I just need get the resource and irq number in the
dw_hdmi-imx/rockchip ,than
pass them to imx_hdmi_bind, as the properties ddc-i2c-bus, reg-io-width,
iahb,isfr, they
are still can be handled in imx_hdmi_bind ?
Basically, what I'm suggesting is just this change to imx_hdmi_bind():
int imx_hdmi_bind(struct device *dev, struct device *master,
void *data, struct drm_encoder *encoder,
+ const struct resource *iores, int irq,
const struct imx_hdmi_plat_data *plat_data)
{
- struct platform_device *pdev = to_platform_device(dev);
...
}
- irq = platform_get_irq(pdev, 0);
if (irq 0)
return irq;
...
return ret;
- iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hdmi-regs = devm_ioremap_resource(dev, iores);
if (IS_ERR(hdmi-regs))
and supplying those as arguments from the caller.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
On Thu, Dec 04, 2014 at 09:40:10AM +0100, Philipp Zabel wrote: You are right, no I don't want this. When I initially wrote this patch I was under the impression that the memory allocated by devm_kzalloc in bind() wouldn't be freed on unbind(). Resources claimed inside bind() will be freed in unbind(). Resources claimed in the driver's probe() will be freed in driver's remove(). They nest, and each is properly dealt with at the appropriate time due to... I remember I stopped pursuing this patch when I got aware of the devres_open/close/remove_group dance in the component framework code, but somehow forgot to drop it altogether locally. ... the use of devres grouping. So, if you use devm_kzalloc() in the driver's probe() function, then that memory will be freed after the driver's remove() function is called. That's fine. The point I was making is: probe() mem = devm_kzalloc(); mem-mmio = ...; ... bind() mem-mmio is set other members of mem are zero ... unbind() ... bind() mem-mmio is set other members of mem are indeterminant ... unbind() ... remove() mem will be freed automatically That's where the problem happens - the second time the bind() function gets called: you might as well not use devm_k*z*alloc() initially, because having the structure initialised to zero _could_ very well hide bugs. When you consider that it's not just the driver code which you have to audit, but also any code the driver calls (eg, because you've embedded some subsystem's struct in your driver private data) it quickly becomes very easy for a bug to creep in here. If we want to go down the route of having the probe function deal with resources etc, then I would recommend against using devm_kzalloc() to allocate the private structure: use devm_kmalloc() instead, which will leave the memory uninitialised. That means you get the same condition on each bind(), which means you have to think more about how to ensure that the initial state of members is dealt with throughout your driver. Alternatively, separate the struct into two sections: one which contains everything initialised by the probe() function, and everything else, and arrange for everything else to get memset() on entry to bind(). -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3 1/8] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*
On Wed, Nov 13, 2013 at 08:53:18AM +0100, Eric Bénard wrote: Hi Russell, Le Tue, 12 Nov 2013 17:04:55 +, Russell King - ARM Linux li...@arm.linux.org.uk a écrit : On Tue, Nov 12, 2013 at 05:49:18PM +0100, Denis Carikli wrote: diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index fc2adb6..586c12f 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -537,6 +537,15 @@ int drm_display_mode_from_videomode(const struct videomode *vm, dmode-flags |= DRM_MODE_FLAG_DBLSCAN; if (vm-flags DISPLAY_FLAGS_DOUBLECLK) dmode-flags |= DRM_MODE_FLAG_DBLCLK; + if (vm-flags DISPLAY_FLAGS_DE_LOW) + dmode-flags |= DRM_MODE_FLAG_DE_LOW; + if (vm-flags DISPLAY_FLAGS_DE_HIGH) + dmode-flags |= DRM_MODE_FLAG_DE_HIGH; + if (vm-flags DISPLAY_FLAGS_PIXDATA_POSEDGE) + dmode-flags |= DRM_MODE_FLAG_PIXDATA_POSEDGE; + if (vm-flags DISPLAY_FLAGS_PIXDATA_NEGEDGE) + dmode-flags |= DRM_MODE_FLAG_PIXDATA_NEGEDGE; + I'm still not convinced that these should be exposed in *any* way to userspace - I'm with Ville Syrjälä on this point. Yes, you've moved their definition out of a uapi header file, but they're still leaking out of kernel space via calls (and with Xorg, they'll leak into the DisplayMode structures within the X server.) Now, here's the thing... The polarity of the display enable signal. That's a property of the connected device right? It doesn't change with respect to the displayed mode unlike the hsync/vsync signals. If that's true, it should not be here. Same goes for the pixel clock edge. If it's a property of the connected device and doesn't have a dependence on the displayed mode, then it should not be in the DRM mode structure. What would be the right way to configure these settings without exposing them to userspace ? As explained in my answer to Fabio, these settings are currently hardcoded into ipuv3-crtc and we need to configure them to support more TFT panels using the IPUV3 Parallel Display Interface. First, realise that what you're doing is configuring components within the IMX driver suite so what you need to do is communicate your requirements _only_ from parallel-display.c to ipuv3-crtc.c. There's already infrastructure in imx-drm for the display bridges to communicate various information to the CRTC layer - there's already the encoder type, and the pins for hsync/vsync being communicated via imx_drm_panel_format*(). This is really no different IMHO. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display.
On Wed, Nov 13, 2013 at 11:43:44AM -0700, Troy Kisky wrote: On 11/13/2013 2:23 AM, Denis Carikli wrote: + /* rgb666 */ +ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666); +ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */ +ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */ +ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */ + return 0; } Since, rgb24 and bgr24 reverse the byte numbers /* rgb24 */ ipu_dc_map_clear(priv, IPU_DC_MAP_RGB24); ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 0, 7, 0xff); /* blue */ ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 1, 15, 0xff); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 2, 23, 0xff); /* red */ /* bgr24 */ ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24); ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 2, 7, 0xff); /* red */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */ Shouldn't rgb666 and bgr666 do the same? Currently we have, /* bgr666 */ ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666); ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */ Yes, I concur - this doesn't make sense to me. BGR666 would mean in memory: 111 721650 BBGGRR which reflects the same order for RGB24 above. Where I'd expect to see /* bgr666 */ ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666); ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */ So this makes sense to me. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] drm: Add LCD display clock polarity flags
On Mon, Dec 02, 2013 at 04:39:26PM +0100, Marek Vasut wrote: Add DRM flags for the LCD display clock polarity so the pixelclk-active DT property can be properly handled by drivers using the DRM API. I still say that not even this should be part of the DRM mode API to userspace. The hint that you're changing the user API is that you're modifying a header file below a 'uapi' directory. The settings of double scan, sync polarity etc are all part of the display mode specification (check CEA-861 documents). Things like pixel clock polarity are not part of the mode specification, they're a property of the display itself and are independent of the mode. Therefore, they should not be part of struct drm_mode_modeinfo. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] drm: Add LCD display clock polarity flags
On Tue, Dec 03, 2013 at 07:44:52PM +0800, Shawn Guo wrote: On Mon, Dec 02, 2013 at 04:39:26PM +0100, Marek Vasut wrote: Add DRM flags for the LCD display clock polarity so the pixelclk-active DT property can be properly handled by drivers using the DRM API. Signed-off-by: Marek Vasut ma...@denx.de Cc: Dave Airlie airl...@gmail.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Philipp Zabel p.za...@pengutronix.de Cc: Sascha Hauer s.ha...@pengutronix.de Cc: Shawn Guo shawn@linaro.org --- drivers/gpu/drm/drm_modes.c | 5 + include/uapi/drm/drm_mode.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 85071a1..d1f3bfc 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -537,6 +537,11 @@ int drm_display_mode_from_videomode(const struct videomode *vm, dmode-flags |= DRM_MODE_FLAG_DBLSCAN; if (vm-flags DISPLAY_FLAGS_DOUBLECLK) dmode-flags |= DRM_MODE_FLAG_DBLCLK; + if (vm-flags DISPLAY_FLAGS_PIXDATA_POSEDGE) + dmode-flags |= DRM_MODE_FLAG_PIXELCLK_PPOL; + else if (vm-flags DISPLAY_FLAGS_PIXDATA_NEGEDGE) + dmode-flags |= DRM_MODE_FLAG_PIXELCLK_NPOL; + drm_mode_set_name(dmode); return 0; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index f104c26..a6169ca 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -73,6 +73,9 @@ #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (714) #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF(814) +/* CRTC LCD clock polarity flags. */ +#define DRM_MODE_FLAG_PIXELCLK_PPOL(119) +#define DRM_MODE_FLAG_PIXELCLK_NPOL(120) Marek, It looks that Denis (copied) is working on the same problem, so you may want to be aware of his effort [1][2]. Indeed, and I made very similar comments in that thread. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] Fix some bugs/races in imx-drm
The following patch series fixes some bugs and races in the imx-drm code, which should probably be applied to an -rc kernel. drivers/staging/imx-drm/imx-drm-core.c | 21 - drivers/staging/imx-drm/ipu-v3/ipu-common.c | 32 +- drivers/staging/imx-drm/ipuv3-plane.c | 10 3 files changed, 41 insertions(+), 22 deletions(-) Russell King (4): imx-drm: imx-drm-core: fix error cleanup path for imx_drm_add_crtc() imx-drm: imx-drm-core: fix DRM cleanup paths imx-drm: fix missing symbol exports imx-drm: ipu-v3: fix potential CRTC device registration race ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] Fix some bugs/races in imx-drm
On Mon, Dec 16, 2013 at 11:33:01AM +, Russell King - ARM Linux wrote: The following patch series fixes some bugs and races in the imx-drm code, which should probably be applied to an -rc kernel. drivers/staging/imx-drm/imx-drm-core.c | 21 - drivers/staging/imx-drm/ipu-v3/ipu-common.c | 32 +- drivers/staging/imx-drm/ipuv3-plane.c | 10 3 files changed, 41 insertions(+), 22 deletions(-) Russell King (4): imx-drm: imx-drm-core: fix error cleanup path for imx_drm_add_crtc() imx-drm: imx-drm-core: fix DRM cleanup paths imx-drm: fix missing symbol exports imx-drm: ipu-v3: fix potential CRTC device registration race I've decided to add four more patches to this set from my 50+ patch set for this driver. The new four come after the above four, so I'll just append them to the thread. New diffstat/shortlog: drivers/staging/imx-drm/imx-drm-core.c | 39 --- drivers/staging/imx-drm/imx-tve.c |9 -- drivers/staging/imx-drm/ipu-v3/ipu-common.c | 32 +++--- drivers/staging/imx-drm/ipuv3-plane.c | 10 +++ 4 files changed, 55 insertions(+), 35 deletions(-) Russell King (8): imx-drm: imx-drm-core: fix error cleanup path for imx_drm_add_crtc() imx-drm: imx-drm-core: fix DRM cleanup paths imx-drm: fix missing symbol exports imx-drm: ipu-v3: fix potential CRTC device registration race imx-drm: imx-tve: don't call sleeping functions beneath enable_lock spinlo imx-drm: imx-drm-core: use defined constant for number of CRTCs. imx-drm: imx-drm-core: make imx_drm_crtc_register() safer imx-drm: imx-drm-core: improve safety of imx_drm_add_crtc() ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] imx-drm: fix missing symbol exports
On Tue, Dec 17, 2013 at 12:28:37PM +0800, Shawn Guo wrote: On Mon, Dec 16, 2013 at 11:34:05AM +, Russell King wrote: Trying to build a modular imx-drm results in a number of missing symbol exports, caused by the recent changes to this driver. Add the necessary exports, and the missing MODULE_LICENSE() tag. Since commit 9c74360 (staging: imx-drm: Fix modular build of DRM_IMX_IPUV3) is in place now, I'm not sure if we still need this patch. As that patch has already been merged, I'll drop this one. Thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] Fix some bugs/races in imx-drm
On Tue, Dec 17, 2013 at 01:04:20PM +0800, Shawn Guo wrote: On Mon, Dec 16, 2013 at 12:38:23PM +, Russell King - ARM Linux wrote: Russell King (8): imx-drm: imx-drm-core: fix error cleanup path for imx_drm_add_crtc() imx-drm: imx-drm-core: fix DRM cleanup paths imx-drm: fix missing symbol exports Except the little doubt I replied on this one, for the whole series: Acked-by: Shawn Guo shawn@linaro.org Tested-by: Shawn Guo shawn@linaro.org imx-drm: ipu-v3: fix potential CRTC device registration race imx-drm: imx-tve: don't call sleeping functions beneath enable_lock spinlo imx-drm: imx-drm-core: use defined constant for number of CRTCs. imx-drm: imx-drm-core: make imx_drm_crtc_register() safer imx-drm: imx-drm-core: improve safety of imx_drm_add_crtc() Greg, Do you want me to re-send them with these acks/tested-by's added, or are you happy to take them as-is? I also have some further patches to send (I think three) which apply on top of these but are not fixes (so aren't -rc material), more cleanups. Thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] Fix some bugs/races in imx-drm
On Tue, Dec 17, 2013 at 08:05:41AM -0800, Greg Kroah-Hartman wrote: On Tue, Dec 17, 2013 at 03:17:02PM +, Russell King - ARM Linux wrote: On Tue, Dec 17, 2013 at 01:04:20PM +0800, Shawn Guo wrote: On Mon, Dec 16, 2013 at 12:38:23PM +, Russell King - ARM Linux wrote: Russell King (8): imx-drm: imx-drm-core: fix error cleanup path for imx_drm_add_crtc() imx-drm: imx-drm-core: fix DRM cleanup paths imx-drm: fix missing symbol exports Except the little doubt I replied on this one, for the whole series: Acked-by: Shawn Guo shawn@linaro.org Tested-by: Shawn Guo shawn@linaro.org imx-drm: ipu-v3: fix potential CRTC device registration race imx-drm: imx-tve: don't call sleeping functions beneath enable_lock spinlo imx-drm: imx-drm-core: use defined constant for number of CRTCs. imx-drm: imx-drm-core: make imx_drm_crtc_register() safer imx-drm: imx-drm-core: improve safety of imx_drm_add_crtc() Greg, Do you want me to re-send them with these acks/tested-by's added, or are you happy to take them as-is? I can take these as-is. Thanks. I also have some further patches to send (I think three) which apply on top of these but are not fixes (so aren't -rc material), more cleanups. Ok, feel free to send them, I can take them. They'll be along shortly. Diffstat looks smaller - just one file: drivers/staging/imx-drm/imx-drm-core.c | 55 +++ 1 files changed, 20 insertions(+), 35 deletions(-) Russell King (3): imx-drm: imx-drm-core: use the crtc drm device for vblank imx-drm: imx-drm-core: avoid going the long route round for drm_device imx-drm: imx-drm-core: merge imx_drm_crtc_register() into imx_drm_add_crtc() ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 00/46] Preview of imx-drm cleanup series
Here is my large patch series which cleans up imx-drm, and gets it ready to move out of drivers/staging. This is a preview only. One of these patches introduces a generic helper in drivers/base which can be used by any subsystem to assemble a sub-devices together and complete the probe of a subsystem when all devices are present - and tear it down when any of those devices go away - this is patch 26. Example usage (with imx-drm) is illustrated in patches 27 through to 41. Some of this duplicates Fabio's imx-drm HDMI patch; indeed some of the changes which were fed back to Fabio are here as their individual patches. I've not updated Fabio's patch in this series since he sent an updated version to Greg for merging. I've also included here support for ALSA based HDMI audio - if you omit the new HDMI drivers from this, it gives a net reduction in LoC. Finally, the last patch attempts to resolve a problem with the way imx-drm works - but this is fundamentally unsolvable without introducing new DT properties to properly specify the mux IDs. I'm only sending this to a limited number of people for comments at present since it's a large series, and the selection of people from maintainers is rather large. arch/arm/boot/dts/imx51-babbage.dts | 10 +- arch/arm/boot/dts/imx53-m53evk.dts |8 +- arch/arm/boot/dts/imx53-mba53.dts |6 + arch/arm/boot/dts/imx53-qsb.dts |8 +- arch/arm/boot/dts/imx6dl.dtsi |5 + arch/arm/boot/dts/imx6q.dtsi|5 + arch/arm/boot/dts/imx6qdl-sabresd.dtsi |6 + arch/arm/boot/dts/imx6qdl.dtsi |9 + drivers/base/Makefile |2 +- drivers/base/component.c| 379 ++ drivers/gpu/drm/drm_crtc_helper.c | 39 +- drivers/staging/imx-drm/Kconfig |6 + drivers/staging/imx-drm/Makefile|5 +- drivers/staging/imx-drm/dw-hdmi-audio.c | 550 drivers/staging/imx-drm/dw-hdmi-audio.h | 13 + drivers/staging/imx-drm/imx-drm-core.c | 827 - drivers/staging/imx-drm/imx-drm.h | 38 +- drivers/staging/imx-drm/imx-fb.c| 47 - drivers/staging/imx-drm/imx-fbdev.c | 74 -- drivers/staging/imx-drm/imx-hdmi.c | 1796 +++ drivers/staging/imx-drm/imx-hdmi.h | 1037 drivers/staging/imx-drm/imx-ldb.c | 125 +-- drivers/staging/imx-drm/imx-tve.c | 134 +- drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h |1 + drivers/staging/imx-drm/ipu-v3/ipu-common.c | 15 +- drivers/staging/imx-drm/ipu-v3/ipu-di.c | 317 ++--- drivers/staging/imx-drm/ipuv3-crtc.c| 59 +- drivers/staging/imx-drm/parallel-display.c | 100 +- include/drm/drm_crtc_helper.h |1 + include/linux/component.h | 31 + include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |1 + 31 files changed, 4519 insertions(+), 1135 deletions(-) create mode 100644 drivers/base/component.c create mode 100644 drivers/staging/imx-drm/dw-hdmi-audio.c create mode 100644 drivers/staging/imx-drm/dw-hdmi-audio.h delete mode 100644 drivers/staging/imx-drm/imx-fb.c delete mode 100644 drivers/staging/imx-drm/imx-fbdev.c create mode 100644 drivers/staging/imx-drm/imx-hdmi.c create mode 100644 drivers/staging/imx-drm/imx-hdmi.h create mode 100644 include/linux/component.h -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Thu, Jan 02, 2014 at 07:10:55PM -0800, Greg Kroah-Hartman wrote:
On Thu, Jan 02, 2014 at 09:27:58PM +, Russell King wrote:
Subsystems such as ALSA, DRM and others require a single card-level
device structure to represent a subsystem. However, firmware tends to
describe the individual devices and the connections between them.
Therefore, we need a way to gather up the individual component devices
together, and indicate when we have all the component devices.
We do this in DT by providing a superdevice node which specifies
the components, eg:
imx-drm {
compatible = fsl,drm;
crtcs = ipu1;
connectors = hdmi;
};
The superdevice is declared into the component support, along with the
subcomponents. The superdevice receives callbacks to locate the
subcomponents, and identify when all components are present. At this
point, we bind the superdevice, which causes the appropriate subsystem
to be initialised in the conventional way.
When any of the components or superdevice are removed from the system,
we unbind the superdevice, thereby taking the subsystem down.
This sounds a lot like the containers code that Rafael just submitted
and I acked for 3.14. Look at the lkml post:
Subject: [PATCH 2/2] ACPI / hotplug / driver core: Handle containers in
a special way
Message-ID: 1991202.gilw172...@vostro.rjw.lan
And see if that could possibly be used instead?
That's really disappointing bcause I've put a hell of a lot of work into
this over the last few months, and if that's true it's all just been a
total waste of my time. Okay, lesson learned - don't spend any time
trying to fix other people's problems after discussing them at
kernel-summit.
In any case, the above message ID doesn't give me access to this containers
code to look at to even evaluate whether it can be used for this - it just
gives two patches for ACPI specific patches but not the core stuff.
http://www.spinics.net/lists/linux-acpi/msg48101.html
http://www.spinics.net/lists/linux-acpi/msg48102.html
Please provide a better reference to the code you're referring to.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Fri, Jan 03, 2014 at 12:58:16PM +0100, Rafael J. Wysocki wrote:
On Friday, January 03, 2014 11:00:30 AM Russell King - ARM Linux wrote:
On Thu, Jan 02, 2014 at 07:10:55PM -0800, Greg Kroah-Hartman wrote:
On Thu, Jan 02, 2014 at 09:27:58PM +, Russell King wrote:
Subsystems such as ALSA, DRM and others require a single card-level
device structure to represent a subsystem. However, firmware tends to
describe the individual devices and the connections between them.
Therefore, we need a way to gather up the individual component devices
together, and indicate when we have all the component devices.
We do this in DT by providing a superdevice node which specifies
the components, eg:
imx-drm {
compatible = fsl,drm;
crtcs = ipu1;
connectors = hdmi;
};
The superdevice is declared into the component support, along with the
subcomponents. The superdevice receives callbacks to locate the
subcomponents, and identify when all components are present. At this
point, we bind the superdevice, which causes the appropriate subsystem
to be initialised in the conventional way.
When any of the components or superdevice are removed from the system,
we unbind the superdevice, thereby taking the subsystem down.
This sounds a lot like the containers code that Rafael just submitted
and I acked for 3.14. Look at the lkml post:
Subject: [PATCH 2/2] ACPI / hotplug / driver core: Handle containers in
a special way
Message-ID: 1991202.gilw172...@vostro.rjw.lan
And see if that could possibly be used instead?
That's really disappointing bcause I've put a hell of a lot of work into
this over the last few months, and if that's true it's all just been a
total waste of my time. Okay, lesson learned - don't spend any time
trying to fix other people's problems after discussing them at
kernel-summit.
Well, I didn't know that you were doing this work and my patch is to address
a specific problem that people are seeing in testing. Also, the generic
containers part in it is very simple and it might be possible to integrate it
with your code, this way or another. In fact, the only only thing I need from
containers at the moment is the online/offline functionality.
We had a session at kernel summit chaired by David Airlie to discuss
various issues associated with DRM which included the problems of
componentised devices registering into card-based subsystems. There
were quite a number of attendees to that session.
It is in that session that I said I would work on this, specifically
with the aim of getting imx-drm out of drivers/staging.
In any case, the above message ID doesn't give me access to this containers
code to look at to even evaluate whether it can be used for this - it just
gives two patches for ACPI specific patches but not the core stuff.
http://www.spinics.net/lists/linux-acpi/msg48101.html
http://www.spinics.net/lists/linux-acpi/msg48102.html
Please provide a better reference to the code you're referring to.
You can use the linux-next branch of the linux-pm.git tree at the moment or I
can set up a separate branch for that if that helps. The two patches above
depend on some earlier material I've gueued up for 3.14, but it's mostly
ACPI hotplug code.
I'm not sure what I'm looking for. I've tried looking at the results of
searching your linux-next branch for container but I don't see
anything implementing similar functionality to the patch I've sent.
https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/log/?h=linux-nextqt=grepq=container
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Fri, Jan 03, 2014 at 02:24:26PM +0100, Rafael J. Wysocki wrote: On Friday, January 03, 2014 12:18:13 PM Russell King - ARM Linux wrote: I'm not sure what I'm looking for. I've tried looking at the results of searching your linux-next branch for container but I don't see anything implementing similar functionality to the patch I've sent. https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/log/?h=linux-nextqt=grepq=container I've just set up the acpi-hotplug branch in linux-pm.git (because I often rebase the linux-next one). The only commit in that branch you need to look at is this one: http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=acpi-hotplugid=caa73ea158de9419f08e456f2716c71d1f06012a but quite frankly I'm not sure how it is related to your work. :-) Yes, I'm coming to that conclusion as well. It looks like your containers aren't about collecting up several individual component devices into one super-device and probing the appropriate subsystem when all components are known. Confused why Greg is pointing me at your patches. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 21/46] drm: provide a helper for the encoder possible_crtcs mask
On Fri, Jan 03, 2014 at 05:05:46PM +0100, David Herrmann wrote: Hi On Thu, Jan 2, 2014 at 10:27 PM, Russell King rmk+ker...@arm.linux.org.uk wrote: The encoder possible_crtcs mask identifies which CRTCs can be bound to a particular encoder. Each bit from bit 0 defines an index in the list of CRTCs held in the DRM mode_config crtc_list. Rather than having drivers trying to track the position of their CRTCs in the list, expose the code which already exists for calculating the appropriate mask bit for a CRTC. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk --- drivers/gpu/drm/drm_crtc_helper.c | 39 include/drm/drm_crtc_helper.h |1 + 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 01361aba033b..10708c248196 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -325,6 +325,29 @@ void drm_helper_disable_unused_functions(struct drm_device *dev) EXPORT_SYMBOL(drm_helper_disable_unused_functions); /** + * drm_helper_crtc_possible_mask - find the mask of a registered CRTC + * @crtc: crtc to find mask for + * + * Given a registered CRTC, return the mask bit of that CRTC for an + * encoder's possible_crtcs field. I think this is a nice cleanup which we could pick up right away. Most drivers can be simplified by using this. Only the name is misleading, imo, I'd use something like drm_helper_crtc_to_mask(). I'm not so sure - since the only place this mask gets used is with the possible_crtcs field. It's got nothing to do with the possible_clones field as that isn't a mask of CRTCs. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 21/46] drm: provide a helper for the encoder possible_crtcs mask
On Fri, Jan 03, 2014 at 05:26:14PM +0100, David Herrmann wrote: Hi On Fri, Jan 3, 2014 at 5:13 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: I'm not so sure - since the only place this mask gets used is with the possible_crtcs field. It's got nothing to do with the possible_clones field as that isn't a mask of CRTCs. ... possible_clones is about encoders, you're right, I missed that. So you can carry it in your series just fine. Thanks for confirming that - it's something which imx-drm seemed to get wrong as put the same value into possible_clones as it did for possible_crtcs. I was fairly certain that was buggy. :) -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 27/46] imx-drm: convert to componentised device support
On Fri, Jan 03, 2014 at 05:48:57PM +0100, Philipp Zabel wrote: Hi Russell, I've tested this series on a BD-SL (SabreLite) with HDMI. Right now the HPD signal doesn't seem to work, but after overwriting the connection check, I got a stable and correct picture on the monitor: Hmm. Does this mean you're not getting any IRQs from the HDMI interface when you plug and unplug the monitor? 147:281 GIC 147 12.hdmi on turning the TV on: 147:282 GIC 147 12.hdmi on unplugging the HDMI cable: 147:283 GIC 147 12.hdmi on replugging the HDMI cable: 147:284 GIC 147 12.hdmi -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 27/46] imx-drm: convert to componentised device support
On Fri, Jan 03, 2014 at 06:26:44PM +0100, Philipp Zabel wrote: Am Freitag, den 03.01.2014, 17:07 + schrieb Russell King - ARM Linux: On Fri, Jan 03, 2014 at 05:48:57PM +0100, Philipp Zabel wrote: Hi Russell, I've tested this series on a BD-SL (SabreLite) with HDMI. Right now the HPD signal doesn't seem to work, but after overwriting the connection check, I got a stable and correct picture on the monitor: Hmm. Does this mean you're not getting any IRQs from the HDMI interface when you plug and unplug the monitor? 147:281 GIC 147 12.hdmi on turning the TV on: 147:282 GIC 147 12.hdmi on unplugging the HDMI cable: 147:283 GIC 147 12.hdmi on replugging the HDMI cable: 147:284 GIC 147 12.hdmi Exactly. And while the RX_SENSE bits of HDMI_IH_PHY_STAT0 change when I (un)plug, the HPD bit stays zero. I'll check with other hardware. My guess would be that someone forgot to wire up the HPD line correctly. As ever, the documentation doesn't describe how this stuff works... -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 27/46] imx-drm: convert to componentised device support
On Mon, Jan 06, 2014 at 06:41:28PM +0100, Philipp Zabel wrote: Hi Eric, Am Freitag, den 03.01.2014, 12:14 -0700 schrieb Eric Nelson: This is an issue we've seen before. The SABRE Lite board has a voltage divider on the HPD pins and some monitors (esp. DVI monitors) either don't drive things high enough to assert HPD or bounce with connect/disconnect. Yes, I used a DVI monitor. We've instrumented our 3.0.35 kernels to use the RX_SENSE bits instead. Reacting to RX_SENSE0 instead of HPD seems to work. However, it's non-compliant, because HPD can be lowered and raised by the sink when it changes its EDID data (eg, because you're connected through a switch and the routing has been changed.) So, reacting to RX_SENSE0 instead of HPD has to be a work-around enabled only for those boards which are broken in this regard. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 24/46] imx-drm: provide common connector mode validation function
On Tue, Jan 07, 2014 at 02:38:05PM +0800, Shawn Guo wrote: On Thu, Jan 02, 2014 at 09:27:48PM +, Russell King wrote: diff --git a/drivers/staging/imx-drm/imx-drm.h b/drivers/staging/imx-drm/imx-drm.h index 5649f180dc44..4eb594ce9cff 100644 --- a/drivers/staging/imx-drm/imx-drm.h +++ b/drivers/staging/imx-drm/imx-drm.h @@ -68,4 +68,7 @@ int imx_drm_encoder_get_mux_id(struct drm_encoder *encoder); int imx_drm_encoder_add_possible_crtcs(struct imx_drm_encoder *imx_drm_encoder, struct device_node *np); +int imx_drm_connector_mode_valid(struct drm_connector *connector, + struct drm_display_mode *mode); + #endif /* _IMX_DRM_H_ */ CC drivers/staging/imx-drm/ipu-v3/ipu-dc.o LD net/ethernet/built-in.o In file included from drivers/staging/imx-drm/ipu-v3/ipu-dc.c:23:0: drivers/staging/imx-drm/ipu-v3/../imx-drm.h:56:9: warning: ‘struct drm_display_mode’ declared inside parameter list [enabled by default] drivers/staging/imx-drm/ipu-v3/../imx-drm.h:56:9: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default] Thanks, fixed. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 30/46] imx-drm: remove separate imx-fbdev
On Tue, Jan 07, 2014 at 02:49:50PM +0800, Shawn Guo wrote:
On Thu, Jan 02, 2014 at 09:28:19PM +, Russell King wrote:
@@ -449,6 +458,24 @@ static int imx_drm_driver_load(struct drm_device *drm,
unsigned long flags)
}
}
+ /*
+* All components are now initialised, so setup the fb helper.
+* The fb helper takes copies of key hardware information, so the
+* crtcs/connectors/encoders must not change after this point.
+*/
+#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER)
+ if (legacyfb_depth != 16 legacyfb_depth != 32) {
+ dev_warn(drm-dev, Invalid legacyfb_depth. Defaulting to
16bpp\n);
+ legacyfb_depth = 16;
+ }
+ imxdrm-fbhelper = drm_fbdev_cma_init(drm, legacyfb_depth,
+ drm-mode_config.num_crtc, 4);
s/4/MAX_CRTC
imx-drm-core.c has the macro.
Possible, but when moving code from one location to another it's best
to move it with minimal changes. Even so, I've now changed this.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 27/46] imx-drm: convert to componentised device support
On Tue, Jan 07, 2014 at 04:59:35PM +0800, Shawn Guo wrote:
On Thu, Jan 02, 2014 at 09:28:03PM +, Russell King wrote:
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index e75e11b36dff..0e005f21d241 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -62,6 +62,12 @@
};
};
+ imx-drm {
+ compatible = fsl,imx-drm;
+ crtcs = ipu1 0, ipu1 1;
+ connectors = ldb;
+ };
+
While the change works fine on imx6dl, it breaks LVDS support on imx6q
right away.
imx-ipuv3 240.ipu: IPUv3H probed
imx-ipuv3 280.ipu: IPUv3H probed
[drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[drm] No driver support for vblank timestamp query.
imx-drm imx-drm.16: bound imx-ipuv3-crtc.0 (ops ipu_crtc_ops)
imx-drm imx-drm.16: bound imx-ipuv3-crtc.1 (ops ipu_crtc_ops)
imx-drm imx-drm.16: failed to bind ldb.10 (ops imx_ldb_ops): -517
Because we have 4 crtcs for lvds-channel on imx6q while imx-drm master
defines only 2 in there, the imx_drm_encoder_parse_of() call from
imx_ldb_register() will always return -EPROBE_DEFER.
lvds-channel@0 {
crtcs = ipu1 0, ipu1 1, ipu2 0, ipu2 1;
};
lvds-channel@1 {
crtcs = ipu1 0, ipu1 1, ipu2 0, ipu2 1;
};
This is why some help would be useful here - I think I got these right
but I've no way to check them.
Can you confirm that adding all four is the right thing not only for
the imx6q but also the imx6dl sabresd please?
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 27/46] imx-drm: convert to componentised device support
On Tue, Jan 07, 2014 at 05:29:55PM +0100, Philipp Zabel wrote:
Thanky you. This is what I came up with so far:
From: Philipp Zabel p.za...@pengutronix.de
Subject: [PATCH 1/2] staging: imx-hdmi: use RX_SENSE0 for plug detection if
HPD is unreliable
On some boards HPD might not reliably detect DVI monitors. Allow to use
RX_SENSE0 as a workaround.
Signed-off-by: Philipp Zabel p.za...@pengutronix.de
---
drivers/staging/imx-drm/imx-hdmi.c | 45
+-
1 file changed, 35 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-hdmi.c
b/drivers/staging/imx-drm/imx-hdmi.c
index 7779337..cc305f3 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/staging/imx-drm/imx-hdmi.c
@@ -139,6 +139,7 @@ struct imx_hdmi {
struct regmap *regmap;
struct i2c_adapter *ddc;
+ bool hpd_unreliable;
void __iomem *regs;
unsigned int sample_rate;
@@ -1309,6 +1310,14 @@ static int imx_hdmi_setup(struct imx_hdmi *hdmi,
struct drm_display_mode *mode)
/* Wait until we are registered to enable interrupts */
static int imx_hdmi_fb_registered(struct imx_hdmi *hdmi)
{
+ int stat_bit = HDMI_IH_PHY_STAT0_HPD;
+ int mask_bits = ~HDMI_PHY_HPD;
+
+ if (hdmi-hpd_unreliable) {
+ stat_bit = HDMI_IH_PHY_STAT0_RX_SENSE0;
+ mask_bits = ~HDMI_PHY_RX_SENSE0;
+ }
+
How about storing these in imx_hdmi instead, so we don't have to compute
them in each interrupt? Maybe sink_detect_status and sink_detect_mask?
Thanks.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 27/46] imx-drm: convert to componentised device support
On Thu, Jan 09, 2014 at 11:25:38PM +0800, Shawn Guo wrote: Yea, adding all four into imx-drm crtcs works for imx6q, but it doesn't for imx6dl, because ipu2 is unavailable for imx6dl at all. Here is how I get around it. Thanks, I've rolled your change into this commit now. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v2] staging: imx-hdmi: use rx sense status for plug detection if hpd is unreliable
On Fri, Jan 10, 2014 at 03:22:24PM +0100, Philipp Zabel wrote: Due to the voltage divider on the HPD line, the HDMI connector on imx6q-sabrelite doesn't reliably detect connected DVI monitors. This patch allows to use the RX_SENSE0 signal as a workaround when enabled by a boolean device tree property 'hpd-unreliable'. Yay, much nicer, easier to read, and smaller, thanks. I'll add it to the series. I don't think we've had David Airlie's eyes on the series yet (but then I think he's rather Rob Clark looked at non-x86 DRM stuff). Also I've yet to hear back from Greg wrt the component stuff, so I suspect this series won't be making this merge window. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Thu, Jan 02, 2014 at 07:10:55PM -0800, Greg Kroah-Hartman wrote:
On Thu, Jan 02, 2014 at 09:27:58PM +, Russell King wrote:
Subsystems such as ALSA, DRM and others require a single card-level
device structure to represent a subsystem. However, firmware tends to
describe the individual devices and the connections between them.
Therefore, we need a way to gather up the individual component devices
together, and indicate when we have all the component devices.
We do this in DT by providing a superdevice node which specifies
the components, eg:
imx-drm {
compatible = fsl,drm;
crtcs = ipu1;
connectors = hdmi;
};
The superdevice is declared into the component support, along with the
subcomponents. The superdevice receives callbacks to locate the
subcomponents, and identify when all components are present. At this
point, we bind the superdevice, which causes the appropriate subsystem
to be initialised in the conventional way.
When any of the components or superdevice are removed from the system,
we unbind the superdevice, thereby taking the subsystem down.
This sounds a lot like the containers code that Rafael just submitted
and I acked for 3.14. Look at the lkml post:
Subject: [PATCH 2/2] ACPI / hotplug / driver core: Handle containers in
a special way
Message-ID: 1991202.gilw172...@vostro.rjw.lan
And see if that could possibly be used instead?
Greg,
Not sure if you saw the outcome to your comment above. My conclusion
was:
Yes, I'm coming to that conclusion as well. It looks like your containers
aren't about collecting up several individual component devices into one
super-device and probing the appropriate subsystem when all components are
known.
Confused why Greg is pointing me at your patches.
Does this mean you're happy with the patch?
Thanks.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Fri, Jan 10, 2014 at 07:07:02AM -0800, Greg Kroah-Hartman wrote: On Fri, Jan 10, 2014 at 02:54:44PM +, Russell King - ARM Linux wrote: Greg, Not sure if you saw the outcome to your comment above. My conclusion was: Yes, I'm coming to that conclusion as well. It looks like your containers aren't about collecting up several individual component devices into one super-device and probing the appropriate subsystem when all components are known. Confused why Greg is pointing me at your patches. Ah, sorry, I missed that in my catch up on 2 weeks of email flood. Does this mean you're happy with the patch? Well, I will not object to it on the grounds of it being a duplicate of Rafael's work now :) I'll be glad to consider it on its own, when you feel the series is ready to be submitted. I'll sort out a new set of patches today, along with a branch to pull if you wish to take them that way. Thanks. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Fri, Jan 10, 2014 at 12:42:59PM -0800, Greg Kroah-Hartman wrote:
On Fri, Jan 10, 2014 at 07:30:21PM +0100, Robert Schwebel wrote:
On Fri, Jan 10, 2014 at 07:35:30AM -0800, Greg Kroah-Hartman wrote:
I'll sort out a new set of patches today, along with a branch to pull if
you wish to take them that way.
It's too late for 3.14, as my tree is now closed for that because 3.13
should be out this weekend. But I'll be glad to queue them up after
3.14-rc1 is out.
Didnt' Linus say there would be an -rc8?
Ah, you are right, nevermind then :)
Russell, if you want to send me just the driver core patch, I can take
that now and get that into 3.14 so that you don't have any cross-tree
dependancies on the rest of this patch series.
Thanks, here it is. Only change from the previously posted one is a
few checkpatch cleanups.
8===
From: Russell King rmk+ker...@arm.linux.org.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Subject: [PATCH] drivers/base: provide an infrastructure for componentised
subsystems
Subsystems such as ALSA, DRM and others require a single card-level
device structure to represent a subsystem. However, firmware tends to
describe the individual devices and the connections between them.
Therefore, we need a way to gather up the individual component devices
together, and indicate when we have all the component devices.
We do this in DT by providing a superdevice node which specifies
the components, eg:
imx-drm {
compatible = fsl,drm;
crtcs = ipu1;
connectors = hdmi;
};
The superdevice is declared into the component support, along with the
subcomponents. The superdevice receives callbacks to locate the
subcomponents, and identify when all components are present. At this
point, we bind the superdevice, which causes the appropriate subsystem
to be initialised in the conventional way.
When any of the components or superdevice are removed from the system,
we unbind the superdevice, thereby taking the subsystem down.
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
---
drivers/base/Makefile | 2 +-
drivers/base/component.c | 382 ++
include/linux/component.h | 32
3 files changed, 415 insertions(+), 1 deletion(-)
create mode 100644 drivers/base/component.c
create mode 100644 include/linux/component.h
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 94e8a80e87f8..870ecfd503af 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -1,6 +1,6 @@
# Makefile for the Linux device tree
-obj-y := core.o bus.o dd.o syscore.o \
+obj-y := component.o core.o bus.o dd.o syscore.o \
driver.o class.o platform.o \
cpu.o firmware.o init.o map.o devres.o \
attribute_container.o transport_class.o \
diff --git a/drivers/base/component.c b/drivers/base/component.c
new file mode 100644
index ..c53efe6c6d8e
--- /dev/null
+++ b/drivers/base/component.c
@@ -0,0 +1,382 @@
+/*
+ * Componentized device handling.
+ *
+ * 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 is work in progress. We gather up the component devices into a list,
+ * and bind them when instructed. At the moment, we're specific to the DRM
+ * subsystem, and only handles one master device, but this doesn't have to be
+ * the case.
+ */
+#include linux/component.h
+#include linux/device.h
+#include linux/kref.h
+#include linux/list.h
+#include linux/module.h
+#include linux/mutex.h
+#include linux/slab.h
+
+struct master {
+ struct list_head node;
+ struct list_head components;
+ bool bound;
+
+ const struct component_master_ops *ops;
+ struct device *dev;
+};
+
+struct component {
+ struct list_head node;
+ struct list_head master_node;
+ struct master *master;
+ bool bound;
+
+ const struct component_ops *ops;
+ struct device *dev;
+};
+
+static DEFINE_MUTEX(component_mutex);
+static LIST_HEAD(component_list);
+static LIST_HEAD(masters);
+
+static struct master *__master_find(struct device *dev,
+ const struct component_master_ops *ops)
+{
+ struct master *m;
+
+ list_for_each_entry(m, masters, node)
+ if (m-dev == dev (!ops || m-ops == ops))
+ return m;
+
+ return NULL;
+}
+
+/* Attach an unattached component to a master. */
+static void component_attach_master(struct master *master, struct component *c)
+{
+ c-master = master;
+
+ list_add_tail(c-master_node, master-components);
+}
+
+/* Detach a component from a master. */
+static void component_detach_master(struct master *master, struct component *c)
+{
+ list_del(c-master_node);
+
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Sat, Jan 11, 2014 at 12:31:19PM +0100, Robert Schwebel wrote:
On Fri, Jan 10, 2014 at 11:23:37PM +, Russell King - ARM Linux wrote:
We do this in DT by providing a superdevice node which specifies
the components, eg:
imx-drm {
compatible = fsl,drm;
crtcs = ipu1;
connectors = hdmi;
};
Saschas comment from 20140109074030.gn6...@pengutronix.de isn't
addressed yet:
This is not a comment against _this_ patch. It was a question for Sean
Paul. You can tell this by the contents of the To: and Cc: headers on
Sascha's email. If that's not what was intended, then the email headers
are misleading.
Sascha Hauer wrote:
Can we have an example with a different number of
encoders/connectors/crtcs, like:
exynos-drm {
compatible = exynos,drm;
crtcs = fimd1;
encoders = dp1, hdmi1, lvds1;
connectors = ptn3460, hdmi1;
};
Otherwise I get the impression that there is some topology of the
components or at least relationship between the components encoded
into the binding.
If I remember correctly, Sascha+Philipp+Lucas still had issues with the
bindings, but I'm not sure if they have been already addressed.
The bindings are not part of this patch. This patch doesn't even care
about DT one bit - that's part of the design goal of it. It doesn't
care about platform devices either.
All it cares about is maintaining lists of struct device pointers,
asking the master device(s) whether they have all their components, and
binding/unbinding the components at the appropriate moment. It's
completely up to the master device operations to decide how to make
these decisions, and how those decisions are made, whether it be by
looking up in DT, or ACPI, or whatever.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] Fix some bugs/races in imx-drm
On Wed, Feb 05, 2014 at 12:02:49PM -0800, Greg Kroah-Hartman wrote: On Tue, Dec 17, 2013 at 05:13:48PM -0800, Greg Kroah-Hartman wrote: I'll apply these after the first round goes into Linus's tree to make the merging easier (i.e. after the next -rc). Now applied. Thanks. I'll see about sending the next set out for review - hopefully I can get all of them out for the next merge window. I do have one minor fix for the component stuff you merged which I'll send out in the next few days (it's just to ensure that devm resources for the master device are properly handled.) And thanks for merging that during the previous merge window, that has been a great help. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Fri, Feb 07, 2014 at 10:04:30AM +0100, Daniel Vetter wrote: I've chatted a bit with Hans Verkuil about this topic at fosdem and apparently both v4l and alsa have something like this already in their helper libraries. Adding more people as fyi in case they want to switch to the new driver core stuff from Russell. It's not ALSA, but ASoC which has this. Mark is already aware of this and will be looking at it from an ASoC perspective. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Fri, Feb 07, 2014 at 12:57:21PM +0100, Jean-Francois Moine wrote: I started to use your code (which works fine, thanks), and it avoids a lot of problems, especially, about probe_defer in a DT context. I was wondering if your componentised mechanism could be extended to the devices defined by DT. It was developed against imx-drm, which is purely DT based. I already have a solution for the cubox armada DRM. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 1/2] drivers/base: permit base components to omit the bind/unbind ops
On Fri, Feb 07, 2014 at 04:55:00PM +0100, Jean-Francois Moine wrote:
Some simple components don't need to do any specific action on
bind to / unbind from a master component.
This patch permits such components to omit the bind/unbind
operations.
Signed-off-by: Jean-Francois Moine moin...@free.fr
---
drivers/base/component.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/base/component.c b/drivers/base/component.c
index c53efe6..0a39d7a 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -225,7 +225,8 @@ static void component_unbind(struct component *component,
{
WARN_ON(!component-bound);
- component-ops-unbind(component-dev, master-dev, data);
+ if (component-ops)
+ component-ops-unbind(component-dev, master-dev, data);
component-bound = false;
/* Release all resources claimed in the binding of this component */
@@ -274,7 +275,11 @@ static int component_bind(struct component *component,
struct master *master,
dev_dbg(master-dev, binding %s (ops %ps)\n,
dev_name(component-dev), component-ops);
- ret = component-ops-bind(component-dev, master-dev, data);
+ if (component-ops)
+ ret = component-ops-bind(component-dev, master-dev, data);
+ else
+ ret = 0;
+
NAK. If this is done, there's absolutely no point to this code.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
On Fri, Feb 07, 2014 at 06:11:08PM +0100, Jean-Francois Moine wrote: This patch series tries to simplify the code of simple devices in case they are part of componentised subsystems, are declared in a DT, and are not using the component bin/unbind functions. I wonder - I said earlier today that this works absolutely fine without modification with DT, so why are you messing about with it adding DT support? This is totally the wrong approach. The idea is that this deals with /devices/ and /devices/ only. It groups up /devices/. It's up to the add_component callback to the master device to decide how to deal with that. Jean-Francois Moine (2): drivers/base: permit base components to omit the bind/unbind ops And this patch has me wondering if you even understand how to use this... The master bind/unbind callbacks are the ones which establish the card based context with the subsystem. Please, before buggering up this nicely designed implementation, please /first/ look at the imx-drm rework which was posted back in early January which illustrates how this is used in a DT context - which is something I've already pointed you at once today already. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 2/2] drivers/base: declare phandle DT nodes as components
On Fri, Feb 07, 2014 at 05:53:27PM +0100, Jean-Francois Moine wrote: At system startup time, some devices depends on the availability of some other devices before starting. The infrastructure for componentised subsystems permits to handle this dependence, each driver defining its own role. This patch does an automatic creation of the lowest components in case of DT. This permits simple devices to be part of complex componentised subsystems without any specific code. A component with no operations makes precisely no sense - with that, there's no way for the component to be a stand-alone driver. Your approach forces your ideas onto every DT device that is referenced as a phandle. That's extremely restrictive. I don't want the component stuff knowing anything about OF. I don't want it knowing about driver matching. I don't want it knowing about ACPI either. That's the whole point behind it - it is 100% agnostic about how that stuff works. The model is quite simply this: - a master device is the covering component for the card - the master device knows what components to expect by some means. In the case of DT, that's by phandle references to the components. - the master device handles the component independent setup of the card, creating the common resources that are required. When it's ready, it asks for the components to be bound. - upon removal of any component, the master component is unbound, which triggers the removal of the card from the subsystem. - as part of the removal, sub-components are unbound. - the master device should have as /little/ knowledge about the components as possible to permit component re-use. - a component driver should be independent of it's master. - A component which is probed from the device model should simply register itself using component_add() with an appropriate operations structure, and a removal function which deletes itself. - When the driver is ready to be initialised (when the card level resources have been set in place) the bind method will be called. At this point, the component does everything that a classical driver model driver would do in it's probe callback. - unbind is the same as remove in the classical driver model. So, please, no DT stuff in the component support - it's simply not required. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
On Fri, Feb 07, 2014 at 07:42:04PM +0100, Jean-Francois Moine wrote: On Fri, 7 Feb 2014 17:33:26 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Fri, Feb 07, 2014 at 06:11:08PM +0100, Jean-Francois Moine wrote: This patch series tries to simplify the code of simple devices in case they are part of componentised subsystems, are declared in a DT, and are not using the component bin/unbind functions. I wonder - I said earlier today that this works absolutely fine without modification with DT, so why are you messing about with it adding DT support? This is totally the wrong approach. The idea is that this deals with /devices/ and /devices/ only. It groups up /devices/. It's up to the add_component callback to the master device to decide how to deal with that. Jean-Francois Moine (2): drivers/base: permit base components to omit the bind/unbind ops And this patch has me wondering if you even understand how to use this... The master bind/unbind callbacks are the ones which establish the card based context with the subsystem. Please, before buggering up this nicely designed implementation, please /first/ look at the imx-drm rework which was posted back in early January which illustrates how this is used in a DT context - which is something I've already pointed you at once today already. As I told in a previous mail, your code works fine in my DT-based Cubox. I am rewriting the TDA988x as a normal encoder/connector, and, yes, the bind/unbind functions are useful in this case. So, which bit of I've already got that was missed? But you opened a door. In a DT context, you know that the probe_defer mechanism does not work correctly. Your work permits to solve delicate cases: your component_add tells exactly when a device is available, and the master bind callback is the green signal for the device waiting for its resources. Indeed, your system was not created for such a usage, but it works as it is (anyway, the component bind/unbind functions may be empty...). Sorry. Deferred probe does work, it's been tested with imx-drm, not only from the master component but also the sub-components. There's no problem here. And no component bind/unbind function should ever be empty. Again, I put it to you that you don't understand this layer. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
On Fri, Feb 07, 2014 at 06:11:08PM +0100, Jean-Francois Moine wrote:
This patch series tries to simplify the code of simple devices in case
they are part of componentised subsystems, are declared in a DT, and
are not using the component bin/unbind functions.
Here's my changes to the TDA998x driver to add support for the component
helper. The TDA998x driver retains support for the old way so that
drivers can be transitioned. For any one DRM card the transition to
using the component layer must be all-in or all-out - partial transitions
are not permitted with the simple locking implementation currently in
the component helper due to the possibility of deadlock. (Master
binds, holding the component lock, master declares i2c device, i2c
device is bound to tda998x, which tries to register with the component
layer, trying to take the held lock.)
http://ftp.arm.linux.org.uk/cgit/linux-cubox.git/log/?h=unstable/tda998x-devel
It's marked unstable because the two drivers/base commits in there are
_not_ the upstream commits. You'll notice that I just sent one of those
to Gregkh in patch form.
Now, the bits which aren't in that branch but which update the Armada
driver is the below, which needs some clean up and removal of some local
differences I have in my cubox tree, but nicely illustrates why /your/
patches to the component stuff is the wrong approach.
Not only does the patch below add support for using the componentised
TDA998x driver in the above link, but it allows that componentised support
to be specified not only via platform data but also via DT. The DT
stuff is untested, but it works exactly the same way as imx-drm does
which has been tested.
And yes, I'm thinking that maybe moving compare_of() into the component
support so that drivers can share this generic function may be a good
idea.
So... as I've been saying, no changes to component.c are required here.
diff --git a/drivers/gpu/drm/armada/armada_drv.c
b/drivers/gpu/drm/armada/armada_drv.c
index 6f6554bac93f..1f2b7c60bff9 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -6,7 +6,9 @@
* published by the Free Software Foundation.
*/
#include linux/clk.h
+#include linux/component.h
#include linux/module.h
+#include linux/platform_data/armada_drm.h
#include drm/drmP.h
#include drm/drm_crtc_helper.h
#include armada_crtc.h
@@ -53,6 +55,11 @@ static const struct armada_drm_slave_config tda19988_config
= {
};
#endif
+static bool is_componentized(struct device *dev)
+{
+ return dev-of_node || dev-platform_data;
+}
+
static void armada_drm_unref_work(struct work_struct *work)
{
struct armada_private *priv =
@@ -171,16 +178,22 @@ static int armada_drm_load(struct drm_device *dev,
unsigned long flags)
goto err_kms;
}
+ if (is_componentized(dev-dev)) {
+ ret = component_bind_all(dev-dev, dev);
+ if (ret)
+ goto err_kms;
+ } else {
#ifdef CONFIG_DRM_ARMADA_TDA1998X
- ret = armada_drm_connector_slave_create(dev, tda19988_config);
- if (ret)
- goto err_kms;
+ ret = armada_drm_connector_slave_create(dev, tda19988_config);
+ if (ret)
+ goto err_kms;
#endif
#ifdef CONFIG_DRM_ARMADA_TDA1998X_NXP
- ret = armada_drm_tda19988_nxp_create(dev);
- if (ret)
- goto err_kms;
+ ret = armada_drm_tda19988_nxp_create(dev);
+ if (ret)
+ goto err_kms;
#endif
+ }
ret = drm_vblank_init(dev, n);
if (ret)
@@ -204,6 +217,10 @@ static int armada_drm_load(struct drm_device *dev,
unsigned long flags)
drm_irq_uninstall(dev);
err_kms:
drm_mode_config_cleanup(dev);
+
+ if (is_componentized(dev-dev))
+ component_unbind_all(dev-dev, dev);
+
drm_mm_takedown(priv-linear);
flush_work(priv-fb_unref_work);
@@ -218,6 +235,10 @@ static int armada_drm_unload(struct drm_device *dev)
armada_fbdev_fini(dev);
drm_irq_uninstall(dev);
drm_mode_config_cleanup(dev);
+
+ if (is_componentized(dev-dev))
+ component_unbind_all(dev-dev, dev);
+
drm_mm_takedown(priv-linear);
flush_work(priv-fb_unref_work);
dev-dev_private = NULL;
@@ -380,14 +401,82 @@ static struct drm_driver armada_drm_driver = {
.fops = armada_drm_fops,
};
+static int armada_drm_bind(struct device *dev)
+{
+ return drm_platform_init(armada_drm_driver, to_platform_device(dev));
+}
+
+static void armada_drm_unbind(struct device *dev)
+{
+ drm_platform_exit(armada_drm_driver, to_platform_device(dev));
+}
+
+static int compare_of(struct device *dev, void *data)
+{
+ return dev-of_node == data;
+}
+
+static int armada_drm_compare_name(struct device *dev, void *data)
+{
+ const char *name = data;
+ return
Re: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
On Fri, Feb 07, 2014 at 06:59:11PM +, Russell King - ARM Linux wrote: Sorry. Deferred probe does work, it's been tested with imx-drm, not only from the master component but also the sub-components. There's no problem here. Here's the proof that it also works with the Cubox, and armada DRM: [drm] Initialized drm 1.1.0 20060810 ... armada-drm armada-510-drm: master bind failed: -517 i2c 0-0070: Driver tda998x requests probe deferral ... tda998x 0-0070: found TDA19988 armada-drm armada-510-drm: bound 0-0070 (ops tda998x_ops) So, in the above sequence, the armada DRM driver was bound to its driver initially, but the TDA998x driver wasn't. Then, the TDA998x driver is bound, which completes the requirements for the DRM master. So the system attempts to bind. In doing so, the master probe function discovers a missing clock (because the SIL5531 driver hasn't probed) and it returns -EPROBE_DEFER. This causes the probe of the TDA998x to be deferred. Later, deferred probes are run - at this time the SIL5531 driver has probed its device, and the clocks are now available. So when the TDA998x driver is re-probed, it retriggers the binding attempt, and as the clock can now be found, the system is bound and the DRM system for the device is initialised. I've just committed a patch locally which makes Armada DRM fully use the component helper, which removes in totality the four armada_output.* and armada_slave.* files since they're no longer required: [cubox-3.13 e2713ff5ac2f] DRM: armada: remove non-component support 7 files changed, 8 insertions(+), 437 deletions(-) delete mode 100644 drivers/gpu/drm/armada/armada_output.c delete mode 100644 drivers/gpu/drm/armada/armada_output.h delete mode 100644 drivers/gpu/drm/armada/armada_slave.c delete mode 100644 drivers/gpu/drm/armada/armada_slave.h -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] imx-drm: imx-drm-core: Remove unused 'imxdrm' variable
On Sat, Feb 08, 2014 at 06:49:49PM -0200, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com Since commit 020a9ea7c2 (imx-drm: imx-drm-core: avoid going the long route round for drm_device) the 'imxdrm' variable is not used anymore, which causes the following build warning: drivers/staging/imx-drm/imx-drm-core.c:87:25: warning: unused variable 'imxdrm' [-Wunused-variable] Just remove it. NAK. My later patches need this, so there wasn't any point removing it in previous patches. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
On Sun, Feb 09, 2014 at 10:22:19AM +0100, Jean-Francois Moine wrote: On Fri, 7 Feb 2014 20:23:51 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: Here's my changes to the TDA998x driver to add support for the component helper. The TDA998x driver retains support for the old way so that drivers can be transitioned. For any one DRM card the transition to I rewrote the tda998x as a simple encoder+connector (i.e. not a slave_encoder) with your component helper, and the code is much clearer and simpler: the DRM driver has nothing to do except to know that the tda998x is a component and to set the possible_crtcs. That's exactly what I've done - the slave encoder veneer can be simply deleted when tilcdc is converted. AFAIK, only the tilcdc drm driver is using the tda998x as a slave_encoder. It does a (encoder+connector) conversion to (slave_encoder). Then, in your changes in the TDA998x, you do a (slave_encoder) translation to (encoder+connector). This seems rather complicated! No. I first split out the slave encoder functions to be a veneer onto the tda998x backend, and then add the encoder connector component support. Later, the slave encoder veneer can be deleted. The reason for this is that virtually all the tda998x backend is what's required by the encoder connector support - which is completely logical when you realise that the generic slave encoder support is just a veneer itself adapting the encoder connector support to a slave encoder. So, we're basically getting rid of two veneers, but we end up with exactly the same functionality. And yes, I'm thinking that maybe moving compare_of() into the component support so that drivers can share this generic function may be a good idea. This function exists already in drivers/of/platform.c as of_dev_node_match(). It just needs to be exported. Good, thanks for pointing that out. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: drm/imx: remove an unnecessary local variable
On Mon, Feb 10, 2014 at 06:29:45PM +0800, Liu Ying wrote: This patch removes an unnecessary local variable defined in the function imx_drm_driver_unload() so as to fix the following build warning. drivers/staging/imx-drm/imx-drm-core.c: \ In function ‘imx_drm_driver_unload’: drivers/staging/imx-drm/imx-drm-core.c:87:25: \ warning: unused variable ‘imxdrm’ [-Wunused-variable] Already-Naked-by: me. This is required by later patches in the series posted earlier. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: drm/imx: remove an unnecessary local variable
On Mon, Feb 10, 2014 at 06:42:57PM +0800, Liu Ying wrote: On 02/10/2014 06:29 PM, Russell King - ARM Linux wrote: On Mon, Feb 10, 2014 at 06:29:45PM +0800, Liu Ying wrote: This patch removes an unnecessary local variable defined in the function imx_drm_driver_unload() so as to fix the following build warning. drivers/staging/imx-drm/imx-drm-core.c: \ In function ‘imx_drm_driver_unload’: drivers/staging/imx-drm/imx-drm-core.c:87:25: \ warning: unused variable ‘imxdrm’ [-Wunused-variable] Already-Naked-by: me. This is required by later patches in the series posted earlier. Sorry. Did you mean that you have already got a fix for this? No, I mean patches which come after the three which Greg has recently merged - one of which removes users of the above variable - reintroduce users of this variable, so deleting the variable will break those patches. It was also pointless to remove it and then bring it back in the series. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/2] drivers/base: permit base components to omit the bind/unbind ops
On Mon, Feb 10, 2014 at 01:53:08PM +0100, Thierry Reding wrote:
On Fri, Feb 07, 2014 at 04:55:00PM +0100, Jean-Francois Moine wrote:
Some simple components don't need to do any specific action on
bind to / unbind from a master component.
This patch permits such components to omit the bind/unbind
operations.
Signed-off-by: Jean-Francois Moine moin...@free.fr
---
drivers/base/component.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/base/component.c b/drivers/base/component.c
index c53efe6..0a39d7a 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -225,7 +225,8 @@ static void component_unbind(struct component
*component,
{
WARN_ON(!component-bound);
- component-ops-unbind(component-dev, master-dev, data);
+ if (component-ops)
+ component-ops-unbind(component-dev, master-dev, data);
This doesn't actually do what the commit message says. This makes
component-ops optional, not component-ops-unbind().
A more correct check would be:
if (component-ops component-ops-unbind)
component-bound = false;
/* Release all resources claimed in the binding of this component */
@@ -274,7 +275,11 @@ static int component_bind(struct component *component,
struct master *master,
dev_dbg(master-dev, binding %s (ops %ps)\n,
dev_name(component-dev), component-ops);
- ret = component-ops-bind(component-dev, master-dev, data);
+ if (component-ops)
+ ret = component-ops-bind(component-dev, master-dev, data);
Same here.
I've NAK'd these patches already - I believe they're based on a
mis-understanding of how this should be used. I believe Jean-Francois
has only looked at the core, rather than looking at the imx-drm example
it was posted with in an attempt to understand it.
Omitting the component bind operations is absurd because it makes the
component code completely pointless, since there is then no way to
control the sequencing of driver initialisation - something which is
one of the primary reasons for this code existing in the first place.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/2] drivers/base: permit base components to omit the bind/unbind ops
On Mon, Feb 10, 2014 at 03:35:51PM +0100, Jean-Francois Moine wrote: On Mon, 10 Feb 2014 13:12:33 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: I've NAK'd these patches already - I believe they're based on a mis-understanding of how this should be used. I believe Jean-Francois has only looked at the core, rather than looking at the imx-drm example it was posted with in an attempt to understand it. Omitting the component bind operations is absurd because it makes the component code completely pointless, since there is then no way to control the sequencing of driver initialisation - something which is one of the primary reasons for this code existing in the first place. I perfectly looked at your example and I use it now in my system. You did not see what could be done with your component code. For example, since november, I have not yet the clock probe_defer in the mainline (http://www.spinics.net/lists/arm-kernel/msg306072.html), so, there are 3 solutions: - hope the patch will be some day in the mainline and, today, reboot when the system does not start correctly, - insert a delay in the tda998x and kirkwood probe sequences (delay long enough to be sure the si5351 is started, or loop), - use your component work. In the last case, it is easy: - the si5351 driver calls component_add (with empty ops: it has no interest in the bind/unbind functions) when it is fully started (i.e. registered - that was the subject of my patch), There is only one word for this: Ewww. Definitely not. The component stuff is there to sort out the subsystems which expect a card but in DT we supply a set of components. It's not there to sort out dependencies between different subsystems. I've no idea why your patch to add the deferred probing hasn't been acked by Mike yet, but that needs to happen before I take it. Or, split it up in two so I can take the clkdev part and Mike can take the CCF part. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC v2 00/35] Second preview of imx-drm cleanup series
On Mon, Feb 10, 2014 at 04:12:19PM +0100, Philipp Zabel wrote: Am Montag, den 10.02.2014, 12:28 + schrieb Russell King - ARM Linux: This is the latest revision of my series cleaning up imx-drm and hopefully getting it ready to be moved out of drivers/staging. This series is updated to v3.14-rc2. Since the last round of patches were posted, the component support has been merged into mainline, and thus dropped from this series. Greg has taken the first three patches and merged them into his linux-next tree - however, I include them here for completness. Most of the comments from last time still apply, and I'll look at incorporating some of the other patches that were posted in the coming week. If I can have some acks for this, I'll start sending some of it to Greg - I'd like to at least get the five or six initial imx-hdmi patches to Greg and queued up for the next merge window sooner rather than later, preferably getting most of this ready for that window too. For the first 9 patches up to (including) imx-drm: ipu-v3: more clocking fixes: Acked-by: Philipp Zabel p.za...@pengutronix.de As you say, comments about the device tree bindings still apply. I'd prefer if the patches that currently use the crtcs property were reworked to use the v4l2 style device tree bindings before they hit mainline. I'm trying /not/ to do that much more work on this because there's other things that need my attention, like a complete rewrite of the SDHCI mess. I want to get the imx-drm stuff off my plate so I don't have to worry about it. So, I'd really like _all_ these patches to go into mainline for v3.15 with the least rework possible so I can spend the next few months working on rewriting SDHCI. What I don't want is carrying hundreds of patches across multiple kernel versions. Now, mind explaining what v4l2 style device tree bindings means? I've no idea since I'm relatively new to DT. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 2/3] staging: imx-drm-core: Use graph to find connection between crtc and encoder
On Mon, Jan 06, 2014 at 03:52:01PM +0100, Philipp Zabel wrote:
@@ -438,24 +453,21 @@ int imx_drm_encoder_parse_of(struct drm_device *drm,
struct drm_encoder *encoder, struct device_node *np)
{
struct imx_drm_device *imxdrm = drm-dev_private;
+ struct device_node *ep, *last_ep = NULL;
uint32_t crtc_mask = 0;
int i, ret = 0;
for (i = 0; !ret; i++) {
- struct of_phandle_args args;
uint32_t mask;
- int id;
- ret = of_parse_phandle_with_args(np, crtcs, #crtc-cells, i,
- args);
- if (ret == -ENOENT)
+ ep = v4l2_of_get_next_endpoint(np, last_ep);
+ if (last_ep)
+ of_node_put(last_ep);
+ if (!ep)
break;
- if (ret 0)
- return ret;
- id = args.args_count 0 ? args.args[0] : 0;
- mask = imx_drm_find_crtc_mask(imxdrm, args.np, id);
- of_node_put(args.np);
+ /* CSI */
+ mask = imx_drm_find_crtc_mask(imxdrm, ep);
/*
* If we failed to find the CRTC(s) which this encoder is
@@ -463,12 +475,20 @@ int imx_drm_encoder_parse_of(struct drm_device *drm,
* not been registered yet. Defer probing, and hope that
* the required CRTC is added later.
*/
- if (mask == 0)
+ if (mask == 0) {
+ of_node_put(ep);
return -EPROBE_DEFER;
+ }
crtc_mask |= mask;
+ last_ep = ep;
}
+ if (ep)
+ of_node_put(ep);
Why is this loop soo complicated? Why do you need to mess around with
this last_ep stuff - you don't actually end up using it.
The loop reduces down to this without comments:
for (i = 0; !ret; i++) {
uint32_t mask;
ep = v4l2_of_get_next_endpoint(np, last_ep);
if (!ep)
break;
/* CSI */
mask = imx_drm_find_crtc_mask(imxdrm, ep);
of_node_put(ep);
if (mask == 0)
return -EPROBE_DEFER;
crtc_mask |= mask;
}
Now, here's the big question: why do we want to use v4l2_* here? We
may want to use this functionality, but if this functionality is going
to be used outside of v4l2, it needs to become something generic, not
v4l2 specific.
Let's think about this for a moment... if we want to build imx-drm into
the kernel, can we do it with modular videodev, or with videodev
completely unconfigured. We may wish to do this because we have no
videodev requirement on a platform. Should the build fail because the
v4l2 function isn't there?
So, before we can change this, I think we first need to get agreement
from Mauro to move this function out of V4L2, so that it can be
available independently of V4L2.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC v2 00/35] Second preview of imx-drm cleanup series
On Mon, Feb 10, 2014 at 06:37:26PM +0100, Philipp Zabel wrote: I'd like all of them to go through, too. If you don't want to have the DT changes integrated, I'd appreciate if you could have a look at my patches on top of your series and possibly append them to your series or let me synchronize somehow: http://archive.arm.linux.org.uk/lurker/message/20140106.145159.a1d82ba9.en.html I replied to your 2nd patch there... I'm not happy ending up with a hard dependency between v4l2 code and code needed for the display side to work, especially when we can end up with imx-drm being the primary console display. I've outlined my thoughts about what I think needs to happen in the reply to patch 2. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC v2 00/35] Second preview of imx-drm cleanup series
On Mon, Feb 10, 2014 at 12:28:03PM +, Russell King - ARM Linux wrote: This is the latest revision of my series cleaning up imx-drm and hopefully getting it ready to be moved out of drivers/staging. This series is updated to v3.14-rc2. Since the last round of patches were posted, the component support has been merged into mainline, and thus dropped from this series. Greg has taken the first three patches and merged them into his linux-next tree - however, I include them here for completness. Most of the comments from last time still apply, and I'll look at incorporating some of the other patches that were posted in the coming week. If I can have some acks for this, I'll start sending some of it to Greg - I'd like to at least get the five or six initial imx-hdmi patches to Greg and queued up for the next merge window sooner rather than later, preferably getting most of this ready for that window too. So far, only the first few patches have been acked by one person. What about the remainder? There's been no comments on them either... -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC v2 00/35] Second preview of imx-drm cleanup series
On Wed, Feb 12, 2014 at 01:04:30PM +0100, Christian Gmeiner wrote: 2014-02-12 12:53 GMT+01:00 Fabio Estevam feste...@gmail.com: On Mon, Feb 10, 2014 at 10:28 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: This is the latest revision of my series cleaning up imx-drm and hopefully getting it ready to be moved out of drivers/staging. This series is updated to v3.14-rc2. Since the last round of patches were posted, the component support has been merged into mainline, and thus dropped from this series. Greg has taken the first three patches and merged them into his linux-next tree - however, I include them here for completness. Most of the comments from last time still apply, and I'll look at incorporating some of the other patches that were posted in the coming week. If I can have some acks for this, I'll start sending some of it to Greg - I'd like to at least get the five or six initial imx-hdmi patches to Greg and queued up for the next merge window sooner rather than later, preferably getting most of this ready for that window too. Series looks good: Reviewed-by: Fabio Estevam fabio.este...@freescale.com Patch 04/35 will not be needed in the final submission, as there is already a fix for the build issue in Greg's tree. I am the only one who does not get the whole series? Even here ony a handfull of patches are listed: http://www.spinics.net/lists/arm-kernel/threads.html#306292 You will find it all on the de...@driverdev.osuosl.org archive. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] hostap: fix hostap: proc: Use remove_proc_subtree()
remove_proc_subtree() doesn't work here as local-ddev has already
been removed, and NULLed out. Use proc_remove() instead.
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Tested-by: Russell King rmk+ker...@arm.linux.org.uk
---
I would include an oops, however the machine I discovered this on has
a page 0 populated - so the symptoms are multiple 'wlan0' entries in
/proc/net/hostap. This is probably a candidate for stable too.
drivers/net/wireless/hostap/hostap_proc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/hostap/hostap_proc.c
b/drivers/net/wireless/hostap/hostap_proc.c
index aa7ad3a7a69b..4e5c0f8c9496 100644
--- a/drivers/net/wireless/hostap/hostap_proc.c
+++ b/drivers/net/wireless/hostap/hostap_proc.c
@@ -496,7 +496,7 @@ void hostap_init_proc(local_info_t *local)
void hostap_remove_proc(local_info_t *local)
{
- remove_proc_subtree(local-ddev-name, hostap_proc);
+ proc_remove(local-proc);
}
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC v2 00/35] Second preview of imx-drm cleanup series
On Tue, Feb 18, 2014 at 12:45:09PM +0100, Philipp Zabel wrote: Thanks for your comments there. I have just updated and resent this series ([RFC PATCH v3 0/9] imx-drm dt bindings), still with a temporary local copy of the v4l2_of functions, as long as the final resting place of the v4l2_of/of_graph functions seems to be a matter of discussion. Okay. My plans are to send more of these patches imminently to Greg. I've just pulled them forward to -rc3, which means that four have now dropped out of the series (as three have been recently merged and one due to that build fix for imx-hdmi.) I'm just debating whether I need to send the whole series out again or not before doing that... any opinion? Nothing's really changed in the patch set since the last time. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v3 1/9] staging: imx-drm-core: don't request probe deferral in imx_drm_encoder_parse_of
On Tue, Feb 18, 2014 at 12:36:02PM +0100, Philipp Zabel wrote: From: Lucas Stach l.st...@pengutronix.de Since imx_drm_encoder_parse_of is called from the encoder bind callbacks, it is too late to request probe deferral. Rather the core should make sure that the crtcs are bound before the encoders, after all needed components are probed. Why is it too late? -EPROBE_DEFER from this point will cause the driver initialisation to correctly unwind and return -EPROBE_DEFER to the last-to-be-added component. This fixes probe failure when using the LDB on i.MX6. More details please. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v3 2/9] staging: imx-drm: Add temporary copies of v4l2-of parsing functions
On Tue, Feb 18, 2014 at 12:36:03PM +0100, Philipp Zabel wrote: From: Philipp Zabel philipp.za...@gmail.com The existing v4l2-of parser functions for the video interface bindings described in Documentation/device-tree/bindings/media/video-interfaces.txt are useful for DRM drivers, too. They will be moved to drivers/media so they can be used by drm drivers, too. Until then, duplicate the v4l2-of parser functions temporarily. Signed-off-by: Philipp Zabel philipp.za...@gmail.com Ergh. So, because we can't get agreement on where to put the common helpers, we have to put up with adding duplicate helpers into the staging directory, inflating not only the kernel source code size but also the binary size as well. Come on people, get agreement on how to deal with this. Staging may be a dumping ground for drivers which aren't ready to be merged as proper drivers, but this is no reason to ignore proper process. Do we, or do we not want to get imx-drm out of drivers/staging? If we do, the only way that's going to happen is if we stop throwing in this kind of stuff. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v3 1/9] staging: imx-drm-core: don't request probe deferral in imx_drm_encoder_parse_of
On Mon, Feb 24, 2014 at 05:56:38PM +0100, Philipp Zabel wrote: Am Montag, den 24.02.2014, 15:49 + schrieb Russell King - ARM Linux: One issue was that the DT parsing code would try to add the imx-ldb component right after the first crtc, and then its bind would fail in imx_drm_encoder_parse_of because the three remaining crtcs were not yet registered. This is already fixed by adding the crtc components first. That's what happens anyway - we add /all/ the CRTCs first before we start adding the connectors. That means the CRTCs will be bound before the connectors (ldb). What you're probably missing is that with what's in my branch, if you want both CRTCs or the second CRTC in the IPU pair, you must specify both in the crtcs property - iow crtcs = ipuX 0, ipuX 1. The reason is there's no way to differentiate the individual platform devices there without delving into the platform data. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] imx-drm conversion to component support
On Mon, Feb 24, 2014 at 12:41:41PM -0800, Greg KH wrote: On Mon, Feb 24, 2014 at 02:20:20PM +, Russell King wrote: Greg, Please incorporate the latest imx-drm updates into staging, which can be found at: git://ftp.arm.linux.org.uk/~rmk/linux-arm.git imx-drm-staging with SHA1 d94905e019acce960feeda1f92a9427fbe45ebbe. These changes, which convert imx-drm to use the recently merged component infrastructure, have been reviewed and acked by Philipp Zabel, Shawn Guo and Fabio Estevam, and are now deemed to be ready. Pulled and pushed out, thanks. Great, many thanks for that. This should be a big step forwards for imx-drm to move out of drivers/staging. Philipp Zabel is working on some patches which clean up the DT side - while it's in staging, we can relatively easily change the DT bindings, but when it moves out, they become more cast in stone. So, we don't want to move it out of staging until we have that resolved. I think we're now very close to it happening though. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 0/5] Move IPUv3 core out of staging, add CSI support
On Wed, Feb 26, 2014 at 11:39:03AM +0300, Dan Carpenter wrote: Please fix the following static checker complaints before moving out of staging: drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c:164 ipu_dmfc_setup_channel() warn: variable dereferenced before check 'dmfc' (see line 157) Note that what's being talked about being moved out is only the above, not the files in drivers/staging/imx-drm. DRM people have not yet reviewed imx-drm itself, which is a necessary step - but nevertheless, thanks for running a static checker on this stuff. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Wed, Feb 26, 2014 at 10:00:25PM +0100, Guennadi Liakhovetski wrote:
Hi Russell
(I suspect this my email will be rejected by ALKML too like other my
recent emails, but at least other MLs will pick it up and individual CCs
too, so, if replying, maybe it would be good to keep my entire reply, all
the more that it's going to be very short)
On Thu, 2 Jan 2014, Russell King wrote:
Subsystems such as ALSA, DRM and others require a single card-level
device structure to represent a subsystem. However, firmware tends to
describe the individual devices and the connections between them.
Therefore, we need a way to gather up the individual component devices
together, and indicate when we have all the component devices.
We do this in DT by providing a superdevice node which specifies
the components, eg:
imx-drm {
compatible = fsl,drm;
crtcs = ipu1;
connectors = hdmi;
};
It is a pity linux-media wasn't CC'ed and apparently V4L developers didn't
notice this and other related patches in a clean up series, and now this
patch is already in the mainline. But at least I'd like to ask whether the
bindings, defined in
Documentation/devicetree/bindings/media/video-interfaces.txt and
implemented in drivers/media/v4l2-core/v4l2-of.c have been considered for
this job, and - if so - why have they been found unsuitable? Wouldn't it
have been better to use and - if needed - extend them to cover any
deficiencies? Even though the implementation is currently located under
drivers/media/v4l2-code/ it's pretty generic and should be easily
transferable to a more generic location.
The component helpers have nothing to do with DT apart from solving
the problem of how to deal with subsystems which expect a single device,
but we have a group of devices and their individual drivers to cope with.
Subsystems like DRM and ALSA.
It is completely agnostic to whether you're using platform data, DT or
even ACPI - this code could *not* care less. None of that comes anywhere
near what this patch does. It merely provides a way to collect up
individual devices from co-operating drivers, and control their binding
such that a subsystem like DRM or ALSA can be presented with a card
level view of the hardware rather than a multi-device medusa with all
the buggy, racy, crap fsckage that people come up to make that kind of
thing work.
Now, as for the binding above, first, what does eg mean... and
secondly, how would a binding which refers to crtcs and connectors
have anything to do with ALSA? Clearly this isn't an example of a
binding for an ALSA use, which was talked about in the very first
line of the above commit commentry. So it's quite clear that what is
given there is an example of how it /could/ be used.
I suppose I could have instead turned imx-drm into a completely unusable
mess by not coming up with some kind of binding, and instead submitted
a whole pile of completely untested code. Alternatively, I could've
used the OF binding as you're suggesting, but that would mean radically
changing the /existing/ bindings for the IPU as a whole - something
which others are better suited at as they have a /much/ better
understanding of the complexities of this hardware than I.
So, what I have done is implemented - for a driver in staging which is
still subject to ongoing development and non-stable DT bindings -
something which allows forward progress with a *minimum* of disruption
to the existing DT bindings for everyone, while still allowing forward
progress.
Better bindings for imx-drm are currently being worked on. Philipp
Zabel of Pengutronix is currently looking at it, and has posted many
RFC patches on this very subject, including moving the V4L2 OF helpers
to a more suitable location. OF people have been involved in that
discussion over the preceding weeks, and there's a working implementation
of imx-drm using these helpers from v4l2.
I'm finding people who are working in the same area and trying to get
everyone talking to each other so that we /do/ end up with a set of
bindings for the display stuff which are suitable for everyone. Tomi
from TI has already expressed his input to this ongoing discussion.
You're welcome to get involved in those discussions too.
I hope this makes it clear, and clears up the confusion.
Thanks.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/6] imx-drm: imx-ldb: Fix array length
On Wed, Feb 26, 2014 at 06:44:36PM -0300, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com Fix the following static checker warning: drivers/staging/imx-drm/imx-ldb.c:340 imx_ldb_get_clk() error: format string overflow. buf_size: 16 length: 18 probably 18 is theory and not real life, but 16 is based on theory as well. I think using snprintf() would be better... snprintf(clkname, sizeof(clkname), di%d, chno); rather than increasing the buffer to hold a maximal-size decimal integer. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 3/6] imx-drm: imx-ldb: Fix array length
On Wed, Feb 26, 2014 at 07:44:33PM -0300, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com Fix the following static checker warning: drivers/staging/imx-drm/imx-ldb.c:340 imx_ldb_get_clk() error: format string overflow. buf_size: 16 length: 18 probably 18 is theory and not real life, but 16 is based on theory as well. I should've pointed out that there's also: sprintf(clkname, di%d_pll, chno); just below which needs the same treatment. Sorry. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
On Thu, Feb 27, 2014 at 02:06:25PM +0100, Philipp Zabel wrote: For the i.MX6 display subsystem there is no clear single master device, and the physical configuration changes across the SoC family. The i.MX6Q/i.MX6D SoCs have two separate display controller devices IPU1 and IPU2, with two output ports each. Not also forgetting that there's another scenario too: you may wish to drive IPU1 and IPU2 as two completely separate display subsystems in some hardware, but as a combined display subsystem in others. Here's another scenario. You may have these two IPUs on the SoC, but there's only one display output. You want to leave the second IPU disabled, as you wouldn't want it to be probed or even exposed to userland. On the face of it, the top-level super-device node doesn't look very hardware-y, but it actually is - it's about how a board uses the hardware provided. This is entirely in keeping with the spirit of DT, which is to describe what hardware is present and how it's connected together, whether it be at the chip or board level. If this wasn't the case, we wouldn't even attempt to describe what devices we have on which I2C buses - we'd just list the hardware on the board without giving any information about how it's wired together. This is no different - however, it doesn't have (and shouldn't) be subsystem specific... but - and this is the challenge we then face - how do you decide that on one board with a single zImage kernel, with both DRM and fbdev built-in, whether to use the DRM interfaces or the fbdev interfaces? We could have both matching the same compatible string, but we'd also need some way to tell each other that they're not allowed to bind. Before anyone argues against it isn't hardware-y, stop and think. What if I design a board with two Epson LCD controllers on board and put a muxing arrangement on their output. Is that one or two devices? What if I want them to operate as one combined system? What if I have two different LCD controllers on a board. How is this any different from the two independent IPU hardware blocks integrated inside an iMX6 SoC with a muxing arrangement on their output? It's very easy to look at a SoC and make the wrong decision... -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
On Thu, Feb 27, 2014 at 03:16:03PM +0200, Tomi Valkeinen wrote: On 27/02/14 13:56, Russell King - ARM Linux wrote: Is there even need for such a master device? You can find all the connected display devices from any single display device, by just following the endpoint links. Please read up on what has been discussed over previous years: http://lists.freedesktop.org/archives/dri-devel/2013-July/041159.html Thanks, that was an interesting thread. Too bad I missed it, it was during the holiday season. And seems Laurent missed it also, as he didn't make any replies. The thread seemed to go over the very same things that had already been discussed with CDF. That may be - but the problem with CDF solving this problem is that it's wrong. It's fixing what is in actual fact a *generic* problem in a much too specific way. To put it another way, it's forcing everyone to fix the same problem in their own separate ways because no one is willing to take a step back and look at the larger picture. We can see that because ASoC has exactly the same problem - it has to wait until all devices (DMA, CPU DAIs, codecs etc) are present before it can initialise, just like DRM. Can you re-use the CDF solution for ASoC? No. Can it be re-used elsewhere in non-display subsystems? No. Therefore, CDF is yet another implementation specific solution to a generic problem which can't be re-used. Yes, I realise that CDF may do other stuff, but because of the above, it's a broken solution. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/6] imx-drm: imx-ldb: Use snprintf()
On Thu, Feb 27, 2014 at 02:54:43PM -0800, Greg KH wrote:
On Wed, Feb 26, 2014 at 08:53:41PM -0300, Fabio Estevam wrote:
diff --git a/drivers/staging/imx-drm/imx-ldb.c
b/drivers/staging/imx-drm/imx-ldb.c
index abf8517..daa54df 100644
--- a/drivers/staging/imx-drm/imx-ldb.c
+++ b/drivers/staging/imx-drm/imx-ldb.c
@@ -334,12 +334,12 @@ static int imx_ldb_get_clk(struct imx_ldb *ldb, int
chno)
{
char clkname[16];
- sprintf(clkname, di%d, chno);
+ snprintf(clkname, sizeof(clkname), di%d, chno);
Are you sure this can not overflow as well? Strings in C are nasty...
Can you indicate how this would overflow?
* snprintf - Format a string and place it in a buffer
...
*
* The return value is the number of characters which would be
* generated for the given input, excluding the trailing null,
* as per ISO C99. If the return is greater than or equal to
* @size, the resulting string is truncated.
Now, there's several layers of protection here. The first obvious one
is using snprintf() instead of sprintf() which wouldn't know the buffer
size.
The second one is something that the static checker can't know, and
that is for existing uses, chno is limited to zero or one:
ret = of_property_read_u32(child, reg, i);
if (ret || i 0 || i 1)
return -EINVAL;
Of course, that could change in the future, but is unlikely to change
significantly - and probably not much beyond two digit decimal.
So, the conversion from sprintf() to snprintf() is technically moot,
since it can only overflow if checks are removed elsewhere in this
code.
So really, this is just to shut the static checker up about something
that is a non-problem.
But there's another problem - and it's one of community process. The
reason these patches got raised is because another kernel maintainer
requested these errors to get fixed, so we're probably heading to the
situation where one maintainer wants them fixed and another doesn't...
I have no opinion either way. Personally, I'd have used snprintf()
here to start with so at least stack corruption can't happen, and
the worst that can happen is the string gets truncated, and the
following clk lookup fails, resulting in an error returned during
the driver probe.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] imx-drm: ipu-dc: Use usleep_range instead of msleep
On Tue, Feb 25, 2014 at 12:43:42PM +0100, Philipp Zabel wrote:
Since msleep(2) can sleep up to 20ms anyway, make this explicit by using
usleep_range(2000, 2).
Signed-off-by: Philipp Zabel p.za...@pengutronix.de
---
drivers/staging/imx-drm/ipu-v3/ipu-dc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
index d0e3bc3..d5de8bb 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
@@ -262,7 +262,7 @@ void ipu_dc_disable_channel(struct ipu_dc *dc)
/* Wait for DC triple buffer to empty */
while ((readl(priv-dc_reg + DC_STAT) val) != val) {
- msleep(2);
+ usleep_range(2000, 2);
timeout -= 2;
if (timeout = 0)
break;
Umm, something which Greg may have missed.
timeout is 50. It gets decremented by two each time around this loop,
so we could end up waiting between 100ms and 1s here, yes? The exact
delay here would depend on the kernel HZ value (which is configurable,
up to 1kHz.)
So... we're using a sleeping function here (msleep or usleep_range),
and so interrupts can't be disabled here, nor can spinlocks be held,
so I'm left wondering why the code doesn't do:
timeout = jiffies + msecs_to_jiffies(100);
do {
if (readl(priv-dc_reg + DC_STAT) val) == val)
break;
usleep_range(2000, 2);
} while (time_is_after_jiffies(timeout));
here. (Note that whether we succeed or timeout appears to have no
effect on the following code.)
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 01/11] staging: imx-drm-core: Use OF graph to find components and connections between encoder and crtcs
On Wed, Mar 05, 2014 at 10:20:52AM +0100, Philipp Zabel wrote:
+struct imx_drm_component {
+ struct device_node *of_node;
+ struct list_head list;
+};
+
The only thing this structure appears to be doing is ensuring that a
single component doesn't get added twice - is that correct? If so,
(and the troublesome problem with the IPU crtcs is now gone)
we can modify the core component code such that it does this:
if (c-master c-master != master)
continue;
if (compare(c-dev, compare_data)) {
if (!c-master)
component_attach_master(master, c);
ret = 0;
break;
}
which will mean that you don't need to build this list anymore to track
what will be added - though I'd like to think a little more about that
before making that change. Please confirm whether this will eliminate
your list generation.
Thanks.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] iio: mxs-lradc: use helper functions to simplify the code
On Thu, Sep 05, 2013 at 05:15:02PM -0300, Fabio Estevam wrote:
Looks good, just one minor suggestion:
On Thu, Sep 5, 2013 at 3:16 PM, Dan Carpenter dan.carpen...@oracle.com
wrote:
+static void lradc_reg_set(struct mxs_lradc *lradc, u32 val, size_t chan)
+{
+ writel(val, lradc-base + chan + STMP_OFFSET_REG_SET);
+}
+
+static void lradc_reg_clear(struct mxs_lradc *lradc, u32 val, size_t chan)
+{
+ writel(val, lradc-base + chan + STMP_OFFSET_REG_CLR);
+}
Maybe 'static inline' ?
GCC will already inline these if it thinks they're appropriate for that
treatment.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/51] DMA mask changes
This started out as a request to look at the DMA mask situation, and how to solve the issues which we have on ARM - notably how the DMA mask should be setup. However, I started off reviewing how the dma_mask and coherent_dma_mask was being used, and what I found was rather messy, and in some cases rather buggy. I tried to get some of the bug fixes in before the last merge window, but it seems that the maintainers preferred to have the full solution rather than a simple -rc suitable bug fix. So, this is an attempt to clean things up. The first point here is that drivers performing DMA should be calling dma_set_mask()/dma_set_coherent_mask() in their probe function to verify that DMA can be performed. Lots of ARM drivers omit this step; please refer to the DMA API documentation on this subject. What this means is that the DMA mask provided by bus code is a default value - nothing more. It doesn't have to accurately reflect what the device is actually capable of. Apart from the storage for dev-dma_mask being initialised for any device which is DMA capable, there is no other initialisation which is strictly necessary at device creation time. Now, these cleanups address two major areas: 1. The setting of DMA masks, particularly when both the coherent and streaming DMA masks are set together. 2. The initialisation of DMA masks by drivers - this seems to be becoming a popular habbit, one which may not be entirely the right solution. Rather than having this scattered throughout the tree, I've pulled that into a central location (and called it coercing the DMA mask - because it really is about forcing the DMA mask to be that value.) 3. Finally, addressing the long held misbelief that DMA masks somehow correspond with physical addresses. We already have established long ago that dma_addr_t values returned from the DMA API are the values which you program into the DMA controller, and so are the bus addresses. It is _only_ sane that DMA masks are also bus related too, and not related to physical address spaces. (3) is a very important point for LPAE systems, which may still have less than 4GB of memory, but this memory is all located above the 4GB physical boundary. This means with the current model, any device using a 32-bit DMA mask fails - even though the DMA controller is still only a 32-bit DMA controller but the 32-bit bus addresses map to system memory. To put it another way, the bus addresses have a 4GB physical offset on them. This email is only being sent to the mailing lists in question, not to anyone personally. The list of individuals is far to great to do that. I'm hoping no mailing lists reject the patches based on the number of recipients. Patches based on v3.12-rc1. Documentation/DMA-API-HOWTO.txt | 37 +-- Documentation/DMA-API.txt |8 +++ arch/arm/include/asm/dma-mapping.h|8 +++ arch/arm/mm/dma-mapping.c | 49 ++-- arch/arm/mm/init.c| 12 +++--- arch/arm/mm/mm.h |2 + arch/powerpc/kernel/vio.c |3 +- block/blk-settings.c |8 ++-- drivers/amba/bus.c|6 +-- drivers/ata/pata_ixp4xx_cf.c |5 ++- drivers/ata/pata_octeon_cf.c |5 +- drivers/block/nvme-core.c | 10 ++--- drivers/crypto/ixp4xx_crypto.c| 48 ++-- drivers/dma/amba-pl08x.c |5 ++ drivers/dma/dw/platform.c |8 +-- drivers/dma/edma.c|6 +-- drivers/dma/pl330.c |4 ++ drivers/firmware/dcdbas.c | 23 +- drivers/firmware/google/gsmi.c| 13 +++-- drivers/gpu/drm/exynos/exynos_drm_drv.c |6 ++- drivers/gpu/drm/omapdrm/omap_dmm_tiler.c |5 +- drivers/media/platform/omap3isp/isp.c |6 +- drivers/media/platform/omap3isp/isp.h |3 - drivers/mmc/card/queue.c |3 +- drivers/mmc/host/sdhci-acpi.c |5 +- drivers/net/ethernet/broadcom/b44.c |3 +- drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |8 +--- drivers/net/ethernet/brocade/bna/bnad.c | 13 ++ drivers/net/ethernet/emulex/benet/be_main.c | 12 + drivers/net/ethernet/intel/e1000/e1000_main.c |9 +--- drivers/net/ethernet/intel/e1000e/netdev.c| 18 +++- drivers/net/ethernet/intel/igb/igb_main.c | 18 +++- drivers/net/ethernet/intel/igbvf/netdev.c | 18 +++- drivers/net/ethernet/intel/ixgb/ixgb_main.c | 16 ++- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
Re: [PATCH 00/51] DMA mask changes
On Thu, Sep 26, 2013 at 10:23:08PM +0200, Rafał Miłecki wrote: 2013/9/19 Russell King - ARM Linux li...@arm.linux.org.uk: This email is only being sent to the mailing lists in question, not to anyone personally. The list of individuals is far to great to do that. I'm hoping no mailing lists reject the patches based on the number of recipients. Huh, I think it was enough to send only 3 patches to the b43-dev@. Like: [PATCH 01/51] DMA-API: provide a helper to set both DMA and coherent DMA masks [PATCH 14/51] DMA-API: net: b43: (...) [PATCH 15/51] DMA-API: net: b43legacy: (...) ;) I believe Joe has some nice script for doing it that way. When fixing some coding style / formatting, he sends only related patches to the given ML. If I did that, then the mailing lists would not get the first patch, because almost none of the lists would be referred to by patch 1. Moreover, people complain when they don't have access to the full patch series - they assume patches are missing for some reason, and they ask for the rest of the series. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support
On Thu, Oct 03, 2013 at 03:51:26PM -0300, Fabio Estevam wrote:
diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index 9e8ae11..65e54b4 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -88,3 +88,7 @@
crtcs = ipu1 0, ipu1 1;
};
};
+
+hdmi {
+ crtcs = ipu1 0, ipu1 1;
+}
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index f024ef2..d2467f5 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -159,3 +159,7 @@
crtcs = ipu1 0, ipu1 1, ipu2 0, ipu2 1;
};
};
+
+hdmi {
+ crtcs = ipu1 0, ipu1 1, ipu2 0, ipu2 1;
+};
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index ccd55c2..b148cb3 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1301,6 +1301,16 @@
};
};
+ hdmi: hdmi@012 {
+ compatible = fsl,imx6q-hdmi;
+ reg = 0x0012 0x9000;
+ interrupts = 0 115 0x04;
+ gpr = gpr;
+ clocks = clks 123, clks 124;
+ clock-names = iahb, isfr;
+ status = disabled;
+ };
+
dcic1: dcic@020e4000 {
reg = 0x020e4000 0x4000;
interrupts = 0 124 0x04;
Shouldn't the above be in patch 1 (or 1.5) rather than patch 2? Patch 2
advertises itself as adding support for the wandboard, but in actual
fact it's adding the generic bits too.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support
On Mon, Oct 14, 2013 at 06:40:30PM +0100, Russell King - ARM Linux wrote:
Another thing that my build testing (and use on cubox-i) picked up:
On Thu, Oct 03, 2013 at 03:51:26PM -0300, Fabio Estevam wrote:
diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index 9e8ae11..65e54b4 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -88,3 +88,7 @@
crtcs = ipu1 0, ipu1 1;
};
};
+
+hdmi {
+ crtcs = ipu1 0, ipu1 1;
+}
Missing semi-colon.
Okay, next question(s).
1. How well tested is any of this imx-drm stuff?
2. What sort of testing has it been subject to?
Answers to these two questions may help stop me wasting a lot of time
chasing what is a really weird bug.
So, I have X up and running on the Cubox-i carrier-1, using the imx-drm
stuff (I've slightly hacked my xf86-armada X driver to get this working.)
This works fine - it detects the connectors, selects an appropriate
video mode and produces a picture of the correct shape and size.
However... I see really weird effects colouring effects - almost like
water over the image. It's certain colour transitions in the image
which seem to trigger this. There are also certain pixels which
twinkle.
Text looks very strange too. Rather than the font being crisp and clear,
it looks like there's red and green shift to it - but its not that it's
all shifted in that way.
Now, if I use the modetest utility from libdrm-2.4.43 to display a SMPTE
test pattern, this again looks right, but there are several vertical
single pixel lines of noise. The most striking one is below the upper
red bar in the lower dark area. I have a single pixel noisy vertical
line.
I've tried the kernel which Rabeeh supplied, which is based on BSP 4.1.0,
and this works fine as far as I can tell with the same image (though,
it's a little difficult converting the XWD dump to something that can
be directly poked into /dev/fb0..., although this results in R/B swap.)
Any ideas where to start looking?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support
On Mon, Oct 14, 2013 at 11:47:17PM -0300, Fabio Estevam wrote: On Mon, Oct 14, 2013 at 2:38 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: Shouldn't the above be in patch 1 (or 1.5) rather than patch 2? Patch 2 advertises itself as adding support for the wandboard, but in actual fact it's adding the generic bits too. Thanks for your review. Will address your comments in v3. Philipp Zabel mentioned that he will move imx-drm out of staging, so will send v3 after Philipp's patch gets into linux-next. That's unfortunate, especially given the bug with the clock polarity (which, although I can tweak the register directly, it's not obvious that changing the clk_pol initialisation is the correct solution.) There's also another issue here: the lack of DRM prime support. As you have a separate GPU and separate VPU, you really need dmabuf support implemented so that you can hand your scanout buffers/overlays to other subsystems. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
Another point on patch 1. Sorry, I don't have patch 1 to reply to, it
seems it was deleted from linux-arm-kernel's moderation queue.
drm_mode_connector_attach_encoder() is called too early, before the
base.id field in the encoder has been initialised. This causes the
connectors encoder array to be empty, and userspace KMS to fail.
There's also bugs in the CSC setting too - it runs off the end of the
array and gcc warns about this. The code was clearly wrong.
You may wish to combine this patch with patch 1 to sort all that out.
For the patch below:
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Tested-by: Russell King rmk+ker...@arm.linux.org.uk
diff --git a/drivers/staging/imx-drm/imx-hdmi.c
b/drivers/staging/imx-drm/imx-hdmi.c
index e227eb1..ca0450b 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/staging/imx-drm/imx-hdmi.c
@@ -507,7 +507,7 @@ static void imx_hdmi_update_csc_coeffs(struct imx_hdmi
*hdmi)
hdmi_writeb(hdmi, ((*csc_coeff)[0][2] 0xff), HDMI_CSC_COEF_A3_LSB);
hdmi_writeb(hdmi, ((*csc_coeff)[0][2] 8), HDMI_CSC_COEF_A3_MSB);
hdmi_writeb(hdmi, ((*csc_coeff)[0][3] 0xff), HDMI_CSC_COEF_A4_LSB);
- hdmi_writeb(hdmi, ((*csc_coeff)[0][4] 8), HDMI_CSC_COEF_A4_MSB);
+ hdmi_writeb(hdmi, ((*csc_coeff)[0][3] 8), HDMI_CSC_COEF_A4_MSB);
hdmi_writeb(hdmi, ((*csc_coeff)[1][0] 0xff), HDMI_CSC_COEF_B1_LSB);
hdmi_writeb(hdmi, ((*csc_coeff)[1][0] 8), HDMI_CSC_COEF_B1_MSB);
@@ -516,7 +516,7 @@ static void imx_hdmi_update_csc_coeffs(struct imx_hdmi
*hdmi)
hdmi_writeb(hdmi, ((*csc_coeff)[1][2] 0xff), HDMI_CSC_COEF_B3_LSB);
hdmi_writeb(hdmi, ((*csc_coeff)[1][2] 8), HDMI_CSC_COEF_B3_MSB);
hdmi_writeb(hdmi, ((*csc_coeff)[1][3] 0xff), HDMI_CSC_COEF_B4_LSB);
- hdmi_writeb(hdmi, ((*csc_coeff)[1][4] 8), HDMI_CSC_COEF_B4_MSB);
+ hdmi_writeb(hdmi, ((*csc_coeff)[1][3] 8), HDMI_CSC_COEF_B4_MSB);
hdmi_writeb(hdmi, ((*csc_coeff)[2][0] 0xff), HDMI_CSC_COEF_C1_LSB);
hdmi_writeb(hdmi, ((*csc_coeff)[2][0] 8), HDMI_CSC_COEF_C1_MSB);
@@ -525,7 +525,7 @@ static void imx_hdmi_update_csc_coeffs(struct imx_hdmi
*hdmi)
hdmi_writeb(hdmi, ((*csc_coeff)[2][2] 0xff), HDMI_CSC_COEF_C3_LSB);
hdmi_writeb(hdmi, ((*csc_coeff)[2][2] 8), HDMI_CSC_COEF_C3_MSB);
hdmi_writeb(hdmi, ((*csc_coeff)[2][3] 0xff), HDMI_CSC_COEF_C4_LSB);
- hdmi_writeb(hdmi, ((*csc_coeff)[2][4] 8), HDMI_CSC_COEF_C4_MSB);
+ hdmi_writeb(hdmi, ((*csc_coeff)[2][3] 8), HDMI_CSC_COEF_C4_MSB);
val = hdmi_readb(hdmi, HDMI_CSC_SCALE);
val = ~HDMI_CSC_SCALE_CSCSCALE_MASK;
@@ -1774,8 +1774,6 @@ static int imx_hdmi_register(struct imx_hdmi *hdmi)
{
int ret;
- drm_mode_connector_attach_encoder(hdmi-connector, hdmi-encoder);
-
hdmi-connector.funcs = imx_hdmi_connector_funcs;
hdmi-encoder.funcs = imx_hdmi_encoder_funcs;
@@ -1803,6 +1801,8 @@ static int imx_hdmi_register(struct imx_hdmi *hdmi)
hdmi-connector.encoder = hdmi-encoder;
+ drm_mode_connector_attach_encoder(hdmi-connector, hdmi-encoder);
+
return 0;
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
On Tue, Oct 15, 2013 at 10:17:07AM -0300, Fabio Estevam wrote:
On Tue, Oct 15, 2013 at 10:10 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
Another point on patch 1. Sorry, I don't have patch 1 to reply to, it
seems it was deleted from linux-arm-kernel's moderation queue.
drm_mode_connector_attach_encoder() is called too early, before the
base.id field in the encoder has been initialised. This causes the
connectors encoder array to be empty, and userspace KMS to fail.
There's also bugs in the CSC setting too - it runs off the end of the
array and gcc warns about this. The code was clearly wrong.
You may wish to combine this patch with patch 1 to sort all that out.
For the patch below:
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Tested-by: Russell King rmk+ker...@arm.linux.org.uk
Thanks, Russell.
Will submit v3 when I am back to the office.
Okay, I still have a problem with HDMI: I have a magenta vertical line
down the left hand side of the frame, the displayed frame is shifted
right by the width of that line and the right hand side is missing some
pixels.
First off, the hsync position programmed into the HDMI registers appears
to be wrong.
I'm at a loss why imx-hdmi is obfuscated with a conversion from a
drm_display_mode structure to a fb_videomode. This adds additional
confusion and additional opportunities for bugs; this is probably
exactly why the hsync position is wrong.
In CEA-861-B for 720p @60Hz:
DE: __^^^
HS: ___^^^___
^ ^ ^
| | 220 clocks
| 40 clocks
110 clocks
The IMX6 manual says HSYINCINDELAY0 is Hsync active edge delay. Integer
number of pixel clock cycles from de non-active edge. So, this should be
110. Yet it ends up being programmed as 220, leading to a magenta vertical
bar down the left hand side of the display.
Now, if you look at Documentation/fb/framebuffer.txt, in this case, the
right margin should be 110 clocks, hsync len should be 40 and the left
margin should be 220 clocks.
However, this is not what your conversion to a fb_videomode does. It
reverses the left and right margin. A similar confusion also exists
in the conversion of the upper/lower margins too.
The DRM model is this (for 720p @60Hz):
0 1280 1390 1430 1650
|===||---|--|
^ ^^ ^ ^
start hdisplay hsync_start hsync_end htotal
The fb model is the same as the above but rotated:
left margindisplayedright margin hsync_len
|--|===||---|
So, the left margin is the bit between hsync_end and htotal, and the
right margin is the bit between hdisplay and hsync_start. Exactly
the same analysis applies to the upper/lower margins.
What I suggest is that the use of fb_videomode is entirely killed off
in this driver to remove this layer of confusion and instead the DRM
model is stuck to within this DRM driver.
Now on to the next problem. HSYNC/VSYNC polarity.
So, this is the mode which is set:
1280x720 (0x41) 74.2MHz +HSync +VSync *current +preferred
h: width 1280 start 1390 end 1430 total 1650 skew0 clock 45.0KHz
v: height 720 start 725 end 730 total 750 clock 60.0Hz
Note the positive HSync and VSync polarity - this is correct, backed
up by CEA-861-B.
The IPU DI0 is configured thusly in its general control register:
0x264 = 0x26, which is what is set when the requested mode
has DRM_MODE_FLAG_PHSYNC and DRM_MODE_FLAG_PVSYNC flags set.
However, if we look at the HDMI config: 0x121000 = 0x18. Active low
hsync/vsync. This is quite obvious when you look at the code -
convert_to_video_timing() does *nothing* at all when converting the
sync polarities, which means hdmi_av_composer() doesn't program them
appropriately. And yes, poking 0x78 into this register finally fixes
the problem.
Yet another reason why this silly conversion from one structure form
to another is a Very Bad Idea. Until I found this, I was merely going
to send a patch to sort out convert_to_video_timing(), but quite frankly
I'm going to kill this thing off right now.
Another thing:
static int imx_hdmi_setup(struct imx_hdmi *hdmi, struct drm_display_mode *mode)
{
int ret;
convert_to_video_timing(hdmi-fb_mode, mode);
hdmi_disable_overflow_interrupts(hdmi);
hdmi-vic = 6;
It's quite wrong to force every video mode set to be CEA mode 6. IIRC,
There is a function in DRM which will tell you the CEA mode number.
Again, I'll fix this in my patch rewriting this bit of the driver to
be correct.
I'm also suspicious of the HDMI Initialization Step comments, because
they make no sense:
/* HDMI Initialization Step B.4 */
static void imx_hdmi_enable_video_path(struct imx_hdmi *hdmi)
{
yet
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
On Wed, Oct 16, 2013 at 12:37:42PM -0700, Troy Kisky wrote:
On 10/16/2013 10:03 AM, Russell King - ARM Linux wrote:
On Tue, Oct 15, 2013 at 10:17:07AM -0300, Fabio Estevam wrote:
On Tue, Oct 15, 2013 at 10:10 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
Another point on patch 1. Sorry, I don't have patch 1 to reply to, it
seems it was deleted from linux-arm-kernel's moderation queue.
drm_mode_connector_attach_encoder() is called too early, before the
base.id field in the encoder has been initialised. This causes the
connectors encoder array to be empty, and userspace KMS to fail.
There's also bugs in the CSC setting too - it runs off the end of the
array and gcc warns about this. The code was clearly wrong.
You may wish to combine this patch with patch 1 to sort all that out.
For the patch below:
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Tested-by: Russell King rmk+ker...@arm.linux.org.uk
Thanks, Russell.
Will submit v3 when I am back to the office.
Okay, I still have a problem with HDMI: I have a magenta vertical line
down the left hand side of the frame, the displayed frame is shifted
right by the width of that line and the right hand side is missing some
pixels.
First off, the hsync position programmed into the HDMI registers appears
to be wrong.
I'm at a loss why imx-hdmi is obfuscated with a conversion from a
drm_display_mode structure to a fb_videomode. This adds additional
confusion and additional opportunities for bugs; this is probably
exactly why the hsync position is wrong.
In CEA-861-B for 720p @60Hz:
DE: __^^^
HS: ___^^^___
^ ^ ^
| | 220 clocks
| 40 clocks
110 clocks
The IMX6 manual says HSYINCINDELAY0 is Hsync active edge delay. Integer
number of pixel clock cycles from de non-active edge. So, this should be
110. Yet it ends up being programmed as 220, leading to a magenta vertical
bar down the left hand side of the display.
Now, if you look at Documentation/fb/framebuffer.txt, in this case, the
right margin should be 110 clocks, hsync len should be 40 and the left
margin should be 220 clocks.
However, this is not what your conversion to a fb_videomode does. It
reverses the left and right margin. A similar confusion also exists
in the conversion of the upper/lower margins too.
The DRM model is this (for 720p @60Hz):
0 1280 1390 1430 1650
|===||---|--|
^ ^^ ^ ^
start hdisplay hsync_start hsync_end htotal
The fb model is the same as the above but rotated:
left margindisplayedright margin hsync_len
|--|===||---|
So, the left margin is the bit between hsync_end and htotal, and the
right margin is the bit between hdisplay and hsync_start. Exactly
the same analysis applies to the upper/lower margins.
What I suggest is that the use of fb_videomode is entirely killed off
in this driver to remove this layer of confusion and instead the DRM
model is stuck to within this DRM driver.
Now on to the next problem. HSYNC/VSYNC polarity.
So, this is the mode which is set:
1280x720 (0x41) 74.2MHz +HSync +VSync *current +preferred
h: width 1280 start 1390 end 1430 total 1650 skew0 clock
45.0KHz
v: height 720 start 725 end 730 total 750 clock
60.0Hz
Note the positive HSync and VSync polarity - this is correct, backed
up by CEA-861-B.
The IPU DI0 is configured thusly in its general control register:
0x264 = 0x26, which is what is set when the requested mode
has DRM_MODE_FLAG_PHSYNC and DRM_MODE_FLAG_PVSYNC flags set.
However, if we look at the HDMI config: 0x121000 = 0x18. Active low
hsync/vsync. This is quite obvious when you look at the code -
convert_to_video_timing() does *nothing* at all when converting the
sync polarities, which means hdmi_av_composer() doesn't program them
appropriately. And yes, poking 0x78 into this register finally fixes
the problem.
Yet another reason why this silly conversion from one structure form
to another is a Very Bad Idea. Until I found this, I was merely going
to send a patch to sort out convert_to_video_timing(), but quite frankly
I'm going to kill this thing off right now.
Another thing:
static int imx_hdmi_setup(struct imx_hdmi *hdmi, struct drm_display_mode
*mode)
{
int ret;
convert_to_video_timing(hdmi-fb_mode, mode);
hdmi_disable_overflow_interrupts(hdmi);
hdmi-vic = 6;
It's quite wrong to force every video mode set to be CEA mode 6. IIRC,
There is a function in DRM which will tell you the CEA mode number.
Again, I'll fix this in my patch rewriting this bit of the driver to
be correct.
I'm also
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
On Wed, Oct 16, 2013 at 02:03:17PM -0700, Troy Kisky wrote:
Freescale's kernel(imx_3.0.35_4.1.0) has this code
video/mxc_hdmi.c-/* Workaround to clear the overflow condition */
video/mxc_hdmi.c-static void mxc_hdmi_clear_overflow(void)
video/mxc_hdmi.c-{
video/mxc_hdmi.c- int count;
video/mxc_hdmi.c- u8 val;
video/mxc_hdmi.c-
video/mxc_hdmi.c- /* TMDS software reset */
video/mxc_hdmi.c: hdmi_writeb((u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ,
HDMI_MC_SWRSTZ);
video/mxc_hdmi.c-
video/mxc_hdmi.c- val = hdmi_readb(HDMI_FC_INVIDCONF);
video/mxc_hdmi.c-
video/mxc_hdmi.c- if (cpu_is_mx6dl()) {
video/mxc_hdmi.c-hdmi_writeb(val, HDMI_FC_INVIDCONF);
video/mxc_hdmi.c-return;
video/mxc_hdmi.c- }
video/mxc_hdmi.c-
video/mxc_hdmi.c- for (count = 0 ; count 5 ; count++)
video/mxc_hdmi.c- hdmi_writeb(val, HDMI_FC_INVIDCONF);
video/mxc_hdmi.c-}
So, perhaps you need to move the software reset first.
Just tried that - and yes, it does work (I'm on i.MX 6Solo for which
cpu_is_mx6dl() would return true.) Well done!
Indeed yes, the workaround in the code Fabio has differs from the
procedure given in the errata.
Note that this gives a new problem: we shouldn't use cpu_is_mx6dl()
in drivers - differences like this should be specified via DT properties.
I think we need this to recognise both fsl,imx6q-hdmi and fsl,imx6dl-hdmi
so that this workaround can detect when its running on a Solo/DL SoC.
Well, I now have quite a pile of patches for the hdmi code. :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
Okay, next problem... As I described via the Cubox-i community last night on google+... I'm now at the point where certain resolutions and refreshes work fine (eg, 720p @ 50 or 60Hz, 1366x768, 1024x768). Others either don't display (1080p, 800x600, 848x480, 640x480), or have speckles, a line of corruption that flashes up, and blanks (576p @50 Hz, 480p @60Hz), possibly a result of loss of signal. I've been looking at the DMFC, suspecting a fifo problem, but that to me looks fine - we seem to always allocate 4 slots, with each slot having a bandwidth of 99Mpixels each. This in theory should give a maximum of 396Mpixels, which is more than plenty. The bandwidth calculation is probably wrong though - the peak bandwidth is actually the pixel clock since this is the rate at which pixels have to be supplied during a line, not the refresh * hdisplayed * vdisplayed - that's the average bandwidth over a full frame. However, if it was a DMFC problem, and as this only carries pixel data, I'd expect that not to cause loss of signal. I've checked the phy MPLL settings back to the manual for certain problem clock rates, and they also seem fine. I've polled the status register to see whether it's unstable, and it appears to remain locked. I think I should probe the HDMI signals, particularly the clock signal to see if that provides any useful clues. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
On Wed, Oct 16, 2013 at 11:31:07AM -0700, Greg Kroah-Hartman wrote: On Wed, Oct 16, 2013 at 07:07:35PM +0100, Russell King - ARM Linux wrote: Sorry, but I don't think imx-drm is driving the hardware correctly, and I know that Greg wants it moved out of drivers/staging, but frankly it seems to be far from ready for that. Certainly the HDMI parts seems to be especially problematical. I want it out of staging if it's working properly. Yours is the first report of it not working properly, and in fact, probably one of the first users of the driver, as I haven't gotten any reports of it working or not at all over the years. Next problem... unbinding, rebinding, and then unbinding the imx-drm device produces the nice oops below. I've not debugged this yet. Alignment trap: not handling instruction e1932f9f at [c00758ec] Unhandled fault: alignment exception (0x001) at 0x6e72666f Internal error: : 1 [#1] SMP ARM Modules linked in: fuse bnep rfcomm bluetooth CPU: 0 PID: 1125 Comm: bash Tainted: GW3.12.0-rc4+ #123 task: d0d6ec00 ti: d7ff2000 task.ti: d7ff2000 PC is at __lock_acquire+0x1a8/0x1e14 LR is at lock_acquire+0x68/0x7c pc : [c00758f0]lr : [c0077aa8]psr: 20070093 sp : d7ff3cc8 ip : fp : d7ff3d54 r10: db2a6334 r9 : r8 : 6e72656b r7 : c0846208 r6 : c08852cc r5 : d0d6ec00 r4 : 0002 r3 : 6e72666f r2 : db2a6334 r1 : 0001 r0 : d7ff2000 Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user Control: 10c53c7d Table: 20df804a DAC: 0015 Process bash (pid: 1125, stack limit = 0xd7ff2240) Stack: (0xd7ff3cc8 to 0xd7ff4000) 3cc0: 0006 0007 c0cd22f0 0006 d7ff3d1c d7ff3ce8 3ce0: c00782f8 c0075050 0001 db801f00 d7ff2000 c0078648 d7ff2000 3d00: 0001 d7ff3e08 d7ff3e60 d7ff3dd8 d7ff3d3c d7ff3d20 d7ff3e08 3d20: d7ff3d7c d7ff3d30 c0072a58 d7ff2000 60070013 d0d6ec00 d7ff2000 3d40: c06679c8 d0d6ec00 d7ff3d8c d7ff3d58 c0077aa8 c0075754 0002 3d60: c02ff27c 0002 db2a62fc c02ff27c c08852cc 3d80: d7ff3de4 d7ff3d90 c05f80c4 c0077a4c 0002 c02ff27c c0072a20 3da0: dad64000 d7ff3e24 d7ff3df8 d7ff3e04 d7ff3e5c d0d6f000 c013f1c8 db322880 3dc0: db2a6440 db2a6000 c0872568 0008 c06679c8 db286000 d7ff3e04 d7ff3de8 3de0: c02ff27c c05f8074 db280f00 db322880 db2a6000 db29b810 d7ff3e1c d7ff3e08 3e00: c02f13ac c02ff268 d0e55000 c087265c d7ff3e2c d7ff3e20 c0464d7c c02f1398 3e20: d7ff3e54 d7ff3e30 c02f506c c0464d68 d0e55000 c087265c db29b810 c0872568 3e40: 0008 db286000 d7ff3e74 d7ff3e58 c02fa284 c02f503c c087265c c087265c 3e60: db29b810 0008 d7ff3e8c d7ff3e78 c02fb900 c02fa258 db29b810 c0872678 3e80: d7ff3e9c d7ff3e90 c0464d54 c02fb8d4 d7ff3eac d7ff3ea0 c0312bfc c0464d44 3ea0: d7ff3ec4 d7ff3eb0 c03113b0 c0312be8 db29b844 db29b810 d7ff3edc d7ff3ec8 3ec0: c0311434 c0311344 c0872678 c085b588 d7ff3efc d7ff3ee0 c03103ac c0311418 3ee0: c941b300 c941b318 d7ff3f70 db28c280 d7ff3f0c d7ff3f00 c030f934 c0310338 3f00: d7ff3f3c d7ff3f10 c013d7d0 c030f918 d7ff3f70 dbb5e600 0008 003ba408 3f20: d7ff3f70 0008 d7ff3f6c d7ff3f40 c00db77c c013d6d4 3f40: 0001 c000eb44 dbb5e600 d7ff3f70 003ba408 0008 3f60: d7ff3fa4 d7ff3f70 c00dbb58 c00db6b8 d7ff3f94 b6f7fa78 3f80: 0008 003ba408 0004 c000eb44 d7ff2000 d7ff3fa8 3fa0: c000e980 c00dbb18 b6f7fa78 0008 0001 003ba408 0008 3fc0: b6f7fa78 0008 003ba408 0004 bee5c9ec 000a6094 003f7f08 3fe0: bee5c96c b6eee747 b6f2611c 40070010 0001 0138 0020 Backtrace: [c0075748] (__lock_acquire+0x0/0x1e14) from [c0077aa8] (lock_acquire+0x68/0x7c) [c0077a40] (lock_acquire+0x0/0x7c) from [c05f80c4] (mutex_lock_nested+0x5c/0x394) r7: r6:c08852cc r5:c02ff27c r4:db2a62fc [c05f8068] (mutex_lock_nested+0x0/0x394) from [c02ff27c] (drm_modeset_lock_all+0x20/0x58) [c02ff25c] (drm_modeset_lock_all+0x0/0x58) from [c02f13ac] (drm_fbdev_cma_restore_mode+0x20/0x34) r6:db29b810 r5:db2a6000 r4:db322880 r3:db280f00 [c02f138c] (drm_fbdev_cma_restore_mode+0x0/0x34) from [c0464d7c] (imx_drm_driver_lastclose+0x20/0x24) r5:c087265c r4:d0e55000 [c0464d5c] (imx_drm_driver_lastclose+0x0/0x24) from [c02f506c] (drm_lastclose+0x3c/0x174) [c02f5030] (drm_lastclose+0x0/0x174) from [c02fa284] (drm_put_dev+0x38/0x154) [c02fa24c] (drm_put_dev+0x0/0x154) from [c02fb900] (drm_platform_exit+0x38/0x5c) r7:0008 r6:db29b810 r5:c087265c r4:c087265c [c02fb8c8] (drm_platform_exit+0x0/0x5c) from [c0464d54] (imx_drm_platform_remove+0x1c/0x24) r5:c0872678 r4:db29b810 [c0464d38] (imx_drm_platform_remove+0x0/0x24) from [c0312bfc] (platform_drv_remove+0x20/0x24) [c0312bdc] (platform_drv_remove+0x0/0x24) from [c03113b0] (__device_release_driver+0x78/0xd4) [c0311338] (__device_release_driver+0x0/0xd4) from [c0311434] (device_release_driver+0x28/0x34) r5:db29b810 r4:db29b844 [c031140c
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
Another problem. After performing several modesets, the IPU seems to lock up and produce no syncs or output data. I've seen this many times over the last week while testing out various aspects of imx-drm, and had put it down to problems with the clocking arrangement getting its settings wrong. Now that I've sorted all that though, and I still have the problem, there's something else going on. What I see is: - the HDMI clock is running correctly (right frequency and unmodulated) - the TMDS data lines show signs of there being some data (probably control, guard bands and data islands from the frame composer in the HDMI interface). The data lines are definitely lacking image data though. - reading the various status registers indicates that all FIFOs within the IPU are empty. - the attached TV says that there is no HDMI signal. One of my tests has been to cycle through all display resolutions from the smallest width to the largest, leaving each one set for 30 seconds. This will occasionally provoke the problem, but obviously is rather slow to do so. I tried this with a less demanding test last night as far as a change in the settings: switching between 720p at 50 and 60Hz. The clocks for these two modes are the same at 74.25MHz, and the vertical timing parameters are identical. The only timing difference is with the horizontal parameters: 1280x720 (0x41) 74.2MHz +HSync +VSync +preferred h: width 1280 start 1390 end 1430 total 1650 skew0 clock 45.0KHz v: height 720 start 725 end 730 total 750 clock 60.0Hz 1280x720 (0x4f) 74.2MHz +HSync +VSync h: width 1280 start 1720 end 1760 total 1980 skew0 clock 37.5KHz v: height 720 start 725 end 730 total 750 clock 50.0Hz This dies within a couple of minutes. I haven't gathered enough information to tell whether it always dies when switching from 50 - 60Hz or whether it's any switch. My test for this is basically: while :; do xrandr -s 1280x720 -r 50 sleep 5 xrandr -s 1280x720 -r 60 sleep 5 done ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
On Sun, Oct 20, 2013 at 02:00:57PM +0100, Russell King - ARM Linux wrote: As for imx-drm, there was a warning which preceded that oops. Here's the full log, below the - marker - this is from unbinding the imx-drm module, and then trying to reboot. imx-drm is really very broken in the way it tries to bend DRM to be used in DT - it doesn't consider the lifetime for anything like the CRTCs, connectors or encoders. All these have empty .destroy functions to them. If we unbind imx-drm, the top level drm_device tries to be destroyed, but it leaves behind all the CRTCs, connectors and encoders, causing the first warning because none of the framebuffers got cleaned up through that destruction (because the functions did nothing.) The second one is through trying to clean up the framebuffer, which is still in use. The third one is caused because there's still allocated memory objects against the DRM memory manager - again, because nothing has been cleaned up. Right, so how imx-drm works as far as DRM initialization is by a wing and a prayer at the moment. It works like this - the driver relies heavily upon this sequence: - imx_drm_init() - creates an imx_drm_device structure to contain references to other parts. - registers the imx-drm platform device and an associated structure. - the platform device is immediately probed, causing it to be registered with the DRM subsystem. - the DRM subsystem creates the drm_device structure, and calls the drivers -load method. - the driver initialises some basic data, places a pointer to the drm_device into the imx_drm_device and returns - imx_pd_driver_init() - registers the imx_pd_driver platform device driver for DT devices with a compatible string of fsl,imx-parallel-display - such devices will be immediately probed - these allocate an imx_parallel_display structure, which contains a drm_connector and drm_encoder structure embedded within. - these structures are registered into the core of imx_drm, and via the imx_drm_device structure, are both attached to the drm_device immediately. - imx_tve_driver_init() essentially the same as imx_pd_driver_init() - imx_ldb_driver_init() essentially the same as imx_pd_driver_init() - imx_ipu_driver_init() - registers a platform driver fot DT devices with a compatible string of fsl,imx51-ipu, fsl,imx53-ipu, or fsl,imx6q-ipu. - initialises such devices, and creates two new platform devices called imx-ipuv3-crtc, one for each display interface. - ipu_drm_driver_init() - registers a platform driver for imx-ipuv3-crtc devices. - for each device found - it allocates a ipu_crtc device structure, which embeds a drm_crtc structure. - it registers a CRTC via imx_drm_add_crtc(). - this allocates an imx_drm_crtc structure, and eventually registers the drm_crtc structure against the drm_device - imx_hdmi_driver_init similar to imx_pd_driver_init All that sequence is in init level 6. The last bit comes in init level 7 (the late_initcall): - imx_fb_helper_init() - this grabs the drm_device, and calls drm_fbdev_cma_init() on it hoping that we know the number of CRTCs at this point. This is held indefinitely. - the resulting drm_fbdev_cma is saved into the imx_drm_device. Now, if the imx-drm device is unbound from its driver, all hell breaks loose - none of these crtc/connector/encoder structures have a meaningful destroy function - their destruction is all in their individual driver remove functions. This causes some warnings to be spat out from DRM. Amongst this is the last_close callback which looks at the imx_drm_device, sees that drm_fbdev_cma is registered against it, and calls drm_fbdev_cma_restore_mode() on it. drm_fbdev_cma contains objects which store a pointer to the drm_device structure that it was registered against, which exists at this point, so everything is fine. The unload proceeds, and eventually the drm_device is freed. Now, if we rebind the imx-drm device, causing the probe actions above to be repeated. imx_drm_device still contains a pointer to the drm_fbdev_cma object that was allocated... Let's ignore the fact that none of the sub-modules have re-initialised anything against this new drm_device. The real fun comes when you try and unbind it again. This time, the drm_device which is being torn down isn't the one in drm_fbdev_cma, but we still call drm_fbdev_cma_restore_mode(). This tries to get a mutex on the _original_ drm_device-mode_config.mutex, which has been freed. The result is a kernel oops. Now, several things stand out here: piece-meal construction of a drm_device in this manner is unsafe: - it relies heavily on all devices already being present at the time that the above sequence starts, and it assumes that the drivers will probe immediately, as soon as they are registered. - the late_initcall() is really a barrier
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
On Sun, Oct 20, 2013 at 05:31:56PM +0100, Russell King - ARM Linux wrote: On Sun, Oct 20, 2013 at 02:00:57PM +0100, Russell King - ARM Linux wrote: As for imx-drm, there was a warning which preceded that oops. Here's the full log, below the - marker - this is from unbinding the imx-drm module, and then trying to reboot. imx-drm is really very broken in the way it tries to bend DRM to be used in DT - it doesn't consider the lifetime for anything like the CRTCs, connectors or encoders. All these have empty .destroy functions to them. If we unbind imx-drm, the top level drm_device tries to be destroyed, but it leaves behind all the CRTCs, connectors and encoders, causing the first warning because none of the framebuffers got cleaned up through that destruction (because the functions did nothing.) The second one is through trying to clean up the framebuffer, which is still in use. The third one is caused because there's still allocated memory objects against the DRM memory manager - again, because nothing has been cleaned up. Right, so how imx-drm works as far as DRM initialization is by a wing and a prayer at the moment. It works like this - the driver relies heavily upon this sequence: - imx_drm_init() - creates an imx_drm_device structure to contain references to other parts. - registers the imx-drm platform device and an associated structure. - the platform device is immediately probed, causing it to be registered with the DRM subsystem. - the DRM subsystem creates the drm_device structure, and calls the drivers -load method. - the driver initialises some basic data, places a pointer to the drm_device into the imx_drm_device and returns - imx_pd_driver_init() - registers the imx_pd_driver platform device driver for DT devices with a compatible string of fsl,imx-parallel-display - such devices will be immediately probed - these allocate an imx_parallel_display structure, which contains a drm_connector and drm_encoder structure embedded within. - these structures are registered into the core of imx_drm, and via the imx_drm_device structure, are both attached to the drm_device immediately. - imx_tve_driver_init() essentially the same as imx_pd_driver_init() - imx_ldb_driver_init() essentially the same as imx_pd_driver_init() - imx_ipu_driver_init() - registers a platform driver fot DT devices with a compatible string of fsl,imx51-ipu, fsl,imx53-ipu, or fsl,imx6q-ipu. - initialises such devices, and creates two new platform devices called imx-ipuv3-crtc, one for each display interface. - ipu_drm_driver_init() - registers a platform driver for imx-ipuv3-crtc devices. - for each device found - it allocates a ipu_crtc device structure, which embeds a drm_crtc structure. - it registers a CRTC via imx_drm_add_crtc(). - this allocates an imx_drm_crtc structure, and eventually registers the drm_crtc structure against the drm_device - imx_hdmi_driver_init similar to imx_pd_driver_init All that sequence is in init level 6. The last bit comes in init level 7 (the late_initcall): - imx_fb_helper_init() - this grabs the drm_device, and calls drm_fbdev_cma_init() on it hoping that we know the number of CRTCs at this point. This is held indefinitely. - the resulting drm_fbdev_cma is saved into the imx_drm_device. Now, if the imx-drm device is unbound from its driver, all hell breaks loose - none of these crtc/connector/encoder structures have a meaningful destroy function - their destruction is all in their individual driver remove functions. This causes some warnings to be spat out from DRM. Amongst this is the last_close callback which looks at the imx_drm_device, sees that drm_fbdev_cma is registered against it, and calls drm_fbdev_cma_restore_mode() on it. drm_fbdev_cma contains objects which store a pointer to the drm_device structure that it was registered against, which exists at this point, so everything is fine. The unload proceeds, and eventually the drm_device is freed. Now, if we rebind the imx-drm device, causing the probe actions above to be repeated. imx_drm_device still contains a pointer to the drm_fbdev_cma object that was allocated... Let's ignore the fact that none of the sub-modules have re-initialised anything against this new drm_device. The real fun comes when you try and unbind it again. This time, the drm_device which is being torn down isn't the one in drm_fbdev_cma, but we still call drm_fbdev_cma_restore_mode(). This tries to get a mutex on the _original_ drm_device-mode_config.mutex, which has been freed. The result is a kernel oops. Now, several things stand out here: piece-meal construction of a drm_device in this manner is unsafe: - it relies heavily on all devices already being present at the time
Re: [PATCH v3] imx-drm: Add mx6 hdmi transmitter support
On Mon, Oct 28, 2013 at 06:26:49PM -0200, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com This is based on the initial work done by Sascha Hauer and Tony Prisk. It looks like you've also taken some of the suggestions I've made too - like the voltage drive and symbol transmit control settings. It would be nice to identify the changes a bit better so that those of us who are testing your drivers (and have patches on top of them to make things work) know what's changed. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] imx-drm: Add mx6 hdmi transmitter support
On Tue, Oct 29, 2013 at 01:45:50PM -0200, Fabio Estevam wrote: On Tue, Oct 29, 2013 at 1:00 PM, Fabio Estevam feste...@gmail.com wrote: Today I was able to get access to a wandboard solo and the HDMI image does look different compared to a wandboard quad board: - The Linux logo penguins are steady on mx6q, but on mx6solo the contour lines look like they flicker. - The kernel log text appears in good white colour with mx6q, but not so white on mx6solo (tending to green). Even the U-boot HDMI splash screen is not very steady on mx6solo. I will try to look at these mx6 solo HDMI issues. With the change below I am able to get a good HDMI quality image on mx6solo (and mx6quad as well): Yes, I need that too on the Carrier-1 board, as suggested by Sascha. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] imx-drm: ipuv3-crtc: Invert IPU DI0 clock polarity
On Tue, Oct 29, 2013 at 07:42:22PM -0200, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com sig_cfg.clk_pol controls the 'di0_polarity_disp_clk' bit of register IPUx_DI0_GENERAL through the following code in imx-drm/ipu-v3/ipu-di.c: if (!sig-clk_pol) di_gen |= DI_GEN_POLARITY_DISP_CLK; With 'di0_polarity_disp_clk' bit set we do not have stable HDMI output on mx6solo: contours of pictures look jittery and the white colour does not appear really white. Russell King initially reported this problem at: http://www.spinics.net/lists/arm-kernel/msg279805.html There's slightly more to this than that mail, and I have to raise the issue about whether this is the correct fix or not. See: http://www.spinics.net/lists/arm-kernel/msg279902.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] imx-drm: Add mx6 hdmi transmitter support
On Tue, Oct 29, 2013 at 08:01:46PM -0200, Fabio Estevam wrote: Hi Greg, On Mon, Oct 28, 2013 at 7:03 PM, Greg KH gre...@linuxfoundation.org wrote: And why is new support being added to this driver instead of working to get it out of staging? Philipp Zabel mentioned that he will submit the imx-drm driver out of staging: http://www.spinics.net/lists/dri-devel/msg46784.html The issues surrounding DT and DRM were given an airing last week at kernel summit in a session chaired by David Airlie. The Kernel Summit session was to discuss issues surrounding DT and DRM, or more specifically DT and componentised subsystems like DRM, ASoC and V4L2. Grant Likely has said that he will attempt to summarise some of the discussion there; unfortunately I'm not aware of anyone making any real minutes of what was discussed - it wasn't a formal session. I also had discussions with David Airlie and, separately, with the Pengutronix folk about imx-drm. It is very clear to me that David is against imx-drm moving into drivers/gpu/drm - the driver requires an amount of rework in various places. So I don't think it will move out of drivers/staging any time soon. I have a few other bits to sort out first, but I've volunteered to try to resolve some of these blocking issues so that imx-drm can move forward and hopefully out of staging. To put it another way, moving this driver out of staging is not as simple as moving the files; it requires real work to address the issues the subsystem maintainer has with this code. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] imx-drm: Add mx6 hdmi transmitter support
On Mon, Oct 28, 2013 at 06:26:49PM -0200, Fabio Estevam wrote:
+ /*Wait for PHY PLL lock */
+ msec = 4;
+ val = hdmi_readb(hdmi, HDMI_PHY_STAT0) HDMI_PHY_TX_PHY_LOCK;
+ while (!val) {
+ udelay(1000);
+ if (msec-- == 0) {
+ dev_dbg(hdmi-dev, PHY PLL not locked\n);
+ return -EINVAL;
+ }
+ val = hdmi_readb(hdmi, HDMI_PHY_STAT0) HDMI_PHY_TX_PHY_LOCK;
+ }
This loop is not always sufficient for the PLL to always lock. It's also
buggy because it behaves like this:
msec = 4
read register
check bit - still one?
yes - wait 1ms
msec == 0?
no - msec = 3
read register
... msec eventually msec = 0
read register
check bit - still one
yes - wait 1ms
msec == 0?
yes - print debug and exit
So the last wait for 1ms performs no real function other than to delay
the inevitable exit. If we really want to wait 5ms for the condition
we're testing to become true, then do it like this:
msec = 5;
do {
val = hdmi_readb(hdmi, HDMI_PHY_STAT0) HDMI_PHY_TX_PHY_LOCK;
if (!val)
break;
if (msec == 0) {
dev_err(hdmi-dev, PHY PLL not locked\n);
return -EINVAL;
}
udelay(1000);
msec--;
} while (1);
This way, we check for the success condition immediately before checking
for the failure condition, and only waiting after that.
I'd also suggest making it a dev_err() so that it's obvious when it
doesn't lock - the current dev_dbg() remains hidden and causes
failures which don't seem to be visible to the user via the logs.
Hence why this issue has been overlooked...
+static void hdmi_av_composer(struct imx_hdmi *hdmi,
+ const struct drm_display_mode *mode)
+{
+ u8 inv_val;
+ struct hdmi_vmode *vmode = hdmi-hdmi_data.video_mode;
+ int hblank, vblank, h_de_hs, v_de_vs, hsync_len, vsync_len;
+
+ vmode-mhsyncpolarity = !!(mode-flags DRM_MODE_FLAG_PHSYNC);
+ vmode-mvsyncpolarity = !!(mode-flags DRM_MODE_FLAG_PVSYNC);
+ vmode-minterlaced = !!(mode-flags DRM_MODE_FLAG_INTERLACE);
+ vmode-mpixelclock = mode-htotal * mode-vtotal *
+ drm_mode_vrefresh(mode);
We can do better here:
vmode-mpixelclock = mode-clock * 1000;
as the above is not quite correct when considering all the
possibilities. Since we're given the pixel clock in the mode structure,
we might as well make use it.
+ /* Set up VSYNC active edge delay (in pixel clks) */
+ vsync_len = mode-vsync_end - mode-vsync_start;
+ hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH);
Commentry - vsync len is in lines not pixel clocks.
Other than that, it seems to work here on the Carrier-1. Thanks.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 19/51] DMA-API: media: dt3155v4l: replace dma_set_mask()+dma_set_coherent_mask() with new helper
On Thu, Oct 31, 2013 at 09:46:40AM -0200, Mauro Carvalho Chehab wrote: Hi Russell, Em Mon, 30 Sep 2013 13:57:47 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: On 09/19/2013 11:44 PM, Russell King wrote: Replace the following sequence: dma_set_mask(dev, mask); dma_set_coherent_mask(dev, mask); with a call to the new helper dma_set_mask_and_coherent(). Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk Acked-by: Hans Verkuil hans.verk...@cisco.com Somehow, I lost your original post (I got unsubscribed on a few days from all vger mailing lists at the end of september). I suspect that you want to sent this via your tree, right? Yes please. If so: Acked-by: Mauro Carvalho Chehab m.che...@samsung.com Added, thanks. - err = dma_set_mask(pdev-dev, DMA_BIT_MASK(32)); - if (err) - return -ENODEV; - err = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32)); + err = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32)); if (err) return -ENODEV; One thing I've just noticed is that return should be return err not return -ENODEV - are you okay for me to change that in this patch? Thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] imx-drm: Add mx6 hdmi transmitter support
On Thu, Oct 31, 2013 at 06:45:41PM -0200, Fabio Estevam wrote: Hi Troy, On Thu, Oct 31, 2013 at 6:38 PM, Troy Kisky troy.ki...@boundarydevices.com wrote: I get a magenta line down the left side of the screen unless I replace the 5 with a 6. ie. +for (count = 0; count 6; count++) +hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF); This is with a imx6q processor. With the 6, the picture looks great! Troy It also works if I change the 5 to a 4! A 3 fails though. To summarize: 0 works 1 fails 2 works 3 fails 4 works 5 fails 6 works Very strange. Interesting. With the monitor I have tested I am not able to see this magenta line. Monitor or TV? Beware of overscans which will hide this effect. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3 1/8] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*
On Tue, Nov 12, 2013 at 05:49:18PM +0100, Denis Carikli wrote: diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index fc2adb6..586c12f 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -537,6 +537,15 @@ int drm_display_mode_from_videomode(const struct videomode *vm, dmode-flags |= DRM_MODE_FLAG_DBLSCAN; if (vm-flags DISPLAY_FLAGS_DOUBLECLK) dmode-flags |= DRM_MODE_FLAG_DBLCLK; + if (vm-flags DISPLAY_FLAGS_DE_LOW) + dmode-flags |= DRM_MODE_FLAG_DE_LOW; + if (vm-flags DISPLAY_FLAGS_DE_HIGH) + dmode-flags |= DRM_MODE_FLAG_DE_HIGH; + if (vm-flags DISPLAY_FLAGS_PIXDATA_POSEDGE) + dmode-flags |= DRM_MODE_FLAG_PIXDATA_POSEDGE; + if (vm-flags DISPLAY_FLAGS_PIXDATA_NEGEDGE) + dmode-flags |= DRM_MODE_FLAG_PIXDATA_NEGEDGE; + I'm still not convinced that these should be exposed in *any* way to userspace - I'm with Ville Syrjälä on this point. Yes, you've moved their definition out of a uapi header file, but they're still leaking out of kernel space via calls (and with Xorg, they'll leak into the DisplayMode structures within the X server.) Now, here's the thing... The polarity of the display enable signal. That's a property of the connected device right? It doesn't change with respect to the displayed mode unlike the hsync/vsync signals. If that's true, it should not be here. Same goes for the pixel clock edge. If it's a property of the connected device and doesn't have a dependence on the displayed mode, then it should not be in the DRM mode structure. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] imx-drm: imx-hdmi: fix hdmi hotplug detection initial state
On Mon, Jun 09, 2014 at 11:29:28AM -0300, Fabio Estevam wrote: On Mon, Jun 9, 2014 at 11:06 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: Please check the status in /sys/class/drm/card0-HDMI-A-1/status. This should report the current state of the hotplug detection. /sys/class/drm/card0-HDMI-A-1/status returns the correct state for HDMI cable connection. Good, so that means we're reporting the correct status to the DRM layer. Please post the kernel boot messages, one with the HDMI cable disconnected, and one with a HDMI sink connected. The HDMI undetected issue I am seeing on sabresd seems to be related to the simultaneous usage of HDMI and LVDS. If I remove the ldb node from the imx6qdl-sabresd.dtsi, then the HDMI cable is correctly detected and HDMI is shown right after boot. I wonder if the problem is that HDMI and LVDS are interfering with each other wrt the required pixel clock, and LVDS is winning. If we have HDMI enabled, many HDMI sinks will only work if we set one of their supported modes (with the dot clock within 1% - though some sinks are more lenient). I think the Novena people have run into this issue, but I've not seen much in the way of contributions back, despite them making use of my patches... -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] imx-drm: imx-hdmi: fix hdmi hotplug detection initial state
On Mon, Jun 09, 2014 at 03:15:16PM -0300, Fabio Estevam wrote: With HDMI cable connected (no image is seen on HDMI, only on lvds cable): imx-drm display-subsystem.11: bound 12.hdmi (ops hdmi_ops) imx-hdmi 12.hdmi: EVENT=plugin imx-hdmi 12.hdmi: Non-CEA mode used in HDMI imx-hdmi 12.hdmi: final pixclk = 0 Right, so it does know that the HDMI sink is connected... imx-drm display-subsystem.11: bound ldb.10 (ops imx_ldb_ops) imx-hdmi 12.hdmi: imx_hdmi_setup DVI mode imx-hdmi 12.hdmi: got edid: width[51] x height[28] imx-hdmi 12.hdmi: Non-CEA mode used in HDMI imx-hdmi 12.hdmi: final pixclk = 13850 imx-hdmi 12.hdmi: imx_hdmi_setup DVI mode imx-hdmi 12.hdmi: Non-CEA mode used in HDMI imx-hdmi 12.hdmi: final pixclk = 13850 imx-hdmi 12.hdmi: imx_hdmi_setup DVI mode And the mode it's setting has a pixel clock frequency of 138.5MHz here. If I disconnect/reconnect the HDMI cable (then image is seen on both HDMI and LVDS): root@freescale /$ imx-hdmi 12.hdmi: EVENT=plugout imx-hdmi 12.hdmi: EVENT=plugin imx-hdmi 12.hdmi: Non-CEA mode used in HDMI imx-hdmi 12.hdmi: final pixclk = 13850 imx-hdmi 12.hdmi: imx_hdmi_setup DVI mode imx-hdmi 12.hdmi: got edid: width[51] x height[28] imx-hdmi 12.hdmi: Non-CEA mode used in HDMI imx-hdmi 12.hdmi: final pixclk = 13850 imx-hdmi 12.hdmi: imx_hdmi_setup DVI mode imx-hdmi 12.hdmi: Non-CEA mode used in HDMI imx-hdmi 12.hdmi: final pixclk = 13850 imx-hdmi 12.hdmi: imx_hdmi_setup DVI mode And here it again configures 138.5MHz pixel clock - same as above. So, from this we can't see any difference in the setup, and we can say that it's not mode-clock which is the problem. Now booting the kernel with HDMI disconnected: imx-hdmi 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 imx-hdmi 12.hdmi: hdmi_set_clk_regenerator: samplerate=48000 ratio=100 pixelclk=7425 N=6144 cts=74250 imx-drm display-subsystem.11: bound 12.hdmi (ops hdmi_ops) Right, so here it knows that there's nothing connected. imx-drm display-subsystem.11: bound ldb.10 (ops imx_ldb_ops) Console: switching to colour frame buffer device 128x48 So it selects an initial mode based upon the LVDS device configuration. And after connecting the HDMI cable: imx-hdmi 12.hdmi: EVENT=plugin imx-hdmi 12.hdmi: Non-CEA mode used in HDMI imx-hdmi 12.hdmi: final pixclk = 0 imx-hdmi 12.hdmi: imx_hdmi_setup DVI mode imx-hdmi 12.hdmi: got edid: width[51] x height[28] imx-hdmi 12.hdmi: Non-CEA mode used in HDMI imx-hdmi 12.hdmi: final pixclk = 7880 imx-hdmi 12.hdmi: imx_hdmi_setup DVI mode imx-hdmi 12.hdmi: Non-CEA mode used in HDMI imx-hdmi 12.hdmi: final pixclk = 7880 imx-hdmi 12.hdmi: imx_hdmi_setup DVI mode Image is seen on both LVDS and HDMI monitor, but HDMI resolution is not correct (this is a different bug though). I'm guessing that here, DRM kept the original configuration rather than selecting one appropriate to the newly connected device. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
