[3/5] auxdisplay: charlcd: add escape sequence for brightness on NEC µPD16314
Commit Message
The NEC µPD16314 can alter the the brightness of the LCD. Make it possible
to set this via escape sequence Y0 - Y3. B and R were already taken, so
I picked Y for luminance.
Signed-off-by: Sean Young <sean@mess.org>
---
drivers/auxdisplay/charlcd.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
Comments
On Mon, Jan 15, 2018 at 10:58 AM, Sean Young <sean@mess.org> wrote:
> The NEC µPD16314 can alter the the brightness of the LCD. Make it possible
> to set this via escape sequence Y0 - Y3. B and R were already taken, so
> I picked Y for luminance.
>
> Signed-off-by: Sean Young <sean@mess.org>
CC'ing Willy and Geert.
> ---
> drivers/auxdisplay/charlcd.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
> index a16c72779722..7a671ad959d1 100644
> --- a/drivers/auxdisplay/charlcd.c
> +++ b/drivers/auxdisplay/charlcd.c
> @@ -39,6 +39,8 @@
> #define LCD_FLAG_F 0x0020 /* Large font mode */
> #define LCD_FLAG_N 0x0040 /* 2-rows mode */
> #define LCD_FLAG_L 0x0080 /* Backlight enabled */
> +#define LCD_BRIGHTNESS_MASK 0x0300 /* Brightness */
> +#define LCD_BRIGHTNESS_SHIFT 8
Not sure about the name (since the brightness is also used in
priv->flags). By the way, should we start using the bitops.h stuff
(e.g. BIT(9) | BIT(8), GENMASK(9, 8)...) in new code? Not sure how
widespread they are.
>
> /* LCD commands */
> #define LCD_CMD_DISPLAY_CLEAR 0x01 /* Clear entire display */
> @@ -490,6 +492,17 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
> charlcd_gotoxy(lcd);
> processed = 1;
> break;
> + case 'Y': /* brightness (luma) */
> + switch (esc[1]) {
> + case '0': /* 25% */
> + case '1': /* 50% */
> + case '2': /* 75% */
> + case '3': /* 100% */
> + priv->flags = (priv->flags & ~(LCD_BRIGHTNESS_MASK)) |
> + (('3' - esc[1]) << LCD_BRIGHTNESS_SHIFT);
> + processed = 1;
> + break;
> + }
> }
>
> /* TODO: This indent party here got ugly, clean it! */
> @@ -507,12 +520,15 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
> ((priv->flags & LCD_FLAG_C) ? LCD_CMD_CURSOR_ON : 0) |
> ((priv->flags & LCD_FLAG_B) ? LCD_CMD_BLINK_ON : 0));
> /* check whether one of F,N flags was changed */
Should we add "or brightness" to the comment?
> - else if ((oldflags ^ priv->flags) & (LCD_FLAG_F | LCD_FLAG_N))
> + else if ((oldflags ^ priv->flags) & (LCD_FLAG_F | LCD_FLAG_N |
> + LCD_BRIGHTNESS_MASK))
> lcd->ops->write_cmd(lcd,
> LCD_CMD_FUNCTION_SET |
> ((lcd->ifwidth == 8) ? LCD_CMD_DATA_LEN_8BITS : 0) |
> ((priv->flags & LCD_FLAG_F) ? LCD_CMD_FONT_5X10_DOTS : 0) |
> - ((priv->flags & LCD_FLAG_N) ? LCD_CMD_TWO_LINES : 0));
> + ((priv->flags & LCD_FLAG_N) ? LCD_CMD_TWO_LINES : 0) |
> + ((priv->flags & LCD_BRIGHTNESS_MASK) >>
> + LCD_BRIGHTNESS_SHIFT));
> /* check whether L flag was changed */
> else if ((oldflags ^ priv->flags) & LCD_FLAG_L)
> charlcd_backlight(lcd, !!(priv->flags & LCD_FLAG_L));
> --
> 2.14.3
>
On Mon, Feb 12, 2018 at 12:56:58PM +0100, Miguel Ojeda wrote:
> On Mon, Jan 15, 2018 at 10:58 AM, Sean Young <sean@mess.org> wrote:
> > The NEC µPD16314 can alter the the brightness of the LCD. Make it possible
> > to set this via escape sequence Y0 - Y3. B and R were already taken, so
> > I picked Y for luminance.
> >
> > Signed-off-by: Sean Young <sean@mess.org>
>
> CC'ing Willy and Geert.
>
> > ---
> > drivers/auxdisplay/charlcd.c | 20 ++++++++++++++++++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
> > index a16c72779722..7a671ad959d1 100644
> > --- a/drivers/auxdisplay/charlcd.c
> > +++ b/drivers/auxdisplay/charlcd.c
> > @@ -39,6 +39,8 @@
> > #define LCD_FLAG_F 0x0020 /* Large font mode */
> > #define LCD_FLAG_N 0x0040 /* 2-rows mode */
> > #define LCD_FLAG_L 0x0080 /* Backlight enabled */
> > +#define LCD_BRIGHTNESS_MASK 0x0300 /* Brightness */
> > +#define LCD_BRIGHTNESS_SHIFT 8
>
> Not sure about the name (since the brightness is also used in
> priv->flags). By the way, should we start using the bitops.h stuff
> (e.g. BIT(9) | BIT(8), GENMASK(9, 8)...) in new code? Not sure how
> widespread they are.
The brightness is not really a flag, maybe it belongs in a separate field;
we could use bit fields in order to waste less space.
BIT() would be nicer and is more idiomatic by current standards.
Note that x, y and flags are currently unsigned long, they could be u8.
>
> >
> > /* LCD commands */
> > #define LCD_CMD_DISPLAY_CLEAR 0x01 /* Clear entire display */
> > @@ -490,6 +492,17 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
> > charlcd_gotoxy(lcd);
> > processed = 1;
> > break;
> > + case 'Y': /* brightness (luma) */
> > + switch (esc[1]) {
> > + case '0': /* 25% */
> > + case '1': /* 50% */
> > + case '2': /* 75% */
> > + case '3': /* 100% */
> > + priv->flags = (priv->flags & ~(LCD_BRIGHTNESS_MASK)) |
> > + (('3' - esc[1]) << LCD_BRIGHTNESS_SHIFT);
> > + processed = 1;
> > + break;
> > + }
> > }
> >
> > /* TODO: This indent party here got ugly, clean it! */
> > @@ -507,12 +520,15 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
> > ((priv->flags & LCD_FLAG_C) ? LCD_CMD_CURSOR_ON : 0) |
> > ((priv->flags & LCD_FLAG_B) ? LCD_CMD_BLINK_ON : 0));
> > /* check whether one of F,N flags was changed */
>
> Should we add "or brightness" to the comment?
Indeed we should.
>
> > - else if ((oldflags ^ priv->flags) & (LCD_FLAG_F | LCD_FLAG_N))
> > + else if ((oldflags ^ priv->flags) & (LCD_FLAG_F | LCD_FLAG_N |
> > + LCD_BRIGHTNESS_MASK))
> > lcd->ops->write_cmd(lcd,
> > LCD_CMD_FUNCTION_SET |
> > ((lcd->ifwidth == 8) ? LCD_CMD_DATA_LEN_8BITS : 0) |
> > ((priv->flags & LCD_FLAG_F) ? LCD_CMD_FONT_5X10_DOTS : 0) |
> > - ((priv->flags & LCD_FLAG_N) ? LCD_CMD_TWO_LINES : 0));
> > + ((priv->flags & LCD_FLAG_N) ? LCD_CMD_TWO_LINES : 0) |
> > + ((priv->flags & LCD_BRIGHTNESS_MASK) >>
> > + LCD_BRIGHTNESS_SHIFT));
> > /* check whether L flag was changed */
> > else if ((oldflags ^ priv->flags) & LCD_FLAG_L)
> > charlcd_backlight(lcd, !!(priv->flags & LCD_FLAG_L));
> > --
> > 2.14.3
> >
I've discovered an issue in my sasem driver, I'll send out a new version
of these patch series once that is resolved.
Thanks,
Sean
@@ -39,6 +39,8 @@
#define LCD_FLAG_F 0x0020 /* Large font mode */
#define LCD_FLAG_N 0x0040 /* 2-rows mode */
#define LCD_FLAG_L 0x0080 /* Backlight enabled */
+#define LCD_BRIGHTNESS_MASK 0x0300 /* Brightness */
+#define LCD_BRIGHTNESS_SHIFT 8
/* LCD commands */
#define LCD_CMD_DISPLAY_CLEAR 0x01 /* Clear entire display */
@@ -490,6 +492,17 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
charlcd_gotoxy(lcd);
processed = 1;
break;
+ case 'Y': /* brightness (luma) */
+ switch (esc[1]) {
+ case '0': /* 25% */
+ case '1': /* 50% */
+ case '2': /* 75% */
+ case '3': /* 100% */
+ priv->flags = (priv->flags & ~(LCD_BRIGHTNESS_MASK)) |
+ (('3' - esc[1]) << LCD_BRIGHTNESS_SHIFT);
+ processed = 1;
+ break;
+ }
}
/* TODO: This indent party here got ugly, clean it! */
@@ -507,12 +520,15 @@ static inline int handle_lcd_special_code(struct charlcd *lcd)
((priv->flags & LCD_FLAG_C) ? LCD_CMD_CURSOR_ON : 0) |
((priv->flags & LCD_FLAG_B) ? LCD_CMD_BLINK_ON : 0));
/* check whether one of F,N flags was changed */
- else if ((oldflags ^ priv->flags) & (LCD_FLAG_F | LCD_FLAG_N))
+ else if ((oldflags ^ priv->flags) & (LCD_FLAG_F | LCD_FLAG_N |
+ LCD_BRIGHTNESS_MASK))
lcd->ops->write_cmd(lcd,
LCD_CMD_FUNCTION_SET |
((lcd->ifwidth == 8) ? LCD_CMD_DATA_LEN_8BITS : 0) |
((priv->flags & LCD_FLAG_F) ? LCD_CMD_FONT_5X10_DOTS : 0) |
- ((priv->flags & LCD_FLAG_N) ? LCD_CMD_TWO_LINES : 0));
+ ((priv->flags & LCD_FLAG_N) ? LCD_CMD_TWO_LINES : 0) |
+ ((priv->flags & LCD_BRIGHTNESS_MASK) >>
+ LCD_BRIGHTNESS_SHIFT));
/* check whether L flag was changed */
else if ((oldflags ^ priv->flags) & LCD_FLAG_L)
charlcd_backlight(lcd, !!(priv->flags & LCD_FLAG_L));