From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/hibmc: Do not use cpp from struct drm_format_info
Date: Thu, 04 Jun 2026 14:13:48 +1000
Message-ID: <review-patch3-20260601114756.51953-4-tzimmermann@suse.de>
In-Reply-To: <20260601114756.51953-4-tzimmermann@suse.de>
References: <20260601114756.51953-1-tzimmermann@suse.de> <20260601114756.51953-4-tzimmermann@suse.de>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**Status: Good, minor nit**

Two cleanups replacing deprecated `cpp[0]` usage:

1. Line width calculation:
```c
reg = drm_format_info_min_pitch(fb->format, 0, fb->width);
```
Correct -- this is the preferred API for computing pitch from format info.

2. Format register programming:
```c
switch (fb->format->format) {
case DRM_FORMAT_XRGB8888:
    reg |= HIBMC_FIELD(HIBMC_CRT_DISP_CTL_FORMAT, 2);
    break;
case DRM_FORMAT_RGB565:
    reg |= HIBMC_FIELD(HIBMC_CRT_DISP_CTL_FORMAT, 1);
    break;
}
```

The switch has no `default` case. This is safe because `atomic_check` ensures only XRGB8888 and RGB565 reach `atomic_update`, and patch 2 restricts the format list. However, a `default: break;` or a `drm_WARN_ON_ONCE` in the default would be a minor defensive improvement. Not blocking -- this matches how other simple drivers handle it.

The absence of a Fixes/Cc:stable tag is correct since patch 2 already removed the format (RGB888) that was broken by the cpp calculation.

---
Generated by Claude Code Patch Reviewer