Message-ID: <anldo4jltvbu2smqo4ognxkauqs6aomm742f5v7bhryi6jappx@lfcig7pn5go6>
Date: Mon, 6 Oct 2025 02:21:59 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Langyan Ye <yelangyan@...qin.corp-partner.google.com>
Cc: neil.armstrong@...aro.org, jessica.zhang@....qualcomm.com,
        airlied@...il.com, simona@...ll.ch, maarten.lankhorst@...ux.intel.com,
        mripard@...nel.org, tzimmermann@...e.de, robh@...nel.org,
        krzk+dt@...nel.org, conor+dt@...nel.org, dianders@...omium.org,
        dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] drm/panel: Add driver for Tianma TL121BVMS07-00
 panel

On Tue, Sep 30, 2025 at 03:50:44PM +0800, Langyan Ye wrote:
> Add a DRM panel driver for the Tianma TL121BVMS07-00 12.1"
> MIPI-DSI TFT LCD panel. The panel requires multiple power
> supplies (AVDD, AVEE, 1.8V logic), an enable GPIO, and a
> backlight device. The panel is based on the Ilitek IL79900A
> controller.

The rest of panels based on Ilitek controllers are handled by
panel-ilitek-iliNNNN.c drivers (which might or might not be shared by
several panels). Is there a reason to deviate from that custom?

> 
> Signed-off-by: Langyan Ye <yelangyan@...qin.corp-partner.google.com>
> ---
>  .../drm/panel/panel-tianma-tl121bvms07-00.c   | 419 ++++++++++++++++++
>  1 file changed, 419 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-tianma-tl121bvms07-00.c
> 
> diff --git a/drivers/gpu/drm/panel/panel-tianma-tl121bvms07-00.c b/drivers/gpu/drm/panel/panel-tianma-tl121bvms07-00.c
> new file mode 100644
> index 000000000000..5facffeda864
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-tianma-tl121bvms07-00.c
> @@ -0,0 +1,419 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DRM panel driver for Tianma TL121BVMS07-00 12.1" MIPI-DSI TFT LCD panel
> + *
> + * Based on drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> + */
> +
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_connector.h>
> +#include <drm/drm_crtc.h>

Unnecessary

> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_panel.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct tm_panel;
> +
> +struct panel_desc {
> +	const struct drm_display_mode *modes;
> +	unsigned int bpc;
> +
> +	/**
> +	 * @width_mm: width of the panel's active display area
> +	 * @height_mm: height of the panel's active display area
> +	 */
> +	struct {
> +		unsigned int width_mm;
> +		unsigned int height_mm;
> +	} size;
> +
> +	unsigned long mode_flags;
> +	enum mipi_dsi_pixel_format format;
> +	int (*init)(struct tm_panel *tm);
> +	unsigned int lanes;
> +	bool discharge_on_disable;
> +	bool lp11_before_reset;
> +};
> +
> +struct tm_panel {
> +	struct drm_panel base;
> +	struct mipi_dsi_device *dsi;
> +
> +	const struct panel_desc *desc;
> +
> +	enum drm_panel_orientation orientation;
> +	struct regulator *pp1800;
> +	struct regulator *avee;
> +	struct regulator *avdd;
> +	struct gpio_desc *enable_gpio;
> +
> +	bool prepared;
> +};
> +
> +static int tm_tl121bvms07_00_init(struct tm_panel *tm)
> +{
> +	struct mipi_dsi_multi_context ctx = { .dsi = tm->dsi };
> +	struct mipi_dsi_device *dsi = ctx.dsi;
> +
> +	mipi_dsi_dcs_write_seq_multi(&ctx, 0xff, 0x5a, 0xa5, 0x06);
> +	mipi_dsi_dcs_write_seq_multi(&ctx, 0x3e, 0x62);
> +
> +
> +	mipi_dsi_generic_write_seq(dsi, 0xff, 0x5a, 0xa5, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&ctx, 0x1b, 0x20);
> +	mipi_dsi_dcs_write_seq_multi(&ctx, 0x5d, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&ctx, 0x5e, 0x40);
> +
> +	mipi_dsi_dcs_write_seq_multi(&ctx, 0xff, 0x5a, 0xa5, 0x07);
> +	mipi_dsi_dcs_write_seq_multi(&ctx, 0X29, 0x00);
> +
> +	mipi_dsi_generic_write_seq(dsi, 0xff, 0x5a, 0xa5, 0x00);
> +
> +	mipi_dsi_dcs_write_seq_multi(&ctx, 0X11);
> +
> +	if (ctx.accum_err)
> +		return ctx.accum_err;
> +
> +	msleep(120);
> +
> +	mipi_dsi_dcs_write_seq_multi(&ctx, 0X29);
> +
> +	if (ctx.accum_err)
> +		return ctx.accum_err;
> +
> +	msleep(80);
> +
> +	return 0;
> +};
> +
> +static inline struct tm_panel *to_tm_panel(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct tm_panel, base);
> +}

I'd expect this function to be present right after struct tm_panel
definition.

> +
> +static int tm_panel_enter_sleep_mode(struct tm_panel *tm)
> +{
> +	struct mipi_dsi_device *dsi = tm->dsi;
> +	int ret;
> +
> +	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	ret = mipi_dsi_dcs_set_display_off(dsi);

Use _multi.

> +	if (ret < 0)
> +		return ret;
> +
> +	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int tm_panel_disable(struct drm_panel *panel)
> +{
> +	struct tm_panel *tm = to_tm_panel(panel);
> +	int ret;
> +
> +	ret = tm_panel_enter_sleep_mode(tm);

Inline

> +	if (ret < 0) {
> +		dev_err(panel->dev, "failed to set panel off: %d\n", ret);
> +		return ret;
> +	}
> +
> +	msleep(150);

And here use DSI-specific wrapper.

> +
> +	return 0;
> +}
> +
> +static int tm_panel_unprepare(struct drm_panel *panel)
> +{
> +	struct tm_panel *tm = to_tm_panel(panel);
> +
> +	if (!tm->prepared)
> +		return 0;
> +
> +	if (tm->desc->discharge_on_disable) {
> +		regulator_disable(tm->avee);
> +		regulator_disable(tm->avdd);
> +		usleep_range(5000, 7000);
> +		gpiod_set_value(tm->enable_gpio, 0);
> +		usleep_range(5000, 7000);
> +		regulator_disable(tm->pp1800);
> +	} else {
> +		gpiod_set_value(tm->enable_gpio, 0);
> +		usleep_range(1000, 2000);
> +		regulator_disable(tm->avee);
> +		regulator_disable(tm->avdd);
> +		usleep_range(5000, 7000);
> +		regulator_disable(tm->pp1800);
> +	}
> +
> +	tm->prepared = false;
> +
> +	return 0;
> +}
> +
> +static int tm_panel_prepare(struct drm_panel *panel)
> +{
> +	struct tm_panel *tm = to_tm_panel(panel);
> +	int ret;
> +
> +	if (tm->prepared)
> +		return 0;
> +
> +	ret = regulator_enable(tm->pp1800);
> +	if (ret < 0)
> +		return ret;
> +
> +	usleep_range(6000, 8000);
> +
> +	ret = regulator_enable(tm->avdd);
> +	if (ret < 0)
> +		goto poweroff1v8;
> +	ret = regulator_enable(tm->avee);
> +	if (ret < 0)
> +		goto poweroffavdd;
> +
> +	usleep_range(11000, 12000);
> +
> +	gpiod_set_value(tm->enable_gpio, 1);
> +
> +	if (tm->desc->lp11_before_reset) {
> +		ret = mipi_dsi_dcs_nop(tm->dsi);
> +		if (ret < 0) {
> +			dev_err(&tm->dsi->dev, "Failed to send NOP: %d\n", ret);
> +			goto poweroff;
> +		}
> +		usleep_range(1000, 2000);
> +	}
> +	gpiod_set_value(tm->enable_gpio, 0);
> +	usleep_range(1000, 2000);
> +	gpiod_set_value(tm->enable_gpio, 1);
> +	usleep_range(20000, 21000);
> +
> +	ret = tm->desc->init(tm);
> +	if (ret < 0)
> +		goto poweroff;
> +
> +	tm->prepared = true;
> +	return 0;
> +
> +poweroff:
> +	gpiod_set_value(tm->enable_gpio, 0);
> +	regulator_disable(tm->avee);
> +poweroffavdd:
> +	regulator_disable(tm->avdd);
> +poweroff1v8:
> +	usleep_range(5000, 7000);
> +	regulator_disable(tm->pp1800);
> +
> +	return ret;
> +}
> +
> +static int tm_panel_enable(struct drm_panel *panel)
> +{
> +	msleep(130);
> +	return 0;
> +}
> +
> +static const struct drm_display_mode tm_tl121bvms07_00_default_mode = {
> +	.clock = 264355,
> +	.hdisplay = 1600,
> +	.hsync_start = 1600 + 20,
> +	.hsync_end = 1600 + 20 + 4,
> +	.htotal = 1600 + 20 + 4 + 20,
> +	.vdisplay = 2560,
> +	.vsync_start = 2560 + 82,
> +	.vsync_end = 2560 + 82 + 2,
> +	.vtotal = 2560 + 82 + 2 + 36,
> +};
> +
> +static const struct panel_desc tm_tl121bvms07_00_desc = {
> +	.modes = &tm_tl121bvms07_00_default_mode,
> +	.bpc = 8,
> +	.size = {
> +		.width_mm = 163,
> +		.height_mm = 260,
> +	},
> +	.lanes = 3,
> +	.format = MIPI_DSI_FMT_RGB888,
> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> +		      MIPI_DSI_MODE_LPM,
> +	.init = tm_tl121bvms07_00_init,
> +	.lp11_before_reset = true,
> +};
> +
> +static int tm_panel_get_modes(struct drm_panel *panel,
> +			       struct drm_connector *connector)
> +{
> +	struct tm_panel *tm = to_tm_panel(panel);
> +	const struct drm_display_mode *m = tm->desc->modes;
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(connector->dev, m);
> +	if (!mode) {
> +		dev_err(panel->dev, "failed to add mode %ux%u@%u\n",
> +			m->hdisplay, m->vdisplay, drm_mode_vrefresh(m));
> +		return -ENOMEM;
> +	}

See drm_connector_helper_get_modes_fixed()

> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_set_name(mode);
> +	drm_mode_probed_add(connector, mode);
> +
> +	connector->display_info.width_mm = tm->desc->size.width_mm;
> +	connector->display_info.height_mm = tm->desc->size.height_mm;
> +	connector->display_info.bpc = tm->desc->bpc;
> +	/*
> +	 * TODO: Remove once all drm drivers call
> +	 * drm_connector_set_orientation_from_panel()
> +	 */
> +	drm_connector_set_panel_orientation(connector, tm->orientation);
> +
> +	return 1;
> +}
> +

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists