Re: [PATCH v2 2/2] gpu/drm/panel: Add Lenovo NT36523W BOE panel

From: Linus Walleij
Date: Tue Mar 07 2023 - 17:19:28 EST


On Tue, Mar 7, 2023 at 2:26 PM Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:

> Introduce support for the BOE panel with a NT36523W touch/driver IC
> found on some Lenovo Tab P11 devices. It's a 2000x1200, 24bit RGB
> MIPI DSI panel with integrated DCS-controlled backlight (that expects
> big-endian communication).
>
> Reviewed-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>

I will think this is some variant of the Novatek NT36523 display
controller packaged up with Lenovo electronics until proven how
wrong I am.

I will listen to reason if it can be demonstrated that NT36523 and
NT36523W are considerably different and need very different
drivers, but I seriously doubt it. (For reasons see below.)

> drivers/gpu/drm/panel/panel-lenovo-nt36523w-boe.c | 747 ++++++++++++++++++++++

We usually share code with different displays using the
same display controller, so panel-novatek-nt36523.c should
be used as name.

> +config DRM_PANEL_LENOVO_NT36523W_BOE
> + tristate "Lenovo NT36523W BOE panel"

Name it after the display controller like the other examples
in the Kconfig, DRM_PANEL_NOVATEK_NT36523

> + mipi_dsi_dcs_write_seq(dsi, 0xff, 0x20);
> + mipi_dsi_dcs_write_seq(dsi, 0xfb, 0x01);
> + mipi_dsi_dcs_write_seq(dsi, 0x05, 0xd9);
> + mipi_dsi_dcs_write_seq(dsi, 0x07, 0x78);
> + mipi_dsi_dcs_write_seq(dsi, 0x08, 0x5a);
> + mipi_dsi_dcs_write_seq(dsi, 0x0d, 0x63);
> + mipi_dsi_dcs_write_seq(dsi, 0x0e, 0x91);
> + mipi_dsi_dcs_write_seq(dsi, 0x0f, 0x73);
> + mipi_dsi_dcs_write_seq(dsi, 0x95, 0xeb);
> + mipi_dsi_dcs_write_seq(dsi, 0x96, 0xeb);
> + mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_PARTIAL_ROWS, 0x11);

I think it looks very similar to Jianhua:s driver:
https://lore.kernel.org/lkml/20230220121258.10727-1-lujianhua000@xxxxxxxxx/T/

Can't you just add this special magic sequence into
that driver instead?

Would it help if we merge Jianhua's driver first so you can patch on
top of it?

Yours,
Linus Walleij