tdo_panel: bcm27xx-gpu: Add dsi panel driver; by flash-fea · Pull Request #7318 · raspberrypi/linux

flash-fea · 2026-04-16T03:21:17Z

- Add vc4-kms-dsi-panel-overlay.dts for tdo panel compatiable.
- Simplify add panel-tdo-dsi-v1.c as module to driver tdo panel.
- __overrides__ parameter can be add in config.txt for customers.

flash-fea · 2026-04-16T08:00:44Z

whether the addition was successful?

pelwell · 2026-04-16T09:04:29Z

The partitioning of the code looks good, and checkpatch is happy.

This isn't my field of expertise so there is limited input I can give regarding the driver itself - perhaps @6by9 has some comments - but there are a few things I have spotted:

The authorship of the code (and therefore the permission to publish it) is a bit vague. The comment at the top of the driver file says Copyright (C) 2024 Waveshare International Limited, but the footer says MODULE_AUTHOR("LK <support@tdo.com>"); and MODULE_DESCRIPTION("TDO DSI panel driver");, none of which seems to match your GitHub user account.
I see a number of dev_err() used for what look like debug output. I do the same myself, but it shouldn't still be around in a production driver. If the information is useful to users, demote it to INFO level or lower, otherwise go for dev_dbg or deletion.

6by9 · 2026-04-16T10:06:25Z

Sorry I had all my review comments pending on the 6.12 version. Now posted.
I suspect that they all apply here too.

Largely the same as pelwell has commented with regard pr_err debugging and authorship, but also a couple of other points.
I suspect that you've copied the panel-waveshare-dsi-v2 driver and amended for your panels, but it'd nice to have that confirmed. That may not have been the best base as it isn't a driver that has been reviewed by upstream, but that isn't critical for a Pi specific product.

flash-fea · 2026-04-16T10:10:22Z

yes,some informations need to change,about two dev_err() only use in debug to find by text easyier,but forget to recover.

pelwell · 2026-04-16T10:17:34Z

