public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/panic: Fix panic logo alignment
@ 2026-03-29 10:24 Dileep Sankhla
  2026-03-31  7:43 ` Claude review: " Claude Code Review Bot
  2026-03-31  7:43 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Dileep Sankhla @ 2026-03-29 10:24 UTC (permalink / raw)
  To: Jocelyn Falempe, Javier Martinez Canillas, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, linux-kernel, Dileep Sankhla

Display the panic logo at the top center of the screen when
drm_panic_type is set to DRM_PANIC_TYPE_USER to keep the layout
properly aligned.

Signed-off-by: Dileep Sankhla <dileepsankhla.ds@gmail.com>
---
 drivers/gpu/drm/drm_panic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index d6d3b8d85dea..c7fecc5e857a 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -491,6 +491,8 @@ static void draw_panic_screen_user(struct drm_scanout_buffer *sb)
 
 	r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
 	drm_panic_logo_rect(&r_logo, font);
+	/* Center the panic logo */
+	drm_rect_translate(&r_logo, (sb->width - r_logo.x2) / 2, 0);
 
 	msg_width = min(get_max_line_len(panic_msg, panic_msg_lines) * font->width, sb->width);
 	msg_height = min(panic_msg_lines * font->height, sb->height);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: drm/panic: Fix panic logo alignment
  2026-03-29 10:24 [PATCH] drm/panic: Fix panic logo alignment Dileep Sankhla
@ 2026-03-31  7:43 ` Claude Code Review Bot
  2026-03-31  7:43 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-31  7:43 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/panic: Fix panic logo alignment
Author: Dileep Sankhla <dileepsankhla.ds@gmail.com>
Patches: 1
Reviewed: 2026-03-31T17:43:30.257136

---

This is a single-patch fix to center the panic logo horizontally in the `DRM_PANIC_TYPE_USER` panic screen. The change is small, correct, and follows the same centering pattern already used for the panic message text a few lines below. The existing code left the logo at x=0 (left-aligned) while the message was centered, which was visually inconsistent.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: drm/panic: Fix panic logo alignment
  2026-03-29 10:24 [PATCH] drm/panic: Fix panic logo alignment Dileep Sankhla
  2026-03-31  7:43 ` Claude review: " Claude Code Review Bot
@ 2026-03-31  7:43 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-31  7:43 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Approach**: The patch adds a `drm_rect_translate()` call to horizontally center `r_logo` after it's initialized by `drm_panic_logo_rect()`. This is the same pattern used at line 502 for centering `r_msg`.

**Correctness**: Since `drm_panic_logo_rect()` initializes the rect at origin (0, 0) (see line 457), `r_logo.x2` equals the logo width, so `(sb->width - r_logo.x2) / 2` correctly computes the left offset for centering. The y-translation of 0 keeps it at the top.

**Potential type concern**: `sb->width` is `unsigned int` and `r_logo.x2` is `int` (from `struct drm_rect`). If the logo is wider than the screen, `(sb->width - r_logo.x2)` could produce a large unsigned value, resulting in a bad translation. However:
1. The same pattern is already used for `r_msg` at line 502 without a guard.
2. `drm_panic_logo_draw()` (line 468) already checks `rect->x2 > sb->width` and bails out, so a garbled rect would simply not be drawn.
3. In practice, panic logos are small relative to any screen.

So this is not a new issue introduced by this patch.

**Nit**: The commit message says "Fix panic logo alignment" which implies the current behavior is a bug. It might be more accurate to say "Center panic logo" since the left-alignment may have been the original intent rather than a bug. But this is subjective — if the intent was always to center it, "Fix" is appropriate.

**Verdict**: Looks good. The change is minimal, follows existing patterns, and improves visual consistency of the user-type panic screen.

Reviewed-by: No issues found that would block merging.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-31  7:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-29 10:24 [PATCH] drm/panic: Fix panic logo alignment Dileep Sankhla
2026-03-31  7:43 ` Claude review: " Claude Code Review Bot
2026-03-31  7:43 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox