Re: [PATCH RFC 2/2] drivers: gpu: drm: panel: Add BOE NT51021 driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The panel initialization sequence lacks the DCS 'Set Display On' (0x29) command.
- [Medium] Missing reset assertion and stabilization delay during panel preparation.
--

commit 864f3dccc86bc8a97845ebafd73a7df32e9d6b9e
Author: Nickolay Goppen <setotau@mainlining.org>

drivers: gpu: drm: panel: Add BOE NT51021 driver

This commit adds support for the BOE NT51021-based 8-inch and 10-inch DSI
video mode panels used in Xiaomi Mi Pad 4 tablets. It includes panel
initialization sequences and content-adaptive brightness control.

> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt51021.c b/drivers/gpu/drm/panel/panel-novatek-nt51021.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt51021.c
[ ... ]
> +static void nt51021_boe_reset(struct boe_nt51021_desc *ctx)
> +{
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	usleep_range(10000, 11000);
> +}

[Severity: Medium]
Does the panel require an explicit reset assertion pulse before de-asserting
the reset line? The current code only de-asserts the reset line here. Without
an explicit assertion pulse and delay prior to de-assertion, the hardware
might not be guaranteed a valid minimum reset pulse width once power is stable.

[ ... ]
> +static int nt51021_boe_8_init(struct boe_nt51021_desc *ctx)
> +{
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
[ ... ]
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_usleep_range(&dsi_ctx, 5000, 6000);
> +	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x8f, 0x00);
> +	mipi_dsi_usleep_range(&dsi_ctx, 1000, 2000);
> +
> +	nt51021_cabc_on(&dsi_ctx);
> +
> +	return dsi_ctx.accum_err;
> +}

[Severity: Medium]
Does this initialization sequence miss a DCS "Set Display On" command?
The panel is awakened from sleep mode via mipi_dsi_dcs_exit_sleep_mode_multi(),
but it is never explicitly instructed to turn on the display output here in
nt51021_boe_8_init(). This could cause the panel to remain blank on some DSI
hosts.

[ ... ]
> +static int nt51021_boe_10wu_init(struct boe_nt51021_desc *ctx)
> +{
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
[ ... ]
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_usleep_range(&dsi_ctx, 5000, 6000);
> +	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x8f, 0x00);
> +	mipi_dsi_usleep_range(&dsi_ctx, 1000, 2000);
> +
> +	nt51021_cabc_on(&dsi_ctx);
> +	mipi_dsi_usleep_range(&dsi_ctx, 1000, 2000);
> +
> +	return dsi_ctx.accum_err;
> +}

[Severity: Medium]
Similarly to nt51021_boe_8_init(), the 10-inch variant initialization path
also appears to omit the mipi_dsi_dcs_set_display_on_multi() command.

> +static int nt51021_boe_off(struct boe_nt51021_desc *ctx)
> +{
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +
> +	nt51021_cabc_off(&dsi_ctx);
> +
> +	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x8f, 0xa5);
> +	mipi_dsi_msleep(&dsi_ctx, 20);
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);

[Severity: Medium]
The power-off sequence explicitly turns the display off using
mipi_dsi_dcs_set_display_off_multi(). Should there be a matching turn-on
command in the initialization sequences to ensure the display activates
correctly across power cycles?

[ ... ]
> +static int nt51021_boe_prepare(struct drm_panel *panel)
> +{
> +	struct boe_nt51021_desc *ctx = to_boe_panel(panel);
> +	struct device *dev = &ctx->dsi->dev;
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(nt51021_boe_supplies), ctx->supplies);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable regulators: %d\n", ret);
> +		return ret;
> +	}
> +	nt51021_boe_reset(ctx);

[Severity: Medium]
Does the panel require a stabilization delay after enabling the regulators
and before de-asserting the reset line? The current code immediately calls
nt51021_boe_reset(ctx). Without a delay, the reset logic might execute
before the regulators have fully ramped up, potentially leading to unstable
hardware initialization.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829-qcom-novatek-nt51021-panels-v1-0-cc1ca61bd261@mainlining.org?part=2