| Message ID | 20220201185954.169768-4-deller@gmx.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Fix regression introduced by disabling accelerated scrolling in fbcon | expand |
On Tue, Feb 1, 2022 at 7:59 PM Helge Deller <deller@gmx.de> wrote: > > Add a config option CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION to > enable bitblt and fillrect hardware acceleration in the framebuffer > console. If disabled, such acceleration will not be used, even if it is > supported by the graphics hardware driver. > > If you plan to use DRM as your main graphics output system, you should > disable this option since it will prevent compiling in code which isn't > used later on when DRM takes over. > > For all other configurations, e.g. if none of your graphic cards support > DRM (yet), DRM isn't available for your architecture, or you can't be > sure that the graphic card in the target system will support DRM, you > most likely want to enable this option. > > In the non-accelerated case (e.g. when DRM is used), the inlined > fb_scrollmode() function is hardcoded to return SCROLL_REDRAW and as such the > compiler is able to optimize much unneccesary code away. > > In this v3 patch version I additionally changed the GETVYRES() and GETVXRES() > macros to take a pointer to the fbcon_display struct. This fixes the build when > console rotation is enabled and helps the compiler again to optimize out code. > > Signed-off-by: Helge Deller <deller@gmx.de> > Cc: stable@vger.kernel.org # v5.10+ > Signed-off-by: Helge Deller <deller@gmx.de> > --- > drivers/video/console/Kconfig | 11 +++++++ > drivers/video/fbdev/core/fbcon.c | 39 ++++++++++++++++++------- > drivers/video/fbdev/core/fbcon.h | 15 +++++++++- > drivers/video/fbdev/core/fbcon_ccw.c | 10 +++---- > drivers/video/fbdev/core/fbcon_cw.c | 10 +++---- > drivers/video/fbdev/core/fbcon_rotate.h | 4 +-- > drivers/video/fbdev/core/fbcon_ud.c | 20 ++++++------- > 7 files changed, 75 insertions(+), 34 deletions(-) > > diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig > index 840d9813b0bc..6029fd41d7d0 100644 > --- a/drivers/video/console/Kconfig > +++ b/drivers/video/console/Kconfig > @@ -78,6 +78,17 @@ config FRAMEBUFFER_CONSOLE > help > Low-level framebuffer-based console driver. > > +config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > + bool "Framebuffer Console hardware acceleration support" > + depends on FRAMEBUFFER_CONSOLE > + default n if DRM > + default y > + help > + If you use a system on which DRM is fully supported you usually want to say N, > + otherwise you probably want to enable this option. > + If enabled the framebuffer console will utilize the hardware acceleration > + of your graphics card by using hardware bitblt and fillrect features. This really doesn't have much to do with DRM but more about running questionable code. That's why I went with the generalized stern warning and default n, and explained when it's ok to do this (single user and you care more about fbcon performance than potential issues because you only run trusted stuff with access to your vt and fbdev /dev node). Also you didn't keep any todo entry for removing DRM accel code, and iirc some nouveau folks also complained that they at least once had some kind of accel, so that's another reason to not tie this to DRM. Anyway aside from this looks reasonable, can you pls respin with a more cautious Kconfig text? Thanks, Daniel > + > config FRAMEBUFFER_CONSOLE_DETECT_PRIMARY > bool "Map the console to the primary display device" > depends on FRAMEBUFFER_CONSOLE > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > index b813985f1403..f7b7d35953e8 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -1136,11 +1136,13 @@ static void fbcon_init(struct vc_data *vc, int init) > > ops->graphics = 0; > > +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > if ((cap & FBINFO_HWACCEL_COPYAREA) && > !(cap & FBINFO_HWACCEL_DISABLED)) > p->scrollmode = SCROLL_MOVE; > else /* default to something safe */ > p->scrollmode = SCROLL_REDRAW; > +#endif > > /* > * ++guenther: console.c:vc_allocate() relies on initializing > @@ -1705,7 +1707,7 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int t, unsigned int b, > count = vc->vc_rows; > if (logo_shown >= 0) > goto redraw_up; > - switch (p->scrollmode) { > + switch (fb_scrollmode(p)) { > case SCROLL_MOVE: > fbcon_redraw_blit(vc, info, p, t, b - t - count, > count); > @@ -1795,7 +1797,7 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int t, unsigned int b, > count = vc->vc_rows; > if (logo_shown >= 0) > goto redraw_down; > - switch (p->scrollmode) { > + switch (fb_scrollmode(p)) { > case SCROLL_MOVE: > fbcon_redraw_blit(vc, info, p, b - 1, b - t - count, > -count); > @@ -1946,12 +1948,12 @@ static void fbcon_bmove_rec(struct vc_data *vc, struct fbcon_display *p, int sy, > height, width); > } > > -static void updatescrollmode(struct fbcon_display *p, > +static void updatescrollmode_accel(struct fbcon_display *p, > struct fb_info *info, > struct vc_data *vc) > { > +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > 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, > @@ -1972,12 +1974,6 @@ static void updatescrollmode(struct fbcon_display *p, > 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 ? > @@ -1991,6 +1987,27 @@ static void updatescrollmode(struct fbcon_display *p, > else > p->scrollmode = SCROLL_REDRAW; > } > +#endif > +} > + > +static void updatescrollmode(struct fbcon_display *p, > + struct fb_info *info, > + struct vc_data *vc) > +{ > + struct fbcon_ops *ops = info->fbcon_par; > + int fh = vc->vc_font.height; > + 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); > + > + 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--; > + > + /* update scrollmode in case hardware acceleration is used */ > + updatescrollmode_accel(p, info, vc); > } > > #define PITCH(w) (((w) + 7) >> 3) > @@ -2148,7 +2165,7 @@ static int fbcon_switch(struct vc_data *vc) > > updatescrollmode(p, info, vc); > > - switch (p->scrollmode) { > + switch (fb_scrollmode(p)) { > case SCROLL_WRAP_MOVE: > scrollback_phys_max = p->vrows - vc->vc_rows; > break; > diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h > index 9315b360c898..0f16cbc99e6a 100644 > --- a/drivers/video/fbdev/core/fbcon.h > +++ b/drivers/video/fbdev/core/fbcon.h > @@ -29,7 +29,9 @@ struct fbcon_display { > /* Filled in by the low-level console driver */ > const u_char *fontdata; > int userfont; /* != 0 if fontdata kmalloc()ed */ > - u_short scrollmode; /* Scroll Method */ > +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > + u_short scrollmode; /* Scroll Method, use fb_scrollmode() */ > +#endif > u_short inverse; /* != 0 text black on white as default */ > short yscroll; /* Hardware scrolling */ > int vrows; /* number of virtual rows */ > @@ -208,6 +210,17 @@ static inline int attr_col_ec(int shift, struct vc_data *vc, > #define SCROLL_REDRAW 0x004 > #define SCROLL_PAN_REDRAW 0x005 > > +static inline u_short fb_scrollmode(struct fbcon_display *fb) > +{ > +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > + return fb->scrollmode; > +#else > + /* hardcoded to SCROLL_REDRAW if acceleration was disabled. */ > + return SCROLL_REDRAW; > +#endif > +} > + > + > #ifdef CONFIG_FB_TILEBLITTING > extern void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info); > #endif > diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c > index 9cd2c4b05c32..2789ace79634 100644 > --- a/drivers/video/fbdev/core/fbcon_ccw.c > +++ b/drivers/video/fbdev/core/fbcon_ccw.c > @@ -65,7 +65,7 @@ static void ccw_bmove(struct vc_data *vc, struct fb_info *info, int sy, > { > struct fbcon_ops *ops = info->fbcon_par; > struct fb_copyarea area; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > > area.sx = sy * vc->vc_font.height; > area.sy = vyres - ((sx + width) * vc->vc_font.width); > @@ -83,7 +83,7 @@ static void ccw_clear(struct vc_data *vc, struct fb_info *info, int sy, > struct fbcon_ops *ops = info->fbcon_par; > struct fb_fillrect region; > int bgshift = (vc->vc_hi_font_mask) ? 13 : 12; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > > region.color = attr_bgcol_ec(bgshift,vc,info); > region.dx = sy * vc->vc_font.height; > @@ -140,7 +140,7 @@ static void ccw_putcs(struct vc_data *vc, struct fb_info *info, > u32 cnt, pitch, size; > u32 attribute = get_attribute(info, scr_readw(s)); > u8 *dst, *buf = NULL; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > > if (!ops->fontbuffer) > return; > @@ -229,7 +229,7 @@ static void ccw_cursor(struct vc_data *vc, struct fb_info *info, int mode, > int attribute, use_sw = vc->vc_cursor_type & CUR_SW; > int err = 1, dx, dy; > char *src; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > > if (!ops->fontbuffer) > return; > @@ -387,7 +387,7 @@ static int ccw_update_start(struct fb_info *info) > { > struct fbcon_ops *ops = info->fbcon_par; > u32 yoffset; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > int err; > > yoffset = (vyres - info->var.yres) - ops->var.xoffset; > diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c > index 88d89fad3f05..86a254c1b2b7 100644 > --- a/drivers/video/fbdev/core/fbcon_cw.c > +++ b/drivers/video/fbdev/core/fbcon_cw.c > @@ -50,7 +50,7 @@ static void cw_bmove(struct vc_data *vc, struct fb_info *info, int sy, > { > struct fbcon_ops *ops = info->fbcon_par; > struct fb_copyarea area; > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vxres = GETVXRES(ops->p, info); > > area.sx = vxres - ((sy + height) * vc->vc_font.height); > area.sy = sx * vc->vc_font.width; > @@ -68,7 +68,7 @@ static void cw_clear(struct vc_data *vc, struct fb_info *info, int sy, > struct fbcon_ops *ops = info->fbcon_par; > struct fb_fillrect region; > int bgshift = (vc->vc_hi_font_mask) ? 13 : 12; > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vxres = GETVXRES(ops->p, info); > > region.color = attr_bgcol_ec(bgshift,vc,info); > region.dx = vxres - ((sy + height) * vc->vc_font.height); > @@ -125,7 +125,7 @@ static void cw_putcs(struct vc_data *vc, struct fb_info *info, > u32 cnt, pitch, size; > u32 attribute = get_attribute(info, scr_readw(s)); > u8 *dst, *buf = NULL; > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vxres = GETVXRES(ops->p, info); > > if (!ops->fontbuffer) > return; > @@ -212,7 +212,7 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, int mode, > int attribute, use_sw = vc->vc_cursor_type & CUR_SW; > int err = 1, dx, dy; > char *src; > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vxres = GETVXRES(ops->p, info); > > if (!ops->fontbuffer) > return; > @@ -369,7 +369,7 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, int mode, > static int cw_update_start(struct fb_info *info) > { > struct fbcon_ops *ops = info->fbcon_par; > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vxres = GETVXRES(ops->p, info); > u32 xoffset; > int err; > > diff --git a/drivers/video/fbdev/core/fbcon_rotate.h b/drivers/video/fbdev/core/fbcon_rotate.h > index e233444cda66..01cbe303b8a2 100644 > --- a/drivers/video/fbdev/core/fbcon_rotate.h > +++ b/drivers/video/fbdev/core/fbcon_rotate.h > @@ -12,11 +12,11 @@ > #define _FBCON_ROTATE_H > > #define GETVYRES(s,i) ({ \ > - (s == SCROLL_REDRAW || s == SCROLL_MOVE) ? \ > + (fb_scrollmode(s) == SCROLL_REDRAW || fb_scrollmode(s) == SCROLL_MOVE) ? \ > (i)->var.yres : (i)->var.yres_virtual; }) > > #define GETVXRES(s,i) ({ \ > - (s == SCROLL_REDRAW || s == SCROLL_MOVE || !(i)->fix.xpanstep) ? \ > + (fb_scrollmode(s) == SCROLL_REDRAW || fb_scrollmode(s) == SCROLL_MOVE || !(i)->fix.xpanstep) ? \ > (i)->var.xres : (i)->var.xres_virtual; }) > > > diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c > index 8d5e66b1bdfb..23bc045769d0 100644 > --- a/drivers/video/fbdev/core/fbcon_ud.c > +++ b/drivers/video/fbdev/core/fbcon_ud.c > @@ -50,8 +50,8 @@ static void ud_bmove(struct vc_data *vc, struct fb_info *info, int sy, > { > struct fbcon_ops *ops = info->fbcon_par; > struct fb_copyarea area; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > + u32 vxres = GETVXRES(ops->p, info); > > area.sy = vyres - ((sy + height) * vc->vc_font.height); > area.sx = vxres - ((sx + width) * vc->vc_font.width); > @@ -69,8 +69,8 @@ static void ud_clear(struct vc_data *vc, struct fb_info *info, int sy, > struct fbcon_ops *ops = info->fbcon_par; > struct fb_fillrect region; > int bgshift = (vc->vc_hi_font_mask) ? 13 : 12; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > + u32 vxres = GETVXRES(ops->p, info); > > region.color = attr_bgcol_ec(bgshift,vc,info); > region.dy = vyres - ((sy + height) * vc->vc_font.height); > @@ -162,8 +162,8 @@ static void ud_putcs(struct vc_data *vc, struct fb_info *info, > u32 mod = vc->vc_font.width % 8, cnt, pitch, size; > u32 attribute = get_attribute(info, scr_readw(s)); > u8 *dst, *buf = NULL; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > + u32 vxres = GETVXRES(ops->p, info); > > if (!ops->fontbuffer) > return; > @@ -259,8 +259,8 @@ static void ud_cursor(struct vc_data *vc, struct fb_info *info, int mode, > int attribute, use_sw = vc->vc_cursor_type & CUR_SW; > int err = 1, dx, dy; > char *src; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > + u32 vxres = GETVXRES(ops->p, info); > > if (!ops->fontbuffer) > return; > @@ -410,8 +410,8 @@ static int ud_update_start(struct fb_info *info) > { > struct fbcon_ops *ops = info->fbcon_par; > int xoffset, yoffset; > - u32 vyres = GETVYRES(ops->p->scrollmode, info); > - u32 vxres = GETVXRES(ops->p->scrollmode, info); > + u32 vyres = GETVYRES(ops->p, info); > + u32 vxres = GETVXRES(ops->p, info); > int err; > > xoffset = vxres - info->var.xres - ops->var.xoffset; > -- > 2.34.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Hello Daniel, On 2/1/22 21:11, Daniel Vetter wrote: > On Tue, Feb 1, 2022 at 7:59 PM Helge Deller <deller@gmx.de> wrote: >> >> Add a config option CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION to >> enable bitblt and fillrect hardware acceleration in the framebuffer >> console. If disabled, such acceleration will not be used, even if it is >> supported by the graphics hardware driver. >> >> If you plan to use DRM as your main graphics output system, you should >> disable this option since it will prevent compiling in code which isn't >> used later on when DRM takes over. >> >> For all other configurations, e.g. if none of your graphic cards support >> DRM (yet), DRM isn't available for your architecture, or you can't be >> sure that the graphic card in the target system will support DRM, you >> most likely want to enable this option. >> >> In the non-accelerated case (e.g. when DRM is used), the inlined >> fb_scrollmode() function is hardcoded to return SCROLL_REDRAW and as such the >> compiler is able to optimize much unneccesary code away. >> >> In this v3 patch version I additionally changed the GETVYRES() and GETVXRES() >> macros to take a pointer to the fbcon_display struct. This fixes the build when >> console rotation is enabled and helps the compiler again to optimize out code. >> >> Signed-off-by: Helge Deller <deller@gmx.de> >> Cc: stable@vger.kernel.org # v5.10+ >> Signed-off-by: Helge Deller <deller@gmx.de> >> --- >> drivers/video/console/Kconfig | 11 +++++++ >> drivers/video/fbdev/core/fbcon.c | 39 ++++++++++++++++++------- >> drivers/video/fbdev/core/fbcon.h | 15 +++++++++- >> drivers/video/fbdev/core/fbcon_ccw.c | 10 +++---- >> drivers/video/fbdev/core/fbcon_cw.c | 10 +++---- >> drivers/video/fbdev/core/fbcon_rotate.h | 4 +-- >> drivers/video/fbdev/core/fbcon_ud.c | 20 ++++++------- >> 7 files changed, 75 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig >> index 840d9813b0bc..6029fd41d7d0 100644 >> --- a/drivers/video/console/Kconfig >> +++ b/drivers/video/console/Kconfig >> @@ -78,6 +78,17 @@ config FRAMEBUFFER_CONSOLE >> help >> Low-level framebuffer-based console driver. >> >> +config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION >> + bool "Framebuffer Console hardware acceleration support" >> + depends on FRAMEBUFFER_CONSOLE >> + default n if DRM >> + default y >> + help >> + If you use a system on which DRM is fully supported you usually want to say N, >> + otherwise you probably want to enable this option. >> + If enabled the framebuffer console will utilize the hardware acceleration >> + of your graphics card by using hardware bitblt and fillrect features. > > This really doesn't have much to do with DRM but more about running > questionable code. That's why I went with the generalized stern > warning and default n, and explained when it's ok to do this (single > user and you care more about fbcon performance than potential issues > because you only run trusted stuff with access to your vt and fbdev > /dev node). I think this is something we both will keep to have different opinion about :-) This console acceleration code is not exported or visible to userspace, e.g. you can't access or trigger it via /dev/fb0. It's only called internally from inside fbcon, so it's independed if it's a single- or multi-user system. The only way to "access" it is via standard stdio, where fbcon then either scrolls the screen via redrawing characters at new positions or by using hardware bitblt to move screen contents. IMHO there is nothing which is critical here. Even when I analyzed the existing bug reports, there was none which affected that specific code. Anyway, let's look at that part in your patch: + bool "Enable legacy fbcon code for hw acceleration" + depends on FRAMEBUFFER_CONSOLE + default n + help + Only enable this options if you are in full control of machine since + the fbcon acceleration code is essentially unmaintained and known + problematic. + + If unsure, select n. Since I'm willing to maintain that scrolling/panning code, I'd like to drop the "is essentially unmaintained" part. And the "known problematic" part is up to now just speculative (which would be valid for other parts of the kernel too, btw). As this whole disussions showed, there are some few architectures/platforms which really still need this acceleration, while others don't. So, why not leave the decision up to the arch maintainers, which may opt-in or opt-out, while keeping the default on "n"? With that, here is a new proposal: +config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION + bool "Enable legacy fbcon hardware acceleration code" + depends on FRAMEBUFFER_CONSOLE + default y if (PARISC) # a few other arches may want to be listed here too... + default n + help + This option enables the fbcon (framebuffer text-based) hardware acceleration for + graphics drivers which were written for the fbdev graphics interface. + + On modern machines, on mainstream machines (like x86-64) or when using a modern + Linux distribution those fbdev drivers usually aren't used. + So enabling this option wouldn't have any effect, which is why you want + to disable this option on such newer machines. + + If you compile this kernel for older machines which still require the fbdev + drivers, you may want to say Y. + + If unsure, select n. Would that be acceptable? Arch maintainers may then later send (or reply now with) patches, e.g.: + default y if (M68K && XYZ) ... > Also you didn't keep any todo entry for removing DRM accel code, That wasn't intentional. I just sent 2 revert-patches and an fbcon-accel-enabling-patch. I'll look up what you had in your patch series and add it as seperate patch. > and iirc some nouveau folks also complained that they at least > once had some kind of accel, so that's another reason to not tie this > to DRM. The above proposal to add additional "default y if XYZ" would enable them to opt-in. > Anyway aside from this looks reasonable, can you pls respin with a > more cautious Kconfig text? Sure, I'll do as soon as we have a decision on the above Kconfig text. Helge
On Tue, Feb 1, 2022 at 11:52 PM Helge Deller <deller@gmx.de> wrote: > > Hello Daniel, > > On 2/1/22 21:11, Daniel Vetter wrote: > > On Tue, Feb 1, 2022 at 7:59 PM Helge Deller <deller@gmx.de> wrote: > >> > >> Add a config option CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION to > >> enable bitblt and fillrect hardware acceleration in the framebuffer > >> console. If disabled, such acceleration will not be used, even if it is > >> supported by the graphics hardware driver. > >> > >> If you plan to use DRM as your main graphics output system, you should > >> disable this option since it will prevent compiling in code which isn't > >> used later on when DRM takes over. > >> > >> For all other configurations, e.g. if none of your graphic cards support > >> DRM (yet), DRM isn't available for your architecture, or you can't be > >> sure that the graphic card in the target system will support DRM, you > >> most likely want to enable this option. > >> > >> In the non-accelerated case (e.g. when DRM is used), the inlined > >> fb_scrollmode() function is hardcoded to return SCROLL_REDRAW and as such the > >> compiler is able to optimize much unneccesary code away. > >> > >> In this v3 patch version I additionally changed the GETVYRES() and GETVXRES() > >> macros to take a pointer to the fbcon_display struct. This fixes the build when > >> console rotation is enabled and helps the compiler again to optimize out code. > >> > >> Signed-off-by: Helge Deller <deller@gmx.de> > >> Cc: stable@vger.kernel.org # v5.10+ > >> Signed-off-by: Helge Deller <deller@gmx.de> > >> --- > >> drivers/video/console/Kconfig | 11 +++++++ > >> drivers/video/fbdev/core/fbcon.c | 39 ++++++++++++++++++------- > >> drivers/video/fbdev/core/fbcon.h | 15 +++++++++- > >> drivers/video/fbdev/core/fbcon_ccw.c | 10 +++---- > >> drivers/video/fbdev/core/fbcon_cw.c | 10 +++---- > >> drivers/video/fbdev/core/fbcon_rotate.h | 4 +-- > >> drivers/video/fbdev/core/fbcon_ud.c | 20 ++++++------- > >> 7 files changed, 75 insertions(+), 34 deletions(-) > >> > >> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig > >> index 840d9813b0bc..6029fd41d7d0 100644 > >> --- a/drivers/video/console/Kconfig > >> +++ b/drivers/video/console/Kconfig > >> @@ -78,6 +78,17 @@ config FRAMEBUFFER_CONSOLE > >> help > >> Low-level framebuffer-based console driver. > >> > >> +config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > >> + bool "Framebuffer Console hardware acceleration support" > >> + depends on FRAMEBUFFER_CONSOLE > >> + default n if DRM > >> + default y > >> + help > >> + If you use a system on which DRM is fully supported you usually want to say N, > >> + otherwise you probably want to enable this option. > >> + If enabled the framebuffer console will utilize the hardware acceleration > >> + of your graphics card by using hardware bitblt and fillrect features. > > > > This really doesn't have much to do with DRM but more about running > > questionable code. That's why I went with the generalized stern > > warning and default n, and explained when it's ok to do this (single > > user and you care more about fbcon performance than potential issues > > because you only run trusted stuff with access to your vt and fbdev > > /dev node). > > I think this is something we both will keep to have different opinion about :-) > > This console acceleration code is not exported or visible to userspace, > e.g. you can't access or trigger it via /dev/fb0. > It's only called internally from inside fbcon, so it's independed if > it's a single- or multi-user system. > The only way to "access" it is via standard stdio, where fbcon then > either scrolls the screen via redrawing characters at new positions > or by using hardware bitblt to move screen contents. > IMHO there is nothing which is critical here. > Even when I analyzed the existing bug reports, there was none which > affected that specific code. Maybe to clarify. The issues that generally result in this code going boom in syzbot are when you race console activity (which can be largely controlled through VT ioctls from userspace too, plus /dev/kmsg) against fbdev resizing on the /dev/fb/0 nodes. The locking there is kinda wild, and the code is very hard to understand. This is why we've resorted to just disabling/deleting this code because most cases we have no idea what's actually going on, aside from something is clearly not going right. Also the multi-user here means "you run untrusted code from other people", not "you run multiple things in parallel" or "multiple people are logged in at the same time". > Anyway, let's look at that part in your patch: > > + bool "Enable legacy fbcon code for hw acceleration" > + depends on FRAMEBUFFER_CONSOLE > + default n > + help > + Only enable this options if you are in full control of machine since > + the fbcon acceleration code is essentially unmaintained and known > + problematic. > + > + If unsure, select n. > > Since I'm willing to maintain that scrolling/panning code, I'd like to > drop the "is essentially unmaintained" part. > And the "known problematic" part is up to now just speculative (which would be > valid for other parts of the kernel too, btw). > > As this whole disussions showed, there are some few architectures/platforms > which really still need this acceleration, while others don't. > So, why not leave the decision up to the arch maintainers, which may opt-in > or opt-out, while keeping the default on "n"? > > With that, here is a new proposal: > > +config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION > + bool "Enable legacy fbcon hardware acceleration code" > + depends on FRAMEBUFFER_CONSOLE > + default y if (PARISC) # a few other arches may want to be listed here too... > + default n > + help > + This option enables the fbcon (framebuffer text-based) hardware acceleration for > + graphics drivers which were written for the fbdev graphics interface. > + > + On modern machines, on mainstream machines (like x86-64) or when using a modern > + Linux distribution those fbdev drivers usually aren't used. > + So enabling this option wouldn't have any effect, which is why you want > + to disable this option on such newer machines. > + > + If you compile this kernel for older machines which still require the fbdev > + drivers, you may want to say Y. > + > + If unsure, select n. > > Would that be acceptable? Perfect. > Arch maintainers may then later send (or reply now with) patches, e.g.: > + default y if (M68K && XYZ) > ... Yeah makes sense. > > Also you didn't keep any todo entry for removing DRM accel code, > > That wasn't intentional. > I just sent 2 revert-patches and an fbcon-accel-enabling-patch. > I'll look up what you had in your patch series and add it as seperate patch. tbh I think it's fine either way. I think it's still rather unclear which way drm will go, least because there's not many people who can occasionally squeeze some time away to move things forward. Could be that we add a new emergency logging console in the kernel for drm, and distros switch over to kmscon (which is in userspace, and accelerated iirc if you have gl, so most modern systems). Or whether we just improve fbcon until it's solid enough. Or some other option. So given that just dropping the todo sounds ok, it was just a bit inconsistent with the Kconfig :-) > > and iirc some nouveau folks also complained that they at least > > once had some kind of accel, so that's another reason to not tie this > > to DRM. > > The above proposal to add additional "default y if XYZ" would enable > them to opt-in. > > > Anyway aside from this looks reasonable, can you pls respin with a > > more cautious Kconfig text? > > Sure, I'll do as soon as we have a decision on the above Kconfig text. Ideally if you can send them out today so it's not missing the drm-misc-fixes train that leaves tomorrow, that would be best. Cheers, Daniel
On 2/2/22 12:05, Daniel Vetter wrote: > On Tue, Feb 1, 2022 at 11:52 PM Helge Deller <deller@gmx.de> wrote: >> >> Hello Daniel, >> >> On 2/1/22 21:11, Daniel Vetter wrote: >>> On Tue, Feb 1, 2022 at 7:59 PM Helge Deller <deller@gmx.de> wrote: >>>> >>>> Add a config option CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION to >>>> enable bitblt and fillrect hardware acceleration in the framebuffer >>>> console. If disabled, such acceleration will not be used, even if it is >>>> supported by the graphics hardware driver. >>>> >>>> If you plan to use DRM as your main graphics output system, you should >>>> disable this option since it will prevent compiling in code which isn't >>>> used later on when DRM takes over. >>>> >>>> For all other configurations, e.g. if none of your graphic cards support >>>> DRM (yet), DRM isn't available for your architecture, or you can't be >>>> sure that the graphic card in the target system will support DRM, you >>>> most likely want to enable this option. >>>> >>>> In the non-accelerated case (e.g. when DRM is used), the inlined >>>> fb_scrollmode() function is hardcoded to return SCROLL_REDRAW and as such the >>>> compiler is able to optimize much unneccesary code away. >>>> >>>> In this v3 patch version I additionally changed the GETVYRES() and GETVXRES() >>>> macros to take a pointer to the fbcon_display struct. This fixes the build when >>>> console rotation is enabled and helps the compiler again to optimize out code. >>>> >>>> Signed-off-by: Helge Deller <deller@gmx.de> >>>> Cc: stable@vger.kernel.org # v5.10+ >>>> Signed-off-by: Helge Deller <deller@gmx.de> >>>> --- >>>> drivers/video/console/Kconfig | 11 +++++++ >>>> drivers/video/fbdev/core/fbcon.c | 39 ++++++++++++++++++------- >>>> drivers/video/fbdev/core/fbcon.h | 15 +++++++++- >>>> drivers/video/fbdev/core/fbcon_ccw.c | 10 +++---- >>>> drivers/video/fbdev/core/fbcon_cw.c | 10 +++---- >>>> drivers/video/fbdev/core/fbcon_rotate.h | 4 +-- >>>> drivers/video/fbdev/core/fbcon_ud.c | 20 ++++++------- >>>> 7 files changed, 75 insertions(+), 34 deletions(-) >>>> >>>> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig >>>> index 840d9813b0bc..6029fd41d7d0 100644 >>>> --- a/drivers/video/console/Kconfig >>>> +++ b/drivers/video/console/Kconfig >>>> @@ -78,6 +78,17 @@ config FRAMEBUFFER_CONSOLE >>>> help >>>> Low-level framebuffer-based console driver. >>>> >>>> +config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION >>>> + bool "Framebuffer Console hardware acceleration support" >>>> + depends on FRAMEBUFFER_CONSOLE >>>> + default n if DRM >>>> + default y >>>> + help >>>> + If you use a system on which DRM is fully supported you usually want to say N, >>>> + otherwise you probably want to enable this option. >>>> + If enabled the framebuffer console will utilize the hardware acceleration >>>> + of your graphics card by using hardware bitblt and fillrect features. >>> >>> This really doesn't have much to do with DRM but more about running >>> questionable code. That's why I went with the generalized stern >>> warning and default n, and explained when it's ok to do this (single >>> user and you care more about fbcon performance than potential issues >>> because you only run trusted stuff with access to your vt and fbdev >>> /dev node). >> >> I think this is something we both will keep to have different opinion about :-) >> >> This console acceleration code is not exported or visible to userspace, >> e.g. you can't access or trigger it via /dev/fb0. >> It's only called internally from inside fbcon, so it's independed if >> it's a single- or multi-user system. >> The only way to "access" it is via standard stdio, where fbcon then >> either scrolls the screen via redrawing characters at new positions >> or by using hardware bitblt to move screen contents. >> IMHO there is nothing which is critical here. >> Even when I analyzed the existing bug reports, there was none which >> affected that specific code. > > Maybe to clarify. The issues that generally result in this code going > boom in syzbot are when you race console activity (which can be > largely controlled through VT ioctls from userspace too, plus > /dev/kmsg) against fbdev resizing on the /dev/fb/0 nodes. The locking > there is kinda wild, and the code is very hard to understand. This is > why we've resorted to just disabling/deleting this code because most > cases we have no idea what's actually going on, aside from something > is clearly not going right. Yes, that's clear. But that (missing) locking needs to be on a higher level in the call chain, so the same lock fixes would fix scroll-redraw and hw acceleration. > Also the multi-user here means "you run untrusted code from other > people", not "you run multiple things in parallel" or "multiple people > are logged in at the same time". > >> Anyway, let's look at that part in your patch: >> >> + bool "Enable legacy fbcon code for hw acceleration" >> + depends on FRAMEBUFFER_CONSOLE >> + default n >> + help >> + Only enable this options if you are in full control of machine since >> + the fbcon acceleration code is essentially unmaintained and known >> + problematic. >> + >> + If unsure, select n. >> >> Since I'm willing to maintain that scrolling/panning code, I'd like to >> drop the "is essentially unmaintained" part. >> And the "known problematic" part is up to now just speculative (which would be >> valid for other parts of the kernel too, btw). >> >> As this whole disussions showed, there are some few architectures/platforms >> which really still need this acceleration, while others don't. >> So, why not leave the decision up to the arch maintainers, which may opt-in >> or opt-out, while keeping the default on "n"? >> >> With that, here is a new proposal: >> >> +config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION >> + bool "Enable legacy fbcon hardware acceleration code" >> + depends on FRAMEBUFFER_CONSOLE >> + default y if (PARISC) # a few other arches may want to be listed here too... >> + default n >> + help >> + This option enables the fbcon (framebuffer text-based) hardware acceleration for >> + graphics drivers which were written for the fbdev graphics interface. >> + >> + On modern machines, on mainstream machines (like x86-64) or when using a modern >> + Linux distribution those fbdev drivers usually aren't used. >> + So enabling this option wouldn't have any effect, which is why you want >> + to disable this option on such newer machines. >> + >> + If you compile this kernel for older machines which still require the fbdev >> + drivers, you may want to say Y. >> + >> + If unsure, select n. >> >> Would that be acceptable? > > Perfect. Great!! >> Arch maintainers may then later send (or reply now with) patches, e.g.: >> + default y if (M68K && XYZ) >> ... > > Yeah makes sense. > >>> Also you didn't keep any todo entry for removing DRM accel code, >> >> That wasn't intentional. >> I just sent 2 revert-patches and an fbcon-accel-enabling-patch. >> I'll look up what you had in your patch series and add it as seperate patch. > > tbh I think it's fine either way. I think it's still rather unclear > which way drm will go, least because there's not many people who can > occasionally squeeze some time away to move things forward. Could be > that we add a new emergency logging console in the kernel for drm, and > distros switch over to kmscon (which is in userspace, and accelerated > iirc if you have gl, so most modern systems). Or whether we just > improve fbcon until it's solid enough. Or some other option. > > So given that just dropping the todo sounds ok, it was just a bit > inconsistent with the Kconfig :-) Ok, I leave it out. >>> and iirc some nouveau folks also complained that they at least >>> once had some kind of accel, so that's another reason to not tie this >>> to DRM. >> >> The above proposal to add additional "default y if XYZ" would enable >> them to opt-in. >> >>> Anyway aside from this looks reasonable, can you pls respin with a >>> more cautious Kconfig text? >> >> Sure, I'll do as soon as we have a decision on the above Kconfig text. > > Ideally if you can send them out today so it's not missing the > drm-misc-fixes train that leaves tomorrow, that would be best. Cool. I'll send the new series in a few minutes. Thanks! Helge
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig index 840d9813b0bc..6029fd41d7d0 100644 --- a/drivers/video/console/Kconfig +++ b/drivers/video/console/Kconfig @@ -78,6 +78,17 @@ config FRAMEBUFFER_CONSOLE help Low-level framebuffer-based console driver. +config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION + bool "Framebuffer Console hardware acceleration support" + depends on FRAMEBUFFER_CONSOLE + default n if DRM + default y + help + If you use a system on which DRM is fully supported you usually want to say N, + otherwise you probably want to enable this option. + If enabled the framebuffer console will utilize the hardware acceleration + of your graphics card by using hardware bitblt and fillrect features. + config FRAMEBUFFER_CONSOLE_DETECT_PRIMARY bool "Map the console to the primary display device" depends on FRAMEBUFFER_CONSOLE diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index b813985f1403..f7b7d35953e8 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -1136,11 +1136,13 @@ static void fbcon_init(struct vc_data *vc, int init) ops->graphics = 0; +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION if ((cap & FBINFO_HWACCEL_COPYAREA) && !(cap & FBINFO_HWACCEL_DISABLED)) p->scrollmode = SCROLL_MOVE; else /* default to something safe */ p->scrollmode = SCROLL_REDRAW; +#endif /* * ++guenther: console.c:vc_allocate() relies on initializing @@ -1705,7 +1707,7 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int t, unsigned int b, count = vc->vc_rows; if (logo_shown >= 0) goto redraw_up; - switch (p->scrollmode) { + switch (fb_scrollmode(p)) { case SCROLL_MOVE: fbcon_redraw_blit(vc, info, p, t, b - t - count, count); @@ -1795,7 +1797,7 @@ static bool fbcon_scroll(struct vc_data *vc, unsigned int t, unsigned int b, count = vc->vc_rows; if (logo_shown >= 0) goto redraw_down; - switch (p->scrollmode) { + switch (fb_scrollmode(p)) { case SCROLL_MOVE: fbcon_redraw_blit(vc, info, p, b - 1, b - t - count, -count); @@ -1946,12 +1948,12 @@ static void fbcon_bmove_rec(struct vc_data *vc, struct fbcon_display *p, int sy, height, width); } -static void updatescrollmode(struct fbcon_display *p, +static void updatescrollmode_accel(struct fbcon_display *p, struct fb_info *info, struct vc_data *vc) { +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION 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, @@ -1972,12 +1974,6 @@ static void updatescrollmode(struct fbcon_display *p, 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 ? @@ -1991,6 +1987,27 @@ static void updatescrollmode(struct fbcon_display *p, else p->scrollmode = SCROLL_REDRAW; } +#endif +} + +static void updatescrollmode(struct fbcon_display *p, + struct fb_info *info, + struct vc_data *vc) +{ + struct fbcon_ops *ops = info->fbcon_par; + int fh = vc->vc_font.height; + 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); + + 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--; + + /* update scrollmode in case hardware acceleration is used */ + updatescrollmode_accel(p, info, vc); } #define PITCH(w) (((w) + 7) >> 3) @@ -2148,7 +2165,7 @@ static int fbcon_switch(struct vc_data *vc) updatescrollmode(p, info, vc); - switch (p->scrollmode) { + switch (fb_scrollmode(p)) { case SCROLL_WRAP_MOVE: scrollback_phys_max = p->vrows - vc->vc_rows; break; diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h index 9315b360c898..0f16cbc99e6a 100644 --- a/drivers/video/fbdev/core/fbcon.h +++ b/drivers/video/fbdev/core/fbcon.h @@ -29,7 +29,9 @@ struct fbcon_display { /* Filled in by the low-level console driver */ const u_char *fontdata; int userfont; /* != 0 if fontdata kmalloc()ed */ - u_short scrollmode; /* Scroll Method */ +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION + u_short scrollmode; /* Scroll Method, use fb_scrollmode() */ +#endif u_short inverse; /* != 0 text black on white as default */ short yscroll; /* Hardware scrolling */ int vrows; /* number of virtual rows */ @@ -208,6 +210,17 @@ static inline int attr_col_ec(int shift, struct vc_data *vc, #define SCROLL_REDRAW 0x004 #define SCROLL_PAN_REDRAW 0x005 +static inline u_short fb_scrollmode(struct fbcon_display *fb) +{ +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION + return fb->scrollmode; +#else + /* hardcoded to SCROLL_REDRAW if acceleration was disabled. */ + return SCROLL_REDRAW; +#endif +} + + #ifdef CONFIG_FB_TILEBLITTING extern void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info); #endif diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c index 9cd2c4b05c32..2789ace79634 100644 --- a/drivers/video/fbdev/core/fbcon_ccw.c +++ b/drivers/video/fbdev/core/fbcon_ccw.c @@ -65,7 +65,7 @@ static void ccw_bmove(struct vc_data *vc, struct fb_info *info, int sy, { struct fbcon_ops *ops = info->fbcon_par; struct fb_copyarea area; - u32 vyres = GETVYRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); area.sx = sy * vc->vc_font.height; area.sy = vyres - ((sx + width) * vc->vc_font.width); @@ -83,7 +83,7 @@ static void ccw_clear(struct vc_data *vc, struct fb_info *info, int sy, struct fbcon_ops *ops = info->fbcon_par; struct fb_fillrect region; int bgshift = (vc->vc_hi_font_mask) ? 13 : 12; - u32 vyres = GETVYRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); region.color = attr_bgcol_ec(bgshift,vc,info); region.dx = sy * vc->vc_font.height; @@ -140,7 +140,7 @@ static void ccw_putcs(struct vc_data *vc, struct fb_info *info, u32 cnt, pitch, size; u32 attribute = get_attribute(info, scr_readw(s)); u8 *dst, *buf = NULL; - u32 vyres = GETVYRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); if (!ops->fontbuffer) return; @@ -229,7 +229,7 @@ static void ccw_cursor(struct vc_data *vc, struct fb_info *info, int mode, int attribute, use_sw = vc->vc_cursor_type & CUR_SW; int err = 1, dx, dy; char *src; - u32 vyres = GETVYRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); if (!ops->fontbuffer) return; @@ -387,7 +387,7 @@ static int ccw_update_start(struct fb_info *info) { struct fbcon_ops *ops = info->fbcon_par; u32 yoffset; - u32 vyres = GETVYRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); int err; yoffset = (vyres - info->var.yres) - ops->var.xoffset; diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c index 88d89fad3f05..86a254c1b2b7 100644 --- a/drivers/video/fbdev/core/fbcon_cw.c +++ b/drivers/video/fbdev/core/fbcon_cw.c @@ -50,7 +50,7 @@ static void cw_bmove(struct vc_data *vc, struct fb_info *info, int sy, { struct fbcon_ops *ops = info->fbcon_par; struct fb_copyarea area; - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vxres = GETVXRES(ops->p, info); area.sx = vxres - ((sy + height) * vc->vc_font.height); area.sy = sx * vc->vc_font.width; @@ -68,7 +68,7 @@ static void cw_clear(struct vc_data *vc, struct fb_info *info, int sy, struct fbcon_ops *ops = info->fbcon_par; struct fb_fillrect region; int bgshift = (vc->vc_hi_font_mask) ? 13 : 12; - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vxres = GETVXRES(ops->p, info); region.color = attr_bgcol_ec(bgshift,vc,info); region.dx = vxres - ((sy + height) * vc->vc_font.height); @@ -125,7 +125,7 @@ static void cw_putcs(struct vc_data *vc, struct fb_info *info, u32 cnt, pitch, size; u32 attribute = get_attribute(info, scr_readw(s)); u8 *dst, *buf = NULL; - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vxres = GETVXRES(ops->p, info); if (!ops->fontbuffer) return; @@ -212,7 +212,7 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, int mode, int attribute, use_sw = vc->vc_cursor_type & CUR_SW; int err = 1, dx, dy; char *src; - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vxres = GETVXRES(ops->p, info); if (!ops->fontbuffer) return; @@ -369,7 +369,7 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, int mode, static int cw_update_start(struct fb_info *info) { struct fbcon_ops *ops = info->fbcon_par; - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vxres = GETVXRES(ops->p, info); u32 xoffset; int err; diff --git a/drivers/video/fbdev/core/fbcon_rotate.h b/drivers/video/fbdev/core/fbcon_rotate.h index e233444cda66..01cbe303b8a2 100644 --- a/drivers/video/fbdev/core/fbcon_rotate.h +++ b/drivers/video/fbdev/core/fbcon_rotate.h @@ -12,11 +12,11 @@ #define _FBCON_ROTATE_H #define GETVYRES(s,i) ({ \ - (s == SCROLL_REDRAW || s == SCROLL_MOVE) ? \ + (fb_scrollmode(s) == SCROLL_REDRAW || fb_scrollmode(s) == SCROLL_MOVE) ? \ (i)->var.yres : (i)->var.yres_virtual; }) #define GETVXRES(s,i) ({ \ - (s == SCROLL_REDRAW || s == SCROLL_MOVE || !(i)->fix.xpanstep) ? \ + (fb_scrollmode(s) == SCROLL_REDRAW || fb_scrollmode(s) == SCROLL_MOVE || !(i)->fix.xpanstep) ? \ (i)->var.xres : (i)->var.xres_virtual; }) diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c index 8d5e66b1bdfb..23bc045769d0 100644 --- a/drivers/video/fbdev/core/fbcon_ud.c +++ b/drivers/video/fbdev/core/fbcon_ud.c @@ -50,8 +50,8 @@ static void ud_bmove(struct vc_data *vc, struct fb_info *info, int sy, { struct fbcon_ops *ops = info->fbcon_par; struct fb_copyarea area; - u32 vyres = GETVYRES(ops->p->scrollmode, info); - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); + u32 vxres = GETVXRES(ops->p, info); area.sy = vyres - ((sy + height) * vc->vc_font.height); area.sx = vxres - ((sx + width) * vc->vc_font.width); @@ -69,8 +69,8 @@ static void ud_clear(struct vc_data *vc, struct fb_info *info, int sy, struct fbcon_ops *ops = info->fbcon_par; struct fb_fillrect region; int bgshift = (vc->vc_hi_font_mask) ? 13 : 12; - u32 vyres = GETVYRES(ops->p->scrollmode, info); - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); + u32 vxres = GETVXRES(ops->p, info); region.color = attr_bgcol_ec(bgshift,vc,info); region.dy = vyres - ((sy + height) * vc->vc_font.height); @@ -162,8 +162,8 @@ static void ud_putcs(struct vc_data *vc, struct fb_info *info, u32 mod = vc->vc_font.width % 8, cnt, pitch, size; u32 attribute = get_attribute(info, scr_readw(s)); u8 *dst, *buf = NULL; - u32 vyres = GETVYRES(ops->p->scrollmode, info); - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); + u32 vxres = GETVXRES(ops->p, info); if (!ops->fontbuffer) return; @@ -259,8 +259,8 @@ static void ud_cursor(struct vc_data *vc, struct fb_info *info, int mode, int attribute, use_sw = vc->vc_cursor_type & CUR_SW; int err = 1, dx, dy; char *src; - u32 vyres = GETVYRES(ops->p->scrollmode, info); - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); + u32 vxres = GETVXRES(ops->p, info); if (!ops->fontbuffer) return; @@ -410,8 +410,8 @@ static int ud_update_start(struct fb_info *info) { struct fbcon_ops *ops = info->fbcon_par; int xoffset, yoffset; - u32 vyres = GETVYRES(ops->p->scrollmode, info); - u32 vxres = GETVXRES(ops->p->scrollmode, info); + u32 vyres = GETVYRES(ops->p, info); + u32 vxres = GETVXRES(ops->p, info); int err; xoffset = vxres - info->var.xres - ops->var.xoffset;