[v3,4/5] fbdev: Improve performance of cfb_imageblit()

[Geert Uytterhoeven]

Hi Thomas,

On Thu, Mar 10, 2022 at 8:22 PM Thomas Zimmermann <tzimmermann at suse.de> wrote:
> Am 09.03.22 um 11:39 schrieb Geert Uytterhoeven:
> > On Wed, Mar 9, 2022 at 10:22 AM Marek Szyprowski
> > <m.szyprowski at samsung.com> wrote:
> >> On 09.03.2022 09:22, Thomas Zimmermann wrote:
> >>> Am 08.03.22 um 23:52 schrieb Marek Szyprowski:
> >>>> On 23.02.2022 20:38, Thomas Zimmermann wrote:
> >>>>> Improve the performance of cfb_imageblit() by manually unrolling
> >>>>> the inner blitting loop and moving some invariants out. The compiler
> >>>>> failed to do this automatically. This change keeps cfb_imageblit()
> >>>>> in sync with sys_imagebit().
> >>>>>
> >>>>> A microbenchmark measures the average number of CPU cycles
> >>>>> for cfb_imageblit() after a stabilizing period of a few minutes
> >>>>> (i7-4790, FullHD, simpledrm, kernel with debugging).
> >>>>>
> >>>>> cfb_imageblit(), new: 15724 cycles
> >>>>> cfb_imageblit(): old: 30566 cycles
> >>>>>
> >>>>> In the optimized case, cfb_imageblit() is now ~2x faster than before.
> >>>>>
> >>>>> v3:
> >>>>>      * fix commit description (Pekka)
> >>>>>
> >>>>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> >>>>> Acked-by: Sam Ravnborg <sam at ravnborg.org>
> >>>>> Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>
> >>>> This patch landed recently in linux next-20220308 as commit 0d03011894d2
> >>>> ("fbdev: Improve performance of cfb_imageblit()"). Sadly it causes a
> >>>> freeze after DRM and emulated fbdev initialization on various Samsung
> >>>> Exynos ARM 32bit based boards. This happens when kernel is compiled from
> >>>> exynos_defconfig. Surprisingly when kernel is compiled from
> >>>> multi_v7_defconfig all those boards boot fine, so this is a matter of
> >>>> one of the debugging options enabled in the exynos_defconfig. I will try
> >>>> to analyze this further and share the results. Reverting $subject on top
> >>>> of next-20220308 fixes the boot issue.
> >>>
> >>> Thanks for reporting. I don't have the hardware to reproduce it and
> >>> there's no obvious difference to the original version. It's supposed
> >>> to be the same algorithm with a different implementation. Unless you
> >>> can figure out the issue, we can also revert the patch easily.
> >>
> >> I've played a bit with .config options and found that the issue is
> >> caused by the compiled-in fonts used for the framebuffer. For some
> >> reasons (so far unknown to me), exynos_defconfig has the following odd
> >> setup:
> >>
> >> CONFIG_FONT_SUPPORT=y
> >> CONFIG_FONTS=y
> >> # CONFIG_FONT_8x8 is not set
> >> # CONFIG_FONT_8x16 is not set
> >> # CONFIG_FONT_6x11 is not set
> >> CONFIG_FONT_7x14=y
> >> # CONFIG_FONT_PEARL_8x8 is not set
> >> # CONFIG_FONT_ACORN_8x8 is not set
> >> # CONFIG_FONT_MINI_4x6 is not set
> >> # CONFIG_FONT_6x10 is not set
> >> # CONFIG_FONT_10x18 is not set
> >> # CONFIG_FONT_SUN8x16 is not set
> >> # CONFIG_FONT_SUN12x22 is not set
> >> # CONFIG_FONT_TER16x32 is not set
> >> # CONFIG_FONT_6x8 is not set
> >>
> >> Such setup causes a freeze during framebuffer initialization (or just
> >> after it got registered). I've reproduced this even on Raspberry Pi 3B
> >> with multi_v7_defconfig and changed fonts configuration (this also
> >> required to disable vivid driver, which forces 8x16 font), where I got
> >> the following panic:
> >>
> >> simple-framebuffer 3eace000.framebuffer: framebuffer at 0x3eace000,
> >> 0x12c000 bytes
> >> simple-framebuffer 3eace000.framebuffer: format=a8r8g8b8,
> >> mode=640x480x32, linelength=2560
> >> 8<--- cut here ---
> >> Unable to handle kernel paging request at virtual address f0aac000
> >
> > So support for images with offsets or widths that are not a multiple
> > of 8 got broken in cfb_imageblit(). Oops...
> >
> > BTW, the various drawing routines used to set a bitmask indicating
> > which alignments were supported (see blit_x), but most of them no
> > longer do, presumably because all alignments are now supported
> > (since ca. 20 years?).
> > So you can (temporarily) work around this by filling in blit_x,
> > preventing the use of the 7x14 font.
>
> How do I activate the 7x14 font? It's compiled into the kernel already
> (CONFIG_FONT_7x14=y).

Documentation/fb/fbcon.rst:1. fbcon=font:<name>

Or just disable all other fonts.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds