Skip to contentSkip to conversation

m68k: Bug fix and cleanup for framebuffer debug console

PATCH Superseded Finn Thain 2 participants 6 messages Mar 19, 2025 – Mar 20, 2025

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Finn Thain

author
#0 lore raw

v2 cover m68k: Bug fix and cleanup for framebuffer debug console

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 only used on Macs at present.

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

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(-)
Signature · 1 line
2.45.3
author
#1 lore raw

PATCH v2 1/3 m68k: Fix lost column on framebuffer debug console

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")
---
 arch/m68k/kernel/head.S | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)
arch/m68k/kernel/head.S +21 -10
index 852255cf60de..d0d77b1adbde 100644
@@ -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 */
+	subil	#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:
+	addil	#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
+	addil	#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
 
Signature · 1 line
2.45.3
#2 lore raw

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

arch/m68k/kernel/head.S L(console_clear_loop): · line 3403

Quoted 9 lines · Finn Thain
arch/m68k/kernel/head.S +1 -0
index 852255cf60de..d0d77b1adbde 100644
@@ -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 */
+	subil	#1,%d1				/* row range is 0 to num - 1 */

s/subil/subql/

Quoted 5 lines
arch/m68k/kernel/head.S +0 -1
@@ -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

arch/m68k/kernel/head.S func_start console_putc,%a0/%a1/%d0-%d7 · line 3556

Quoted 10 lines
arch/m68k/kernel/head.S +2 -3
-	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:
+	addil	#1,%d0

s/addil/addql/

Quoted 18 lines
arch/m68k/kernel/head.S +1 -6
@@ -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)
 
+	/*

arch/m68k/kernel/head.S L(console_do_font_scanline): · line 3640

Quoted 10 lines
arch/m68k/kernel/head.S +10 -0
+	 *	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
+	addil	#1,%d0

s/addil/addql/

Signature · 3 lines
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
author
#3 lore raw

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

Quoted 7 lines · Andreas Schwab

s/subil/subql/

s/addil/addql/

Will do. Thanks.

author
#4 lore raw

PATCH v2 2/3 m68k: Avoid pointless recursion in debug console rendering

In reply to 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.

---
 arch/m68k/kernel/head.S | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
arch/m68k/kernel/head.S +4 -7
index d0d77b1adbde..bf7fa344cd3a 100644
@@ -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
Signature · 1 line
2.45.3
author
#5 lore raw

PATCH v2 3/3 m68k: Remove unused "cursor home" code from debug console

In reply to 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.

---
 arch/m68k/kernel/head.S | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)
arch/m68k/kernel/head.S +12 -19
index bf7fa344cd3a..48ef74e45535 100644
@@ -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
 
Signature · 1 line
2.45.3