* [PATCH 1/3] fbcon: Disable accelerated scrolling
@ 2020-10-29 10:14 Daniel Vetter
2020-10-29 10:14 ` [PATCH 2/3] fbcon: Drop EXPORT_SYMBOL Daniel Vetter
` (4 more replies)
0 siblings, 5 replies; 27+ messages in thread
From: Daniel Vetter @ 2020-10-29 10:14 UTC (permalink / raw)
To: DRI Development
Cc: George Kennedy, Sam Ravnborg, Bartlomiej Zolnierkiewicz,
Tetsuo Handa, Daniel Vetter, Intel Graphics Development,
Gustavo A. R. Silva, Peter Rosin, Linus Torvalds, Tomi Valkeinen,
Ben Skeggs, Thomas Zimmermann, Greg Kroah-Hartman, nouveau,
Daniel Vetter, Nathan Chancellor, Jiri Slaby, Peilin Ye
So ever since syzbot discovered fbcon, we have solid proof that it's
full of bugs. And often the solution is to just delete code and remove
features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
Now the problem is that most modern-ish drivers really only treat
fbcon as an dumb kernel console until userspace takes over, and Oops
printer for some emergencies. Looking at drm drivers and the basic
vesa/efi fbdev drivers shows that only 3 drivers support any kind of
acceleration:
- nouveau, seems to be enabled by default
- omapdrm, when a DMM remapper exists using remapper rewriting for
y/xpanning
- gma500, but that is getting deleted now for the GTT remapper trick,
and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
flag, so unused (and could be deleted already I think).
No other driver supportes accelerated fbcon. And fbcon is the only
user of this accel code (it's not exposed as uapi through ioctls),
which means we could garbage collect fairly enormous amounts of code
if we kill this.
Plus because syzbot only runs on virtual hardware, and none of the
drivers for that have acceleration, we'd remove a huge gap in testing.
And there's no other even remotely comprehensive testing aside from
syzbot.
This patch here just disables the acceleration code by always
redrawing when scrolling. The plan is that once this has been merged
for well over a year in released kernels, we can start to go around
and delete a lot of code.
v2:
- Drop a few more unused local variables, somehow I missed the
compiler warnings (Sam)
- Fix typo in comment (Jiri)
- add a todo entry for the cleanup (Thomas)
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Peilin Ye <yepeilin.cs@gmail.com>
Cc: George Kennedy <george.kennedy@oracle.com>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Peter Rosin <peda@axentia.se>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
Documentation/gpu/todo.rst | 18 ++++++++++++++
drivers/video/fbdev/core/fbcon.c | 42 ++++++--------------------------
2 files changed, 25 insertions(+), 35 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 6b224ef14455..bec99341a904 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -277,6 +277,24 @@ 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 cef437817b0d..a68253485244 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1147,11 +1147,13 @@ static void fbcon_init(struct vc_data *vc, int init)
ops->graphics = 0;
- if ((cap & FBINFO_HWACCEL_COPYAREA) &&
- !(cap & FBINFO_HWACCEL_DISABLED))
- p->scrollmode = SCROLL_MOVE;
- else /* default to something safe */
- p->scrollmode = SCROLL_REDRAW;
+ /*
+ * 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
@@ -1961,45 +1963,15 @@ 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.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/3] fbcon: Drop EXPORT_SYMBOL
2020-10-29 10:14 [PATCH 1/3] fbcon: Disable accelerated scrolling Daniel Vetter
@ 2020-10-29 10:14 ` Daniel Vetter
2020-10-29 19:15 ` Thomas Zimmermann
2020-10-29 10:14 ` [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers Daniel Vetter
` (3 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: Daniel Vetter @ 2020-10-29 10:14 UTC (permalink / raw)
To: DRI Development
Cc: Tetsuo Handa, Daniel Vetter, Intel Graphics Development,
Peilin Ye, Greg Kroah-Hartman, Daniel Vetter, Jiri Slaby,
Helge Deller
Every since
commit 6104c37094e729f3d4ce65797002112735d49cd1
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Tue Aug 1 17:32:07 2017 +0200
fbcon: Make fbcon a built-time depency for fbdev
these are no longer distinct loadable modules, so exporting symbols is
kinda pointless.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Helge Deller <deller@gmx.de>
Cc: Peilin Ye <yepeilin.cs@gmail.com>
---
drivers/video/fbdev/core/bitblit.c | 3 ---
drivers/video/fbdev/core/fbcon_ccw.c | 1 -
drivers/video/fbdev/core/fbcon_cw.c | 1 -
drivers/video/fbdev/core/fbcon_rotate.c | 1 -
drivers/video/fbdev/core/fbcon_ud.c | 1 -
drivers/video/fbdev/core/softcursor.c | 2 --
drivers/video/fbdev/core/tileblit.c | 2 --
7 files changed, 11 deletions(-)
diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
index 9725ecd1255b..f98e8f298bc1 100644
--- a/drivers/video/fbdev/core/bitblit.c
+++ b/drivers/video/fbdev/core/bitblit.c
@@ -404,6 +404,3 @@ void fbcon_set_bitops(struct fbcon_ops *ops)
if (ops->rotate)
fbcon_set_rotate(ops);
}
-
-EXPORT_SYMBOL(fbcon_set_bitops);
-
diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c
index bbd869efd03b..9cd2c4b05c32 100644
--- a/drivers/video/fbdev/core/fbcon_ccw.c
+++ b/drivers/video/fbdev/core/fbcon_ccw.c
@@ -409,4 +409,3 @@ void fbcon_rotate_ccw(struct fbcon_ops *ops)
ops->cursor = ccw_cursor;
ops->update_start = ccw_update_start;
}
-EXPORT_SYMBOL(fbcon_rotate_ccw);
diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c
index a34cbe8e9874..88d89fad3f05 100644
--- a/drivers/video/fbdev/core/fbcon_cw.c
+++ b/drivers/video/fbdev/core/fbcon_cw.c
@@ -392,4 +392,3 @@ void fbcon_rotate_cw(struct fbcon_ops *ops)
ops->cursor = cw_cursor;
ops->update_start = cw_update_start;
}
-EXPORT_SYMBOL(fbcon_rotate_cw);
diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
index ac72d4f85f7d..df6b469aa2c2 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.c
+++ b/drivers/video/fbdev/core/fbcon_rotate.c
@@ -110,4 +110,3 @@ void fbcon_set_rotate(struct fbcon_ops *ops)
break;
}
}
-EXPORT_SYMBOL(fbcon_set_rotate);
diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c
index 199cbc7abe35..8d5e66b1bdfb 100644
--- a/drivers/video/fbdev/core/fbcon_ud.c
+++ b/drivers/video/fbdev/core/fbcon_ud.c
@@ -436,4 +436,3 @@ void fbcon_rotate_ud(struct fbcon_ops *ops)
ops->cursor = ud_cursor;
ops->update_start = ud_update_start;
}
-EXPORT_SYMBOL(fbcon_rotate_ud);
diff --git a/drivers/video/fbdev/core/softcursor.c b/drivers/video/fbdev/core/softcursor.c
index fc93f254498e..29e5b21cf373 100644
--- a/drivers/video/fbdev/core/softcursor.c
+++ b/drivers/video/fbdev/core/softcursor.c
@@ -74,5 +74,3 @@ int soft_cursor(struct fb_info *info, struct fb_cursor *cursor)
info->fbops->fb_imageblit(info, image);
return 0;
}
-
-EXPORT_SYMBOL(soft_cursor);
diff --git a/drivers/video/fbdev/core/tileblit.c b/drivers/video/fbdev/core/tileblit.c
index 628fe5e010c0..7539ae9040f8 100644
--- a/drivers/video/fbdev/core/tileblit.c
+++ b/drivers/video/fbdev/core/tileblit.c
@@ -151,5 +151,3 @@ void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info)
info->tileops->fb_settile(info, &map);
}
}
-
-EXPORT_SYMBOL(fbcon_set_tileops);
--
2.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] fbcon: Drop EXPORT_SYMBOL
2020-10-29 10:14 ` [PATCH 2/3] fbcon: Drop EXPORT_SYMBOL Daniel Vetter
@ 2020-10-29 19:15 ` Thomas Zimmermann
0 siblings, 0 replies; 27+ messages in thread
From: Thomas Zimmermann @ 2020-10-29 19:15 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: Tetsuo Handa, Greg Kroah-Hartman, Intel Graphics Development,
Helge Deller, Daniel Vetter, Jiri Slaby, Peilin Ye
[-- Attachment #1.1.1.1: Type: text/plain, Size: 4177 bytes --]
Am 29.10.20 um 11:14 schrieb Daniel Vetter:
> Every since
>
> commit 6104c37094e729f3d4ce65797002112735d49cd1
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Tue Aug 1 17:32:07 2017 +0200
>
> fbcon: Make fbcon a built-time depency for fbdev
>
> these are no longer distinct loadable modules, so exporting symbols is
> kinda pointless.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Helge Deller <deller@gmx.de>
> Cc: Peilin Ye <yepeilin.cs@gmail.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/video/fbdev/core/bitblit.c | 3 ---
> drivers/video/fbdev/core/fbcon_ccw.c | 1 -
> drivers/video/fbdev/core/fbcon_cw.c | 1 -
> drivers/video/fbdev/core/fbcon_rotate.c | 1 -
> drivers/video/fbdev/core/fbcon_ud.c | 1 -
> drivers/video/fbdev/core/softcursor.c | 2 --
> drivers/video/fbdev/core/tileblit.c | 2 --
> 7 files changed, 11 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
> index 9725ecd1255b..f98e8f298bc1 100644
> --- a/drivers/video/fbdev/core/bitblit.c
> +++ b/drivers/video/fbdev/core/bitblit.c
> @@ -404,6 +404,3 @@ void fbcon_set_bitops(struct fbcon_ops *ops)
> if (ops->rotate)
> fbcon_set_rotate(ops);
> }
> -
> -EXPORT_SYMBOL(fbcon_set_bitops);
> -
> diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c
> index bbd869efd03b..9cd2c4b05c32 100644
> --- a/drivers/video/fbdev/core/fbcon_ccw.c
> +++ b/drivers/video/fbdev/core/fbcon_ccw.c
> @@ -409,4 +409,3 @@ void fbcon_rotate_ccw(struct fbcon_ops *ops)
> ops->cursor = ccw_cursor;
> ops->update_start = ccw_update_start;
> }
> -EXPORT_SYMBOL(fbcon_rotate_ccw);
> diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c
> index a34cbe8e9874..88d89fad3f05 100644
> --- a/drivers/video/fbdev/core/fbcon_cw.c
> +++ b/drivers/video/fbdev/core/fbcon_cw.c
> @@ -392,4 +392,3 @@ void fbcon_rotate_cw(struct fbcon_ops *ops)
> ops->cursor = cw_cursor;
> ops->update_start = cw_update_start;
> }
> -EXPORT_SYMBOL(fbcon_rotate_cw);
> diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
> index ac72d4f85f7d..df6b469aa2c2 100644
> --- a/drivers/video/fbdev/core/fbcon_rotate.c
> +++ b/drivers/video/fbdev/core/fbcon_rotate.c
> @@ -110,4 +110,3 @@ void fbcon_set_rotate(struct fbcon_ops *ops)
> break;
> }
> }
> -EXPORT_SYMBOL(fbcon_set_rotate);
> diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c
> index 199cbc7abe35..8d5e66b1bdfb 100644
> --- a/drivers/video/fbdev/core/fbcon_ud.c
> +++ b/drivers/video/fbdev/core/fbcon_ud.c
> @@ -436,4 +436,3 @@ void fbcon_rotate_ud(struct fbcon_ops *ops)
> ops->cursor = ud_cursor;
> ops->update_start = ud_update_start;
> }
> -EXPORT_SYMBOL(fbcon_rotate_ud);
> diff --git a/drivers/video/fbdev/core/softcursor.c b/drivers/video/fbdev/core/softcursor.c
> index fc93f254498e..29e5b21cf373 100644
> --- a/drivers/video/fbdev/core/softcursor.c
> +++ b/drivers/video/fbdev/core/softcursor.c
> @@ -74,5 +74,3 @@ int soft_cursor(struct fb_info *info, struct fb_cursor *cursor)
> info->fbops->fb_imageblit(info, image);
> return 0;
> }
> -
> -EXPORT_SYMBOL(soft_cursor);
> diff --git a/drivers/video/fbdev/core/tileblit.c b/drivers/video/fbdev/core/tileblit.c
> index 628fe5e010c0..7539ae9040f8 100644
> --- a/drivers/video/fbdev/core/tileblit.c
> +++ b/drivers/video/fbdev/core/tileblit.c
> @@ -151,5 +151,3 @@ void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info)
> info->tileops->fb_settile(info, &map);
> }
> }
> -
> -EXPORT_SYMBOL(fbcon_set_tileops);
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.1.1.2: OpenPGP_0x680DC11D530B7A23.asc --]
[-- Type: application/pgp-keys, Size: 4259 bytes --]
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers
2020-10-29 10:14 [PATCH 1/3] fbcon: Disable accelerated scrolling Daniel Vetter
2020-10-29 10:14 ` [PATCH 2/3] fbcon: Drop EXPORT_SYMBOL Daniel Vetter
@ 2020-10-29 10:14 ` Daniel Vetter
2020-10-29 11:13 ` Gerd Hoffmann
` (3 more replies)
2020-10-29 12:42 ` [Intel-gfx] [PATCH 1/3] fbcon: Disable accelerated scrolling kernel test robot
` (2 subsequent siblings)
4 siblings, 4 replies; 27+ messages in thread
From: Daniel Vetter @ 2020-10-29 10:14 UTC (permalink / raw)
To: DRI Development
Cc: spice-devel, Daniel Vetter, Intel Graphics Development,
virtualization, Gerd Hoffmann, Daniel Vetter, Dave Airlie
These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev
emulation code").
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_drv.h | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 3602e8b34189..86eee66ecbad 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -166,20 +166,6 @@ struct qxl_drm_image {
struct list_head chunk_list;
};
-struct qxl_fb_image {
- struct qxl_device *qdev;
- uint32_t pseudo_palette[16];
- struct fb_image fb_image;
- uint32_t visual;
-};
-
-struct qxl_draw_fill {
- struct qxl_device *qdev;
- struct qxl_rect rect;
- uint32_t color;
- uint16_t rop;
-};
-
/*
* Debugfs
*/
--
2.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers
2020-10-29 10:14 ` [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers Daniel Vetter
@ 2020-10-29 11:13 ` Gerd Hoffmann
2020-10-29 13:33 ` [PATCH] " Daniel Vetter
` (2 subsequent siblings)
3 siblings, 0 replies; 27+ messages in thread
From: Gerd Hoffmann @ 2020-10-29 11:13 UTC (permalink / raw)
To: Daniel Vetter
Cc: Dave Airlie, Intel Graphics Development, DRI Development,
virtualization, spice-devel, Daniel Vetter
On Thu, Oct 29, 2020 at 11:14:28AM +0100, Daniel Vetter wrote:
> These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev
> emulation code").
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH] drm/qxl: Remove fbcon acceleration leftovers
2020-10-29 10:14 ` [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers Daniel Vetter
2020-10-29 11:13 ` Gerd Hoffmann
@ 2020-10-29 13:33 ` Daniel Vetter
2020-11-17 10:01 ` Daniel Vetter
2020-10-29 13:56 ` [PATCH 3/3] " kernel test robot
2020-10-30 0:37 ` kernel test robot
3 siblings, 1 reply; 27+ messages in thread
From: Daniel Vetter @ 2020-10-29 13:33 UTC (permalink / raw)
To: DRI Development
Cc: spice-devel, Daniel Vetter, Intel Graphics Development,
virtualization, Gerd Hoffmann, Daniel Vetter, Dave Airlie
These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev
emulation code").
v2: Somehow these structs provided the struct qxl_device pre-decl,
reorder the header to not anger compilers.
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_drv.h | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 3602e8b34189..6239626503ef 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -166,20 +166,6 @@ struct qxl_drm_image {
struct list_head chunk_list;
};
-struct qxl_fb_image {
- struct qxl_device *qdev;
- uint32_t pseudo_palette[16];
- struct fb_image fb_image;
- uint32_t visual;
-};
-
-struct qxl_draw_fill {
- struct qxl_device *qdev;
- struct qxl_rect rect;
- uint32_t color;
- uint16_t rop;
-};
-
/*
* Debugfs
*/
@@ -188,8 +174,6 @@ struct qxl_debugfs {
unsigned int num_files;
};
-int qxl_debugfs_fence_init(struct qxl_device *rdev);
-
struct qxl_device {
struct drm_device ddev;
@@ -271,6 +255,8 @@ struct qxl_device {
#define to_qxl(dev) container_of(dev, struct qxl_device, ddev)
+int qxl_debugfs_fence_init(struct qxl_device *rdev);
+
extern const struct drm_ioctl_desc qxl_ioctls[];
extern int qxl_max_ioctl;
--
2.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/qxl: Remove fbcon acceleration leftovers
2020-10-29 13:33 ` [PATCH] " Daniel Vetter
@ 2020-11-17 10:01 ` Daniel Vetter
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2020-11-17 10:01 UTC (permalink / raw)
To: DRI Development
Cc: spice-devel, Daniel Vetter, Intel Graphics Development,
virtualization, Gerd Hoffmann, Daniel Vetter, Dave Airlie
On Thu, Oct 29, 2020 at 02:33:47PM +0100, Daniel Vetter wrote:
> These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev
> emulation code").
>
> v2: Somehow these structs provided the struct qxl_device pre-decl,
> reorder the header to not anger compilers.
>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: spice-devel@lists.freedesktop.org
Entire series applied to drm-misc-next.
-Daniel
> ---
> drivers/gpu/drm/qxl/qxl_drv.h | 18 ++----------------
> 1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> index 3602e8b34189..6239626503ef 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.h
> +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> @@ -166,20 +166,6 @@ struct qxl_drm_image {
> struct list_head chunk_list;
> };
>
> -struct qxl_fb_image {
> - struct qxl_device *qdev;
> - uint32_t pseudo_palette[16];
> - struct fb_image fb_image;
> - uint32_t visual;
> -};
> -
> -struct qxl_draw_fill {
> - struct qxl_device *qdev;
> - struct qxl_rect rect;
> - uint32_t color;
> - uint16_t rop;
> -};
> -
> /*
> * Debugfs
> */
> @@ -188,8 +174,6 @@ struct qxl_debugfs {
> unsigned int num_files;
> };
>
> -int qxl_debugfs_fence_init(struct qxl_device *rdev);
> -
> struct qxl_device {
> struct drm_device ddev;
>
> @@ -271,6 +255,8 @@ struct qxl_device {
>
> #define to_qxl(dev) container_of(dev, struct qxl_device, ddev)
>
> +int qxl_debugfs_fence_init(struct qxl_device *rdev);
> +
> extern const struct drm_ioctl_desc qxl_ioctls[];
> extern int qxl_max_ioctl;
>
> --
> 2.28.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers
2020-10-29 10:14 ` [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers Daniel Vetter
2020-10-29 11:13 ` Gerd Hoffmann
2020-10-29 13:33 ` [PATCH] " Daniel Vetter
@ 2020-10-29 13:56 ` kernel test robot
2020-10-30 0:37 ` kernel test robot
3 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2020-10-29 13:56 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: kbuild-all, Daniel Vetter, Intel Graphics Development,
virtualization, Gerd Hoffmann, Dave Airlie, spice-devel
[-- Attachment #1: Type: text/plain, Size: 2195 bytes --]
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.10-rc1 next-20201028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: alpha-randconfig-r003-20201029 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/188b22d2b66860695df5d07bf2b7115976790b2c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618
git checkout 188b22d2b66860695df5d07bf2b7115976790b2c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/qxl/qxl_drv.c:31:
>> drivers/gpu/drm/qxl/qxl_drv.h:178:35: warning: 'struct qxl_device' declared inside parameter list will not be visible outside of this definition or declaration
178 | int qxl_debugfs_fence_init(struct qxl_device *rdev);
| ^~~~~~~~~~
vim +178 drivers/gpu/drm/qxl/qxl_drv.h
f64122c1f6ade30 Dave Airlie 2013-02-25 177
f64122c1f6ade30 Dave Airlie 2013-02-25 @178 int qxl_debugfs_fence_init(struct qxl_device *rdev);
f64122c1f6ade30 Dave Airlie 2013-02-25 179
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27013 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers
2020-10-29 10:14 ` [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers Daniel Vetter
` (2 preceding siblings ...)
2020-10-29 13:56 ` [PATCH 3/3] " kernel test robot
@ 2020-10-30 0:37 ` kernel test robot
3 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2020-10-30 0:37 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: kbuild-all, Daniel Vetter, Intel Graphics Development,
virtualization, clang-built-linux, Gerd Hoffmann, Dave Airlie,
spice-devel
[-- Attachment #1: Type: text/plain, Size: 2352 bytes --]
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.10-rc1 next-20201029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: powerpc-randconfig-r015-20201029 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 772aaa602383cf82795572ebcd86b0e660f3579f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/0day-ci/linux/commit/188b22d2b66860695df5d07bf2b7115976790b2c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618
git checkout 188b22d2b66860695df5d07bf2b7115976790b2c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/qxl/qxl_drv.c:31:
>> drivers/gpu/drm/qxl/qxl_drv.h:178:35: warning: declaration of 'struct qxl_device' will not be visible outside of this function [-Wvisibility]
int qxl_debugfs_fence_init(struct qxl_device *rdev);
^
1 warning generated.
vim +178 drivers/gpu/drm/qxl/qxl_drv.h
f64122c1f6ade30 Dave Airlie 2013-02-25 177
f64122c1f6ade30 Dave Airlie 2013-02-25 @178 int qxl_debugfs_fence_init(struct qxl_device *rdev);
f64122c1f6ade30 Dave Airlie 2013-02-25 179
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31155 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Intel-gfx] [PATCH 1/3] fbcon: Disable accelerated scrolling
2020-10-29 10:14 [PATCH 1/3] fbcon: Disable accelerated scrolling Daniel Vetter
2020-10-29 10:14 ` [PATCH 2/3] fbcon: Drop EXPORT_SYMBOL Daniel Vetter
2020-10-29 10:14 ` [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers Daniel Vetter
@ 2020-10-29 12:42 ` kernel test robot
2020-10-29 13:22 ` [PATCH] " Daniel Vetter
2020-10-29 19:17 ` [PATCH 1/3] " Thomas Zimmermann
4 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2020-10-29 12:42 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: kbuild-all, Bartlomiej Zolnierkiewicz, Tetsuo Handa,
Daniel Vetter, Intel Graphics Development, Gustavo A. R. Silva,
George Kennedy, Tomi Valkeinen, Sam Ravnborg, Peter Rosin
[-- Attachment #1: Type: text/plain, Size: 20621 bytes --]
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.10-rc1 next-20201028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: sparc64-randconfig-r005-20201029 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/b457f0ea024ca7202fa63f5a94f9d5abf65529f8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618
git checkout b457f0ea024ca7202fa63f5a94f9d5abf65529f8
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/video/fbdev/core/fbcon.c: In function 'fbcon_init':
>> drivers/video/fbdev/core/fbcon.c:1089:6: warning: variable 'cap' set but not used [-Wunused-but-set-variable]
1089 | int cap, ret;
| ^~~
drivers/video/fbdev/core/fbcon.c: In function 'fbcon_exit':
drivers/video/fbdev/core/fbcon.c:3646:7: warning: variable 'pending' set but not used [-Wunused-but-set-variable]
3646 | int pending = 0;
| ^~~~~~~
vim +/cap +1089 drivers/video/fbdev/core/fbcon.c
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1080
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1081 static void fbcon_init(struct vc_data *vc, int init)
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1082 {
1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1083 struct fb_info *info;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1084 struct fbcon_ops *ops;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1085 struct vc_data **default_mode = vc->vc_display_fg;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1086 struct vc_data *svc = *default_mode;
50233393f0cf9b drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1087 struct fbcon_display *t, *p = &fb_display[vc->vc_num];
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1088 int logo = 1, new_rows, new_cols, rows, cols, charcnt = 256;
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 @1089 int cap, ret;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1090
1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1091 if (WARN_ON(info_idx == -1))
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1092 return;
306958e8e8d150 drivers/video/console/fbcon.c Adrian Bunk 2005-05-01 1093
1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1094 if (con2fb_map[vc->vc_num] == -1)
1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1095 con2fb_map[vc->vc_num] = info_idx;
1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1096
1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1097 info = registered_fb[con2fb_map[vc->vc_num]];
306958e8e8d150 drivers/video/console/fbcon.c Adrian Bunk 2005-05-01 1098 cap = info->flags;
306958e8e8d150 drivers/video/console/fbcon.c Adrian Bunk 2005-05-01 1099
3c5a1b111373e6 drivers/video/fbdev/core/fbcon.c Andreas Schwab 2019-05-06 1100 if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
10993504d64735 drivers/video/fbdev/core/fbcon.c Prarit Bhargava 2019-02-08 1101 logo_shown = FBCON_LOGO_DONTSHOW;
10993504d64735 drivers/video/fbdev/core/fbcon.c Prarit Bhargava 2019-02-08 1102
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1103 if (vc != svc || logo_shown == FBCON_LOGO_DONTSHOW ||
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1104 (info->fix.type == FB_TYPE_TEXT))
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1105 logo = 0;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1106
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1107 if (var_to_display(p, &info->var, info))
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1108 return;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1109
d1baa4ffa677bf drivers/video/console/fbcon.c Antonino A. Daplas 2007-07-17 1110 if (!info->fbcon_par)
d1baa4ffa677bf drivers/video/console/fbcon.c Antonino A. Daplas 2007-07-17 1111 con2fb_acquire_newinfo(vc, info, vc->vc_num, -1);
d1baa4ffa677bf drivers/video/console/fbcon.c Antonino A. Daplas 2007-07-17 1112
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1113 /* If we are not the first console on this
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1114 fb, copy the font from that console */
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1115 t = &fb_display[fg_console];
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1116 if (!p->fontdata) {
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1117 if (t->fontdata) {
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1118 struct vc_data *fvc = vc_cons[fg_console].d;
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1119
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1120 vc->vc_font.data = (void *)(p->fontdata =
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1121 fvc->vc_font.data);
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1122 vc->vc_font.width = fvc->vc_font.width;
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1123 vc->vc_font.height = fvc->vc_font.height;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1124 p->userfont = t->userfont;
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1125
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1126 if (p->userfont)
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1127 REFCOUNT(p->fontdata)++;
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1128 } else {
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1129 const struct font_desc *font = NULL;
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1130
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1131 if (!fontname[0] || !(font = find_font(fontname)))
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1132 font = get_default_font(info->var.xres,
2d2699d9849248 drivers/video/console/fbcon.c Antonino A. Daplas 2007-05-08 1133 info->var.yres,
2d2699d9849248 drivers/video/console/fbcon.c Antonino A. Daplas 2007-05-08 1134 info->pixmap.blit_x,
2d2699d9849248 drivers/video/console/fbcon.c Antonino A. Daplas 2007-05-08 1135 info->pixmap.blit_y);
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1136 vc->vc_font.width = font->width;
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1137 vc->vc_font.height = font->height;
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1138 vc->vc_font.data = (void *)(p->fontdata = font->data);
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1139 vc->vc_font.charcount = 256; /* FIXME Need to
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1140 support more fonts */
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1141 }
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1142 }
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1143
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1144 if (p->userfont)
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1145 charcnt = FNTCHARCNT(p->fontdata);
e614b18dcedb24 drivers/video/console/fbcon.c Antonino A. Daplas 2006-06-26 1146
b8c909454f046b drivers/video/console/fbcon.c Antonino A. Daplas 2005-09-09 1147 vc->vc_can_do_color = (fb_get_color_depth(&info->var, &info->fix)!=1);
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1148 vc->vc_complement_mask = vc->vc_can_do_color ? 0x7700 : 0x0800;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1149 if (charcnt == 256) {
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1150 vc->vc_hi_font_mask = 0;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1151 } else {
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1152 vc->vc_hi_font_mask = 0x100;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1153 if (vc->vc_can_do_color)
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1154 vc->vc_complement_mask <<= 1;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1155 }
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1156
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1157 if (!*svc->vc_uni_pagedir_loc)
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1158 con_set_default_unimap(svc);
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1159 if (!*vc->vc_uni_pagedir_loc)
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1160 con_copy_unimap(vc, svc);
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1161
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1162 ops = info->fbcon_par;
f235f664a8afab drivers/video/console/fbcon.c Scot Doyle 2015-10-09 1163 ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
c9e6a36492504e drivers/video/fbdev/core/fbcon.c Hans de Goede 2017-11-25 1164
2428e59b530928 drivers/video/console/fbcon.c Marcin Slusarz 2008-02-06 1165 p->con_rotate = initial_rotation;
c9e6a36492504e drivers/video/fbdev/core/fbcon.c Hans de Goede 2017-11-25 1166 if (p->con_rotate == -1)
c9e6a36492504e drivers/video/fbdev/core/fbcon.c Hans de Goede 2017-11-25 1167 p->con_rotate = info->fbcon_rotate_hint;
c9e6a36492504e drivers/video/fbdev/core/fbcon.c Hans de Goede 2017-11-25 1168 if (p->con_rotate == -1)
f2f4946b0adfd6 drivers/video/fbdev/core/fbcon.c Hans de Goede 2017-11-25 1169 p->con_rotate = FB_ROTATE_UR;
c9e6a36492504e drivers/video/fbdev/core/fbcon.c Hans de Goede 2017-11-25 1170
b73deed32d0874 drivers/video/console/fbcon.c Antonino A. Daplas 2006-01-09 1171 set_blitting_type(vc, info);
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1172
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1173 cols = vc->vc_cols;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1174 rows = vc->vc_rows;
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1175 new_cols = FBCON_SWAP(ops->rotate, info->var.xres, info->var.yres);
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1176 new_rows = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1177 new_cols /= vc->vc_font.width;
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1178 new_rows /= vc->vc_font.height;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1179
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1180 /*
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1181 * We must always set the mode. The mode of the previous console
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1182 * driver could be in the same resolution but we are using different
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1183 * hardware so we have to initialize the hardware.
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1184 *
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1185 * We need to do it in fbcon_init() to prevent screen corruption.
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1186 */
6ca8dfd78187d8 drivers/video/console/fbcon.c Jiri Slaby 2016-06-23 1187 if (con_is_visible(vc) && vc->vc_mode == KD_TEXT) {
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1188 if (info->fbops->fb_set_par &&
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 1189 !(ops->flags & FBCON_FLAGS_INIT)) {
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 1190 ret = info->fbops->fb_set_par(info);
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 1191
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 1192 if (ret)
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 1193 printk(KERN_ERR "fbcon_init: detected "
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 1194 "unhandled fb_set_par error, "
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 1195 "error code %d\n", ret);
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 1196 }
0fcf6ada2b8eb4 drivers/video/console/fbcon.c Florian Tobias Schandinat 2009-09-22 1197
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1198 ops->flags |= FBCON_FLAGS_INIT;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1199 }
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1200
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1201 ops->graphics = 0;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1202
b457f0ea024ca7 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2020-10-29 1203 /*
b457f0ea024ca7 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2020-10-29 1204 * No more hw acceleration for fbcon.
b457f0ea024ca7 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2020-10-29 1205 *
b457f0ea024ca7 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2020-10-29 1206 * FIXME: Garbage collect all the now dead code after sufficient time
b457f0ea024ca7 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2020-10-29 1207 * has passed.
b457f0ea024ca7 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2020-10-29 1208 */
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1209 p->scrollmode = SCROLL_REDRAW;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1210
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1211 /*
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1212 * ++guenther: console.c:vc_allocate() relies on initializing
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1213 * vc_{cols,rows}, but we must not set those if we are only
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1214 * resizing the console.
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1215 */
0035fe00f77d2b drivers/video/console/fbcon.c Johannes Weiner 2009-08-06 1216 if (init) {
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1217 vc->vc_cols = new_cols;
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1218 vc->vc_rows = new_rows;
0035fe00f77d2b drivers/video/console/fbcon.c Johannes Weiner 2009-08-06 1219 } else
0035fe00f77d2b drivers/video/console/fbcon.c Johannes Weiner 2009-08-06 1220 vc_resize(vc, new_cols, new_rows);
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1221
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1222 if (logo)
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1223 fbcon_prepare_logo(vc, info, cols, rows, new_cols, new_rows);
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1224
4d9c5b6eb42d9e drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-07 1225 if (vc == svc && softback_buf)
4d9c5b6eb42d9e drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-07 1226 fbcon_update_softback(vc);
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1227
b73deed32d0874 drivers/video/console/fbcon.c Antonino A. Daplas 2006-01-09 1228 if (ops->rotate_font && ops->rotate_font(info, vc)) {
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1229 ops->rotate = FB_ROTATE_UR;
b73deed32d0874 drivers/video/console/fbcon.c Antonino A. Daplas 2006-01-09 1230 set_blitting_type(vc, info);
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1231 }
e4fc27618b7523 drivers/video/console/fbcon.c Antonino A. Daplas 2005-11-08 1232
1a37d5f51020b9 drivers/video/console/fbcon.c Antonino A. Daplas 2006-03-31 1233 ops->p = &fb_display[fg_console];
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1234 }
^1da177e4c3f41 drivers/video/console/fbcon.c Linus Torvalds 2005-04-16 1235
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32314 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH] fbcon: Disable accelerated scrolling
2020-10-29 10:14 [PATCH 1/3] fbcon: Disable accelerated scrolling Daniel Vetter
` (2 preceding siblings ...)
2020-10-29 12:42 ` [Intel-gfx] [PATCH 1/3] fbcon: Disable accelerated scrolling kernel test robot
@ 2020-10-29 13:22 ` Daniel Vetter
2020-10-30 8:30 ` Tomi Valkeinen
2020-10-31 10:27 ` Geert Uytterhoeven
2020-10-29 19:17 ` [PATCH 1/3] " Thomas Zimmermann
4 siblings, 2 replies; 27+ messages in thread
From: Daniel Vetter @ 2020-10-29 13:22 UTC (permalink / raw)
To: DRI Development
Cc: George Kennedy, Sam Ravnborg, Bartlomiej Zolnierkiewicz,
Tetsuo Handa, Daniel Vetter, Intel Graphics Development,
Gustavo A. R. Silva, Peter Rosin, Linus Torvalds, Tomi Valkeinen,
Ben Skeggs, Thomas Zimmermann, Greg Kroah-Hartman, nouveau,
Daniel Vetter, Nathan Chancellor, Jiri Slaby, Peilin Ye
So ever since syzbot discovered fbcon, we have solid proof that it's
full of bugs. And often the solution is to just delete code and remove
features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
Now the problem is that most modern-ish drivers really only treat
fbcon as an dumb kernel console until userspace takes over, and Oops
printer for some emergencies. Looking at drm drivers and the basic
vesa/efi fbdev drivers shows that only 3 drivers support any kind of
acceleration:
- nouveau, seems to be enabled by default
- omapdrm, when a DMM remapper exists using remapper rewriting for
y/xpanning
- gma500, but that is getting deleted now for the GTT remapper trick,
and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
flag, so unused (and could be deleted already I think).
No other driver supportes accelerated fbcon. And fbcon is the only
user of this accel code (it's not exposed as uapi through ioctls),
which means we could garbage collect fairly enormous amounts of code
if we kill this.
Plus because syzbot only runs on virtual hardware, and none of the
drivers for that have acceleration, we'd remove a huge gap in testing.
And there's no other even remotely comprehensive testing aside from
syzbot.
This patch here just disables the acceleration code by always
redrawing when scrolling. The plan is that once this has been merged
for well over a year in released kernels, we can start to go around
and delete a lot of code.
v2:
- Drop a few more unused local variables, somehow I missed the
compiler warnings (Sam)
- Fix typo in comment (Jiri)
- add a todo entry for the cleanup (Thomas)
v3: Remove more unused variables (0day)
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Peilin Ye <yepeilin.cs@gmail.com>
Cc: George Kennedy <george.kennedy@oracle.com>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Peter Rosin <peda@axentia.se>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
Documentation/gpu/todo.rst | 18 +++++++++++++
drivers/video/fbdev/core/fbcon.c | 45 ++++++--------------------------
2 files changed, 26 insertions(+), 37 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 6b224ef14455..bec99341a904 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -277,6 +277,24 @@ 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 cef437817b0d..8d1ae973041a 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1033,7 +1033,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, charcnt = 256;
- int cap, ret;
+ int ret;
if (WARN_ON(info_idx == -1))
return;
@@ -1042,7 +1042,6 @@ 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;
@@ -1147,11 +1146,13 @@ static void fbcon_init(struct vc_data *vc, int init)
ops->graphics = 0;
- if ((cap & FBINFO_HWACCEL_COPYAREA) &&
- !(cap & FBINFO_HWACCEL_DISABLED))
- p->scrollmode = SCROLL_MOVE;
- else /* default to something safe */
- p->scrollmode = SCROLL_REDRAW;
+ /*
+ * 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
@@ -1961,45 +1962,15 @@ 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.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-29 13:22 ` [PATCH] " Daniel Vetter
@ 2020-10-30 8:30 ` Tomi Valkeinen
2020-10-30 8:52 ` Daniel Vetter
2020-10-31 10:27 ` Geert Uytterhoeven
1 sibling, 1 reply; 27+ messages in thread
From: Tomi Valkeinen @ 2020-10-30 8:30 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: George Kennedy, Sam Ravnborg, Bartlomiej Zolnierkiewicz,
Tetsuo Handa, Greg Kroah-Hartman, Intel Graphics Development,
Gustavo A. R. Silva, Peter Rosin, Linus Torvalds, Ben Skeggs,
Thomas Zimmermann, nouveau, Daniel Vetter, Nathan Chancellor,
Jiri Slaby, Peilin Ye
On 29/10/2020 15:22, Daniel Vetter wrote:
> So ever since syzbot discovered fbcon, we have solid proof that it's
> full of bugs. And often the solution is to just delete code and remove
> features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
>
> Now the problem is that most modern-ish drivers really only treat
> fbcon as an dumb kernel console until userspace takes over, and Oops
> printer for some emergencies. Looking at drm drivers and the basic
> vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> acceleration:
>
> - nouveau, seems to be enabled by default
> - omapdrm, when a DMM remapper exists using remapper rewriting for
> y/xpanning
> - gma500, but that is getting deleted now for the GTT remapper trick,
> and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> flag, so unused (and could be deleted already I think).
>
> No other driver supportes accelerated fbcon. And fbcon is the only
> user of this accel code (it's not exposed as uapi through ioctls),
> which means we could garbage collect fairly enormous amounts of code
> if we kill this.
>
> Plus because syzbot only runs on virtual hardware, and none of the
> drivers for that have acceleration, we'd remove a huge gap in testing.
> And there's no other even remotely comprehensive testing aside from
> syzbot.
>
> This patch here just disables the acceleration code by always
> redrawing when scrolling. The plan is that once this has been merged
> for well over a year in released kernels, we can start to go around
> and delete a lot of code.
>
> v2:
> - Drop a few more unused local variables, somehow I missed the
> compiler warnings (Sam)
> - Fix typo in comment (Jiri)
> - add a todo entry for the cleanup (Thomas)
>
> v3: Remove more unused variables (0day)
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Peilin Ye <yepeilin.cs@gmail.com>
> Cc: George Kennedy <george.kennedy@oracle.com>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Peter Rosin <peda@axentia.se>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> Documentation/gpu/todo.rst | 18 +++++++++++++
> drivers/video/fbdev/core/fbcon.c | 45 ++++++--------------------------
> 2 files changed, 26 insertions(+), 37 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 6b224ef14455..bec99341a904 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -277,6 +277,24 @@ 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.
Apparently omapdrm's accelerated panning has been broken for some time, and no one has noticed. It does:
strcmp(fbi->fix.id, MODULE_NAME), which is a comparison of omapdrmdrmfb == omapdrm and always fails.
Fixing that, and applying this patch, things work fine (unaccelerated, of course). I did notice a
single call to omap_fbdev_pan_display() when loading the drivers. This comes from fbcon_switch ->
bit_update_start -> fb_pan_display. Maybe this is from the clearing you mention above?
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-30 8:30 ` Tomi Valkeinen
@ 2020-10-30 8:52 ` Daniel Vetter
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2020-10-30 8:52 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: George Kennedy, Sam Ravnborg, Bartlomiej Zolnierkiewicz,
Tetsuo Handa, Greg Kroah-Hartman, Intel Graphics Development,
Gustavo A. R. Silva, DRI Development, Peter Rosin,
Linus Torvalds, Ben Skeggs, Thomas Zimmermann, Nouveau Dev,
Daniel Vetter, Nathan Chancellor, Jiri Slaby, Peilin Ye
On Fri, Oct 30, 2020 at 9:30 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> On 29/10/2020 15:22, Daniel Vetter wrote:
> > So ever since syzbot discovered fbcon, we have solid proof that it's
> > full of bugs. And often the solution is to just delete code and remove
> > features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
> >
> > Now the problem is that most modern-ish drivers really only treat
> > fbcon as an dumb kernel console until userspace takes over, and Oops
> > printer for some emergencies. Looking at drm drivers and the basic
> > vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> > acceleration:
> >
> > - nouveau, seems to be enabled by default
> > - omapdrm, when a DMM remapper exists using remapper rewriting for
> > y/xpanning
> > - gma500, but that is getting deleted now for the GTT remapper trick,
> > and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> > flag, so unused (and could be deleted already I think).
> >
> > No other driver supportes accelerated fbcon. And fbcon is the only
> > user of this accel code (it's not exposed as uapi through ioctls),
> > which means we could garbage collect fairly enormous amounts of code
> > if we kill this.
> >
> > Plus because syzbot only runs on virtual hardware, and none of the
> > drivers for that have acceleration, we'd remove a huge gap in testing.
> > And there's no other even remotely comprehensive testing aside from
> > syzbot.
> >
> > This patch here just disables the acceleration code by always
> > redrawing when scrolling. The plan is that once this has been merged
> > for well over a year in released kernels, we can start to go around
> > and delete a lot of code.
> >
> > v2:
> > - Drop a few more unused local variables, somehow I missed the
> > compiler warnings (Sam)
> > - Fix typo in comment (Jiri)
> > - add a todo entry for the cleanup (Thomas)
> >
> > v3: Remove more unused variables (0day)
> >
> > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Cc: nouveau@lists.freedesktop.org
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > Cc: Peilin Ye <yepeilin.cs@gmail.com>
> > Cc: George Kennedy <george.kennedy@oracle.com>
> > Cc: Nathan Chancellor <natechancellor@gmail.com>
> > Cc: Peter Rosin <peda@axentia.se>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > Documentation/gpu/todo.rst | 18 +++++++++++++
> > drivers/video/fbdev/core/fbcon.c | 45 ++++++--------------------------
> > 2 files changed, 26 insertions(+), 37 deletions(-)
> >
> > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> > index 6b224ef14455..bec99341a904 100644
> > --- a/Documentation/gpu/todo.rst
> > +++ b/Documentation/gpu/todo.rst
> > @@ -277,6 +277,24 @@ 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.
>
> Apparently omapdrm's accelerated panning has been broken for some time, and no one has noticed. It does:
>
> strcmp(fbi->fix.id, MODULE_NAME), which is a comparison of omapdrmdrmfb == omapdrm and always fails.
>
> Fixing that, and applying this patch, things work fine (unaccelerated, of course). I did notice a
> single call to omap_fbdev_pan_display() when loading the drivers. This comes from fbcon_switch ->
> bit_update_start -> fb_pan_display. Maybe this is from the clearing you mention above?
The accel left is through fb_fillrect and fb_imageblt, plus there's
still fb_cursor (but not much use of that even in fbdev drivers).
I'm honestly not sure why there's the pan call in there, maybe just to
reset to default state in case an fbdev chardev user moved the origin
around. Aside from the accel code there's a call to this in
fbcon_switch and fbcon_modechanged, so I think it's just that.
-Daniel
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>
> Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-29 13:22 ` [PATCH] " Daniel Vetter
2020-10-30 8:30 ` Tomi Valkeinen
@ 2020-10-31 10:27 ` Geert Uytterhoeven
2020-10-31 14:17 ` Daniel Vetter
1 sibling, 1 reply; 27+ messages in thread
From: Geert Uytterhoeven @ 2020-10-31 10:27 UTC (permalink / raw)
To: Daniel Vetter
Cc: linux-fbdev, Jiri Slaby, Bartlomiej Zolnierkiewicz, Tetsuo Handa,
Linus Torvalds, Intel Graphics Development, Gustavo A. R. Silva,
DRI Development, Peilin Ye, George Kennedy, Tomi Valkeinen,
Ben Skeggs, Thomas Zimmermann, Greg Kroah-Hartman, nouveau,
Daniel Vetter, Nathan Chancellor, Sam Ravnborg, Peter Rosin
Hi Daniel,
CC linux-fbdev
Thanks for your patch!
On Thu, 29 Oct 2020, Daniel Vetter wrote:
> So ever since syzbot discovered fbcon, we have solid proof that it's
> full of bugs. And often the solution is to just delete code and remove
> features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
>
> Now the problem is that most modern-ish drivers really only treat
> fbcon as an dumb kernel console until userspace takes over, and Oops
> printer for some emergencies. Looking at drm drivers and the basic
> vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> acceleration:
>
> - nouveau, seems to be enabled by default
> - omapdrm, when a DMM remapper exists using remapper rewriting for
> y/xpanning
> - gma500, but that is getting deleted now for the GTT remapper trick,
> and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> flag, so unused (and could be deleted already I think).
>
> No other driver supportes accelerated fbcon. And fbcon is the only
> user of this accel code (it's not exposed as uapi through ioctls),
> which means we could garbage collect fairly enormous amounts of code
> if we kill this.
"git grep FBINFO_HWACCEL_COPYAREA" shows me there are 32 more drivers
using acceleration under drivers/video/fbdev/.
> Plus because syzbot only runs on virtual hardware, and none of the
> drivers for that have acceleration, we'd remove a huge gap in testing.
> And there's no other even remotely comprehensive testing aside from
> syzbot.
That sounds like a great argument to remove all hardware drivers from
the kernel ;-)
Seriously, how hard can it be to add "software-accelerated" acceleration
hooks to drivers/video/fbdev/vfb.c, to enable syzbot to exercise the
core acceleration code paths?
> This patch here just disables the acceleration code by always
> redrawing when scrolling. The plan is that once this has been merged
> for well over a year in released kernels, we can start to go around
> and delete a lot of code.
Have you benchmarked the performance impact on traditional fbdev
drivers?
Thanks!
> v2:
> - Drop a few more unused local variables, somehow I missed the
> compiler warnings (Sam)
> - Fix typo in comment (Jiri)
> - add a todo entry for the cleanup (Thomas)
>
> v3: Remove more unused variables (0day)
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Peilin Ye <yepeilin.cs@gmail.com>
> Cc: George Kennedy <george.kennedy@oracle.com>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Peter Rosin <peda@axentia.se>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> Documentation/gpu/todo.rst | 18 +++++++++++++
> drivers/video/fbdev/core/fbcon.c | 45 ++++++--------------------------
> 2 files changed, 26 insertions(+), 37 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 6b224ef14455..bec99341a904 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -277,6 +277,24 @@ 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 cef437817b0d..8d1ae973041a 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -1033,7 +1033,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, charcnt = 256;
> - int cap, ret;
> + int ret;
>
> if (WARN_ON(info_idx == -1))
> return;
> @@ -1042,7 +1042,6 @@ 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;
> @@ -1147,11 +1146,13 @@ static void fbcon_init(struct vc_data *vc, int init)
>
> ops->graphics = 0;
>
> - if ((cap & FBINFO_HWACCEL_COPYAREA) &&
> - !(cap & FBINFO_HWACCEL_DISABLED))
> - p->scrollmode = SCROLL_MOVE;
> - else /* default to something safe */
> - p->scrollmode = SCROLL_REDRAW;
> + /*
> + * 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
> @@ -1961,45 +1962,15 @@ 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.28.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-31 10:27 ` Geert Uytterhoeven
@ 2020-10-31 14:17 ` Daniel Vetter
2020-11-18 9:21 ` Geert Uytterhoeven
0 siblings, 1 reply; 27+ messages in thread
From: Daniel Vetter @ 2020-10-31 14:17 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux Fbdev development list, Jiri Slaby,
Bartlomiej Zolnierkiewicz, Tetsuo Handa, Linus Torvalds,
Intel Graphics Development, Gustavo A. R. Silva, DRI Development,
Peilin Ye, George Kennedy, Tomi Valkeinen, Ben Skeggs,
Thomas Zimmermann, Greg Kroah-Hartman, Nouveau Dev,
Daniel Vetter, Nathan Chancellor, Sam Ravnborg, Peter Rosin
On Sat, Oct 31, 2020 at 11:28 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Daniel,
>
> CC linux-fbdev
>
> Thanks for your patch!
>
> On Thu, 29 Oct 2020, Daniel Vetter wrote:
> > So ever since syzbot discovered fbcon, we have solid proof that it's
> > full of bugs. And often the solution is to just delete code and remove
> > features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
> >
> > Now the problem is that most modern-ish drivers really only treat
> > fbcon as an dumb kernel console until userspace takes over, and Oops
> > printer for some emergencies. Looking at drm drivers and the basic
> > vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> > acceleration:
> >
> > - nouveau, seems to be enabled by default
> > - omapdrm, when a DMM remapper exists using remapper rewriting for
> > y/xpanning
> > - gma500, but that is getting deleted now for the GTT remapper trick,
> > and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> > flag, so unused (and could be deleted already I think).
> >
> > No other driver supportes accelerated fbcon. And fbcon is the only
> > user of this accel code (it's not exposed as uapi through ioctls),
> > which means we could garbage collect fairly enormous amounts of code
> > if we kill this.
>
> "git grep FBINFO_HWACCEL_COPYAREA" shows me there are 32 more drivers
> using acceleration under drivers/video/fbdev/.
>
> > Plus because syzbot only runs on virtual hardware, and none of the
> > drivers for that have acceleration, we'd remove a huge gap in testing.
> > And there's no other even remotely comprehensive testing aside from
> > syzbot.
>
> That sounds like a great argument to remove all hardware drivers from
> the kernel ;-)
fbdev is unmaintained, has no one volunteering to put in the work (and
there's huge amounts of work needed), and there's no test suite. No,
fbtest.c doesn't can't, that's not even close. We're not going to
delete everything in the kernel, but slowly sunsetting stuff that's
just costing and not bringing in up is a good idea.
> Seriously, how hard can it be to add "software-accelerated" acceleration
> hooks to drivers/video/fbdev/vfb.c, to enable syzbot to exercise the
> core acceleration code paths?
Just this one is 5 combinations, which means I'd need to convince
syzbot to test 5 different machine setups.
Plus we're still lacking a test suite, and judging from how much time
it took to get something basic going for kms, that's about 2 engineer
years of effort that no one is even close to willing to spend.
> > This patch here just disables the acceleration code by always
> > redrawing when scrolling. The plan is that once this has been merged
> > for well over a year in released kernels, we can start to go around
> > and delete a lot of code.
>
> Have you benchmarked the performance impact on traditional fbdev
> drivers?
There's still some acceleration if you have an image blit engine for
redrawing the screen. But the complexity is contained in the old
drivers that no one cares about.
For anything I have access to the difference is 0.
Also note that for anything remotely modern the fbcon acceleration
framework is pretty badly designed, because it does not allow
sufficient pipelining and queuing of operations. We've had an fbcon
acceleration prototype for i915 10 years ago or so, it's just not
worth the bother.
And again, no one is volunteering to create an fbcon accel framework
that doesn't just suck, despite that this has been discussed in
various places for years. I've done a summary of the sorry state of 2d
acceleration 2 years ago because it's come up so many times:
https://blog.ffwll.ch/2018/08/no-2d-in-drm.html Nothing at all
happened since then.
Reality is that fbdev is just there nowadays for Oops printing and
emergency usage, and it's plenty good enough for that. If there's
anyone who cares beyond that, they're most definitely not able to put
in time for upstream work.
-Daniel
> Thanks!
>
> > v2:
> > - Drop a few more unused local variables, somehow I missed the
> > compiler warnings (Sam)
> > - Fix typo in comment (Jiri)
> > - add a todo entry for the cleanup (Thomas)
> >
> > v3: Remove more unused variables (0day)
> >
> > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Cc: nouveau@lists.freedesktop.org
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > Cc: Peilin Ye <yepeilin.cs@gmail.com>
> > Cc: George Kennedy <george.kennedy@oracle.com>
> > Cc: Nathan Chancellor <natechancellor@gmail.com>
> > Cc: Peter Rosin <peda@axentia.se>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > Documentation/gpu/todo.rst | 18 +++++++++++++
> > drivers/video/fbdev/core/fbcon.c | 45 ++++++--------------------------
> > 2 files changed, 26 insertions(+), 37 deletions(-)
> >
> > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> > index 6b224ef14455..bec99341a904 100644
> > --- a/Documentation/gpu/todo.rst
> > +++ b/Documentation/gpu/todo.rst
> > @@ -277,6 +277,24 @@ 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 cef437817b0d..8d1ae973041a 100644
> > --- a/drivers/video/fbdev/core/fbcon.c
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -1033,7 +1033,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, charcnt = 256;
> > - int cap, ret;
> > + int ret;
> >
> > if (WARN_ON(info_idx == -1))
> > return;
> > @@ -1042,7 +1042,6 @@ 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;
> > @@ -1147,11 +1146,13 @@ static void fbcon_init(struct vc_data *vc, int init)
> >
> > ops->graphics = 0;
> >
> > - if ((cap & FBINFO_HWACCEL_COPYAREA) &&
> > - !(cap & FBINFO_HWACCEL_DISABLED))
> > - p->scrollmode = SCROLL_MOVE;
> > - else /* default to something safe */
> > - p->scrollmode = SCROLL_REDRAW;
> > + /*
> > + * 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
> > @@ -1961,45 +1962,15 @@ 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.28.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-31 14:17 ` Daniel Vetter
@ 2020-11-18 9:21 ` Geert Uytterhoeven
0 siblings, 0 replies; 27+ messages in thread
From: Geert Uytterhoeven @ 2020-11-18 9:21 UTC (permalink / raw)
To: Daniel Vetter
Cc: Linux Fbdev development list, Jiri Slaby,
Bartlomiej Zolnierkiewicz, Tetsuo Handa, Linus Torvalds,
Intel Graphics Development, Gustavo A. R. Silva, DRI Development,
Peilin Ye, George Kennedy, Tomi Valkeinen, Ben Skeggs,
Thomas Zimmermann, Greg Kroah-Hartman, Nouveau Dev,
Daniel Vetter, Nathan Chancellor, Sam Ravnborg, Peter Rosin
Hi Daniel,
Replying "early" (see below), as this was applied to
drm-misc/for-linux-next.
On Sat, Oct 31, 2020 at 3:17 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Sat, Oct 31, 2020 at 11:28 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Thu, 29 Oct 2020, Daniel Vetter wrote:
> > > So ever since syzbot discovered fbcon, we have solid proof that it's
> > > full of bugs. And often the solution is to just delete code and remove
> > > features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
> > >
> > > Now the problem is that most modern-ish drivers really only treat
> > > fbcon as an dumb kernel console until userspace takes over, and Oops
> > > printer for some emergencies. Looking at drm drivers and the basic
> > > vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> > > acceleration:
> > >
> > > - nouveau, seems to be enabled by default
> > > - omapdrm, when a DMM remapper exists using remapper rewriting for
> > > y/xpanning
> > > - gma500, but that is getting deleted now for the GTT remapper trick,
> > > and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> > > flag, so unused (and could be deleted already I think).
> > >
> > > No other driver supportes accelerated fbcon. And fbcon is the only
> > > user of this accel code (it's not exposed as uapi through ioctls),
> > > which means we could garbage collect fairly enormous amounts of code
> > > if we kill this.
> >
> > "git grep FBINFO_HWACCEL_COPYAREA" shows me there are 32 more drivers
> > using acceleration under drivers/video/fbdev/.
> >
> > > Plus because syzbot only runs on virtual hardware, and none of the
> > > drivers for that have acceleration, we'd remove a huge gap in testing.
> > > And there's no other even remotely comprehensive testing aside from
> > > syzbot.
> >
> > That sounds like a great argument to remove all hardware drivers from
> > the kernel ;-)
>
> fbdev is unmaintained, has no one volunteering to put in the work (and
> there's huge amounts of work needed), and there's no test suite. No,
> fbtest.c doesn't can't, that's not even close. We're not going to
> delete everything in the kernel, but slowly sunsetting stuff that's
> just costing and not bringing in up is a good idea.
The fbcon acceleration code is indeed not tested by fbset, and it is
purely in-kernel acceleration for the console.
> > Seriously, how hard can it be to add "software-accelerated" acceleration
> > hooks to drivers/video/fbdev/vfb.c, to enable syzbot to exercise the
> > core acceleration code paths?
>
> Just this one is 5 combinations, which means I'd need to convince
> syzbot to test 5 different machine setups.
Why 5 combinations?
Enable vfb (which can be a module) and be done with it?
> Plus we're still lacking a test suite, and judging from how much time
> it took to get something basic going for kms, that's about 2 engineer
> years of effort that no one is even close to willing to spend.
Sure, writing test suites is hard, and takes time.
> > > This patch here just disables the acceleration code by always
> > > redrawing when scrolling. The plan is that once this has been merged
> > > for well over a year in released kernels, we can start to go around
> > > and delete a lot of code.
> >
> > Have you benchmarked the performance impact on traditional fbdev
> > drivers?
>
> There's still some acceleration if you have an image blit engine for
> redrawing the screen. But the complexity is contained in the old
> drivers that no one cares about.
>
> For anything I have access to the difference is 0.
Sure, you're doing DRM drivers ;-)
> Reality is that fbdev is just there nowadays for Oops printing and
> emergency usage, and it's plenty good enough for that. If there's
That's true for systems that are supported by a DRM driver.
> anyone who cares beyond that, they're most definitely not able to put
> in time for upstream work.
There exist actual products using out-of-tree fbdev drivers that never
got the chance of being merged upstream due to the moratorium on new
fbdev drivers.
BTW, I'm trying to convert an old fbdev driver to DRM, but don't have it
working yet. I had hoped to get something working before replying to
this email, so I could provide more detailed feedback.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] fbcon: Disable accelerated scrolling
2020-10-29 10:14 [PATCH 1/3] fbcon: Disable accelerated scrolling Daniel Vetter
` (3 preceding siblings ...)
2020-10-29 13:22 ` [PATCH] " Daniel Vetter
@ 2020-10-29 19:17 ` Thomas Zimmermann
4 siblings, 0 replies; 27+ messages in thread
From: Thomas Zimmermann @ 2020-10-29 19:17 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: Jiri Slaby, Bartlomiej Zolnierkiewicz, Tetsuo Handa,
Linus Torvalds, Intel Graphics Development, Gustavo A. R. Silva,
Peilin Ye, George Kennedy, Tomi Valkeinen, Ben Skeggs,
Greg Kroah-Hartman, nouveau, Daniel Vetter, Nathan Chancellor,
Sam Ravnborg, Peter Rosin
[-- Attachment #1.1.1.1: Type: text/plain, Size: 6791 bytes --]
Am 29.10.20 um 11:14 schrieb Daniel Vetter:
> So ever since syzbot discovered fbcon, we have solid proof that it's
> full of bugs. And often the solution is to just delete code and remove
> features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
>
> Now the problem is that most modern-ish drivers really only treat
> fbcon as an dumb kernel console until userspace takes over, and Oops
> printer for some emergencies. Looking at drm drivers and the basic
> vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> acceleration:
>
> - nouveau, seems to be enabled by default
> - omapdrm, when a DMM remapper exists using remapper rewriting for
> y/xpanning
> - gma500, but that is getting deleted now for the GTT remapper trick,
> and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> flag, so unused (and could be deleted already I think).
>
> No other driver supportes accelerated fbcon. And fbcon is the only
> user of this accel code (it's not exposed as uapi through ioctls),
> which means we could garbage collect fairly enormous amounts of code
> if we kill this.
>
> Plus because syzbot only runs on virtual hardware, and none of the
> drivers for that have acceleration, we'd remove a huge gap in testing.
> And there's no other even remotely comprehensive testing aside from
> syzbot.
>
> This patch here just disables the acceleration code by always
> redrawing when scrolling. The plan is that once this has been merged
> for well over a year in released kernels, we can start to go around
> and delete a lot of code.
>
> v2:
> - Drop a few more unused local variables, somehow I missed the
> compiler warnings (Sam)
> - Fix typo in comment (Jiri)
> - add a todo entry for the cleanup (Thomas)
Thanks :)
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Peilin Ye <yepeilin.cs@gmail.com>
> Cc: George Kennedy <george.kennedy@oracle.com>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Peter Rosin <peda@axentia.se>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> Documentation/gpu/todo.rst | 18 ++++++++++++++
> drivers/video/fbdev/core/fbcon.c | 42 ++++++--------------------------
> 2 files changed, 25 insertions(+), 35 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 6b224ef14455..bec99341a904 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -277,6 +277,24 @@ 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 cef437817b0d..a68253485244 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -1147,11 +1147,13 @@ static void fbcon_init(struct vc_data *vc, int init)
>
> ops->graphics = 0;
>
> - if ((cap & FBINFO_HWACCEL_COPYAREA) &&
> - !(cap & FBINFO_HWACCEL_DISABLED))
> - p->scrollmode = SCROLL_MOVE;
> - else /* default to something safe */
> - p->scrollmode = SCROLL_REDRAW;
> + /*
> + * 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
> @@ -1961,45 +1963,15 @@ 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)
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.1.1.2: OpenPGP_0x680DC11D530B7A23.asc --]
[-- Type: application/pgp-keys, Size: 4259 bytes --]
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH] fbcon: Disable accelerated scrolling
@ 2020-10-28 16:06 Daniel Vetter
2020-10-28 16:45 ` Sam Ravnborg
` (4 more replies)
0 siblings, 5 replies; 27+ messages in thread
From: Daniel Vetter @ 2020-10-28 16:06 UTC (permalink / raw)
To: DRI Development
Cc: Jiri Slaby, Greg Kroah-Hartman, Tetsuo Handa, nouveau,
Bartlomiej Zolnierkiewicz, Gustavo A. R. Silva, Peter Rosin,
George Kennedy, Tomi Valkeinen, Ben Skeggs, Daniel Vetter,
Daniel Vetter, Nathan Chancellor, Linus Torvalds, Peilin Ye
So ever since syzbot discovered fbcon, we have solid proof that it's
full of bugs. And often the solution is to just delete code and remove
features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
Now the problem is that most modern-ish drivers really only treat
fbcon as an dumb kernel console until userspace takes over, and Oops
printer for some emergencies. Looking at drm drivers and the basic
vesa/efi fbdev drivers shows that only 3 drivers support any kind of
acceleration:
- nouveau, seems to be enabled by default
- omapdrm, when a DMM remapper exists using remapper rewriting for
y/xpanning
- gma500, but that is getting deleted now for the GTT remapper trick,
and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
flag, so unused (and could be deleted already I think).
No other driver supportes accelerated fbcon. And fbcon is the only
user of this accel code (it's not exposed as uapi through ioctls),
which means we could garbage collect fairly enormous amounts of code
if we kill this.
Plus because syzbot only runs on virtual hardware, and none of the
drivers for that have acceleration, we'd remove a huge gap in testing.
And there's no other even remotely comprehensive testing aside from
syzbot.
This patch here just disables the acceleration code by always
redrawing when scrolling. The plan is that once this has been merged
for well over a year in released kernels, we can start to go around
and delete a lot of code.
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Peilin Ye <yepeilin.cs@gmail.com>
Cc: George Kennedy <george.kennedy@oracle.com>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Peter Rosin <peda@axentia.se>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/video/fbdev/core/fbcon.c | 38 ++++++--------------------------
1 file changed, 7 insertions(+), 31 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index cef437817b0d..d74ccbbb29bb 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1147,11 +1147,13 @@ static void fbcon_init(struct vc_data *vc, int init)
ops->graphics = 0;
- if ((cap & FBINFO_HWACCEL_COPYAREA) &&
- !(cap & FBINFO_HWACCEL_DISABLED))
- p->scrollmode = SCROLL_MOVE;
- else /* default to something safe */
- p->scrollmode = SCROLL_REDRAW;
+ /*
+ * No more hw acceleration for fbcon.
+ *
+ * FIXME: Garabge collect all the now dead code after sufficient time
+ * has passed.
+ */
+ p->scrollmode = SCROLL_REDRAW;
/*
* ++guenther: console.c:vc_allocate() relies on initializing
@@ -1961,7 +1963,6 @@ 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);
@@ -1969,37 +1970,12 @@ static void updatescrollmode(struct fbcon_display *p,
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.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-28 16:06 [PATCH] " Daniel Vetter
@ 2020-10-28 16:45 ` Sam Ravnborg
2020-10-28 16:48 ` Daniel Vetter
2020-10-28 16:57 ` Greg Kroah-Hartman
` (3 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: Sam Ravnborg @ 2020-10-28 16:45 UTC (permalink / raw)
To: Daniel Vetter
Cc: Bartlomiej Zolnierkiewicz, Tetsuo Handa, Greg Kroah-Hartman,
Linus Torvalds, Gustavo A. R. Silva, DRI Development, Peilin Ye,
George Kennedy, Tomi Valkeinen, Ben Skeggs, nouveau,
Daniel Vetter, Nathan Chancellor, Jiri Slaby, Peter Rosin
Hi Daniel et al.
On Wed, Oct 28, 2020 at 05:06:00PM +0100, Daniel Vetter wrote:
> So ever since syzbot discovered fbcon, we have solid proof that it's
> full of bugs. And often the solution is to just delete code and remove
> features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
>
> Now the problem is that most modern-ish drivers really only treat
> fbcon as an dumb kernel console until userspace takes over, and Oops
> printer for some emergencies. Looking at drm drivers and the basic
> vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> acceleration:
>
> - nouveau, seems to be enabled by default
> - omapdrm, when a DMM remapper exists using remapper rewriting for
> y/xpanning
> - gma500, but that is getting deleted now for the GTT remapper trick,
> and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> flag, so unused (and could be deleted already I think).
>
> No other driver supportes accelerated fbcon. And fbcon is the only
> user of this accel code (it's not exposed as uapi through ioctls),
> which means we could garbage collect fairly enormous amounts of code
> if we kill this.
>
> Plus because syzbot only runs on virtual hardware, and none of the
> drivers for that have acceleration, we'd remove a huge gap in testing.
> And there's no other even remotely comprehensive testing aside from
> syzbot.
>
> This patch here just disables the acceleration code by always
> redrawing when scrolling.
So far I follow you - and agree.
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> The plan is that once this has been merged
> for well over a year in released kernels, we can start to go around
> and delete a lot of code.
Why wait one year? We deleted the scrollback code without any prior
warning - which was fine. And acceleration support has less users
so there should be no reason to wait.
So unless there are good arguments that I miss then we should just
delete the acceleration code outright.
Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-28 16:45 ` Sam Ravnborg
@ 2020-10-28 16:48 ` Daniel Vetter
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2020-10-28 16:48 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Bartlomiej Zolnierkiewicz, Tetsuo Handa, Greg Kroah-Hartman,
Linus Torvalds, Gustavo A. R. Silva, DRI Development, Peilin Ye,
George Kennedy, Tomi Valkeinen, Ben Skeggs, Nouveau Dev,
Daniel Vetter, Nathan Chancellor, Jiri Slaby, Peter Rosin
On Wed, Oct 28, 2020 at 5:45 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Daniel et al.
>
> On Wed, Oct 28, 2020 at 05:06:00PM +0100, Daniel Vetter wrote:
> > So ever since syzbot discovered fbcon, we have solid proof that it's
> > full of bugs. And often the solution is to just delete code and remove
> > features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
> >
> > Now the problem is that most modern-ish drivers really only treat
> > fbcon as an dumb kernel console until userspace takes over, and Oops
> > printer for some emergencies. Looking at drm drivers and the basic
> > vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> > acceleration:
> >
> > - nouveau, seems to be enabled by default
> > - omapdrm, when a DMM remapper exists using remapper rewriting for
> > y/xpanning
> > - gma500, but that is getting deleted now for the GTT remapper trick,
> > and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> > flag, so unused (and could be deleted already I think).
> >
> > No other driver supportes accelerated fbcon. And fbcon is the only
> > user of this accel code (it's not exposed as uapi through ioctls),
> > which means we could garbage collect fairly enormous amounts of code
> > if we kill this.
> >
> > Plus because syzbot only runs on virtual hardware, and none of the
> > drivers for that have acceleration, we'd remove a huge gap in testing.
> > And there's no other even remotely comprehensive testing aside from
> > syzbot.
> >
> > This patch here just disables the acceleration code by always
> > redrawing when scrolling.
>
> So far I follow you - and agree.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> > The plan is that once this has been merged
> > for well over a year in released kernels, we can start to go around
> > and delete a lot of code.
>
> Why wait one year? We deleted the scrollback code without any prior
> warning - which was fine. And acceleration support has less users
> so there should be no reason to wait.
>
> So unless there are good arguments that I miss then we should just
> delete the acceleration code outright.
If you grep for FBINFO_HWACCEL and related stuff, we could delete like
half the driver code, plus a ton of the related support code in fbcon
and fbdev core. It's going to be a lot of work, and I don't want to do
that and then have to back it out again. Compared to this the
softscrollback removal was nothing.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-28 16:06 [PATCH] " Daniel Vetter
2020-10-28 16:45 ` Sam Ravnborg
@ 2020-10-28 16:57 ` Greg Kroah-Hartman
2020-10-28 18:50 ` Sam Ravnborg
` (2 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-28 16:57 UTC (permalink / raw)
To: Daniel Vetter
Cc: Jiri Slaby, Bartlomiej Zolnierkiewicz, Tetsuo Handa, nouveau,
Gustavo A. R. Silva, DRI Development, Peter Rosin,
George Kennedy, Tomi Valkeinen, Ben Skeggs, Daniel Vetter,
Nathan Chancellor, Linus Torvalds, Peilin Ye
On Wed, Oct 28, 2020 at 05:06:00PM +0100, Daniel Vetter wrote:
> So ever since syzbot discovered fbcon, we have solid proof that it's
> full of bugs. And often the solution is to just delete code and remove
> features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
>
> Now the problem is that most modern-ish drivers really only treat
> fbcon as an dumb kernel console until userspace takes over, and Oops
> printer for some emergencies. Looking at drm drivers and the basic
> vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> acceleration:
>
> - nouveau, seems to be enabled by default
> - omapdrm, when a DMM remapper exists using remapper rewriting for
> y/xpanning
> - gma500, but that is getting deleted now for the GTT remapper trick,
> and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> flag, so unused (and could be deleted already I think).
>
> No other driver supportes accelerated fbcon. And fbcon is the only
> user of this accel code (it's not exposed as uapi through ioctls),
> which means we could garbage collect fairly enormous amounts of code
> if we kill this.
>
> Plus because syzbot only runs on virtual hardware, and none of the
> drivers for that have acceleration, we'd remove a huge gap in testing.
> And there's no other even remotely comprehensive testing aside from
> syzbot.
>
> This patch here just disables the acceleration code by always
> redrawing when scrolling. The plan is that once this has been merged
> for well over a year in released kernels, we can start to go around
> and delete a lot of code.
>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Peilin Ye <yepeilin.cs@gmail.com>
> Cc: George Kennedy <george.kennedy@oracle.com>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Peter Rosin <peda@axentia.se>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/video/fbdev/core/fbcon.c | 38 ++++++--------------------------
> 1 file changed, 7 insertions(+), 31 deletions(-)
Nice!
But I'm with Sam, delete early :)
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-28 16:06 [PATCH] " Daniel Vetter
2020-10-28 16:45 ` Sam Ravnborg
2020-10-28 16:57 ` Greg Kroah-Hartman
@ 2020-10-28 18:50 ` Sam Ravnborg
2020-10-28 19:57 ` Daniel Vetter
2020-10-28 19:02 ` Thomas Zimmermann
2020-10-29 5:42 ` Jiri Slaby
4 siblings, 1 reply; 27+ messages in thread
From: Sam Ravnborg @ 2020-10-28 18:50 UTC (permalink / raw)
To: Daniel Vetter
Cc: Bartlomiej Zolnierkiewicz, Tetsuo Handa, Greg Kroah-Hartman,
Linus Torvalds, Gustavo A. R. Silva, DRI Development, Peilin Ye,
George Kennedy, Tomi Valkeinen, Ben Skeggs, nouveau,
Daniel Vetter, Nathan Chancellor, Jiri Slaby, Peter Rosin
Hi Daniel.
On Wed, Oct 28, 2020 at 05:06:00PM +0100, Daniel Vetter wrote:
> So ever since syzbot discovered fbcon, we have solid proof that it's
> full of bugs. And often the solution is to just delete code and remove
> features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
>
> Now the problem is that most modern-ish drivers really only treat
> fbcon as an dumb kernel console until userspace takes over, and Oops
> printer for some emergencies. Looking at drm drivers and the basic
> vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> acceleration:
>
> - nouveau, seems to be enabled by default
> - omapdrm, when a DMM remapper exists using remapper rewriting for
> y/xpanning
> - gma500, but that is getting deleted now for the GTT remapper trick,
> and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> flag, so unused (and could be deleted already I think).
>
> No other driver supportes accelerated fbcon. And fbcon is the only
> user of this accel code (it's not exposed as uapi through ioctls),
> which means we could garbage collect fairly enormous amounts of code
> if we kill this.
>
> Plus because syzbot only runs on virtual hardware, and none of the
> drivers for that have acceleration, we'd remove a huge gap in testing.
> And there's no other even remotely comprehensive testing aside from
> syzbot.
>
> This patch here just disables the acceleration code by always
> redrawing when scrolling. The plan is that once this has been merged
> for well over a year in released kernels, we can start to go around
> and delete a lot of code.
See below for a warning fix.
Some figures from trying to toss accel code out from a few fbdev drivers:
drivers/video/fbdev/cirrusfb.c | 300 +----------------------------------------
1 file changed, 4 insertions(+), 296 deletions(-)
drivers/video/fbdev/aty/radeon_accel.c | 174 ---------------------------------
drivers/video/fbdev/aty/radeon_base.c | 43 ++------
drivers/video/fbdev/aty/radeon_pm.c | 7 --
drivers/video/fbdev/aty/radeonfb.h | 3 -
4 files changed, 7 insertions(+), 220 deletions(-)
This may open up the discussion if the right course of action would be
to drop the drivers in favour of drm counterparts - but thats another
story.
Sam
> @@ -1961,7 +1963,6 @@ 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);
> @@ -1969,37 +1970,12 @@ static void updatescrollmode(struct fbcon_display *p,
> 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);
Some bot will likely tell you that this causes warnings.
At least it did in my sparc64 build.
Fix:
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 398914e035e9..e8b009c621d8 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2150,10 +2150,6 @@ static void updatescrollmode(struct fbcon_display *p,
{
struct fbcon_ops *ops = info->fbcon_par;
int fh = vc->vc_font.height;
- 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);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-28 18:50 ` Sam Ravnborg
@ 2020-10-28 19:57 ` Daniel Vetter
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2020-10-28 19:57 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Bartlomiej Zolnierkiewicz, Tetsuo Handa, Greg Kroah-Hartman,
Linus Torvalds, Gustavo A. R. Silva, DRI Development, Peilin Ye,
George Kennedy, Tomi Valkeinen, Ben Skeggs, Nouveau Dev,
Daniel Vetter, Nathan Chancellor, Jiri Slaby, Peter Rosin
On Wed, Oct 28, 2020 at 7:50 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Daniel.
>
> On Wed, Oct 28, 2020 at 05:06:00PM +0100, Daniel Vetter wrote:
> > So ever since syzbot discovered fbcon, we have solid proof that it's
> > full of bugs. And often the solution is to just delete code and remove
> > features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
> >
> > Now the problem is that most modern-ish drivers really only treat
> > fbcon as an dumb kernel console until userspace takes over, and Oops
> > printer for some emergencies. Looking at drm drivers and the basic
> > vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> > acceleration:
> >
> > - nouveau, seems to be enabled by default
> > - omapdrm, when a DMM remapper exists using remapper rewriting for
> > y/xpanning
> > - gma500, but that is getting deleted now for the GTT remapper trick,
> > and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> > flag, so unused (and could be deleted already I think).
> >
> > No other driver supportes accelerated fbcon. And fbcon is the only
> > user of this accel code (it's not exposed as uapi through ioctls),
> > which means we could garbage collect fairly enormous amounts of code
> > if we kill this.
> >
> > Plus because syzbot only runs on virtual hardware, and none of the
> > drivers for that have acceleration, we'd remove a huge gap in testing.
> > And there's no other even remotely comprehensive testing aside from
> > syzbot.
> >
> > This patch here just disables the acceleration code by always
> > redrawing when scrolling. The plan is that once this has been merged
> > for well over a year in released kernels, we can start to go around
> > and delete a lot of code.
>
> See below for a warning fix.
>
> Some figures from trying to toss accel code out from a few fbdev drivers:
>
> drivers/video/fbdev/cirrusfb.c | 300 +----------------------------------------
> 1 file changed, 4 insertions(+), 296 deletions(-)
>
> drivers/video/fbdev/aty/radeon_accel.c | 174 ---------------------------------
> drivers/video/fbdev/aty/radeon_base.c | 43 ++------
> drivers/video/fbdev/aty/radeon_pm.c | 7 --
> drivers/video/fbdev/aty/radeonfb.h | 3 -
> 4 files changed, 7 insertions(+), 220 deletions(-)
>
> This may open up the discussion if the right course of action would be
> to drop the drivers in favour of drm counterparts - but thats another
> story.
Yeah I think we can start deleting drivers for which we have drm
drivers which are mostly feature parity and see whether anyone pipes
up. There's always going to be the odd corner case (like apparently
the fbdev ati driver works better on some ppc machines than the drm
one).
The thing is, we can't delete the entire accel code with this, I think
only fb_copyarea goes away. The other hooks I think still have some
users.
-Daniel
>
> Sam
>
> > @@ -1961,7 +1963,6 @@ 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);
> > @@ -1969,37 +1970,12 @@ static void updatescrollmode(struct fbcon_display *p,
> > 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);
>
> Some bot will likely tell you that this causes warnings.
> At least it did in my sparc64 build.
>
> Fix:
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 398914e035e9..e8b009c621d8 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -2150,10 +2150,6 @@ static void updatescrollmode(struct fbcon_display *p,
> {
> struct fbcon_ops *ops = info->fbcon_par;
> int fh = vc->vc_font.height;
> - 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);
>
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-28 16:06 [PATCH] " Daniel Vetter
` (2 preceding siblings ...)
2020-10-28 18:50 ` Sam Ravnborg
@ 2020-10-28 19:02 ` Thomas Zimmermann
2020-10-28 19:55 ` Daniel Vetter
2020-10-29 5:42 ` Jiri Slaby
4 siblings, 1 reply; 27+ messages in thread
From: Thomas Zimmermann @ 2020-10-28 19:02 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: Bartlomiej Zolnierkiewicz, Tetsuo Handa, Greg Kroah-Hartman,
Linus Torvalds, Gustavo A. R. Silva, Peilin Ye, George Kennedy,
Tomi Valkeinen, Ben Skeggs, nouveau, Daniel Vetter,
Nathan Chancellor, Jiri Slaby, Peter Rosin
Hi
Am 28.10.20 um 17:06 schrieb Daniel Vetter:
> So ever since syzbot discovered fbcon, we have solid proof that it's
> full of bugs. And often the solution is to just delete code and remove
> features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
>
> Now the problem is that most modern-ish drivers really only treat
> fbcon as an dumb kernel console until userspace takes over, and Oops
> printer for some emergencies. Looking at drm drivers and the basic
> vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> acceleration:
>
> - nouveau, seems to be enabled by default
> - omapdrm, when a DMM remapper exists using remapper rewriting for
> y/xpanning
> - gma500, but that is getting deleted now for the GTT remapper trick,
> and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> flag, so unused (and could be deleted already I think).
>
> No other driver supportes accelerated fbcon. And fbcon is the only
> user of this accel code (it's not exposed as uapi through ioctls),
> which means we could garbage collect fairly enormous amounts of code
> if we kill this.
>
> Plus because syzbot only runs on virtual hardware, and none of the
> drivers for that have acceleration, we'd remove a huge gap in testing.
> And there's no other even remotely comprehensive testing aside from
> syzbot.
>
> This patch here just disables the acceleration code by always
> redrawing when scrolling. The plan is that once this has been merged
> for well over a year in released kernels, we can start to go around
> and delete a lot of code.
Why wait a year? I'd say delete early, delete often. ;)
>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Peilin Ye <yepeilin.cs@gmail.com>
> Cc: George Kennedy <george.kennedy@oracle.com>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Peter Rosin <peda@axentia.se>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/video/fbdev/core/fbcon.c | 38 ++++++--------------------------
> 1 file changed, 7 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index cef437817b0d..d74ccbbb29bb 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -1147,11 +1147,13 @@ static void fbcon_init(struct vc_data *vc, int init)
>
> ops->graphics = 0;
>
> - if ((cap & FBINFO_HWACCEL_COPYAREA) &&
> - !(cap & FBINFO_HWACCEL_DISABLED))
> - p->scrollmode = SCROLL_MOVE;
> - else /* default to something safe */
> - p->scrollmode = SCROLL_REDRAW;
> + /*
> + * No more hw acceleration for fbcon.
> + *
> + * FIXME: Garabge collect all the now dead code after sufficient time
> + * has passed.
> + */
> + p->scrollmode = SCROLL_REDRAW;
I just grepped for scrollmode and there aren't many places that use it.
Could you remove it as well?
In any case
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Best regards
Thomas
>
> /*
> * ++guenther: console.c:vc_allocate() relies on initializing
> @@ -1961,7 +1963,6 @@ 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);
> @@ -1969,37 +1970,12 @@ static void updatescrollmode(struct fbcon_display *p,
> 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)
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-28 19:02 ` Thomas Zimmermann
@ 2020-10-28 19:55 ` Daniel Vetter
2020-10-29 8:07 ` Thomas Zimmermann
0 siblings, 1 reply; 27+ messages in thread
From: Daniel Vetter @ 2020-10-28 19:55 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Bartlomiej Zolnierkiewicz, Tetsuo Handa, Greg Kroah-Hartman,
Linus Torvalds, Gustavo A. R. Silva, DRI Development, Peilin Ye,
George Kennedy, Tomi Valkeinen, Ben Skeggs, Nouveau Dev,
Daniel Vetter, Nathan Chancellor, Jiri Slaby, Peter Rosin
On Wed, Oct 28, 2020 at 8:02 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 28.10.20 um 17:06 schrieb Daniel Vetter:
> > So ever since syzbot discovered fbcon, we have solid proof that it's
> > full of bugs. And often the solution is to just delete code and remove
> > features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
> >
> > Now the problem is that most modern-ish drivers really only treat
> > fbcon as an dumb kernel console until userspace takes over, and Oops
> > printer for some emergencies. Looking at drm drivers and the basic
> > vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> > acceleration:
> >
> > - nouveau, seems to be enabled by default
> > - omapdrm, when a DMM remapper exists using remapper rewriting for
> > y/xpanning
> > - gma500, but that is getting deleted now for the GTT remapper trick,
> > and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> > flag, so unused (and could be deleted already I think).
> >
> > No other driver supportes accelerated fbcon. And fbcon is the only
> > user of this accel code (it's not exposed as uapi through ioctls),
> > which means we could garbage collect fairly enormous amounts of code
> > if we kill this.
> >
> > Plus because syzbot only runs on virtual hardware, and none of the
> > drivers for that have acceleration, we'd remove a huge gap in testing.
> > And there's no other even remotely comprehensive testing aside from
> > syzbot.
> >
> > This patch here just disables the acceleration code by always
> > redrawing when scrolling. The plan is that once this has been merged
> > for well over a year in released kernels, we can start to go around
> > and delete a lot of code.
>
> Why wait a year? I'd say delete early, delete often. ;)
>
> >
> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Cc: nouveau@lists.freedesktop.org
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > Cc: Peilin Ye <yepeilin.cs@gmail.com>
> > Cc: George Kennedy <george.kennedy@oracle.com>
> > Cc: Nathan Chancellor <natechancellor@gmail.com>
> > Cc: Peter Rosin <peda@axentia.se>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > drivers/video/fbdev/core/fbcon.c | 38 ++++++--------------------------
> > 1 file changed, 7 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> > index cef437817b0d..d74ccbbb29bb 100644
> > --- a/drivers/video/fbdev/core/fbcon.c
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -1147,11 +1147,13 @@ static void fbcon_init(struct vc_data *vc, int init)
> >
> > ops->graphics = 0;
> >
> > - if ((cap & FBINFO_HWACCEL_COPYAREA) &&
> > - !(cap & FBINFO_HWACCEL_DISABLED))
> > - p->scrollmode = SCROLL_MOVE;
> > - else /* default to something safe */
> > - p->scrollmode = SCROLL_REDRAW;
> > + /*
> > + * No more hw acceleration for fbcon.
> > + *
> > + * FIXME: Garabge collect all the now dead code after sufficient time
> > + * has passed.
> > + */
> > + p->scrollmode = SCROLL_REDRAW;
>
> I just grepped for scrollmode and there aren't many places that use it.
> Could you remove it as well?
Removing scrollmode will start the delete feast. In fbcon alone I
think we can drop half the code.
-Daniel
>
> In any case
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> Best regards
> Thomas
>
> >
> > /*
> > * ++guenther: console.c:vc_allocate() relies on initializing
> > @@ -1961,7 +1963,6 @@ 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);
> > @@ -1969,37 +1970,12 @@ static void updatescrollmode(struct fbcon_display *p,
> > 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)
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-28 19:55 ` Daniel Vetter
@ 2020-10-29 8:07 ` Thomas Zimmermann
0 siblings, 0 replies; 27+ messages in thread
From: Thomas Zimmermann @ 2020-10-29 8:07 UTC (permalink / raw)
To: Daniel Vetter
Cc: Jiri Slaby, Bartlomiej Zolnierkiewicz, Tetsuo Handa,
Greg Kroah-Hartman, Gustavo A. R. Silva, DRI Development,
Peter Rosin, George Kennedy, Tomi Valkeinen, Ben Skeggs,
Nouveau Dev, Daniel Vetter, Nathan Chancellor, Linus Torvalds,
Peilin Ye
Hi
Am 28.10.20 um 20:55 schrieb Daniel Vetter:
> On Wed, Oct 28, 2020 at 8:02 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 28.10.20 um 17:06 schrieb Daniel Vetter:
>>> So ever since syzbot discovered fbcon, we have solid proof that it's
>>> full of bugs. And often the solution is to just delete code and remove
>>> features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
>>>
>>> Now the problem is that most modern-ish drivers really only treat
>>> fbcon as an dumb kernel console until userspace takes over, and Oops
>>> printer for some emergencies. Looking at drm drivers and the basic
>>> vesa/efi fbdev drivers shows that only 3 drivers support any kind of
>>> acceleration:
>>>
>>> - nouveau, seems to be enabled by default
>>> - omapdrm, when a DMM remapper exists using remapper rewriting for
>>> y/xpanning
>>> - gma500, but that is getting deleted now for the GTT remapper trick,
>>> and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
>>> flag, so unused (and could be deleted already I think).
>>>
>>> No other driver supportes accelerated fbcon. And fbcon is the only
>>> user of this accel code (it's not exposed as uapi through ioctls),
>>> which means we could garbage collect fairly enormous amounts of code
>>> if we kill this.
>>>
>>> Plus because syzbot only runs on virtual hardware, and none of the
>>> drivers for that have acceleration, we'd remove a huge gap in testing.
>>> And there's no other even remotely comprehensive testing aside from
>>> syzbot.
>>>
>>> This patch here just disables the acceleration code by always
>>> redrawing when scrolling. The plan is that once this has been merged
>>> for well over a year in released kernels, we can start to go around
>>> and delete a lot of code.
>>
>> Why wait a year? I'd say delete early, delete often. ;)
>>
>>>
>>> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>>> Cc: Ben Skeggs <bskeggs@redhat.com>
>>> Cc: nouveau@lists.freedesktop.org
>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: Jiri Slaby <jirislaby@kernel.org>
>>> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
>>> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>>> Cc: Peilin Ye <yepeilin.cs@gmail.com>
>>> Cc: George Kennedy <george.kennedy@oracle.com>
>>> Cc: Nathan Chancellor <natechancellor@gmail.com>
>>> Cc: Peter Rosin <peda@axentia.se>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> ---
>>> drivers/video/fbdev/core/fbcon.c | 38 ++++++--------------------------
>>> 1 file changed, 7 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
>>> index cef437817b0d..d74ccbbb29bb 100644
>>> --- a/drivers/video/fbdev/core/fbcon.c
>>> +++ b/drivers/video/fbdev/core/fbcon.c
>>> @@ -1147,11 +1147,13 @@ static void fbcon_init(struct vc_data *vc, int init)
>>>
>>> ops->graphics = 0;
>>>
>>> - if ((cap & FBINFO_HWACCEL_COPYAREA) &&
>>> - !(cap & FBINFO_HWACCEL_DISABLED))
>>> - p->scrollmode = SCROLL_MOVE;
>>> - else /* default to something safe */
>>> - p->scrollmode = SCROLL_REDRAW;
>>> + /*
>>> + * No more hw acceleration for fbcon.
>>> + *
>>> + * FIXME: Garabge collect all the now dead code after sufficient time
>>> + * has passed.
>>> + */
>>> + p->scrollmode = SCROLL_REDRAW;
>>
>> I just grepped for scrollmode and there aren't many places that use it.
>> Could you remove it as well?
>
> Removing scrollmode will start the delete feast. In fbcon alone I
> think we can drop half the code.
That really deserves a TODO item then IMHO.
Best regards
Thomas
> -Daniel
>
>>
>> In any case
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>
>> Best regards
>> Thomas
>>
>>>
>>> /*
>>> * ++guenther: console.c:vc_allocate() relies on initializing
>>> @@ -1961,7 +1963,6 @@ 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);
>>> @@ -1969,37 +1970,12 @@ static void updatescrollmode(struct fbcon_display *p,
>>> 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)
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] fbcon: Disable accelerated scrolling
2020-10-28 16:06 [PATCH] " Daniel Vetter
` (3 preceding siblings ...)
2020-10-28 19:02 ` Thomas Zimmermann
@ 2020-10-29 5:42 ` Jiri Slaby
4 siblings, 0 replies; 27+ messages in thread
From: Jiri Slaby @ 2020-10-29 5:42 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: Bartlomiej Zolnierkiewicz, Tetsuo Handa, nouveau,
Gustavo A. R. Silva, Peter Rosin, George Kennedy, Tomi Valkeinen,
Ben Skeggs, Greg Kroah-Hartman, Daniel Vetter, Nathan Chancellor,
Linus Torvalds, Peilin Ye
On 28. 10. 20, 17:06, Daniel Vetter wrote:
> So ever since syzbot discovered fbcon, we have solid proof that it's
> full of bugs. And often the solution is to just delete code and remove
> features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
...
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -1147,11 +1147,13 @@ static void fbcon_init(struct vc_data *vc, int init)
>
> ops->graphics = 0;
>
> - if ((cap & FBINFO_HWACCEL_COPYAREA) &&
> - !(cap & FBINFO_HWACCEL_DISABLED))
> - p->scrollmode = SCROLL_MOVE;
> - else /* default to something safe */
> - p->scrollmode = SCROLL_REDRAW;
> + /*
> + * No more hw acceleration for fbcon.
> + *
> + * FIXME: Garabge collect all the now dead code after sufficient time
If you go this non-invasive path, then only a nit here: "Garbage"
thanks,
--
js
suse labs
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2020-11-18 9:21 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-29 10:14 [PATCH 1/3] fbcon: Disable accelerated scrolling Daniel Vetter
2020-10-29 10:14 ` [PATCH 2/3] fbcon: Drop EXPORT_SYMBOL Daniel Vetter
2020-10-29 19:15 ` Thomas Zimmermann
2020-10-29 10:14 ` [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers Daniel Vetter
2020-10-29 11:13 ` Gerd Hoffmann
2020-10-29 13:33 ` [PATCH] " Daniel Vetter
2020-11-17 10:01 ` Daniel Vetter
2020-10-29 13:56 ` [PATCH 3/3] " kernel test robot
2020-10-30 0:37 ` kernel test robot
2020-10-29 12:42 ` [Intel-gfx] [PATCH 1/3] fbcon: Disable accelerated scrolling kernel test robot
2020-10-29 13:22 ` [PATCH] " Daniel Vetter
2020-10-30 8:30 ` Tomi Valkeinen
2020-10-30 8:52 ` Daniel Vetter
2020-10-31 10:27 ` Geert Uytterhoeven
2020-10-31 14:17 ` Daniel Vetter
2020-11-18 9:21 ` Geert Uytterhoeven
2020-10-29 19:17 ` [PATCH 1/3] " Thomas Zimmermann
-- strict thread matches above, loose matches on Subject: below --
2020-10-28 16:06 [PATCH] " Daniel Vetter
2020-10-28 16:45 ` Sam Ravnborg
2020-10-28 16:48 ` Daniel Vetter
2020-10-28 16:57 ` Greg Kroah-Hartman
2020-10-28 18:50 ` Sam Ravnborg
2020-10-28 19:57 ` Daniel Vetter
2020-10-28 19:02 ` Thomas Zimmermann
2020-10-28 19:55 ` Daniel Vetter
2020-10-29 8:07 ` Thomas Zimmermann
2020-10-29 5:42 ` Jiri Slaby
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).