[PATCH 0/3] drm_panel_follower support for RMI4

[PATCH 0/3] drm_panel_follower support for RMI4

[Konrad Dybcio]

RMI4 is often implemented on in-cell panels which require tight power
seqeuncing. The kernel way to approach that is the panel_follower API.

This series adds that in the RMI4 driver and wires it up on a
Snapdragon 845-based Sony Xperia XZ2, also resolving the DT warnings
stemming from the current hack.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Konrad Dybcio (3):
      dt-bindings: input: syna,rmi4: Allow 'panel' property
      Input: synaptics-rmi4: Support drm_panel_follower
      arm64: dts: qcom: sdm845-tama: Declare touchscreen as a panel follower

 .../devicetree/bindings/input/syna,rmi4.yaml       |   4 +
 .../boot/dts/qcom/sdm845-sony-xperia-tama.dtsi     |   8 +-
 drivers/input/rmi4/Kconfig                         |   1 +
 drivers/input/rmi4/rmi_i2c.c                       | 170 ++++++++++++++++++---
 4 files changed, 153 insertions(+), 30 deletions(-)
---
base-commit: 944a035ecca915ae947905dcfb03f2b9dc6d032c
change-id: 20260911-topic-rmi4_panel_follower-6a83fb0009a6

Best regards,
--  
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

[PATCH 1/3] dt-bindings: input: syna,rmi4: Allow 'panel' property

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Many RMI4 implementations are present in in-cell panels where the
precise power seqeuencing is paramount to both the display and the
touch parts to function properly.

Allow binding to a panel node, which is expected to handle that.

Assisted-by: LLM
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/input/syna,rmi4.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
index fb4804ac3544..08a1786d3c8a 100644
--- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
+++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
@@ -57,6 +57,10 @@ properties:
   vdd-supply: true
   vio-supply: true
 
+  panel:
+    description: Reference to the panel this touchscreen follows for power sequencing.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
   rmi4-f01@1:
     type: object
     additionalProperties: false

-- 
2.55.0

[PATCH 2/3] Input: synaptics-rmi4: Support drm_panel_follower

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Many RMI4 implementations are present on in-cell panels with very
tight power sequencing requirements, normally handled by the DRM panel
driver in Linux.

If that doesn't take place first, the integrated touchscreen IC refuses
to work properly.

Add support for drm_panel_follower to take care of that dependency.

Assisted-by: LLM
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/input/rmi4/Kconfig   |   1 +
 drivers/input/rmi4/rmi_i2c.c | 170 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 147 insertions(+), 24 deletions(-)

diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index 5db58fc9e11b..15f7534d6626 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -16,6 +16,7 @@ if RMI4_CORE
 config RMI4_I2C
 	tristate "RMI4 I2C Support"
 	depends on I2C
+	depends on DRM || !DRM
 	help
 	  Say Y here if you want to support RMI4 devices connected to an I2C
 	  bus.
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index e11d0acb9b96..84877030aefe 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -9,6 +9,7 @@
 #include <linux/of.h>
 #include <linux/delay.h>
 #include <linux/regulator/consumer.h>
+#include <drm/drm_panel.h>
 #include "rmi_driver.h"
 
 #define BUFFER_SIZE_INCREMENT 32
@@ -40,6 +41,9 @@ struct rmi_i2c_xport {
 
 	struct regulator_bulk_data supplies[2];
 	u32 startup_delay;
+	struct drm_panel_follower panel_follower;
+	bool powered;
+	bool transport_registered;
 };
 
 #define RMI_PAGE_SELECT_REGISTER 0xff
@@ -187,17 +191,99 @@ static void rmi_i2c_regulator_bulk_disable(void *data)
 {
 	struct rmi_i2c_xport *rmi_i2c = data;
 
+	guard(mutex)(&rmi_i2c->page_mutex);
+
+	if (!rmi_i2c->powered)
+		return;
+
 	regulator_bulk_disable(ARRAY_SIZE(rmi_i2c->supplies),
 			       rmi_i2c->supplies);
+	rmi_i2c->page = -1;
+	rmi_i2c->powered = false;
 }
 
 static void rmi_i2c_unregister_transport(void *data)
 {
 	struct rmi_i2c_xport *rmi_i2c = data;
 
+	if (!rmi_i2c->transport_registered)
+		return;
+
 	rmi_unregister_transport_device(&rmi_i2c->xport);
+	rmi_i2c->transport_registered = false;
 }
 
