[PATCH v4 4/8] drm/panel: sw43408: Add enable/disable and reset functions

David Heidelberg via B4 Relay posted 8 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v4 4/8] drm/panel: sw43408: Add enable/disable and reset functions
Posted by David Heidelberg via B4 Relay 2 months, 2 weeks ago
From: David Heidelberg <david@ixit.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.

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@ixit.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)
 {
 	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
Re: [PATCH v4 4/8] drm/panel: sw43408: Add enable/disable and reset functions
Posted by Dmitry Baryshkov 2 months ago
On Tue, Nov 25, 2025 at 09:29:39PM +0100, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.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@ixit.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
Re: [PATCH v4 4/8] drm/panel: sw43408: Add enable/disable and reset functions
Posted by David Heidelberg 2 months ago
On 06/12/2025 05:25, Dmitry Baryshkov wrote:
> On Tue, Nov 25, 2025 at 09:29:39PM +0100, David Heidelberg via B4 Relay wrote:
>> From: David Heidelberg <david@ixit.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?
> 

Yes, the panel works same way as before without patchset.
Still not recovering from panel off, we tried to debug, not there yet, 
but the sequences are verified to work fine).

Reset sequence or/and regulators may not be correct enough to recover.

I'm sending another series with moved _enable function and all R-Bs.

David

[...]