[PATCH v3 0/3] m68k: Bug fix and cleanup for framebuffer debug console

[PATCH v3 0/3] m68k: Bug fix and cleanup for framebuffer debug console

[Finn Thain]

This series has a bug fix for the early bootconsole as well as some
related efficiency improvements and cleanup.

The relevant code is subject to CONSOLE_DEBUG, which is presently only
used with CONFIG_MAC. To test this series (in qemu-system-m68k, for example)
it's helpful to enable CONFIG_EARLY_PRINTK and
CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER and boot with
kernel parameters 'console=ttyS0 earlyprintk keep_bootcon'.

---
Changed since v1: 
 - Solved problem with line wrap while scrolling.
 - Added two additional patches.

Changed since v2:
 - Adopted addq and subq as suggested by Andreas.


Finn Thain (3):
  m68k: Fix lost column on framebuffer debug console
  m68k: Avoid pointless recursion in debug console rendering
  m68k: Remove unused "cursor home" code from debug console

 arch/m68k/kernel/head.S | 73 +++++++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 36 deletions(-)

-- 
2.45.3

[PATCH v3 1/3] m68k: Fix lost column on framebuffer debug console

[Finn Thain]

Move the cursor position rightward after rendering the character,
not before. This avoids complications that arise when the recursive
console_putc call has to wrap the line and/or scroll the display.
This also fixes the linewrap bug that crops off the rightmost column.

When the cursor is at the bottom of the display, a linefeed will not
move the cursor position further downward. Instead, the display scrolls
upward. Avoid the repeated add/subtract sequence by way of a single
subtraction at the initialization of console_struct_num_rows.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
Changed since v2:
 - addil and subil are now addql and subql resp.
---
 arch/m68k/kernel/head.S | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/m68k/kernel/head.S b/arch/m68k/kernel/head.S
index 852255cf60de..9bd8adaa756d 100644
--- a/arch/m68k/kernel/head.S
+++ b/arch/m68k/kernel/head.S
@@ -3400,6 +3400,7 @@ L(console_clear_loop):
 
 	movel	%d4,%d1				/* screen height in pixels */
 	divul	%a0@(FONT_DESC_HEIGHT),%d1	/* d1 = max num rows */
+	subql	#1,%d1				/* row range is 0 to num - 1 */
 
 	movel	%d0,%a2@(Lconsole_struct_num_columns)
 	movel	%d1,%a2@(Lconsole_struct_num_rows)
@@ -3546,15 +3547,14 @@ func_start	console_putc,%a0/%a1/%d0-%d7
 	cmpib	#10,%d7
 	jne	L(console_not_lf)
 	movel	%a0@(Lconsole_struct_cur_row),%d0
-	addil	#1,%d0
-	movel	%d0,%a0@(Lconsole_struct_cur_row)
 	movel	%a0@(Lconsole_struct_num_rows),%d1
 	cmpl	%d1,%d0
 	jcs	1f
-	subil	#1,%d0
-	movel	%d0,%a0@(Lconsole_struct_cur_row)
 	console_scroll
+	jra	L(console_exit)
 1:
+	addql	#1,%d0
+	movel	%d0,%a0@(Lconsole_struct_cur_row)
 	jra	L(console_exit)
 
 L(console_not_lf):
@@ -3581,12 +3581,6 @@ L(console_not_cr):
  */
 L(console_not_home):
 	movel	%a0@(Lconsole_struct_cur_column),%d0
