From b08714bb8fcca13b6765fd2a1c8beaa848b5d758 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Fri, 28 Apr 2023 17:13:17 -0700 Subject: [PATCH] Take in framebuffer resolution, not output res Fixes #7911 --- .../bindings/picodvi/Framebuffer.c | 16 ++++---- .../adafruit_feather_rp2040_dvi/board.c | 2 +- .../common-hal/picodvi/Framebuffer.c | 37 ++++++++++--------- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/ports/raspberrypi/bindings/picodvi/Framebuffer.c b/ports/raspberrypi/bindings/picodvi/Framebuffer.c index e4584370af..4e522d0f5f 100644 --- a/ports/raspberrypi/bindings/picodvi/Framebuffer.c +++ b/ports/raspberrypi/bindings/picodvi/Framebuffer.c @@ -53,7 +53,7 @@ //| blue_dn: microcontroller.Pin, //| color_depth: int = 8, //| ) -> None: -//| """Create a Framebuffer object with the given dimensions (640x480 or 800x480). Memory is +//| """Create a Framebuffer object with the given dimensions. Memory is //| allocated outside of onto the heap and then moved outside on VM //| end. //| @@ -67,22 +67,22 @@ //| less than dn for all pairs or dp must be greater than dn for all pairs. //| //| The framebuffer pixel format varies depending on color_depth: +//| //| * 1 - Each bit is a pixel. Either white (1) or black (0). //| * 2 - Each 2 bits is a pixels. Grayscale between white (0x3) and black (0x0). //| * 8 - Each byte is a pixels in RGB332 format. //| * 16 - Each two bytes are a pixel in RGB565 format. //| -//| Monochrome framebuffers (color_depth=1 or 2) will be full resolution. -//| Color framebuffers will be half resolution and pixels will be -//| duplicated to create a signal with the target dimensions. +//| Two output resolutions are currently supported, 640x480 and 800x480. +//| Monochrome framebuffers (color_depth=1 or 2) must be full resolution. +//| Color framebuffers must be half resolution (320x240 or 400x240) and +//| pixels will be duplicated to create the signal. //| //| A Framebuffer is often used in conjunction with a //| `framebufferio.FramebufferDisplay`. //| -//| :param int width: the width of the target display signal. It will be halved when -//| color_depth >= 8 when creating the framebuffer. Only 640 or 800 is currently supported. -//| :param int height: the height of the target display signal. It will be halved when -//| color_depth >= 8 when creating the framebuffer. Only 480 is currently supported. +//| :param int width: the width of the target display signal. Only 320, 400, 640 or 800 is currently supported depending on color_depth. +//| :param int height: the height of the target display signal. Only 240 or 480 is currently supported depenting on color_depth. //| :param ~microcontroller.Pin clk_dp: the positive clock signal pin //| :param ~microcontroller.Pin clk_dn: the negative clock signal pin //| :param ~microcontroller.Pin red_dp: the positive red signal pin diff --git a/ports/raspberrypi/boards/adafruit_feather_rp2040_dvi/board.c b/ports/raspberrypi/boards/adafruit_feather_rp2040_dvi/board.c index 140a15cc04..c6112a5555 100644 --- a/ports/raspberrypi/boards/adafruit_feather_rp2040_dvi/board.c +++ b/ports/raspberrypi/boards/adafruit_feather_rp2040_dvi/board.c @@ -33,7 +33,7 @@ void board_init(void) { picodvi_framebuffer_obj_t *fb = &allocate_display_bus()->picodvi; fb->base.type = &picodvi_framebuffer_type; - common_hal_picodvi_framebuffer_construct(fb, 640, 480, + common_hal_picodvi_framebuffer_construct(fb, 320, 240, &pin_GPIO17, &pin_GPIO16, &pin_GPIO19, &pin_GPIO18, &pin_GPIO21, &pin_GPIO20, diff --git a/ports/raspberrypi/common-hal/picodvi/Framebuffer.c b/ports/raspberrypi/common-hal/picodvi/Framebuffer.c index 20f447a824..5629d67d1c 100644 --- a/ports/raspberrypi/common-hal/picodvi/Framebuffer.c +++ b/ports/raspberrypi/common-hal/picodvi/Framebuffer.c @@ -138,23 +138,25 @@ void common_hal_picodvi_framebuffer_construct(picodvi_framebuffer_obj_t *self, const mcu_pin_obj_t *green_dp, const mcu_pin_obj_t *green_dn, const mcu_pin_obj_t *blue_dp, const mcu_pin_obj_t *blue_dn, mp_uint_t color_depth) { - - const struct dvi_timing *timing = NULL; - if (width == 640 && height == 480) { - timing = &dvi_timing_640x480p_60hz; - } else if (width == 800 && height == 480) { - timing = &dvi_timing_800x480p_60hz; - } else { - if (height == 480) { - mp_raise_ValueError_varg(translate("%q must be %d"), MP_QSTR_width, 480); - } - mp_raise_ValueError_varg(translate("Invalid %q"), MP_QSTR_height); - } - if (active_picodvi != NULL) { mp_raise_msg_varg(&mp_type_RuntimeError, translate("%q in use"), MP_QSTR_picodvi); } + bool color_framebuffer = color_depth >= 8; + const struct dvi_timing *timing = NULL; + if ((!color_framebuffer && width == 640 && height == 480) || + (color_framebuffer && width == 320 && height == 240)) { + timing = &dvi_timing_640x480p_60hz; + } else if ((!color_framebuffer && width == 800 && height == 480) || + (color_framebuffer && width == 400 && height == 240)) { + timing = &dvi_timing_800x480p_60hz; + } else { + if (height != 480 && height != 240) { + mp_raise_ValueError_varg(translate("Invalid %q"), MP_QSTR_height); + } + mp_raise_ValueError_varg(translate("Invalid %q"), MP_QSTR_width); + } + bool invert_diffpairs = clk_dn->number < clk_dp->number; int8_t other_pins[4]; int8_t *a; @@ -214,12 +216,12 @@ void common_hal_picodvi_framebuffer_construct(picodvi_framebuffer_obj_t *self, self->height = height; size_t tmds_bufs_per_scanline; - if (color_depth >= 8) { + size_t scanline_width = width; + if (color_framebuffer) { dvi_vertical_repeat = 2; dvi_monochrome_tmds = false; - self->width /= 2; - self->height /= 2; tmds_bufs_per_scanline = 3; + scanline_width *= 2; } else { dvi_vertical_repeat = 1; dvi_monochrome_tmds = true; @@ -233,8 +235,7 @@ void common_hal_picodvi_framebuffer_construct(picodvi_framebuffer_obj_t *self, } self->pitch /= sizeof(uint32_t); size_t framebuffer_size = self->pitch * self->height; - // use width here because it hasn't been downsized for the frame buffer - self->tmdsbuf_size = tmds_bufs_per_scanline * width / DVI_SYMBOLS_PER_WORD + 1; + self->tmdsbuf_size = tmds_bufs_per_scanline * scanline_width / DVI_SYMBOLS_PER_WORD + 1; size_t total_allocation_size = sizeof(uint32_t) * (framebuffer_size + DVI_N_TMDS_BUFFERS * self->tmdsbuf_size); self->allocation = allocate_memory(total_allocation_size, false, true); if (self->allocation == NULL) {