LinuxLists.cc - [PATCH 0/2] Fix regression introduced by disabling accelerated scrolling in fbcon

[permalink] [raw]

Subject: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.

Revert this patch. This patch started to introduce the regression that
all hardware acceleration of more than 35 existing fbdev drivers were
bypassed and thus fbcon console output for those was dramatically slowed
down by factor of 10 and more.

Reverting this commit has no impact on DRM, since none of the DRM drivers are
tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
FBINFO_HWACCEL_FILLRECT or others.

Signed-off-by: Helge Deller <[email protected]>
Cc: [email protected] # v5.16
---
Documentation/gpu/todo.rst | 21 ---------------
drivers/video/fbdev/core/fbcon.c | 45 ++++++++++++++++++++++++++------
2 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 29506815d24a..a1212b5b3026 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -300,27 +300,6 @@ Contact: Daniel Vetter, Noralf Tronnes

Level: Advanced

-Garbage collect fbdev scrolling acceleration
---------------------------------------------
-
-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:
-
-- 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
-
-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.
-
-Contact: Daniel Vetter
-
-Level: Intermediate
-
idr_init_base()
---------------

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 22bb3892f6bd..b813985f1403 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1025,7 +1025,7 @@ static void fbcon_init(struct vc_data *vc, int init)
struct vc_data *svc = *default_mode;
struct fbcon_display *t, *p = &fb_display[vc->vc_num];
int logo = 1, new_rows, new_cols, rows, cols;
- int ret;
+ int cap, ret;

if (WARN_ON(info_idx == -1))
return;
@@ -1034,6 +1034,7 @@ static void fbcon_init(struct vc_data *vc, int init)
con2fb_map[vc->vc_num] = info_idx;

info = registered_fb[con2fb_map[vc->vc_num]];
+ cap = info->flags;

if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
logo_shown = FBCON_LOGO_DONTSHOW;
@@ -1135,13 +1136,11 @@ static void fbcon_init(struct vc_data *vc, int init)

ops->graphics = 0;

- /*
- * No more hw acceleration for fbcon.
- *
- * FIXME: Garbage collect all the now dead code after sufficient time
- * has passed.
- */
- p->scrollmode = SCROLL_REDRAW;
+ if ((cap & FBINFO_HWACCEL_COPYAREA) &&
+ !(cap & FBINFO_HWACCEL_DISABLED))
+ p->scrollmode = SCROLL_MOVE;
+ else /* default to something safe */
+ p->scrollmode = SCROLL_REDRAW;

/*
* ++guenther: console.c:vc_allocate() relies on initializing
@@ -1953,15 +1952,45 @@ static void updatescrollmode(struct fbcon_display *p,
{
struct fbcon_ops *ops = info->fbcon_par;
int fh = vc->vc_font.height;
+ int cap = info->flags;
+ u16 t = 0;
+ int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep,
+ info->fix.xpanstep);
+ int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t);
int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual,
info->var.xres_virtual);
+ int good_pan = (cap & FBINFO_HWACCEL_YPAN) &&
+ divides(ypan, vc->vc_font.height) && vyres > yres;
+ int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) &&
+ divides(ywrap, vc->vc_font.height) &&
+ divides(vc->vc_font.height, vyres) &&
+ divides(vc->vc_font.height, yres);
+ int reading_fast = cap & FBINFO_READS_FAST;
+ int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) &&
+ !(cap & FBINFO_HWACCEL_DISABLED);
+ int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) &&
+ !(cap & FBINFO_HWACCEL_DISABLED);

p->vrows = vyres/fh;
if (yres > (fh * (vc->vc_rows + 1)))
p->vrows -= (yres - (fh * vc->vc_rows)) / fh;
if ((yres % fh) && (vyres % fh < yres % fh))
p->vrows--;
+
+ if (good_wrap || good_pan) {
+ if (reading_fast || fast_copyarea)
+ p->scrollmode = good_wrap ?
+ SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE;
+ else
+ p->scrollmode = good_wrap ? SCROLL_REDRAW :
+ SCROLL_PAN_REDRAW;
+ } else {
+ if (reading_fast || (fast_copyarea && !fast_imageblit))
+ p->scrollmode = SCROLL_MOVE;
+ else
+ p->scrollmode = SCROLL_REDRAW;
+ }
}

#define PITCH(w) (((w) + 7) >> 3)
--
2.31.1

2022-01-21 19:10:57

[permalink] [raw]

Subject: [PATCH 1/2] Revert "fbdev: Garbage collect fbdev scrolling acceleration, part 1 (from TODO list)"

This reverts commit b3ec8cdf457e5e63d396fe1346cc788cf7c1b578.

Revert this patch. This and the previous patch introduced the
regression that all hardware acceleration of more than 35 existing fbdev
drivers were bypassed and thus fbcon console output for those was
dramatically slowed down by factor of 10 and more.

Reverting this commit has no impact on DRM, since none of the DRM drivers are
tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
FBINFO_HWACCEL_FILLRECT or others.

Signed-off-by: Helge Deller <[email protected]>
Cc: [email protected] # v5.16
---
Documentation/gpu/todo.rst | 13 +-
drivers/video/fbdev/core/bitblit.c | 16 +
drivers/video/fbdev/core/fbcon.c | 509 +++++++++++++++++++++++-
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, 678 insertions(+), 51 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index da138dd39883..29506815d24a 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -303,19 +303,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.
diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
index 01fae2c96965..f98e8f298bc1 100644
--- a/drivers/video/fbdev/core/bitblit.c
+++ b/drivers/video/fbdev/core/bitblit.c
@@ -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)
{
@@ -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;
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 99ecd9a6d844..22bb3892f6bd 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -173,6 +173,8 @@ static void fbcon_putcs(struct vc_data *vc, const unsigned short *s,
int count, int ypos, int xpos);
static void fbcon_clear_margins(struct vc_data *vc, int bottom_only);
static void fbcon_cursor(struct vc_data *vc, int mode);
+static void fbcon_bmove(struct vc_data *vc, int sy, int sx, int dy, int dx,
+ int height, int width);
static int fbcon_switch(struct vc_data *vc);
static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch);
static void fbcon_set_palette(struct vc_data *vc, const unsigned char *table);
@@ -180,8 +182,16 @@ static void fbcon_set_palette(struct vc_data *vc, const unsigned char *table);
/*
* Internal routines
*/
+static __inline__ void ywrap_up(struct vc_data *vc, int count);
+static __inline__ void ywrap_down(struct vc_data *vc, int count);
+static __inline__ void ypan_up(struct vc_data *vc, int count);
+static __inline__ void ypan_down(struct vc_data *vc, int count);
+static void fbcon_bmove_rec(struct vc_data *vc, struct fbcon_display *p, int sy, int sx,
+ int dy, int dx, int height, int width, u_int y_break);
static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
int unit);
+static void fbcon_redraw_move(struct vc_data *vc, struct fbcon_display *p,
+ int line, int count, int dy);
static void fbcon_modechanged(struct fb_info *info);
static void fbcon_set_all_vcs(struct fb_info *info);
static void fbcon_start(void);
@@ -1125,6 +1135,14 @@ static void fbcon_init(struct vc_data *vc, int init)

ops->graphics = 0;

+ /*
+ * No more hw acceleration for fbcon.
+ *
+ * FIXME: Garbage collect all the now dead code after sufficient time
+ * has passed.
+ */
+ p->scrollmode = SCROLL_REDRAW;
+
/*
* ++guenther: console.c:vc_allocate() relies on initializing
* vc_{cols,rows}, but we must not set those if we are only
@@ -1211,13 +1229,14 @@ static void fbcon_deinit(struct vc_data *vc)
* This system is now divided into two levels because of complications
* caused by hardware scrolling. Top level functions:
*
- * fbcon_clear(), fbcon_putc(), fbcon_clear_margins()
+ * fbcon_bmove(), fbcon_clear(), fbcon_putc(), fbcon_clear_margins()
*
* handles y values in range [0, scr_height-1] that correspond to real
* screen positions. y_wrap shift means that first line of bitmap may be
* anywhere on this display. These functions convert lineoffsets to
* bitmap offsets and deal with the wrap-around case by splitting blits.
*
+ * fbcon_bmove_physical_8() -- These functions fast implementations
* fbcon_clear_physical_8() -- of original fbcon_XXX fns.
* fbcon_putc_physical_8() -- (font width != 8) may be added later
*
@@ -1390,6 +1409,224 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
}
}

+static __inline__ void ywrap_up(struct vc_data *vc, int count)
+{
+ struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+ struct fbcon_ops *ops = info->fbcon_par;
+ struct fbcon_display *p = &fb_display[vc->vc_num];
+
+ p->yscroll += count;
+ if (p->yscroll >= p->vrows) /* Deal with wrap */
+ p->yscroll -= p->vrows;
+ ops->var.xoffset = 0;
+ ops->var.yoffset = p->yscroll * vc->vc_font.height;
+ ops->var.vmode |= FB_VMODE_YWRAP;
+ ops->update_start(info);
+ scrollback_max += count;
+ if (scrollback_max > scrollback_phys_max)
+ scrollback_max = scrollback_phys_max;
+ scrollback_current = 0;
+}
+
+static __inline__ void ywrap_down(struct vc_data *vc, int count)
+{
+ struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+ struct fbcon_ops *ops = info->fbcon_par;
+ struct fbcon_display *p = &fb_display[vc->vc_num];
+
+ p->yscroll -= count;
+ if (p->yscroll < 0) /* Deal with wrap */
+ p->yscroll += p->vrows;
+ ops->var.xoffset = 0;
+ ops->var.yoffset = p->yscroll * vc->vc_font.height;
+ ops->var.vmode |= FB_VMODE_YWRAP;
+ ops->update_start(info);
+ scrollback_max -= count;
+ if (scrollback_max < 0)
+ scrollback_max = 0;
+ scrollback_current = 0;
+}
+
+static __inline__ void ypan_up(struct vc_data *vc, int count)
+{
+ struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+ struct fbcon_display *p = &fb_display[vc->vc_num];
+ struct fbcon_ops *ops = info->fbcon_par;
+
+ p->yscroll += count;
+ if (p->yscroll > p->vrows - vc->vc_rows) {
+ ops->bmove(vc, info, p->vrows - vc->vc_rows,
+ 0, 0, 0, vc->vc_rows, vc->vc_cols);
+ p->yscroll -= p->vrows - vc->vc_rows;
+ }
+
+ ops->var.xoffset = 0;
+ ops->var.yoffset = p->yscroll * vc->vc_font.height;
+ ops->var.vmode &= ~FB_VMODE_YWRAP;
+ ops->update_start(info);
+ fbcon_clear_margins(vc, 1);
+ scrollback_max += count;
+ if (scrollback_max > scrollback_phys_max)
+ scrollback_max = scrollback_phys_max;
+ scrollback_current = 0;
+}
+
+static __inline__ void ypan_up_redraw(struct vc_data *vc, int t, int count)
+{
+ struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+ struct fbcon_ops *ops = info->fbcon_par;
+ struct fbcon_display *p = &fb_display[vc->vc_num];
+
+ p->yscroll += count;
+
+ if (p->yscroll > p->vrows - vc->vc_rows) {
+ p->yscroll -= p->vrows - vc->vc_rows;
+ fbcon_redraw_move(vc, p, t + count, vc->vc_rows - count, t);
+ }
+
+ ops->var.xoffset = 0;
+ ops->var.yoffset = p->yscroll * vc->vc_font.height;
+ ops->var.vmode &= ~FB_VMODE_YWRAP;
+ ops->update_start(info);
+ fbcon_clear_margins(vc, 1);
+ scrollback_max += count;
+ if (scrollback_max > scrollback_phys_max)
+ scrollback_max = scrollback_phys_max;
+ scrollback_current = 0;
+}
+
+static __inline__ void ypan_down(struct vc_data *vc, int count)
+{
+ struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+ struct fbcon_display *p = &fb_display[vc->vc_num];
+ struct fbcon_ops *ops = info->fbcon_par;
+
+ p->yscroll -= count;
+ if (p->yscroll < 0) {
+ ops->bmove(vc, info, 0, 0, p->vrows - vc->vc_rows,
+ 0, vc->vc_rows, vc->vc_cols);
+ p->yscroll += p->vrows - vc->vc_rows;
+ }
+
+ ops->var.xoffset = 0;
+ ops->var.yoffset = p->yscroll * vc->vc_font.height;
+ ops->var.vmode &= ~FB_VMODE_YWRAP;
+ ops->update_start(info);
+ fbcon_clear_margins(vc, 1);
+ scrollback_max -= count;
+ if (scrollback_max < 0)
+ scrollback_max = 0;
+ scrollback_current = 0;
+}
+
+static __inline__ void ypan_down_redraw(struct vc_data *vc, int t, int count)
+{
+ struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+ struct fbcon_ops *ops = info->fbcon_par;
+ struct fbcon_display *p = &fb_display[vc->vc_num];
+
+ p->yscroll -= count;
+
+ if (p->yscroll < 0) {
+ p->yscroll += p->vrows - vc->vc_rows;
+ fbcon_redraw_move(vc, p, t, vc->vc_rows - count, t + count);
+ }
+
+ ops->var.xoffset = 0;
+ ops->var.yoffset = p->yscroll * vc->vc_font.height;
+ ops->var.vmode &= ~FB_VMODE_YWRAP;
+ ops->update_start(info);
+ fbcon_clear_margins(vc, 1);
+ scrollback_max -= count;
+ if (scrollback_max < 0)
+ scrollback_max = 0;
+ scrollback_current = 0;
+}
+
+static void fbcon_redraw_move(struct vc_data *vc, struct fbcon_display *p,
+ int line, int count, int dy)
+{
+ unsigned short *s = (unsigned short *)
+ (vc->vc_origin + vc->vc_size_row * line);
+
+ while (count--) {
+ unsigned short *start = s;
+ unsigned short *le = advance_row(s, 1);
+ unsigned short c;
+ int x = 0;
+ unsigned short attr = 1;
+
+ do {
+ c = scr_readw(s);
+ if (attr != (c & 0xff00)) {
+ attr = c & 0xff00;
+ if (s > start) {
+ fbcon_putcs(vc, start, s - start,
+ dy, x);
+ x += s - start;
+ start = s;
+ }
+ }
+ console_conditional_schedule();
+ s++;
+ } while (s < le);
+ if (s > start)
+ fbcon_putcs(vc, start, s - start, dy, x);
+ console_conditional_schedule();
+ dy++;
+ }
+}
+
+static void fbcon_redraw_blit(struct vc_data *vc, struct fb_info *info,
+ struct fbcon_display *p, int line, int count, int ycount)
+{
+ int offset = ycount * vc->vc_cols;
+ unsigned short *d = (unsigned short *)
+ (vc->vc_origin + vc->vc_size_row * line);
+ unsigned short *s = d + offset;
+ struct fbcon_ops *ops = info->fbcon_par;
+
+ while (count--) {
+ unsigned short *start = s;
+ unsigned short *le = advance_row(s, 1);
+ unsigned short c;
+ int x = 0;
+
+ do {
+ c = scr_readw(s);
+
+ if (c == scr_readw(d)) {
+ if (s > start) {
+ ops->bmove(vc, info, line + ycount, x,
+ line, x, 1, s-start);
+ x += s - start + 1;
+ start = s + 1;
+ } else {
+ x++;
+ start++;
+ }
+ }
+
+ scr_writew(c, d);
+ console_conditional_schedule();
+ s++;
+ d++;
+ } while (s < le);
+ if (s > start)
+ ops->bmove(vc, info, line + ycount, x, line, x, 1,
+ s-start);
+ console_conditional_schedule();
+ if (ycount > 0)
+ line++;
+ else {
+ line--;
+ /* NOTE: We subtract two lines from these pointers */
+ s -= vc->vc_size_row;
+ d -= vc->vc_size_row;
+ }
+ }
+}
+
static void fbcon_redraw(struct vc_data *vc, struct fbcon_display *p,
int line, int count, int offset)
{
@@ -1450,6 +1687,7 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
{
struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
struct fbcon_display *p = &fb_display[vc->vc_num];
+ int scroll_partial = info->flags & FBINFO_PARTIAL_PAN_OK;

if (fbcon_is_inactive(vc, info))
return true;
@@ -1466,32 +1704,249 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
case SM_UP:
if (count > vc->vc_rows) /* Maximum realistic size */
count = vc->vc_rows;
- fbcon_redraw(vc, p, t, b - t - count,
- count * vc->vc_cols);
- fbcon_clear(vc, b - count, 0, count, vc->vc_cols);
- scr_memsetw((unsigned short *) (vc->vc_origin +
- vc->vc_size_row *
- (b - count)),
- vc->vc_video_erase_char,
- vc->vc_size_row * count);
- return true;
+ if (logo_shown >= 0)
+ goto redraw_up;
+ switch (p->scrollmode) {
+ case SCROLL_MOVE:
+ fbcon_redraw_blit(vc, info, p, t, b - t - count,
+ count);
+ fbcon_clear(vc, b - count, 0, count, vc->vc_cols);
+ scr_memsetw((unsigned short *) (vc->vc_origin +
+ vc->vc_size_row *
+ (b - count)),
+ vc->vc_video_erase_char,
+ vc->vc_size_row * count);
+ return true;
+
+ case SCROLL_WRAP_MOVE:
+ if (b - t - count > 3 * vc->vc_rows >> 2) {
+ if (t > 0)
+ fbcon_bmove(vc, 0, 0, count, 0, t,
+ vc->vc_cols);
+ ywrap_up(vc, count);
+ if (vc->vc_rows - b > 0)
+ fbcon_bmove(vc, b - count, 0, b, 0,
+ vc->vc_rows - b,
+ vc->vc_cols);
+ } else if (info->flags & FBINFO_READS_FAST)
+ fbcon_bmove(vc, t + count, 0, t, 0,
+ b - t - count, vc->vc_cols);
+ else
+ goto redraw_up;
+ fbcon_clear(vc, b - count, 0, count, vc->vc_cols);
+ break;
+
+ case SCROLL_PAN_REDRAW:
+ if ((p->yscroll + count <=
+ 2 * (p->vrows - vc->vc_rows))
+ && ((!scroll_partial && (b - t == vc->vc_rows))
+ || (scroll_partial
+ && (b - t - count >
+ 3 * vc->vc_rows >> 2)))) {
+ if (t > 0)
+ fbcon_redraw_move(vc, p, 0, t, count);
+ ypan_up_redraw(vc, t, count);
+ if (vc->vc_rows - b > 0)
+ fbcon_redraw_move(vc, p, b,
+ vc->vc_rows - b, b);
+ } else
+ fbcon_redraw_move(vc, p, t + count, b - t - count, t);
+ fbcon_clear(vc, b - count, 0, count, vc->vc_cols);
+ break;
+
+ case SCROLL_PAN_MOVE:
+ if ((p->yscroll + count <=
+ 2 * (p->vrows - vc->vc_rows))
+ && ((!scroll_partial && (b - t == vc->vc_rows))
+ || (scroll_partial
+ && (b - t - count >
+ 3 * vc->vc_rows >> 2)))) {
+ if (t > 0)
+ fbcon_bmove(vc, 0, 0, count, 0, t,
+ vc->vc_cols);
+ ypan_up(vc, count);
+ if (vc->vc_rows - b > 0)
+ fbcon_bmove(vc, b - count, 0, b, 0,
+ vc->vc_rows - b,
+ vc->vc_cols);
+ } else if (info->flags & FBINFO_READS_FAST)
+ fbcon_bmove(vc, t + count, 0, t, 0,
+ b - t - count, vc->vc_cols);
+ else
+ goto redraw_up;
+ fbcon_clear(vc, b - count, 0, count, vc->vc_cols);
+ break;
+
+ case SCROLL_REDRAW:
+ redraw_up:
+ fbcon_redraw(vc, p, t, b - t - count,
+ count * vc->vc_cols);
+ fbcon_clear(vc, b - count, 0, count, vc->vc_cols);
+ scr_memsetw((unsigned short *) (vc->vc_origin +
+ vc->vc_size_row *
+ (b - count)),
+ vc->vc_video_erase_char,
+ vc->vc_size_row * count);
+ return true;
+ }
+ break;

case SM_DOWN:
if (count > vc->vc_rows) /* Maximum realistic size */
count = vc->vc_rows;
- fbcon_redraw(vc, p, b - 1, b - t - count,
- -count * vc->vc_cols);
- fbcon_clear(vc, t, 0, count, vc->vc_cols);
- scr_memsetw((unsigned short *) (vc->vc_origin +
- vc->vc_size_row *
- t),
- vc->vc_video_erase_char,
- vc->vc_size_row * count);
- return true;
+ if (logo_shown >= 0)
+ goto redraw_down;
+ switch (p->scrollmode) {
+ case SCROLL_MOVE:
+ fbcon_redraw_blit(vc, info, p, b - 1, b - t - count,
+ -count);
+ fbcon_clear(vc, t, 0, count, vc->vc_cols);
+ scr_memsetw((unsigned short *) (vc->vc_origin +
+ vc->vc_size_row *
+ t),
+ vc->vc_video_erase_char,
+ vc->vc_size_row * count);
+ return true;
+
+ case SCROLL_WRAP_MOVE:
+ if (b - t - count > 3 * vc->vc_rows >> 2) {
+ if (vc->vc_rows - b > 0)
+ fbcon_bmove(vc, b, 0, b - count, 0,
+ vc->vc_rows - b,
+ vc->vc_cols);
+ ywrap_down(vc, count);
+ if (t > 0)
+ fbcon_bmove(vc, count, 0, 0, 0, t,
+ vc->vc_cols);
+ } else if (info->flags & FBINFO_READS_FAST)
+ fbcon_bmove(vc, t, 0, t + count, 0,
+ b - t - count, vc->vc_cols);
+ else
+ goto redraw_down;
+ fbcon_clear(vc, t, 0, count, vc->vc_cols);
+ break;
+
+ case SCROLL_PAN_MOVE:
+ if ((count - p->yscroll <= p->vrows - vc->vc_rows)
+ && ((!scroll_partial && (b - t == vc->vc_rows))
+ || (scroll_partial
+ && (b - t - count >
+ 3 * vc->vc_rows >> 2)))) {
+ if (vc->vc_rows - b > 0)
+ fbcon_bmove(vc, b, 0, b - count, 0,
+ vc->vc_rows - b,
+ vc->vc_cols);
+ ypan_down(vc, count);
+ if (t > 0)
+ fbcon_bmove(vc, count, 0, 0, 0, t,
+ vc->vc_cols);
+ } else if (info->flags & FBINFO_READS_FAST)
+ fbcon_bmove(vc, t, 0, t + count, 0,
+ b - t - count, vc->vc_cols);
+ else
+ goto redraw_down;
+ fbcon_clear(vc, t, 0, count, vc->vc_cols);
+ break;
+
+ case SCROLL_PAN_REDRAW:
+ if ((count - p->yscroll <= p->vrows - vc->vc_rows)
+ && ((!scroll_partial && (b - t == vc->vc_rows))
+ || (scroll_partial
+ && (b - t - count >
+ 3 * vc->vc_rows >> 2)))) {
+ if (vc->vc_rows - b > 0)
+ fbcon_redraw_move(vc, p, b, vc->vc_rows - b,
+ b - count);
+ ypan_down_redraw(vc, t, count);
+ if (t > 0)
+ fbcon_redraw_move(vc, p, count, t, 0);
+ } else
+ fbcon_redraw_move(vc, p, t, b - t - count, t + count);
+ fbcon_clear(vc, t, 0, count, vc->vc_cols);
+ break;
+
+ case SCROLL_REDRAW:
+ redraw_down:
+ fbcon_redraw(vc, p, b - 1, b - t - count,
+ -count * vc->vc_cols);
+ fbcon_clear(vc, t, 0, count, vc->vc_cols);
+ scr_memsetw((unsigned short *) (vc->vc_origin +
+ vc->vc_size_row *
+ t),
+ vc->vc_video_erase_char,
+ vc->vc_size_row * count);
+ return true;
+ }
}
return false;
}

+
+static void fbcon_bmove(struct vc_data *vc, int sy, int sx, int dy, int dx,
+ int height, int width)
+{
+ struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+ struct fbcon_display *p = &fb_display[vc->vc_num];
+
+ if (fbcon_is_inactive(vc, info))
+ return;
+
+ if (!width || !height)
+ return;
+
+ /* Split blits that cross physical y_wrap case.
+ * Pathological case involves 4 blits, better to use recursive
+ * code rather than unrolled case
+ *
+ * Recursive invocations don't need to erase the cursor over and
+ * over again, so we use fbcon_bmove_rec()
+ */
+ fbcon_bmove_rec(vc, p, sy, sx, dy, dx, height, width,
+ p->vrows - p->yscroll);
+}
+
+static void fbcon_bmove_rec(struct vc_data *vc, struct fbcon_display *p, int sy, int sx,
+ int dy, int dx, int height, int width, u_int y_break)
+{
+ struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+ struct fbcon_ops *ops = info->fbcon_par;
+ u_int b;
+
+ if (sy < y_break && sy + height > y_break) {
+ b = y_break - sy;
+ if (dy < sy) { /* Avoid trashing self */
+ fbcon_bmove_rec(vc, p, sy, sx, dy, dx, b, width,
+ y_break);
+ fbcon_bmove_rec(vc, p, sy + b, sx, dy + b, dx,
+ height - b, width, y_break);
+ } else {
+ fbcon_bmove_rec(vc, p, sy + b, sx, dy + b, dx,
+ height - b, width, y_break);
+ fbcon_bmove_rec(vc, p, sy, sx, dy, dx, b, width,
+ y_break);
+ }
+ return;
+ }
+
+ if (dy < y_break && dy + height > y_break) {
+ b = y_break - dy;
+ if (dy < sy) { /* Avoid trashing self */
+ fbcon_bmove_rec(vc, p, sy, sx, dy, dx, b, width,
+ y_break);
+ fbcon_bmove_rec(vc, p, sy + b, sx, dy + b, dx,
+ height - b, width, y_break);
+ } else {
+ fbcon_bmove_rec(vc, p, sy + b, sx, dy + b, dx,
+ height - b, width, y_break);
+ fbcon_bmove_rec(vc, p, sy, sx, dy, dx, b, width,
+ y_break);
+ }
+ return;
+ }
+ ops->bmove(vc, info, real_y(p, sy), sx, real_y(p, dy), dx,
+ height, width);
+}
+
static void updatescrollmode(struct fbcon_display *p,
struct fb_info *info,
struct vc_data *vc)
@@ -1664,7 +2119,21 @@ static int fbcon_switch(struct vc_data *vc)

updatescrollmode(p, info, vc);

- scrollback_phys_max = 0;
+ switch (p->scrollmode) {
+ case SCROLL_WRAP_MOVE:
+ scrollback_phys_max = p->vrows - vc->vc_rows;
+ break;
+ case SCROLL_PAN_MOVE:
+ case SCROLL_PAN_REDRAW:
+ scrollback_phys_max = p->vrows - 2 * vc->vc_rows;
+ if (scrollback_phys_max < 0)
+ scrollback_phys_max = 0;
+ break;
+ default:
+ scrollback_phys_max = 0;
+ break;
+ }
+
scrollback_max = 0;
scrollback_current = 0;

diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index a00603b4451a..9315b360c898 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -29,6 +29,7 @@ struct fbcon_display {
/* Filled in by the low-level console driver */
const u_char *fontdata;
int userfont; /* != 0 if fontdata kmalloc()ed */
+ u_short scrollmode; /* Scroll Method */
u_short inverse; /* != 0 text black on white as default */
short yscroll; /* Hardware scrolling */
int vrows; /* number of virtual rows */
@@ -51,6 +52,8 @@ struct fbcon_display {
};

struct fbcon_ops {
+ void (*bmove)(struct vc_data *vc, struct fb_info *info, int sy,
+ int sx, int dy, int dx, int height, int width);
void (*clear)(struct vc_data *vc, struct fb_info *info, int sy,
int sx, int height, int width);
void (*putcs)(struct vc_data *vc, struct fb_info *info,
@@ -149,6 +152,62 @@ static inline int attr_col_ec(int shift, struct vc_data *vc,
#define attr_bgcol_ec(bgshift, vc, info) attr_col_ec(bgshift, vc, info, 0)
#define attr_fgcol_ec(fgshift, vc, info) attr_col_ec(fgshift, vc, info, 1)

+ /*
+ * Scroll Method
+ */
+
+/* There are several methods fbcon can use to move text around the screen:
+ *
+ * Operation Pan Wrap
+ *---------------------------------------------
+ * SCROLL_MOVE copyarea No No
+ * SCROLL_PAN_MOVE copyarea Yes No
+ * SCROLL_WRAP_MOVE copyarea No Yes
+ * SCROLL_REDRAW imageblit No No
+ * SCROLL_PAN_REDRAW imageblit Yes No
+ * SCROLL_WRAP_REDRAW imageblit No Yes
+ *
+ * (SCROLL_WRAP_REDRAW is not implemented yet)
+ *
+ * In general, fbcon will choose the best scrolling
+ * method based on the rule below:
+ *
+ * Pan/Wrap > accel imageblit > accel copyarea >
+ * soft imageblit > (soft copyarea)
+ *
+ * Exception to the rule: Pan + accel copyarea is
+ * preferred over Pan + accel imageblit.
+ *
+ * The above is typical for PCI/AGP cards. Unless
+ * overridden, fbcon will never use soft copyarea.
+ *
+ * If you need to override the above rule, set the
+ * appropriate flags in fb_info->flags. For example,
+ * to prefer copyarea over imageblit, set
+ * FBINFO_READS_FAST.
+ *
+ * Other notes:
+ * + use the hardware engine to move the text
+ * (hw-accelerated copyarea() and fillrect())
+ * + use hardware-supported panning on a large virtual screen
+ * + amifb can not only pan, but also wrap the display by N lines
+ * (i.e. visible line i = physical line (i+N) % yres).
+ * + read what's already rendered on the screen and
+ * write it in a different place (this is cfb_copyarea())
+ * + re-render the text to the screen
+ *
+ * Whether to use wrapping or panning can only be figured out at
+ * runtime (when we know whether our font height is a multiple
+ * of the pan/wrap step)
+ *
+ */
+
+#define SCROLL_MOVE 0x001
+#define SCROLL_PAN_MOVE 0x002
+#define SCROLL_WRAP_MOVE 0x003
+#define SCROLL_REDRAW 0x004
+#define SCROLL_PAN_REDRAW 0x005
+
#ifdef CONFIG_FB_TILEBLITTING
extern void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info);
#endif
diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c
index ffa78936eaab..9cd2c4b05c32 100644
--- a/drivers/video/fbdev/core/fbcon_ccw.c
+++ b/drivers/video/fbdev/core/fbcon_ccw.c
@@ -59,12 +59,31 @@ static void ccw_update_attr(u8 *dst, u8 *src, int attribute,
}
}

+
+static void ccw_bmove(struct vc_data *vc, struct fb_info *info, int sy,
+ int sx, int dy, int dx, int height, int width)
+{
+ struct fbcon_ops *ops = info->fbcon_par;
+ struct fb_copyarea area;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);
+
+ area.sx = sy * vc->vc_font.height;
+ area.sy = vyres - ((sx + width) * vc->vc_font.width);
+ area.dx = dy * vc->vc_font.height;
+ area.dy = vyres - ((dx + width) * vc->vc_font.width);
+ area.width = height * vc->vc_font.height;
+ area.height = width * vc->vc_font.width;
+
+ info->fbops->fb_copyarea(info, &area);
+}
+
static void ccw_clear(struct vc_data *vc, struct fb_info *info, int sy,
int sx, int height, int width)
{
+ struct fbcon_ops *ops = info->fbcon_par;
struct fb_fillrect region;
int bgshift = (vc->vc_hi_font_mask) ? 13 : 12;
- u32 vyres = info->var.yres;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);

region.color = attr_bgcol_ec(bgshift,vc,info);
region.dx = sy * vc->vc_font.height;
@@ -121,7 +140,7 @@ static void ccw_putcs(struct vc_data *vc, struct fb_info *info,
u32 cnt, pitch, size;
u32 attribute = get_attribute(info, scr_readw(s));
u8 *dst, *buf = NULL;
- u32 vyres = info->var.yres;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);

if (!ops->fontbuffer)
return;
@@ -210,7 +229,7 @@ static void ccw_cursor(struct vc_data *vc, struct fb_info *info, int mode,
int attribute, use_sw = vc->vc_cursor_type & CUR_SW;
int err = 1, dx, dy;
char *src;
- u32 vyres = info->var.yres;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);

if (!ops->fontbuffer)
return;
@@ -368,7 +387,7 @@ static int ccw_update_start(struct fb_info *info)
{
struct fbcon_ops *ops = info->fbcon_par;
u32 yoffset;
- u32 vyres = info->var.yres;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);
int err;

yoffset = (vyres - info->var.yres) - ops->var.xoffset;
@@ -383,6 +402,7 @@ static int ccw_update_start(struct fb_info *info)

void fbcon_rotate_ccw(struct fbcon_ops *ops)
{
+ ops->bmove = ccw_bmove;
ops->clear = ccw_clear;
ops->putcs = ccw_putcs;
ops->clear_margins = ccw_clear_margins;
diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c
index 92e5b7fb51ee..88d89fad3f05 100644
--- a/drivers/video/fbdev/core/fbcon_cw.c
+++ b/drivers/video/fbdev/core/fbcon_cw.c
@@ -44,12 +44,31 @@ static void cw_update_attr(u8 *dst, u8 *src, int attribute,
}
}

+
+static void cw_bmove(struct vc_data *vc, struct fb_info *info, int sy,
+ int sx, int dy, int dx, int height, int width)
+{
+ struct fbcon_ops *ops = info->fbcon_par;
+ struct fb_copyarea area;
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);
+
+ area.sx = vxres - ((sy + height) * vc->vc_font.height);
+ area.sy = sx * vc->vc_font.width;
+ area.dx = vxres - ((dy + height) * vc->vc_font.height);
+ area.dy = dx * vc->vc_font.width;
+ area.width = height * vc->vc_font.height;
+ area.height = width * vc->vc_font.width;
+
+ info->fbops->fb_copyarea(info, &area);
+}
+
static void cw_clear(struct vc_data *vc, struct fb_info *info, int sy,
int sx, int height, int width)
{
+ struct fbcon_ops *ops = info->fbcon_par;
struct fb_fillrect region;
int bgshift = (vc->vc_hi_font_mask) ? 13 : 12;
- u32 vxres = info->var.xres;
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);

region.color = attr_bgcol_ec(bgshift,vc,info);
region.dx = vxres - ((sy + height) * vc->vc_font.height);
@@ -106,7 +125,7 @@ static void cw_putcs(struct vc_data *vc, struct fb_info *info,
u32 cnt, pitch, size;
u32 attribute = get_attribute(info, scr_readw(s));
u8 *dst, *buf = NULL;
- u32 vxres = info->var.xres;
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);

if (!ops->fontbuffer)
return;
@@ -193,7 +212,7 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, int mode,
int attribute, use_sw = vc->vc_cursor_type & CUR_SW;
int err = 1, dx, dy;
char *src;
- u32 vxres = info->var.xres;
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);

if (!ops->fontbuffer)
return;
@@ -350,7 +369,7 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, int mode,
static int cw_update_start(struct fb_info *info)
{
struct fbcon_ops *ops = info->fbcon_par;
- u32 vxres = info->var.xres;
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);
u32 xoffset;
int err;

@@ -366,6 +385,7 @@ static int cw_update_start(struct fb_info *info)

void fbcon_rotate_cw(struct fbcon_ops *ops)
{
+ ops->bmove = cw_bmove;
ops->clear = cw_clear;
ops->putcs = cw_putcs;
ops->clear_margins = cw_clear_margins;
diff --git a/drivers/video/fbdev/core/fbcon_rotate.h b/drivers/video/fbdev/core/fbcon_rotate.h
index b528b2e54283..e233444cda66 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.h
+++ b/drivers/video/fbdev/core/fbcon_rotate.h
@@ -11,6 +11,15 @@
#ifndef _FBCON_ROTATE_H
#define _FBCON_ROTATE_H

+#define GETVYRES(s,i) ({ \
+ (s == SCROLL_REDRAW || s == SCROLL_MOVE) ? \
+ (i)->var.yres : (i)->var.yres_virtual; })
+
+#define GETVXRES(s,i) ({ \
+ (s == SCROLL_REDRAW || s == SCROLL_MOVE || !(i)->fix.xpanstep) ? \
+ (i)->var.xres : (i)->var.xres_virtual; })
+
+
static inline int pattern_test_bit(u32 x, u32 y, u32 pitch, const char *pat)
{
u32 tmp = (y * pitch) + x, index = tmp / 8, bit = tmp % 8;
diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c
index 09619bd8e021..8d5e66b1bdfb 100644
--- a/drivers/video/fbdev/core/fbcon_ud.c
+++ b/drivers/video/fbdev/core/fbcon_ud.c
@@ -44,13 +44,33 @@ static void ud_update_attr(u8 *dst, u8 *src, int attribute,
}
}

+
+static void ud_bmove(struct vc_data *vc, struct fb_info *info, int sy,
+ int sx, int dy, int dx, int height, int width)
+{
+ struct fbcon_ops *ops = info->fbcon_par;
+ struct fb_copyarea area;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);
+
+ area.sy = vyres - ((sy + height) * vc->vc_font.height);
+ area.sx = vxres - ((sx + width) * vc->vc_font.width);
+ area.dy = vyres - ((dy + height) * vc->vc_font.height);
+ area.dx = vxres - ((dx + width) * vc->vc_font.width);
+ area.height = height * vc->vc_font.height;
+ area.width = width * vc->vc_font.width;
+
+ info->fbops->fb_copyarea(info, &area);
+}
+
static void ud_clear(struct vc_data *vc, struct fb_info *info, int sy,
int sx, int height, int width)
{
+ struct fbcon_ops *ops = info->fbcon_par;
struct fb_fillrect region;
int bgshift = (vc->vc_hi_font_mask) ? 13 : 12;
- u32 vyres = info->var.yres;
- u32 vxres = info->var.xres;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);

region.color = attr_bgcol_ec(bgshift,vc,info);
region.dy = vyres - ((sy + height) * vc->vc_font.height);
@@ -142,8 +162,8 @@ static void ud_putcs(struct vc_data *vc, struct fb_info *info,
u32 mod = vc->vc_font.width % 8, cnt, pitch, size;
u32 attribute = get_attribute(info, scr_readw(s));
u8 *dst, *buf = NULL;
- u32 vyres = info->var.yres;
- u32 vxres = info->var.xres;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);

if (!ops->fontbuffer)
return;
@@ -239,8 +259,8 @@ static void ud_cursor(struct vc_data *vc, struct fb_info *info, int mode,
int attribute, use_sw = vc->vc_cursor_type & CUR_SW;
int err = 1, dx, dy;
char *src;
- u32 vyres = info->var.yres;
- u32 vxres = info->var.xres;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);

if (!ops->fontbuffer)
return;
@@ -390,8 +410,8 @@ static int ud_update_start(struct fb_info *info)
{
struct fbcon_ops *ops = info->fbcon_par;
int xoffset, yoffset;
- u32 vyres = info->var.yres;
- u32 vxres = info->var.xres;
+ u32 vyres = GETVYRES(ops->p->scrollmode, info);
+ u32 vxres = GETVXRES(ops->p->scrollmode, info);
int err;

xoffset = vxres - info->var.xres - ops->var.xoffset;
@@ -409,6 +429,7 @@ static int ud_update_start(struct fb_info *info)

void fbcon_rotate_ud(struct fbcon_ops *ops)
{
+ ops->bmove = ud_bmove;
ops->clear = ud_clear;
ops->putcs = ud_putcs;
ops->clear_margins = ud_clear_margins;
diff --git a/drivers/video/fbdev/core/tileblit.c b/drivers/video/fbdev/core/tileblit.c
index 72af95053bcb..2768eff247ba 100644
--- a/drivers/video/fbdev/core/tileblit.c
+++ b/drivers/video/fbdev/core/tileblit.c
@@ -16,6 +16,21 @@
#include <asm/types.h>
#include "fbcon.h"

+static void tile_bmove(struct vc_data *vc, struct fb_info *info, int sy,
+ int sx, int dy, int dx, int height, int width)
+{
+ struct fb_tilearea area;
+
+ area.sx = sx;
+ area.sy = sy;
+ area.dx = dx;
+ area.dy = dy;
+ area.height = height;
+ area.width = width;
+
+ info->tileops->fb_tilecopy(info, &area);
+}
+
static void tile_clear(struct vc_data *vc, struct fb_info *info, int sy,
int sx, int height, int width)
{
@@ -118,6 +133,7 @@ void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info)
struct fb_tilemap map;
struct fbcon_ops *ops = info->fbcon_par;

+ ops->bmove = tile_bmove;
ops->clear = tile_clear;
ops->putcs = tile_putcs;
ops->clear_margins = tile_clear_margins;
diff --git a/drivers/video/fbdev/skeletonfb.c b/drivers/video/fbdev/skeletonfb.c
index 0fe922f726e9..bcacfb6934fa 100644
--- a/drivers/video/fbdev/skeletonfb.c
+++ b/drivers/video/fbdev/skeletonfb.c
@@ -505,15 +505,15 @@ void xxxfb_fillrect(struct fb_info *p, const struct fb_fillrect *region)
}

/**
- * xxxfb_copyarea - OBSOLETE function.
+ * xxxfb_copyarea - REQUIRED function. Can use generic routines if
+ * non acclerated hardware and packed pixel based.
* Copies one area of the screen to another area.
- * Will be deleted in a future version
*
* @info: frame buffer structure that represents a single frame buffer
* @area: Structure providing the data to copy the framebuffer contents
* from one region to another.
*
- * This drawing operation copied a rectangular area from one area of the
+ * This drawing operation copies a rectangular area from one area of the
* screen to another area.
*/
void xxxfb_copyarea(struct fb_info *p, const struct fb_copyarea *area)
@@ -645,9 +645,9 @@ static const struct fb_ops xxxfb_ops = {
.fb_setcolreg = xxxfb_setcolreg,
.fb_blank = xxxfb_blank,
.fb_pan_display = xxxfb_pan_display,
- .fb_fillrect = xxxfb_fillrect, /* Needed !!! */
- .fb_copyarea = xxxfb_copyarea, /* Obsolete */
- .fb_imageblit = xxxfb_imageblit, /* Needed !!! */
+ .fb_fillrect = xxxfb_fillrect, /* Needed !!! */
+ .fb_copyarea = xxxfb_copyarea, /* Needed !!! */
+ .fb_imageblit = xxxfb_imageblit, /* Needed !!! */
.fb_cursor = xxxfb_cursor, /* Optional !!! */
.fb_sync = xxxfb_sync,
.fb_ioctl = xxxfb_ioctl,
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 3da95842b207..02f362c661c8 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -262,7 +262,7 @@ struct fb_ops {

/* Draws a rectangle */
void (*fb_fillrect) (struct fb_info *info, const struct fb_fillrect *rect);
- /* Copy data from area to another. Obsolete. */
+ /* Copy data from area to another */
void (*fb_copyarea) (struct fb_info *info, const struct fb_copyarea *region);
/* Draws a image to the display */
void (*fb_imageblit) (struct fb_info *info, const struct fb_image *image);
--
2.31.1

2022-01-21 19:11:16

by Greg KH

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

2022-01-21 19:11:18

by Greg KH

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote:
> > This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
> >
> > Revert this patch. This patch started to introduce the regression that
> > all hardware acceleration of more than 35 existing fbdev drivers were
> > bypassed and thus fbcon console output for those was dramatically slowed
> > down by factor of 10 and more.
> >
> > Reverting this commit has no impact on DRM, since none of the DRM drivers are
> > tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
> > FBINFO_HWACCEL_FILLRECT or others.
> >
> > Signed-off-by: Helge Deller <[email protected]>
> > Cc: [email protected] # v5.16
>
> Why just 5.16? This commit came in on 5.11 and was backported to
> 5.10.5.
>
> As for "why", I think there was a number of private bugs that were
> reported in this code, which is why it was removed. I do not think it
> can be safely added back in without addressing them first. Let me go
> dig through my email to see if I can find them...

Ah, no, that was just the soft scrollback code I was thinking of, which
was a different revert and is still gone, thankfully :)

This one was just removed because Daniel noticed that only 3 drivers
used this (nouveau, omapdrm, and gma600), so this shouldn't have caused
any regressions in any other drivers like you are reporting here.

So perhaps this regression is caused by something else?

thanks,

greg k-h

2022-01-21 19:12:08

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hi Greg,

On Wed, Jan 19, 2022 at 12:28 PM Greg Kroah-Hartman
<[email protected]> wrote:
> On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote:
> > > This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
> > >
> > > Revert this patch. This patch started to introduce the regression that
> > > all hardware acceleration of more than 35 existing fbdev drivers were
> > > bypassed and thus fbcon console output for those was dramatically slowed
> > > down by factor of 10 and more.
> > >
> > > Reverting this commit has no impact on DRM, since none of the DRM drivers are
> > > tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
> > > FBINFO_HWACCEL_FILLRECT or others.
> > >
> > > Signed-off-by: Helge Deller <[email protected]>

> > As for "why", I think there was a number of private bugs that were
> > reported in this code, which is why it was removed. I do not think it
> > can be safely added back in without addressing them first. Let me go
> > dig through my email to see if I can find them...
>
> Ah, no, that was just the soft scrollback code I was thinking of, which

So the bugs argument is moot.

> was a different revert and is still gone, thankfully :)

FTR, not everyone else was thankful about that one...

> This one was just removed because Daniel noticed that only 3 drivers
> used this (nouveau, omapdrm, and gma600), so this shouldn't have caused
> any regressions in any other drivers like you are reporting here.
>
> So perhaps this regression is caused by something else?

1. Daniel's patch was not CCed to linux-fbdev,
2. When I discovered the patch, I pointed out that the premise of 3
drivers was not true, and that it affects 32 more fbdev drivers[1] .
The patch was applied regardless.
3. When the patch was suggested for backporting, I pointed out the
same[2].
The patch was backported regardless.

[1] https://lore.kernel.org/r/[email protected]/
[2] https://lore.kernel.org/r/CAMuHMdXRgam2zahPEGcw8+76Xm-0AO-Ci9-YmVa5JpTKVHphRw@mail.gmail.com/

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-01-21 19:14:06

[permalink] [raw]

Subject: Re: [PATCH 0/2] Fix regression introduced by disabling accelerated scrolling in fbcon

Hi Helge,

On Wed, Jan 19, 2022 at 12:10 PM Helge Deller <[email protected]> wrote:
> 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"

Thank you for this series, and the prior analysis!

Acked-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-01-21 19:14:12

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hello Greg,

On 1/19/22 12:47, Geert Uytterhoeven wrote:
> On Wed, Jan 19, 2022 at 12:28 PM Greg Kroah-Hartman
> <[email protected]> wrote:
>> On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote:
>>> On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote:
>>>> This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
>>>>
>>>> Revert this patch. This patch started to introduce the regression that
>>>> all hardware acceleration of more than 35 existing fbdev drivers were
>>>> bypassed and thus fbcon console output for those was dramatically slowed
>>>> down by factor of 10 and more.
>>>>
>>>> Reverting this commit has no impact on DRM, since none of the DRM drivers are
>>>> tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
>>>> FBINFO_HWACCEL_FILLRECT or others.
>>>>
>>>> Signed-off-by: Helge Deller <[email protected]>
>
>>> As for "why", I think there was a number of private bugs that were
>>> reported in this code, which is why it was removed. I do not think it
>>> can be safely added back in without addressing them first. Let me go
>>> dig through my email to see if I can find them...
>>
>> Ah, no, that was just the soft scrollback code I was thinking of, which

Right.
That was commit 973c096f6a85 and it was about vgacon, not fbcon.

I did mentioned it in my cover letter, together with my analysis of
the reported bugs.

Maybe I should have put all the information from the cover letter into
the patch here as well. If you haven't read the cover letter yet, please do.

Helge

> So the bugs argument is moot.
>
>> was a different revert and is still gone, thankfully :)
>
> FTR, not everyone else was thankful about that one...
>
>> This one was just removed because Daniel noticed that only 3 drivers
>> used this (nouveau, omapdrm, and gma600), so this shouldn't have caused
>> any regressions in any other drivers like you are reporting here.
>>
>> So perhaps this regression is caused by something else?
>
> 1. Daniel's patch was not CCed to linux-fbdev,
> 2. When I discovered the patch, I pointed out that the premise of 3
> drivers was not true, and that it affects 32 more fbdev drivers[1] .
> The patch was applied regardless.
> 3. When the patch was suggested for backporting, I pointed out the
> same[2].
> The patch was backported regardless.
>
> [1] https://lore.kernel.org/r/[email protected]/
> [2] https://lore.kernel.org/r/CAMuHMdXRgam2zahPEGcw8+76Xm-0AO-Ci9-YmVa5JpTKVHphRw@mail.gmail.com/
>
> Gr{oetje,eeting}s,
>
> Geert

2022-01-21 19:14:43

by Sven Schnelle

[permalink] [raw]

Subject: Re: [PATCH 0/2] Fix regression introduced by disabling accelerated scrolling in fbcon

Helge Deller <[email protected]> writes:

> 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(-)

Thanks Helge!

Feel free to add my:

Acked-by: Sven Schnelle <[email protected]>

2022-01-21 19:15:41

by Greg KH

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Wed, Jan 19, 2022 at 02:01:44PM +0100, Sven Schnelle wrote:
> Hi Greg,
>
> Greg Kroah-Hartman <[email protected]> writes:
>
> > On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote:
> >> On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote:
> >> > This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
> >> >
> >> > Revert this patch. This patch started to introduce the regression that
> >> > all hardware acceleration of more than 35 existing fbdev drivers were
> >> > bypassed and thus fbcon console output for those was dramatically slowed
> >> > down by factor of 10 and more.
> >> >
> >> > Reverting this commit has no impact on DRM, since none of the DRM drivers are
> >> > tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
> >> > FBINFO_HWACCEL_FILLRECT or others.
> >> >
> >> > Signed-off-by: Helge Deller <[email protected]>
> >> > Cc: [email protected] # v5.16
> >>
> >> Why just 5.16? This commit came in on 5.11 and was backported to
> >> 5.10.5.
> >>
> >> As for "why", I think there was a number of private bugs that were
> >> reported in this code, which is why it was removed. I do not think it
> >> can be safely added back in without addressing them first. Let me go
> >> dig through my email to see if I can find them...
> >
> > Ah, no, that was just the soft scrollback code I was thinking of, which
> > was a different revert and is still gone, thankfully :)
> >
> > This one was just removed because Daniel noticed that only 3 drivers
> > used this (nouveau, omapdrm, and gma600), so this shouldn't have caused
> > any regressions in any other drivers like you are reporting here.
>
> I'm counting more than 3 drivers using this. I think one of the reasons
> why it was reverted was that no one is actively maintaining fbdev. With
> Helge now volunteering i don't see a reason why it should stay reverted.
> If there are issues coming up i'm pretty sure Helge would care, and i
> would probably also take a look.

Ok, no objection from me, but I think Daniel should weigh in as it is
his commit that is being reverted here.

thanks,

greg k-h

2022-01-21 19:15:41

by Sven Schnelle

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hi Greg,

Greg Kroah-Hartman <[email protected]> writes:

> On Wed, Jan 19, 2022 at 12:22:55PM +0100, Greg Kroah-Hartman wrote:
>> On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote:
>> > This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
>> >
>> > Revert this patch. This patch started to introduce the regression that
>> > all hardware acceleration of more than 35 existing fbdev drivers were
>> > bypassed and thus fbcon console output for those was dramatically slowed
>> > down by factor of 10 and more.
>> >
>> > Reverting this commit has no impact on DRM, since none of the DRM drivers are
>> > tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
>> > FBINFO_HWACCEL_FILLRECT or others.
>> >
>> > Signed-off-by: Helge Deller <[email protected]>
>> > Cc: [email protected] # v5.16
>>
>> Why just 5.16? This commit came in on 5.11 and was backported to
>> 5.10.5.
>>
>> As for "why", I think there was a number of private bugs that were
>> reported in this code, which is why it was removed. I do not think it
>> can be safely added back in without addressing them first. Let me go
>> dig through my email to see if I can find them...
>
> Ah, no, that was just the soft scrollback code I was thinking of, which
> was a different revert and is still gone, thankfully :)
>
> This one was just removed because Daniel noticed that only 3 drivers
> used this (nouveau, omapdrm, and gma600), so this shouldn't have caused
> any regressions in any other drivers like you are reporting here.

I'm counting more than 3 drivers using this. I think one of the reasons
why it was reverted was that no one is actively maintaining fbdev. With
Helge now volunteering i don't see a reason why it should stay reverted.
If there are issues coming up i'm pretty sure Helge would care, and i
would probably also take a look.

/Sven

2022-01-21 19:17:24

by Linus Torvalds

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Wed, Jan 19, 2022 at 2:29 PM Helge Deller <[email protected]> wrote:
>
> >>
> >> Ah, no, that was just the soft scrollback code I was thinking of, which
>
> Right.
> That was commit 973c096f6a85 and it was about vgacon, not fbcon.

No, fbcon had some bug too, although I've paged out the details. See
commit 50145474f6ef ("fbcon: remove soft scrollback code").

If I remember correctly (and it's entirely possible that I don't), the
whole "softback_lines" logic had serious problems with resizing the
console (or maybe changing the font size).

There may have been some other bad interaction with
foreground/background consoles too, I forget.

Linus

2022-01-21 19:18:27

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds
<[email protected]> wrote:
>
> On Wed, Jan 19, 2022 at 2:29 PM Helge Deller <[email protected]> wrote:
> >
> > >>
> > >> Ah, no, that was just the soft scrollback code I was thinking of, which
> >
> > Right.
> > That was commit 973c096f6a85 and it was about vgacon, not fbcon.
>
> No, fbcon had some bug too, although I've paged out the details. See
> commit 50145474f6ef ("fbcon: remove soft scrollback code").

tbh I've paged it all out too.

> If I remember correctly (and it's entirely possible that I don't), the
> whole "softback_lines" logic had serious problems with resizing the
> console (or maybe changing the font size).

Yeah that pile of reverts was my motiviation to look into this and see
what else we could rip out most likely and still have an fbcon that
works as well as it does right now for almost all users (which is not
so great, but oh well).

> There may have been some other bad interaction with
> foreground/background consoles too, I forget.

Irrespective of this code being buggy or not buggy I think the bigger
pictures, and really the reason I want to see as much code ditched
from the fbdev/fbcon stack as we possible can, are very clear:

- it's full of bugs
- there's no test coverage/CI to speak of
- it's very arcane code which is damn hard to understand and fix issues within
- the locking is busted (largely thanks to console_lock, and the
effort to make that reasonable from -rt folks has been slowly creeping
forward for years).

Iow this subsystem is firmly stuck in the 90s, and I think it's best
to just leave it there. There's also not been anyone actually capable
and willing to put in the work to change this (pretty much all actual
changes/fixes have been done by drm folks anyway, like me having a
small pet project to make the fbdev vs fbcon locking slightly less
busted).

The other side is that being a maintainer is about collaboration, and
this entire fbdev maintainership takeover has been a demonstration of
anything but that. MAINTAINERS entry was a bit confusing since defacto
drm has been maintaining it for years, but for the above reasons we've
done that by just aggressively deleting stuff that isn't absolutely
needed - hence why I figured "orphaned" is a reasonable description of
the state of things. This entire affair of rushing in a maintainer
change over the w/e and then being greeted by a lot of wtf mails next
Monday does leave a rather sour aftertaste. Plus that thread shows a
lot of misunderstandings of what's all been going on and what drm can
and cannot do by Helge, which doesn't improve the entire "we need
fbdev back" argument.

But if the overall consensus is that that fbdev needs to be brought
back to it's full 90s glory then I think we need a copy of that code
for drm drivers (should work out if we intercept fb_open() and put our
own file_ops in there, maybe some more fun with fbcon), so that at
least for anything modern using drm driver we can keep on maintaining
that compat support code.

And with maintaining here I don't mean build a museum around it, but
actually try to keep/move the thing towards a state where we can still
tell distros that enabling it is an ok thing to do and not just a CVE
subscription (well it is that too right now, but at least we can fix a
lot of them by just deleting code).

I think until that mess is sorted out resurrecting code that's not
strictly needed is just not a bright idea.

Also wrt the issue at hand of "fbcon scrolling": The way to actually
do that with some speed is to render into a fully cached shadow buffer
and upload changed areas with a timer. Not with hw accelerated
scrolling, at least not if we just don't have full scale development
teams for each driver because creating 2d accel that doesn't suck is
really hard. drm fbdev compat helpers give you that shadow buffer for
free (well you got to set some options).

Unfortunately just ditching fbdev/fbcon compat is not an option for
many distros still, althought things are very slowly moving towards
that. Until we've arrived there I can't just pretend to not care about
what's going on in drivers/video.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2022-01-21 19:18:50

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On 1/19/22 15:01, Linus Torvalds wrote:
> On Wed, Jan 19, 2022 at 2:29 PM Helge Deller <[email protected]> wrote:
>>
>>>>
>>>> Ah, no, that was just the soft scrollback code I was thinking of, which
>>
>> Right.
>> That was commit 973c096f6a85 and it was about vgacon, not fbcon.
>
> No, fbcon had some bug too, although I've paged out the details. See
> commit 50145474f6ef ("fbcon: remove soft scrollback code").

Yes, but those bugs didn't affected

> If I remember correctly (and it's entirely possible that I don't), the
> whole "softback_lines" logic had serious problems with resizing the
> console (or maybe changing the font size).

Right.
I'm not asking to revert any soft scrollback functions.

It's about if you allow to use the fbdev-drivers hardware acceleration
to move parts of the screen or if you are stuck to software memcpy() and
repainting instead.
None of the bug reports touched that part.

> There may have been some other bad interaction with
> foreground/background consoles too, I forget.

I think that is independend if you use hardware acceleration or not.

Helge

2022-01-21 19:25:29

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On 1/19/22 15:34, Daniel Vetter wrote:
> On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds
> <[email protected]> wrote:
>>
>> On Wed, Jan 19, 2022 at 2:29 PM Helge Deller <[email protected]> wrote:
>>>
>>>>>
>>>>> Ah, no, that was just the soft scrollback code I was thinking of, which
>>>
>>> Right.
>>> That was commit 973c096f6a85 and it was about vgacon, not fbcon.
>>
>> No, fbcon had some bug too, although I've paged out the details. See
>> commit 50145474f6ef ("fbcon: remove soft scrollback code").
>
> tbh I've paged it all out too.
>
>> If I remember correctly (and it's entirely possible that I don't), the
>> whole "softback_lines" logic had serious problems with resizing the
>> console (or maybe changing the font size).
>
> Yeah that pile of reverts was my motiviation to look into this and see
> what else we could rip out most likely and still have an fbcon that
> works as well as it does right now for almost all users (which is not
> so great, but oh well).
>
>> There may have been some other bad interaction with
>> foreground/background consoles too, I forget.
>
> Irrespective of this code being buggy or not buggy I think the bigger
> pictures, and really the reason I want to see as much code ditched
> from the fbdev/fbcon stack as we possible can, are very clear:
>
> - it's full of bugs

I'm sure that there are bugs. Not just in fbcon/fbdev.
Other than that, if there are bugs I'm sure they are independend
of the question if you use hardware acceleration or not.

> - there's no test coverage/CI to speak of
> - it's very arcane code which is damn hard to understand and fix issues within
> - the locking is busted (largely thanks to console_lock, and the
> effort to make that reasonable from -rt folks has been slowly creeping
> forward for years).
>
> Iow this subsystem is firmly stuck in the 90s, and I think it's best
> to just leave it there. There's also not been anyone actually capable
> and willing to put in the work to change this (pretty much all actual
> changes/fixes have been done by drm folks anyway, like me having a
> small pet project to make the fbdev vs fbcon locking slightly less
> busted).

Yes, drm folks fixed a lot of bugs in the generic fbcon code.
I think everyone is thankful for this.

> The other side is that being a maintainer is about collaboration, and
> this entire fbdev maintainership takeover has been a demonstration of
> anything but that. MAINTAINERS entry was a bit confusing since defacto
> drm has been maintaining it for years, but for the above reasons we've
> done that by just aggressively deleting stuff that isn't absolutely
> needed - hence why I figured "orphaned" is a reasonable description of
> the state of things. This entire affair of rushing in a maintainer
> change over the w/e and then being greeted by a lot of wtf mails next
> Monday does leave a rather sour aftertaste. Plus that thread shows a
> lot of misunderstandings of what's all been going on and what drm can
> and cannot do by Helge, which doesn't improve the entire "we need
> fbdev back" argument.

I'm happy to *really* maintain fbdev code & drivers.
Up to now only those parts which were still needed by drm (like fbcon)
were fixed & "maintained" by drm folks.
Nearly all other patches sent to the fbdev list were ignored and even new
submissions for fbdev drivers were denied.
Now in next step really important infrastructure for fbdev-drivers was
ripped out of fbcon, like suddenly denying fbdev-drivers to use hardware
acceleration.
According to the docs the next step would have been to drop even more
code from the fbdev drivers.
This is not what "maintain" really is about.

> But if the overall consensus is that that fbdev needs to be brought
> back to it's full 90s glory then I think we need a copy of that code
> for drm drivers (should work out if we intercept fb_open() and put our
> own file_ops in there, maybe some more fun with fbcon), so that at
> least for anything modern using drm driver we can keep on maintaining
> that compat support code.

It's not about to keep something alive or to stop future developments.
It's about fairness and not actively breaking other parts of the kernel
for no good reason.

> And with maintaining here I don't mean build a museum around it, but
> actually try to keep/move the thing towards a state where we can still
> tell distros that enabling it is an ok thing to do and not just a CVE
> subscription (well it is that too right now, but at least we can fix a
> lot of them by just deleting code).
>
> I think until that mess is sorted out resurrecting code that's not
> strictly needed is just not a bright idea.

That's wrong.
It's strictly needed by more than 35 fbdev drivers and as such
you introduced a regression for those.

> Also wrt the issue at hand of "fbcon scrolling": The way to actually
> do that with some speed is to render into a fully cached shadow buffer
> and upload changed areas with a timer. Not with hw accelerated
> scrolling, at least not if we just don't have full scale development
> teams for each driver because creating 2d accel that doesn't suck is
> really hard. drm fbdev compat helpers give you that shadow buffer for
> free (well you got to set some options).
>
> Unfortunately just ditching fbdev/fbcon compat is not an option for
> many distros still, althought things are very slowly moving towards
> that. Until we've arrived there I can't just pretend to not care about
> what's going on in drivers/video.

I'm happy to take care about it.
That's why I stepped up as maintainer.

Helge

2022-01-21 19:27:40

by Sven Schnelle

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Daniel Vetter <[email protected]> writes:

> On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds
> <[email protected]> wrote:
> Irrespective of this code being buggy or not buggy I think the bigger
> pictures, and really the reason I want to see as much code ditched
> from the fbdev/fbcon stack as we possible can, are very clear:
>
> - it's full of bugs
> - there's no test coverage/CI to speak of
> - it's very arcane code which is damn hard to understand and fix issues within
> - the locking is busted (largely thanks to console_lock, and the
> effort to make that reasonable from -rt folks has been slowly creeping
> forward for years).
>
> Iow this subsystem is firmly stuck in the 90s, and I think it's best
> to just leave it there. There's also not been anyone actually capable
> and willing to put in the work to change this (pretty much all actual
> changes/fixes have been done by drm folks anyway, like me having a
> small pet project to make the fbdev vs fbcon locking slightly less
> busted).

Saying it's stuck in the 90ies, and actively trying to prevent
Helge from taking over maintainership at the same time looks odd.
I think Helge should at least get a chance to fix the issues. If the
state is still the same in a year or so it should be discussed again.

> The other side is that being a maintainer is about collaboration, and
> this entire fbdev maintainership takeover has been a demonstration of
> anything but that. MAINTAINERS entry was a bit confusing since defacto
> drm has been maintaining it for years.

It was marked as 'Orphaned'. Anyone is free to send a Patch/PR to take
over maintainership. If you have strong opinions about that code (And you
obviously have reading your mail, set it to 'maintained' and care about
it. Everything else is just wrong in my opinion.

/Sven

2022-01-21 19:34:13

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hi

Am 19.01.22 um 16:05 schrieb Sven Schnelle:
> Daniel Vetter <[email protected]> writes:
>
>> On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds
>> <[email protected]> wrote:
>> Irrespective of this code being buggy or not buggy I think the bigger
>> pictures, and really the reason I want to see as much code ditched
>> from the fbdev/fbcon stack as we possible can, are very clear:
>>
>> - it's full of bugs
>> - there's no test coverage/CI to speak of
>> - it's very arcane code which is damn hard to understand and fix issues within
>> - the locking is busted (largely thanks to console_lock, and the
>> effort to make that reasonable from -rt folks has been slowly creeping
>> forward for years).
>>
>> Iow this subsystem is firmly stuck in the 90s, and I think it's best
>> to just leave it there. There's also not been anyone actually capable
>> and willing to put in the work to change this (pretty much all actual
>> changes/fixes have been done by drm folks anyway, like me having a
>> small pet project to make the fbdev vs fbcon locking slightly less
>> busted).
>
> Saying it's stuck in the 90ies, and actively trying to prevent
> Helge from taking over maintainership at the same time looks odd.

The issues are in the design itself. It's impossible to model today's
hardware and constraints with fbdev. It's impossible to change
configuration in a reliable way (i.e., what DRM calls atomic). Fbdev
mmaps plain video ram to userspace, which is one of the reasons why
DRM's fbdev support is hard to improve.

> I think Helge should at least get a chance to fix the issues. If the
> state is still the same in a year or so it should be discussed again.

You cannot fix that in 10yrs.

>
>> The other side is that being a maintainer is about collaboration, and
>> this entire fbdev maintainership takeover has been a demonstration of
>> anything but that. MAINTAINERS entry was a bit confusing since defacto
>> drm has been maintaining it for years.
>
> It was marked as 'Orphaned'. Anyone is free to send a Patch/PR to take
> over maintainership. If you have strong opinions about that code (And you
> obviously have reading your mail, set it to 'maintained' and care about
> it. Everything else is just wrong in my opinion.

No, it's not wrong. Helge takes fbdev over the weekend, without
noteworthy experience, and ignores advice from the people that have kept
it alive over the past years. This isn't going to work in the long term.

Best regards
Thomas

>
> /Sven

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachments:

OpenPGP_signature (855.00 B)
OpenPGP digital signature

2022-01-21 19:39:53

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Wed, Jan 19, 2022 at 4:06 PM Sven Schnelle <[email protected]> wrote:
>
> Daniel Vetter <[email protected]> writes:
>
> > On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds
> > <[email protected]> wrote:
> > Irrespective of this code being buggy or not buggy I think the bigger
> > pictures, and really the reason I want to see as much code ditched
> > from the fbdev/fbcon stack as we possible can, are very clear:
> >
> > - it's full of bugs
> > - there's no test coverage/CI to speak of
> > - it's very arcane code which is damn hard to understand and fix issues within
> > - the locking is busted (largely thanks to console_lock, and the
> > effort to make that reasonable from -rt folks has been slowly creeping
> > forward for years).
> >
> > Iow this subsystem is firmly stuck in the 90s, and I think it's best
> > to just leave it there. There's also not been anyone actually capable
> > and willing to put in the work to change this (pretty much all actual
> > changes/fixes have been done by drm folks anyway, like me having a
> > small pet project to make the fbdev vs fbcon locking slightly less
> > busted).
>
> Saying it's stuck in the 90ies, and actively trying to prevent
> Helge from taking over maintainership at the same time looks odd.
> I think Helge should at least get a chance to fix the issues. If the
> state is still the same in a year or so it should be discussed again.

You don't need maintainership to fix issues. You need to submit patches.

If otoh you get the maintainership first to be able to cram in reverts
without discussions, then it's very backwards.

> > The other side is that being a maintainer is about collaboration, and
> > this entire fbdev maintainership takeover has been a demonstration of
> > anything but that. MAINTAINERS entry was a bit confusing since defacto
> > drm has been maintaining it for years.
>
> It was marked as 'Orphaned'. Anyone is free to send a Patch/PR to take
> over maintainership. If you have strong opinions about that code (And you
> obviously have reading your mail, set it to 'maintained' and care about
> it. Everything else is just wrong in my opinion.

I already added dri-devel so anything we drastically change can be
discussed first. If that's indeed not strong enough then yes I can
whack in full maintainer entry with a bugfix-only status.

But really I try to not create facts with just editing MAINTAINERS
first and ask questions later, that's just not a great way to
collaborate.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2022-01-21 19:42:23

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On 1/19/22 16:42, Daniel Vetter wrote:
> On Wed, Jan 19, 2022 at 4:06 PM Sven Schnelle <[email protected]> wrote:
>>
>> Daniel Vetter <[email protected]> writes:
>>
>>> On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds
>>> <[email protected]> wrote:
>>> Irrespective of this code being buggy or not buggy I think the bigger
>>> pictures, and really the reason I want to see as much code ditched
>>> from the fbdev/fbcon stack as we possible can, are very clear:
>>>
>>> - it's full of bugs
>>> - there's no test coverage/CI to speak of
>>> - it's very arcane code which is damn hard to understand and fix issues within
>>> - the locking is busted (largely thanks to console_lock, and the
>>> effort to make that reasonable from -rt folks has been slowly creeping
>>> forward for years).
>>>
>>> Iow this subsystem is firmly stuck in the 90s, and I think it's best
>>> to just leave it there. There's also not been anyone actually capable
>>> and willing to put in the work to change this (pretty much all actual
>>> changes/fixes have been done by drm folks anyway, like me having a
>>> small pet project to make the fbdev vs fbcon locking slightly less
>>> busted).
>>
>> Saying it's stuck in the 90ies, and actively trying to prevent
>> Helge from taking over maintainership at the same time looks odd.
>> I think Helge should at least get a chance to fix the issues. If the
>> state is still the same in a year or so it should be discussed again.
>
> You don't need maintainership to fix issues. You need to submit patches.

The very first email of this thread is my patch.
And you just added your comments to this patch.

> If otoh you get the maintainership first to be able to cram in reverts
> without discussions, then it's very backwards.
I'm working on the Linux kernel since at least 23 years and am a maintainer of parts of it.
I know that and would never push something which is controversal without discussions.

>>> The other side is that being a maintainer is about collaboration, and
>>> this entire fbdev maintainership takeover has been a demonstration of
>>> anything but that. MAINTAINERS entry was a bit confusing since defacto
>>> drm has been maintaining it for years.
>>
>> It was marked as 'Orphaned'. Anyone is free to send a Patch/PR to take
>> over maintainership. If you have strong opinions about that code (And you
>> obviously have reading your mail, set it to 'maintained' and care about
>> it. Everything else is just wrong in my opinion.
>
> I already added dri-devel so anything we drastically change can be
> discussed first. If that's indeed not strong enough then yes I can
> whack in full maintainer entry with a bugfix-only status.
>
> But really I try to not create facts with just editing MAINTAINERS
> first and ask questions later, that's just not a great way to
> collaborate.

Helge

2022-01-21 19:46:59

by Jani Nikula

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Wed, 19 Jan 2022, Helge Deller <[email protected]> wrote:
> On 1/19/22 16:42, Daniel Vetter wrote:
>> If otoh you get the maintainership first to be able to cram in reverts
>> without discussions, then it's very backwards.
> I'm working on the Linux kernel since at least 23 years and am a maintainer of parts of it.
> I know that and would never push something which is controversal without discussions.

I think the entire MAINTAINERS change was controversial and rushed to
Linus without discussion over a weekend.

BR,
Jani.

--
Jani Nikula, Intel Open Source Graphics Center

2022-01-21 19:50:52

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hi Thomas,

On 1/19/22 16:37, Thomas Zimmermann wrote:
> Am 19.01.22 um 16:05 schrieb Sven Schnelle:
>> Daniel Vetter <[email protected]> writes:
>>
>>> On Wed, Jan 19, 2022 at 3:01 PM Linus Torvalds
>>> <[email protected]> wrote:
>>> Irrespective of this code being buggy or not buggy I think the bigger
>>> pictures, and really the reason I want to see as much code ditched
>>> from the fbdev/fbcon stack as we possible can, are very clear:
>>>
>>> - it's full of bugs
>>> - there's no test coverage/CI to speak of
>>> - it's very arcane code which is damn hard to understand and fix issues within
>>> - the locking is busted (largely thanks to console_lock, and the
>>> effort to make that reasonable from -rt folks has been slowly creeping
>>> forward for years).
>>>
>>> Iow this subsystem is firmly stuck in the 90s, and I think it's best
>>> to just leave it there. There's also not been anyone actually capable
>>> and willing to put in the work to change this (pretty much all actual
>>> changes/fixes have been done by drm folks anyway, like me having a
>>> small pet project to make the fbdev vs fbcon locking slightly less
>>> busted).
>>
>> Saying it's stuck in the 90ies, and actively trying to prevent
>> Helge from taking over maintainership at the same time looks odd.
>
> The issues are in the design itself. It's impossible to model today's
> hardware and constraints with fbdev. It's impossible to change
> configuration in a reliable way (i.e., what DRM calls atomic). Fbdev
> mmaps plain video ram to userspace, which is one of the reasons why
> DRM's fbdev support is hard to improve.

That's fully understood, but I think you are mixing up things here...

The fbdev userspace api is most likely not the best way forward.
I'm sure that drm can and will provide better solutions for userspace.
And userspace will surely pick up those new interfaces.
DRM folks will drive it in the right direction, I'm sure!

But in addition fbdev/fbcon is the kernel framework for nearly
all existing graphic cards which are not (yet) supported by DRM.
They need fbdev/fbcon to show their text console and maybe a simple X server.
If you break fbdev for those cards, they are completely stuck.
Hopefully those drivers will be ported to DRM, but that's currently
not easily possible (or they would be so slow that they are unuseable).

So, I don't think you should try to improve DRM's /dev/fb0 support further,
but instead work forward for a new interface which perfectly suits DRM.
That's Ok, and my goal is not to prevent that.

>> I think Helge should at least get a chance to fix the issues. If the
>> state is still the same in a year or so it should be discussed again.
>
> You cannot fix that in 10yrs.
>
>>
>>> The other side is that being a maintainer is about collaboration, and
>>> this entire fbdev maintainership takeover has been a demonstration of
>>> anything but that. MAINTAINERS entry was a bit confusing since defacto
>>> drm has been maintaining it for years.
>>
>> It was marked as 'Orphaned'. Anyone is free to send a Patch/PR to take
>> over maintainership. If you have strong opinions about that code (And you
>> obviously have reading your mail, set it to 'maintained' and care about
>> it. Everything else is just wrong in my opinion.
>
> No, it's not wrong. Helge takes fbdev over the weekend, without noteworthy experience, and ignores advice from the people that have kept it alive over the past years. This isn't going to work in the long term.
>
> Best regards
> Thomas
>
>>
>> /Sven
>

2022-01-21 19:50:56

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Wed, Jan 19, 2022 at 06:13:39PM +0200, Jani Nikula wrote:
> On Wed, 19 Jan 2022, Helge Deller <[email protected]> wrote:
> > On 1/19/22 16:42, Daniel Vetter wrote:
> >> If otoh you get the maintainership first to be able to cram in reverts
> >> without discussions, then it's very backwards.
> > I'm working on the Linux kernel since at least 23 years and am a maintainer of parts of it.
> > I know that and would never push something which is controversal without discussions.
>
> I think the entire MAINTAINERS change was controversial and rushed to
> Linus without discussion over a weekend.

Yeah just looking at the size of the thread is pretty clear indiciation
that this went wrong real good.

And I said that you need to clean this up in my very first reply. None of
this should have been a surprise, but somehow it all happened.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2022-01-21 22:16:22

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote:
> This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
>
> Revert this patch. This patch started to introduce the regression that
> all hardware acceleration of more than 35 existing fbdev drivers were
> bypassed and thus fbcon console output for those was dramatically slowed
> down by factor of 10 and more.
>
> Reverting this commit has no impact on DRM, since none of the DRM drivers are
> tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
> FBINFO_HWACCEL_FILLRECT or others.
>
> Signed-off-by: Helge Deller <[email protected]>
> Cc: [email protected] # v5.16

So if this really has to come back then I think the pragmatic approach is
to do it behind a CONFIG_FBCON_ACCEL, default n, and with a huge warning
that enabling that shouldn't be done for any distro which only enables
firmware and drm fbdev drivers.

Plus adjusting the todo to limit it to drm drivers. Maybe also #ifdef out
the code that's then dead from fbcon.

Also in that case I guess it's ok to cc: stable, and really if you cc:
stable it needs to go down to 5.11, not 5.16.

And if we do that, I think that should go in through a -next cycle, or at
least quite some soaking before it's cherry-picked over. Enough to give
syzbot a chance to discover any path we've missed at least.
-Daniel

> ---
> Documentation/gpu/todo.rst | 21 ---------------
> drivers/video/fbdev/core/fbcon.c | 45 ++++++++++++++++++++++++++------
> 2 files changed, 37 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 29506815d24a..a1212b5b3026 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -300,27 +300,6 @@ Contact: Daniel Vetter, Noralf Tronnes
>
> Level: Advanced
>
> -Garbage collect fbdev scrolling acceleration
> ---------------------------------------------
> -
> -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:
> -
> -- 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
> -
> -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.
> -
> -Contact: Daniel Vetter
> -
> -Level: Intermediate
> -
> idr_init_base()
> ---------------
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 22bb3892f6bd..b813985f1403 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -1025,7 +1025,7 @@ static void fbcon_init(struct vc_data *vc, int init)
> struct vc_data *svc = *default_mode;
> struct fbcon_display *t, *p = &fb_display[vc->vc_num];
> int logo = 1, new_rows, new_cols, rows, cols;
> - int ret;
> + int cap, ret;
>
> if (WARN_ON(info_idx == -1))
> return;
> @@ -1034,6 +1034,7 @@ static void fbcon_init(struct vc_data *vc, int init)
> con2fb_map[vc->vc_num] = info_idx;
>
> info = registered_fb[con2fb_map[vc->vc_num]];
> + cap = info->flags;
>
> if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
> logo_shown = FBCON_LOGO_DONTSHOW;
> @@ -1135,13 +1136,11 @@ static void fbcon_init(struct vc_data *vc, int init)
>
> ops->graphics = 0;
>
> - /*
> - * No more hw acceleration for fbcon.
> - *
> - * FIXME: Garbage collect all the now dead code after sufficient time
> - * has passed.
> - */
> - p->scrollmode = SCROLL_REDRAW;
> + if ((cap & FBINFO_HWACCEL_COPYAREA) &&
> + !(cap & FBINFO_HWACCEL_DISABLED))
> + p->scrollmode = SCROLL_MOVE;
> + else /* default to something safe */
> + p->scrollmode = SCROLL_REDRAW;
>
> /*
> * ++guenther: console.c:vc_allocate() relies on initializing
> @@ -1953,15 +1952,45 @@ static void updatescrollmode(struct fbcon_display *p,
> {
> struct fbcon_ops *ops = info->fbcon_par;
> int fh = vc->vc_font.height;
> + int cap = info->flags;
> + u16 t = 0;
> + int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep,
> + info->fix.xpanstep);
> + int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t);
> int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
> int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual,
> info->var.xres_virtual);
> + int good_pan = (cap & FBINFO_HWACCEL_YPAN) &&
> + divides(ypan, vc->vc_font.height) && vyres > yres;
> + int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) &&
> + divides(ywrap, vc->vc_font.height) &&
> + divides(vc->vc_font.height, vyres) &&
> + divides(vc->vc_font.height, yres);
> + int reading_fast = cap & FBINFO_READS_FAST;
> + int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) &&
> + !(cap & FBINFO_HWACCEL_DISABLED);
> + int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) &&
> + !(cap & FBINFO_HWACCEL_DISABLED);
>
> p->vrows = vyres/fh;
> if (yres > (fh * (vc->vc_rows + 1)))
> p->vrows -= (yres - (fh * vc->vc_rows)) / fh;
> if ((yres % fh) && (vyres % fh < yres % fh))
> p->vrows--;
> +
> + if (good_wrap || good_pan) {
> + if (reading_fast || fast_copyarea)
> + p->scrollmode = good_wrap ?
> + SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE;
> + else
> + p->scrollmode = good_wrap ? SCROLL_REDRAW :
> + SCROLL_PAN_REDRAW;
> + } else {
> + if (reading_fast || (fast_copyarea && !fast_imageblit))
> + p->scrollmode = SCROLL_MOVE;
> + else
> + p->scrollmode = SCROLL_REDRAW;
> + }
> }
>
> #define PITCH(w) (((w) + 7) >> 3)
> --
> 2.31.1
>

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2022-01-21 22:27:21

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hello Daniel,

On 1/20/22 15:30, Daniel Vetter wrote:
> On Wed, Jan 19, 2022 at 12:08:39PM +0100, Helge Deller wrote:
>> This reverts commit 39aead8373b3c20bb5965c024dfb51a94e526151.
>>
>> Revert this patch. This patch started to introduce the regression that
>> all hardware acceleration of more than 35 existing fbdev drivers were
>> bypassed and thus fbcon console output for those was dramatically slowed
>> down by factor of 10 and more.
>>
>> Reverting this commit has no impact on DRM, since none of the DRM drivers are
>> tagged with the acceleration flags FBINFO_HWACCEL_COPYAREA,
>> FBINFO_HWACCEL_FILLRECT or others.
>>
>> Signed-off-by: Helge Deller <[email protected]>
>> Cc: [email protected] # v5.16
>
> So if this really has to come back then I think the pragmatic approach is
> to do it behind a CONFIG_FBCON_ACCEL, default n, and with a huge warning
> that enabling that shouldn't be done for any distro which only enables
> firmware and drm fbdev drivers.

Thanks for coming back on this, but quite frankly I don't understand
that request. How should that warning look like, something along:
"BE WARNED: The framebuffer text console on your non-DRM supported
graphic card will then run faster and smoother if you enable this option."
That doesn't make sense. People and distros would want to enable that.

And if a distro *just* has firmware and drm fbdev drivers enabled,
none of the non-DRM graphic cards would be loaded anyway and this code
wouldn't be executed anyway.

I think what you want is that DRM drivers are preferred over standard
fbdev drivers, esp. if there is a driver for both available.
But that's completely independed of fbdev-drivers console hardware acceleration.

> Plus adjusting the todo to limit it to drm drivers. Maybe also #ifdef out
> the code that's then dead from fbcon.

Sorry, I don't understand that either.
I assume you mean to put code of fbcon which is only used by fbdev-drivers
into and #ifdef CONFIG_FB .. #endif (CONFIG_FB may be wrong in this example).
That's probably possible, but I don't see a big win.
If there is no fbdev driver compiled-in or as module, none of this fbdev-drivers
will be loaded and that code path wouldn't be executed anyway.
In that case you will win a few bytes of code, but probably not much.

> Also in that case I guess it's ok to cc: stable, and really if you cc:
> stable it needs to go down to 5.11, not 5.16.

Yes, I missed that in my patch request. Will fix.

> And if we do that, I think that should go in through a -next cycle, or at
> least quite some soaking before it's cherry-picked over. Enough to give
> syzbot a chance to discover any path we've missed at least.

Sure. We don't need to hurry.

Thanks!
Helge

> -Daniel
>
>> ---
>> Documentation/gpu/todo.rst | 21 ---------------
>> drivers/video/fbdev/core/fbcon.c | 45 ++++++++++++++++++++++++++------
>> 2 files changed, 37 insertions(+), 29 deletions(-)
>>
>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 29506815d24a..a1212b5b3026 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -300,27 +300,6 @@ Contact: Daniel Vetter, Noralf Tronnes
>>
>> Level: Advanced
>>
>> -Garbage collect fbdev scrolling acceleration
>> ---------------------------------------------
>> -
>> -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:
>> -
>> -- 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
>> -
>> -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.
>> -
>> -Contact: Daniel Vetter
>> -
>> -Level: Intermediate
>> -
>> idr_init_base()
>> ---------------
>>
>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
>> index 22bb3892f6bd..b813985f1403 100644
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>> @@ -1025,7 +1025,7 @@ static void fbcon_init(struct vc_data *vc, int init)
>> struct vc_data *svc = *default_mode;
>> struct fbcon_display *t, *p = &fb_display[vc->vc_num];
>> int logo = 1, new_rows, new_cols, rows, cols;
>> - int ret;
>> + int cap, ret;
>>
>> if (WARN_ON(info_idx == -1))
>> return;
>> @@ -1034,6 +1034,7 @@ static void fbcon_init(struct vc_data *vc, int init)
>> con2fb_map[vc->vc_num] = info_idx;
>>
>> info = registered_fb[con2fb_map[vc->vc_num]];
>> + cap = info->flags;
>>
>> if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
>> logo_shown = FBCON_LOGO_DONTSHOW;
>> @@ -1135,13 +1136,11 @@ static void fbcon_init(struct vc_data *vc, int init)
>>
>> ops->graphics = 0;
>>
>> - /*
>> - * No more hw acceleration for fbcon.
>> - *
>> - * FIXME: Garbage collect all the now dead code after sufficient time
>> - * has passed.
>> - */
>> - p->scrollmode = SCROLL_REDRAW;
>> + if ((cap & FBINFO_HWACCEL_COPYAREA) &&
>> + !(cap & FBINFO_HWACCEL_DISABLED))
>> + p->scrollmode = SCROLL_MOVE;
>> + else /* default to something safe */
>> + p->scrollmode = SCROLL_REDRAW;
>>
>> /*
>> * ++guenther: console.c:vc_allocate() relies on initializing
>> @@ -1953,15 +1952,45 @@ static void updatescrollmode(struct fbcon_display *p,
>> {
>> struct fbcon_ops *ops = info->fbcon_par;
>> int fh = vc->vc_font.height;
>> + int cap = info->flags;
>> + u16 t = 0;
>> + int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep,
>> + info->fix.xpanstep);
>> + int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t);
>> int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
>> int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual,
>> info->var.xres_virtual);
>> + int good_pan = (cap & FBINFO_HWACCEL_YPAN) &&
>> + divides(ypan, vc->vc_font.height) && vyres > yres;
>> + int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) &&
>> + divides(ywrap, vc->vc_font.height) &&
>> + divides(vc->vc_font.height, vyres) &&
>> + divides(vc->vc_font.height, yres);
>> + int reading_fast = cap & FBINFO_READS_FAST;
>> + int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) &&
>> + !(cap & FBINFO_HWACCEL_DISABLED);
>> + int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) &&
>> + !(cap & FBINFO_HWACCEL_DISABLED);
>>
>> p->vrows = vyres/fh;
>> if (yres > (fh * (vc->vc_rows + 1)))
>> p->vrows -= (yres - (fh * vc->vc_rows)) / fh;
>> if ((yres % fh) && (vyres % fh < yres % fh))
>> p->vrows--;
>> +
>> + if (good_wrap || good_pan) {
>> + if (reading_fast || fast_copyarea)
>> + p->scrollmode = good_wrap ?
>> + SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE;
>> + else
>> + p->scrollmode = good_wrap ? SCROLL_REDRAW :
>> + SCROLL_PAN_REDRAW;
>> + } else {
>> + if (reading_fast || (fast_copyarea && !fast_imageblit))
>> + p->scrollmode = SCROLL_MOVE;
>> + else
>> + p->scrollmode = SCROLL_REDRAW;
>> + }
>> }
>>
>> #define PITCH(w) (((w) + 7) >> 3)
>> --
>> 2.31.1
>>
>

2022-01-22 00:39:05

by Gerd Hoffmann

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hi,

> > So if this really has to come back then I think the pragmatic approach is
> > to do it behind a CONFIG_FBCON_ACCEL, default n, and with a huge warning
> > that enabling that shouldn't be done for any distro which only enables
> > firmware and drm fbdev drivers.
>
> Thanks for coming back on this, but quite frankly I don't understand
> that request. How should that warning look like, something along:
> "BE WARNED: The framebuffer text console on your non-DRM supported
> graphic card will then run faster and smoother if you enable this option."
> That doesn't make sense. People and distros would want to enable that.

Nope. Most distros want disable fbdev drivers rather sooner than later.
The fbdev drivers enabled in the fedora kernel today:

CONFIG_FB_VGA16=m
CONFIG_FB_VESA=y
CONFIG_FB_EFI=y
CONFIG_FB_SSD1307=m

CONFIG_FB_VESA + CONFIG_FB_EFI will go away soon, with simpledrm taking
over their role.

> And if a distro *just* has firmware and drm fbdev drivers enabled,
> none of the non-DRM graphic cards would be loaded anyway and this code
> wouldn't be executed anyway.

Yes, exactly. That's why there is no point in compiling that code.

take care,
Gerd

2022-01-24 19:05:54

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On 1/21/22 08:20, Gerd Hoffmann wrote:
>>> So if this really has to come back then I think the pragmatic approach is
>>> to do it behind a CONFIG_FBCON_ACCEL, default n, and with a huge warning
>>> that enabling that shouldn't be done for any distro which only enables
>>> firmware and drm fbdev drivers.
>>
>> Thanks for coming back on this, but quite frankly I don't understand
>> that request. How should that warning look like, something along:
>> "BE WARNED: The framebuffer text console on your non-DRM supported
>> graphic card will then run faster and smoother if you enable this option."
>> That doesn't make sense. People and distros would want to enable that.
>
> Nope. Most distros want disable fbdev drivers rather sooner than later.
> The fbdev drivers enabled in the fedora kernel today:
>
> CONFIG_FB_VGA16=m
> CONFIG_FB_VESA=y
> CONFIG_FB_EFI=y
> CONFIG_FB_SSD1307=m
>
> CONFIG_FB_VESA + CONFIG_FB_EFI will go away soon, with simpledrm taking
> over their role.

That's Ok.
Nevertheless, some distros and platforms will still need fbdev drivers for
various reasons.

>> And if a distro *just* has firmware and drm fbdev drivers enabled,
>> none of the non-DRM graphic cards would be loaded anyway and this code
>> wouldn't be executed anyway.
>
> Yes, exactly. That's why there is no point in compiling that code.

As long as you have a graphic card which is not supported by DRM you still need it.

Here is my proposed way forward:
a) I will resend the patches which reverts the remove-fbcon-hardware-scolling patches
to the mailing lists. I'll adjust the stable tags and update the commit messages.
b) Then after some days I'll include it in the fbdev for-next git branch. That way it's
included in the various build & test chains.
c) If everything is working well, I'll push that change during the next merge window
for kernel 5.18. If problems arise we will need to discuss.

While the patches are in the fbdev git tree we should decide how to exclude code
which is not needed for DRM.

What about this proposal:
a) adding a Kconfig option like:
CONFIG_FB_DRIVERS - enable if you need the fbdev drivers. For DRM-only this should be disabled.
b) Add to every native fbdev driver a "depends on FB_DRIVERS" in the Kconfig files.
c) That way we can use "#if defined(CONFIG_FB_DRIVERS).." to exclude code in fbcon which
isn't needed by DRM.

Thoughts?

Helge

2022-01-24 19:06:43

by Javier Martinez Canillas

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

[snip]

>
> What about this proposal:
> a) adding a Kconfig option like:
> CONFIG_FB_DRIVERS - enable if you need the fbdev drivers. For DRM-only this should be disabled.
> b) Add to every native fbdev driver a "depends on FB_DRIVERS" in the Kconfig files.
> c) That way we can use "#if defined(CONFIG_FB_DRIVERS).." to exclude code in fbcon which
> isn't needed by DRM.
>

I proposed something similar in:

https://lore.kernel.org/lkml/[email protected]/t/

Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat

2022-01-24 19:06:47

by Javier Martinez Canillas

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On 1/24/22 12:33, Thomas Zimmermann wrote:

[snip]

>> Thoughts?
>
> I can't say I approve keeping fbdev alive, but...
>
> With fbdev emulation, every DRM driver is an fbdev driver too. So
> CONFIG_FB_DRIVER is somewhat misleading. Better add an option like
> CONFIG_FBCON_HW_SCROLLING and have it selected by the fbdev drivers that
> absolutely need HW acceleration. That option would then protect the rsp
> code.
>

Agreed that this option would be better and allow distros
to disable the code that was reverted.

Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat

2022-01-24 19:06:50

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hi

Am 24.01.22 um 12:10 schrieb Helge Deller:
[...]
>
> Here is my proposed way forward:
> a) I will resend the patches which reverts the remove-fbcon-hardware-scolling patches
> to the mailing lists. I'll adjust the stable tags and update the commit messages.
> b) Then after some days I'll include it in the fbdev for-next git branch. That way it's
> included in the various build & test chains.
> c) If everything is working well, I'll push that change during the next merge window
> for kernel 5.18. If problems arise we will need to discuss.
>
> While the patches are in the fbdev git tree we should decide how to exclude code
> which is not needed for DRM.
>
> What about this proposal:
> a) adding a Kconfig option like:
> CONFIG_FB_DRIVERS - enable if you need the fbdev drivers. For DRM-only this should be disabled.
> b) Add to every native fbdev driver a "depends on FB_DRIVERS" in the Kconfig files.
> c) That way we can use "#if defined(CONFIG_FB_DRIVERS).." to exclude code in fbcon which
> isn't needed by DRM.
>
> Thoughts?

I can't say I approve keeping fbdev alive, but...

With fbdev emulation, every DRM driver is an fbdev driver too. So
CONFIG_FB_DRIVER is somewhat misleading. Better add an option like
CONFIG_FBCON_HW_SCROLLING and have it selected by the fbdev drivers that
absolutely need HW acceleration. That option would then protect the rsp
code.

Best regards
Thomas

>
> Helge

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachments:

OpenPGP_signature (855.00 B)
OpenPGP digital signature

2022-01-24 19:27:49

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On 1/24/22 12:50, Javier Martinez Canillas wrote:
> On 1/24/22 12:33, Thomas Zimmermann wrote:
>
> [snip]
>
>>> Thoughts?
>>
>> I can't say I approve keeping fbdev alive, but...
>>
>> With fbdev emulation, every DRM driver is an fbdev driver too. So
>> CONFIG_FB_DRIVER is somewhat misleading. Better add an option like
>> CONFIG_FBCON_HW_SCROLLING and have it selected by the fbdev drivers that
>> absolutely need HW acceleration. That option would then protect the rsp
>> code.

I'm not a fan of something like CONFIG_FBCON_HW_SCROLLING, but I'm not
against it either.
For me it sounds that this is not the real direction you want to go,
which is to prevent that any other drivers take the framebuffer before
you take it with simpledrm or similiar.
CONFIG_FBCON_HW_SCROLLING IMHO just disables the (from your POV) neglectable accleration part.
With an option like CONFIG_FB_DRIVER (maybe better: CONFIG_FB_LEGACY_DRIVERS)
it's an easy option for distros to disable all of the legacy drivers
from being built & shipped.

Instead of CONFIG_FBCON_HW_SCROLLING we could also choose
CONFIG_FBCON_LEGACY_ACCELERATION, because it includes fillrect() as well...

> Agreed that this option would be better and allow distros
> to disable the code that was reverted.

Yes, but IMHO it doesn't hurt either to leave it in.
It doesn't break anything at least.
Anyway...

Helge

2022-01-24 19:28:54

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hi

Am 24.01.22 um 16:29 schrieb Helge Deller:
> On 1/24/22 12:50, Javier Martinez Canillas wrote:
>> On 1/24/22 12:33, Thomas Zimmermann wrote:
>>
>> [snip]
>>
>>>> Thoughts?
>>>
>>> I can't say I approve keeping fbdev alive, but...
>>>
>>> With fbdev emulation, every DRM driver is an fbdev driver too. So
>>> CONFIG_FB_DRIVER is somewhat misleading. Better add an option like
>>> CONFIG_FBCON_HW_SCROLLING and have it selected by the fbdev drivers that
>>> absolutely need HW acceleration. That option would then protect the rsp
>>> code.
>
> I'm not a fan of something like CONFIG_FBCON_HW_SCROLLING, but I'm not
> against it either.
> For me it sounds that this is not the real direction you want to go,
> which is to prevent that any other drivers take the framebuffer before
> you take it with simpledrm or similiar.
> CONFIG_FBCON_HW_SCROLLING IMHO just disables the (from your POV) neglectable accleration part.
> With an option like CONFIG_FB_DRIVER (maybe better: CONFIG_FB_LEGACY_DRIVERS)
> it's an easy option for distros to disable all of the legacy drivers
> from being built & shipped.

These drivers have been disabled by most distros a long time ago. Those
that still remain are the generic, soon to be replaced, ones; and
drivers for niche architectures where no DRM-based replacement exists.

If I run DRM with fbdev emulation, HW scrolling is unused, possibly
buggy, and I'd want to not built it if possible. I guess that's what
most distros would want as well. That's the use case for FBCON_HW_SCROLLING.

Best regards
Thomas

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachments:

OpenPGP_signature (855.00 B)
OpenPGP digital signature

2022-01-24 19:29:19

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Mon, Jan 24, 2022 at 12:33 PM Thomas Zimmermann <[email protected]> wrote:
> With fbdev emulation, every DRM driver is an fbdev driver too. So

Some are even without?

drivers/gpu/drm/vmwgfx/vmwgfx_fb.c: ret = register_framebuffer(info);

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-01-24 19:29:31

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

Hi Helge,

On Mon, Jan 24, 2022 at 4:30 PM Helge Deller <[email protected]> wrote:
> On 1/24/22 12:50, Javier Martinez Canillas wrote:
> > On 1/24/22 12:33, Thomas Zimmermann wrote:
> >
> > [snip]
> >
> >>> Thoughts?
> >>
> >> I can't say I approve keeping fbdev alive, but...
> >>
> >> With fbdev emulation, every DRM driver is an fbdev driver too. So
> >> CONFIG_FB_DRIVER is somewhat misleading. Better add an option like
> >> CONFIG_FBCON_HW_SCROLLING and have it selected by the fbdev drivers that
> >> absolutely need HW acceleration. That option would then protect the rsp
> >> code.
>
> I'm not a fan of something like CONFIG_FBCON_HW_SCROLLING, but I'm not
> against it either.
> For me it sounds that this is not the real direction you want to go,
> which is to prevent that any other drivers take the framebuffer before
> you take it with simpledrm or similiar.
> CONFIG_FBCON_HW_SCROLLING IMHO just disables the (from your POV) neglectable accleration part.
> With an option like CONFIG_FB_DRIVER (maybe better: CONFIG_FB_LEGACY_DRIVERS)
> it's an easy option for distros to disable all of the legacy drivers
> from being built & shipped.
>
> Instead of CONFIG_FBCON_HW_SCROLLING we could also choose
> CONFIG_FBCON_LEGACY_ACCELERATION, because it includes fillrect() as well...

As this is about resurrecting features indicated by the various
FBINFO_HWACCEL_* flags, what about CONFIG_FB_HWACCEL?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-01-24 19:29:39

[permalink] [raw]

Subject: Re: [PATCH 2/2] Revert "fbcon: Disable accelerated scrolling"

On Mon, Jan 24, 2022 at 04:29:34PM +0100, Helge Deller wrote:
> On 1/24/22 12:50, Javier Martinez Canillas wrote:
> > On 1/24/22 12:33, Thomas Zimmermann wrote:
> >
> > [snip]
> >
> >>> Thoughts?
> >>
> >> I can't say I approve keeping fbdev alive, but...
> >>
> >> With fbdev emulation, every DRM driver is an fbdev driver too. So
> >> CONFIG_FB_DRIVER is somewhat misleading. Better add an option like
> >> CONFIG_FBCON_HW_SCROLLING and have it selected by the fbdev drivers that
> >> absolutely need HW acceleration. That option would then protect the rsp
> >> code.
>
> I'm not a fan of something like CONFIG_FBCON_HW_SCROLLING, but I'm not
> against it either.
> For me it sounds that this is not the real direction you want to go,
> which is to prevent that any other drivers take the framebuffer before
> you take it with simpledrm or similiar.
> CONFIG_FBCON_HW_SCROLLING IMHO just disables the (from your POV) neglectable accleration part.
> With an option like CONFIG_FB_DRIVER (maybe better: CONFIG_FB_LEGACY_DRIVERS)
> it's an easy option for distros to disable all of the legacy drivers
> from being built & shipped.
>
> Instead of CONFIG_FBCON_HW_SCROLLING we could also choose
> CONFIG_FBCON_LEGACY_ACCELERATION, because it includes fillrect() as well...

+1 on that name, since on the lwn discussions I've also seen some noise
about resurrecting scrollback. And I guess we could do that too and then
just add it all behind that same option.
-Daniel

> > Agreed that this option would be better and allow distros
> > to disable the code that was reverted.
>
> Yes, but IMHO it doesn't hurt either to leave it in.
> It doesn't break anything at least.
> Anyway...
>
> Helge

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2022-01-24 19:33:45