mbox series

[0/2] Fix regression introduced by disabling accelerated scrolling in fbcon

Message ID 20220119110839.33187-1-deller@gmx.de
Headers show
Series Fix regression introduced by disabling accelerated scrolling in fbcon | expand

Message

Helge Deller Jan. 19, 2022, 11:08 a.m. UTC 
This series reverts two patches which disabled scrolling acceleration in
fbcon/fbdev. Those patches introduced a regression for fbdev-supported graphic
cards because of the performance penalty by doing screen scrolling by software
instead of using hardware acceleration.

Console scrolling acceleration was disabled by dropping code which checked at
runtime the driver hardware possibilities for the BINFO_HWACCEL_COPYAREA or
FBINFO_HWACCEL_FILLRECT flags and if set, it enabled scrollmode SCROLL_MOVE
which uses hardware acceleration to move screen contents.  After dropping those
checks scrollmode was hard-wired to SCROLL_REDRAW instead, which forces all
graphic cards to redraw every character at the new screen position when
scrolling.

This change effectively disabled all hardware-based scrolling acceleration for
ALL drivers, because now all kind of 2D hardware acceleration (bitblt,
fillrect) in the drivers isn't used any longer.

The original commit message mentions that only 3 DRM drivers (nouveau, omapdrm
and gma500) used hardware acceleration in the past and thus code for checking
and using scrolling acceleration is obsolete.

This statement is NOT TRUE, because beside the DRM drivers there are around 35
other fbdev drivers which depend on fbdev/fbcon and still provide hardware
acceleration for fbdev/fbcon.

The original commit message also states that syzbot found lots of bugs in fbcon
and thus it's "often the solution to just delete code and remove features".
This is true, and the bugs - which actually affected all users of fbcon,
including DRM - were fixed, or code was dropped like e.g. the support for
software scrollback in vgacon (commit 973c096f6a85).

So to further analyze which bugs were found by syzbot, I've looked through all
patches in drivers/video which were tagged with syzbot or syzkaller back to
year 2005. The vast majority fixed the reported issues on a higher level, e.g.
when screen is to be resized, or when font size is to be changed. The few ones
which touched driver code fixed a real driver bug, e.g. by adding a check.

But NONE of those patches touched code of either the SCROLL_MOVE or the
SCROLL_REDRAW case.

That means, there was no real reason why SCROLL_MOVE had to be ripped-out and
just SCROLL_REDRAW had to be used instead. The only reason I can imagine so far
was that SCROLL_MOVE wasn't used by DRM and as such it was assumed that it
could go away. That argument completely missed the fact that SCROLL_MOVE is
still heavily used by fbdev (non-DRM) drivers.

Some people mention that using memcpy() instead of the hardware acceleration is
pretty much the same speed. But that's not true, at least not for older graphic
cards and machines where we see speed decreases by factor 10 and more and thus
this change leads to console responsiveness way worse than before.

That's why I propose to revert those patches, re-introduce hardware-based
scrolling acceleration and fix the performance-regression for fbdev drivers.
There isn't any impact on DRM when reverting those patches.

Helge Deller (2):
  Revert "fbdev: Garbage collect fbdev scrolling acceleration, part 1
    (from TODO list)"
  Revert "fbcon: Disable accelerated scrolling"

 Documentation/gpu/todo.rst              |  24 --
 drivers/video/fbdev/core/bitblit.c      |  16 +
 drivers/video/fbdev/core/fbcon.c        | 540 +++++++++++++++++++++++-
 drivers/video/fbdev/core/fbcon.h        |  59 +++
 drivers/video/fbdev/core/fbcon_ccw.c    |  28 +-
 drivers/video/fbdev/core/fbcon_cw.c     |  28 +-
 drivers/video/fbdev/core/fbcon_rotate.h |   9 +
 drivers/video/fbdev/core/fbcon_ud.c     |  37 +-
 drivers/video/fbdev/core/tileblit.c     |  16 +
 drivers/video/fbdev/skeletonfb.c        |  12 +-
 include/linux/fb.h                      |   2 +-
 11 files changed, 703 insertions(+), 68 deletions(-)