+static int rmi_i2c_panel_prepared(struct drm_panel_follower *follower)
+{
+	struct rmi_i2c_xport *rmi_i2c = container_of(follower,
+						    struct rmi_i2c_xport,
+						    panel_follower);
+	int error;
+
+	error = regulator_bulk_enable(ARRAY_SIZE(rmi_i2c->supplies),
+				      rmi_i2c->supplies);
+	if (error)
+		return error;
+	rmi_i2c->powered = true;
+
+	msleep(rmi_i2c->startup_delay);
+
+	/* The page-select register is reset when the sensor loses power. */
+	mutex_lock(&rmi_i2c->page_mutex);
+	error = rmi_set_page(rmi_i2c, 0);
+	mutex_unlock(&rmi_i2c->page_mutex);
+	if (error)
+		goto err_power_off;
+
+	if (rmi_i2c->transport_registered) {
+		error = rmi_driver_resume(rmi_i2c->xport.rmi_dev, false);
+		if (error) {
+			dev_warn(&rmi_i2c->client->dev,
+				 "Failed to resume device: %d\n", error);
+			rmi_driver_suspend(rmi_i2c->xport.rmi_dev, false);
+			rmi_i2c_regulator_bulk_disable(rmi_i2c);
+		}
+		return error;
+	}
+
+	dev_info(&rmi_i2c->client->dev,
+		 "registering I2C-connected sensor\n");
+
+	error = rmi_register_transport_device(&rmi_i2c->xport);
+	if (error)
+		goto err_power_off;
+
+	rmi_i2c->transport_registered = true;
+	return 0;
+
+err_power_off:
+	rmi_i2c_regulator_bulk_disable(rmi_i2c);
+	return error;
+}
+
+static int rmi_i2c_panel_unpreparing(struct drm_panel_follower *follower)
+{
+	struct rmi_i2c_xport *rmi_i2c = container_of(follower,
+						    struct rmi_i2c_xport,
+						    panel_follower);
+	int error = 0;
+
+	if (rmi_i2c->transport_registered) {
+		error = rmi_driver_suspend(rmi_i2c->xport.rmi_dev, false);
+		if (error)
+			dev_warn(&rmi_i2c->client->dev,
+				 "Failed to suspend device: %d\n", error);
+	}
+	rmi_i2c_regulator_bulk_disable(rmi_i2c);
+
+	return error;
+}
+
+static const struct drm_panel_follower_funcs rmi_i2c_panel_follower_funcs = {
+	.panel_prepared = rmi_i2c_panel_prepared,
+	.panel_unpreparing = rmi_i2c_panel_unpreparing,
+};
+
 static int rmi_i2c_probe(struct i2c_client *client)
 {
 	struct rmi_device_platform_data *pdata;
@@ -235,10 +321,12 @@ static int rmi_i2c_probe(struct i2c_client *client)
 	if (error < 0)
 		return error;
 
-	error = regulator_bulk_enable(ARRAY_SIZE(rmi_i2c->supplies),
-				       rmi_i2c->supplies);
-	if (error < 0)
-		return error;
+	of_property_read_u32(client->dev.of_node, "syna,startup-delay-ms",
+			     &rmi_i2c->startup_delay);
+
+	rmi_i2c->client = client;
+	mutex_init(&rmi_i2c->page_mutex);
+	rmi_i2c->page = -1;
 
 	error = devm_add_action_or_reset(&client->dev,
 					  rmi_i2c_regulator_bulk_disable,
@@ -246,27 +334,53 @@ static int rmi_i2c_probe(struct i2c_client *client)
 	if (error)
 		return error;
 
-	of_property_read_u32(client->dev.of_node, "syna,startup-delay-ms",
-			     &rmi_i2c->startup_delay);
-
-	msleep(rmi_i2c->startup_delay);
-
-	rmi_i2c->client = client;
-	mutex_init(&rmi_i2c->page_mutex);
-
 	rmi_i2c->xport.dev = &client->dev;
 	rmi_i2c->xport.proto_name = "i2c";
 	rmi_i2c->xport.ops = &rmi_i2c_ops;
 
 	i2c_set_clientdata(client, rmi_i2c);
 
+	error = devm_add_action_or_reset(&client->dev,
+					  rmi_i2c_unregister_transport,
+					  rmi_i2c);
+	if (error)
+		return error;
+
+	if (drm_is_panel_follower(&client->dev)) {
+		rmi_i2c->panel_follower.funcs = &rmi_i2c_panel_follower_funcs;
+
+		if (device_can_wakeup(&client->dev)) {
+			dev_warn(&client->dev,
+				 "Can't wakeup if following panel\n");
+			device_set_wakeup_capable(&client->dev, false);
+		}
+
+		error = devm_drm_panel_add_follower(&client->dev,
+						     &rmi_i2c->panel_follower);
+		if (error)
+			return error;
+
+		return 0;
+	}
+
+	error = regulator_bulk_enable(ARRAY_SIZE(rmi_i2c->supplies),
+				      rmi_i2c->supplies);
+	if (error)
+		return error;
+	rmi_i2c->powered = true;
+
+	msleep(rmi_i2c->startup_delay);
+
 	/*
 	 * Setting the page to zero will (a) make sure the PSR is in a
 	 * known state, and (b) make sure we can talk to the device.
 	 */
+	mutex_lock(&rmi_i2c->page_mutex);
 	error = rmi_set_page(rmi_i2c, 0);
+	mutex_unlock(&rmi_i2c->page_mutex);
 	if (error) {
 		dev_err(&client->dev, "Failed to set page select to 0\n");
+		rmi_i2c_regulator_bulk_disable(rmi_i2c);
 		return error;
 	}
 
@@ -275,14 +389,10 @@ static int rmi_i2c_probe(struct i2c_client *client)
 	error = rmi_register_transport_device(&rmi_i2c->xport);
 	if (error) {
 		dev_err(&client->dev, "failed to register sensor: %d\n", error);
+		rmi_i2c_regulator_bulk_disable(rmi_i2c);
 		return error;
 	}
-
-	error = devm_add_action_or_reset(&client->dev,
-					  rmi_i2c_unregister_transport,
-					  rmi_i2c);
-	if (error)
-		return error;
+	rmi_i2c->transport_registered = true;
 
 	return 0;
 }
@@ -293,12 +403,14 @@ static int rmi_i2c_suspend(struct device *dev)
 	struct rmi_i2c_xport *rmi_i2c = i2c_get_clientdata(client);
 	int ret;
 
+	if (rmi_i2c->panel_follower.panel)
+		return 0;
+
 	ret = rmi_driver_suspend(rmi_i2c->xport.rmi_dev, true);
 	if (ret)
 		dev_warn(dev, "Failed to resume device: %d\n", ret);
 
-	regulator_bulk_disable(ARRAY_SIZE(rmi_i2c->supplies),
-			       rmi_i2c->supplies);
+	rmi_i2c_regulator_bulk_disable(rmi_i2c);
 
 	return ret;
 }
