Skip to contentSkip to conversation

Lenovo Yoga C630 (sdm845) fixes

PATCH In review Ard Biesheuvel 2 participants 9 messages Nov 11

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover Lenovo Yoga C630 (sdm845) fixes

From: Ard Biesheuvel <[email protected]>

I went down a rabbit hole with Claude to finally get my Yoga c630 working properly at EL2 (minus audio).

This series does not cover all of that, but it contains some of the fixes I picked up along the way that are unrelated to EL2 but were needed nonetheless so I am sending those out first.

The EL2 changes themselves may not be as palatable upstream, as they hack around the removal of the QHEE hypervisor using slbounce, so they will be RFC material in any case.

Cc Bjorn Andersson <[email protected]>
Cc Konrad Dybcio <[email protected]>
Cc Abel Vesa <[email protected]>
Cc Dmitry Baryshkov <[email protected]>

Ard Biesheuvel (7):
arm64: dts: qcom: sdm845: Bump fastrpc shared DMA pool to 32MB for 16k
pages
arm64: dts: qcom: c630: Use LN_BB_CLK2 as the eDP bridge reference
clock
arm64: dts: qcom: c630: Enable DisplayPort alt mode
arm64: dts: qcom: c630: Use the idle states from the ACPI tables
arm64: dts: qcom: c630: Wait for the touchpad at resume
platform: arm64: lenovo-yoga-c630: Disable the EC interrupt while
suspended
platform: arm64: lenovo-yoga-c630: Expose the keyboard backlight as an
LED

 arch/arm64/boot/dts/qcom/sdm845.dtsi                 |   4 +-
 arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts |  44 ++++--
 drivers/platform/arm64/Kconfig                       |   4 +-
 drivers/platform/arm64/lenovo-yoga-c630.c            | 155 +++++++++++++++++++-
 4 files changed, 190 insertions(+), 17 deletions(-)

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Signature · 1 line
2.56.0.611.g1030f0b940-goog
#1 lore raw

PATCH 1/7 arm64: dts: qcom: sdm845: Bump fastrpc shared DMA pool to 32MB for 16k pages

From: Ard Biesheuvel <[email protected]>

The CMA driver wants memory pools whose base and size are aligned to the PMD size, which is 32M when running with 16k pages. So increase the shared DMA pool size from 16M to 32M and set its minimum alignment to 32M as well.

---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/qcom/sdm845.dtsi +2 -2
index 844b840bc546..5f554f6b06f7 100644
@@ -894,8 +894,8 @@ mdata_mem: mpss-metadata {
 		fastrpc_mem: fastrpc {
 			compatible = "shared-dma-pool";
 			alloc-ranges = <0x0 0x00000000 0x0 0xffffffff>;
-			alignment = <0x0 0x400000>;
-			size = <0x0 0x1000000>;
+			alignment = <0x0 0x2000000>;
+			size = <0x0 0x2000000>;
 			reusable;
 		};
 	};
Signature · 1 line
2.56.0.611.g1030f0b940-goog
#2 lore raw

PATCH 2/7 arm64: dts: qcom: c630: Use LN_BB_CLK2 as the eDP bridge reference clock

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

The SN65DSI86 eDP bridge on the Yoga C630 is described as being clocked by an always-on fixed 19.2 MHz reference clock. However, as on the SDM845 reference design (Cheza), its REFCLK appears to be fed by the LN_BB_CLK2 output of the PMIC, which also serves as the master clock of the WCD9340 audio codec.

As nothing tells Linux that the bridge depends on LN_BB_CLK2, the clock only runs when the codec happens to have it enabled, and the WCD934x driver only enables it on demand. If the bridge is brought up while LN_BB_CLK2 is off or being toggled, e.g., when the display stack probes before the codec, or while the codec is being initialized, all AUX transfers fail and the DP PLL does not lock, leaving the panel dark:

ti_sn65dsi86 10-002c: Can't read lane count (-110); assuming 4 ti_sn65dsi86 10-002c: Can't read eDP rev (-110), assuming 1.1 ti_sn65dsi86 10-002c: Can't read max rate (-110); assuming 5.4 GHz ti_sn65dsi86 10-002c: DP_PLL_LOCK polling failed (-110)

So describe the actual clock instead, so that the bridge driver keeps it enabled for as long as the bridge is powered up.

Fixes 956e9c85f47b ("arm64: dts: qcom: c630: Define eDP bridge and panel")
---
 arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts +1 -8