-	addql	#1,%a0@(Lconsole_struct_cur_column)
-	movel	%a0@(Lconsole_struct_num_columns),%d1
-	cmpl	%d1,%d0
-	jcs	1f
-	console_putc	#'\n'	/* recursion is OK! */
-1:
 	movel	%a0@(Lconsole_struct_cur_row),%d1
 
 	/*
@@ -3633,6 +3627,23 @@ L(console_do_font_scanline):
 	addq	#1,%d1
 	dbra	%d7,L(console_read_char_scanline)
 
+	/*
+	 *	Register usage in the code below:
+	 *	a0 = pointer to console globals
+	 *	d0 = cursor column
+	 *	d1 = cursor column limit
+	 */
+
+	lea	%pc@(L(console_globals)),%a0
+
+	movel	%a0@(Lconsole_struct_cur_column),%d0
+	addql	#1,%d0
+	movel	%d0,%a0@(Lconsole_struct_cur_column)	/* Update cursor pos */
+	movel	%a0@(Lconsole_struct_num_columns),%d1
+	cmpl	%d1,%d0
+	jcs	L(console_exit)
+	console_putc	#'\n'		/* Line wrap using tail recursion */
+
 L(console_exit):
 func_return	console_putc
 
-- 
2.45.3

[PATCH v3 2/3] m68k: Avoid pointless recursion in debug console rendering

[Finn Thain]

The recursive call to console_putc to effect a carriage return is
needlessly slow and complicated. Instead, just clear the column counter
directly. Setup %a0 earlier to avoid a repeated comparison.

Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
 arch/m68k/kernel/head.S | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/m68k/kernel/head.S b/arch/m68k/kernel/head.S
index 9bd8adaa756d..49e079b0d0b5 100644
--- a/arch/m68k/kernel/head.S
+++ b/arch/m68k/kernel/head.S
@@ -3533,19 +3533,16 @@ func_start	console_putc,%a0/%a1/%d0-%d7
 	tstl	%pc@(L(console_font))
 	jeq	L(console_exit)
 
+	lea	%pc@(L(console_globals)),%a0
+
 	/* Output character in d7 on console.
 	 */
 	movel	ARG1,%d7
 	cmpib	#'\n',%d7
-	jbne	1f
+	jne	L(console_not_lf)
 
-	/* A little safe recursion is good for the soul */
-	console_putc	#'\r'
-1:
-	lea	%pc@(L(console_globals)),%a0
+	clrl	%a0@(Lconsole_struct_cur_column)	/* implicit \r */
 
-	cmpib	#10,%d7
-	jne	L(console_not_lf)
 	movel	%a0@(Lconsole_struct_cur_row),%d0
 	movel	%a0@(Lconsole_struct_num_rows),%d1
 	cmpl	%d1,%d0
-- 
2.45.3

[PATCH v3 3/3] m68k: Remove unused "cursor home" code from debug console

[Finn Thain]

The cursor home operation is unused and seems undesirable for logging.
Remove it. The console_not_cr label actually means "not line feed and
not carriage return either" so take the opportunity to replace it with
something less confusing. Rectify some inconsistent whitespace while
we're here.

Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
 arch/m68k/kernel/head.S | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/arch/m68k/kernel/head.S b/arch/m68k/kernel/head.S
index 49e079b0d0b5..6465333d5f7c 100644
--- a/arch/m68k/kernel/head.S
+++ b/arch/m68k/kernel/head.S
@@ -3555,28 +3555,21 @@ func_start	console_putc,%a0/%a1/%d0-%d7
 	jra	L(console_exit)
 
 L(console_not_lf):
-	cmpib	#13,%d7
-	jne	L(console_not_cr)
+	cmpib	#'\r',%d7
+	jne	L(console_not_lf_not_cr)
 	clrl	%a0@(Lconsole_struct_cur_column)
 	jra	L(console_exit)
 
-L(console_not_cr):
-	cmpib	#1,%d7
-	jne	L(console_not_home)
-	clrl	%a0@(Lconsole_struct_cur_row)
-	clrl	%a0@(Lconsole_struct_cur_column)
-	jra	L(console_exit)
-
-/*
- *	At this point we know that the %d7 character is going to be
- *	rendered on the screen.  Register usage is -
- *		a0 = pointer to console globals
- *		a1 = font data
- *		d0 = cursor column
- *		d1 = cursor row to draw the character
- *		d7 = character number
- */
-L(console_not_home):
+	/*
+	 *	At this point we know that the %d7 character is going to be
+	 *	rendered on the screen.  Register usage is -
+	 *	a0 = pointer to console globals
+	 *	a1 = font data
+	 *	d0 = cursor column
+	 *	d1 = cursor row to draw the character
+	 *	d7 = character number
+	 */
+L(console_not_lf_not_cr):
 	movel	%a0@(Lconsole_struct_cur_column),%d0
 	movel	%a0@(Lconsole_struct_cur_row),%d1
 