@@ -309,10 +421,14 @@ static int rmi_i2c_resume(struct device *dev)
 	struct rmi_i2c_xport *rmi_i2c = i2c_get_clientdata(client);
 	int ret;
 
+	if (rmi_i2c->panel_follower.panel)
+		return 0;
+
 	ret = regulator_bulk_enable(ARRAY_SIZE(rmi_i2c->supplies),
-				    rmi_i2c->supplies);
+				   rmi_i2c->supplies);
 	if (ret)
 		return ret;
+	rmi_i2c->powered = true;
 
 	msleep(rmi_i2c->startup_delay);
 
@@ -329,12 +445,14 @@ static int rmi_i2c_runtime_suspend(struct device *dev)
 	struct rmi_i2c_xport *rmi_i2c = i2c_get_clientdata(client);
 	int ret;
 
+	if (rmi_i2c->panel_follower.panel)
+		return 0;
+
 	ret = rmi_driver_suspend(rmi_i2c->xport.rmi_dev, false);
 	if (ret)
 		dev_warn(dev, "Failed to resume device: %d\n", ret);
 
-	regulator_bulk_disable(ARRAY_SIZE(rmi_i2c->supplies),
-			       rmi_i2c->supplies);
+	rmi_i2c_regulator_bulk_disable(rmi_i2c);
 
 	return 0;
 }
@@ -345,10 +463,14 @@ static int rmi_i2c_runtime_resume(struct device *dev)
 	struct rmi_i2c_xport *rmi_i2c = i2c_get_clientdata(client);
 	int ret;
 
