[PATCH v3 0/5] video: Improve syncing performance with cyclic

[Simon Glass]

Hi Caleb,

On Thu, 4 Jul 2024 at 16:17, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>
> Hi Simon,
>
> On 04/07/2024 09:48, Simon Glass wrote:
> > Now that U-Boot has a background-processing feature, it is possible to
> > reduce the amount of 'foreground' syncing of the display. At present
> > this happens quite often.
> >
> > Foreground syncing blocks all other processing, sometimes for 10ms or
> > more. When pasting commands into U-Boot over the UART, this typically
> > result in characters being dropped. For example, on rpi_4 it isn't
> > possible to paste in more than 35 characters before things fail. This
> > makes updating the environment or entering long commands very painful
> > over the console, since text must be pasted in chunks, or the
> > vidconsole device must be dropped from stdout.
>
> I'm not intimately familiar with U-Boots event loop, but to me this
> sounds more like a bug in the console code. I can confirm this issue
> appears on Qualcomm platforms with the GENI serial driver, I tried
> enabling CONFIG_SERIAL_RX_BUFFER and it seemed to help significantly
> (though there were still a few dropped characters when pasting).

Thanks for digging into this. Yes I agree.

The fix I have been contemplating is to adjust video_sync() to only
flush the cache if 'force' is given, then have vidconsole_putc() pass
force=true if a newline is written.

>
> Of note, the USB ACM serial driver doesn't seem to have any issues here,
> presumably since it isn't restricted by the baud rate.

Or it might just have a large serial buffer?

>
> If there is an abort, does this approach still ensure that it gets
> printed properly on the video console?

What sort of abort?

>
> Kind regards,
> >
> > This series introduces background syncing, enabled by default for
> > boards which use video. The sync rates for foreground and background
> > are configurable.
> >
> > With this series it is possible to paste in any amount of text to the
> > command line. Some sandbox-specific workarounds can now be removed and
> > sandbox video (./u-boot -Dl) is significantly more responsive.
> >
> > This obviously increases code size, since it enables a subsystem not
> > normally used by default. However it only applies to boards which have
> > VIDEO enabled, which are presumably less worried about memory space
> > since the video code is fairly large.
> >
> > Also it is possible to disable CMD_CYCLIC and reduce the growth to:
> >
> >     aarch64: (for 1/1 boards) all +1081.0 rodata +65.0 text +1016.0
> >         arm: (for 1/1 boards) all +945.0 rodata +65.0 text +880.0
> >
> > Without that, the increase doubles.
> >
> > It is of course possible to disable CYCLIC and still use VIDEO but this
> > reverts to the current behaviour
> >
> > Changes in v3:
> > - Drop inclusion of kconfig.h in cyclic.h
> > - Adapt to new cyclic API
> >
> > Changes in v2:
> > - Add an SPL_CYCLIC symbol
> > - Add a lot more explanation about the header files
> > - Expand help for CONFIG_VIDEO
> > - Fix 'groth' and 'work-around' typos in cover letter
> >
> > Simon Glass (5):
> >    cyclic: Add a symbol for SPL
> >    video: Move last_sync to private data
> >    video: Use cyclic to handle video sync
> >    sandbox: Increase cyclic CPU-time limit
> >    sandbox: Drop video-sync in serial driver
> >
> >   common/Kconfig                    |  9 ++++++
> >   common/Makefile                   |  2 +-
> >   drivers/serial/sandbox.c          |  2 --
> >   drivers/video/Kconfig             | 35 +++++++++++++++++++++
> >   drivers/video/video-uclass.c      | 52 +++++++++++++++++++++++++------
> >   drivers/watchdog/Kconfig          |  1 +
> >   include/asm-generic/global_data.h |  2 +-
> >   include/cyclic.h                  |  5 +--
> >   include/video.h                   |  2 ++
> >   9 files changed, 95 insertions(+), 15 deletions(-)
> >
>
> --
> // Caleb (they/them)

Regards,
Simon