index 9d38ae83c979..a4a573ea7e4a 100644
@@ -97,13 +97,6 @@ sw_edp_1p2: edp-1p2-regulator {
 		vin-supply = <&vreg_l2a_1p2>;
 	};
 
-	sn65dsi86_refclk: sn65dsi86-refclk {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-
-		clock-frequency = <19200000>;
-	};
-
 	vph_pwr: regulator-vph-pwr {
 		compatible = "regulator-fixed";
 		regulator-name = "vph_pwr";
@@ -528,7 +521,7 @@ sn65dsi86: bridge@2c {
 		vpll-supply = <&vreg_l14a_1p88>;
 		vccio-supply = <&vreg_l14a_1p88>;
 
-		clocks = <&sn65dsi86_refclk>;
+		clocks = <&rpmhcc RPMH_LN_BB_CLK2>;
 		clock-names = "refclk";
 
 		no-hpd;
Signature · 1 line
2.56.0.611.g1030f0b940-goog
#3 lore raw

PATCH 3/7 arm64: dts: qcom: c630: Enable DisplayPort alt mode

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

The right-hand USB-C port supports DisplayPort alt mode. The EC takes care of entering it, and ucsi_yoga_c630 already sets the orientation of the combo PHY and passes the EC's DP hotplug events down the bridge chain. All that is missing is the DisplayPort controller, which sdm845.dtsi already connects to the combo PHY.

Limit it to two lanes. That is what the PHY provides alongside USB3 in its default mode, and it matches pin assignment D, which the EC picked with the monitor I tested. Four lanes would need the PHY to be switched to its DP only mode, which nothing does on this platform.

Tested with a USB-C monitor: 1920x1200@60 over two lanes at HBR2, with the monitor's USB3 hub working at the same time.

---
 arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 8 ++++++++
 1 file changed, 8 insertions(+)
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts +8 -0
index a4a573ea7e4a..0645ab429177 100644
@@ -589,6 +589,14 @@ &mdss {
 	status = "okay";
 };
 
+&mdss_dp {
+	status = "okay";
+};
+
+&mdss_dp_out {
+	data-lanes = <0 1>;
+};
+
 &mdss_dsi0 {
 	status = "okay";
 	vdda-supply = <&vreg_l26a_1p2>;
Signature · 1 line
2.56.0.611.g1030f0b940-goog
#4 lore raw

PATCH 4/7 arm64: dts: qcom: c630: Use the idle states from the ACPI tables

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

Commit 07db10de262f ("arm64: dts: qcom: sdm850-lenovo-yoga-c630: lower PSCI cluster idle") dropped bit 14 from the PSCI suspend parameter of the cluster idle state, as the C630 would occasionally reset with the sdm845 default. However, the firmware refuses the resulting value of 0x41008244:
the cluster power domain is never entered, in EL1 or EL2, and the last CPU of the cluster keeps retrying instead, including during suspend to idle.

In EL2, the firmware accepts the sdm845 default of 0x4100c244. However, besides powering down the cluster, this state also puts the rest of the SoC to sleep (LLCC off, AOSS sleep), and entering it at runtime while the display is off, e.g., right after locking the screen, makes the C630 reset.

The _LPI objects in the C630's DSDT describe the states that Windows uses:
on top of CPU state C4 (0x40000004), the cluster container has L3Cluster.D4 (0x40) at level 0x01000000, and the system container has platform.F2 (0xc300). So use L3Cluster.D4, i.e., 0x41000044, which only powers down the cluster and its L3 cache, as the cluster idle state.

Add platform.F2, i.e., 0x4100c344, as a second, deeper cluster idle state, which lets the SoC reach AOSS sleep and CX collapse during suspend to idle. It differs from the sdm845 default in bits 11:8, which are 3 rather than 2, like in the corresponding states of newer SoCs and of the sc8180x and sc8280xp WoA platforms. Give it a minimum residency of about 71 minutes to keep the runtime governor away from it: suspend to idle always picks the deepest state that satisfies the CPU wakeup latency QoS constraint, regardless of its residency.

---
 arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 25 +++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts +24 -1
index 0645ab429177..3e4df4335be8 100644
@@ -37,6 +37,24 @@ aliases {
 		serial1 = &uart6;
 	};
 
+	cpus {
+		domain-idle-states {
+			/*
+			 * platform.F2 on top of L3Cluster.D4 from the ACPI tables,
+			 * which also lets the rest of the SoC sleep. Suspend to idle
+			 * picks the deepest state regardless of its residency, so
+			 * use one that keeps the runtime governor away from it.
+			 */
+			cluster_sleep_1: cluster-sleep-1 {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x4100c344>;
+				entry-latency-us = <3263>;
+				exit-latency-us = <6562>;
+				min-residency-us = <0xffffffff>;
+			};
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 
@@ -340,8 +358,13 @@ &cdsp_pas {
 	status = "okay";
 };
 
+&cluster_pd {
+	domain-idle-states = <&cluster_sleep_0>, <&cluster_sleep_1>;
+};
+
+/* L3Cluster.D4 from the ACPI tables: powers down the cluster and its L3 */
 &cluster_sleep_0 {
-	arm,psci-suspend-param = <0x41008244>;
+	arm,psci-suspend-param = <0x41000044>;
 };
 
 &gcc {
Signature · 1 line
2.56.0.611.g1030f0b940-goog
#5 lore raw

PATCH 5/7 arm64: dts: qcom: c630: Wait for the touchpad at resume

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

When the SoC reaches deep sleep (AOSS sleep and CX collapse) in suspend to idle, the touchpad is often reset, presumably because it loses power, and it may still be starting up when it is powered on again at resume. If it doesn't acknowledge the power on command, i2c-hid tries once more 0.5 ms later, and then fails the resume:

  [  712.566907] i2c_hid_of 3-0015: failed to change power setting.
  [  712.567528] i2c_hid_of 3-0015: PM: dpm_run_callback(): i2c_hid_core_pm_resume [i2c_hid] returns -6

hid-multitouch then never puts the touchpad back into touchpad mode, so it stays in the mouse mode it starts up in, without two-finger scrolling, until the next suspend and resume.

This happened after about one in eight suspends. With a debug patch that keeps trying, the touchpad answered the next attempt, about 6 ms later, each time it had missed the first two.

So give the touchpad a post power on delay of 20 ms, which i2c-hid-of waits for at probe, and at every resume before powering the touchpad on, as the touchpad isn't a wakeup source.

---
 arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 2 ++
 1 file changed, 2 insertions(+)
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts +2 -0
index 3e4df4335be8..accd80005195 100644
@@ -496,6 +496,8 @@ tsel: hid@15 {
 		hid-descr-addr = <0x1>;
 
 		interrupts-extended = <&tlmm 37 IRQ_TYPE_LEVEL_HIGH>;
+
+		post-power-on-delay-ms = <20>;
 	};
 
 	tsc2: hid@2c {
Signature · 1 line
2.56.0.611.g1030f0b940-goog
#6 lore raw

Re: [PATCH 5/7] arm64: dts: qcom: c630: Wait for the touchpad at resume

Quoted 20 lines · Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

When the SoC reaches deep sleep (AOSS sleep and CX collapse) in suspend to idle, the touchpad is often reset, presumably because it loses power, and it may still be starting up when it is powered on again at resume. If it doesn't acknowledge the power on command, i2c-hid tries once more 0.5 ms later, and then fails the resume:

  [  712.566907] i2c_hid_of 3-0015: failed to change power setting.
  [  712.567528] i2c_hid_of 3-0015: PM: dpm_run_callback(): 

i2c_hid_core_pm_resume [i2c_hid] returns -6

hid-multitouch then never puts the touchpad back into touchpad mode, so it stays in the mouse mode it starts up in, without two-finger scrolling, until the next suspend and resume.

This happened after about one in eight suspends. With a debug patch that keeps trying, the touchpad answered the next attempt, about 6 ms later, each time it had missed the first two.

Quoted 9 lines

So give the touchpad a post power on delay of 20 ms, which i2c-hid-of waits for at probe, and at every resume before powering the touchpad on, as the touchpad isn't a wakeup source.

---
 arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 2 ++
 1 file changed, 2 insertions(+)

Please disregard this one - it doesn't help in the end.

#7 lore raw

PATCH 6/7 platform: arm64: lenovo-yoga-c630: Disable the EC interrupt while suspended

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

The EC interrupt handler reads the pending event from the EC over I2C. If the interrupt fires while the system is suspending or resuming, between the late suspend and early resume phases of the I2C controller, the transfer fails, as runtime PM is disabled for the controller:

geni_i2c 884000.i2c: error turning SE resources:-13

The handler then returns without having read the event, and as the interrupt is level triggered, it fires again right away. On the Yoga C630, this shows up as tens to thousands of these errors in about half of the suspends.

So disable the EC interrupt when suspending, and enable it again when resuming. Pending events are then handled once the I2C controller can be used again.

---
 drivers/platform/arm64/lenovo-yoga-c630.c | 34 +++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)
drivers/platform/arm64/lenovo-yoga-c630.c +33 -1
index cad40c58c55b..2776063bfb58 100644
@@ -10,11 +10,13 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
+#include <linux/interrupt.h>
 #include <linux/irqreturn.h>
 #include <linux/lockdep.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/notifier.h>
+#include <linux/pm.h>
 #include <linux/slab.h>
 #include <linux/platform_data/lenovo-yoga-c630.h>
 
@@ -216,6 +218,7 @@ static int yoga_c630_ec_probe(struct i2c_client *client)
 	mutex_init(&ec->lock);
 	ec->client = client;
 	BLOCKING_INIT_NOTIFIER_HEAD(&ec->notifier_list);
+	i2c_set_clientdata(client, ec);
 
 	ret = devm_request_threaded_irq(dev, client->irq,
 					NULL, yoga_c630_ec_thread_intr,
@@ -230,6 +233,34 @@ static int yoga_c630_ec_probe(struct i2c_client *client)
 	return yoga_c630_aux_init(dev, YOGA_C630_DEV_UCSI, ec);
 }
 
+/*
+ * Keep the EC interrupt disabled until the system has resumed. Its
+ * handler reads the pending event over I2C, which fails between the late
+ * suspend and early resume phases of the I2C controller, and as the
+ * interrupt is level triggered, it would fire over and over again until the
+ * handler gets to read the event.
+ */
+static int yoga_c630_ec_suspend(struct device *dev)
+{
+	struct yoga_c630_ec *ec = dev_get_drvdata(dev);
+
+	disable_irq(ec->client->irq);
+
+	return 0;
+}
+
+static int yoga_c630_ec_resume(struct device *dev)
+{
+	struct yoga_c630_ec *ec = dev_get_drvdata(dev);
+
+	enable_irq(ec->client->irq);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(yoga_c630_ec_pm_ops, yoga_c630_ec_suspend,
+				yoga_c630_ec_resume);
+
 
 static const struct of_device_id yoga_c630_ec_of_match[] = {
 	{ .compatible = "lenovo,yoga-c630-ec" },
@@ -246,7 +277,8 @@ MODULE_DEVICE_TABLE(i2c, yoga_c630_ec_i2c_id_table);
 static struct i2c_driver yoga_c630_ec_i2c_driver = {
 	.driver = {
 		.name = "yoga-c630-ec",
-		.of_match_table = yoga_c630_ec_of_match
+		.of_match_table = yoga_c630_ec_of_match,
+		.pm = pm_sleep_ptr(&yoga_c630_ec_pm_ops),
 	},
 	.probe = yoga_c630_ec_probe,
 	.id_table = yoga_c630_ec_i2c_id_table,
Signature · 1 line
2.56.0.611.g1030f0b940-goog
#8 lore raw

PATCH 7/7 platform: arm64: lenovo-yoga-c630: Expose the keyboard backlight as an LED

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

The EC turns the keyboard backlight on and off as Fn+Space is pressed, cycling through two brightness levels, which the EC registers do not seem to reflect. However, bit 0 of EC register 0x03 reflects whether the backlight is on, and setting or clearing it, using the EC's register write command (0xb1, as used by the DSDT's ECNW method), turns the backlight on at the selected brightness level, or off.

Expose this as a platform::kbd_backlight LED with a maximum brightness of 1, so that userspace can turn off the keyboard backlight, e.g., when the system is idle. Whenever the EC raises an event, check whether the backlight was turned on or off, and report any such change through brightness_hw_changed, in case the EC raises an event when Fn+Space is pressed.

---
 drivers/platform/arm64/Kconfig            |   4 +-
 drivers/platform/arm64/lenovo-yoga-c630.c | 121 +++++++++++++++++++-
 2 files changed, 120 insertions(+), 5 deletions(-)
drivers/platform/arm64/Kconfig +3 -1
index 2a84775a13a9..44d5a749f764 100644
@@ -75,10 +75,12 @@ config EC_LENOVO_YOGA_C630
 	depends on ARCH_QCOM || COMPILE_TEST
 	depends on I2C
 	select AUXILIARY_BUS
+	select LEDS_CLASS
+	select NEW_LEDS
 	help
 	  Driver for the Embedded Controller in the Qualcomm Snapdragon-based
 	  Lenovo Yoga C630, which provides battery and power adapter
-	  information.
+	  information, and access to the keyboard backlight.
 
 	  This driver provides battery and AC status support for the mentioned
 	  laptop where this information is not properly exposed via the
drivers/platform/arm64/lenovo-yoga-c630.c +117 -4

188 lines not on this page · raw message

Signature · 1 line
2.56.0.611.g1030f0b940-goog