This functionally undoes 39aead8373b3 ("fbcon: Disable accelerated scrolling"), but behind the FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION option.
References: https://lore.kernel.org/dri-devel/feea8303-2b83-fc36-972c-4fc8ad723bde@gmx.d... Fixes: 39aead8373b3 ("fbcon: Disable accelerated scrolling") Cc: Helge Deller deller@gmx.de Cc: stable@vger.kernel.org # v5.11+ Cc: Claudio Suarez cssk@net-c.es Cc: Dave Airlie airlied@gmail.com Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Linux Fbdev development list linux-fbdev@vger.kernel.org Cc: Pavel Machek pavel@ucw.cz Cc: Sam Ravnborg sam@ravnborg.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Javier Martinez Canillas javierm@redhat.com Cc: DRI Development dri-devel@lists.freedesktop.org Cc: Linux Kernel Mailing List linux-kernel@vger.kernel.org Cc: Claudio Suarez cssk@net-c.es Cc: Tomi Valkeinen tomi.valkeinen@ti.com Cc: Geert Uytterhoeven geert@linux-m68k.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Daniel Vetter daniel.vetter@intel.com Cc: Sven Schnelle svens@stackframe.org Cc: Gerd Hoffmann kraxel@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/video/fbdev/core/fbcon.c | 48 ++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 6 deletions(-)
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 + 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 + p->scrollmode = SCROLL_REDRAW; +#endif }
#define PITCH(w) (((w) + 7) >> 3)
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.
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
On 2/1/22 11:16, Helge Deller 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.
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
actually: #ifndef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION
- p->scrollmode = SCROLL_REDRAW;
+#endif }
#define PITCH(w) (((w) + 7) >> 3)
still reviewing the other patches...
Helge
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
On 2/1/22 11:36, Daniel Vetter wrote:
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.
Me neither. The ifdefs give a better separation, but prevents that the compiler checks the various paths when building.
I wonder whether your approach, also with GETVX/YRES adjusted somehow, wouldn't look cleaner?
I think so. You wouldn't even need to touch GETVX/YRES because the compiler will optimize/reduce it from
#define GETVYRES(s,i) ({ \ (s == SCROLL_REDRAW || s == SCROLL_MOVE) ? \ (i)->var.yres : (i)->var.yres_virtual; })
to just become:
#define GETVYRES(s,i) ((i)->var.yres)
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?
The benefit of my patch series was, that it could be easily backported first, and then cleaned up afterwards. Even a small additional backport patch to disable the fbcon acceleration for DRM in the old releases would be easy. But I'm not insisting on backporting the patches, if we find good way forward.
So, either with the 4 (or 2) patches would be OK for me (or even your approach).
Helge
On Tue, Feb 1, 2022 at 12:01 PM Helge Deller deller@gmx.de wrote:
On 2/1/22 11:36, Daniel Vetter wrote:
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.
Me neither. The ifdefs give a better separation, but prevents that the compiler checks the various paths when building.
I wonder whether your approach, also with GETVX/YRES adjusted somehow, wouldn't look cleaner?
I think so. You wouldn't even need to touch GETVX/YRES because the compiler will optimize/reduce it from
#define GETVYRES(s,i) ({ \ (s == SCROLL_REDRAW || s == SCROLL_MOVE) ? \ (i)->var.yres : (i)->var.yres_virtual; })
to just become:
#define GETVYRES(s,i) ((i)->var.yres)
Yeah, but you need to roll out your helper to all the callsites. But since you #ifdef out info->scrollmode we should catch them all I guess.
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?
The benefit of my patch series was, that it could be easily backported first, and then cleaned up afterwards. Even a small additional backport patch to disable the fbcon acceleration for DRM in the old releases would be easy. But I'm not insisting on backporting the patches, if we find good way forward.
So, either with the 4 (or 2) patches would be OK for me (or even your approach).
The idea behind the squash was that it's then impossible to backport without the Kconfig, and so we'll only enable this code when people intentionally want it. Maybe I'm too paranoid?
Anyway, you feel like finishing off your approach? Or should I send out v2 of this with the issues fixed you spotted? Like I said either is fine with me. -Daniel
On 2/1/22 14:45, Daniel Vetter wrote:
On Tue, Feb 1, 2022 at 12:01 PM Helge Deller deller@gmx.de wrote:
On 2/1/22 11:36, Daniel Vetter wrote:
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.
Me neither. The ifdefs give a better separation, but prevents that the compiler checks the various paths when building.
I wonder whether your approach, also with GETVX/YRES adjusted somehow, wouldn't look cleaner?
I think so. You wouldn't even need to touch GETVX/YRES because the compiler will optimize/reduce it from
#define GETVYRES(s,i) ({ \ (s == SCROLL_REDRAW || s == SCROLL_MOVE) ? \ (i)->var.yres : (i)->var.yres_virtual; })
to just become:
#define GETVYRES(s,i) ((i)->var.yres)
Yeah, but you need to roll out your helper to all the callsites. But since you #ifdef out info->scrollmode we should catch them all I guess.
Right. That was the only reason why I ifdef'ed it out. Technically we don't need that ifdef.
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?
The benefit of my patch series was, that it could be easily backported first, and then cleaned up afterwards. Even a small additional backport patch to disable the fbcon acceleration for DRM in the old releases would be easy. But I'm not insisting on backporting the patches, if we find good way forward.
So, either with the 4 (or 2) patches would be OK for me (or even your approach).
The idea behind the squash was that it's then impossible to backport without the Kconfig,
Yes, my proposal was to simply revert the 2 patches and immediatly send the Kconfig patch to disable it again.
and so we'll only enable this code when people intentionally want it. Maybe I'm too paranoid?
I think you are too paranoid :-) If all patches incl. the Kconfig patch are backported then people shouldn't do it wrong.
Anyway, you feel like finishing off your approach? Or should I send out v2 of this with the issues fixed you spotted? Like I said either is fine with me.
Ok, then let me try to finish my approach until tomorrow, and then you check if you can and want to add your locking and other patches on top of it. In the end I leave the decision which approach to take to you. Ok?
Helge
On Tue, Feb 1, 2022 at 3:52 PM Helge Deller deller@gmx.de wrote:
On 2/1/22 14:45, Daniel Vetter wrote:
On Tue, Feb 1, 2022 at 12:01 PM Helge Deller deller@gmx.de wrote:
On 2/1/22 11:36, Daniel Vetter wrote:
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.
Me neither. The ifdefs give a better separation, but prevents that the compiler checks the various paths when building.
I wonder whether your approach, also with GETVX/YRES adjusted somehow, wouldn't look cleaner?
I think so. You wouldn't even need to touch GETVX/YRES because the compiler will optimize/reduce it from
#define GETVYRES(s,i) ({ \ (s == SCROLL_REDRAW || s == SCROLL_MOVE) ? \ (i)->var.yres : (i)->var.yres_virtual; })
to just become:
#define GETVYRES(s,i) ((i)->var.yres)
Yeah, but you need to roll out your helper to all the callsites. But since you #ifdef out info->scrollmode we should catch them all I guess.
Right. That was the only reason why I ifdef'ed it out. Technically we don't need that ifdef.
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?
The benefit of my patch series was, that it could be easily backported first, and then cleaned up afterwards. Even a small additional backport patch to disable the fbcon acceleration for DRM in the old releases would be easy. But I'm not insisting on backporting the patches, if we find good way forward.
So, either with the 4 (or 2) patches would be OK for me (or even your approach).
The idea behind the squash was that it's then impossible to backport without the Kconfig,
Yes, my proposal was to simply revert the 2 patches and immediatly send the Kconfig patch to disable it again.
and so we'll only enable this code when people intentionally want it. Maybe I'm too paranoid?
I think you are too paranoid :-) If all patches incl. the Kconfig patch are backported then people shouldn't do it wrong.
Anyway, you feel like finishing off your approach? Or should I send out v2 of this with the issues fixed you spotted? Like I said either is fine with me.
Ok, then let me try to finish my approach until tomorrow, and then you check if you can and want to add your locking and other patches on top of it. In the end I leave the decision which approach to take to you. Ok?
Sounds good, and yeah rough idea is that the maintainers + revert + Kconfig should go in for rc3 or rc4 if we hit another bump, and the locking stuff then in for -next (since it needs a backmerge and is defo tricky stuff).
Cheers, Daniel
linux-stable-mirror@lists.linaro.org