DRM driver is slow

mcerveny · October 15, 2020, 8:25pm

Description

New DRM driver code does not fulfill my expectation.

performance is very low - lv_example/benchmark is 6 FPS with DRM but 187 FPS / 84% with FBDEV
many compiler of warnings (see below)

What MCU/Processor/Board and compiler are you using?

Allwinner V3s + display 800x480 (RGB)
gcc (Debian 8.3.0-6) 8.3.0

What do you experience?

compiler warnings:

/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:71:10: warning: no previous prototype for ‘get_plane_property_id’ [-Wmissing-prototypes]
 uint32_t get_plane_property_id(const char *name)
          ^~~~~~~~~~~~~~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:86:10: warning: no previous prototype for ‘get_crtc_property_id’ [-Wmissing-prototypes]
 uint32_t get_crtc_property_id(const char *name)
          ^~~~~~~~~~~~~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:101:10: warning: no previous prototype for ‘get_conn_property_id’ [-Wmissing-prototypes]
 uint32_t get_conn_property_id(const char *name)
          ^~~~~~~~~~~~~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c: In function ‘drm_add_plane_property’:
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:34:30: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ {aka ‘long long unsigned int’} [-Wformat=]
 #define err(msg, ...)  print("error: " msg "\n", ##__VA_ARGS__)
                              ^~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:197:61:
   err("drmModeAtomicAddProperty (%s:%lu) failed: %d", name, value, ret);
                                                             ~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:33:41: note: in definition of macro ‘print’
 #define print(msg, ...) fprintf(stderr, msg, ##__VA_ARGS__);
                                         ^~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:197:3: note: in expansion of macro ‘err’
   err("drmModeAtomicAddProperty (%s:%lu) failed: %d", name, value, ret);
   ^~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:197:39: note: format string is defined here
   err("drmModeAtomicAddProperty (%s:%lu) failed: %d", name, value, ret);
                                     ~~^
                                     %llu
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c: In function ‘drm_add_crtc_property’:
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:34:30: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ {aka ‘long long unsigned int’} [-Wformat=]
 #define err(msg, ...)  print("error: " msg "\n", ##__VA_ARGS__)
                              ^~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:216:61:
   err("drmModeAtomicAddProperty (%s:%lu) failed: %d", name, value, ret);
                                                             ~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:33:41: note: in definition of macro ‘print’
 #define print(msg, ...) fprintf(stderr, msg, ##__VA_ARGS__);
                                         ^~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:216:3: note: in expansion of macro ‘err’
   err("drmModeAtomicAddProperty (%s:%lu) failed: %d", name, value, ret);
   ^~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:216:39: note: format string is defined here
   err("drmModeAtomicAddProperty (%s:%lu) failed: %d", name, value, ret);
                                     ~~^
                                     %llu
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c: In function ‘drm_add_conn_property’:
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:34:30: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ {aka ‘long long unsigned int’} [-Wformat=]
 #define err(msg, ...)  print("error: " msg "\n", ##__VA_ARGS__)
                              ^~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:235:61:
   err("drmModeAtomicAddProperty (%s:%lu) failed: %d", name, value, ret);
                                                             ~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:33:41: note: in definition of macro ‘print’
 #define print(msg, ...) fprintf(stderr, msg, ##__VA_ARGS__);
                                         ^~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:235:3: note: in expansion of macro ‘err’
   err("drmModeAtomicAddProperty (%s:%lu) failed: %d", name, value, ret);
   ^~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:235:39: note: format string is defined here
   err("drmModeAtomicAddProperty (%s:%lu) failed: %d", name, value, ret);
                                     ~~^
                                     %llu
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c: In function ‘drm_find_connector’:
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:343:9: warning: unused variable ‘j’ [-Wunused-variable]
  int i, j;
         ^
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c: In function ‘drm_allocate_dumb’:
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:653:8: warning: pointer targets in passing argument 5 of ‘drmModeAddFB2’ differ in signedness [-Wpointer-sign]
        handles, pitches, offsets, &buf->fb_handle, 0);
        ^~~~~~~
In file included from /root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:26:
/usr/include/xf86drmMode.h:371:12: note: expected ‘const uint32_t *’ {aka ‘const unsigned int *’} but argument is of type ‘int *’
 extern int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
            ^~~~~~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:653:17: warning: pointer targets in passing argument 6 of ‘drmModeAddFB2’ differ in signedness [-Wpointer-sign]
        handles, pitches, offsets, &buf->fb_handle, 0);
                 ^~~~~~~
In file included from /root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:26:
/usr/include/xf86drmMode.h:371:12: note: expected ‘const uint32_t *’ {aka ‘const unsigned int *’} but argument is of type ‘int *’
 extern int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
            ^~~~~~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:653:26: warning: pointer targets in passing argument 7 of ‘drmModeAddFB2’ differ in signedness [-Wpointer-sign]
        handles, pitches, offsets, &buf->fb_handle, 0);
                          ^~~~~~~
