Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The commit message claims a change that is not present in the code diff. - [Medium] Fallback to user mode silently flushes a blank screen if the screen is narrower than a single character, defeating failure preservation. --
--- Patch [7]: [PATCH v2 07/13] drm/panic: Retry in dispatch function if panic output fails --- commit 43648cf544bdeed6f6c85be5e74f3e855c778814 Author: Thomas Zimmermann <[email protected]> drm/panic: Retry in dispatch function if panic output fails This patch introduces explicit fallback to DRM_PANIC_TYPE_USER in drm_panic_dispatch() when QR code or kmsg outputs fail. It also increases the minimum QR code test size to 494x494. > Also pass module parameter drm_panic_type as a parameter to > drm_panic_dispatch(). This simpifies the implementation. It will later > allow for moving the function into a helper library. [Severity: Medium] This isn't a bug, but the commit message mentions passing drm_panic_type as a parameter to drm_panic_dispatch(), which isn't present in this patch's code changes. Was this already implemented in a preceding commit? > diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c > index f597ef04c5bcb..bc7c096c42363 100644 > --- a/drivers/gpu/drm/drm_panic.c > +++ b/drivers/gpu/drm/drm_panic.c [ ... ] > @@ -895,13 +886,22 @@ static int draw_panic_dispatch(struct > drm_scanout_buffer *sb, enum drm_panic_typ > { > int ret; > > +retry: > switch (panic_type) { > case DRM_PANIC_TYPE_KMSG: > ret = draw_panic_screen_kmsg(sb, fg_color, bg_color); > + if (ret) { > + panic_type = DRM_PANIC_TYPE_USER; > + goto retry; > + } [Severity: Medium] Does this fallback logic inadvertently cause the screen to be overwritten with a blank screen if the screen is narrower than a single character? If drm_panic_type is set to KMSG and sb->width < font->width, draw_panic_screen_kmsg() returns -EINVAL. The retry logic here catches this and falls back to DRM_PANIC_TYPE_USER. However, looking at draw_panic_screen_user(), it lacks a similar width check. It clears the screen with the background color: drivers/gpu/drm/drm_panic.c:draw_panic_screen_user() { ... /* Fill with the background color, and draw text on top */ drm_panic_fill(sb, &r_screen, bg_color); ... } It then returns 0 (success) without drawing any text. This success code causes draw_panic_plane() to flush the blank screen, destroying the existing visual evidence instead of preserving it. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
