Re: [PATCH 0/9] Enable splash screen

[Simon Glass <sjg@chromium.org>]

Hi Nikhil,

On Fri, 17 Mar 2023 at 02:13, Nikhil M Jain <n-jain1@ti.com> wrote:
>
> Hi Simon,
>
> On 16/03/23 10:10, Nikhil M Jain wrote:
> > Hi Simon,
> >
> > On 15/03/23 19:38, Simon Glass wrote:
> >> Hi Nikhil,
> >>
> >> On Wed, 15 Mar 2023 at 00:07, Nikhil M Jain <n-jain1@ti.com> wrote:
> >>>
> >>> Hi Simon,
> >>>
> >>> On 15/03/23 03:38, Simon Glass wrote:
> >>>> Hi Nikhil,
> >>>>
> >>>> On Mon, 13 Mar 2023 at 04:15, Nikhil M Jain <n-jain1@ti.com> wrote:
> >>>>>
> >>>>> To enable splash screen at SPL stage move video driver and splash
> >>>>> screen
> >>>>> framework at SPL, which will bring up image on display very quickly
> >>>>> and
> >>>>> thus have early display support in SPL.
> >>>>>
> >>>>> Nikhil M Jain (9):
> >>>>>     drivers: video: Kconfig: Necessary configs for video at SPL
> >>>>>     drivers: video: tidss: Kconfig: Configs to enable TIDSS at SPL
> >>>>>     cmd: Kconfig: Add necessary configs for splash screen at SPL
> >>>>>     drivers: video: Makefile: Compile video driver files at SPL
> >>>>>     drivers: video: tidss: Makefile: Add condition to compile TIDSS
> >>>>> at SPL
> >>>>>     cmd: Makefile: Add rules to build bmp.c and read.c at SPL
> >>>>>     common: splash: Enable splash_display at SPL stage
> >>>>>     drivers: video: video-uclass: Disable u-boot logo at SPL
> >>>>>     board: ti: am62x: evm: OSPI support for splash screen
> >>>>>
> >>>>>    board/ti/am62x/evm.c         |  6 ++++++
> >>>>>    cmd/Kconfig                  | 17 +++++++++++++++++
> >>>>>    cmd/Makefile                 |  2 ++
> >>>>>    common/splash.c              |  2 +-
> >>>>>    drivers/video/Kconfig        | 32 ++++++++++++++++++++++++++++----
> >>>>>    drivers/video/Makefile       |  6 ++++++
> >>>>>    drivers/video/tidss/Kconfig  |  6 ++++++
> >>>>>    drivers/video/tidss/Makefile |  1 +
> >>>>>    drivers/video/video-uclass.c |  2 +-
> >>>>>    include/splash.h             |  2 +-
> >>>>>    10 files changed, 69 insertions(+), 7 deletions(-)
> >>>>
> >>>> I'm not necessarily arguing against this, but what is the need for
> >>>> this? How many milliseconds earlier does the image appear with this
> >>>> patch? What is the bottleneck? We should be able to get to U-Boot
> >>>> proper very quickly.
> >>>>
> >>> There is a significant difference in time, by adding support in SPL
> >>> splash screen comes up by approx 650ms and at u-boot proper it comes at
> >>> 2.6s, measured from first print in console as seen on AM62x. Also we
> >>> plan to skip u-boot proper and load kernel directly.
> >>
> >> Yes that really is terrible. It should be under a second for U-Boot
> >> proper!
> >>
> >> Have you tried using bootstage to report the numbers?
> >>
> > No I haven't used the bootstage, I will use it to get the numbers.
> >
> Boot stage logs
> link: https://gist.github.com/NikMJain/beb60fe42b1e89829cdd5b8713284330
>
>
> >> Have you tried using tracing to figure out what is wrong? Is it just
> >> slow storage?
> >>
> > U-boot proper comes up in one sec but the splash display is called
> > through stdio_add_devices which is late in the board_init_r sequence
> > defined in board_r.c.
> >  >> Regards,
> >> Simon
> >
> > Thanks
>
> On AM62x we have multi-stage boot first R5 SPL comes up then A53 SPL and
> u-boot proper, due to which splash screen at u-boot proper takes over 2
> sec. On AM62x we have an aggressive requirement for an early splash
> screen. Hence we are moving splash screen support to SPL., which will
> bring splash screen time to approx 600 ms.
>
> We also want to support falcon boot mode from A53 SPL, we have customers
> who don't want u-boot proper in production boot flow and still require
> splash screen, the only way to support it is to add  splash screen at SPL.
>
> u-boot logs with splash screen timings, measurement through
> timer_get_boot_us at u-boot proper and SPL.
> link: https://gist.github.com/NikMJain/3be0b6c92092678b6aec8e5fdbc46a98

I think it would be worth looking at what is taking so long. Could you
add bootstage in there (which supports SPL as well) and produce a
final report? Is BL31 taking a while?You can use
bootstage_start()/bootstage_accum() to collect time on MMC loading,
etc. even if you don't want to enable CONFIG_TRACE.

This is an important area that will affect a lot of boards. If it
takes 2 seconds to get into U-Boot, everyone is going to want to avoid
it.

Regards,
Simon