+	if (rmi_i2c->panel_follower.panel)
+		return 0;
+
 	ret = regulator_bulk_enable(ARRAY_SIZE(rmi_i2c->supplies),
-				    rmi_i2c->supplies);
+				   rmi_i2c->supplies);
 	if (ret)
 		return ret;
+	rmi_i2c->powered = true;
 
 	msleep(rmi_i2c->startup_delay);
 

-- 
2.55.0

[PATCH 3/3] arm64: dts: qcom: sdm845-tama: Declare touchscreen as a panel follower

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Currently, the touchscreen relies on the idiosyncracies of Linux and
fw_devlink, wired up via a totally fake supply reference to keep the
driver probe order in check.

Resolve that properly by declaring the touchscreen as a panel follower
of the in-cell display panel.

Assisted-by: LLM
Fixes: 3ab8216de2fa ("arm64: dts: qcom: sdm845-tama: Add Synaptics Touchscreen")
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
index 05e5a3674e5f..dd79b93befb4 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
@@ -442,12 +442,8 @@ touchscreen: touchscreen@2c {
 
 		interrupts-extended = <&tlmm 125 IRQ_TYPE_EDGE_FALLING>;
 		vdd-supply = <&vreg_l14a_1p8>;
-		/*
-		 * This is a blatant abuse of OF, but the panel driver *needs*
-		 * to probe first, as the power/gpio switching needs to be precisely
-		 * timed in order for both the display and touch panel to function properly.
-		 */
-		incell-supply = <&panel>;
+
+		panel = <&panel>;
 
 		syna,reset-delay-ms = <220>;
 		syna,startup-delay-ms = <1000>;

-- 
2.55.0

Re: [PATCH 2/3] Input: synaptics-rmi4: Support drm_panel_follower

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent I2C accesses can wedge the hardware during panel power-on because `page_mutex` is not held during the startup delay.
--

commit 31e9aa4b11936c658845014c2ef2567c8d7947f3
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Input: synaptics-rmi4: Support drm_panel_follower

Many RMI4 implementations are present on in-cell panels with very tight power
sequencing requirements, normally handled by the DRM panel driver in Linux.
This commit adds support for drm_panel_follower to handle that dependency.

> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
> index e11d0acb9b96d..84877030aefe9 100644
> --- a/drivers/input/rmi4/rmi_i2c.c
> +++ b/drivers/input/rmi4/rmi_i2c.c
[ ... ]
> @@ -187,17 +191,99 @@ static void rmi_i2c_regulator_bulk_disable(void *data)
[ ... ]
> +static int rmi_i2c_panel_prepared(struct drm_panel_follower *follower)
> +{
> +	struct rmi_i2c_xport *rmi_i2c = container_of(follower,
> +						    struct rmi_i2c_xport,
> +						    panel_follower);
> +	int error;
> +
> +	error = regulator_bulk_enable(ARRAY_SIZE(rmi_i2c->supplies),
> +				      rmi_i2c->supplies);
> +	if (error)
> +		return error;
> +	rmi_i2c->powered = true;
> +
> +	msleep(rmi_i2c->startup_delay);
> +
> +	/* The page-select register is reset when the sensor loses power. */
> +	mutex_lock(&rmi_i2c->page_mutex);

[Severity: High]
Can concurrent I2C accesses wedge the hardware during panel power-on because
page_mutex is not held during this startup delay?

The drm_panel_follower subsystem calls rmi_i2c_panel_prepared()
asynchronously when the display panel turns on. Since userspace is not
frozen during this event, a process reading a world-readable RMI sysfs
attribute (such as update_status, which calls
rmi_f34v7_check_command_status() then rmi_read_block()) or interacting
with /dev/rmi0 can invoke rmi_i2c_read_block().

Because rmi_i2c->powered is set to true before the msleep(), and page_mutex
is not held during the delay, a concurrent rmi_i2c_read_block() can acquire
page_mutex and issue an i2c_transfer() to the sensor while it is still in
the middle of its hardware boot sequence.

Does this early I2C access violate the strict timing requirements and leave
the touchscreen completely unresponsive until a reboot or driver reload?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-topic-rmi4_panel_follower-v1-0-c1b2cf1cdb62@oss.qualcomm.com?part=2