[PATCH 1/3] video: add support for drawing 8bpp bitmap on 32bpp framebuffer

[PATCH 1/3] video: add support for drawing 8bpp bitmap on 32bpp framebuffer

[Anatolij Gustschin]

cfb_console driver supported 8bpp bitmap drawing on 24bpp/32bpp
displays and some boards used this configuration for drawing
the logo. After conversion to DM_VIDEO the logo drawing on such
boards doesn't work any more due to missing rendering code in the
updated bmp command code for DM_VIDEO. Add 8bpp bitmap support
for 32bpp displays.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/video/video-uclass.c |  6 ++++
 drivers/video/video_bmp.c    | 62 ++++++++++++++++++++++++------------
 include/video.h              |  2 +-
 3 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 12057c8a5b..44380a38d9 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -228,6 +228,12 @@ static int video_post_probe(struct udevice *dev)
 	struct udevice *cons;
 	int ret;
 
+	if (priv->bpix == VIDEO_BPP32) {
+		priv->cmap = realloc(priv->cmap, 256 * sizeof(uint));
+		if (!priv->cmap)
+			return -ENOMEM;
+	}
+
 	/* Set up the line and display size */
 	priv->fb = map_sysmem(plat->base, plat->size);
 	if (!priv->line_length)
diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c
index 8768228029..79d6c6afa5 100644
--- a/drivers/video/video_bmp.c
+++ b/drivers/video/video_bmp.c
@@ -172,16 +172,29 @@ static void video_set_cmap(struct udevice *dev,
 			   struct bmp_color_table_entry *cte, unsigned colours)
 {
 	struct video_priv *priv = dev_get_uclass_priv(dev);
+	u16 *cmap16;
+	u32 *cmap32;
 	int i;
-	ushort *cmap = priv->cmap;
 
 	debug("%s: colours=%d\n", __func__, colours);
-	for (i = 0; i < colours; ++i) {
-		*cmap = ((cte->red   << 8) & 0xf800) |
-			((cte->green << 3) & 0x07e0) |
-			((cte->blue  >> 3) & 0x001f);
-		cmap++;
-		cte++;
+	if (priv->bpix == VIDEO_BPP16) {
+		cmap16 = priv->cmap;
+		for (i = 0; i < colours; ++i) {
+			*cmap16 = ((cte->red << 8) & 0xf800) |
+				((cte->green << 3) & 0x07e0) |
+				((cte->blue  >> 3) & 0x001f);
+			cmap16++;
+			cte++;
+		}
+	} else if (priv->bpix == VIDEO_BPP32) {
+		cmap32 = priv->cmap;
+		for (i = 0; i < colours; ++i) {
+			*cmap32 = (cte->red  << 16) |
+				  (cte->green << 8) |
+				  cte->blue;
+			cmap32++;
+			cte++;
+		}
 	}
 }
 
@@ -189,7 +202,8 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
 		      bool align)
 {
 	struct video_priv *priv = dev_get_uclass_priv(dev);
-	ushort *cmap_base = NULL;
+	u32 *cmap32_base = NULL;
+	u16 *cmap16_base = NULL;
 	int i, j;
 	uchar *fb;
 	struct bmp_image *bmp = map_sysmem(bmp_image, 0);
@@ -227,11 +241,11 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
 	}
 
 	/*
-	 * We support displaying 8bpp and 24bpp BMPs on 16bpp LCDs
-	 * and displaying 24bpp BMPs on 32bpp LCDs
+	 * We support displaying 8bpp and 24bpp BMPs on 16bpp or 32bpp LCDs
 	 */
 	if (bpix != bmp_bpix &&
 	    !(bmp_bpix == 8 && bpix == 16) &&
+	    !(bmp_bpix == 8 && bpix == 32) &&
 	    !(bmp_bpix == 24 && bpix == 16) &&
 	    !(bmp_bpix == 24 && bpix == 32)) {
 		printf("Error: %d bit/pixel mode, but BMP has %d bit/pixel\n",
@@ -264,36 +278,42 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
 	switch (bmp_bpix) {
 	case 1:
 	case 8: {
-		cmap_base = priv->cmap;
 #ifdef CONFIG_VIDEO_BMP_RLE8
 		u32 compression = get_unaligned_le32(&bmp->header.compression);
 		debug("compressed %d %d\n", compression, BMP_BI_RLE8);
 		if (compression == BMP_BI_RLE8) {
 			if (bpix != 16) {
 				/* TODO implement render code for bpix != 16 */
-				printf("Error: only support 16 bpix");
+				printf("Error: only support 16 bpix\n");
 				return -EPROTONOSUPPORT;
 			}
-			video_display_rle8_bitmap(dev, bmp, cmap_base, fb, x,
+			video_display_rle8_bitmap(dev, bmp, priv->cmap, fb, x,
 						  y, width, height);
 			break;
 		}
 #endif
 
-		if (bpix != 16)
-			byte_width = width;
-		else
+		cmap16_base = priv->cmap;
+		cmap32_base = priv->cmap;
+
+		if (bpix == 16)
 			byte_width = width * 2;
+		else if (bpix == 32)
+			byte_width = width * 4;
+		else
+			byte_width = width;
 
 		for (i = 0; i < height; ++i) {
 			WATCHDOG_RESET();
 			for (j = 0; j < width; j++) {
-				if (bpix != 16) {
-					fb_put_byte(&fb, &bmap);
-				} else {
-					*(uint16_t *)fb = cmap_base[*bmap];
-					bmap++;
+				if (bpix == 16) {
+					*(uint16_t *)fb = cmap16_base[*bmap++];
 					fb += sizeof(uint16_t) / sizeof(*fb);
+				} else if (bpix == 32) {
+					*(uint32_t *)fb = cmap32_base[*bmap++];
+					fb += 4;
+				} else {
+					fb_put_byte(&fb, &bmap);
 				}
 			}
 			bmap += (padded_width - width);
diff --git a/include/video.h b/include/video.h
index e7c58e86cb..2f3183f873 100644
--- a/include/video.h
+++ b/include/video.h
@@ -93,7 +93,7 @@ struct video_priv {
 	u32 colour_fg;
 	u32 colour_bg;
 	bool flush_dcache;
-	ushort *cmap;
+	void *cmap;
 	u8 fg_col_idx;
 	u8 bg_col_idx;
 };
-- 
2.17.1

[PATCH 2/3] splash: fix banner output on small displays

[Anatolij Gustschin]

On boards with small displays (i.e. 480x272) the banner
output on video console can sometimes overwrite the logo
(depending on the logo size and banner length). Check
for free space right of the logo before drawing the
banner and adjust the starting position to draw the
banner string below the logo. Also adjust the cursor
position after banner output to avoid overwriting the
rendered banner when the user switches stdout to the
vidconsole.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 common/splash.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/common/splash.c b/common/splash.c
index e7d847726d..c12dd38975 100644
--- a/common/splash.c
+++ b/common/splash.c
@@ -123,26 +123,38 @@ void splash_get_pos(int *x, int *y)
 
 void splash_display_banner(void)
 {
+	struct vidconsole_priv *priv;
 	struct udevice *dev;
 	char buf[DISPLAY_OPTIONS_BANNER_LENGTH];
 	int col, row, ret;
+	size_t banner_len;
 
 	ret = uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &dev);
 	if (ret)
 		return;
 
+	priv = dev_get_uclass_priv(dev);
+	if (!priv)
+		return;
+
 #ifdef CONFIG_VIDEO_LOGO
 	col = BMP_LOGO_WIDTH / VIDEO_FONT_WIDTH + 1;
 	row = BMP_LOGO_HEIGHT / VIDEO_FONT_HEIGHT + 1;
 #else
 	col = 0;
-	row = 0;
+	row = 1;
 #endif
-
 	display_options_get_banner(false, buf, sizeof(buf));
-	vidconsole_position_cursor(dev, col, 1);
+
+	banner_len = strlen(buf);
+	if (priv->cols <= (col + banner_len))
+		col = 0;
+
+	vidconsole_position_cursor(dev, col, row);
 	vidconsole_put_string(dev, buf);
-	vidconsole_position_cursor(dev, 0, row);
+	if (priv->cols < banner_len)
+		row++;
+	vidconsole_position_cursor(dev, 0, row + 1);
 }
 #endif /* CONFIG_DM_VIDEO && !CONFIG_HIDE_LOGO_VERSION */
 
-- 
2.17.1

[PATCH 3/3] imx: mx6ul_14x14_evk: configure for 24bpp display

[Anatolij Gustschin]

Before DM_VIDEO conversion this board used 24bpp
display configuration, so use it again.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi b/arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi
index e9efdb9831..d0cbf79e33 100644
--- a/arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi
@@ -31,7 +31,7 @@
 	u-boot,dm-pre-reloc;
 
 	display0: display at 0 {
-		bits-per-pixel = <16>;
+		bits-per-pixel = <24>;
 		bus-width = <24>;
 
 		display-timings {
-- 
2.17.1

[PATCH 3/3] imx: mx6ul_14x14_evk: configure for 24bpp display

[Fabio Estevam]

Hi Anatolij,

On Wed, Feb 5, 2020 at 1:50 PM Anatolij Gustschin <agust@denx.de> wrote:
>
> Before DM_VIDEO conversion this board used 24bpp
> display configuration, so use it again.
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
>  arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi b/arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi
> index e9efdb9831..d0cbf79e33 100644
> --- a/arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx6ul-14x14-evk-u-boot.dtsi
> @@ -31,7 +31,7 @@
>         u-boot,dm-pre-reloc;
>
>         display0: display at 0 {
> -               bits-per-pixel = <16>;
> +               bits-per-pixel = <24>;

Yes, this fix is correct;

Reviewed-by: Fabio Estevam <festevam@gmail.com>

[PATCH 3/3] imx: mx6ul_14x14_evk: configure for 24bpp display

[sbabic at denx.de]

> Before DM_VIDEO conversion this board used 24bpp
> display configuration, so use it again.
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[PATCH 1/3] video: add support for drawing 8bpp bitmap on 32bpp framebuffer

[Igor Opaniuk]

Hi Anatolij,

On Wed, Feb 5, 2020 at 6:50 PM Anatolij Gustschin <agust@denx.de> wrote:
>
> cfb_console driver supported 8bpp bitmap drawing on 24bpp/32bpp
> displays and some boards used this configuration for drawing
> the logo. After conversion to DM_VIDEO the logo drawing on such
> boards doesn't work any more due to missing rendering code in the
> updated bmp command code for DM_VIDEO. Add 8bpp bitmap support
> for 32bpp displays.
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
>  drivers/video/video-uclass.c |  6 ++++
>  drivers/video/video_bmp.c    | 62 ++++++++++++++++++++++++------------
>  include/video.h              |  2 +-
>  3 files changed, 48 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index 12057c8a5b..44380a38d9 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -228,6 +228,12 @@ static int video_post_probe(struct udevice *dev)
>         struct udevice *cons;
>         int ret;
>
> +       if (priv->bpix == VIDEO_BPP32) {
> +               priv->cmap = realloc(priv->cmap, 256 * sizeof(uint));
> +               if (!priv->cmap)
> +                       return -ENOMEM;
> +       }
> +
>         /* Set up the line and display size */
>         priv->fb = map_sysmem(plat->base, plat->size);
>         if (!priv->line_length)
> diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c
> index 8768228029..79d6c6afa5 100644
> --- a/drivers/video/video_bmp.c
> +++ b/drivers/video/video_bmp.c
> @@ -172,16 +172,29 @@ static void video_set_cmap(struct udevice *dev,
>                            struct bmp_color_table_entry *cte, unsigned colours)
>  {
>         struct video_priv *priv = dev_get_uclass_priv(dev);
> +       u16 *cmap16;
> +       u32 *cmap32;
>         int i;
> -       ushort *cmap = priv->cmap;
>
>         debug("%s: colours=%d\n", __func__, colours);
> -       for (i = 0; i < colours; ++i) {
> -               *cmap = ((cte->red   << 8) & 0xf800) |
> -                       ((cte->green << 3) & 0x07e0) |
> -                       ((cte->blue  >> 3) & 0x001f);
> -               cmap++;
> -               cte++;
> +       if (priv->bpix == VIDEO_BPP16) {
> +               cmap16 = priv->cmap;
> +               for (i = 0; i < colours; ++i) {
> +                       *cmap16 = ((cte->red << 8) & 0xf800) |
> +                               ((cte->green << 3) & 0x07e0) |
> +                               ((cte->blue  >> 3) & 0x001f);
> +                       cmap16++;
> +                       cte++;
> +               }
> +       } else if (priv->bpix == VIDEO_BPP32) {
> +               cmap32 = priv->cmap;
> +               for (i = 0; i < colours; ++i) {
> +                       *cmap32 = (cte->red  << 16) |
> +                                 (cte->green << 8) |
> +                                 cte->blue;
> +                       cmap32++;
> +                       cte++;
> +               }
>         }
>  }
>
> @@ -189,7 +202,8 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
>                       bool align)
>  {
>         struct video_priv *priv = dev_get_uclass_priv(dev);
> -       ushort *cmap_base = NULL;
> +       u32 *cmap32_base = NULL;
> +       u16 *cmap16_base = NULL;
>         int i, j;
>         uchar *fb;
>         struct bmp_image *bmp = map_sysmem(bmp_image, 0);
> @@ -227,11 +241,11 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
>         }
>
>         /*
> -        * We support displaying 8bpp and 24bpp BMPs on 16bpp LCDs
> -        * and displaying 24bpp BMPs on 32bpp LCDs
> +        * We support displaying 8bpp and 24bpp BMPs on 16bpp or 32bpp LCDs
>          */
>         if (bpix != bmp_bpix &&
>             !(bmp_bpix == 8 && bpix == 16) &&
> +           !(bmp_bpix == 8 && bpix == 32) &&
>             !(bmp_bpix == 24 && bpix == 16) &&
>             !(bmp_bpix == 24 && bpix == 32)) {
>                 printf("Error: %d bit/pixel mode, but BMP has %d bit/pixel\n",
> @@ -264,36 +278,42 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
>         switch (bmp_bpix) {
>         case 1:
>         case 8: {
> -               cmap_base = priv->cmap;
>  #ifdef CONFIG_VIDEO_BMP_RLE8
>                 u32 compression = get_unaligned_le32(&bmp->header.compression);
>                 debug("compressed %d %d\n", compression, BMP_BI_RLE8);
>                 if (compression == BMP_BI_RLE8) {
>                         if (bpix != 16) {
>                                 /* TODO implement render code for bpix != 16 */
> -                               printf("Error: only support 16 bpix");
> +                               printf("Error: only support 16 bpix\n");
>                                 return -EPROTONOSUPPORT;
>                         }
> -                       video_display_rle8_bitmap(dev, bmp, cmap_base, fb, x,
> +                       video_display_rle8_bitmap(dev, bmp, priv->cmap, fb, x,
>                                                   y, width, height);
>                         break;
>                 }
>  #endif
>
> -               if (bpix != 16)
> -                       byte_width = width;
> -               else
> +               cmap16_base = priv->cmap;
> +               cmap32_base = priv->cmap;
> +
> +               if (bpix == 16)
>                         byte_width = width * 2;
> +               else if (bpix == 32)
> +                       byte_width = width * 4;
> +               else
> +                       byte_width = width;
>
>                 for (i = 0; i < height; ++i) {
>                         WATCHDOG_RESET();
>                         for (j = 0; j < width; j++) {
> -                               if (bpix != 16) {
> -                                       fb_put_byte(&fb, &bmap);
> -                               } else {
> -                                       *(uint16_t *)fb = cmap_base[*bmap];
> -                                       bmap++;
> +                               if (bpix == 16) {
> +                                       *(uint16_t *)fb = cmap16_base[*bmap++];
>                                         fb += sizeof(uint16_t) / sizeof(*fb);
> +                               } else if (bpix == 32) {
> +                                       *(uint32_t *)fb = cmap32_base[*bmap++];
> +                                       fb += 4;
> +                               } else {
> +                                       fb_put_byte(&fb, &bmap);
>                                 }
>                         }
>                         bmap += (padded_width - width);
> diff --git a/include/video.h b/include/video.h
> index e7c58e86cb..2f3183f873 100644
> --- a/include/video.h
> +++ b/include/video.h
> @@ -93,7 +93,7 @@ struct video_priv {
>         u32 colour_fg;
>         u32 colour_bg;
>         bool flush_dcache;
> -       ushort *cmap;
> +       void *cmap;
>         u8 fg_col_idx;
>         u8 bg_col_idx;
>  };
> --
> 2.17.1
>

Any chance to get this merged?

Thanks

-- 
Best regards - Freundliche Gr?sse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[PATCH 1/3] video: add support for drawing 8bpp bitmap on 32bpp framebuffer

[Anatolij Gustschin]

Hi Igor,

On Tue, 23 Jun 2020 14:40:45 +0300
Igor Opaniuk igor.opaniuk at gmail.com wrote:
... 
> Any chance to get this merged?

We had a display issue on mx6ul_14x14_evk with these patches applied,
I didn't have time to debug this further. From what I remember now,
it was not exactly clear if this issue is caused by other mx6ul SoC
or driver changes in mainline or if this is the result of the mx6ul
DM_VIDEO conversion.

Also, there is another patch [1] addressing this drawing problem.
I hope to find time later this week to decide which patch should
be merged better.

[1] http://patchwork.ozlabs.org/project/uboot/patch/1591782743-22846-3-git-send-email-ye.li at nxp.com

--
Anatolij

[PATCH 1/3] video: add support for drawing 8bpp bitmap on 32bpp framebuffer

[Anatolij Gustschin]

Hi Igor,

On Tue, 23 Jun 2020 14:40:45 +0300
Igor Opaniuk igor.opaniuk at gmail.com wrote:
... 
> Any chance to get this merged?

I've merged another reworked patch to fix the logo drawing problem.

--
Anatolij

[PATCH 1/3] video: add support for drawing 8bpp bitmap on 32bpp framebuffer

[Igor Opaniuk]

Hi Anatolij,

On Mon, Jun 29, 2020, 10:52 Anatolij Gustschin <agust@denx.de> wrote:

> Hi Igor,
>
> On Tue, 23 Jun 2020 14:40:45 +0300
> Igor Opaniuk igor.opaniuk at gmail.com wrote:
> ...
> > Any chance to get this merged?
>
> I've merged another reworked patch to fix the logo drawing problem.
>
Thanks, I also successfully tested it on Toradex modules.


> --
> Anatolij
>

[PATCH 1/3] video: add support for drawing 8bpp bitmap on 32bpp framebuffer

[Anatolij Gustschin]

Hi Igor,

On Fri, 3 Jul 2020 15:42:08 +0300
Igor Opaniuk igor.opaniuk at gmail.com wrote:
... 
> Thanks, I also successfully tested it on Toradex modules.

OK, thanks for testing!

--
Anatolij