| Message ID | cover.1689252746.git.geert@linux-m68k.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: fb-helper/ssd130x: Add support for DRM_FORMAT_R1 | expand |
Geert Uytterhoeven <geert@linux-m68k.org> writes: Hello Geert, > Hi all, > > The native display format of ssd1306 OLED displays is monochrome > light-on-dark (R1). This patch series adds support for the R1 buffer > format to both the ssd130x DRM driver and the FB helpers, so monochrome > applications (including fbdev emulation and the text console) can avoid > the overhead of back-and-forth conversions between R1 and XR24. > I've tested your series on a ssd1306 I2C OLED panel and fbcon did work for me, but had some issues when trying to run your fbtest suite. Because the test005 gets killed with a SIGSEGV. $ ./fbtest -d fb_init() fb_open() fb_get_var() fb_get_fix() fb_map() fb_start = 0, fb_offset = 0, fb_len = 1000 fb_save() fb_clear() Using drawops planar (monochrome and (interleaved) bitplanes) Available visuals: Monochrome Grayscale 2 Using visops monochrome Running all tests Running test test001 test001: PASSED Running test test002 test002: PASSED Running test test003 Requirement num_colors >= 16 not met Running test test004 test004: PASSED Running test test005 Caught signal 11. Exiting fb_cleanup() fb_restore() fb_unmap() fb_set_var() fb_get_var() fb_get_fix() fb_close() Maybe more tests are missing the minimum num_colors requirement? Also, the penguin in test004 is not displayed correctly. I was expecting that to be working correctly since you mentioned to be using the Linux logo on boot. Another question, do you know if is possible to change the default format? I believe that fbset won't work because the DRM fbdev emulation layer does not implement mode settings but I thought that changing the mode using the atomic KMS API would work. $ modetest -M ssd130x -P 31@33:128x64@XR24 -a $ echo $? 0 but after that I still get: $ fbset -i mode "128x64" geometry 128 64 128 64 1 timings 0 0 0 0 0 0 0 rgba 1/0,1/0,1/0,0/0 endmode Frame buffer device information: Name : ssd130xdrmfb Address : (nil) Size : 4096 Type : PACKED PIXELS Visual : MONO10 XPanStep : 1 YPanStep : 1 YWrapStep : 0 LineLength : 16 Accelerator : No Maybe I'm doing something wrong or misunderstading about how should work?
Hi Javier, On Sun, Jul 16, 2023 at 3:30 PM Javier Martinez Canillas <javierm@redhat.com> wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: > > The native display format of ssd1306 OLED displays is monochrome > > light-on-dark (R1). This patch series adds support for the R1 buffer > > format to both the ssd130x DRM driver and the FB helpers, so monochrome > > applications (including fbdev emulation and the text console) can avoid > > the overhead of back-and-forth conversions between R1 and XR24. > > I've tested your series on a ssd1306 I2C OLED panel and fbcon did work for > me, but had some issues when trying to run your fbtest suite. Because the Thanks, due to the limited userspace environment on my RV32 test system, I didn't run fbtest myself. > test005 gets killed with a SIGSEGV. > > $ ./fbtest -d > fb_init() > fb_open() > fb_get_var() > fb_get_fix() > fb_map() > fb_start = 0, fb_offset = 0, fb_len = 1000 [...] > Running test test005 > Caught signal 11. Exiting Strange. > Maybe more tests are missing the minimum num_colors requirement? Also, the On monochrome, test005 should make the left half of the screen black, and the right half white. It works on ARAnyM, and there don't seem to be off-by-one errors in the call to fill_rect(). Can you please run this under gdb or valgrind? > penguin in test004 is not displayed correctly. I was expecting that to be > working correctly since you mentioned to be using the Linux logo on boot. Linux has logos for displays using 2, 16, and 256 colors. Note that the default logos are 80x80, which is larger than your display, so no logo is drawn. Fbtest has only the full color logo, so it will look bad on a monochrome display. > Another question, do you know if is possible to change the default format? AFAIK DRM does not support that. > I believe that fbset won't work because the DRM fbdev emulation layer does > not implement mode settings but I thought that changing the mode using the Correct. > atomic KMS API would work. > > $ modetest -M ssd130x -P 31@33:128x64@XR24 -a > $ echo $? > 0 > > but after that I still get: > > $ fbset -i > > mode "128x64" > geometry 128 64 128 64 1 > timings 0 0 0 0 0 0 0 > rgba 1/0,1/0,1/0,0/0 > endmode > > Frame buffer device information: > Name : ssd130xdrmfb > Address : (nil) > Size : 4096 > Type : PACKED PIXELS > Visual : MONO10 > XPanStep : 1 > YPanStep : 1 > YWrapStep : 0 > LineLength : 16 > Accelerator : No > > Maybe I'm doing something wrong or misunderstading about how should work? Do you need the "-d" option (drop master after mode set) of modetest? Still, that would only impact DRM. The fbdev emulation on top of DRM has already been initialized before, and is never reinitialized. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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
Geert Uytterhoeven <geert@linux-m68k.org> writes: Hello Geert, > Hi Javier, > > On Sun, Jul 16, 2023 at 3:30 PM Javier Martinez Canillas > <javierm@redhat.com> wrote: >> Geert Uytterhoeven <geert@linux-m68k.org> writes: >> > The native display format of ssd1306 OLED displays is monochrome >> > light-on-dark (R1). This patch series adds support for the R1 buffer >> > format to both the ssd130x DRM driver and the FB helpers, so monochrome >> > applications (including fbdev emulation and the text console) can avoid >> > the overhead of back-and-forth conversions between R1 and XR24. >> >> I've tested your series on a ssd1306 I2C OLED panel and fbcon did work for >> me, but had some issues when trying to run your fbtest suite. Because the > > Thanks, due to the limited userspace environment on my RV32 test system, > I didn't run fbtest myself. > You are welcome. >> test005 gets killed with a SIGSEGV. >> >> $ ./fbtest -d >> fb_init() >> fb_open() >> fb_get_var() >> fb_get_fix() >> fb_map() >> fb_start = 0, fb_offset = 0, fb_len = 1000 > > [...] > >> Running test test005 >> Caught signal 11. Exiting > > Strange. > >> Maybe more tests are missing the minimum num_colors requirement? Also, the > > On monochrome, test005 should make the left half of the screen black, > and the right half white. It works on ARAnyM, and there don't seem > to be off-by-one errors in the call to fill_rect(). > Can you please run this under gdb or valgrind? > Sure. I only spent my free time on these panels though so likely will do during the week or more likely the weekend. I believe the bug is somewhere in the test though and not in your kernel patches. >> penguin in test004 is not displayed correctly. I was expecting that to be >> working correctly since you mentioned to be using the Linux logo on boot. > > Linux has logos for displays using 2, 16, and 256 colors. Note that the > default logos are 80x80, which is larger than your display, so no logo > is drawn. > Fbtest has only the full color logo, so it will look bad on a monochrome > display. > I see. Should the test check for minimum num_colors and skip that test then? >> Another question, do you know if is possible to change the default format? > > AFAIK DRM does not support that. > >> I believe that fbset won't work because the DRM fbdev emulation layer does >> not implement mode settings but I thought that changing the mode using the > > Correct. > [...] >> Maybe I'm doing something wrong or misunderstading about how should work? > > Do you need the "-d" option (drop master after mode set) of modetest? > Still, that would only impact DRM. The fbdev emulation on top of DRM has > already been initialized before, and is never reinitialized. > Ah, that explains and makes sense now. I tested other user-space fbdev programs that only support XRGB8888 and those were working correctly so I guess that user-space that supports R1 be defaulted to that and not able to change is an acceptable behaviour. When you post v2, feel free to add: Tested-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Javier, On Mon, Jul 17, 2023 at 11:13 AM Javier Martinez Canillas <javierm@redhat.com> wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: > > On Sun, Jul 16, 2023 at 3:30 PM Javier Martinez Canillas > > <javierm@redhat.com> wrote: > >> Geert Uytterhoeven <geert@linux-m68k.org> writes: > >> > The native display format of ssd1306 OLED displays is monochrome > >> > light-on-dark (R1). This patch series adds support for the R1 buffer > >> > format to both the ssd130x DRM driver and the FB helpers, so monochrome > >> > applications (including fbdev emulation and the text console) can avoid > >> > the overhead of back-and-forth conversions between R1 and XR24. > >> > >> I've tested your series on a ssd1306 I2C OLED panel and fbcon did work for > >> me, but had some issues when trying to run your fbtest suite. Because the > > > > Thanks, due to the limited userspace environment on my RV32 test system, > > I didn't run fbtest myself. > > > > You are welcome. > > >> test005 gets killed with a SIGSEGV. > >> > >> $ ./fbtest -d > >> fb_init() > >> fb_open() > >> fb_get_var() > >> fb_get_fix() > >> fb_map() > >> fb_start = 0, fb_offset = 0, fb_len = 1000 > > > > [...] > > > >> Running test test005 > >> Caught signal 11. Exiting > > > > Strange. > > > >> Maybe more tests are missing the minimum num_colors requirement? Also, the > > > > On monochrome, test005 should make the left half of the screen black, > > and the right half white. It works on ARAnyM, and there don't seem > > to be off-by-one errors in the call to fill_rect(). > > Can you please run this under gdb or valgrind? > > > > Sure. I only spent my free time on these panels though so likely will do > during the week or more likely the weekend. I believe the bug is somewhere > in the test though and not in your kernel patches. OK. > >> penguin in test004 is not displayed correctly. I was expecting that to be > >> working correctly since you mentioned to be using the Linux logo on boot. > > > > Linux has logos for displays using 2, 16, and 256 colors. Note that the > > default logos are 80x80, which is larger than your display, so no logo > > is drawn. > > Fbtest has only the full color logo, so it will look bad on a monochrome > > display. > > I see. Should the test check for minimum num_colors and skip that test then? The test still works (you did see an ugly black-and-white penguin), doesn't it? > When you post v2, feel free to add: > > Tested-by: Javier Martinez Canillas <javierm@redhat.com> Thanks! Gr{oetje,eeting}s, Geert
Geert Uytterhoeven <geert@linux-m68k.org> writes: > Hi Javier, [...] >> >> penguin in test004 is not displayed correctly. I was expecting that to be >> >> working correctly since you mentioned to be using the Linux logo on boot. >> > >> > Linux has logos for displays using 2, 16, and 256 colors. Note that the >> > default logos are 80x80, which is larger than your display, so no logo >> > is drawn. >> > Fbtest has only the full color logo, so it will look bad on a monochrome >> > display. >> >> I see. Should the test check for minimum num_colors and skip that test then? > > The test still works (you did see an ugly black-and-white penguin), doesn't it? > Fair enough. But when it defaulted to XRGB8888, it looked better. So I thought that it was a regression. No strong opinion though if the test should be skipped or not.
Hi Javier, On Mon, Jul 17, 2023 at 11:33 AM Javier Martinez Canillas <javierm@redhat.com> wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: > >> >> penguin in test004 is not displayed correctly. I was expecting that to be > >> >> working correctly since you mentioned to be using the Linux logo on boot. > >> > > >> > Linux has logos for displays using 2, 16, and 256 colors. Note that the > >> > default logos are 80x80, which is larger than your display, so no logo > >> > is drawn. > >> > Fbtest has only the full color logo, so it will look bad on a monochrome > >> > display. > >> > >> I see. Should the test check for minimum num_colors and skip that test then? > > > > The test still works (you did see an ugly black-and-white penguin), doesn't it? > > Fair enough. But when it defaulted to XRGB8888, it looked better. So I > thought that it was a regression. No strong opinion though if the test > should be skipped or not. IC, fbtest's mono_match_color() just finds the closest color (black or white), while drm_fb_xrgb8888_to_gray8_line() uses a weighted average of the RGB components. That might make a small but visible difference. We could make it look even better using Floyd-Steinberg dithering... ;-) Fbtest does have an unused match_color_error() helper, so I must have had that in mind, initially... Gr{oetje,eeting}s, Geert
Geert Uytterhoeven <geert@linux-m68k.org> writes: > Hi Javier, > > On Mon, Jul 17, 2023 at 11:33 AM Javier Martinez Canillas > <javierm@redhat.com> wrote: >> Geert Uytterhoeven <geert@linux-m68k.org> writes: >> >> >> penguin in test004 is not displayed correctly. I was expecting that to be >> >> >> working correctly since you mentioned to be using the Linux logo on boot. >> >> > >> >> > Linux has logos for displays using 2, 16, and 256 colors. Note that the >> >> > default logos are 80x80, which is larger than your display, so no logo >> >> > is drawn. >> >> > Fbtest has only the full color logo, so it will look bad on a monochrome >> >> > display. >> >> >> >> I see. Should the test check for minimum num_colors and skip that test then? >> > >> > The test still works (you did see an ugly black-and-white penguin), doesn't it? >> >> Fair enough. But when it defaulted to XRGB8888, it looked better. So I >> thought that it was a regression. No strong opinion though if the test >> should be skipped or not. > > IC, fbtest's mono_match_color() just finds the closest color (black or > white), while drm_fb_xrgb8888_to_gray8_line() uses a weighted average > of the RGB components. That might make a small but visible difference. > > We could make it look even better using Floyd-Steinberg dithering... ;-) > Fbtest does have an unused match_color_error() helper, so I must have > had that in mind, initially... > Interesting. I'll take a look to that if I have time! But as mentioned, all that is something to improve in your fbtest suite and the ssd130x changes in the driver do look good to me and are working as expected. > Gr{oetje,eeting}s, >
Hi all, The native display format of ssd1306 OLED displays is monochrome light-on-dark (R1). This patch series adds support for the R1 buffer format to both the ssd130x DRM driver and the FB helpers, so monochrome applications (including fbdev emulation and the text console) can avoid the overhead of back-and-forth conversions between R1 and XR24. This series is based on drm-misc/for-linux-next with [1] applied, and consists of 4 parts: - Patches 1-3 contain miscellaneous fixes, - Patch 4 adds R1 support to the ssd130x DRM driver, - Patches 5-6 update the DRM client and FB helper code to avoid calling drm_mode_legacy_fb_format() where the exact buffer format is already known, to prepare for R1 support, - Patch 7 adds support for R1 to fbdev emulation and the text console, - Patch 8 switches ssd130x to R1 for fbdev emulation and the text console. This has been tested on an Adafruit FeatherWing 128x32 OLED, connected to an OrangeCrab ECP5 FPGA board running a 64 MHz VexRiscv RISC-V softcore, using the fbdev text console. Thanks for your comments! P.S. Note that the biggest hurdle was the copious use of the drm_mode_legacy_fb_format() helper in various places. This helper cannot decide between C1 and R1 without knowledge of the capabilities of the full display pipeline. Instead of special-casing its return value in three callers, I did so in only one place, and got rid of two of these calls in the call chain. I think Thomas' grand plan is to replace preferred_{bpp,depth} by a preferred fourcc format? That would simplify things a lot... [1] "[PATCH] drm/ssd130x: Change pixel format used to compute the buffer size" https://lore.kernel.org/all/20230713085859.907127-1-javierm@redhat.com Geert Uytterhoeven (8): drm/ssd130x: Fix pitch calculation in ssd130x_fb_blit_rect() drm/dumb-buffers: Fix drm_mode_create_dumb() for bpp < 8 [RFC] drm/ssd130x: Bail out early if data_array is not yet available drm/ssd130x: Add support for DRM_FORMAT_R1 drm/client: Convert drm_mode_create_dumb() to drm_mode_addfb2() drm/fb-helper: Pass buffer format via drm_fb_helper_surface_size drm/fb-helper: Add support for DRM_FORMAT_R1 drm/ssd130x: Switch preferred_bpp/depth to 1 drivers/gpu/drm/drm_client.c | 13 +++--- drivers/gpu/drm/drm_dumb_buffers.c | 3 +- drivers/gpu/drm/drm_fb_helper.c | 42 ++++++++++++++----- drivers/gpu/drm/drm_fbdev_generic.c | 9 ++--- drivers/gpu/drm/solomon/ssd130x.c | 62 ++++++++++++++++++++--------- include/drm/drm_fb_helper.h | 2 + 6 files changed, 87 insertions(+), 44 deletions(-)