[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdUuTAsqpx4=WnosfyjLo-t9ryQPQMaE1OeMBk4Ws9DTpQ@mail.gmail.com>
Date: Wed, 9 Mar 2022 21:04:03 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Javier Martinez Canillas <javierm@...hat.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Thomas Zimmermann <tzimmermann@...e.de>,
Sam Ravnborg <sam@...nborg.org>,
DRI Development <dri-devel@...ts.freedesktop.org>,
Maxime Ripard <maxime@...no.tech>,
Noralf Trønnes <noralf@...nnes.org>,
Daniel Vetter <daniel@...ll.ch>,
David Airlie <airlied@...ux.ie>,
Lee Jones <lee.jones@...aro.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thierry Reding <thierry.reding@...il.com>,
Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>,
Linux PWM List <linux-pwm@...r.kernel.org>
Subject: Re: [PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
Hi Javier,
On Mon, Feb 14, 2022 at 2:37 PM Javier Martinez Canillas
<javierm@...hat.com> 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@...hat.com>
> --- /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@...ux-m68k.org
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
Powered by blists - more mailing lists