[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <jqgwmjet2ncit5bqilvuqldxrtcapiozmpbk5klc5tybwbfezi@4muxgno7do45>
Date: Sat, 6 Dec 2025 06:25:56 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: david@...t.cz
Cc: Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Sumit Semwal <sumit.semwal@...aro.org>,
Casey Connolly <casey.connolly@...aro.org>,
Neil Armstrong <neil.armstrong@...aro.org>,
Jessica Zhang <jesszhan0024@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Marijn Suijten <marijn.suijten@...ainline.org>,
Dmitry Baryshkov <lumag@...nel.org>, Vinod Koul <vkoul@...nel.org>,
phodina@...tonmail.com, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
phone-devel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v4 4/8] drm/panel: sw43408: Add enable/disable and reset
functions
On Tue, Nov 25, 2025 at 09:29:39PM +0100, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@...t.cz>
>
> Introduce enable(), disable() and reset() functions.
>
> The enable() and disable() callbacks keep the symmetry in the commands
> sent to the panel and also make a clearer distinction between panel
> initialization and configuration.
This also makes those to to be executed after starting the DSI stream.
Is it fine?
>
> Splitting reset() from prepare() follows clean coding practices and lets
> us potentially make reset optional in the future for flicker-less
> takeover from a bootloader or framebuffer driver where the panel is
> already configured.
>
> Signed-off-by: David Heidelberg <david@...t.cz>
> ---
> drivers/gpu/drm/panel/panel-lg-sw43408.c | 83 ++++++++++++++++++++------------
> 1 file changed, 53 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> index dcca7873acf8e..20217877e107f 100644
> --- a/drivers/gpu/drm/panel/panel-lg-sw43408.c
> +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> @@ -38,11 +38,10 @@ static inline struct sw43408_panel *to_panel_info(struct drm_panel *panel)
> return container_of(panel, struct sw43408_panel, base);
> }
>
> -static int sw43408_unprepare(struct drm_panel *panel)
> +static int sw43408_disable(struct drm_panel *panel)
> {
> struct sw43408_panel *sw43408 = to_panel_info(panel);
> struct mipi_dsi_multi_context ctx = { .dsi = sw43408->link };
> - int ret;
>
> mipi_dsi_dcs_set_display_off_multi(&ctx);
>
> @@ -50,19 +49,55 @@ static int sw43408_unprepare(struct drm_panel *panel)
>
> mipi_dsi_msleep(&ctx, 100);
>
> + return ctx.accum_err;
> +}
> +
> +static int sw43408_unprepare(struct drm_panel *panel)
> +{
> + struct sw43408_panel *sw43408 = to_panel_info(panel);
> + int ret;
> +
> gpiod_set_value(sw43408->reset_gpio, 1);
>
> ret = regulator_bulk_disable(ARRAY_SIZE(sw43408->supplies), sw43408->supplies);
>
> - return ret ? : ctx.accum_err;
> + return ret;
> }
>
> -static int sw43408_program(struct drm_panel *panel)
> +static int sw43408_enable(struct drm_panel *panel)
Please move it below sw43408_program() to ease code review.
> {
> struct sw43408_panel *sw43408 = to_panel_info(panel);
> struct mipi_dsi_multi_context ctx = { .dsi = sw43408->link };
> struct drm_dsc_picture_parameter_set pps;
>
> + mipi_dsi_dcs_set_display_on_multi(&ctx);
> +
> + mipi_dsi_msleep(&ctx, 50);
> +
> + sw43408->link->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> + drm_dsc_pps_payload_pack(&pps, sw43408->link->dsc);
> +
> + mipi_dsi_picture_parameter_set_multi(&ctx, &pps);
> +
> + sw43408->link->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> + /*
> + * This panel uses PPS selectors with offset:
> + * PPS 1 if pps_identifier is 0
> + * PPS 2 if pps_identifier is 1
> + */
> + mipi_dsi_compression_mode_ext_multi(&ctx, true,
> + MIPI_DSI_COMPRESSION_DSC, 1);
> +
> + return ctx.accum_err;
> +}
> +
> +static int sw43408_program(struct drm_panel *panel)
> +{
> + struct sw43408_panel *sw43408 = to_panel_info(panel);
> + struct mipi_dsi_multi_context ctx = { .dsi = sw43408->link };
> +
> mipi_dsi_dcs_write_seq_multi(&ctx, MIPI_DCS_SET_GAMMA_CURVE, 0x02);
>
> mipi_dsi_dcs_set_tear_on_multi(&ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> @@ -97,26 +132,19 @@ static int sw43408_program(struct drm_panel *panel)
> mipi_dsi_dcs_write_seq_multi(&ctx, 0x55, 0x04, 0x61, 0xdb, 0x04, 0x70, 0xdb);
> mipi_dsi_dcs_write_seq_multi(&ctx, 0xb0, 0xca);
>
> - mipi_dsi_dcs_set_display_on_multi(&ctx);
> -
> - mipi_dsi_msleep(&ctx, 50);
> -
> - sw43408->link->mode_flags &= ~MIPI_DSI_MODE_LPM;
> -
> - drm_dsc_pps_payload_pack(&pps, sw43408->link->dsc);
> -
> - mipi_dsi_picture_parameter_set_multi(&ctx, &pps);
> + return ctx.accum_err;
> +}
>
> - sw43408->link->mode_flags |= MIPI_DSI_MODE_LPM;
> +static void sw43408_reset(struct sw43408_panel *ctx)
> +{
> + usleep_range(5000, 6000);
>
> - /*
> - * This panel uses PPS selectors with offset:
> - * PPS 1 if pps_identifier is 0
> - * PPS 2 if pps_identifier is 1
> - */
> - mipi_dsi_compression_mode_ext_multi(&ctx, true,
> - MIPI_DSI_COMPRESSION_DSC, 1);
> - return ctx.accum_err;
> + gpiod_set_value(ctx->reset_gpio, 0);
> + usleep_range(9000, 10000);
> + gpiod_set_value(ctx->reset_gpio, 1);
> + usleep_range(1000, 2000);
> + gpiod_set_value(ctx->reset_gpio, 0);
> + usleep_range(9000, 10000);
> }
>
> static int sw43408_prepare(struct drm_panel *panel)
> @@ -128,14 +156,7 @@ static int sw43408_prepare(struct drm_panel *panel)
> if (ret < 0)
> return ret;
>
> - usleep_range(5000, 6000);
> -
> - gpiod_set_value(ctx->reset_gpio, 0);
> - usleep_range(9000, 10000);
> - gpiod_set_value(ctx->reset_gpio, 1);
> - usleep_range(1000, 2000);
> - gpiod_set_value(ctx->reset_gpio, 0);
> - usleep_range(9000, 10000);
> + sw43408_reset(ctx);
>
> ret = sw43408_program(panel);
> if (ret)
> @@ -208,6 +229,8 @@ static int sw43408_backlight_init(struct sw43408_panel *ctx)
> }
>
> static const struct drm_panel_funcs sw43408_funcs = {
> + .disable = sw43408_disable,
> + .enable = sw43408_enable,
> .unprepare = sw43408_unprepare,
> .prepare = sw43408_prepare,
> .get_modes = sw43408_get_modes,
>
> --
> 2.51.0
>
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists