Skip to content

Commit

Permalink
Revert "fbdev: Garbage collect fbdev scrolling acceleration, part 1 (…
Browse files Browse the repository at this point in the history
…from TODO list)"

commit 1148836 upstream.

This reverts commit b3ec8cd.

Revert the second (of 2) commits which disabled scrolling acceleration
in fbcon/fbdev.  It introduced a regression for fbdev-supported graphic
cards because of the performance penalty by doing screen scrolling by
software instead of using the existing graphic card 2D hardware
acceleration.

Console scrolling acceleration was disabled by dropping code which
checked at runtime the driver hardware capabilities 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 973c096).

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 the original commit is to be reverted. By reverting we
reintroduce hardware-based scrolling acceleration and fix the
performance regression for fbdev drivers.

There isn't any impact on DRM when reverting those patches.

Signed-off-by: Helge Deller <deller@gmx.de>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Sven Schnelle <svens@stackframe.org>
Cc: stable@vger.kernel.org # v5.16+
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20220202135531.92183-2-deller@gmx.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
hdeller authored and gregkh committed Feb 8, 2022
1 parent 1899c3c commit 0e90cb3
Show file tree
Hide file tree
Showing 11 changed files with 678 additions and 51 deletions.
13 changes: 5 additions & 8 deletions Documentation/gpu/todo.rst
Original file line number Diff line number Diff line change
Expand Up @@ -314,19 +314,16 @@ Level: Advanced
Garbage collect fbdev scrolling acceleration
--------------------------------------------

Scroll acceleration has been disabled in fbcon. Now it works as the old
SCROLL_REDRAW mode. A ton of code was removed in fbcon.c and the hook bmove was
removed from fbcon_ops.
Remaining tasks:
Scroll acceleration is disabled in fbcon by hard-wiring p->scrollmode =
SCROLL_REDRAW. There's a ton of code this will allow us to remove:

- a bunch of the hooks in fbcon_ops could be removed or simplified by calling
- lots of code in fbcon.c

- a bunch of the hooks in fbcon_ops, maybe the remaining hooks could be called
directly instead of the function table (with a switch on p->rotate)

- fb_copyarea is unused after this, and can be deleted from all drivers

- after that, fb_copyarea can be deleted from fb_ops in include/linux/fb.h as
well as cfb_copyarea

Note that not all acceleration code can be deleted, since clearing and cursor
support is still accelerated, which might be good candidates for further
deletion projects.
Expand Down
16 changes: 16 additions & 0 deletions drivers/video/fbdev/core/bitblit.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@ static void update_attr(u8 *dst, u8 *src, int attribute,
}
}

static void bit_bmove(struct vc_data *vc, struct fb_info *info, int sy,
int sx, int dy, int dx, int height, int width)
{
struct fb_copyarea area;

area.sx = sx * vc->vc_font.width;
area.sy = sy * vc->vc_font.height;
area.dx = dx * vc->vc_font.width;
area.dy = dy * vc->vc_font.height;
area.height = height * vc->vc_font.height;
area.width = width * vc->vc_font.width;

info->fbops->fb_copyarea(info, &area);
}

static void bit_clear(struct vc_data *vc, struct fb_info *info, int sy,
int sx, int height, int width)
{
Expand Down Expand Up @@ -378,6 +393,7 @@ static int bit_update_start(struct fb_info *info)

void fbcon_set_bitops(struct fbcon_ops *ops)
{
ops->bmove = bit_bmove;
ops->clear = bit_clear;
ops->putcs = bit_putcs;
ops->clear_margins = bit_clear_margins;
Expand Down

0 comments on commit 0e90cb3

Please sign in to comment.