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

[Geert Uytterhoeven]

Hi Marek,

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.

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