Add driver for the Waveshare DSI-TOUCH series panels. #6566

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!

drivers/regulator/waveshare-panel-regulator.c

drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c

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

drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c

+		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

drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c

+	.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

arch/arm/configs/bcm2711_defconfig

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

drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c

+	.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

drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c

+	  &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

drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c

+	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

drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c

+	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

drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c

+	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

drivers/regulator/waveshare-panel-regulator.c

+
+	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

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

drivers/regulator/waveshare-panel-regulator.c

+
+	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

arch/arm/boot/dts/overlays/vc4-kms-dsi-waveshare-panel-v2-overlay.dts

+				#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

drivers/regulator/waveshare-panel-regulator.c

+			 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

drivers/regulator/waveshare-panel-regulator.c

+}
+
+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.

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>

pelwell requested changes Dec 28, 2024

View reviewed changes

drivers/regulator/waveshare-panel-regulator.c Outdated Show resolved Hide resolved

drivers/regulator/waveshare-panel-regulator.c Outdated Show resolved Hide resolved

drivers/gpu/drm/panel/panel-waveshare-dsi-v2.c Outdated Show resolved Hide resolved

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

Open

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_Team added 4 commits March 13, 2025 18:33

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

e4dd716

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…

73141f2

…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

f2014b2

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 c374327 to f2014b2 Compare March 13, 2025 10:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add driver for the Waveshare DSI-TOUCH series panels. #6566

Add driver for the Waveshare DSI-TOUCH series panels. #6566

waveshare commented Dec 28, 2024

pelwell commented Dec 28, 2024

waveshare commented Dec 30, 2024

6by9 left a comment

6by9 Jan 1, 2025

waveshare Feb 28, 2025

6by9 Mar 3, 2025

waveshare Mar 7, 2025

6by9 Feb 24, 2025

waveshare Feb 28, 2025

6by9 Feb 24, 2025

waveshare Feb 28, 2025 •

edited

Loading

6by9 Feb 24, 2025

pelwell Feb 24, 2025

waveshare Feb 28, 2025

6by9 left a comment

6by9 Mar 11, 2025

waveshare Mar 12, 2025

6by9 Mar 11, 2025

waveshare Mar 12, 2025

6by9 Mar 11, 2025

waveshare Mar 12, 2025

6by9 Mar 11, 2025

waveshare Mar 12, 2025

6by9 Mar 11, 2025

waveshare Mar 12, 2025

6by9 Mar 11, 2025

waveshare Mar 12, 2025

6by9 Mar 12, 2025

6by9 Mar 11, 2025

waveshare Mar 12, 2025

6by9 Mar 12, 2025

6by9 Mar 11, 2025

waveshare Mar 13, 2025

	{ .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;

Add driver for the Waveshare DSI-TOUCH series panels. #6566

Are you sure you want to change the base?

Add driver for the Waveshare DSI-TOUCH series panels. #6566

Conversation

waveshare commented Dec 28, 2024

pelwell commented Dec 28, 2024

waveshare commented Dec 30, 2024

6by9 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waveshare Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

6by9 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waveshare Feb 28, 2025 •

edited

Loading