Hi Marijn, thanks for your patch!
overall this looks very nice. When adding new panels it's nice if we can figure out which display controller it is actually using, because that makes it possible to share a driver between multiple displays using a per-display-controller driver. But this one beats me, so keep it as-is... no idea what display controller (DDIC) this is. On Mon, Dec 22, 2025 at 12:32 AM Marijn Suijten <[email protected]> wrote: > > Sony provides an LGD LH599QH3-EDB1 panel + Atmel maXTouch assembly in > its Xperia XZ3 (tama akatsuki) phone, with custom DCS commands to match. > > The panel is 1440x2880 pixels and runs at 60Hz. It requires Display > Stream Compression 1.1 to be driven at that mode. > > Signed-off-by: Marijn Suijten <[email protected]> (...) > +config DRM_PANEL_LGD_LH599QH3_EDB1 > + tristate "LGD LH599QH3-EDB1 DSI cmd mode panel" > + depends on GPIOLIB > + depends on OF > + depends on DRM_MIPI_DSI > + depends on BACKLIGHT_CLASS_DEVICE > + select DRM_DISPLAY_DSC_HELPER > + select DRM_DISPLAY_HELPER Nice use of helpers! > +static int lgd_lh599qh3_edb1_program(struct lgd_lh599qh3_edb1 *ctx) > +{ > + struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi }; > + > + dsi_ctx.dsi->mode_flags |= MIPI_DSI_MODE_LPM; > + > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7f, 0x5a, 0x5a); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x5a, 0x5a); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf1, 0x5a, 0x5a); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf2, 0x5a, 0x5a); Clearly an "unlock" sequence. > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x02, 0x01); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x59, 0x01); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, > + WRITE_CONTROL_DISPLAY_BACKLIGHT); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x57, 0x20, 0x80, 0xde, 0x60, > 0x00); > + mipi_dsi_dcs_set_column_address_multi(&dsi_ctx, 0, 1440 - 1); > + mipi_dsi_dcs_set_page_address_multi(&dsi_ctx, 0, 2880 - 1); > + > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_POWER_SAVE, > 0x00); > + > + mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, > MIPI_DSI_DCS_TEAR_MODE_VBLANK); > + > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7f, 0x5a, 0x5a); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf0, 0x5a, 0x5a); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf1, 0x5a, 0x5a); > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf2, 0x5a, 0x5a); Cleary another copy of the "unlock" sequence. Can you break it out to a subroutine? lgd_lh599qh3_edb1_unlock()? +/- this nitpick: Reviewed-by: Linus Walleij <[email protected]> Yours, Linus Walleij
