Skip to content

tdo_panel: bcm27xx-gpu: Add dsi panel driver;#7318

Open
flash-fea wants to merge 3 commits intoraspberrypi:rpi-6.18.yfrom
flash-fea:rpi-6.18.y
Open

tdo_panel: bcm27xx-gpu: Add dsi panel driver;#7318
flash-fea wants to merge 3 commits intoraspberrypi:rpi-6.18.yfrom
flash-fea:rpi-6.18.y

Conversation

@flash-fea
Copy link
Copy Markdown

@flash-fea flash-fea commented Apr 16, 2026 

- 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 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
Copy link
Copy Markdown
Author

flash-fea commented Apr 16, 2026

whether the addition was successful?

@pelwell
Copy link
Copy Markdown
Contributor

pelwell commented Apr 16, 2026

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:

  1. 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.
  2. 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.

kun_liu added 3 commits April 16, 2026 18:01
drm/panel: add TDO DSI V1 panel driver
f932817 
panel-tdo-dsi-v1.c as module to driver tdo panel.

Signed-off-by: kun_liu <kun_liu@shtdo.com>
ARM: dts: overlays: add vc4-kms-dsi-tdo-panel
c8b73f0 
__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
054c466 
Share panel drivers to bcmxxx_defconfig as module to use in all.

Signed-off-by: kun_liu <kun_liu@shtdo.com>
@6by9
Copy link
Copy Markdown
Contributor

6by9 commented Apr 16, 2026

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
Copy link
Copy Markdown
Author

flash-fea commented Apr 16, 2026

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

pelwell
pelwell reviewed Apr 16, 2026
View reviewed changes
Comment thread arch/arm/boot/dts/overlays/vc4-kms-dsi-tdo-panel-overlay.dts
@@ -0,0 +1,69 @@
/*
* Device Tree overlay for Waveshare DSI Touchscreens
Copy link
Copy Markdown
Contributor

@pelwell pelwell Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs updating

Comment thread arch/arm/boot/dts/overlays/vc4-kms-dsi-tdo-panel-overlay.dts
compatible = "tdo,4.0-dsi-tl040wvs17";
status = "okay";
reg = <0>;
reset-gpios = <&gpio 47 1>; // Dummy GPIO , Unused or change
Copy link
Copy Markdown
Contributor

@pelwell pelwell Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete, as 6by9 says

Comment thread drivers/gpu/drm/panel/panel-tdo-dsi-v1.c
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,
Copy link
Copy Markdown
Contributor

@pelwell pelwell Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No user needs to see this - dev_dbg or delete.

Comment thread drivers/gpu/drm/panel/panel-tdo-dsi-v1.c
struct tdo_panel *ctx;
int ret;

dev_info(&dsi->dev, "dsi panel: %s\n",
Copy link
Copy Markdown
Contributor

@pelwell pelwell Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread drivers/gpu/drm/panel/panel-tdo-dsi-v1.c
struct tdo_panel *ctx = mipi_dsi_get_drvdata(dsi);

if (ctx->reset) {
dev_info(&dsi->dev, "shutdown\n");
Copy link
Copy Markdown
Contributor

@pelwell pelwell Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You really don't need this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pelwell pelwell pelwell left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@flash-fea @pelwell @6by9