Re: [PATCH v5 2/3] drm: panel: Add Samsung S6E8FC0 DSI controller for M1906F9 panel

[David Heidelberg <david@ixit.cz>]

On 17/03/2026 19:50, Yedaya Katsman wrote:
> Add driver for Samsung S6E8FC0 DSI controller for M1906F9 video mode panel,
> found in Xiaomi Mi A3 mobile phone.
> 
> Co-developed-by: Kamil Gołda <kamil.golda@protonmail.com>
> Signed-off-by: Kamil Gołda <kamil.golda@protonmail.com>
> Signed-off-by: Yedaya Katsman <yedaya.ka@gmail.com>
> 
> ---
> 
> The downstream dts claims a max brightness of 2047, but seems to ignore the
> MSB.
> ---

[...]
> +
> +static const struct regulator_bulk_data s6e8fc0_m1906f9_supplies[] = {
> +	{ .supply = "vdd" },
> +	{ .supply = "vci" },
> +};

these power supplies should be only DDIC related

> +
> +static inline
> +struct s6e8fc0_ctx *to_s6e8fc0_ctx(struct drm_panel *panel)
> +{
> +	return container_of_const(panel, struct s6e8fc0_ctx, panel);
> +}
> +
> +static void s6e8fc0_m1906f9_reset(struct s6e8fc0_ctx *ctx)
> +{
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	usleep_range(12000, 13000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	usleep_range(2000, 3000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	usleep_range(10000, 11000);
> +}
> +
> +#define s6e8fc0_m1906f9_test_key_on_lvl2(ctx) \
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xf0, 0x5a, 0x5a)
> +#define s6e8fc0_m1906f9_test_key_off_lvl2(ctx) \
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xf0, 0xa5, 0xa5)
> +#define s6e8fc0_m1906f9_test_key_on_lvl3(ctx) \
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xfc, 0x5a, 0x5a)
> +#define s6e8fc0_m1906f9_test_key_off_lvl3(ctx) \
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xfc, 0xa5, 0xa5)
> +

macros are definitely DDIC only (not panel related, thus s6e8fc0 only)

> +static int s6e8fc0_m1906f9_on(struct s6e8fc0_ctx *ctx)
> +{
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +
> +	s6e8fc0_m1906f9_test_key_on_lvl3(&dsi_ctx);
> +
> +	mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, 0x0000);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY,
> +				     0x20);
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 50);
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb0, 0x04, 0xed);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xed,
> +				     0xe4, 0x08, 0x96, 0xa4, 0x2a, 0x72, 0xe2,
> +				     0xca, 0x00);
> +	s6e8fc0_m1906f9_test_key_off_lvl3(&dsi_ctx);
> +	s6e8fc0_m1906f9_test_key_on_lvl2(&dsi_ctx);
> +	s6e8fc0_m1906f9_test_key_on_lvl3(&dsi_ctx);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe1, 0x93);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb0, 0x05, 0xf4);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf4, 0x03);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xed, 0x01, 0x81, 0x04);
> +	s6e8fc0_m1906f9_test_key_off_lvl2(&dsi_ctx);
> +	s6e8fc0_m1906f9_test_key_off_lvl3(&dsi_ctx);
> +
> +	return dsi_ctx.accum_err;
> +}
> +

[...]

> +
> +static const struct of_device_id samsung_s6e8fc0_m1906f9_of_match[] = {

this struct is only DDIC related, the content of it below is panel related

> +	{ .compatible = "samsung,s6e8fc0-m1906f9" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, samsung_s6e8fc0_m1906f9_of_match);
> +
> +static struct mipi_dsi_driver s6e8fc0_m1906f9_driver = {
> +	.probe = s6e8fc0_m1906f9_probe,
> +	.remove = s6e8fc0_remove,
> +	.driver = {
> +		.name = "panel-samsung-s6e8fc0-m1906f9",
> +		.of_match_table = samsung_s6e8fc0_m1906f9_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(s6e8fc0_m1906f9_driver);

mipi_dsi_driver is DDIC related, not panel.


Otherwise LGTM, with requested changes

Reviewed-by: David Heidelberg <david@ixit.cz>

Thank you!
David

> +
> +MODULE_AUTHOR("Kamil Gołda <kamil.golda@protonmail.com>");
> +MODULE_DESCRIPTION("DRM driver for Samsung s6e8fc0 DSI controller");
> +MODULE_LICENSE("GPL");
> 

-- 
David Heidelberg