Re: [PATCH] video: panel: Support for startek-kd070fhfid015 MIPI-DSI panel

On 8/6/26 6:53 AM, Julien Stephan wrote:
> This driver support the Startek KD070FHFID015, which is a 7-inch TFT LCD

s/support/supports/

> display using MIPI DSI interface. It is based on kernel commit
> b080a60731ad ("drm/panel: startek-kd070fhfid015: transition to mipi_dsi
> wrapped functions").
> 
> Signed-off-by: Julien Stephan <[email protected]>
> ---
> This driver support the Startek KD070FHFID015, which is a 7-inch TFT LCD
> display using MIPI DSI interface. It is based on kernel commit
> b080a60731ad ("drm/panel: startek-kd070fhfid015: transition to mipi_dsi
> wrapped functions").
> 
> This panel is used on some Mediatek Genio EVKS, such as Genio-350.
> 
> The full video pipeline series enabling full support will be sent later.
> 
> This series depends on [1], for mipi_dsi_write_seq helpers.
> 
> [1]: https://lore.kernel.org/all/20260806-add_mipi_dsi_write_seq_helper_macros-v1-0-6b533788d0ac@baylibre.com/
> ---
>  drivers/video/Kconfig                       |  10 ++
>  drivers/video/Makefile                      |   1 +
>  drivers/video/panel-startek-kd070fhfid015.c | 267 ++++++++++++++++++++++++++++

Probably should add MAINTAINERS entry.

>  3 files changed, 278 insertions(+)
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index cb3ba6a6f63..6de28d78705 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -320,6 +320,16 @@ config PANEL_HX8238D
>  	  It can drive a maximum 960x240 dot graphics on a-TFT panel
>  	  displays in 16M colors with dithering.
>  
> +config PANEL_STARTEK_KD070FHFID015
> +	bool "STARTEK KD070FHFID015 panel"
> +	depends on PANEL && DM_GPIO

&& DM_REGULATOR

> +	select VIDEO_MIPI_DSI
> +	help
> +	  Say Y here if you want to enable support for STARTEK KD070FHFID015 DSI panel.

This line is getting a bit long.

> +	  The panel is a 7-inch TFT LCD display with a resolution of 1200 x 1920
> +	  pixels. It provides a MIPI DSI interface to the host, a built-in LED
> +	  backlight and touch controller.
> +
>  config VIDEO_BOCHS
>  	bool "Enable Bochs video emulation for QEMU"
>  	help
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index a20011b9b17..7a54f1f290d 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_$(PHASE_)VIDEO) += video-uclass.o vidconsole-uclass.o
>  obj-$(CONFIG_$(PHASE_)VIDEO) += video_bmp.o
>  obj-$(CONFIG_$(PHASE_)PANEL) += panel-uclass.o
>  obj-$(CONFIG_PANEL_HX8238D) += hx8238d.o
> +obj-$(CONFIG_PANEL_STARTEK_KD070FHFID015) += panel-startek-kd070fhfid015.o
>  obj-$(CONFIG_$(PHASE_)SIMPLE_PANEL) += simple_panel.o
>  
>  obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.bmp.o
> diff --git a/drivers/video/panel-startek-kd070fhfid015.c b/drivers/video/panel-startek-kd070fhfid015.c
> new file mode 100644
> index 00000000000..c0b3cf1a5ff
> --- /dev/null
> +++ b/drivers/video/panel-startek-kd070fhfid015.c
> @@ -0,0 +1,267 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2016 InforceComputing
> + * Copyright (C) 2016 Linaro Ltd
> + * Copyright (C) 2026 BayLibre, SAS
> + *
> + * Authors:
> + * - Vinay Simha BN <[email protected]>
> + * - Sumit Semwal <[email protected]>
> + * - Guillaume La Roque <[email protected]>
> + *
> + * U-Boot port:
> + * Authors:
> + * - Julien Stephan <[email protected]>
> + */
> +
> +#include <asm-generic/gpio.h>
> +#include <backlight.h>
> +#include <dm.h>
> +#include <dm/device_compat.h>
> +#include <errno.h>
> +#include <linux/delay.h>
> +#include <malloc.h>
> +#include <mipi_dsi.h>
> +#include <panel.h>
> +#include <power/regulator.h>

Not all of these are used, e.g. backlight.h and malloc.h.
And should have explicit mipi_display.h.

> +
> +#define DSI_REG_MCAP	0xb0
> +#define DSI_REG_IS	0xb3 /* Interface Setting */
> +#define DSI_REG_IIS	0xb4 /* Interface ID Setting */
> +#define DSI_REG_CTRL	0xb6
> +
> +struct stk_panel {
> +	const struct drm_display_mode *mode;
> +	struct udevice *dev;
> +	struct gpio_desc *enable_gpio; /* Power IC supply enable */
> +	struct gpio_desc *reset_gpio; /* External reset */
> +	struct mipi_dsi_device *dsi;
> +	struct udevice *iovcc;
> +	struct udevice *power;
> +};
> +
> +static const struct drm_display_mode default_mode = {
> +	.clock = 163204,
> +	.hdisplay = 1200,
> +	.hsync_start = 1200 + 144,
> +	.hsync_end = 1200 + 144 + 16,
> +	.htotal = 1200 + 144 + 16 + 45,
> +	.vdisplay = 1920,
> +	.vsync_start = 1920 + 8,
> +	.vsync_end = 1920 + 8 + 4,
> +	.vtotal = 1920 + 8 + 4 + 4,
> +};
> +
> +static int stk_panel_init(struct stk_panel *stk)
> +{
> +	struct mipi_dsi_device *dsi = stk->dsi;
> +	struct udevice *dev = stk->dsi->dev;

This is the DSI host device. Probably meant st->dev here.

> +	int ret;
> +
> +	ret = mipi_dsi_dcs_soft_reset(dsi);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to mipi_dsi_dcs_soft_reset: %d\n", ret);
> +		return ret;
> +	}
> +	mdelay(5);
> +
> +	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set exit sleep mode: %d\n", ret);
> +		return ret;
> +	}
> +	mdelay(120);
> +
> +	mipi_dsi_generic_write_seq(dsi, DSI_REG_MCAP, 0x04);
> +
> +	/* Interface setting, video mode */
> +	mipi_dsi_generic_write_seq(dsi, DSI_REG_IS, 0x14, 0x08, 0x00, 0x22, 0x00);
> +	mipi_dsi_generic_write_seq(dsi, DSI_REG_IIS, 0x0c, 0x00);
> +	mipi_dsi_generic_write_seq(dsi, DSI_REG_CTRL, 0x3a, 0xd3);
> +
> +	ret = mipi_dsi_dcs_set_display_brightness(dsi, 0x77);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to write display brightness: %d\n", ret);
> +		return ret;
> +	}
> +
> +	mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
> +			       MIPI_DCS_WRITE_MEMORY_START);
> +
> +	ret = mipi_dsi_dcs_set_pixel_format(dsi, 0x77);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set pixel format: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = mipi_dsi_dcs_set_column_address(dsi, 0, stk->mode->hdisplay - 1);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set column address: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = mipi_dsi_dcs_set_page_address(dsi, 0, stk->mode->vdisplay - 1);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set page address: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int stk_panel_on(struct stk_panel *stk)
> +{
> +	struct mipi_dsi_device *dsi = stk->dsi;
> +	struct udevice *dev = stk->dsi->dev;
> +	int ret;
> +
> +	ret = mipi_dsi_dcs_set_display_on(dsi);
> +	if (ret < 0)
> +		dev_err(dev, "failed to set display on: %d\n", ret);
> +
> +	mdelay(20);
> +
> +	return ret;
> +}
> +
> +static int stk_panel_enable_backlight(struct udevice *dev)
> +{
> +	struct stk_panel *stk = dev_get_priv(dev);
> +	struct mipi_dsi_panel_plat *plat = dev_get_plat(dev);
> +	struct mipi_dsi_device *dsi = plat->device;
> +	int ret;
> +
> +	stk->dsi = dsi;
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0)
> +		return ret;
> +
> +	dm_gpio_set_value(stk->reset_gpio, 0);
> +	dm_gpio_set_value(stk->enable_gpio, 0);
> +	ret = regulator_enable(stk->iovcc);
> +	if (ret < 0)

Do we need to unwind with mipi_dsi_detach() here?

> +		return ret;
> +
> +	mdelay(8);
> +	ret = regulator_enable(stk->power);
> +	if (ret < 0)
> +		goto iovccoff;
> +
> +	mdelay(20);
> +	dm_gpio_set_value(stk->enable_gpio, 1);
> +	mdelay(20);
> +	dm_gpio_set_value(stk->reset_gpio, 1);
> +	mdelay(10);
> +
> +	ret = stk_panel_init(stk);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to init panel: %d\n", ret);
> +		goto poweroff;
> +	}
> +
> +	ret = stk_panel_on(stk);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set panel on: %d\n", ret);
> +		goto poweroff;
> +	}
> +
> +	return 0;
> +
> +poweroff:
> +	regulator_disable(stk->power);
> +iovccoff:
> +	regulator_disable(stk->iovcc);
> +	dm_gpio_set_value(stk->reset_gpio, 0);
> +	dm_gpio_set_value(stk->enable_gpio, 0);
> +
> +	return ret;
> +}
> +
> +static int stk_panel_add(struct stk_panel *stk)
> +{
> +	struct udevice *dev = stk->dev;
> +	int ret;
> +
> +	stk->mode = &default_mode;
> +
> +	ret = device_get_supply_regulator(dev, "iovcc-supply", &stk->iovcc);
> +	if (ret) {
> +		dev_err(dev, "Failed to get iovcc regulator: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = device_get_supply_regulator(dev, "power-supply", &stk->power);
> +	if (ret) {
> +		dev_err(dev, "Failed to get power regulator: %d\n", ret);
> +		return ret;
> +	}
> +
> +	stk->reset_gpio = devm_gpiod_get(dev, "reset",
> +					 GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);

Driving the gpios before enabling the regulators is probably not a good idea.

> +	if (IS_ERR(stk->reset_gpio)) {
> +		ret = PTR_ERR(stk->reset_gpio);
> +		dev_err(dev, "cannot get reset-gpios %d\n", ret);
> +		return ret;
> +	}
> +
> +	stk->enable_gpio = devm_gpiod_get(dev, "enable",
> +					  GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
> +	if (IS_ERR(stk->enable_gpio)) {
> +		ret = PTR_ERR(stk->enable_gpio);
> +		dev_err(dev, "cannot get enable-gpio %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int stk_panel_probe(struct udevice *dev)
> +{
> +	struct stk_panel *stk = dev_get_priv(dev);
> +	struct mipi_dsi_panel_plat *plat = dev_get_plat(dev);
> +	int ret;
> +
> +	stk->dev = dev;
> +
> +	/* fill characteristics of DSI data link */
> +	plat->lanes = 4;
> +	plat->format = MIPI_DSI_FMT_RGB888;
> +	plat->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_LPM;
> +
> +	ret = stk_panel_add(stk);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +int stk_panel_get_modes(struct udevice *dev, const struct drm_display_mode **modes)

static

> +{
> +	struct stk_panel *stk = dev_get_priv(dev);
> +
> +	if (!stk->mode)
> +		return -ENODEV;
> +
> +	*modes = stk->mode;

nit: blank line before return

> +	return 1;
> +}
> +
> +static const struct panel_ops stk_panel_ops = {
> +	.enable_backlight	= stk_panel_enable_backlight,
> +	.get_modes		= stk_panel_get_modes,
> +};
> +
> +static const struct udevice_id stk_of_match[] = {
> +	{ .compatible = "startek,kd070fhfid015", },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(stk_panel_driver) = {

Suggest we make the prefix stk078 in this file instead of stk as this
is not the only startek panel. I used stk015 in the driver I recently
submitted.

> +	.name		= "panel-startek-kd070fhfid015",
> +	.id		= UCLASS_PANEL,
> +	.of_match	= stk_of_match,
> +	.ops		= &stk_panel_ops,
> +	.probe		= stk_panel_probe,
> +	.plat_auto	= sizeof(struct mipi_dsi_panel_plat),
> +	.priv_auto	= sizeof(struct stk_panel),
> +};
>