Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support 16/24/32 bit color depth #70

Merged
merged 1 commit into from
Nov 23, 2024
Merged

Support 16/24/32 bit color depth #70

merged 1 commit into from
Nov 23, 2024

Conversation

huaxinliao
Copy link
Contributor

Mado currently uses 32-bit bpp, supporting display devices on desktop/laptop computers, while Raspberry Pi devices use 16-bit bpp. To improve mado’s compatibility across various Linux framebuffer environments, testing showed that
tx->fb_var.bits_per_pixel automatically sets the appropriate bpp without requiring manual configuration.

@jserv jserv requested a review from shengwen-tw November 7, 2024 14:48
@huaxinliao
Copy link
Contributor Author

huaxinliao commented Nov 7, 2024

The current demo-fbdev can output on Raspberry Pi devices, but the display output is not normal. The result is as shown in the image.
image
image

@jserv jserv marked this pull request as draft November 7, 2024 15:39
@jserv
Copy link
Contributor

jserv commented Nov 7, 2024

The current demo-fbdev can run on Raspberry Pi devices, but the display output is not normal.

Each RGB565 pixel requires 2 bytes. Typically, when using 16 bits per pixel, these bits are split to represent red, green, and blue values packed into a 16-bit word, with RGB565 as the standard format. In this layout, 5 bits are allocated to red, 6 to green, and 5 to blue, with red occupying the most significant bits in framebuffer memory. However, determining whether these bytes are stored in big-endian or little-endian format in memory can be challenging.

This means that, after arranging R, G, and B values into a 16-bit word in the application, you may need to swap the bytes if the endianness of the CPU differs from that of the display device.

To convert 8-bit R, G, and B color values to a 16-bit RGB565 format for Mado, you can use a function like this:

static uint16_t rgb888_to_rgb565(uint8_t r, uint8_t g, uint8_t b)
{
    return (((uint16_t)r & 0xF8) << 8) | (((uint16_t)g & 0xFC) << 3) | (b >> 3);
}

Pixman would be useful for providing the accelerated routines for such purpose.

This conversion function provides color translation for Mado's framebuffer backend.
Reference:

This pull request has been converted to a draft as further rework is required.

@jserv jserv requested a review from Bennctu November 7, 2024 15:59
@huaxinliao
Copy link
Contributor Author

huaxinliao commented Nov 11, 2024

In the _twin_fbdev_put_span function, I added a conversion process for handling cases where the color depth is 16 bits. For displays using 16-bit color depth, each 32-bit ARGB pixel from the pixels array is converted to the 16-bit RGB565 format before being stored in the framebuffer (dest array).

Here is the code to obtain the result in the image.

static void _twin_fbdev_put_span(twin_coord_t left,
                                 twin_coord_t top,
                                 twin_coord_t right,
                                 twin_argb32_t *pixels,
                                 void *closure)
{
    ....

    twin_coord_t width = right - left;
    off_t off = top * screen->width + left;
    uint32_t *dest =
        (uint32_t *) ((uintptr_t) tx->fb_base + (off * sizeof(*dest)));
        
+   /* If the framebuffer is in 16 bpp mode, convert each pixel from 32 bpp (ARGB) to 16 bpp (RGB565).
+    * - Extracts the red, green, and blue channels from the 32-bit color value.
+    * - Shifts and masks each channel to fit into the 16-bit RGB565 format:
+    *      - 5 bits for red
+    *      - 6 bits for green
+    *      - 5 bits for blue
+    * - Combines the shifted color values into a single 16-bit pixel.
+    * If the framebuffer is in 32 bpp mode, directly copy the pixels to the framebuffer.
+    */
+    if(tx->fb_var.bits_per_pixel == 16){
+        for (int i = 0; i < width; i++) {
+                twin_argb32_t color = pixels[i];
+                uint8_t r = (color >> 16) & 0xFF;
+                uint8_t g = (color >> 8) & 0xFF;
+                uint8_t b = color & 0xFF;
+                uint16_t pixel_value = (((uint16_t)r & 0xF8) << 8) | (((uint16_t)g & 0xFC) << 3) | ((uint16_t)b >> 3);
+                dest[i] = pixel_value;
+        }
+    }else{
+        memcpy(dest, pixels, width * sizeof(*dest));
+    }
}

