I tested this in the PineTime. Seems to work, with just minor modifications!

1. the pinetime has only 240x240 pixels. needed to adjust that. I think this needs to become a parameter if the ili9341, and I think the disp_dev needs a way to query that.

2. the pinetime uses gpios to control the backlight. Maybe support for that also needs to be added to disp_dev?

I have a picture, but somehow my Gnome/Wayland doesn't let me paste it here. ;(

I think this needs to become a parameter if the ili9341, and I think the disp_dev needs a way to query that

It's not that simple without patching lvgl. The LV_HOR_RES_MAX and LV_VER_RES_MAX are used internally when initializing the display, see here.
in lv_conf.h, these macro can be overridden: this provides compile time configuration.

Maybe support for that also needs to be added to disp_dev?

Probably doable, with get/set functions.

It's not that simple without patching lvgl. The LV_HOR_RES_MAX and LV_VER_RES_MAX are used internally when initializing the display, see here.
in lv_conf.h, these macro can be overridden: this provides compile time configuration.

Hm, looking at the "porting" example here, they first initialize the driver struct (using lv_disp_drv_init(), which just clears and sets values), then afterwards override the resolution and buffers. Maybe we can do it similarly?

BTW, at this point, does it make sense to introduce disp_dev_t? We could just use lv_disp_t. It even provides a method to add user defined fields (https://github.com/littlevgl/lvgl/blob/51928059d2dda465ab76ee66cf37b9d2edb9f64e/src/lv_hal/lv_hal_disp.h#L115-L117).

I see the merits of having a display_dev abstraction for display devices. However for now I would like to propose to either:

• split the display_dev header to a separate PR and continue this with only support for the ili9341 device driver, it is the only display supported at the moment anyway.

or:

• Keep the display_dev as it is now but mark it as experimental, I think it is a good start and we can iterate over it in a follow up PR.

I see the merits of having a display_dev abstraction for display devices

The abstraction design has no external dependency so each display driver can provide the required driver functions (only one for the moment). If we reuse the abstraction API that comes from lvgl, then this introduce a hard dependency to it. I would prefer to avoid this.
@fjmolinas has a driver for other LCD modules that could benefit from this very easily.

So for now I would use option B, as proposed by @bergzand: keep the disp_dev header and improve later.

Regarding the while loop in the task thread, I agree it's not nice, especially because of power consumption.
We could use a blocking msg_receive function there when the system is inactive and send a message to this thread on wakeup. Something like:

while (1) {
    /*Normal operation (no sleep) in < 1 sec inactivity*/
    if (lv_disp_get_inactive_time(NULL) < 1000) {
        lv_task_handler();
    }
    /* Block after 1 sec inactivity */
    else {
        msg_t msg;
        msg_receive(&msg);
    }
    xtimer_usleep(5 * US_PER_MS);
}

Then the screen could be explicitly restarted from user code with a simple msg_send.

So for now I would use option B, as proposed by @bergzand: keep the disp_dev header and improve later.

ok

Then the screen could be explicitly restarted from user code with a simple msg_send.

better use thread flags. Cannot get lost (e.g., if set from ISR), doesn't block caller.

better use thread flags.

@kaspar030, I just pushed a couple of commits that implement a mechanism to allow stopping the lvgl task handler thread after some inactivity period (defined by the LVGL_ACTIVITY_PERIOD macro). The glue code now provides an extra function lvgl_wakeup that needs to be called when lvgl should be restarted (or the inactivity limit be kicked).

In the sysmon example, this function is called each time the sysmon_task is called: this ensures the chart is always refreshed. But if you tweak the REFR_TIME macro in main.c to be greater than LVGL_ACTIVITY_PERIOD, then lvgl is stopped after LVGL_ACTIVITY_PERIOD is elapsed and an explicit call to lvgl_wakeup is needed to restart it (this can be done in a gpio callback for example).

What do you think of the approach ?

I added a bit of documentation.

@kaspar030, if you think the thread_flags is good enough, this PR is close to be mergeable I think.

I think I addressed all your comments @bergzand :)

Comments addressed @bergzand :)

ping @bergzand :)

Please squash after addressing my last comment

Awesome ! Thanks a lot for the reviews and tests :)