[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected
From: Fabio Estevam fabio.estevam@freescale.com
Since commit d9b894603 (mx6sabresd: Add LVDS splash screen support) the following hang happens if the HDMI cable is not connected or the 'panel' variable is not set:
U-Boot 2013.10-rc2-12978-g47ac53d-dirty (Sep 11 2013 - 15:07:38)
CPU: Freescale i.MX6Q rev1.2 at 792 MHz Reset cause: POR Board: MX6-SabreSD DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2 ...
Provide a check to 'dev->detect' in order to prevent the hang.
Reported-by: Pardeep Kumar Singla b45784@freescale.com Suggested-by: Eric Bénard eric@eukrea.com Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - Check dev->detect
board/freescale/mx6sabresd/mx6sabresd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c index c832bd9..0f91fe2 100644 --- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -313,7 +313,7 @@ int board_video_skip(void) if (!panel) { for (i = 0; i < ARRAY_SIZE(displays); i++) { struct display_info_t const *dev = displays+i; - if (dev->detect(dev)) { + if (dev->detect && dev->detect(dev)) { panel = dev->mode.name; printf("auto-detected panel %s\n", panel); break;
From: Fabio Estevam fabio.estevam@freescale.com
If a HDMI cable is not connected, the following message is seen on boot:
CPU: Freescale i.MX6Q rev1.1 at 792 MHz Reset cause: POR Board: MX6-SabreSD DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2 No panel detected: default to HDMI unsupported panel HDMI
Reset the 'i' variable to fix the 'unsupported panel' message.
This follows the same idea of commit 47ac53d7ae (imx: nitrogen6x/mx6qsabrelite: Fix bug in board_video_skip).
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- board/freescale/mx6sabresd/mx6sabresd.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c index 0f91fe2..61fe67c 100644 --- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -322,6 +322,7 @@ int board_video_skip(void) if (!panel) { panel = displays[0].mode.name; printf("No panel detected: default to %s\n", panel); + i = 0; } } else { for (i = 0; i < ARRAY_SIZE(displays); i++) {
Hi Fabio,
On 11/09/2013 23:14, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
If a HDMI cable is not connected, the following message is seen on boot:
CPU: Freescale i.MX6Q rev1.1 at 792 MHz Reset cause: POR Board: MX6-SabreSD DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2 No panel detected: default to HDMI unsupported panel HDMI
Reset the 'i' variable to fix the 'unsupported panel' message.
This follows the same idea of commit 47ac53d7ae (imx: nitrogen6x/mx6qsabrelite: Fix bug in board_video_skip).
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
board/freescale/mx6sabresd/mx6sabresd.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c index 0f91fe2..61fe67c 100644 --- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -322,6 +322,7 @@ int board_video_skip(void) if (!panel) { panel = displays[0].mode.name; printf("No panel detected: default to %s\n", panel);
i = 0;
Robert sents the same fix for nitrogen6x, and it was integrated. The two functions board_video_skip() are exactly the same. What about to factorize the code putting it maybe in imx-common ?
Best regards, Stefano
Hi Stefano,
On 09/12/2013 03:17 AM, Stefano Babic wrote:
Hi Fabio,
On 11/09/2013 23:14, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
If a HDMI cable is not connected, the following message is seen on boot:
CPU: Freescale i.MX6Q rev1.1 at 792 MHz Reset cause: POR Board: MX6-SabreSD DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2 No panel detected: default to HDMI unsupported panel HDMI
Reset the 'i' variable to fix the 'unsupported panel' message.
This follows the same idea of commit 47ac53d7ae (imx: nitrogen6x/mx6qsabrelite: Fix bug in board_video_skip).
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
board/freescale/mx6sabresd/mx6sabresd.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c index 0f91fe2..61fe67c 100644 --- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -322,6 +322,7 @@ int board_video_skip(void) if (!panel) { panel = displays[0].mode.name; printf("No panel detected: default to %s\n", panel);
i = 0;Robert sents the same fix for nitrogen6x, and it was integrated. The two functions board_video_skip() are exactly the same. What about to factorizee the code putting it maybe in imx-common ?
We can do this, but as I mentioned in my earlier e-mail, I hope this code doesn't have a long shelf life.
It seems reasonable to have a small number of displays supported by various boards (those offered as part of standard EVKs), but we integrate dozens of displays every year and it's not reasonable to change the code base for all of them.
To distill some earlier conversations: - Display timing information should be data, and preferably stored in the environment so it can be used early in the boot process.
- Display selection in U-Boot should be translatable into kernel parameters (bootargs or device-tree)
The main problem lies in the second, since at the moment, both the 3.0.35 kernel tree and the 3.5.7 kernel tree use either mode strings or named display types as display configuration. The mode strings (VESA GTF timings) are useful, but not all panels work optimally with those timings, causing the kernel to suffer the same requirement to update a data table for the new display.
For any that didn't participate in the earlier discussion, some notes can be found in this thread: http://lists.denx.de/pipermail/u-boot/2013-July/thread.html#159514
After my first shudder at using DT for this, I am currently of the belief that some form of detailed timing as is done in the kernel FB bindings is appropriate:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documen...
The primary concern I have is whether adding the ability to parse this nested structure is appropriate, and again, the inability to pass the information to current i.MX6 kernels.
I think the right thing may be to simply require **all** fields from the DT structure and allow prompting of each.
And of course, add kernel patches to allow them to accept the detailed information.
Please let me know your thoughts on this.
Regards,
Eric
On 11/09/2013 23:14, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
If a HDMI cable is not connected, the following message is seen on boot:
CPU: Freescale i.MX6Q rev1.1 at 792 MHz Reset cause: POR Board: MX6-SabreSD DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2 No panel detected: default to HDMI unsupported panel HDMI
Reset the 'i' variable to fix the 'unsupported panel' message.
This follows the same idea of commit 47ac53d7ae (imx: nitrogen6x/mx6qsabrelite: Fix bug in board_video_skip).
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Applied to u-boot-imx, thanks!
Best regards, Stefano Babic
Hi Fabio,
CC to Eric and Troy.
On 11/09/2013 23:14, Fabio Estevam wrote:
diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c index c832bd9..0f91fe2 100644 --- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -313,7 +313,7 @@ int board_video_skip(void) if (!panel) { for (i = 0; i < ARRAY_SIZE(displays); i++) { struct display_info_t const *dev = displays+i;
if (dev->detect(dev)) {
if (dev->detect && dev->detect(dev)) {
The same should happen on the Nitrogen board. Should this fix extended to the other boards using hdmi ?
Best regards, Stefano
Thanks Stefano,
On 09/12/2013 02:47 AM, Stefano Babic wrote:
Hi Fabio,
CC to Eric and Troy.
On 11/09/2013 23:14, Fabio Estevam wrote:
diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c index c832bd9..0f91fe2 100644 --- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -313,7 +313,7 @@ int board_video_skip(void) if (!panel) { for (i = 0; i < ARRAY_SIZE(displays); i++) { struct display_info_t const *dev = displays+i;
if (dev->detect(dev)) {
if (dev->detect && dev->detect(dev)) {The same should happen on the Nitrogen board. Should this fix extended to the other boards using hdmi ?
This isn't needed yet in the stock (main-line) code base, since we haven't added any panels without detection.
We do have a form this patch and a lot of panels in our local tree on Github, but wanted to avoid unnecessary noise on the list because we've integrated a dozen or so panels and the existing structure really doesn't scale.
We had a separate discussion regarding treating the displays as data (environment), but have stalled somewhat on that front.
The current device-tree code for i.MX6 uses mode strings instead of the detailed timing data that's really needed for a proper solution.
Regards,
Eric
Hi Eric,
On 12/09/2013 17:02, Eric Nelson wrote:
This isn't needed yet in the stock (main-line) code base, since we haven't added any panels without detection.
ok, thanks - I will then apply Fabio's patches for the Freescale's boards.
We do have a form this patch and a lot of panels in our local tree on Github, but wanted to avoid unnecessary noise on the list because we've integrated a dozen or so panels and the existing structure really doesn't scale.
We had a separate discussion regarding treating the displays as data (environment), but have stalled somewhat on that front.
I think also that using the environment seems a clumsy solution. We used a lot of times in the past, and we have not unified their usage.
The current device-tree code for i.MX6 uses mode strings instead of the detailed timing data that's really needed for a proper solution.
I admit that using DT also for u-boot seems a better solution. You're right about i.MX6 in kernel, but on the other hand I like how it is described for i.MX28 boards. The display-timings node contain all information we need. It would be nice to have the same for i.MX6.
Best regards, Stefano
Hi Stefano,
On Fri, Sep 13, 2013 at 5:16 AM, Stefano Babic sbabic@denx.de wrote:
Hi Eric,
On 12/09/2013 17:02, Eric Nelson wrote:
This isn't needed yet in the stock (main-line) code base, since we haven't added any panels without detection.
ok, thanks - I will then apply Fabio's patches for the Freescale's boards.
Cool, thanks.
I admit that using DT also for u-boot seems a better solution. You're right about i.MX6 in kernel, but on the other hand I like how it is described for i.MX28 boards. The display-timings node contain all information we need. It would be nice to have the same for i.MX6.
We currently have the same in DT for mx6 as well. Check this commit, for example: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/arch...
Regards,
Fabio Estevam
Hi Fabio,
On 13/09/2013 15:58, Fabio Estevam wrote:
We currently have the same in DT for mx6 as well. Check this commit, for example: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/arch...
Cool, thanks for the hint !
Regards, Stefano Babic
Hi Fabio,
On 09/13/2013 06:58 AM, Fabio Estevam wrote:
Hi Stefano,
On Fri, Sep 13, 2013 at 5:16 AM, Stefano Babic sbabic@denx.de wrote:
I admit that using DT also for u-boot seems a better solution. You're right about i.MX6 in kernel, but on the other hand I like how it is described for i.MX28 boards. The display-timings node contain all information we need. It would be nice to have the same for i.MX6.
We currently have the same in DT for mx6 as well. Check this commit, for example: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/arch...
Nice!
So, how should we get this done?
We have support for parsing individual lines of DT, and the primary thing(s) needed by U-Boot's display are in the timing block of the device tree, but I wonder whether it makes sense to implement a full parser for that, or a simpler parser for fb_videomode.
Since both cfb_console in U-Boot and the of_get_fb_videomode() routine in the kernel use fb_videomode, it should be straightforward to hand the information off.
I don't think the outer information (the lvds-channel@0 and ldb blocks in imx6q-sabrelite.dts) should be parsed by U-Boot as device-tree code since that path leads to having a full device-tree compiler that seems inappropriate.
Let me know your thoughts.
Eric
Hi Eric,
On Fri, Sep 13, 2013 at 11:51 AM, Eric Nelson eric.nelson@boundarydevices.com wrote:
Nice!
So, how should we get this done?
We have support for parsing individual lines of DT, and the primary thing(s) needed by U-Boot's display are in the timing block of the device tree, but I wonder whether it makes sense to implement a full parser for that, or a simpler parser for fb_videomode.
Looks like the parser for fb_videomode is enough.
I am not familiar with the usage of device tree in U-boot yet.
How do other SoC families handle the video display-timings in dt currently in U-boot?
Regards,
Fabio Estevam
Hi Fabio,
On 09/13/2013 11:11 AM, Fabio Estevam wrote:
Hi Eric,
On Fri, Sep 13, 2013 at 11:51 AM, Eric Nelson eric.nelson@boundarydevices.com wrote:
Nice!
So, how should we get this done?
We have support for parsing individual lines of DT, and the primary thing(s) needed by U-Boot's display are in the timing block of the device tree, but I wonder whether it makes sense to implement a full parser for that, or a simpler parser for fb_videomode.
Looks like the parser for fb_videomode is enough.
I am not familiar with the usage of device tree in U-boot yet.
How do other SoC families handle the video display-timings in dt currently in U-boot?
AFAIK, with a full device-tree blob, but without any means of updating it, which ends up in the same place as the compiled-in settings.
In order to configure for a new kernel, you have to "compile" the DTB and save it to the boot media.
This is a perfectly reasonable thing to do late in the development process, but makes connecting new panels cumbersome.
Regards,
Eric
On 11/09/2013 23:14, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Since commit d9b894603 (mx6sabresd: Add LVDS splash screen support) the following hang happens if the HDMI cable is not connected or the 'panel' variable is not set:
U-Boot 2013.10-rc2-12978-g47ac53d-dirty (Sep 11 2013 - 15:07:38)
CPU: Freescale i.MX6Q rev1.2 at 792 MHz Reset cause: POR Board: MX6-SabreSD DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2 ...
Provide a check to 'dev->detect' in order to prevent the hang.
Reported-by: Pardeep Kumar Singla b45784@freescale.com Suggested-by: Eric Bénard eric@eukrea.com Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Applied to u-boot-imx, thanks!
Best regards, Stefano Babic
participants (3)
-
Eric Nelson -
Fabio Estevam -
Stefano Babic