[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[Fabio Estevam]

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;
-- 
1.8.1.2

[U-Boot] [PATCH v2 2/2] mx6sabresd: Reset counter to prevent error message

[Fabio Estevam]

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++) {
-- 
1.8.1.2

[U-Boot] [PATCH v2 2/2] mx6sabresd: Reset counter to prevent error message

[Stefano Babic]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH v2 2/2] mx6sabresd: Reset counter to prevent error message

[Eric Nelson]

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/Documentation/devicetree/bindings/fb/mxsfb.txt?id=refs/tags/v3.11

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
	

[U-Boot] [PATCH v2 2/2] mx6sabresd: Reset counter to prevent error message

[Stefano Babic]

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



-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[Eric Nelson]

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

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[Stefano Babic]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[Fabio Estevam]

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/arm/boot/dts/imx6q-sabrelite.dts?id=722cfacd2e719b6447fb4bd5cd3372725876336d

Regards,

Fabio Estevam

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[Stefano Babic]

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/arm/boot/dts/imx6q-sabrelite.dts?id=722cfacd2e719b6447fb4bd5cd3372725876336d

Cool, thanks for the hint !

Regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[Eric Nelson]

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/arm/boot/dts/imx6q-sabrelite.dts?id=722cfacd2e719b6447fb4bd5cd3372725876336d
>

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 at 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

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[Fabio Estevam]

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

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[Eric Nelson]

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

[U-Boot] [PATCH v2 1/2] mx6sabresd: Avoid hang when HDMI cable is not connected

[Stefano Babic]

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



-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================