image
image

Subsequently, I applied the code provided in #65 to fix the memory region in the framebuffer, resulting in the following output.

static void _twin_fbdev_put_span(twin_coord_t left,
                                 twin_coord_t top,
                                 twin_coord_t right,
                                 twin_argb32_t *pixels,
                                 void *closure)
{
    ....

    twin_coord_t width = right - left;
+   uint32_t *dest;
+   off_t off = sizeof(*dest) * left + top * tx->fb_fix.line_length;
+   dest = (uint32_t *) ((uintptr_t) tx->fb_base + off);
        
    /* If the framebuffer is in 16 bpp mode, convert each pixel from 32 bpp (ARGB) to 16 bpp (RGB565).
    * - Extracts the red, green, and blue channels from the 32-bit color value.
    * - Shifts and masks each channel to fit into the 16-bit RGB565 format:
    *      - 5 bits for red
    *      - 6 bits for green
    *      - 5 bits for blue
    * - Combines the shifted color values into a single 16-bit pixel.
    * If the framebuffer is in 32 bpp mode, directly copy the pixels to the framebuffer.
    */
    if(tx->fb_var.bits_per_pixel == 16){
        for (int i = 0; i < width; i++) {
                twin_argb32_t color = pixels[i];
                uint8_t r = (color >> 16) & 0xFF;
                uint8_t g = (color >> 8) & 0xFF;
                uint8_t b = color & 0xFF;
                uint16_t pixel_value = (((uint16_t)r & 0xF8) << 8) | (((uint16_t)g & 0xFC) << 3) | ((uint16_t)b >> 3);
                dest[i] = pixel_value;
        }
    }else{
        memcpy(dest, pixels, width * sizeof(*dest));
    }

}

image
image

@jserv

This comment was marked as resolved.

@huaxinliao

This comment was marked as resolved.

@jserv
Copy link
Contributor

jserv commented Nov 11, 2024

Consider the use of helper macro. E.g., src/gfx/convert.h from DirectFB2.

#define RGB32_TO_ARGB8565(pixel)    ( 0xff0000 | \
                                      (((pixel) & 0xf80000) >> 8) |           \
                                      (((pixel) & 0x00fc00) >> 5) |           \
                                      (((pixel) & 0x0000f8) >> 3) )

@jserv
Copy link
Contributor

jserv commented Nov 11, 2024

Determine the format of Linux framebuffer. See awtk-port/lcd_linux/fb_info.h from awtk.

@huaxinliao
Copy link
Contributor Author

huaxinliao commented Nov 13, 2024

I use a helper macro from DirectFB2 to refactor the code, making it more concise. The results are shown in the figure.

+#define RGB32_TO_ARGB8565(pixel)(((pixel & 0x00f80000) >> 8) | \
+                                 ((pixel & 0x0000fc00) >> 5) | \
+                                 ((pixel & 0x000000f8) >> 3) )

static void _twin_fbdev_put_span(twin_coord_t left,
                                 twin_coord_t top,
                                 twin_coord_t right,
                                 twin_argb32_t *pixels,
                                 void *closure)
{
    ....

    twin_coord_t width = right - left;
    uint32_t *dest;
    off_t off = sizeof(*dest) * left + top * tx->fb_fix.line_length;
    dest = (uint32_t *) ((uintptr_t) tx->fb_base + off);
        
    /* If the framebuffer is in 16 bpp mode, convert each pixel from 32 bpp (ARGB) to 16 bpp (RGB565).
    * - Extracts the red, green, and blue channels from the 32-bit color value.
    * - Shifts and masks each channel to fit into the 16-bit RGB565 format:
    *      - 5 bits for red
    *      - 6 bits for green
    *      - 5 bits for blue
    * - Combines the shifted color values into a single 16-bit pixel.
    * If the framebuffer is in 32 bpp mode, directly copy the pixels to the framebuffer.
    */
    if(tx->fb_var.bits_per_pixel == 16){
        for (int i = 0; i < width; i++) {
+              dest[i] = RGB32_TO_ARGB8565(pixels[i]);
        }
    }else{
        memcpy(dest, pixels, width * sizeof(*dest));
    }
}

