diff mbox series

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

Message ID 20200205164959.31404-1-agust@denx.de
State New
Headers show
Series [1/3] video: add support for drawing 8bpp bitmap on 32bpp framebuffer | expand

Commit Message

Anatolij Gustschin Feb. 5, 2020, 4:49 p.m. UTC 
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 at 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(-)

Comments

Igor Opaniuk June 23, 2020, 11:40 a.m. UTC | #1 
Hi Anatolij,

On Wed, Feb 5, 2020 at 6:50 PM Anatolij Gustschin <agust at 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 at 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
Anatolij Gustschin June 23, 2020, 12:06 p.m. UTC | #2 
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
Anatolij Gustschin June 29, 2020, 7:52 a.m. UTC | #3 
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
Igor Opaniuk July 3, 2020, 12:42 p.m. UTC | #4 
Hi Anatolij,

On Mon, Jun 29, 2020, 10:52 Anatolij Gustschin <agust at 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
>
Anatolij Gustschin July 3, 2020, 12:47 p.m. UTC | #5 
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
diff mbox series

Patch

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;
 };