All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] auxdisplay: hd44780: move cursor home after clear display command
@ 2023-07-06 18:50 Hugo Villeneuve
  2023-07-06 19:33 ` Miguel Ojeda
  0 siblings, 1 reply; 9+ messages in thread
From: Hugo Villeneuve @ 2023-07-06 18:50 UTC (permalink / raw)
  To: Miguel Ojeda; +Cc: hugo, Hugo Villeneuve, linux-kernel

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

The "clear display" command on the NewHaven NHD-0220DZW-AG5 display
does NOT change the DDRAM address to 00h (home position) like the
standard Hitachi HD44780 controller. As a consequence, the starting
position of the initial string LCD_INIT_TEXT is not guaranteed to be
at 0,0 depending on where the cursor was before the clear display
command.

Extract of CLEAR_DISPLAY command from datasheets of:

    Hitachi HD44780:
        ... It then sets DDRAM address 0 into the address counter...

    NewHaven NHD-0220DZW-AG5 datasheet:
	... This instruction does not change the DDRAM Address

Move the cursor home after sending clear display command to support
non-standard LCDs.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/auxdisplay/hd44780_common.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/auxdisplay/hd44780_common.c b/drivers/auxdisplay/hd44780_common.c
index 3934c2eebf33..70c818945a74 100644
--- a/drivers/auxdisplay/hd44780_common.c
+++ b/drivers/auxdisplay/hd44780_common.c
@@ -82,7 +82,13 @@ int hd44780_common_clear_display(struct charlcd *lcd)
 	hdc->write_cmd(hdc, LCD_CMD_DISPLAY_CLEAR);
 	/* datasheet says to wait 1,64 milliseconds */
 	long_sleep(2);
-	return 0;
+
+	/*
+	 * Some LCDs (ex: NewHaven) do not reset the DDRAM address when
+	 * executing the CLEAR_DISPLAY command. Explicitely move cursor
+	 * to home position to account for these non-standard LCDs:
+	 */
+	return hd44780_common_home(lcd);
 }
 EXPORT_SYMBOL_GPL(hd44780_common_clear_display);
 

base-commit: c17414a273b81fe4e34e11d69fc30cc8b1431614
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] auxdisplay: hd44780: move cursor home after clear display command
  2023-07-06 18:50 [PATCH] auxdisplay: hd44780: move cursor home after clear display command Hugo Villeneuve
@ 2023-07-06 19:33 ` Miguel Ojeda
  2023-07-06 19:49   ` Hugo Villeneuve
  0 siblings, 1 reply; 9+ messages in thread
From: Miguel Ojeda @ 2023-07-06 19:33 UTC (permalink / raw)
  To: Hugo Villeneuve
  Cc: Miguel Ojeda, Hugo Villeneuve, linux-kernel, Lars Poeschel, geert

On Thu, Jul 6, 2023 at 8:51 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> The "clear display" command on the NewHaven NHD-0220DZW-AG5 display
> does NOT change the DDRAM address to 00h (home position) like the
> standard Hitachi HD44780 controller. As a consequence, the starting
> position of the initial string LCD_INIT_TEXT is not guaranteed to be
> at 0,0 depending on where the cursor was before the clear display
> command.
>
> Extract of CLEAR_DISPLAY command from datasheets of:
>
>     Hitachi HD44780:
>         ... It then sets DDRAM address 0 into the address counter...
>
>     NewHaven NHD-0220DZW-AG5 datasheet:
>         ... This instruction does not change the DDRAM Address
>
> Move the cursor home after sending clear display command to support
> non-standard LCDs.
>
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Thanks! Sounds good to me, as long the extra command does not
introduce some issue with the actual HD44780 -- can we double-check
the HD44780 still works as expected?

Cc'ing Lars and Geert since they may be able to give it a quick test.

> +       /*
> +        * Some LCDs (ex: NewHaven) do not reset the DDRAM address when
> +        * executing the CLEAR_DISPLAY command. Explicitely move cursor
> +        * to home position to account for these non-standard LCDs:
> +        */
> +       return hd44780_common_home(lcd);

Few nits:

  - Explicitely -> Explicitly.
  - Isn't the command `DISPLAY_CLEAR` instead of `CLEAR_DISPLAY`? (at
least the identifier above is `LCD_CMD_DISPLAY_CLEAR`).
  - `:` -> `.`.

What about something like:

    The Hitachi HD44780 controller (and compatible ones) reset the
DDRAM address when executing the `DISPLAY_CLEAR` command, thus the
following call is not required. However, other controllers do not
(e.g. NewHaven NHD-0220DZW-AG5), thus move the cursor to home
unconditionally to support both.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] auxdisplay: hd44780: move cursor home after clear display command
  2023-07-06 19:33 ` Miguel Ojeda
@ 2023-07-06 19:49   ` Hugo Villeneuve
  2023-07-20 16:49     ` poeschel
  0 siblings, 1 reply; 9+ messages in thread
From: Hugo Villeneuve @ 2023-07-06 19:49 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Miguel Ojeda, Hugo Villeneuve, linux-kernel, Lars Poeschel, geert

On Thu, 6 Jul 2023 21:33:05 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Thu, Jul 6, 2023 at 8:51 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > The "clear display" command on the NewHaven NHD-0220DZW-AG5 display
> > does NOT change the DDRAM address to 00h (home position) like the
> > standard Hitachi HD44780 controller. As a consequence, the starting
> > position of the initial string LCD_INIT_TEXT is not guaranteed to be
> > at 0,0 depending on where the cursor was before the clear display
> > command.
> >
> > Extract of CLEAR_DISPLAY command from datasheets of:
> >
> >     Hitachi HD44780:
> >         ... It then sets DDRAM address 0 into the address counter...
> >
> >     NewHaven NHD-0220DZW-AG5 datasheet:
> >         ... This instruction does not change the DDRAM Address
> >
> > Move the cursor home after sending clear display command to support
> > non-standard LCDs.
> >
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Thanks! Sounds good to me, as long the extra command does not
> introduce some issue with the actual HD44780 -- can we double-check
> the HD44780 still works as expected?
> 
> Cc'ing Lars and Geert since they may be able to give it a quick test.

Hi Miguel,
I do not have a standard Hitachi controller to test it on, so lets wait
for feedback from Lars and Geert or others.

> > +       /*
> > +        * Some LCDs (ex: NewHaven) do not reset the DDRAM address when
> > +        * executing the CLEAR_DISPLAY command. Explicitely move cursor
> > +        * to home position to account for these non-standard LCDs:
> > +        */
> > +       return hd44780_common_home(lcd);
> 
> Few nits:
> 
>   - Explicitely -> Explicitly.
>   - Isn't the command `DISPLAY_CLEAR` instead of `CLEAR_DISPLAY`? (at
> least the identifier above is `LCD_CMD_DISPLAY_CLEAR`).

Yes, I have also modified the commit log message to be consistent with
the code.

>   - `:` -> `.`.
> What about something like:
> 
>     The Hitachi HD44780 controller (and compatible ones) reset the
> DDRAM address when executing the `DISPLAY_CLEAR` command, thus the
> following call is not required. However, other controllers do not
> (e.g. NewHaven NHD-0220DZW-AG5), thus move the cursor to home
> unconditionally to support both.

Ok, changed comments to that.

Thank you,
Hugo.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] auxdisplay: hd44780: move cursor home after clear display command
  2023-07-06 19:49   ` Hugo Villeneuve
@ 2023-07-20 16:49     ` poeschel
  2023-07-22 14:46       ` David Reaver
  0 siblings, 1 reply; 9+ messages in thread
From: poeschel @ 2023-07-20 16:49 UTC (permalink / raw)
  To: Hugo Villeneuve
  Cc: Miguel Ojeda, Miguel Ojeda, Hugo Villeneuve, linux-kernel, geert,
	Christian Meusel

Am 2023-07-06 21:49, schrieb Hugo Villeneuve:
> On Thu, 6 Jul 2023 21:33:05 +0200
> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> 
>> On Thu, Jul 6, 2023 at 8:51 PM Hugo Villeneuve <hugo@hugovil.com> 
>> wrote:
>> >
>> > The "clear display" command on the NewHaven NHD-0220DZW-AG5 display
>> > does NOT change the DDRAM address to 00h (home position) like the
>> > standard Hitachi HD44780 controller. As a consequence, the starting
>> > position of the initial string LCD_INIT_TEXT is not guaranteed to be
>> > at 0,0 depending on where the cursor was before the clear display
>> > command.
>> >
>> > Extract of CLEAR_DISPLAY command from datasheets of:
>> >
>> >     Hitachi HD44780:
>> >         ... It then sets DDRAM address 0 into the address counter...
>> >
>> >     NewHaven NHD-0220DZW-AG5 datasheet:
>> >         ... This instruction does not change the DDRAM Address
>> >
>> > Move the cursor home after sending clear display command to support
>> > non-standard LCDs.
>> >
>> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>> 
>> Thanks! Sounds good to me, as long the extra command does not
>> introduce some issue with the actual HD44780 -- can we double-check
>> the HD44780 still works as expected?
>> 
>> Cc'ing Lars and Geert since they may be able to give it a quick test.
> 
> Hi Miguel,
> I do not have a standard Hitachi controller to test it on, so lets wait
> for feedback from Lars and Geert or others.

Sorry guys,
I do not have access to the relevant hardware anymore. I am CC'ing 
Christian,
who has the relevant hardware and maybe he can help testing the patch.
Christian is on vacation up until mid august, so we have to wait a bit 
more
for someone able to test this.

BTW: The displays I did the work back then on were for sure not genuine
Hitachi ones either.
I do not see, that the little patch should do any harm.

Regards,
Lars

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] auxdisplay: hd44780: move cursor home after clear display command
  2023-07-20 16:49     ` poeschel
@ 2023-07-22 14:46       ` David Reaver
  2023-07-22 16:04         ` Miguel Ojeda
  0 siblings, 1 reply; 9+ messages in thread
From: David Reaver @ 2023-07-22 14:46 UTC (permalink / raw)
  To: poeschel
  Cc: Hugo Villeneuve, Miguel Ojeda, Miguel Ojeda, Hugo Villeneuve,
	linux-kernel, geert, Christian Meusel


poeschel@lemonage.de writes:

> Am 2023-07-06 21:49, schrieb Hugo Villeneuve:
>> On Thu, 6 Jul 2023 21:33:05 +0200
>> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
>>
>>> On Thu, Jul 6, 2023 at 8:51 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
>>> >
>>> > The "clear display" command on the NewHaven NHD-0220DZW-AG5 display
>>> > does NOT change the DDRAM address to 00h (home position) like the
>>> > standard Hitachi HD44780 controller. As a consequence, the starting
>>> > position of the initial string LCD_INIT_TEXT is not guaranteed to be
>>> > at 0,0 depending on where the cursor was before the clear display
>>> > command.
>>> >
>>> > Extract of CLEAR_DISPLAY command from datasheets of:
>>> >
>>> >     Hitachi HD44780:
>>> >         ... It then sets DDRAM address 0 into the address counter...
>>> >
>>> >     NewHaven NHD-0220DZW-AG5 datasheet:
>>> >         ... This instruction does not change the DDRAM Address
>>> >
>>> > Move the cursor home after sending clear display command to support
>>> > non-standard LCDs.
>>> >
>>> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>>> Thanks! Sounds good to me, as long the extra command does not
>>> introduce some issue with the actual HD44780 -- can we double-check
>>> the HD44780 still works as expected?
>>> Cc'ing Lars and Geert since they may be able to give it a quick test.
>> Hi Miguel,
>> I do not have a standard Hitachi controller to test it on, so lets wait
>> for feedback from Lars and Geert or others.
>
> Sorry guys,
> I do not have access to the relevant hardware anymore. I am CC'ing Christian,
> who has the relevant hardware and maybe he can help testing the patch.
> Christian is on vacation up until mid august, so we have to wait a bit more
> for someone able to test this.
>
> BTW: The displays I did the work back then on were for sure not genuine
> Hitachi ones either.
> I do not see, that the little patch should do any harm.
>
> Regards,
> Lars