-- 
2.45.3

Re: [PATCH v3 0/3] m68k: Bug fix and cleanup for framebuffer debug console

[Stan Johnson]

Tested-by: Stan Johnson <userm57@yahoo.com>

On a Mac IIci, the three patches fix the lost-column issue.

I didn't notice any regressions booting the new kernel to multiuser
and accessing serial, scsi and network.

-Stan Johnson


On 3/27/25 4:39 PM, Finn Thain wrote:
> This series has a bug fix for the early bootconsole as well as some
> related efficiency improvements and cleanup.
> 
> The relevant code is subject to CONSOLE_DEBUG, which is presently only
> used with CONFIG_MAC. To test this series (in qemu-system-m68k, for example)
> it's helpful to enable CONFIG_EARLY_PRINTK and
> CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER and boot with
> kernel parameters 'console=ttyS0 earlyprintk keep_bootcon'.
> 
> ---
> Changed since v1: 
>  - Solved problem with line wrap while scrolling.
>  - Added two additional patches.
> 
> Changed since v2:
>  - Adopted addq and subq as suggested by Andreas.
> 
> 
> Finn Thain (3):
>   m68k: Fix lost column on framebuffer debug console
>   m68k: Avoid pointless recursion in debug console rendering
>   m68k: Remove unused "cursor home" code from debug console
> 
>  arch/m68k/kernel/head.S | 73 +++++++++++++++++++++--------------------
>  1 file changed, 37 insertions(+), 36 deletions(-)
> 

Re: [PATCH v3 1/3] m68k: Fix lost column on framebuffer debug console

[Geert Uytterhoeven]

On Thu, 27 Mar 2025 at 23:44, Finn Thain <fthain@linux-m68k.org> wrote:
> Move the cursor position rightward after rendering the character,
> not before. This avoids complications that arise when the recursive
> console_putc call has to wrap the line and/or scroll the display.
> This also fixes the linewrap bug that crops off the rightmost column.
>
> When the cursor is at the bottom of the display, a linefeed will not
> move the cursor position further downward. Instead, the display scrolls
> upward. Avoid the repeated add/subtract sequence by way of a single
> subtraction at the initialization of console_struct_num_rows.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
> ---
> Changed since v2:
>  - addil and subil are now addql and subql resp.

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
i.e. will queue in the m68k tree for v6.17.

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

Re: [PATCH v3 2/3] m68k: Avoid pointless recursion in debug console rendering

[Geert Uytterhoeven]

On Thu, 27 Mar 2025 at 23:44, Finn Thain <fthain@linux-m68k.org> wrote:
> The recursive call to console_putc to effect a carriage return is
> needlessly slow and complicated. Instead, just clear the column counter
> directly. Setup %a0 earlier to avoid a repeated comparison.
>
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
i.e. will queue in the m68k tree for v6.17.

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

Re: [PATCH v3 3/3] m68k: Remove unused "cursor home" code from debug console

[Geert Uytterhoeven]

On Thu, 27 Mar 2025 at 23:44, Finn Thain <fthain@linux-m68k.org> wrote:
> The cursor home operation is unused and seems undesirable for logging.
> Remove it. The console_not_cr label actually means "not line feed and
> not carriage return either" so take the opportunity to replace it with
> something less confusing. Rectify some inconsistent whitespace while
> we're here.
>
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
i.e. will queue in the m68k tree for v6.17.

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