Re: [PATCH v3 2/2] drm/panel: simple: add NEC NL6448BC33-70C

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The horizontal and vertical display timings incorrectly map front porch, back porch, and sync pulse length values, swapping them in the additive configuration.
--

commit 7c39ee64a0bc09df1a4fc9c15180db0749bff494
Author: Steffen Trumtrar <s.trumtrar@pengutronix.de>

drm/panel: simple: add NEC NL6448BC33-70C

This commit adds support for the NEC NL6448BC33-70C 10.4" 640x480 LCD module.

> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 236bd56208ccf..0bf50b1ed69e6 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -3659,6 +3659,32 @@ static const struct panel_desc nec_nl4827hc19_05b = {
[ ... ]
> +static const struct drm_display_mode nec_nl6448bc33_70c_mode = {
> +	.clock = 25175,
> +	.hdisplay = 640,
> +	.hsync_start = 640 + 16,
> +	.hsync_end = 640 + 16 + 48,
> +	.htotal = 640 + 16 + 48 + 96,

Are the sync pulse length and back porch values transposed here?

The additive timing formula is hsync_end = hsync_start + hsync_len and
htotal = hsync_end + hback_porch. This configuration results in an hsync_len
of 48 and an hback_porch of 96. Standard 640x480 timing with a 25.175 MHz
clock typically uses an hsync_len of 96 and an hback_porch of 48.

> +	.vdisplay = 480,
> +	.vsync_start = 480 + 2,
> +	.vsync_end = 480 + 2 + 31,
> +	.vtotal = 480 + 2 + 31 + 31,

Does this code similarly transpose the vertical timings?

This setup yields a vsync_len of 31 lines, which seems abnormally large
for an LCD panel. Could this cause the panel to fail to sync or incorrectly
shift the displayed image?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-v7-1-topic-panel-simple-nl6448bc33-v3-0-21ea14a6e835@pengutronix.de?part=2