Re: [PATCH v2 31/32] auxdisplay: charlcd: Do not print chars at end of line
On Tue, Sep 22, 2020 at 12:20:36PM +0200, Miguel Ojeda wrote: > Hi Lars, Willy, > > On Tue, Sep 22, 2020 at 12:04 PM Willy Tarreau wrote: > > > > The points you mention are good enough indicators that it's extremely > > unlikely anyone has ever used that feature with these drivers. So I > > think it's best to keep your changes and wait to see if anyone pops > > up with an obscure use case, in which case their explanation might > > help figure another approach. > > Agreed -- Lars, please add an explanation for the removed feature in > the appropriate commit message and also mention it in the cover letter > so that such user-facing change (and any other) is documented. Ok, I will do this. Regards, Lars
Re: [PATCH v2 31/32] auxdisplay: charlcd: Do not print chars at end of line
Hi Lars, Willy, On Tue, Sep 22, 2020 at 12:04 PM Willy Tarreau wrote: > > The points you mention are good enough indicators that it's extremely > unlikely anyone has ever used that feature with these drivers. So I > think it's best to keep your changes and wait to see if anyone pops > up with an obscure use case, in which case their explanation might > help figure another approach. Agreed -- Lars, please add an explanation for the removed feature in the appropriate commit message and also mention it in the cover letter so that such user-facing change (and any other) is documented. Cheers, Miguel
Re: [PATCH v2 31/32] auxdisplay: charlcd: Do not print chars at end of line
On Tue, Sep 22, 2020 at 11:44:22AM +0200, Lars Poeschel wrote: > On Tue, Sep 22, 2020 at 07:27:01AM +0200, Willy Tarreau wrote: > > On Mon, Sep 21, 2020 at 04:46:43PM +0200, poesc...@lemonage.de wrote: > > > From: Lars Poeschel > > > > > > Skip printing characters at the end of a display line. This fits to the > > > behaviour we already had, that the cursor is nailed to last position of > > > a line. > > > > Just very old memories, but wasn't this used with the ability to shift > > the display ? IIRC the HD44780 has a 2x64 chars buffer and you can make > > the buffered characters appear when you shift the display. That's akin > > to seeing the display as an adjustable window over the buffer. I don't > > remember having used that feature, so if it didn't previously work, please > > disregard my comment, I just want to be sure we don't break a feature > > others might be relying on. > > Yes, indeed. But this is a point, where this was inconsistent. The > feature you described worked only for displays with 2 lines or less. On > displays with 4 lines there simply is no buffer that is not visible and > in this case it sticks to the end of the line already. OK. > To make this work for all displays this would require a whole lot of > work. We would need a fallback implementation that emulates this in > software and then redraws the whole display from that software > (linux-side) buffer when shifting. I don't think that's worth it at all, really! > Currently my patchset cuts this feature for displays with one or two > lines. I don't see clean an easy way to leave that in. Any ideas ? The points you mention are good enough indicators that it's extremely unlikely anyone has ever used that feature with these drivers. So I think it's best to keep your changes and wait to see if anyone pops up with an obscure use case, in which case their explanation might help figure another approach. Willy
Re: [PATCH v2 31/32] auxdisplay: charlcd: Do not print chars at end of line
On Tue, Sep 22, 2020 at 07:27:01AM +0200, Willy Tarreau wrote: > On Mon, Sep 21, 2020 at 04:46:43PM +0200, poesc...@lemonage.de wrote: > > From: Lars Poeschel > > > > Skip printing characters at the end of a display line. This fits to the > > behaviour we already had, that the cursor is nailed to last position of > > a line. > > Just very old memories, but wasn't this used with the ability to shift > the display ? IIRC the HD44780 has a 2x64 chars buffer and you can make > the buffered characters appear when you shift the display. That's akin > to seeing the display as an adjustable window over the buffer. I don't > remember having used that feature, so if it didn't previously work, please > disregard my comment, I just want to be sure we don't break a feature > others might be relying on. Yes, indeed. But this is a point, where this was inconsistent. The feature you described worked only for displays with 2 lines or less. On displays with 4 lines there simply is no buffer that is not visible and in this case it sticks to the end of the line already. To make this work for all displays this would require a whole lot of work. We would need a fallback implementation that emulates this in software and then redraws the whole display from that software (linux-side) buffer when shifting. Currently my patchset cuts this feature for displays with one or two lines. I don't see clean an easy way to leave that in. Any ideas ? Regards, Lars
Re: [PATCH v2 31/32] auxdisplay: charlcd: Do not print chars at end of line
On Mon, Sep 21, 2020 at 04:46:43PM +0200, poesc...@lemonage.de wrote: > From: Lars Poeschel > > Skip printing characters at the end of a display line. This fits to the > behaviour we already had, that the cursor is nailed to last position of > a line. Just very old memories, but wasn't this used with the ability to shift the display ? IIRC the HD44780 has a 2x64 chars buffer and you can make the buffered characters appear when you shift the display. That's akin to seeing the display as an adjustable window over the buffer. I don't remember having used that feature, so if it didn't previously work, please disregard my comment, I just want to be sure we don't break a feature others might be relying on. Willy
[PATCH v2 31/32] auxdisplay: charlcd: Do not print chars at end of line
From: Lars Poeschel
Skip printing characters at the end of a display line. This fits to the
behaviour we already had, that the cursor is nailed to last position of
a line.
Signed-off-by: Lars Poeschel
---
drivers/auxdisplay/charlcd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index cd3a304c08ec..1d5b3af9cca7 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -111,6 +111,9 @@ static void charlcd_home(struct charlcd *lcd)
static void charlcd_print(struct charlcd *lcd, char c)
{
+ if (lcd->addr.x >= lcd->width)
+ return;
+
if (lcd->char_conv)
c = lcd->char_conv[(unsigned char)c];
--
2.28.0