@@ -0,0 +1,69 @@
+/*
+ * Device Tree overlay for Waveshare DSI Touchscreens


This also needs updating

pelwell · 2026-04-16T10:17:58Z

+				compatible = "tdo,4.0-dsi-tl040wvs17";
+				status = "okay";
+				reg = <0>;
+				reset-gpios = <&gpio 47 1>;   // Dummy GPIO , Unused or change


Delete, as 6by9 says

delete all? reset-gpios maybe use in future,i see these were added also in other dts.

You can add it at a later stage via an override if it is needed.

As it stands the driver will claim a totally unrelated GPIO for no particular reason. GPIO 47 isn't necessarily unused or even present on all devices, so could actually cause issues if left in.

some my device's driver need it, as how i write to overlay in use

For those devices that do require it you can add an additional fragment to configure it which can be enabled by the relevant override.

A similar case is https://github.com/raspberrypi/linux/blob/rpi-6.12.y/arch/arm/boot/dts/overlays/edt-ft5406.dtsi#L11-L16 where we want to add the touchscreen-inverted-x; property on demand, triggered by the invx override

With the driver calling devm_gpiod_get_optional, the driver doesn't need it. It's permitted to call any of the gpiod_ functions with the returned (NULL) handle.

like this?
reset_pin = <&panel>,"reset-gpios:47";
and modify driver for devm_gpiod_get_optional.

Close:

reset_pin = <&panel>,"reset-gpios:0=",<&gpio>, <&panel>,"reset-gpios:4=47", <&panel>,"reset-gpios:8=1";

will create the reset-gpios property if you use the reset_pin parameter, but 6by9 was suggesting that you would build that into a product declaration:

new_panel_needing_reset = <&panel>, "compatible=tdo,new-panel-needing-reset", <&panel>,"reset-gpios:0=",<&gpio>, <&panel>,"reset-gpios:4=47", <&panel>,"reset-gpios:8=1";

But it might be cleaner to have a dormant fragment that gets enabled as needed.

The driver is already using

ctx->reset = devm_gpiod_get_optional(&dsi->dev, "reset", GPIOD_OUT_LOW);

That means that ctx->reset will not return an error if reset-gpios is missing.

Add an additional fragment along the lines of:

fragment@10 { target = <&panel>; reset_gpio_frag: __dormant__ { reset-gpios = <&gpio 47 1>; }; }; __overrides__ { reset_pin = <0>, "+10", <&reset_gpio_frag>, "reset-gpios:4"; };

You can then have dtoverlay=vc4-kms-dsi-tdo-panel,reset_pin=47 to enable the reset gpio and set it to 47 (or whatever other GPIO number you wish to use).

Just looking through the various Pi variants, gpio 47 is used for:

Pi1A/B: SD card detect

Pi1A+, B+, 2B, 3B, Zero, ZeroW: Activity LED

Pi 3B+/A+, CM3, CM4, 02W, CM0 : I2C to SMPS

CM1: EMMC_ENABLE_N

This is why I say it can't be claimed generically by the overlay.
(Pi5 appears to use it for 2712_WAKE)

this reset pin is not decided yet,can you give me a suggest? now dts add this->
+fragment@4 {

target = <&panel>;

reset_gpio_frag: __dormant__ {

reset-gpios = <&gpio 47 1>;

};

};

dsi0 = <&dsi_frag>, "target:0=",<&dsi0>,
<&i2c_frag>, "target:0=",<&i2c_csi_dsi0>;

reset_pin = <0>, "+10",

<&reset_gpio_frag>, "reset-gpios:4";

ctx->reset = devm_gpiod_get_optional(&dsi->dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(ctx->reset))
dev_warn(&dsi->dev, "Couldn't get the reset GPIO\n");

pelwell · 2026-04-16T10:19:15Z

+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(connector->dev, ctx->desc->mode);
+	dev_info(&ctx->dsi->dev, "%ux,%ux,%ux\n", ctx->desc->mode->hdisplay,


No user needs to see this - dev_dbg or delete.

pelwell · 2026-04-16T10:21:38Z

+	struct tdo_panel *ctx;
+	int ret;
+
+	dev_info(&dsi->dev, "dsi panel: %s\n",


It is preferred for successful probing to generate minimal output. Perhaps defer this line to after the number of lanes is available and combine the two?

pelwell · 2026-04-16T10:22:03Z

+	struct tdo_panel *ctx = mipi_dsi_get_drvdata(dsi);
+
+	if (ctx->reset) {
+		dev_info(&dsi->dev, "shutdown\n");


You really don't need this.

struct tdo_panel *ctx = mipi_dsi_get_drvdata(dsi);

gpiod_set_value_cansleep(ctx->reset, 0);

like this,is it ok?

I was referring to the dev_info()

panel-tdo-dsi-v1.c as module to driver tdo panel. Signed-off-by: kun_liu <kun_liu@shtdo.com>

6by9 · 2026-04-16T17:23:51Z

I've just noticed that someone is upstreaming the Waveshare DSI display drivers to mainline at present

V2 displays https://lore.kernel.org/dri-devel/20260413-waveshare-dsi-touch-v3-21-3aeb53022c32@oss.qualcomm.com/

https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/waveshare-dsi.c for the V1 displays is already merged.
https://lore.kernel.org/dri-devel/20260412-ws-lcd-v3-0-db22c2631828@oss.qualcomm.com/T/ is extending that.

At some point I'll update the overlays and delete the old panel-waveshare-dsi and panel-waveshare-dsi-v2 drivers.

There are a couple of tdo panel drivers already in mainline, so if any of them match these new panels then the configuration really should be added to those drivers rather than this new one.
https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c
https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c

flash-fea · 2026-04-17T02:08:09Z

my company need only one driver to use all our mipi panel,it seems to add some after this request finish.

pelwell · 2026-04-17T08:29:18Z

+        rotation                Display rotation {0,90,180,270}
+        dsi0                    Use DSI0 and i2c_csi_dsi0 (rather than
+                                the default DSI1 and i2c_csi_dsi).
+        reset_pin               GPIO pin for RESET (default 25)


This is not true - there is no default value, because you can't enable the reset pin without assigning a value.

pelwell · 2026-04-17T08:30:02Z

+		rotation = <&panel>, "rotation:0";
+		dsi0 = <&dsi_frag>, "target:0=",<&dsi0>,
+		       <&i2c_frag>, "target:0=",<&i2c_csi_dsi0>;
+		reset_pin = <0>, "+10", 


The number after the "+" is the number of the fragment to enable, which in this case should be 4.

pelwell · 2026-04-17T09:02:18Z

+
+	/* And reset it */
+	if (ctx->reset) {
+		gpiod_set_value_cansleep(ctx->reset, 0);


Is this correct? You declare the reset pin to be active low (that's what the 1 in <&gpio 25 1> means), which suggests that the device is held in reset as long as the GPIO is low. When using the gpiod_ API, the value that you set is a logical value, not a physical line level - 1 means assert, 0 means deassert. This sequence will end up driving the GPIO high (deasserted) then low (asserted), which will leave the device in reset.

Have a look at the uses of the reset GPIO, decide in each case whether reset should be asserted or deasserted, then set the values appropriately. The flags value in the overlay then lets you specify the sense of the reset line on a per-board basis.

yes,you say is right,i review it carefulless.the pin not use in our first panel.so just add but not check.

Unless you really feel the need to abuse the gpiod API and confuse the reader, any reset sequence should be assert then deassert. All you've done is invert the sense of the GPIO.

__overrides__ parameter can be add in config.txt for customers. README describe the dts config info. Signed-off-by: kun_liu <kun_liu@shtdo.com>

Share panel drivers to bcmxxx_defconfig as module to use in all. Signed-off-by: kun_liu <kun_liu@shtdo.com>

flash-fea force-pushed the rpi-6.18.y branch 2 times, most recently from 7a2e2f8 to 840f6f9 Compare April 16, 2026 06:19

flash-fea force-pushed the rpi-6.18.y branch from 840f6f9 to 054c466 Compare April 16, 2026 10:03

pelwell reviewed Apr 16, 2026

View reviewed changes

drm/panel: add TDO DSI V1 panel driver

a810202

panel-tdo-dsi-v1.c as module to driver tdo panel. Signed-off-by: kun_liu <kun_liu@shtdo.com>

flash-fea force-pushed the rpi-6.18.y branch from 054c466 to c71f6c8 Compare April 16, 2026 11:41

flash-fea force-pushed the rpi-6.18.y branch from c71f6c8 to 618a653 Compare April 17, 2026 06:10

pelwell reviewed Apr 17, 2026

View reviewed changes

flash-fea force-pushed the rpi-6.18.y branch from 618a653 to 11aee5d Compare April 17, 2026 08:43

pelwell reviewed Apr 17, 2026

View reviewed changes

kun_liu added 2 commits April 17, 2026 17:37

ARM: dts: overlays: add vc4-kms-dsi-tdo-panel

db93d78

__overrides__ parameter can be add in config.txt for customers. README describe the dts config info. Signed-off-by: kun_liu <kun_liu@shtdo.com>

ARM/ARM64: defconfig: enable DRM_PANEL_TDO_DSI_V1

0e8762a

Share panel drivers to bcmxxx_defconfig as module to use in all. Signed-off-by: kun_liu <kun_liu@shtdo.com>

flash-fea force-pushed the rpi-6.18.y branch from 11aee5d to 0e8762a Compare April 17, 2026 09:37

		@@ -0,0 +1,69 @@
		/*
		* Device Tree overlay for Waveshare DSI Touchscreens

Conversation

flash-fea commented Apr 16, 2026

Uh oh!

flash-fea commented Apr 16, 2026

Uh oh!

pelwell commented Apr 16, 2026

Uh oh!

6by9 commented Apr 16, 2026

Uh oh!

flash-fea commented Apr 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pelwell Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

6by9 commented Apr 16, 2026

Uh oh!

flash-fea commented Apr 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pelwell Apr 16, 2026 •

edited

Loading