Skip to content

Commit 10cbb96

Browse files
committed
cog: fix drm platform for kiosk use; pin meson options
Six upstream-ready patches against 0.18.5 (developed and verified on a Dell Wyse 5010: radeon PALM/DCE4.1, both llvmpipe/SHM and r600/dmabuf paths): - 001: cold-start initialization. CRTC selection required an existing encoder binding (none exists without a prior fbcon/firmware modeset), kms_screen_create crashed on modeless connectors, init_cursor used the possibly-dead screens[0]. The incomplete display setup from the old CRTC selection is also what left DCE4.1's cursor fetch uninitialized - initially misdiagnosed as a hardware/kernel bug. - 002: SHM exported-buffer handling. Root cause of a crash family: the renderer stored itself in the buffer resource's user_data, which for SHM buffers IS libwayland's wl_shm_buffer - from the second attach on, wl_shm_buffer_get() returned the renderer as buffer metadata. Plus early buffer release (avoids deferred pool-resize stale mappings) and copy hardening. - 003: legacy hardware cursor (drmModeSetCursor) for non-atomic drivers without a universal cursor plane, with correct 64x64 tightly-packed premultiplied cursor image. - 004: software cursor option (COG_PLATFORM_DRM_CURSOR=sw), composited into SHM frames with in-place motion updates; dmabuf-safe. - 005: cursor.x/y are unsigned, so the existing 'if (cursor.x < 0)' lower clamp was dead code; moving past the top/left edge wrapped the position around and the upper clamp teleported the cursor to the opposite edge. Clamp in floating point before the unsigned store. Present upstream (master) as well. - 006: scale the 16x16 cursor artwork by the view's device scale factor (nearest neighbour; up to 4x within the 64x64 hardware cursor buffer, same factor for the modeset renderer's software cursor), so the pointer keeps its apparent size next to a --device-scale'd UI. Makefile: pin the complete meson option set of 0.18.5 (platforms drm/headless/wayland, wpe_api 2.0, libmanette, plugin path). PKG_RELEASE 9. Patchset already proposed upstream: Igalia/cog#794 Assisted-By: Claude Fable + Opus Signed-off-by: Mirko Vogt <mirko-openwrt@nanl.de>
1 parent a951381 commit 10cbb96

7 files changed

Lines changed: 1228 additions & 1 deletion

libs/cog/Makefile

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ include $(TOPDIR)/rules.mk
22

33
PKG_NAME:=cog
44
PKG_VERSION:=0.18.5
5-
PKG_RELEASE:=1
5+
PKG_RELEASE:=9
66

77
PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.xz
88
PKG_SOURCE_URL:=https://wpewebkit.org/releases
@@ -44,7 +44,18 @@ libcogcore is a library with ready-to-use components typically needed
4444
for implementing applications which use the WPE WebKit API.
4545
endef
4646

47+
# every meson option of this release pinned explicitly, so a version
48+
# bump cannot silently change the platform set or defaults
4749
MESON_ARGS += \
50+
-Dplugin_path=/usr/lib/cog/modules \
51+
-Dplatforms=drm,headless,wayland \
52+
-Dprograms=true \
53+
-Dwpe_api=2.0 \
54+
-Dlibmanette=enabled \
55+
-Dwayland_weston_direct_display=false \
56+
-Dwayland_weston_content_protection=false \
57+
-Dcog_appid=com.igalia.Cog \
58+
-Dcog_home_uri= \
4859
-Dcog_dbus_control=system \
4960
-Dcog_dbus_system_owner=cog \
5061
-Ddocumentation=false \
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
From: Mirko Vogt <foss@mirko.in>
2+
Subject: [PATCH 1/6] drm: fix cold-start initialization
3+
4+
On a display with no prior modeset (no fbcon/fbdev emulation, no
5+
firmware POST pass - e.g. a bootloader handing straight over to bare
6+
KMS), the drm platform failed to initialize:
7+
8+
- The CRTC was picked only by matching an encoder's CURRENT binding
9+
(encoder->crtc_id), which is 0 on cold start, aborting with "no crtc
10+
for encoder found". Prefer the target connector's own encoders and
11+
fall back to the first CRTC an encoder can drive (possible_crtcs).
12+
This also fixes picking another output's encoder whose CRTC cannot
13+
drive the chosen connector. Note that the incomplete display setup
14+
resulting from the old selection is also what left e.g. radeon
15+
DCE4.1's hardware cursor fetch uninitialized (cursor rendered as
16+
garbage) - a full cold-start modeset cures that as well.
17+
18+
- kms_screen_create() dereferenced con->modes[0] unconditionally; it
19+
is NULL for disconnected/modeless connectors (any dual-output card
20+
with one output unused), crashing at startup when the cursor is
21+
enabled. A zeroed mode now marks the screen unusable.
22+
23+
- init_cursor() used screens[0], which may be that dead connector;
24+
pick the first usable screen. The initial cursor plane set may fail
25+
while the CRTC is still off (first modeset happens at first frame
26+
commit) - treat that as non-fatal, the pointer-motion handler
27+
retries.
28+
29+
Signed-off-by: Mirko Vogt <foss@mirko.in>
30+
---
31+
diff -ruN '--exclude=*.orig' a/platform/drm/cog-platform-drm.c b/platform/drm/cog-platform-drm.c
32+
--- a/platform/drm/cog-platform-drm.c
33+
+++ b/platform/drm/cog-platform-drm.c
34+
@@ -362,6 +362,15 @@
35+
}
36+
}
37+
38+
+ /* Cold start: with no prior modeset (no fbcon/firmware POST) the
39+
+ * encoder is bound to no CRTC yet (crtc_id == 0), so the match on
40+
+ * the CURRENT binding above finds nothing. Fall back to the first
41+
+ * CRTC this encoder can physically drive. */
42+
+ for (int i = 0; i < resources->count_crtcs; i++) {
43+
+ if (encoder->possible_crtcs & (1 << i))
44+
+ return resources->crtcs[i];
45+
+ }
46+
+
47+
/* no match found */
48+
return -1;
49+
}
50+
@@ -508,21 +517,35 @@
51+
(long)((drm_data.mode - drm_data.connector.obj->modes) / sizeof(drmModeModeInfo *)), drm_data.mode->name,
52+
drm_data.mode->vrefresh);
53+
54+
- /* Try the currently connected encoder+crtc */
55+
- for (int i = 0; i < drm_data.base_resources->count_encoders; ++i) {
56+
- drm_data.encoder = drmModeGetEncoder(drm_data.fd, drm_data.base_resources->encoders[i]);
57+
- if (!drm_data.encoder) {
58+
- /* cannot retrieve encoder, ignoring... */
59+
+ /* Prefer THIS connector's own encoders: scanning all encoders can
60+
+ * pick another output's encoder whose CRTC does not drive our
61+
+ * connector. find_crtc_for_encoder() handles the cold-start case
62+
+ * (no active CRTC binding yet) via its possible_crtcs fallback. */
63+
+ for (int i = 0; i < drm_data.connector.obj->count_encoders && !drm_data.encoder; ++i) {
64+
+ drmModeEncoder *enc = drmModeGetEncoder(drm_data.fd, drm_data.connector.obj->encoders[i]);
65+
+ if (!enc)
66+
continue;
67+
+ const int32_t crtc_id = find_crtc_for_encoder(drm_data.base_resources, enc);
68+
+ if (crtc_id != -1) {
69+
+ drm_data.encoder = enc;
70+
+ drm_data.crtc.obj_id = crtc_id;
71+
+ break;
72+
}
73+
+ drmModeFreeEncoder(enc);
74+
+ }
75+
76+
- const int32_t crtc_id = find_crtc_for_encoder(drm_data.base_resources, drm_data.encoder);
77+
+ /* Last resort: any encoder that yields a CRTC. */
78+
+ for (int i = 0; i < drm_data.base_resources->count_encoders && !drm_data.encoder; ++i) {
79+
+ drmModeEncoder *enc = drmModeGetEncoder(drm_data.fd, drm_data.base_resources->encoders[i]);
80+
+ if (!enc)
81+
+ continue;
82+
+ const int32_t crtc_id = find_crtc_for_encoder(drm_data.base_resources, enc);
83+
if (crtc_id != -1) {
84+
+ drm_data.encoder = enc;
85+
drm_data.crtc.obj_id = crtc_id;
86+
break;
87+
}
88+
-
89+
- g_clear_pointer (&drm_data.encoder, drmModeFreeEncoder);
90+
+ drmModeFreeEncoder(enc);
91+
}
92+
93+
if (!drm_data.encoder) {
94+
@@ -623,8 +646,10 @@
95+
}
96+
97+
cursor.device = kms_device_open(drm_data.fd);
98+
- if (!cursor.device)
99+
+ if (!cursor.device) {
100+
+ g_warning("cursor: kms_device_open failed");
101+
return FALSE;
102+
+ }
103+
104+
cursor.plane = kms_device_find_plane_by_type(cursor.device, DRM_PLANE_TYPE_CURSOR, 0);
105+
if (!cursor.plane) {
106+
@@ -640,21 +665,41 @@
107+
108+
cursor.cursor = create_cursor_framebuffer(cursor.device, format);
109+
if (!cursor.cursor) {
110+
+ g_warning("cursor: framebuffer creation failed");
111+
g_clear_pointer(&cursor.device, kms_device_free);
112+
return FALSE;
113+
}
114+
115+
- cursor.x = (cursor.device->screens[0]->width - cursor.cursor->width) / 2;
116+
- cursor.y = (cursor.device->screens[0]->height - cursor.cursor->height) / 2;
117+
- cursor.screen_width = cursor.device->screens[0]->width;
118+
- cursor.screen_height = cursor.device->screens[0]->height;
119+
-
120+
- if (kms_plane_set(cursor.plane, cursor.cursor, cursor.x, cursor.y)) {
121+
+ /* screens[0] may be a disconnected/modeless connector (e.g. the
122+
+ * unused second output of a dual-output card) - pick the first
123+
+ * usable screen instead. */
124+
+ struct kms_screen *cursor_screen = NULL;
125+
+ for (unsigned int i = 0; i < cursor.device->num_screens; i++) {
126+
+ if (cursor.device->screens[i]->connected && cursor.device->screens[i]->width > 0) {
127+
+ cursor_screen = cursor.device->screens[i];
128+
+ break;
129+
+ }
130+
+ }
131+
+ if (!cursor_screen) {
132+
+ g_warning("cursor: no usable screen (num_screens %u)", cursor.device->num_screens);
133+
g_clear_pointer(&cursor.device, kms_device_free);
134+
g_clear_pointer(&cursor.cursor, kms_framebuffer_free);
135+
return FALSE;
136+
}
137+
138+
+ cursor.x = (cursor_screen->width - cursor.cursor->width) / 2;
139+
+ cursor.y = (cursor_screen->height - cursor.cursor->height) / 2;
140+
+ cursor.screen_width = cursor_screen->width;
141+
+ cursor.screen_height = cursor_screen->height;
142+
+
143+
+ /* The CRTC is usually not lit yet at platform setup time - the
144+
+ * renderer performs the first modeset on the first frame commit -
145+
+ * so this initial cursor upload may fail (display still off). Not
146+
+ * fatal: the pointer-motion handler retries on every move and
147+
+ * succeeds once a mode is active. */
148+
+ if (kms_plane_set(cursor.plane, cursor.cursor, cursor.x, cursor.y))
149+
+ g_message("cursor plane not set yet (display off?) - will appear on first pointer motion");
150+
+
151+
cursor.enabled = TRUE;
152+
153+
return TRUE;
154+
diff -ruN '--exclude=*.orig' a/platform/drm/kms.c b/platform/drm/kms.c
155+
--- a/platform/drm/kms.c
156+
+++ b/platform/drm/kms.c
157+
@@ -361,9 +361,15 @@
158+
else
159+
screen->connected = false;
160+
161+
- memcpy(&screen->mode, &con->modes[0], sizeof(drmModeModeInfo));
162+
- screen->width = screen->mode.hdisplay;
163+
- screen->height = screen->mode.vdisplay;
164+
+ /* Disconnected connectors (or connected ones without EDID) have no
165+
+ * mode list - con->modes is NULL and count_modes 0. screen is
166+
+ * calloc'd, so leaving mode/width/height zeroed marks the screen
167+
+ * unusable instead of reading address 0. */
168+
+ if (con->count_modes > 0 && con->modes) {
169+
+ memcpy(&screen->mode, &con->modes[0], sizeof(drmModeModeInfo));
170+
+ screen->width = screen->mode.hdisplay;
171+
+ screen->height = screen->mode.vdisplay;
172+
+ }
173+
174+
drmModeFreeConnector(con);
175+
}
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
From: Mirko Vogt <foss@mirko.in>
2+
Subject: [PATCH 2/6] drm: modeset: fix SHM exported buffer handling
3+
4+
Three related fixes for the modeset renderer's SHM path, found running
5+
WPE on software rendering (llvmpipe/kms_swrast):
6+
7+
- Never store the renderer in the buffer resource's user_data: for SHM
8+
buffers user_data is owned by libwayland's shm implementation - it IS
9+
the wl_shm_buffer. Overwriting it made wl_shm_buffer_get() return the
10+
renderer object from the second attach onwards: garbage width/stride,
11+
spurious "deferred resize pending" warnings, and a crashing copy
12+
loop. Keep the owning renderer in the buffer_object instead.
13+
14+
- Release the exported SHM buffer right after copying it into the dumb
15+
buffer instead of parking it until the frame retires: holding it
16+
keeps an external reference on the wl_shm pool, deferring client-side
17+
pool resizes into stale mappings.
18+
19+
- Harden the copy: bail out on a NULL data pointer, clamp to the bo's
20+
dimensions, and skip frames whose exported buffer carries impossible
21+
geometry (dispatching frame-complete so the client's pacing
22+
survives). Persistently bogus exports terminate the process for a
23+
clean supervisor restart rather than continuing on corrupted state.
24+
25+
Signed-off-by: Mirko Vogt <foss@mirko.in>
26+
---
27+
diff -ruN a/platform/drm/cog-drm-modeset-renderer.c b/platform/drm/cog-drm-modeset-renderer.c
28+
--- a/platform/drm/cog-drm-modeset-renderer.c
29+
+++ b/platform/drm/cog-drm-modeset-renderer.c
30+
@@ -74,6 +74,7 @@
31+
uint32_t fb_id;
32+
struct gbm_bo *bo;
33+
struct wl_resource *buffer_resource;
34+
+ void *renderer; /* owning CogDrmModesetRenderer */
35+
36+
struct {
37+
struct wl_resource *resource;
38+
@@ -137,14 +138,13 @@
39+
destroy_buffer_notify(struct wl_listener *listener, void *data)
40+
{
41+
struct buffer_object *buffer = wl_container_of(listener, buffer, destroy_listener);
42+
- CogDrmModesetRenderer *renderer = wl_resource_get_user_data(buffer->buffer_resource);
43+
+ CogDrmModesetRenderer *renderer = buffer->renderer;
44+
45+
if (renderer->committed_buffer == buffer)
46+
renderer->committed_buffer = NULL;
47+
48+
wl_list_remove(&buffer->link);
49+
50+
- wl_resource_set_user_data(buffer->buffer_resource, NULL);
51+
destroy_buffer(renderer, buffer);
52+
}
53+
54+
@@ -216,7 +216,7 @@
55+
wl_list_insert(&self->buffer_list, &buffer->link);
56+
buffer->destroy_listener.notify = destroy_buffer_notify;
57+
wl_resource_add_destroy_listener(buffer_resource, &buffer->destroy_listener);
58+
- wl_resource_set_user_data(buffer_resource, self);
59+
+ buffer->renderer = self;
60+
61+
buffer->fb_id = fb_id;
62+
buffer->bo = bo;
63+
@@ -271,7 +271,7 @@
64+
wl_list_insert(&self->buffer_list, &buffer->link);
65+
buffer->destroy_listener.notify = destroy_buffer_notify;
66+
wl_resource_add_destroy_listener(buffer_resource, &buffer->destroy_listener);
67+
- wl_resource_set_user_data(buffer_resource, self);
68+
+ buffer->renderer = self;
69+
70+
buffer->fb_id = fb_id;
71+
buffer->bo = bo;
72+
@@ -287,6 +287,19 @@
73+
int32_t height = wl_shm_buffer_get_height(shm_buffer);
74+
int32_t stride = wl_shm_buffer_get_stride(shm_buffer);
75+
76+
+ /* Never write past the bo - clamp and report a size mismatch
77+
+ * (stale buffer_object?) instead of crashing. */
78+
+ uint32_t pre_bo_width = gbm_bo_get_width(bo);
79+
+ uint32_t pre_bo_height = gbm_bo_get_height(bo);
80+
+ if ((uint32_t) width > pre_bo_width || (uint32_t) height > pre_bo_height) {
81+
+ g_warning("SHM->bo size mismatch: shm %dx%d (stride %d) vs bo %ux%u - clamping",
82+
+ width, height, stride, pre_bo_width, pre_bo_height);
83+
+ if ((uint32_t) width > pre_bo_width)
84+
+ width = pre_bo_width;
85+
+ if ((uint32_t) height > pre_bo_height)
86+
+ height = pre_bo_height;
87+
+ }
88+
+
89+
uint32_t bo_stride = 0;
90+
void *map_data = NULL;
91+
gbm_bo_map(bo, 0, 0, width, height, GBM_BO_TRANSFER_WRITE, &bo_stride, &map_data);
92+
@@ -296,6 +309,11 @@
93+
wl_shm_buffer_begin_access(shm_buffer);
94+
95+
uint8_t *src = wl_shm_buffer_get_data(shm_buffer);
96+
+ if (!src) {
97+
+ wl_shm_buffer_end_access(shm_buffer);
98+
+ gbm_bo_unmap(bo, map_data);
99+
+ return;
100+
+ }
101+
uint8_t *dst = map_data;
102+
103+
uint32_t bo_width = gbm_bo_get_width(bo);
104+
@@ -522,6 +540,8 @@
105+
}
106+
}
107+
108+
+static unsigned bogus_streak = 0;
109+
+
110+
static void
111+
on_export_shm_buffer(void *data, struct wpe_fdo_shm_exported_buffer *exported_buffer)
112+
{
113+
@@ -530,20 +550,46 @@
114+
struct wl_resource *exported_resource = wpe_fdo_shm_exported_buffer_get_resource(exported_buffer);
115+
struct wl_shm_buffer *exported_shm_buffer = wpe_fdo_shm_exported_buffer_get_shm_buffer(exported_buffer);
116+
117+
- struct buffer_object *buffer = drm_buffer_for_resource(self, exported_resource);
118+
- if (buffer) {
119+
- drm_copy_shm_buffer_into_bo(exported_shm_buffer, buffer->bo);
120+
-
121+
- buffer->export.shm_buffer = exported_buffer;
122+
- drm_commit_buffer(self, buffer);
123+
- return;
124+
+ /* Never touch buffers with impossible geometry (a stale or dangling
125+
+ * wl_shm_buffer, e.g. exported by a buggy backend): skip the frame
126+
+ * instead of copying from/into wild memory. Do NOT release the
127+
+ * wrapper - that would poke the possibly-dangling resource; leaking
128+
+ * it is the lesser evil in an already-broken session. */
129+
+ {
130+
+ int32_t w = exported_shm_buffer ? wl_shm_buffer_get_width(exported_shm_buffer) : -1;
131+
+ int32_t h = exported_shm_buffer ? wl_shm_buffer_get_height(exported_shm_buffer) : -1;
132+
+ int32_t st = exported_shm_buffer ? wl_shm_buffer_get_stride(exported_shm_buffer) : -1;
133+
+ g_debug("shm export: resource %p shm %p %dx%d stride %d",
134+
+ (void *) exported_resource, (void *) exported_shm_buffer, w, h, st);
135+
+ if (!exported_resource || w <= 0 || h <= 0 || w > 16384 || h > 16384 || st < w * 4) {
136+
+ g_warning("shm export with bogus geometry (resource %p, shm %p, %dx%d stride %d) - frame skipped (streak %u)",
137+
+ (void *) exported_resource, (void *) exported_shm_buffer, w, h, st, ++bogus_streak);
138+
+ if (bogus_streak >= 4) {
139+
+ g_critical("persistent bogus SHM exports - exiting for a clean respawn");
140+
+ exit(70);
141+
+ }
142+
+ /* keep the frame pacing alive - without this WebKit waits
143+
+ * for the frame callback of the skipped frame forever */
144+
+ wpe_view_backend_exportable_fdo_dispatch_frame_complete(self->exportable);
145+
+ return;
146+
+ }
147+
+ bogus_streak = 0;
148+
}
149+
150+
- buffer = drm_create_buffer_for_shm_buffer(self, exported_resource, exported_shm_buffer);
151+
+ struct buffer_object *buffer = drm_buffer_for_resource(self, exported_resource);
152+
+ if (!buffer)
153+
+ buffer = drm_create_buffer_for_shm_buffer(self, exported_resource, exported_shm_buffer);
154+
if (buffer) {
155+
drm_copy_shm_buffer_into_bo(exported_shm_buffer, buffer->bo);
156+
157+
- buffer->export.shm_buffer = exported_buffer;
158+
+ /* The dumb buffer owns the pixels now: release the client's SHM
159+
+ * buffer right away instead of parking it until the frame retires.
160+
+ * Holding it keeps an external reference on the wl_shm pool, which
161+
+ * turns client-side pool resizes into deferred remaps - the next
162+
+ * copy would then use a stale mapping (libwayland warns "Buffer
163+
+ * address requested when its parent pool has an external reference
164+
+ * and a deferred resize pending.") and walk off its end. */
165+
+ wpe_view_backend_exportable_fdo_dispatch_release_shm_exported_buffer(self->exportable, exported_buffer);
166+
drm_commit_buffer(self, buffer);
167+
}
168+
}

0 commit comments

Comments
 (0)