Re: [PATCH 03/21] fbcon: Restore fbcon scrolling acceleration

On Tue, Feb 1, 2022 at 11:16 AM Helge Deller deller@gmx.de wrote:

On 1/31/22 22:05, Daniel Vetter wrote:

...

This functionally undoes 39aead8373b3 ("fbcon: Disable accelerated scrolling"), but behind the FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION option.

you have two trivial copy-n-paste errors in this patch which still prevent the console acceleration.

Duh :-(

But before we dig into details I think the big picture would be better. I honestly don't like the #ifdef pile here that much. I wonder whether your approach, also with GETVX/YRES adjusted somehow, wouldn't look cleaner? Like I said in the cover letter I got mostly distracted with fbcon locking last week, not really with this one here at all, so maybe going with your 4 (or 2 if we squash them like I did here) patches is neater?

Cheers, Daniel

...

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 2ff90061c7f3..39dc18a5de86 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -1125,13 +1125,15 @@ static void fbcon_init(struct vc_data *vc, int init)

  ops->graphics = 0;

• /*
  

•  * No more hw acceleration for fbcon.
  

•  *
  

•  * FIXME: Garbage collect all the now dead code after sufficient time
  

•  * has passed.
  

•  */
  

+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION

should be: #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION

...

• if ((info->flags & FBINFO_HWACCEL_COPYAREA) &&
  

•     !(info->flags & FBINFO_HWACCEL_DISABLED))
  

•         p->scrollmode = SCROLL_MOVE;
  

• else /* default to something safe */
  

•         p->scrollmode = SCROLL_REDRAW;
  

+#else p->scrollmode = SCROLL_REDRAW; +#endif

  /*
   *  ++guenther: console.c:vc_allocate() relies on initializing

@@ -1971,15 +1973,49 @@ 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;
  

• }
  

+#ifndef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION

same here... it needs to be: #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION

...

• p->scrollmode = SCROLL_REDRAW;
  

+#endif }

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

still reviewing the other patches...

Helge