[linux-sunxi] Re: [RFC PATCH] arm64: dts: allwinner: a64/h5: Add CPU idle states
On Mon, 22 Mar 2021 01:25:14 -0500 Samuel Holland wrote: Hi, > Powering off idle CPUs saves about 33 mW compared to using WFI only. > Additional power savings are possible by idling the L2 and downclocking > the cluster when all CPUs are idle. > > Entry and exit latency were measured using a logic analyzer, with GPIO > pins toggled in Linux after the calls to trace_cpu_idle() in > cpuidle_enter_state(), and in the power management firmware after CPU > power-off completes and immediately after detecting an interrupt. > > 800 us and 1500 us are worst-case values, largely driven by the fact > that the power management firmware is single threaded. It can only > handle commands to power off CPUs one at a time, and it cannot process > any commands while powering on a CPU in response to an interrupt. > > The cluster suspend process reliably takes 36 us; I rounded this up to > 50 us. If all CPUs enter the cluster idle state at the same time, exit > latency is actually reduced, because there is no contention in that > case. However, if only some CPUs enter the cluster idle state, behavior > is the same as for CPU idle. > > Polling delay for the power management firmware to detect a pending > interrupt is insignificant; it is less than 20 us. > > min-residency was chosen as the point where enabling the idle state > consumed no more average power than disabling the idle state at a > variety of interrupt rates. > > Signed-off-by: Samuel Holland > --- > > I'm sending this patch as an RFC because it raises questions about how > we handle firmware versioning. How far back does (or should) our support > for old TF-A and Crust versions go? > > cpuidle has a problem that without working firmware support, CPUs will > enter idle states and be unable to wake up. As a result, the system will > hang at some point during boot, usually before getting to userspace. > > For over a year[0], TF-A has exposed the PSCI CPU_SUSPEND function when > a SCPI implementation is present[1]. Implementing CPU_SUSPEND is > required for implementing SYSTEM_SUSPEND[2], even if CPU_SUSPEND is not > itself used for anything. > > However, there was no code to actually wake up a CPU once it called the > CPU_SUSPEND function, because I could not find the register providing > the necessary information. The fact that CPU_SUSPEND was broken affected > nobody, because nothing ever called it -- there were no idle states in > the DTS. In hindsight, what I should have done was always return failure > from sunxi_validate_power_state(), but that ship has long sailed. > > I finally found the elusive register and implemented the wakeup code > earlier this month[3]. So now, CPU_SUSPEND actually works, if all of > your firmware is up to date, and cpuidle works if you add the states in > your device tree. > > Unfortunately, there is currently nothing verifying that compatibility. > So you can get into four possible scenarios: > 1) No idle states in DTS, any firmware => Linux works, with baseline > power consumption. > 2) Idle states added to DTS, no Crust/SCPI => Linux works, but every > attempt to enter an idle state is rejected because CPU_SUSPEND is > not hooked up. So power consumption increases by a sizable amount. > 3) Idle states added to DTS, "old" Crust/SCPI (before [3]) => Linux > fails to boot, because CPUs never return from idle states. > 4) Idle states added to DTS, "new" Crust/SCPI (after [3]) => Linux > works, with improved power consumption compared to the baseline. > > Obviously, we want to prevent scenario 3 if possible. So I think the core of the problem is that the DT describes some firmware feature, but we have the DT bundled with the kernel, not the firmware. So is there any way we can detect an older crust version in U-Boot, then remove any potential idle states from the DT? Granted, this requires recent U-Boot as well, but at least we could try to mitigate the worst case a bit? A better solution could be to only *add* the idle states if the rest of the firmware is deemed worthy. So the mainline DTs would not carry the properties in the first place, and only U-Boot adds them, on detecting a capable firmware? Admittedly this changes the "flow" of the DT, where the kernel is the authority, but it might help to solve this problem? Or any other way, which involves U-Boot patching the DTB? (This would apply to the DTB passed to the kernel, regardless of where and when it's loaded from) Any opinions? Cheers, Andre > Enter the current patch: I chose the arm,psci-suspend-param values > specifically so they would be _rejected_ by the current TF-A code. This > makes scenario 3 behave like scenario 2. I then have some follow-up TF-A > patches (not yet submitted) to switch to the new parameter encoding[4]. > > This brings me back to my original question. Once the TF-A patches in > [4] are merged, scenario 3 (with an updated TF-A but an old Crust) would > fail to boot again. Do we care? > > Should I
[linux-sunxi] [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
Replace of_drm_find_panel with drm_of_find_panel_or_bridge
for finding panel, this indeed help to find the bridge if
bridge support added.
Added NULL in bridge argument, same will replace with bridge
parameter once bridge supported.
Signed-off-by: Jagan Teki
---
Changes for v4, v3:
- none
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 4f5efcace68e..2e9e7b2d4145 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -21,6 +21,7 @@
#include
#include
+#include
#include
#include
#include
@@ -963,10 +964,14 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)
{
struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
- struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
+ struct drm_panel *panel;
+ int ret;
+
+ ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
+ , NULL);
+ if (ret)
+ return ret;
- if (IS_ERR(panel))
- return PTR_ERR(panel);
if (!dsi->drm || !dsi->drm->registered)
return -EPROBE_DEFER;
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20210322140152.101709-2-jagan%40amarulasolutions.com.
[linux-sunxi] [DO NOT MERGE] [PATCH v4 4/4] ARM: dts: sun8i: bananapi-m2m: Enable S070WV20-CT16 panel
This patch add support for Bananapi S070WV20-CT16 panel to
BPI-M2M board.
Bananapi S070WV20-CT16 is a pure RGB output panel with ICN6211 DSI/RGB
converter bridge, so enable bridge along with associated panel.
DSI panel connected via board DSI port with,
- DCDC1 as VCC-DSI supply
- PL5 gpio for bridge enable gpio pin
- PB7 gpio for lcd enable gpio pin
- PL4 gpio for backlight enable pin
Signed-off-by: Jagan Teki
---
Changes for v4:
- replace reset with enable-gpios
Changes for v3:
- none
arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts | 85
1 file changed, 85 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
index 293016d081cd..6f33e1ae8ffc 100644
--- a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
+++ b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
@@ -44,6 +44,7 @@
#include "sun8i-a33.dtsi"
#include
+#include
/ {
model = "BananaPi M2 Magic";
@@ -55,12 +56,21 @@ aliases {
i2c2 =
serial0 =
serial1 =
+ mmc0 =
};
chosen {
stdout-path = "serial0:115200n8";
};
+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = < 0 5 PWM_POLARITY_INVERTED>;
+ brightness-levels = <1 2 4 8 16 32 64 128 255>;
+ default-brightness-level = <8>;
+ enable-gpios = <_pio 0 4 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PL4
*/
+ };
+
leds {
compatible = "gpio-leds";
@@ -81,6 +91,18 @@ led-2 {
};
};
+ panel {
+ compatible = "bananapi,s070wv20-ct16";
+ enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PB7
*/
+ backlight = <>;
+
+ port {
+ panel_out_bridge: endpoint {
+ remote-endpoint = <_out_panel>;
+ };
+ };
+ };
+
reg_vcc5v0: vcc5v0 {
compatible = "regulator-fixed";
regulator-name = "vcc5v0";
@@ -122,6 +144,59 @@ {
status = "okay";
};
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dsi_out: port@0 {
+ reg = <0>;
+
+ dsi_out_bridge: endpoint {
+ remote-endpoint = <_out_dsi>;
+ };
+ };
+ };
+
+ bridge@0 {
+ compatible = "chipone,icn6211";
+ reg = <0>;
+ enable-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5 */
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ bridge_in: port@0 {
+ reg = <0>;
+
+ bridge_out_dsi: endpoint {
+ remote-endpoint = <_out_bridge>;
+ };
+ };
+
+ bridge_out: port@1 {
+ reg = <1>;
+
+ bridge_out_panel: endpoint {
+ remote-endpoint = <_out_bridge>;
+ };
+ };
+ };
+ };
+};
+
{
status = "okay";
};
@@ -157,6 +232,12 @@ {
status = "okay";
};
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pin>;
+ status = "okay";
+};
+
_rsb {
status = "okay";
@@ -269,6 +350,10 @@ {
status = "okay";
};
+ {
+ status = "okay";
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pb_pins>;
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20210322140152.101709-5-jagan%40amarulasolutions.com.
[linux-sunxi] [PATCH v4 3/4] drm: sun4i: dsi: Convert to bridge driver
DRM bridge drivers have build-in handling of treating all display
pipeline components as bridges.
So, convert the existing to a drm bridge driver with a built-in
encoder support for compatibility with existing component drivers.
Signed-off-by: Jagan Teki
---
Changes for v4:
- none
Changes for v3:
- new patch
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 75 --
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 6 +++
2 files changed, 54 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 39321299dc27..6f3c5330a468 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -714,10 +714,10 @@ static int sun6i_dsi_start(struct sun6i_dsi *dsi,
return 0;
}
-static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
+static void sun6i_dsi_bridge_enable(struct drm_bridge *bridge)
{
- struct drm_display_mode *mode = >crtc->state->adjusted_mode;
- struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
+ struct drm_display_mode *mode =
>encoder->crtc->state->adjusted_mode;
+ struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
struct mipi_dsi_device *device = dsi->device;
union phy_configure_opts opts = { };
struct phy_configure_opts_mipi_dphy *cfg = _dphy;
@@ -801,9 +801,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder
*encoder)
sun6i_dsi_start(dsi, DSI_START_HSD);
}
-static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
+static void sun6i_dsi_bridge_disable(struct drm_bridge *bridge)
{
- struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
+ struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
DRM_DEBUG_DRIVER("Disabling DSI output\n");
@@ -852,9 +852,40 @@ static const struct drm_connector_funcs
sun6i_dsi_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
-static const struct drm_encoder_helper_funcs sun6i_dsi_enc_helper_funcs = {
- .disable= sun6i_dsi_encoder_disable,
- .enable = sun6i_dsi_encoder_enable,
+static int sun6i_dsi_bridge_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
+ int ret;
+
+ if (dsi->panel_bridge)
+ return drm_bridge_attach(bridge->encoder, dsi->panel_bridge,
NULL, 0);
+
+ if (dsi->panel) {
+ drm_connector_helper_add(>connector,
+_dsi_connector_helper_funcs);
+ ret = drm_connector_init(bridge->dev, >connector,
+_dsi_connector_funcs,
+DRM_MODE_CONNECTOR_DSI);
+ if (ret) {
+ dev_err(dsi->dev, "Couldn't initialise the DSI
connector\n");
+ goto err_cleanup_connector;
+ }
+
+ drm_connector_attach_encoder(>connector, >encoder);
+ }
+
+ return 0;
+
+err_cleanup_connector:
+ drm_encoder_cleanup(>encoder);
+ return ret;
+}
+
+static const struct drm_bridge_funcs sun6i_dsi_bridge_funcs = {
+ .enable = sun6i_dsi_bridge_enable,
+ .disable= sun6i_dsi_bridge_disable,
+ .attach = sun6i_dsi_bridge_attach,
};
static u32 sun6i_dsi_dcs_build_pkt_hdr(struct sun6i_dsi *dsi,
@@ -1063,8 +1094,6 @@ static int sun6i_dsi_bind(struct device *dev, struct
device *master,
struct sun6i_dsi *dsi = dev_get_drvdata(dev);
int ret;
- drm_encoder_helper_add(>encoder,
- _dsi_enc_helper_funcs);
ret = drm_simple_encoder_init(drm, >encoder,
DRM_MODE_ENCODER_DSI);
if (ret) {
@@ -1073,27 +1102,12 @@ static int sun6i_dsi_bind(struct device *dev, struct
device *master,
}
dsi->encoder.possible_crtcs = BIT(0);
- drm_connector_helper_add(>connector,
-_dsi_connector_helper_funcs);
- ret = drm_connector_init(drm, >connector,
-_dsi_connector_funcs,
-DRM_MODE_CONNECTOR_DSI);
+ ret = drm_bridge_attach(>encoder, >bridge, NULL, 0);
if (ret) {
- dev_err(dsi->dev,
- "Couldn't initialise the DSI connector\n");
+ dev_err(dsi->dev, "Couldn't attach drm bridge\n");
goto err_cleanup_connector;
}
- drm_connector_attach_encoder(>connector, >encoder);
-
- if (dsi->panel_bridge) {
- ret = drm_bridge_attach(>encoder, dsi->panel_bridge, NULL,
0);
- if (ret) {
- dev_err(dsi->dev, "Couldn't attach drm bridge\n");
- goto err_cleanup_connector;
- }
-
[linux-sunxi] [PATCH v4 2/4] drm: sun4i: dsi: Add bridge support
Some display panels would come up with a non-DSI output which
can have an option to connect DSI interface by means of bridge
converter.
This DSI to non-DSI bridge converter would require a bridge
driver that would communicate the DSI controller for bridge
functionalities.
So, add support for bridge functionalities in Allwinner DSI
controller.
Cc: Samuel Holland
Signed-off-by: Jagan Teki
---
Note:
Samuel Holland, The existing kms hotplug dropped in order to
attach the bridge properly.
However, I did try several ways to support hotplug with the
bridge but it's resulting in a deadlock where bind never attach
bridge until bridge pointer found and bridge pointer cannot
found until bind finishes. Any inputs on this would be appreciated.
Changes for v4:
- none
Changes for v3:
- updated with new API's
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 34 +-
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 +-
2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 2e9e7b2d4145..39321299dc27 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -773,6 +773,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder
*encoder)
if (dsi->panel)
drm_panel_prepare(dsi->panel);
+ if (dsi->panel_bridge)
+ dsi->panel_bridge->funcs->pre_enable(dsi->panel_bridge);
+
/*
* FIXME: This should be moved after the switch to HS mode.
*
@@ -788,6 +791,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder
*encoder)
if (dsi->panel)
drm_panel_enable(dsi->panel);
+ if (dsi->panel_bridge)
+ dsi->panel_bridge->funcs->enable(dsi->panel_bridge);
+
sun6i_dsi_start(dsi, DSI_START_HSC);
udelay(1000);
@@ -804,6 +810,9 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder
*encoder)
if (dsi->panel) {
drm_panel_disable(dsi->panel);
drm_panel_unprepare(dsi->panel);
+ } else if (dsi->panel_bridge) {
+ dsi->panel_bridge->funcs->disable(dsi->panel_bridge);
+ dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge);
}
phy_power_off(dsi->dphy);
@@ -964,23 +973,17 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)
{
struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
- struct drm_panel *panel;
int ret;
ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
- , NULL);
+ >panel, >panel_bridge);
if (ret)
return ret;
- if (!dsi->drm || !dsi->drm->registered)
- return -EPROBE_DEFER;
-
- dsi->panel = panel;
dsi->device = device;
- drm_kms_helper_hotplug_event(dsi->drm);
-
- dev_info(host->dev, "Attached device %s\n", device->name);
+ dev_info(host->dev, "Attached %s %s\n",
+device->name, dsi->panel ? "panel" : "bridge");
return 0;
}
@@ -991,9 +994,10 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host,
struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
dsi->panel = NULL;
+ dsi->panel_bridge = NULL;
dsi->device = NULL;
- drm_kms_helper_hotplug_event(dsi->drm);
+ drm_of_panel_bridge_remove(dsi->dev->of_node, 0, 0);
return 0;
}
@@ -1082,7 +1086,13 @@ static int sun6i_dsi_bind(struct device *dev, struct
device *master,
drm_connector_attach_encoder(>connector, >encoder);
- dsi->drm = drm;
+ if (dsi->panel_bridge) {
+ ret = drm_bridge_attach(>encoder, dsi->panel_bridge, NULL,
0);
+ if (ret) {
+ dev_err(dsi->dev, "Couldn't attach drm bridge\n");
+ goto err_cleanup_connector;
+ }
+ }
return 0;
@@ -1096,7 +1106,7 @@ static void sun6i_dsi_unbind(struct device *dev, struct
device *master,
{
struct sun6i_dsi *dsi = dev_get_drvdata(dev);
- dsi->drm = NULL;
+ drm_encoder_cleanup(>encoder);
}
static const struct component_ops sun6i_dsi_ops = {
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index c863900ae3b4..370ecb356a63 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -29,8 +29,8 @@ struct sun6i_dsi {
struct device *dev;
struct mipi_dsi_device *device;
- struct drm_device *drm;
struct drm_panel*panel;
+ struct drm_bridge *panel_bridge;
};
static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
--
2.25.1
--
You received this message because you are subscribed to the Google
[linux-sunxi] [PATCH v4 0/4] drm: sun4i: dsi: Convert drm bridge
This series convert Allwinner DSI controller to full functional drm bridge driver for supporting slave panel, bridges. Here, are the previous version changes[1]. Patch 1: use drm_of_find_panel_or_bridge API Patch 2: Adding DRM Bridge support Patch 3: Convert to bridge driver, that indeed drop encoder API's and support bridge API's Patch 4: Overlay patch for bridge enablement in BPI-M2M Note: Only nit on this series is kms hotplug, added Samuel Holland for reviews and comments as he is authorized the code before. [1] https://lkml.org/lkml/2021/2/14/173 Any inputs on this would be appreciated! Jagan. Jagan Teki (4): drm: sun4i: dsi: Use drm_of_find_panel_or_bridge drm: sun4i: dsi: Add bridge support drm: sun4i: dsi: Convert to bridge driver [DO NOT MERGE] ARM: dts: sun8i: bananapi-m2m: Enable S070WV20-CT16 panel arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts | 85 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 100 +-- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 8 +- 3 files changed, 160 insertions(+), 33 deletions(-) -- 2.25.1 -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20210322140152.101709-1-jagan%40amarulasolutions.com.
[linux-sunxi] [RFC PATCH] arm64: dts: allwinner: a64/h5: Add CPU idle states
Powering off idle CPUs saves about 33 mW compared to using WFI only. Additional power savings are possible by idling the L2 and downclocking the cluster when all CPUs are idle. Entry and exit latency were measured using a logic analyzer, with GPIO pins toggled in Linux after the calls to trace_cpu_idle() in cpuidle_enter_state(), and in the power management firmware after CPU power-off completes and immediately after detecting an interrupt. 800 us and 1500 us are worst-case values, largely driven by the fact that the power management firmware is single threaded. It can only handle commands to power off CPUs one at a time, and it cannot process any commands while powering on a CPU in response to an interrupt. The cluster suspend process reliably takes 36 us; I rounded this up to 50 us. If all CPUs enter the cluster idle state at the same time, exit latency is actually reduced, because there is no contention in that case. However, if only some CPUs enter the cluster idle state, behavior is the same as for CPU idle. Polling delay for the power management firmware to detect a pending interrupt is insignificant; it is less than 20 us. min-residency was chosen as the point where enabling the idle state consumed no more average power than disabling the idle state at a variety of interrupt rates. Signed-off-by: Samuel Holland --- I'm sending this patch as an RFC because it raises questions about how we handle firmware versioning. How far back does (or should) our support for old TF-A and Crust versions go? cpuidle has a problem that without working firmware support, CPUs will enter idle states and be unable to wake up. As a result, the system will hang at some point during boot, usually before getting to userspace. For over a year[0], TF-A has exposed the PSCI CPU_SUSPEND function when a SCPI implementation is present[1]. Implementing CPU_SUSPEND is required for implementing SYSTEM_SUSPEND[2], even if CPU_SUSPEND is not itself used for anything. However, there was no code to actually wake up a CPU once it called the CPU_SUSPEND function, because I could not find the register providing the necessary information. The fact that CPU_SUSPEND was broken affected nobody, because nothing ever called it -- there were no idle states in the DTS. In hindsight, what I should have done was always return failure from sunxi_validate_power_state(), but that ship has long sailed. I finally found the elusive register and implemented the wakeup code earlier this month[3]. So now, CPU_SUSPEND actually works, if all of your firmware is up to date, and cpuidle works if you add the states in your device tree. Unfortunately, there is currently nothing verifying that compatibility. So you can get into four possible scenarios: 1) No idle states in DTS, any firmware => Linux works, with baseline power consumption. 2) Idle states added to DTS, no Crust/SCPI => Linux works, but every attempt to enter an idle state is rejected because CPU_SUSPEND is not hooked up. So power consumption increases by a sizable amount. 3) Idle states added to DTS, "old" Crust/SCPI (before [3]) => Linux fails to boot, because CPUs never return from idle states. 4) Idle states added to DTS, "new" Crust/SCPI (after [3]) => Linux works, with improved power consumption compared to the baseline. Obviously, we want to prevent scenario 3 if possible. Enter the current patch: I chose the arm,psci-suspend-param values specifically so they would be _rejected_ by the current TF-A code. This makes scenario 3 behave like scenario 2. I then have some follow-up TF-A patches (not yet submitted) to switch to the new parameter encoding[4]. This brings me back to my original question. Once the TF-A patches in [4] are merged, scenario 3 (with an updated TF-A but an old Crust) would fail to boot again. Do we care? Should I implement some kind of runtime version checking, so TF-A can disable CPU_SUSPEND if it would be broken? Or instead, should we wait some amount of time to merge this patch (or the patches at [4]) and assume people have upgraded? Where would people expect this sort of possibly-breaking change to be documented? Separately, since I assume most A64/H5 users (outside of LibreELEC and the PinePhone) are not using Crust, scenario 2 would be very common. If merging this patch increases their idle power draw by 500 mW, is that an acceptable cost for decreasing other users' idle power draw by 50 mW? Sorry for the wall of text, Samuel [0]: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/plat/allwinner/common/sunxi_pm.c?id=e382c88e2a26995099bb931d49e754dcaebc5593 [1]: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/plat/allwinner/common/sunxi_scpi_pm.c?id=2e0e51f42586826a1f6f6c1e532f90e6df642cf5#n190 [2]: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/lib/psci/psci_setup.c?id=2e0e51f42586826a1f6f6c1e532f90e6df642cf5#n251 [3]:
