Add driver for the Waveshare DSI-TOUCH series panels. by waveshare · Pull Request #6566 · raspberrypi/linux

waveshare · 2024-12-28T07:38:20Z

Hello, everyone:
This is the driver for the DSI-TOUCH series of panels to be released by Waveshare.
Regarding the missing help warning, it should be caused by the length of the help description being less than 4, so i think it allowed to ignore things.

If you have any other suggestions we'd be happy to make changes!

pelwell · 2024-12-28T12:20:35Z

Check the auto-build errors. And as a wider question, does this warrant a new driver? Are the devices so different from the v1s?

waveshare · 2024-12-30T03:48:08Z

Yes, I think a new driver needs to be developed. v2 versions have significant architectural changes compared to the v1 series.

v1 series uses bridge chips to convert DSI to other display protocols
v2 series utilizes a 2-lane/4-lane mipi DSI direct-drive architecture that completely eliminates the need for a bridge chip.
The two have significant differences in hardware architecture, communication protocols, initialization sequences, and scalability
We believe that the v2 version will be more cost-effective, maintainable and scalable, and will bring a better experience and continued development to the Raspberry Pi DSI display series.

btw，The previous check auto-build errors has been resolved

6by9

Sorry, I had a couple of these comments pending when I thought I'd submitted them.

Other than possibly EMC, seeing as both the 2 and 4 lane modes achieve the same refresh rate, is there a benefit in providing options for both?

6by9 · 2025-01-01T12:03:06Z

+		return ret;
+	}
+
+	ctx->panel.prepare_prev_first = true;


Duplicate of line 1205.

Do I need to push it again to solve this problem? Avoid repeated reviews, or do I directly push the modified parts?

Update the patches on your local repo (git commit --amend and git rebase -i ) to make a consistent set of what you want to merge, and then git push -f ... to push to the same waveshare DSI-LCD branch on Github. That automatically updates the PR, and we'll review the changes.

The issues raised have been fixed , PTAL.

6by9 · 2025-02-24T19:08:48Z

+	.height_mm = 172,
+};
+
+static const struct drm_display_mode ws_panel_8_a_2lane_mode = {


This mode is identical to ws_panel_8_a_mode, so you could remove the duplication.

And the same as above, we will modify it

6by9 · 2025-02-24T19:24:52Z

+	.htotal = 800 + 40 + 20 + 20,
+	.vdisplay = 1280,
+	.vsync_start = 1280 + 30,
+	.vsync_end = 1280 + 30 + 10,


Is there a need to vary the vertical front porch and sync width between the 2 and 4 lane modes? Both appear to work as well as each other in 2 lane mode (haven't tested 4) when using kmstest --flip -r 70000000,800/40/20/20/+,1280/30/10/4/+ and kmstest --flip -r 70000000,800/40/20/20/+,1280/20/20/4/+.

Both seem to work normally, we will unify them

Suggested change

.vsync_end = 1280 + 30 + 10,

.vsync_end = 1280 + 20+ 10,

6by9 · 2025-02-24T19:27:53Z

+	  &ws_panel_10_1_inch_a_4lane_desc },
+	{ .compatible = "waveshare,10.1-dsi-touch-a",
+	  &ws_panel_10_1_inch_a_desc },
+	{ .compatible = "waveshare,8.0-dsi-touch-a,4lane",


I don't believe commas are permitted within compatibles other than separating the vendor and product name. @pelwell Could you confirm?

Ditto for waveshare,10.1-dsi-touch-a,4lane

Yes. The current version of the spec says:

The recommended format is "manufacturer,model", where manufacturer is a string describing the name
of the manufacturer (such as a stock ticker symbol), and model specifies the model number.
The compatible string should consist only of lowercase letters, digits and dashes, and should start with a letter.
A single comma is typically only used following a vendor prefix. Underscores should not be used.

Thank you for the reminder, we will change it

Suggested change

{ .compatible = "waveshare,8.0-dsi-touch-a,4lane",

{ .compatible = "waveshare,8.0-dsi-touch-a-4lane",

6by9

A few comments, however as this driver isn't going to be upstreamed in the current form, I don't view any of them as critical to be resolved.

If you want to implement any of the changes then please respond in the next 48 hours to say so, otherwise I'm happy for this to be merged.

6by9 · 2025-03-11T13:17:03Z

+	int ret;
+
+	/* And reset it */
+	if (ctx->reset != NULL) {


Can be simplified to if (ctx->reset) {
This occurs in number of places.

That's a good suggestion, we'll deal with it.

6by9 · 2025-03-11T13:18:26Z

+	return 0;
+}
+
+static int ws_panel_enable(struct drm_panel *panel)


All the panel functions are optional, so you can just omit these.

OK, we'll delete this part.

6by9 · 2025-03-11T13:22:09Z

+	struct ws_panel *ctx = panel_to_ws(panel);
+
+	mipi_dsi_dcs_set_display_off(ctx->dsi);
+	mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);


Slight mismatch with these two as mipi_dsi_dcs_set_display_on and mipi_dsi_dcs_exit_sleep_mode aren't called in prepare

Initialized to add display and postpone sleep instructions. Because some screens need to add other instructions to the mipi_dsi_dcs_set_display_on (0x29) and mipi_dsi_dcs_exit_sleep_mode (0x11) , there is no direct function call.

6by9 · 2025-03-11T13:27:47Z

+
+	state->direction_state &= ~(1 << offset);
+
+	last_val = state->poweron_state;


state->poweron_state is being accessed here without the mutex being claimed.

Is the mutex needed to protect state->direction_state? You are doing a read/modify/write with the &=, and |= in waveshare_panel_gpio_direction_in

OK, we'll add a mutex

So you now read state->poweron_state without the mutex held, and only take the mutex to write the value?! You need to take the mutex to make the entire read/modify/write operation atomic.

6by9 · 2025-03-11T13:29:37Z

+
+	last_val = state->poweron_state;
+	if (brightness)
+		last_val |= (1 << 2); // Enable BL_EN


Bit 2 is going to be advertised as a GPIO that another driver could request, however you are modifying it internally from the driver. You have the potential for issues there if others make use of this driver from their own overlays.

OK, we will use gpiod_set_value_cansleep to modify the BL_EN hardware instead of writing the registers directly

6by9 · 2025-03-11T13:34:27Z

+				#gpio-cells = <2>;
+			};
+
+			touch: goodix@5d {


Seeing as we have no control over the interrupt line, I assume it is pulled down so that the touch controller will reliably come out of reset on address 0x5d. (Using the INT line to determine I2C address is a right pain with these Goodix controllers).

The muc is connected to the INT pin. If necessary, we can set the INT pin level when exiting the reset through the MCU. By default, the I2C address of Goodix controllers is 0x5d

As long as you have it in hand.

Avoiding a repeat of #6538 would be good, and I've been in exactly the same position with an early version of the Raspberry Pi Touch Display 2.
The vc4-dpi-hyperpixel4 overlay hacks around the issue by defining the touch controller on both addresses, and accepts that one will fail to probe.

6by9 · 2025-03-11T13:36:38Z

+			 data);
+
+	state->direction_state = 0;
+	state->poweron_state = (1 << 9) | (1 << 8) | (1 << 4) |


There's a general preference to use the BIT(n) macro to denote bit-usage such as these. It could probably replace all your 1 << n uses in the driver.

Ok, we'll use BIT(n) replace, maybe we're a bit old-school，lol

I would often go for the 1<<n style too, but the Linux coding style can be a bit fussy and I'm now used to reviewing based on that.

6by9 · 2025-03-11T13:38:33Z

+}
+
+static int waveshare_panel_gpio_direction_out(struct gpio_chip *gc,
+					      unsigned int offset, int val)


Do you need to be able to control the direction with the current panels? If not, then keep the driver simple for now, and add direction control later when it is needed.

It can control the direction of some GPIOs, such as the INT pin mentioned above, which we hope to keep. If necessary, we can detect the level of some pins.

6by9

Sorry for the delay in reviewing.

Other than this one issue, I'm happy.

6by9 · 2025-04-03T14:02:14Z

+
+	state->direction_state &= ~(1 << offset);
+
+	last_val = state->poweron_state;


So you now read state->poweron_state without the mutex held, and only take the mutex to write the value?! You need to take the mutex to make the entire read/modify/write operation atomic.

pelwell · 2025-04-03T15:17:21Z

Since this PR was opened we've gained another defconfig - arch/arm64/configs/bcm2711_rt_defconfig. Please include it in the set of defconfigs updated by these patches.

waveshare · 2025-04-07T11:28:41Z

Since this PR was opened we've gained another defconfig - arch/arm64/configs/bcm2711_rt_defconfig. Please include it in the set of defconfigs updated by these patches.

We have just returned from the Qingming Festival holiday and will supplement the defconfig and resubmit shortly. Additionally, I inadvertently clicked the reviewers button earlier - please disregard that action.

The regulator of the Waveshare DSI-TOUCH series panels is different. Add a new driver for this regulator. Signed-off-by: Waveshare_Team <support@waveshare.com>

…ries panels. the driver are provided for the Waveshare DSI-TOUCH series panels, modelled after the other Ilitek controller drivers, but not limited to Ilitek series controllers. The aim is to offer a more consistent operation and experience for the Waveshare DSI-TOUCH series panels. Signed-off-by: Waveshare_Team <support@waveshare.com>

…vice Tree support Add the device tree for the Waveshare DSI-TOUCH series panels. Signed-off-by: Waveshare_Team <support@waveshare.com>

Enable the Waveshare DSI-TOUCH series panel driver in the defconfig files that support the Pi5/Pi4/Pi3 family. Signed-off-by: Waveshare_Team <support@waveshare.com>

waveshare · 2025-04-08T14:34:38Z

@pelwell As requested:
We confirm arch/arm64/configs/bcm2711_rt_defconfig only exists in the rpi-6.12.y branch. We've therefore:

Redirected this PR's patches to target rpi-6.12.y
Force-pushed the updated series with commit [916bd95]

waveshare · 2025-04-28T08:36:48Z

To be reviewed.
Please take a look. @pelwell
If there are any questions, we'd be happy to continue adjusting.

pelwell · 2025-04-28T08:51:39Z

No, I think we're good here.

kernel: Add driver for the Waveshare DSI-TOUCH series panels See: raspberrypi/linux#6566 kernel: Add support for TI ADS7828/ADS7830 I2C ADCs See: raspberrypi/linux#6819 kernel: Add RS485 mode support for UART2, UART3, UART4, and UART5 See: raspberrypi/linux#6820

pelwell requested changes Dec 28, 2024

View reviewed changes

Comment thread drivers/regulator/waveshare-panel-regulator.c Outdated

Comment thread drivers/regulator/waveshare-panel-regulator.c Outdated

Comment thread drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c Outdated

waveshare force-pushed the DSI-LCD branch from 91228a4 to 47a0e52 Compare December 28, 2024 10:19

pelwell mentioned this pull request Dec 28, 2024

Waveshare 5inch Capacitive DSI Touchscreen - Touch not always working on boot #6538

Closed

waveshare force-pushed the DSI-LCD branch 2 times, most recently from aeec1c0 to 086de2c Compare December 30, 2024 03:47

6by9 force-pushed the DSI-LCD branch from 086de2c to c894c3e Compare February 24, 2025 17:55

6by9 reviewed Feb 24, 2025

View reviewed changes

waveshare force-pushed the DSI-LCD branch from c894c3e to c374327 Compare March 7, 2025 02:53

6by9 approved these changes Mar 11, 2025

View reviewed changes

waveshare force-pushed the DSI-LCD branch from c374327 to f2014b2 Compare March 13, 2025 10:42

6by9 reviewed Apr 3, 2025

View reviewed changes

waveshare requested a review from pelwell April 7, 2025 11:23

Waveshare_Team added 4 commits April 8, 2025 20:04

drivers/regulator : Add a regulator to Waveshare DSI-TOUCH series panels

7218dbf

The regulator of the Waveshare DSI-TOUCH series panels is different. Add a new driver for this regulator. Signed-off-by: Waveshare_Team <support@waveshare.com>

arch/arm/boot/dts/overlays : Add Waveshare DSI-TOUCH series panels De…

a54d4e0

…vice Tree support Add the device tree for the Waveshare DSI-TOUCH series panels. Signed-off-by: Waveshare_Team <support@waveshare.com>

configs : Enable configs of new series of Waveshare DSI panels

916bd95

Enable the Waveshare DSI-TOUCH series panel driver in the defconfig files that support the Pi5/Pi4/Pi3 family. Signed-off-by: Waveshare_Team <support@waveshare.com>

waveshare force-pushed the DSI-LCD branch from f2014b2 to 916bd95 Compare April 8, 2025 13:55

waveshare changed the base branch from rpi-6.6.y to rpi-6.12.y April 8, 2025 13:55

KonstaT mentioned this pull request Apr 17, 2025

Waveshare v2 panel support raspberry-vanilla/android_kernel_brcm_rpi#4

Closed

pelwell merged commit 57bc100 into raspberrypi:rpi-6.12.y Apr 28, 2025
24 of 26 checks passed

waveshare deleted the DSI-LCD branch July 14, 2025 06:15

	{ .compatible = "waveshare,8.0-dsi-touch-a,4lane",
	{ .compatible = "waveshare,8.0-dsi-touch-a-4lane",


		state->direction_state &= ~(1 << offset);

		last_val = state->poweron_state;

Conversation

waveshare commented Dec 28, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pelwell commented Dec 28, 2024

Uh oh!

waveshare commented Dec 30, 2024

Uh oh!

6by9 left a comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waveshare Feb 28, 2025 • 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!

6by9 left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

6by9 left a comment

waveshare Feb 28, 2025 •

edited

Loading