| Message ID | 1378934070-11721-1-git-send-email-festevam@gmail.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Stefano Babic |
| Headers | show |
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/arm/boot/dts/imx6q-sabrelite.dts?id=722cfacd2e719b6447fb4bd5cd3372725876336d 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/arm/boot/dts/imx6q-sabrelite.dts?id=722cfacd2e719b6447fb4bd5cd3372725876336d 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/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@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
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;