S__33890339

@huaxinliao
Copy link
Contributor Author

huaxinliao commented Nov 13, 2024

Determine the format of Linux framebuffer. See awtk-port/lcd_linux/fb_info.h from awtk.

After reviewing awtk-port/lcd_linux/fb_info.h from AWTK, I noticed that mado/backend/fbdev.c does not set the format of the Linux framebuffer for 16 and 32-bit color depths. I would like to confirm if it’s possible to implement this part."

@jserv
Copy link
Contributor

jserv commented Nov 13, 2024

After reviewing awtk-port/lcd_linux/fb_info.h from AWTK, I noticed that mado/backend/fbdev.c does not set the format of the Linux framebuffer for 16 and 32-bit color depths. I would like to confirm if it’s possible to implement this part.

For the Linux framebuffer backend implementation, we shall at least distinguish the color format and apply the appropriate action or perform color format conversion.

@huaxinliao
Copy link
Contributor Author

huaxinliao commented Nov 15, 2024

After reviewing awtk-port/lcd_linux/fb_info.h from AWTK, I noticed that mado/backend/fbdev.c does not set the format of the Linux framebuffer for 16 and 32-bit color depths. I would like to confirm if it’s possible to implement this part.

For the Linux framebuffer backend implementation, we shall at least distinguish the color format and apply the appropriate action or perform color format conversion.

When implementing a function to check the color format, I referred to linux/drivers/video/pxa168fb.c from Marvell and compared it with awtk-port/lcd_linux/fb_info.h from AWTK. I noticed a difference in the offsets between the two.

The code is from Marvell.

case PIX_FMT_RGB565:
	var->bits_per_pixel = 16;
	var->red.offset = 11;    var->red.length = 5;
	var->green.offset = 5;   var->green.length = 6;
	var->blue.offset = 0;    var->blue.length = 5;
	var->transp.offset = 0;  var->transp.length = 0;
...
case PIX_FMT_RGBA888:
	var->bits_per_pixel = 32;
	var->red.offset = 16;    var->red.length = 8;
	var->green.offset = 8;   var->green.length = 8;
	var->blue.offset = 0;    var->blue.length = 8;
	var->transp.offset = 24; var->transp.length = 8;
	break;

The code is from AWTK.

static inline bool_t fb_is_rgb565(fb_info_t* fb) {
  struct fb_var_screeninfo* var = &(fb->var);
  if (var->bits_per_pixel == 16 && var->red.offset == 0 && var->green.offset == 5 &&
      var->blue.offset == 11 && var->red.length == 5 && var->green.length == 6 &&
      var->blue.length == 5) {
    return TRUE;
  } else {
    return FALSE;
  }
}

static inline bool_t fb_is_rgba8888(fb_info_t* fb) {
  struct fb_var_screeninfo* var = &(fb->var);
  if (var->bits_per_pixel == 32 && var->red.offset == 0 && var->green.offset == 8 &&
      var->blue.offset == 16 && var->red.length == 8 && var->green.length == 8 &&
      var->blue.length == 8) {
    return TRUE;
  } else {
    return FALSE;
  }
}

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refine the code changes.

@huaxinliao
Copy link
Contributor Author

huaxinliao commented Nov 16, 2024

I implemented a function to examine the Linux framebuffer format and tested it in 16 and 32 bpp environments to ensure it works correctly.

