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 <[email protected]> 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-cc1ca61bd...@mainlining.org?part=2