I was actually hooking up a 16x2 HD44780 on my BeagleBone Black last
night before I came across this patch, so I was able to test this. It
works fine for me. I tested with:

    $ printf '\f' > /dev/lcd
    $ printf 'Hello\nWorld!\n' > /dev/lcd
    $ printf '\x1b[LR' > /dev/lcd
    $ printf '\x1b[LR' > /dev/lcd
    $ printf '\x1b[LR' > /dev/lcd
    $ printf '\f' > /dev/lcd
    $ printf 'Goodbye\nWorld!\n' > /dev/lcd

As expected, "Goodbye World!" was correctly placed left-aligned on the
display, split over both lines. Let me know if there is something else
you would like me to do to test this!

Tested-by: David Reaver <me@davidreaver.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] auxdisplay: hd44780: move cursor home after clear display command
  2023-07-22 14:46       ` David Reaver
@ 2023-07-22 16:04         ` Miguel Ojeda
  2023-07-22 18:07           ` Hugo Villeneuve
  0 siblings, 1 reply; 9+ messages in thread
From: Miguel Ojeda @ 2023-07-22 16:04 UTC (permalink / raw)
  To: me
  Cc: poeschel, Hugo Villeneuve, Miguel Ojeda, Hugo Villeneuve,
	linux-kernel, geert, Christian Meusel

On Sat, Jul 22, 2023 at 4:54 PM David Reaver <me@davidreaver.com> wrote:
>
> I was actually hooking up a 16x2 HD44780 on my BeagleBone Black last
> night before I came across this patch, so I was able to test this. It
> works fine for me. I tested with:
>
>     $ printf '\f' > /dev/lcd
>     $ printf 'Hello\nWorld!\n' > /dev/lcd
>     $ printf '\x1b[LR' > /dev/lcd
>     $ printf '\x1b[LR' > /dev/lcd
>     $ printf '\x1b[LR' > /dev/lcd
>     $ printf '\f' > /dev/lcd
>     $ printf 'Goodbye\nWorld!\n' > /dev/lcd
>
> As expected, "Goodbye World!" was correctly placed left-aligned on the
> display, split over both lines. Let me know if there is something else
> you would like me to do to test this!
>
> Tested-by: David Reaver <me@davidreaver.com>

Thanks a lot, that is very helpful!

I will wait a while in case Christian or somebody else wants to test
it, and send it for 6.6.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] auxdisplay: hd44780: move cursor home after clear display command
  2023-07-22 16:04         ` Miguel Ojeda
@ 2023-07-22 18:07           ` Hugo Villeneuve
  2023-08-16 15:22             ` Christian Meusel
  0 siblings, 1 reply; 9+ messages in thread
From: Hugo Villeneuve @ 2023-07-22 18:07 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: me, poeschel, Miguel Ojeda, Hugo Villeneuve, linux-kernel, geert,
	Christian Meusel

On Sat, 22 Jul 2023 18:04:03 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Sat, Jul 22, 2023 at 4:54 PM David Reaver <me@davidreaver.com> wrote:
> >
> > I was actually hooking up a 16x2 HD44780 on my BeagleBone Black last
> > night before I came across this patch, so I was able to test this. It
> > works fine for me. I tested with:
> >
> >     $ printf '\f' > /dev/lcd
> >     $ printf 'Hello\nWorld!\n' > /dev/lcd
> >     $ printf '\x1b[LR' > /dev/lcd
> >     $ printf '\x1b[LR' > /dev/lcd
> >     $ printf '\x1b[LR' > /dev/lcd
> >     $ printf '\f' > /dev/lcd
> >     $ printf 'Goodbye\nWorld!\n' > /dev/lcd
> >
> > As expected, "Goodbye World!" was correctly placed left-aligned on the
> > display, split over both lines. Let me know if there is something else
> > you would like me to do to test this!
> >
> > Tested-by: David Reaver <me@davidreaver.com>
> 
> Thanks a lot, that is very helpful!
> 
> I will wait a while in case Christian or somebody else wants to test
> it, and send it for 6.6.

Hi Miguel,
in the meantime, I will send V2 of the patch with the changes you
suggested for the commit message and the comments.

Hugo.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] auxdisplay: hd44780: move cursor home after clear display command
  2023-07-22 18:07           ` Hugo Villeneuve
@ 2023-08-16 15:22             ` Christian Meusel
  2023-08-19 19:56               ` Miguel Ojeda
  0 siblings, 1 reply; 9+ messages in thread
From: Christian Meusel @ 2023-08-16 15:22 UTC (permalink / raw)
  To: Hugo Villeneuve, Miguel Ojeda, Miguel Ojeda, Hugo Villeneuve
  Cc: me, Lars Pöschel, linux-kernel, geert

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

Hello everyone,

>> I will wait a while in case Christian or somebody else wants to test
>> it, and send it for 6.6.

I finally managed to test this patch with our hardware and I'm seeing the same expected behavior as beforehand.

When executing David's test sequence with a EH002004A [1], I'm getting garbled contents when shifting with '\x1b[LR'. But this also happens with the version of the driver we're currently using, does not look related to this patch and might be an issue with our display model.


Best regards,

Christian


--
[1] https://www.winstar.com.tw/pt/products/oled-module/oled-character-display/20x4-oled.html

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] auxdisplay: hd44780: move cursor home after clear display command
  2023-08-16 15:22             ` Christian Meusel
@ 2023-08-19 19:56               ` Miguel Ojeda
  0 siblings, 0 replies; 9+ messages in thread
From: Miguel Ojeda @ 2023-08-19 19:56 UTC (permalink / raw)
  To: Christian Meusel
  Cc: Hugo Villeneuve, Miguel Ojeda, Hugo Villeneuve, me,
	Lars Pöschel, linux-kernel, geert

On Wed, Aug 16, 2023 at 5:22 PM Christian Meusel
<christian.meusel@posteo.de> wrote:
>
> I finally managed to test this patch with our hardware and I'm seeing the same expected behavior as beforehand.
>
> When executing David's test sequence with a EH002004A [1], I'm getting garbled contents when shifting with '\x1b[LR'. But this also happens with the version of the driver we're currently using, does not look related to this patch and might be an issue with our display model.

Thanks Christian & David for testing it!

In that case, Christian, I am applying the patch. If someone else
detects the same problem before the merge window (and it does not
happen without the patch), please let us know.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-08-20  0:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-06 18:50 [PATCH] auxdisplay: hd44780: move cursor home after clear display command Hugo Villeneuve
2023-07-06 19:33 ` Miguel Ojeda
2023-07-06 19:49   ` Hugo Villeneuve
2023-07-20 16:49     ` poeschel
2023-07-22 14:46       ` David Reaver
2023-07-22 16:04         ` Miguel Ojeda
2023-07-22 18:07           ` Hugo Villeneuve
2023-08-16 15:22             ` Christian Meusel
2023-08-19 19:56               ` Miguel Ojeda

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.