...
+static bool fb_is_rgb565(twin_fbdev_t* tx) {
+  return tx->fb_var.red.offset == 11 && tx->fb_var.red.length == 5 &&
+         tx->fb_var.green.offset == 5 && tx->fb_var.green.length == 6 && 
+         tx->fb_var.blue.offset == 0 && tx->fb_var.blue.length == 5;
+}
+
+static bool fb_is_argb32(twin_fbdev_t* tx) {
+  return tx->fb_var.red.offset == 16 && tx->fb_var.red.length == 8 &&
+         tx->fb_var.green.offset == 8 && tx->fb_var.green.length == 8 && 
+         tx->fb_var.blue.offset == 0 && tx->fb_var.blue.length == 8;
+}

static bool twin_fbdev_apply_config(twin_fbdev_t *tx)
{
    /* Read changable information of the framebuffer */
    if (ioctl(tx->fb_fd, FBIOGET_VSCREENINFO, &tx->fb_var) == -1) {
        log_error("Failed to get framebuffer information");
        return false;
    }

    /* Set the virtual screen size to be the same as the physical screen */
    tx->fb_var.xres_virtual = tx->fb_var.xres;
    tx->fb_var.yres_virtual = tx->fb_var.yres;
    
    if (ioctl(tx->fb_fd, FBIOPUT_VSCREENINFO, &tx->fb_var) < 0) {
        log_error("Failed to set framebuffer mode");
        return false;
    }

    /* Read changable information of the framebuffer again */
    if (ioctl(tx->fb_fd, FBIOGET_VSCREENINFO, &tx->fb_var) < 0) {
        log_error("Failed to get framebuffer information");
        return false;
    }
+    /* Examine the framebuffer format */
+    if (tx->fb_var.bits_per_pixel == 16) {
+        if(!fb_is_rgb565(tx)){
+            log_error("Invalid framebuffer format for 16 bpp");
+            return false;
+        }
+    }else{
+        if(!fb_is_argb32(tx)){
+            log_error("Invalid framebuffer format for 32 bpp");
+            return false;
+        }
+    }
    /* Read unchangable information of the framebuffer */
    ioctl(tx->fb_fd, FBIOGET_FSCREENINFO, &tx->fb_fix);
    
    /* Align the framebuffer memory address with the page size */
    off_t pgsize = getpagesize();
    off_t start = (off_t) tx->fb_fix.smem_start & (pgsize - 1);

    /* Round up the framebuffer memory size to match the page size */
    tx->fb_len = start + (size_t) tx->fb_fix.smem_len + (pgsize - 1);
    tx->fb_len &= ~(pgsize - 1);
    /* Map framebuffer device to the virtual memory */
    tx->fb_base = mmap(NULL, tx->fb_len, PROT_READ | PROT_WRITE, MAP_SHARED,
                       tx->fb_fd, 0);

    if (tx->fb_base == MAP_FAILED) {
        log_error("Failed to mmap framebuffer");
        return false;
    }
    return true;
}

@jserv

This comment was marked as resolved.

backend/fbdev.c Outdated Show resolved Hide resolved
backend/fbdev.c Outdated Show resolved Hide resolved
backend/fbdev.c Outdated Show resolved Hide resolved
backend/fbdev.c Outdated Show resolved Hide resolved
backend/fbdev.c Outdated Show resolved Hide resolved
backend/fbdev.c Outdated Show resolved Hide resolved
backend/fbdev.c Outdated Show resolved Hide resolved
backend/fbdev.c Outdated Show resolved Hide resolved
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebase the latest main branch.

@huaxinliao huaxinliao force-pushed the main branch 2 times, most recently from 47c0cc6 to 272be09 Compare November 21, 2024 18:22
backend/fbdev.c Outdated Show resolved Hide resolved
@jserv jserv changed the title Support 16/32 bit color depth Support 16/24/32 bit color depth Nov 22, 2024
backend/fbdev.c Outdated Show resolved Hide resolved
@jserv
Copy link
Contributor

jserv commented Nov 22, 2024

