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/panel: simple: add NEC NL6448BC33-70C
Date: Sat, 16 May 2026 09:23:02 +1000
Message-ID: <review-patch2-20260515-v7-1-topic-panel-simple-nl6448bc33-v2-2-74ab0baa1817@pengutronix.de>
In-Reply-To: <20260515-v7-1-topic-panel-simple-nl6448bc33-v2-2-74ab0baa1817@pengutronix.de>
References: <20260515-v7-1-topic-panel-simple-nl6448bc33-v2-0-74ab0baa1817@pengutronix.de> <20260515-v7-1-topic-panel-simple-nl6448bc33-v2-2-74ab0baa1817@pengutronix.de>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Status: One question on timing values.**

The `panel_desc` structure is well-formed and follows conventions:

```c
static const struct panel_desc nec_nl6448bc33_70c =3D {
	.modes =3D &nec_nl6448bc33_70c_mode,
	.num_modes =3D 1,
	.bpc =3D 6,
	.size =3D {
		.width =3D 211,
		.height =3D 158,
	},
	.bus_format =3D MEDIA_BUS_FMT_RGB666_1X18,
	.bus_flags =3D DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
	.connector_type =3D DRM_MODE_CONNECTOR_LVDS,
};
```

- `.bpc =3D 6` with `MEDIA_BUS_FMT_RGB666_1X18` =E2=80=94 consistent (6 bit=
s =C3=97 3 channels =3D 18 bits).
- `.connector_type =3D DRM_MODE_CONNECTOR_LVDS` =E2=80=94 correctly set (th=
e neighboring `nec_nl4827hc19_05b` omits this and would hit the "Specify mi=
ssing connector_type" warning at runtime).
- `.size` =3D 211=C3=97158 mm =E2=80=94 correct for a 10.4" panel with 4:3 =
aspect (10.4" =C3=97 4/5 =E2=89=88 211 mm, 10.4" =C3=97 3/5 =E2=89=88 158 m=
m).
- OF match table entry is in correct alphabetical position.

**Timing concern:**

```c
static const struct drm_display_mode nec_nl6448bc33_70c_mode =3D {
	.clock =3D 25175,
	.hdisplay =3D 640,
	.hsync_start =3D 640 + 16,      /* HFP =3D 16 */
	.hsync_end =3D 640 + 16 + 48,   /* HSW =3D 48 */
	.htotal =3D 640 + 16 + 48 + 96, /* HBP =3D 96 */
	.vdisplay =3D 480,
	.vsync_start =3D 480 + 2,       /* VFP =3D 2  */
	.vsync_end =3D 480 + 2 + 31,    /* VSW =3D 31 */
	.vtotal =3D 480 + 2 + 31 + 31,  /* VBP =3D 31 */
	.flags =3D DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_PHSYNC,
};
```

The htotal (800) is standard for 640=C3=97480, but:

1. **HSW and HBP appear swapped vs. standard VGA** =E2=80=94 standard 640=
=C3=97480@60 has HSW=3D96 and HBP=3D48, i.e. the reverse of what's here. If=
 these values come from the datasheet they may be correct, but it's worth d=
ouble-checking.

2. **vtotal =3D 544 instead of 525** =E2=80=94 this yields a refresh rate o=
f 25175000 / (800 =C3=97 544) =E2=89=88 **57.85 Hz**, which is below the ty=
pical 59.94 Hz. The vertical sync width of 31 lines and back porch of 31 li=
nes are quite different from standard VGA (VSW=3D2, VBP=3D33). Again, this =
may be correct per the panel's specifications, but the submitter should con=
firm these are from the NL6448BC33-70C datasheet and not a transcription er=
ror.

Everything else =E2=80=94 code structure, naming, placement =E2=80=94 looks=
 correct and ready to merge pending timing confirmation.

---
Generated by Claude Code Patch Reviewer