In file included from /root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:26:
/usr/include/xf86drmMode.h:371:12: note: expected ‘const uint32_t *’ {aka ‘const unsigned int *’} but argument is of type ‘int *’
 extern int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
            ^~~~~~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c: In function ‘drm_flush’:
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:717:48: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
  if (w != drm_dev.width || h != drm_dev.height && drm_dev.cur_bufs[0])
                            ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:721:34: warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
                 memcpy(fbuf->map + (area->x1 * (LV_COLOR_SIZE/8)) + (fbuf->pitch * i),
                                  ^
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:721:67: warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
                 memcpy(fbuf->map + (area->x1 * (LV_COLOR_SIZE/8)) + (fbuf->pitch * i),
                                                                   ^
/root/lv_port_linux_frame_buffer/lv_drivers/display/drm.c:722:40: warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
                        (void *)color_p + (w * (LV_COLOR_SIZE/8) * y),
                                        ^

What do you expect?

Better and tested code.
Is it possible to hook DRM double buffer to LVGL double buffer lv_disp_buf_init() ?
I tested small patch to disable double buffer in DRM code then performance equals to FBDEV (185 FPS/85 %):

diff --git a/display/drm.c b/display/drm.c
index bda938e..0c7311d 100644
--- a/display/drm.c
+++ b/display/drm.c
@@ -713,31 +713,22 @@ void drm_flush(lv_disp_drv_t *disp_drv, const lv_area_t *area, lv_color_t *color
 
 	dbg("x %d:%d y %d:%d w %d h %d", area->x1, area->x2, area->y1, area->y2, w, h);
 
-	/* Partial update */
-	if (w != drm_dev.width || h != drm_dev.height && drm_dev.cur_bufs[0])
-		memcpy(fbuf->map, drm_dev.cur_bufs[0]->map, fbuf->size);
-
 	for (y = 0, i = area->y1 ; i <= area->y2 ; ++i, ++y) {
                 memcpy(fbuf->map + (area->x1 * (LV_COLOR_SIZE/8)) + (fbuf->pitch * i),
                        (void *)color_p + (w * (LV_COLOR_SIZE/8) * y),
 		       w * (LV_COLOR_SIZE/8));
 	}
 
-	if (drm_dev.req)
+	if (!drm_dev.cur_bufs[0]) {
+		/* show fbuf plane */
+		if (drm_dmabuf_set_plane(fbuf)) {
+			err("Flush fail");
+			return;
+		}
+		else
+			dbg("Flush done");
 		drm_wait_vsync(disp_drv);
-
-	/* show fbuf plane */
-	if (drm_dmabuf_set_plane(fbuf)) {
-		err("Flush fail");
-		return;
 	}
-	else
-		dbg("Flush done");
-
-	if (!drm_dev.cur_bufs[0])
-		drm_dev.cur_bufs[1] = &drm_dev.drm_bufs[1];
-	else
-		drm_dev.cur_bufs[1] = drm_dev.cur_bufs[0];
 
 	drm_dev.cur_bufs[0] = fbuf;

embeddedt · October 15, 2020, 10:18pm

This came as part of an external contribution: https://github.com/lvgl/lv_drivers/pull/107

I’ve asked the author to take a look.

superna9999 · October 16, 2020, 5:08am

Hi,

Please submit a fix for the warnings.

Concerning the performance issues, the drm backend was designed with double buffering in mind to avoid tearing and respect display vsync

It has been designed to work the single lvgl buffer or double buffering, if you wish to have the best performance with the drm backend please provide 2 buffes to lvgl to enable double buffering.

The code change disables vsync sync and will create tearing on animations.

superna9999 · October 16, 2020, 7:12am

Hi,

I pushed a fix for the warnings at https://github.com/lvgl/lv_drivers/pull/110

Concerning the LVGL double buffering, I use the following:

lv_disp_drv_t disp_drv;
lv_disp_drv_init(&disp_drv);
uint32_t dpi = 0;

drm_get_sizes(&disp_drv.hor_res, &disp_drv.ver_res, &dpi);

// Allocate LVGL buffers
void * lvgl_buf1 = malloc(disp_drv.hor_res * disp_drv.ver_res * (LV_COLOR_SIZE/8));
if (!lvgl_buf1)
	return -1;
void *lvgl_buf2 = malloc(disp_drv.hor_res * disp_drv.ver_res * (LV_COLOR_SIZE/8));
if (!lvgl_buf2)
	return -1;

lv_disp_buf_init(&disp_buf, lvgl_buf1, lvgl_buf2, disp_drv.hor_res * disp_drv.ver_res);

disp_drv.buffer   = &disp_buf;
disp_drv.flush_cb = drm_flush;
disp_drv.wait_cb = drm_wait_vsync;
lv_disp_drv_register(&disp_drv);

to make sure LVGL uses double buffering and not partial updating.

@mcerveny I’m happy the DRM logic worked fine on Allwinner V3s, this is the critical point

@embeddedt would it make sense a create a “drm” lvgl example ? It should run on raspberrypi and other mainline linux based boards (I personnally tested it on Amlogic A311D via HDMIA and A113D via MIPI-DSI)

superna9999 · October 16, 2020, 7:26am

And I had horrible performance / load reports until I used:

/*Handle LitlevGL tasks (tickless mode)*/
while(1)
	usleep(lv_task_handler() * 1000);

instead of a fixed 5ms sleep.

This is due to the blocking VSYNC wait, that should be taken in account in the sleep between 2 lv_task_handler() calls.

Peter_Cao · March 1, 2022, 3:02am

@mcerveny I prefer fps than animations.