Consider the following (incomplete) macro-based code generator:

/* color conversion */
#define ARGB32_TO_RGB565_PERLINE(dest, pixels, width)   \
    do {                                                \
        for (int i = 0; i < width; i++)                 \
            dest[i] = ((pixels[i] & 0x00f80000) >> 8) | \
                      ((pixels[i] & 0x0000fc00) >> 5) | \
                      ((pixels[i] & 0x000000f8) >> 3);  \
    } while (0)

/* Requires validation in 24-bit per pixel environments. */
#define ARGB32_TO_RGB888_PERLINE(pixel)       \
    do {                                      \
        for (int i = 0; i < width; i++)       \
            dest[i] = 0xff000000 | pixels[i]; \
    } while (0)

#define ARGB32_TO_ARGB32_PERLINE(pixel) memcpy(dest, pixels, width * sizeof(*dest))

#define FBDEV_PUT_SPAN_IMPL(bpp, op)                                           \
    static void _twin_fbdev_put_span##bpp(                                     \
        twin_coord_t left, twin_coord_t top, twin_coord_t right,               \
        twin_argb32_t *pixels, void *closure)                                  \
    {                                                                          \
        uint32_t *dest;                                                        \
        twin_screen_t *screen = SCREEN(closure);                               \
        twin_fbdev_t *tx = PRIV(closure);                                      \
        off_t off = (top) * (screen->width) + (left);                          \
        *(dest) =                                                              \
            (uint32_t *) ((uintptr_t) tx->fb_base + (off * sizeof(uint32_t))); \
        twin_coord_t width = right - left;                                     \
        op(dest, pixels, width);                                               \
    }

FBDEV_PUT_SPAN_IMPL(16, ARGB32_TO_RGB565_PERLINE)
FBDEV_PUT_SPAN_IMPL(24, ARGB32_TO_RGB888_PERLINE)
FBDEV_PUT_SPAN_IMPL(32, ARGB32_TO_ARGB32_PERLINE)

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebase the latest main branch and resolve the conflicts.

backend/fbdev.c Show resolved Hide resolved
Comment on lines +26 to +57
#define ARGB32_TO_RGB565_PERLINE(dest, pixels, width) \
do { \
for (int i = 0; i < width; i++) \
dest[i] = ((pixels[i] & 0x00f80000) >> 8) | \
((pixels[i] & 0x0000fc00) >> 5) | \
((pixels[i] & 0x000000f8) >> 3); \
} while (0)

/* Requires validation in 24-bit per pixel environments */
#define ARGB32_TO_RGB888_PERLINE(dest, pixels, width) \
do { \
for (int i = 0; i < width; i++) \
dest[i] = 0xff000000 | pixels[i]; \
} while (0)
Copy link
Contributor

@jserv jserv Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bennctu, can you review these color conversion macros and suggest any potential use of Pixman for performance? These macros handle ARGB32 to RGB565 and RGB888 conversions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pixman API does not support color conversion.

@jserv jserv marked this pull request as ready for review November 22, 2024 08:43
Mado currently supports 32-bit color depth for desktop and laptop
display devices. To enhance its compatibility across diverse Linux
framebuffer environments, the following changes have been introduced to
 support 16-bit, 24-bit, and 32-bit color depths:

- Distinguish the color format and perform color format conversion.
- Ensure the correctness of the framebuffer format based on the color
  format.

Additionally, initial support for color format conversion in 24-bit per
pixel environments has been implemented. However, this implementation
requires further testing to verify its correctness and stability.

These updates expand Mado's compatibility, enabling its use on devices
with limited memory resources, such as the Raspberry Pi, while
maintaining compatibility with desktop display devices.
@jserv jserv merged commit 35ddf50 into sysprog21:main Nov 23, 2024
3 checks passed
@jserv
Copy link
Contributor

jserv commented Nov 23, 2024

Thank @p96114175 for contributing!

@jserv jserv mentioned this pull request Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants