Hi Javier, On Mon, Feb 14, 2022 at 2:37 PM Javier Martinez Canillas <javierm@xxxxxxxxxx> wrote: > This adds a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon > OLED display controllers. > > It's only the core part of the driver and a bus specific driver is needed > for each transport interface supported by the display controllers. > > Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> > --- /dev/null > +++ b/drivers/gpu/drm/solomon/ssd130x.c > +static int ssd130x_update_rect(struct ssd130x_device *ssd130x, u8 *buf, > + struct drm_rect *rect) > +{ > + unsigned int x = rect->x1; > + unsigned int y = rect->y1; > + unsigned int width = drm_rect_width(rect); > + unsigned int height = drm_rect_height(rect); > + unsigned int line_length = DIV_ROUND_UP(width, 8); > + unsigned int pages = DIV_ROUND_UP(y % 8 + height, 8); > + u32 array_idx = 0; > + int ret, i, j, k; > + u8 *data_array = NULL; > + > + data_array = kcalloc(width, pages, GFP_KERNEL); > + if (!data_array) > + return -ENOMEM; > + > + /* > + * The screen is divided in pages, each having a height of 8 > + * pixels, and the width of the screen. When sending a byte of > + * data to the controller, it gives the 8 bits for the current > + * column. I.e, the first byte are the 8 bits of the first > + * column, then the 8 bits for the second column, etc. > + * > + * > + * Representation of the screen, assuming it is 5 bits > + * wide. Each letter-number combination is a bit that controls > + * one pixel. > + * > + * A0 A1 A2 A3 A4 > + * B0 B1 B2 B3 B4 > + * C0 C1 C2 C3 C4 > + * D0 D1 D2 D3 D4 > + * E0 E1 E2 E3 E4 > + * F0 F1 F2 F3 F4 > + * G0 G1 G2 G3 G4 > + * H0 H1 H2 H3 H4 > + * > + * If you want to update this screen, you need to send 5 bytes: > + * (1) A0 B0 C0 D0 E0 F0 G0 H0 > + * (2) A1 B1 C1 D1 E1 F1 G1 H1 > + * (3) A2 B2 C2 D2 E2 F2 G2 H2 > + * (4) A3 B3 C3 D3 E3 F3 G3 H3 > + * (5) A4 B4 C4 D4 E4 F4 G4 H4 > + */ > + > + ret = ssd130x_set_col_range(ssd130x, ssd130x->col_offset + x, width); > + if (ret < 0) > + goto out_free; > + > + ret = ssd130x_set_page_range(ssd130x, ssd130x->page_offset + y / 8, pages); > + if (ret < 0) > + goto out_free; > + > + for (i = y / 8; i < y / 8 + pages; i++) { > + int m = 8; > + > + /* Last page may be partial */ > + if (8 * (i + 1) > ssd130x->height) > + m = ssd130x->height % 8; > + for (j = x; j < x + width; j++) { > + u8 data = 0; > + > + for (k = 0; k < m; k++) { > + u8 byte = buf[(8 * i + k) * line_length + j / 8]; As buf does not point to (0, 0), the above is not correct if rect.x1 != 0 or rect.y1 != 0. After fixing that, writing more than one text line to the console works, but I still see an issue with updates where the rectangle size and/or position are not aligned to 8 pixels horizontally. Will do more investigation, and send fixes... > + u8 bit = (byte >> (j % 8)) & 1; > + > + data |= bit << k; > + } > + data_array[array_idx++] = data; > + } > + } > + > + ret = ssd130x_write_data(ssd130x, data_array, width * pages); > + > +out_free: > + kfree(data_array); > + return ret; > +} > + > +static void ssd130x_clear_screen(struct ssd130x_device *ssd130x) > +{ > + u8 *buf = NULL; > + struct drm_rect fullscreen = { > + .x1 = 0, > + .x2 = ssd130x->width, > + .y1 = 0, > + .y2 = ssd130x->height, > + }; > + > + buf = kcalloc(ssd130x->width, ssd130x->height, GFP_KERNEL); This buffer is larger than needed. Will send a fix. > + if (!buf) > + return; > + > + ssd130x_update_rect(ssd130x, buf, &fullscreen); > + > + kfree(buf); > +} > + > +static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct dma_buf_map *map, > + struct drm_rect *rect) > +{ > + struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev); > + void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */ > + int ret = 0; > + u8 *buf = NULL; > + > + buf = kcalloc(fb->width, fb->height, GFP_KERNEL); This buffer is much larger than needed. Will send a fix. > + if (!buf) > + return -ENOMEM; > + > + drm_fb_xrgb8888_to_mono_reversed(buf, 0, vmap, fb, rect); > + > + ssd130x_update_rect(ssd130x, buf, rect); > + > + kfree(buf); > + > + return ret; > +} Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds