Skip to content

Commit 185551c

Browse files
gmarullclaude
andcommitted
fw/services/compositor: make freeze wait for in-flight display updates
compositor_freeze() only raised a flag that the KernelMain render paths consult. It was called from the BT task by the screenshot endpoint, which then copied the first framebuffer chunk immediately, so a display update that had already been kicked off could still be in flight while the screenshot read the framebuffer. On sf32lb boards (obelix, getafix) the JDI driver converts the dirty rows of the compositor framebuffer in-place from 2-2-2 to 3-3-2 for the LCDC and only converts them back after the DMA completes, from a KernelMain callback. Screenshots sampling those rows in that window carry 3-3-2 pixels reinterpreted as GColor8: orange (0xF4) reads back as GColorIslamicGreen (0xC8), which is the green status bar in the report. Turn compositor_freeze() into an asynchronous request: the freeze takes effect on KernelMain and a callback fires there once no display update is in progress, either straight away or from the display update complete handler. The screenshot starts streaming from that callback on the system task, and the ALS console command drops its hand-rolled spin on compositor_display_update_in_progress(). Add unit tests for both the idle and the update-in-flight cases. Fixes FIRM-4357 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
1 parent bacc491 commit 185551c

5 files changed

Lines changed: 111 additions & 8 deletions

File tree

include/pbl/services/compositor/compositor.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,13 @@ void compositor_set_modal_transition_offset(GPoint modal_offset);
116116
//! Stops an existing transition in its tracks.
117117
void compositor_transition_cancel(void);
118118

119-
//! Don't allow new frames to be pushed to the compostor from either the app or the modal.
120-
void compositor_freeze(void);
119+
typedef void (*CompositorFrozenCallback)(void *data);
120+
121+
//! Don't allow new frames to be pushed to the compositor from either the app or the modal.
122+
//! Callable from any task. \a callback runs on KernelMain once the freeze is in effect and no
123+
//! display update is in flight, at which point the framebuffer is stable until
124+
//! \ref compositor_unfreeze. Only one freeze may be outstanding at a time.
125+
void compositor_freeze(CompositorFrozenCallback callback, void *data);
121126

122127
//! Resuming allowing new frames to be pushed to the compositor, undoes the effects of
123128
//! compositor_freeze.

src/fw/services/compositor/compositor.c

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ static DeferredRender s_deferred_render;
7777
static CompositorTransitionState s_animation_state;
7878

7979
static bool s_framebuffer_frozen;
80+
static CompositorFrozenCallback s_frozen_callback;
81+
static void *s_frozen_callback_data;
8082

8183
//! Animation .update function for the AnimationImplementation we use to drive our transitions.
8284
//! Wraps the .update function of the current CompositorTransition.
@@ -101,6 +103,8 @@ void compositor_init(void) {
101103
s_animation_state = (CompositorTransitionState) { 0 };
102104

103105
s_framebuffer_frozen = false;
106+
s_frozen_callback = NULL;
107+
s_frozen_callback_data = NULL;
104108
}
105109

106110
// Helper functions to make implementing transitions easier
@@ -173,6 +177,15 @@ void compositor_render_modal(void) {
173177
// Compositor implementation
174178
///////////////////////////////////////////////////////////
175179

180+
static void prv_notify_frozen(void) {
181+
if (!s_frozen_callback || compositor_display_update_in_progress()) {
182+
return;
183+
}
184+
CompositorFrozenCallback callback = s_frozen_callback;
185+
s_frozen_callback = NULL;
186+
callback(s_frozen_callback_data);
187+
}
188+
176189
T_STATIC void prv_handle_display_update_complete(void) {
177190
if (s_deferred_render.transition_complete.pending) {
178191
s_deferred_render.transition_complete.pending = false;
@@ -193,6 +206,7 @@ T_STATIC void prv_handle_display_update_complete(void) {
193206
s_deferred_render.app.pending = false;
194207
compositor_app_render_ready();
195208
}
209+
prv_notify_frozen();
196210
}
197211

198212
static void prv_compositor_flush(void) {
@@ -481,8 +495,16 @@ void compositor_transition_cancel(void) {
481495
}
482496
}
483497

484-
void compositor_freeze(void) {
498+
static void prv_compositor_freeze_cb(void *ignored) {
485499
s_framebuffer_frozen = true;
500+
prv_notify_frozen();
501+
}
502+
503+
void compositor_freeze(CompositorFrozenCallback callback, void *data) {
504+
s_frozen_callback = callback;
505+
s_frozen_callback_data = data;
506+
507+
launcher_task_add_callback(prv_compositor_freeze_cb, NULL);
486508
}
487509

488510
static void prv_compositor_unfreeze_cb(void *ignored) {

src/fw/services/compositor/screenshot_pp.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,10 @@ void screenshot_send_next_chunk(void* raw_state) {
196196
system_task_add_callback(screenshot_send_next_chunk, state);
197197
}
198198

199+
static void prv_frozen_cb(void *data) {
200+
system_task_add_callback(screenshot_send_next_chunk, data);
201+
}
202+
199203
void screenshot_protocol_msg_callback(CommSession *session, const uint8_t* msg_data, unsigned int msg_len) {
200204
uint8_t sub_command = msg_data[0];
201205
if (sub_command != 0x00) {
@@ -215,8 +219,6 @@ void screenshot_protocol_msg_callback(CommSession *session, const uint8_t* msg_d
215219

216220
prv_request_fast_connection(session);
217221

218-
compositor_freeze();
219-
220222
s_screenshot_state = (ScreenshotState) {
221223
.session = session,
222224
.framebuffer = (FrameBufferState) {
@@ -229,5 +231,5 @@ void screenshot_protocol_msg_callback(CommSession *session, const uint8_t* msg_d
229231
.sent_header = false,
230232
};
231233

232-
screenshot_send_next_chunk(&s_screenshot_state);
234+
compositor_freeze(prv_frozen_cb, &s_screenshot_state);
233235
}

src/fw/services/light/console.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ void command_als_lux(void) {
8080

8181
// Set by prv_als_flush_cb on KernelMain once the panel update is kicked.
8282
static volatile bool s_als_flush_done;
83+
static volatile bool s_als_frozen;
84+
85+
static void prv_als_frozen_cb(void *unused) {
86+
s_als_frozen = true;
87+
}
8388

8489
// Runs on KernelMain (the compositor's task): push the staged system framebuffer
8590
// to the panel, the same way the PULSE framebuffer domain does.
@@ -134,8 +139,9 @@ void command_als_curve(void) {
134139
};
135140

136141
animation_private_pause();
137-
compositor_freeze();
138-
while (compositor_display_update_in_progress()) {
142+
s_als_frozen = false;
143+
compositor_freeze(prv_als_frozen_cb, NULL);
144+
while (!s_als_frozen) {
139145
psleep(2);
140146
}
141147

tests/fw/services/compositor/test_compositor.c

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,24 @@ static const CompositorTransition s_transition_b = {
196196
.update = prv_compositor_update_func_b
197197
};
198198

199+
static void (*s_launcher_callback)(void *data);
200+
static void *s_launcher_callback_data;
199201
void launcher_task_add_callback(void (*callback)(void *data), void *data) {
202+
s_launcher_callback = callback;
203+
s_launcher_callback_data = data;
204+
}
205+
206+
static void prv_run_launcher_callback(void) {
207+
void (*callback)(void *data) = s_launcher_callback;
208+
s_launcher_callback = NULL;
209+
callback(s_launcher_callback_data);
210+
}
211+
212+
static int s_count_frozen = 0;
213+
static void *s_frozen_data;
214+
static void prv_frozen_cb(void *data) {
215+
++s_count_frozen;
216+
s_frozen_data = data;
200217
}
201218

202219

@@ -226,6 +243,11 @@ void test_compositor__initialize(void) {
226243

227244
s_render_pending = false;
228245

246+
s_launcher_callback = NULL;
247+
s_launcher_callback_data = NULL;
248+
s_count_frozen = 0;
249+
s_frozen_data = NULL;
250+
229251
compositor_init();
230252
}
231253

@@ -443,3 +465,49 @@ void test_compositor__cancel_modal_to_app_with_another_modal(void) {
443465
// App should be free to render again
444466
cl_assert_equal_i(s_render_pending, false);
445467
}
468+
469+
void test_compositor__freeze_idle_display(void) {
470+
int data;
471+
compositor_freeze(prv_frozen_cb, &data);
472+
// Nothing happens until the freeze runs on KernelMain
473+
cl_assert_equal_i(s_count_frozen, 0);
474+
compositor_app_render_ready();
475+
cl_assert_equal_i(s_count_display_update, 1);
476+
477+
// No display update in flight: the callback fires as soon as the freeze takes effect
478+
prv_run_launcher_callback();
479+
cl_assert_equal_i(s_count_frozen, 1);
480+
cl_assert_equal_p(s_frozen_data, &data);
481+
482+
// Frames are held back while frozen
483+
compositor_app_render_ready();
484+
cl_assert_equal_i(s_count_display_update, 1);
485+
486+
// ... and flushed once unfrozen
487+
compositor_unfreeze();
488+
prv_run_launcher_callback();
489+
cl_assert_equal_i(s_count_display_update, 2);
490+
cl_assert_equal_i(s_count_frozen, 1);
491+
}
492+
493+
void test_compositor__freeze_waits_for_display_update(void) {
494+
s_display_update_in_progress = true;
495+
compositor_freeze(prv_frozen_cb, NULL);
496+
prv_run_launcher_callback();
497+
// The framebuffer is still being pushed to the display, so it isn't stable yet
498+
cl_assert_equal_i(s_count_frozen, 0);
499+
500+
compositor_app_render_ready();
501+
cl_assert_equal_i(s_count_display_update, 0);
502+
503+
// The update completes: the deferred frame stays held back and the callback fires
504+
s_display_update_in_progress = false;
505+
prv_handle_display_update_complete();
506+
cl_assert_equal_i(s_count_display_update, 0);
507+
cl_assert_equal_i(s_count_frozen, 1);
508+
509+
compositor_unfreeze();
510+
prv_run_launcher_callback();
511+
cl_assert_equal_i(s_count_display_update, 1);
512+
cl_assert_equal_i(s_count_frozen, 1);
513+
}

0 commit comments

Comments
 (0)