[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD++jL=c0AVm+BD_G_AJjiXYVyWdM_HscqyoNQ-tBByJ1Dp1jQ@mail.gmail.com>
Date: Fri, 26 Dec 2025 14:12:25 +0100
From: Linus Walleij <linusw@...nel.org>
To: Marijn Suijten <marijn.suijten@...ainline.org>
Cc: Neil Armstrong <neil.armstrong@...aro.org>, Sam Ravnborg <sam@...nborg.org>,
David Airlie <airlied@...il.com>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
Andy Gross <agross@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
Jessica Zhang <jesszhan0024@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, Simona Vetter <simona@...ll.ch>,
Casey Connolly <casey.connolly@...aro.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Simona Vetter <simona.vetter@...ll.ch>,
~postmarketos/upstreaming@...ts.sr.ht,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...ainline.org>,
Martin Botka <martin.botka@...ainline.org>, Jami Kettunen <jami.kettunen@...ainline.org>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
Abhinav Kumar <quic_abhinavk@...cinc.com>, Kuogee Hsieh <quic_khsieh@...cinc.com>,
Jessica Zhang <quic_jesszhan@...cinc.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Konrad Dybcio <konrad.dybcio@....qualcomm.com>, Konrad Dybcio <konradybcio@...nel.org>,
Dmitry Baryshkov <lumag@...nel.org>
Subject: Re: [PATCH v2 03/11] drm/panel: Add LGD LH599QH3-EDB1 panel driver
for Sony Xperia XZ3
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
<marijn.suijten@...ainline.org> 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 <marijn.suijten@...ainline.org>
(...)
> +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 <linusw@...nel.org>
Yours,
Linus Walleij
Powered by blists - more mailing lists