On Tue, Nov 25, 2025 at 09:29:39PM +0100, David Heidelberg via B4 Relay wrote: > From: David Heidelberg <[email protected]> > > 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 <[email protected]> > --- > 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
