Currently, when a message shorter than the display size is written, the
content wraps around (e.g., "123" on a 4-digit display shows "1231")
without scrolling, which is confusing and unintuitive.

Change behavior to display short messages statically with space padding
(e.g. "123 ") while only scrolling messages longer than the display width.
This provides more natural behavior that aligns with user expectations
and current linedisp_display() kernel-doc.

The scroll logic is also consolidated into a helper function for clarity.

No API changes are introduced.

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 drivers/auxdisplay/line-display.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index e44341b1e..ea23c43bb 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -36,6 +36,11 @@ static struct linedisp *to_linedisp(struct device *dev)
 	return container_of(dev, struct linedisp, dev);
 }
 
+static inline bool should_scroll(struct linedisp *linedisp)
+{
+	return linedisp->message_len > linedisp->num_chars && linedisp->scroll_rate;
+}
+
 /**
  * linedisp_scroll() - scroll the display by a character
  * @t: really a pointer to the private data structure
@@ -67,7 +72,7 @@ static void linedisp_scroll(struct timer_list *t)
 	linedisp->scroll_pos %= linedisp->message_len;
 
 	/* rearm the timer */
-	if (linedisp->message_len > num_chars && linedisp->scroll_rate)
+	if (should_scroll(linedisp))
 		mod_timer(&linedisp->timer, jiffies + linedisp->scroll_rate);
 }
 
@@ -118,8 +123,16 @@ static int linedisp_display(struct linedisp *linedisp, const char *msg,
 	linedisp->message_len = count;
 	linedisp->scroll_pos = 0;
 
-	/* update the display */
-	linedisp_scroll(&linedisp->timer);
+	if (should_scroll(linedisp)) {
+		/* display scrolling message */
+		linedisp_scroll(&linedisp->timer);
+	} else {
+		/* display static message */
+		memset(linedisp->buf, ' ', linedisp->num_chars);
+		memcpy(linedisp->buf, linedisp->message,
+		       umin(linedisp->num_chars, linedisp->message_len));
+		linedisp->ops->update(linedisp);
+	}
 
 	return 0;
 }
@@ -186,12 +199,12 @@ static ssize_t scroll_step_ms_store(struct device *dev,
 	if (err)
 		return err;
 
+	timer_delete_sync(&linedisp->timer);
+
 	linedisp->scroll_rate = msecs_to_jiffies(ms);
-	if (linedisp->message && linedisp->message_len > linedisp->num_chars) {
-		timer_delete_sync(&linedisp->timer);
-		if (linedisp->scroll_rate)
-			linedisp_scroll(&linedisp->timer);
-	}
+
+	if (should_scroll(linedisp))
+		linedisp_scroll(&linedisp->timer);
 
 	return count;
 }
-- 
2.43.0

On Sun, Aug 31, 2025 at 10:00:26PM -0400, Jean-François Lessard wrote:
> Currently, when a message shorter than the display size is written, the
> content wraps around (e.g., "123" on a 4-digit display shows "1231")
> without scrolling, which is confusing and unintuitive.
> 
> Change behavior to display short messages statically with space padding
> (e.g. "123 ") while only scrolling messages longer than the display width.
> This provides more natural behavior that aligns with user expectations
> and current linedisp_display() kernel-doc.
> 
> The scroll logic is also consolidated into a helper function for clarity.
> 
> No API changes are introduced.

...

>  /**
>   * linedisp_scroll() - scroll the display by a character
>   * @t: really a pointer to the private data structure

>  	linedisp->scroll_pos %= linedisp->message_len;
>  
>  	/* rearm the timer */
> -	if (linedisp->message_len > num_chars && linedisp->scroll_rate)
> +	if (should_scroll(linedisp))
>  		mod_timer(&linedisp->timer, jiffies + linedisp->scroll_rate);
>  }
>  

...

>  	linedisp->message_len = count;
>  	linedisp->scroll_pos = 0;
>  
> -	/* update the display */
> -	linedisp_scroll(&linedisp->timer);
> +	if (should_scroll(linedisp)) {
> +		/* display scrolling message */
> +		linedisp_scroll(&linedisp->timer);
> +	} else {
> +		/* display static message */
> +		memset(linedisp->buf, ' ', linedisp->num_chars);
> +		memcpy(linedisp->buf, linedisp->message,
> +		       umin(linedisp->num_chars, linedisp->message_len));
> +		linedisp->ops->update(linedisp);
> +	}

Hmm... But it seems the linedisp_scroll already has a check, why do we need
an additional one here? Perhaps we need to pad a message somewhere else and
guarantee it won't ever be less than num_chars?

-- 
With Best Regards,
Andy Shevchenko

Le 2 septembre 2025 06 h 03 min 00 s HAE, Andy Shevchenko <andriy.shevchenko@intel.com> a écrit :
>On Sun, Aug 31, 2025 at 10:00:26PM -0400, Jean-François Lessard wrote:
>> Currently, when a message shorter than the display size is written, the
>> content wraps around (e.g., "123" on a 4-digit display shows "1231")
>> without scrolling, which is confusing and unintuitive.
>> 
>> Change behavior to display short messages statically with space padding
>> (e.g. "123 ") while only scrolling messages longer than the display width.
>> This provides more natural behavior that aligns with user expectations
>> and current linedisp_display() kernel-doc.
>> 
>> The scroll logic is also consolidated into a helper function for clarity.
>> 
>> No API changes are introduced.
>
>...
>
>>  /**
>>   * linedisp_scroll() - scroll the display by a character
>>   * @t: really a pointer to the private data structure
>
>>  	linedisp->scroll_pos %= linedisp->message_len;
>>  
>>  	/* rearm the timer */
>> -	if (linedisp->message_len > num_chars && linedisp->scroll_rate)
>> +	if (should_scroll(linedisp))
>>  		mod_timer(&linedisp->timer, jiffies + linedisp->scroll_rate);
>>  }
>>  
>
>...
>
>>  	linedisp->message_len = count;
>>  	linedisp->scroll_pos = 0;
>>  
>> -	/* update the display */
>> -	linedisp_scroll(&linedisp->timer);
>> +	if (should_scroll(linedisp)) {
>> +		/* display scrolling message */
>> +		linedisp_scroll(&linedisp->timer);
>> +	} else {
>> +		/* display static message */
>> +		memset(linedisp->buf, ' ', linedisp->num_chars);
>> +		memcpy(linedisp->buf, linedisp->message,
>> +		       umin(linedisp->num_chars, linedisp->message_len));
>> +		linedisp->ops->update(linedisp);
>> +	}
>
>Hmm... But it seems the linedisp_scroll already has a check, why do we need
>an additional one here? Perhaps we need to pad a message somewhere else and
>guarantee it won't ever be less than num_chars?
>

Semantically, linedisp_scroll should scroll. I think it's better to have two
distinct paths with their specific logic:
1. Scroll: circular display and rearm the timer
2. Static: padding and direct update

But you're absolutely right. Given the explicit should_scroll() conditional
outside, the check inside linedisp_scroll() is now redundant. I'll remove it
after testing to streamline the code paths and eliminate the duplication.