[PATCH v5 1/2] drm/bridge: parade-ps8640: Enable runtime power management

[PATCH v5 1/2] drm/bridge: parade-ps8640: Enable runtime power management

[Philip Chen]

Fit ps8640 driver into runtime power management framework:

First, break _poweron() to 3 parts: (1) turn on power and wait for
ps8640's internal MCU to finish init (2) check panel HPD (which is
proxied by GPIO9) (3) the other configs. As runtime_resume() can be
called before panel is powered, we only add (1) to _resume() and leave
(2)(3) to _pre_enable(). We also add (2) to _aux_transfer() as we want
to ensure panel HPD is asserted before we start AUX CH transactions.

Second, the original driver has a mysterious delay of 50 ms between (2)
and (3). Since Parade's support can't explain what the delay is for,
and we don't see removing the delay break any boards at hand, remove
the delay to fit into this driver change.

In addition, rename "powered" to "pre_enabled" and don't check for it
in the pm_runtime calls. The pm_runtime calls are already refcounted
so there's no reason to check there. The other user of "powered",
_get_edid(), only cares if pre_enable() has already been called.

Lastly, change some existing DRM_...() logging to dev_...() along the
way, since DRM_...() seem to be deprecated in [1].

[1] https://patchwork.freedesktop.org/patch/454760/

Signed-off-by: Philip Chen <philipchen@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
In v3, I added pm_suspend_ignore_children() in ps8640_probe().
Also, I moved the change of "put_sync_suspend" from patch 2/2 to here.
But I forgot to mention both changes. So edit v3 change log retroactively.

In v4, I moved the change of "ps8640_ensure_hpd" return data type
from patch 2/2 to here. But I forgot to mention it. So edit v4 change log
retroactively.

Changes in v5:
- Move the implementation of _runtime_disable() around to resolve merge
  conflict when rebasing.
- Improve the document for how autosuspend_delay is picked.

Changes in v4:
- Make ps8640_ensure_hpd() return int (This change was mis-added to
  patch 2/2 in this patch series).

Changes in v3:
- Fix typo/wording in the commit message.
- Add ps8640_aux_transfer_msg() for AUX operation. In
  ps8640_aux_transfer(), wrap around ps8640_aux_transfer_msg()
  with PM operations and HPD check.
- Document why autosuspend_delay is set to 500ms.
- Add pm_suspend_ignore_children() in ps8640_probe()
- Replace _put_sync() with _put_sync_suspend() in ps8640_post_disable()
  (The change was mis-added to patch 2/2 in this patch series.)

 drivers/gpu/drm/bridge/parade-ps8640.c | 190 ++++++++++++++++---------
 1 file changed, 119 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
index 4b36e4dc78f1..63a817d92c1d 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -9,6 +9,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/of_graph.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
@@ -100,7 +101,7 @@ struct ps8640 {
 	struct regulator_bulk_data supplies[2];
 	struct gpio_desc *gpio_reset;
 	struct gpio_desc *gpio_powerdown;
-	bool powered;
+	bool pre_enabled;
 };
 
 static const struct regmap_config ps8640_regmap_config[] = {
@@ -148,8 +149,29 @@ static inline struct ps8640 *aux_to_ps8640(struct drm_dp_aux *aux)
 	return container_of(aux, struct ps8640, aux);
 }
 
-static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
-				   struct drm_dp_aux_msg *msg)
+static int ps8640_ensure_hpd(struct ps8640 *ps_bridge)
+{
+	struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
+	struct device *dev = &ps_bridge->page[PAGE2_TOP_CNTL]->dev;
+	int status;
+	int ret;
+
+	/*
+	 * Apparently something about the firmware in the chip signals that
+	 * HPD goes high by reporting GPIO9 as high (even though HPD isn't
+	 * actually connected to GPIO9).
+	 */
+	ret = regmap_read_poll_timeout(map, PAGE2_GPIO_H, status,
+				status & PS_GPIO9, 20 * 1000, 200 * 1000);
+
+	if (ret < 0)
+		dev_warn(dev, "HPD didn't go high: %d\n", ret);
+
+	return ret;
+}
+
+static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
+				       struct drm_dp_aux_msg *msg)
 {
 	struct ps8640 *ps_bridge = aux_to_ps8640(aux);
 	struct regmap *map = ps_bridge->regmap[PAGE0_DP_CNTL];
@@ -274,38 +296,49 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
 	return len;
 }
 
-static int ps8640_bridge_vdo_control(struct ps8640 *ps_bridge,
-				     const enum ps8640_vdo_control ctrl)
+static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
+				   struct drm_dp_aux_msg *msg)
+{
+	struct ps8640 *ps_bridge = aux_to_ps8640(aux);
+	struct device *dev = &ps_bridge->page[PAGE0_DP_CNTL]->dev;
+	int ret;
+
+	pm_runtime_get_sync(dev);
+	ret = ps8640_ensure_hpd(ps_bridge);
+	if (!ret)
+		ret = ps8640_aux_transfer_msg(aux, msg);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
+static void ps8640_bridge_vdo_control(struct ps8640 *ps_bridge,
+				      const enum ps8640_vdo_control ctrl)
 {
 	struct regmap *map = ps_bridge->regmap[PAGE3_DSI_CNTL1];
+	struct device *dev = &ps_bridge->page[PAGE3_DSI_CNTL1]->dev;
 	u8 vdo_ctrl_buf[] = { VDO_CTL_ADD, ctrl };
 	int ret;
 
 	ret = regmap_bulk_write(map, PAGE3_SET_ADD,
 				vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
 
-	if (ret < 0) {
-		DRM_ERROR("failed to %sable VDO: %d\n",
-			  ctrl == ENABLE ? "en" : "dis", ret);
-		return ret;
-	}
-
-	return 0;
+	if (ret < 0)
+		dev_err(dev, "failed to %sable VDO: %d\n",
+			ctrl == ENABLE ? "en" : "dis", ret);
 }
 
-static void ps8640_bridge_poweron(struct ps8640 *ps_bridge)
+static int __maybe_unused ps8640_resume(struct device *dev)
 {
-	struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
-	int ret, status;
-
-	if (ps_bridge->powered)
-		return;
+	struct ps8640 *ps_bridge = dev_get_drvdata(dev);
+	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(ps_bridge->supplies),
 				    ps_bridge->supplies);
 	if (ret < 0) {
-		DRM_ERROR("cannot enable regulators %d\n", ret);
-		return;
+		dev_err(dev, "cannot enable regulators %d\n", ret);
+		return ret;
 	}
 
 	gpiod_set_value(ps_bridge->gpio_powerdown, 0);
@@ -314,86 +347,78 @@ static void ps8640_bridge_poweron(struct ps8640 *ps_bridge)
 	gpiod_set_value(ps_bridge->gpio_reset, 0);
 
 	/*
-	 * Wait for the ps8640 embedded MCU to be ready
-	 * First wait 200ms and then check the MCU ready flag every 20ms
+	 * Mystery 200 ms delay for the "MCU to be ready". It's unclear if
+	 * this is truly necessary since the MCU will already signal that
+	 * things are "good to go" by signaling HPD on "gpio 9". See
+	 * ps8640_ensure_hpd(). For now we'll keep this mystery delay just in
+	 * case.
 	 */
 	msleep(200);
 
-	ret = regmap_read_poll_timeout(map, PAGE2_GPIO_H, status,
-				       status & PS_GPIO9, 20 * 1000, 200 * 1000);
-
-	if (ret < 0) {
-		DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", ret);
-		goto err_regulators_disable;
-	}
-
-	msleep(50);
-
-	/*
-	 * The Manufacturer Command Set (MCS) is a device dependent interface
-	 * intended for factory programming of the display module default
-	 * parameters. Once the display module is configured, the MCS shall be
-	 * disabled by the manufacturer. Once disabled, all MCS commands are
-	 * ignored by the display interface.
-	 */
-
-	ret = regmap_update_bits(map, PAGE2_MCS_EN, MCS_EN, 0);
-	if (ret < 0) {
-		DRM_ERROR("failed write PAGE2_MCS_EN: %d\n", ret);
-		goto err_regulators_disable;
-	}
-
-	/* Switch access edp panel's edid through i2c */
-	ret = regmap_write(map, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
-	if (ret < 0) {
-		DRM_ERROR("failed write PAGE2_I2C_BYPASS: %d\n", ret);
-		goto err_regulators_disable;
-	}
-
-	ps_bridge->powered = true;
-
-	return;
-
-err_regulators_disable:
-	regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
-			       ps_bridge->supplies);
+	return 0;
 }
 
-static void ps8640_bridge_poweroff(struct ps8640 *ps_bridge)
+static int __maybe_unused ps8640_suspend(struct device *dev)
 {
+	struct ps8640 *ps_bridge = dev_get_drvdata(dev);
 	int ret;
 
-	if (!ps_bridge->powered)
-		return;
-
 	gpiod_set_value(ps_bridge->gpio_reset, 1);
 	gpiod_set_value(ps_bridge->gpio_powerdown, 1);
 	ret = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
 				     ps_bridge->supplies);
 	if (ret < 0)
-		DRM_ERROR("cannot disable regulators %d\n", ret);
+		dev_err(dev, "cannot disable regulators %d\n", ret);
 
-	ps_bridge->powered = false;
+	return ret;
 }
 
+static const struct dev_pm_ops ps8640_pm_ops = {
+	SET_RUNTIME_PM_OPS(ps8640_suspend, ps8640_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+};
+
 static void ps8640_pre_enable(struct drm_bridge *bridge)
 {
 	struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
+	struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
+	struct device *dev = &ps_bridge->page[PAGE0_DP_CNTL]->dev;
 	int ret;
 
-	ps8640_bridge_poweron(ps_bridge);
+	pm_runtime_get_sync(dev);
+	ps8640_ensure_hpd(ps_bridge);
+
+	/*
+	 * The Manufacturer Command Set (MCS) is a device dependent interface
+	 * intended for factory programming of the display module default
+	 * parameters. Once the display module is configured, the MCS shall be
+	 * disabled by the manufacturer. Once disabled, all MCS commands are
+	 * ignored by the display interface.
+	 */
+
+	ret = regmap_update_bits(map, PAGE2_MCS_EN, MCS_EN, 0);
+	if (ret < 0)
+		dev_warn(dev, "failed write PAGE2_MCS_EN: %d\n", ret);
 
-	ret = ps8640_bridge_vdo_control(ps_bridge, ENABLE);
+	/* Switch access edp panel's edid through i2c */
+	ret = regmap_write(map, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
 	if (ret < 0)
-		ps8640_bridge_poweroff(ps_bridge);
+		dev_warn(dev, "failed write PAGE2_MCS_EN: %d\n", ret);
+
+	ps8640_bridge_vdo_control(ps_bridge, ENABLE);
+
+	ps_bridge->pre_enabled = true;
 }
 
 static void ps8640_post_disable(struct drm_bridge *bridge)
 {
 	struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
 
+	ps_bridge->pre_enabled = false;
+
 	ps8640_bridge_vdo_control(ps_bridge, DISABLE);
-	ps8640_bridge_poweroff(ps_bridge);
+	pm_runtime_put_sync_suspend(&ps_bridge->page[PAGE0_DP_CNTL]->dev);
 }
 
 static int ps8640_bridge_attach(struct drm_bridge *bridge,
@@ -426,7 +451,7 @@ static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge,
 					   struct drm_connector *connector)
 {
 	struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
-	bool poweroff = !ps_bridge->powered;
+	bool poweroff = !ps_bridge->pre_enabled;
 	struct edid *edid;
 
 	/*
@@ -456,6 +481,12 @@ static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge,
 	return edid;
 }
 
+static void ps8640_runtime_disable(void *data)
+{
+	pm_runtime_dont_use_autosuspend(data);
+	pm_runtime_disable(data);
+}
+
 static const struct drm_bridge_funcs ps8640_bridge_funcs = {
 	.attach = ps8640_bridge_attach,
 	.detach = ps8640_bridge_detach,
@@ -586,6 +617,22 @@ static int ps8640_probe(struct i2c_client *client)
 	ps_bridge->aux.transfer = ps8640_aux_transfer;
 	drm_dp_aux_init(&ps_bridge->aux);
 
+	pm_runtime_enable(dev);
+	/*
+	 * Powering on ps8640 takes ~300ms. To avoid wasting time on power
+	 * cycling ps8640 too often, set autosuspend_delay to 500ms to ensure
+	 * the bridge wouldn't suspend in between each _aux_transfer_msg() call
+	 * during EDID read (~20ms in my experiment) and in between the last
+	 * _aux_transfer_msg() call during EDID read and the _pre_enable() call
+	 * (~100ms in my experiment).
+	 */
+	pm_runtime_set_autosuspend_delay(dev, 500);
+	pm_runtime_use_autosuspend(dev);
+	pm_suspend_ignore_children(dev, true);
+	ret = devm_add_action_or_reset(dev, ps8640_runtime_disable, dev);
+	if (ret)
+		return ret;
+
 	drm_bridge_add(&ps_bridge->bridge);
 
 	ret = ps8640_bridge_host_attach(dev, ps_bridge);
@@ -620,6 +667,7 @@ static struct i2c_driver ps8640_driver = {
 	.driver = {
 		.name = "ps8640",
 		.of_match_table = ps8640_match,
+		.pm = &ps8640_pm_ops,
 	},
 };
 module_i2c_driver(ps8640_driver);
-- 
2.33.0.1079.g6e70778dc9-goog

[PATCH v5 2/2] drm/bridge: parade-ps8640: Populate devices on aux-bus

[Philip Chen]

Conventionally, panel is listed under the root of the device tree.
When userland asks for display mode, ps8640 bridge is responsible
for returning EDID when ps8640_bridge_get_edid() is called.

Now enable a new option of listing panel under "aux-bus" of ps8640
bridge node in the device tree. In this case, panel driver can retrieve
EDID by triggering AUX transactions, without ps8640_bridge_get_edid()
calls at all.

To prevent the "old" and "new" options from interfering with each
other's logic flow, disable DRM_BRIDGE_OP_EDID when the new option
is taken.

Signed-off-by: Philip Chen <philipchen@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
In v4, I factored out the "ps8640_ensure_hpd" change and added it to patch 1/2
in this patch series. But I forgot to mention it in v4 change log. Edit v4
change log retroactively.

In v3, I factored out the "put_sync_suspend" change and added it to patch 1/2
in this patch series. But I forgot to mention it in v3 change log. Edit v3
change log retroactively.

(no changes since v4)

Changes in v4:
- Move the change of "ps8640_ensure_hpd" to patch 1/2 in this patch series.

Changes in v3:
- Fix when to call of_node_put() in ps8640_of_panel_on_aux_bus()
- Move the change of "put_sync_suspend" to patch 1/2 in this patch series.

Changes in v2:
- Add of_node_put() calls in ps8640_of_panel_on_aux_bus()
- Select DRM_DP_AUX_BUS for PS8640 driver in Kconfig

 drivers/gpu/drm/bridge/Kconfig         |  1 +
 drivers/gpu/drm/bridge/parade-ps8640.c | 51 ++++++++++++++++++++------
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 431b6e12a81f..61db5a66b493 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -182,6 +182,7 @@ config DRM_PARADE_PS8622
 config DRM_PARADE_PS8640
 	tristate "Parade PS8640 MIPI DSI to eDP Converter"
 	depends on OF
+	select DRM_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
index 63a817d92c1d..448988d9eb41 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -14,6 +14,7 @@
 #include <linux/regulator/consumer.h>
 
 #include <drm/drm_bridge.h>
+#include <drm/drm_dp_aux_bus.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_of.h>
@@ -149,6 +150,23 @@ static inline struct ps8640 *aux_to_ps8640(struct drm_dp_aux *aux)
 	return container_of(aux, struct ps8640, aux);
 }
 
+static bool ps8640_of_panel_on_aux_bus(struct device *dev)
+{
+	struct device_node *bus, *panel;
+
+	bus = of_get_child_by_name(dev->of_node, "aux-bus");
+	if (!bus)
+		return false;
+
+	panel = of_get_child_by_name(bus, "panel");
+	of_node_put(bus);
+	if (!panel)
+		return false;
+	of_node_put(panel);
+
+	return true;
+}
+
 static int ps8640_ensure_hpd(struct ps8640 *ps_bridge)
 {
 	struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
@@ -555,17 +573,6 @@ static int ps8640_probe(struct i2c_client *client)
 	if (!ps_bridge)
 		return -ENOMEM;
 
-	/* port@1 is ps8640 output port */
-	ret = drm_of_find_panel_or_bridge(np, 1, 0, &panel, NULL);
-	if (ret < 0)
-		return ret;
-	if (!panel)
-		return -ENODEV;
-
-	ps_bridge->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
-	if (IS_ERR(ps_bridge->panel_bridge))
-		return PTR_ERR(ps_bridge->panel_bridge);
-
 	ps_bridge->supplies[0].supply = "vdd33";
 	ps_bridge->supplies[1].supply = "vdd12";
 	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ps_bridge->supplies),
@@ -588,9 +595,16 @@ static int ps8640_probe(struct i2c_client *client)
 
 	ps_bridge->bridge.funcs = &ps8640_bridge_funcs;
 	ps_bridge->bridge.of_node = dev->of_node;
-	ps_bridge->bridge.ops = DRM_BRIDGE_OP_EDID;
 	ps_bridge->bridge.type = DRM_MODE_CONNECTOR_eDP;
 
+	/*
+	 * In the device tree, if panel is listed under aux-bus of the bridge
+	 * node, panel driver should be able to retrieve EDID by itself using
+	 * aux-bus. So let's not set DRM_BRIDGE_OP_EDID here.
+	 */
+	if (!ps8640_of_panel_on_aux_bus(&client->dev))
+		ps_bridge->bridge.ops = DRM_BRIDGE_OP_EDID;
+
 	ps_bridge->page[PAGE0_DP_CNTL] = client;
 
 	ps_bridge->regmap[PAGE0_DP_CNTL] = devm_regmap_init_i2c(client, ps8640_regmap_config);
@@ -633,6 +647,19 @@ static int ps8640_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	devm_of_dp_aux_populate_ep_devices(&ps_bridge->aux);
+
+	/* port@1 is ps8640 output port */
+	ret = drm_of_find_panel_or_bridge(np, 1, 0, &panel, NULL);
+	if (ret < 0)
+		return ret;
+	if (!panel)
+		return -ENODEV;
+
+	ps_bridge->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
+	if (IS_ERR(ps_bridge->panel_bridge))
+		return PTR_ERR(ps_bridge->panel_bridge);
+
 	drm_bridge_add(&ps_bridge->bridge);
 
 	ret = ps8640_bridge_host_attach(dev, ps_bridge);
-- 
2.33.0.1079.g6e70778dc9-goog

Re: [PATCH v5 1/2] drm/bridge: parade-ps8640: Enable runtime power management

[Philip Chen]

Add "Sam Ravnborg <sam@ravnborg.org>" to cc list for vis.
Remove "Andrzej Hajda <a.hajda@samsung.com>" from cc list as the
address can't be found.

On Thu, Oct 28, 2021 at 10:58 AM Philip Chen <philipchen@chromium.org> wrote:
>
> Fit ps8640 driver into runtime power management framework:
>
> First, break _poweron() to 3 parts: (1) turn on power and wait for
> ps8640's internal MCU to finish init (2) check panel HPD (which is
> proxied by GPIO9) (3) the other configs. As runtime_resume() can be
> called before panel is powered, we only add (1) to _resume() and leave
> (2)(3) to _pre_enable(). We also add (2) to _aux_transfer() as we want
> to ensure panel HPD is asserted before we start AUX CH transactions.
>
> Second, the original driver has a mysterious delay of 50 ms between (2)
> and (3). Since Parade's support can't explain what the delay is for,
> and we don't see removing the delay break any boards at hand, remove
> the delay to fit into this driver change.
>
> In addition, rename "powered" to "pre_enabled" and don't check for it
> in the pm_runtime calls. The pm_runtime calls are already refcounted
> so there's no reason to check there. The other user of "powered",
> _get_edid(), only cares if pre_enable() has already been called.
>
> Lastly, change some existing DRM_...() logging to dev_...() along the
> way, since DRM_...() seem to be deprecated in [1].
>
> [1] https://patchwork.freedesktop.org/patch/454760/
>
> Signed-off-by: Philip Chen <philipchen@chromium.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
> In v3, I added pm_suspend_ignore_children() in ps8640_probe().
> Also, I moved the change of "put_sync_suspend" from patch 2/2 to here.
> But I forgot to mention both changes. So edit v3 change log retroactively.
>
> In v4, I moved the change of "ps8640_ensure_hpd" return data type
> from patch 2/2 to here. But I forgot to mention it. So edit v4 change log
> retroactively.
>
> Changes in v5:
> - Move the implementation of _runtime_disable() around to resolve merge
>   conflict when rebasing.
> - Improve the document for how autosuspend_delay is picked.
>
> Changes in v4:
> - Make ps8640_ensure_hpd() return int (This change was mis-added to
>   patch 2/2 in this patch series).
>
> Changes in v3:
> - Fix typo/wording in the commit message.
> - Add ps8640_aux_transfer_msg() for AUX operation. In
>   ps8640_aux_transfer(), wrap around ps8640_aux_transfer_msg()
>   with PM operations and HPD check.
> - Document why autosuspend_delay is set to 500ms.
> - Add pm_suspend_ignore_children() in ps8640_probe()
> - Replace _put_sync() with _put_sync_suspend() in ps8640_post_disable()
>   (The change was mis-added to patch 2/2 in this patch series.)
>
>  drivers/gpu/drm/bridge/parade-ps8640.c | 190 ++++++++++++++++---------
>  1 file changed, 119 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> index 4b36e4dc78f1..63a817d92c1d 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -9,6 +9,7 @@
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/of_graph.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>
> @@ -100,7 +101,7 @@ struct ps8640 {
>         struct regulator_bulk_data supplies[2];
>         struct gpio_desc *gpio_reset;
>         struct gpio_desc *gpio_powerdown;
> -       bool powered;
> +       bool pre_enabled;
>  };
>
>  static const struct regmap_config ps8640_regmap_config[] = {
> @@ -148,8 +149,29 @@ static inline struct ps8640 *aux_to_ps8640(struct drm_dp_aux *aux)
>         return container_of(aux, struct ps8640, aux);
>  }
>
> -static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
> -                                  struct drm_dp_aux_msg *msg)
> +static int ps8640_ensure_hpd(struct ps8640 *ps_bridge)
> +{
> +       struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
> +       struct device *dev = &ps_bridge->page[PAGE2_TOP_CNTL]->dev;
> +       int status;
> +       int ret;
> +
> +       /*
> +        * Apparently something about the firmware in the chip signals that
> +        * HPD goes high by reporting GPIO9 as high (even though HPD isn't
> +        * actually connected to GPIO9).
> +        */
> +       ret = regmap_read_poll_timeout(map, PAGE2_GPIO_H, status,
> +                               status & PS_GPIO9, 20 * 1000, 200 * 1000);
> +
> +       if (ret < 0)
> +               dev_warn(dev, "HPD didn't go high: %d\n", ret);
> +
> +       return ret;
> +}
> +
> +static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux,
> +                                      struct drm_dp_aux_msg *msg)
>  {
>         struct ps8640 *ps_bridge = aux_to_ps8640(aux);
>         struct regmap *map = ps_bridge->regmap[PAGE0_DP_CNTL];
> @@ -274,38 +296,49 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
>         return len;
>  }
>
> -static int ps8640_bridge_vdo_control(struct ps8640 *ps_bridge,
> -                                    const enum ps8640_vdo_control ctrl)
> +static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
> +                                  struct drm_dp_aux_msg *msg)
> +{
> +       struct ps8640 *ps_bridge = aux_to_ps8640(aux);
> +       struct device *dev = &ps_bridge->page[PAGE0_DP_CNTL]->dev;
> +       int ret;
> +
> +       pm_runtime_get_sync(dev);
> +       ret = ps8640_ensure_hpd(ps_bridge);
> +       if (!ret)
> +               ret = ps8640_aux_transfer_msg(aux, msg);
> +       pm_runtime_mark_last_busy(dev);
> +       pm_runtime_put_autosuspend(dev);
> +
> +       return ret;
> +}
> +
> +static void ps8640_bridge_vdo_control(struct ps8640 *ps_bridge,
> +                                     const enum ps8640_vdo_control ctrl)
>  {
>         struct regmap *map = ps_bridge->regmap[PAGE3_DSI_CNTL1];
> +       struct device *dev = &ps_bridge->page[PAGE3_DSI_CNTL1]->dev;
>         u8 vdo_ctrl_buf[] = { VDO_CTL_ADD, ctrl };
>         int ret;
>
>         ret = regmap_bulk_write(map, PAGE3_SET_ADD,
>                                 vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
>
> -       if (ret < 0) {
> -               DRM_ERROR("failed to %sable VDO: %d\n",
> -                         ctrl == ENABLE ? "en" : "dis", ret);
> -               return ret;
> -       }
> -
> -       return 0;
> +       if (ret < 0)
> +               dev_err(dev, "failed to %sable VDO: %d\n",
> +                       ctrl == ENABLE ? "en" : "dis", ret);
>  }
>
> -static void ps8640_bridge_poweron(struct ps8640 *ps_bridge)
> +static int __maybe_unused ps8640_resume(struct device *dev)
>  {
> -       struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
> -       int ret, status;
> -
> -       if (ps_bridge->powered)
> -               return;
> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
> +       int ret;
>
>         ret = regulator_bulk_enable(ARRAY_SIZE(ps_bridge->supplies),
>                                     ps_bridge->supplies);
>         if (ret < 0) {
> -               DRM_ERROR("cannot enable regulators %d\n", ret);
> -               return;
> +               dev_err(dev, "cannot enable regulators %d\n", ret);
> +               return ret;
>         }
>
>         gpiod_set_value(ps_bridge->gpio_powerdown, 0);
> @@ -314,86 +347,78 @@ static void ps8640_bridge_poweron(struct ps8640 *ps_bridge)
>         gpiod_set_value(ps_bridge->gpio_reset, 0);
>
>         /*
> -        * Wait for the ps8640 embedded MCU to be ready
> -        * First wait 200ms and then check the MCU ready flag every 20ms
> +        * Mystery 200 ms delay for the "MCU to be ready". It's unclear if
> +        * this is truly necessary since the MCU will already signal that
> +        * things are "good to go" by signaling HPD on "gpio 9". See
> +        * ps8640_ensure_hpd(). For now we'll keep this mystery delay just in
> +        * case.
>          */
>         msleep(200);
>
> -       ret = regmap_read_poll_timeout(map, PAGE2_GPIO_H, status,
> -                                      status & PS_GPIO9, 20 * 1000, 200 * 1000);
> -
> -       if (ret < 0) {
> -               DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", ret);
> -               goto err_regulators_disable;
> -       }
> -
> -       msleep(50);
> -
> -       /*
> -        * The Manufacturer Command Set (MCS) is a device dependent interface
> -        * intended for factory programming of the display module default
> -        * parameters. Once the display module is configured, the MCS shall be
> -        * disabled by the manufacturer. Once disabled, all MCS commands are
> -        * ignored by the display interface.
> -        */
> -
> -       ret = regmap_update_bits(map, PAGE2_MCS_EN, MCS_EN, 0);
> -       if (ret < 0) {
> -               DRM_ERROR("failed write PAGE2_MCS_EN: %d\n", ret);
> -               goto err_regulators_disable;
> -       }
> -
> -       /* Switch access edp panel's edid through i2c */
> -       ret = regmap_write(map, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
> -       if (ret < 0) {
> -               DRM_ERROR("failed write PAGE2_I2C_BYPASS: %d\n", ret);
> -               goto err_regulators_disable;
> -       }
> -
> -       ps_bridge->powered = true;
> -
> -       return;
> -
> -err_regulators_disable:
> -       regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
> -                              ps_bridge->supplies);
> +       return 0;
>  }
>
> -static void ps8640_bridge_poweroff(struct ps8640 *ps_bridge)
> +static int __maybe_unused ps8640_suspend(struct device *dev)
>  {
> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
>         int ret;
>
> -       if (!ps_bridge->powered)
> -               return;
> -
>         gpiod_set_value(ps_bridge->gpio_reset, 1);
>         gpiod_set_value(ps_bridge->gpio_powerdown, 1);
>         ret = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
>                                      ps_bridge->supplies);
>         if (ret < 0)
> -               DRM_ERROR("cannot disable regulators %d\n", ret);
> +               dev_err(dev, "cannot disable regulators %d\n", ret);
>
> -       ps_bridge->powered = false;
> +       return ret;
>  }
>
> +static const struct dev_pm_ops ps8640_pm_ops = {
> +       SET_RUNTIME_PM_OPS(ps8640_suspend, ps8640_resume, NULL)
> +       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +                               pm_runtime_force_resume)
> +};
> +
>  static void ps8640_pre_enable(struct drm_bridge *bridge)
>  {
>         struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> +       struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
> +       struct device *dev = &ps_bridge->page[PAGE0_DP_CNTL]->dev;
>         int ret;
>
> -       ps8640_bridge_poweron(ps_bridge);
> +       pm_runtime_get_sync(dev);
> +       ps8640_ensure_hpd(ps_bridge);
> +
> +       /*
> +        * The Manufacturer Command Set (MCS) is a device dependent interface
> +        * intended for factory programming of the display module default
> +        * parameters. Once the display module is configured, the MCS shall be
> +        * disabled by the manufacturer. Once disabled, all MCS commands are
> +        * ignored by the display interface.
> +        */
> +
> +       ret = regmap_update_bits(map, PAGE2_MCS_EN, MCS_EN, 0);
> +       if (ret < 0)
> +               dev_warn(dev, "failed write PAGE2_MCS_EN: %d\n", ret);
>
> -       ret = ps8640_bridge_vdo_control(ps_bridge, ENABLE);
> +       /* Switch access edp panel's edid through i2c */
> +       ret = regmap_write(map, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
>         if (ret < 0)
> -               ps8640_bridge_poweroff(ps_bridge);
> +               dev_warn(dev, "failed write PAGE2_MCS_EN: %d\n", ret);
> +
> +       ps8640_bridge_vdo_control(ps_bridge, ENABLE);
> +
> +       ps_bridge->pre_enabled = true;
>  }
>
>  static void ps8640_post_disable(struct drm_bridge *bridge)
>  {
>         struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>
> +       ps_bridge->pre_enabled = false;
> +
>         ps8640_bridge_vdo_control(ps_bridge, DISABLE);
> -       ps8640_bridge_poweroff(ps_bridge);
> +       pm_runtime_put_sync_suspend(&ps_bridge->page[PAGE0_DP_CNTL]->dev);
>  }
>
>  static int ps8640_bridge_attach(struct drm_bridge *bridge,
> @@ -426,7 +451,7 @@ static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge,
>                                            struct drm_connector *connector)
>  {
>         struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> -       bool poweroff = !ps_bridge->powered;
> +       bool poweroff = !ps_bridge->pre_enabled;
>         struct edid *edid;
>
>         /*
> @@ -456,6 +481,12 @@ static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge,
>         return edid;
>  }
>
> +static void ps8640_runtime_disable(void *data)
> +{
> +       pm_runtime_dont_use_autosuspend(data);
> +       pm_runtime_disable(data);
> +}
> +
>  static const struct drm_bridge_funcs ps8640_bridge_funcs = {
>         .attach = ps8640_bridge_attach,
>         .detach = ps8640_bridge_detach,
> @@ -586,6 +617,22 @@ static int ps8640_probe(struct i2c_client *client)
>         ps_bridge->aux.transfer = ps8640_aux_transfer;
>         drm_dp_aux_init(&ps_bridge->aux);
>
> +       pm_runtime_enable(dev);
> +       /*
> +        * Powering on ps8640 takes ~300ms. To avoid wasting time on power
> +        * cycling ps8640 too often, set autosuspend_delay to 500ms to ensure
> +        * the bridge wouldn't suspend in between each _aux_transfer_msg() call
> +        * during EDID read (~20ms in my experiment) and in between the last
> +        * _aux_transfer_msg() call during EDID read and the _pre_enable() call
> +        * (~100ms in my experiment).
> +        */
> +       pm_runtime_set_autosuspend_delay(dev, 500);
> +       pm_runtime_use_autosuspend(dev);
> +       pm_suspend_ignore_children(dev, true);
> +       ret = devm_add_action_or_reset(dev, ps8640_runtime_disable, dev);
> +       if (ret)
> +               return ret;
> +
>         drm_bridge_add(&ps_bridge->bridge);
>
>         ret = ps8640_bridge_host_attach(dev, ps_bridge);
> @@ -620,6 +667,7 @@ static struct i2c_driver ps8640_driver = {
>         .driver = {
>                 .name = "ps8640",
>                 .of_match_table = ps8640_match,
> +               .pm = &ps8640_pm_ops,
>         },
>  };
>  module_i2c_driver(ps8640_driver);
> --
> 2.33.0.1079.g6e70778dc9-goog
>

Re: [PATCH v5 1/2] drm/bridge: parade-ps8640: Enable runtime power management

[Doug Anderson]

Hi,

On Thu, Oct 28, 2021 at 11:02 AM Philip Chen <philipchen@chromium.org> wrote:
>
> Add "Sam Ravnborg <sam@ravnborg.org>" to cc list for vis.
> Remove "Andrzej Hajda <a.hajda@samsung.com>" from cc list as the
> address can't be found.

Looking at <https://lore.kernel.org/all/b2fb88db-009e-4b38-dc3d-5ce9163257de@samsung.com/>,
it should be Andrzej Hajda <andrzej.hajda@intel.com>. I've added.


> On Thu, Oct 28, 2021 at 10:58 AM Philip Chen <philipchen@chromium.org> wrote:
> >
> > Fit ps8640 driver into runtime power management framework:
> >
> > First, break _poweron() to 3 parts: (1) turn on power and wait for
> > ps8640's internal MCU to finish init (2) check panel HPD (which is
> > proxied by GPIO9) (3) the other configs. As runtime_resume() can be
> > called before panel is powered, we only add (1) to _resume() and leave
> > (2)(3) to _pre_enable(). We also add (2) to _aux_transfer() as we want
> > to ensure panel HPD is asserted before we start AUX CH transactions.
> >
> > Second, the original driver has a mysterious delay of 50 ms between (2)
> > and (3). Since Parade's support can't explain what the delay is for,
> > and we don't see removing the delay break any boards at hand, remove
> > the delay to fit into this driver change.
> >
> > In addition, rename "powered" to "pre_enabled" and don't check for it
> > in the pm_runtime calls. The pm_runtime calls are already refcounted
> > so there's no reason to check there. The other user of "powered",
> > _get_edid(), only cares if pre_enable() has already been called.
> >
> > Lastly, change some existing DRM_...() logging to dev_...() along the
> > way, since DRM_...() seem to be deprecated in [1].
> >
> > [1] https://patchwork.freedesktop.org/patch/454760/
> >
> > Signed-off-by: Philip Chen <philipchen@chromium.org>
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> > In v3, I added pm_suspend_ignore_children() in ps8640_probe().
> > Also, I moved the change of "put_sync_suspend" from patch 2/2 to here.
> > But I forgot to mention both changes. So edit v3 change log retroactively.
> >
> > In v4, I moved the change of "ps8640_ensure_hpd" return data type
> > from patch 2/2 to here. But I forgot to mention it. So edit v4 change log
> > retroactively.
> >
> > Changes in v5:
> > - Move the implementation of _runtime_disable() around to resolve merge
> >   conflict when rebasing.
> > - Improve the document for how autosuspend_delay is picked.

The new text looks good to me, thanks!

Since this is from @chromium.org and only reviewed-by @chromium.org
people, I'll plan to give it a 2-week snooze to give others ample time
to comment on these two patches. If 2 weeks pass w/ no comments then
I'll land to drm-misc-next. If someone gives an Ack and/or Reviewed-by
then I'll likely land sooner.

-Doug

Re: [PATCH v5 2/2] drm/bridge: parade-ps8640: Populate devices on aux-bus

[Doug Anderson]

Hi,

On Thu, Oct 28, 2021 at 10:58 AM Philip Chen <philipchen@chromium.org> wrote:
>
> Conventionally, panel is listed under the root of the device tree.
> When userland asks for display mode, ps8640 bridge is responsible
> for returning EDID when ps8640_bridge_get_edid() is called.
>
> Now enable a new option of listing panel under "aux-bus" of ps8640
> bridge node in the device tree. In this case, panel driver can retrieve
> EDID by triggering AUX transactions, without ps8640_bridge_get_edid()
> calls at all.
>
> To prevent the "old" and "new" options from interfering with each
> other's logic flow, disable DRM_BRIDGE_OP_EDID when the new option
> is taken.
>
> Signed-off-by: Philip Chen <philipchen@chromium.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
> In v4, I factored out the "ps8640_ensure_hpd" change and added it to patch 1/2
> in this patch series. But I forgot to mention it in v4 change log. Edit v4
> change log retroactively.
>
> In v3, I factored out the "put_sync_suspend" change and added it to patch 1/2
> in this patch series. But I forgot to mention it in v3 change log. Edit v3
> change log retroactively.
>
> (no changes since v4)
>
> Changes in v4:
> - Move the change of "ps8640_ensure_hpd" to patch 1/2 in this patch series.
>
> Changes in v3:
> - Fix when to call of_node_put() in ps8640_of_panel_on_aux_bus()
> - Move the change of "put_sync_suspend" to patch 1/2 in this patch series.
>
> Changes in v2:
> - Add of_node_put() calls in ps8640_of_panel_on_aux_bus()
> - Select DRM_DP_AUX_BUS for PS8640 driver in Kconfig
>
>  drivers/gpu/drm/bridge/Kconfig         |  1 +
>  drivers/gpu/drm/bridge/parade-ps8640.c | 51 ++++++++++++++++++++------
>  2 files changed, 40 insertions(+), 12 deletions(-)

Should have carried my tag from v4, but here it is again:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v5 1/2] drm/bridge: parade-ps8640: Enable runtime power management

[Doug Anderson]

Hi,

On Thu, Oct 28, 2021 at 12:39 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Oct 28, 2021 at 11:02 AM Philip Chen <philipchen@chromium.org> wrote:
> >
> > Add "Sam Ravnborg <sam@ravnborg.org>" to cc list for vis.
> > Remove "Andrzej Hajda <a.hajda@samsung.com>" from cc list as the
> > address can't be found.
>
> Looking at <https://lore.kernel.org/all/b2fb88db-009e-4b38-dc3d-5ce9163257de@samsung.com/>,
> it should be Andrzej Hajda <andrzej.hajda@intel.com>. I've added.
>
>
> > On Thu, Oct 28, 2021 at 10:58 AM Philip Chen <philipchen@chromium.org> wrote:
> > >
> > > Fit ps8640 driver into runtime power management framework:
> > >
> > > First, break _poweron() to 3 parts: (1) turn on power and wait for
> > > ps8640's internal MCU to finish init (2) check panel HPD (which is
> > > proxied by GPIO9) (3) the other configs. As runtime_resume() can be
> > > called before panel is powered, we only add (1) to _resume() and leave
> > > (2)(3) to _pre_enable(). We also add (2) to _aux_transfer() as we want
> > > to ensure panel HPD is asserted before we start AUX CH transactions.
> > >
> > > Second, the original driver has a mysterious delay of 50 ms between (2)
> > > and (3). Since Parade's support can't explain what the delay is for,
> > > and we don't see removing the delay break any boards at hand, remove
> > > the delay to fit into this driver change.
> > >
> > > In addition, rename "powered" to "pre_enabled" and don't check for it
> > > in the pm_runtime calls. The pm_runtime calls are already refcounted
> > > so there's no reason to check there. The other user of "powered",
> > > _get_edid(), only cares if pre_enable() has already been called.
> > >
> > > Lastly, change some existing DRM_...() logging to dev_...() along the
> > > way, since DRM_...() seem to be deprecated in [1].
> > >
> > > [1] https://patchwork.freedesktop.org/patch/454760/
> > >
> > > Signed-off-by: Philip Chen <philipchen@chromium.org>
> > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> > > ---
> > > In v3, I added pm_suspend_ignore_children() in ps8640_probe().
> > > Also, I moved the change of "put_sync_suspend" from patch 2/2 to here.
> > > But I forgot to mention both changes. So edit v3 change log retroactively.
> > >
> > > In v4, I moved the change of "ps8640_ensure_hpd" return data type
> > > from patch 2/2 to here. But I forgot to mention it. So edit v4 change log
> > > retroactively.
> > >
> > > Changes in v5:
> > > - Move the implementation of _runtime_disable() around to resolve merge
> > >   conflict when rebasing.
> > > - Improve the document for how autosuspend_delay is picked.
>
> The new text looks good to me, thanks!
>
> Since this is from @chromium.org and only reviewed-by @chromium.org
> people, I'll plan to give it a 2-week snooze to give others ample time
> to comment on these two patches. If 2 weeks pass w/ no comments then
> I'll land to drm-misc-next. If someone gives an Ack and/or Reviewed-by
> then I'll likely land sooner.

My 2-week snooze went off, so this is now pushed to drm-misc-next
fixing a small whitespace warning that the dim tool complained about.

e9d9f9582c3d drm/bridge: parade-ps8640: Populate devices on aux-bus
826cff3f7ebb drm/bridge: parade-ps8640: Enable runtime power management

-Doug

Re: [PATCH v5 1/2] drm/bridge: parade-ps8640: Enable runtime power management

[Doug Anderson]

Hi,

On Thu, Oct 28, 2021 at 12:39 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Oct 28, 2021 at 11:02 AM Philip Chen <philipchen@chromium.org> wrote:
> >
> > Add "Sam Ravnborg <sam@ravnborg.org>" to cc list for vis.
> > Remove "Andrzej Hajda <a.hajda@samsung.com>" from cc list as the
> > address can't be found.
>
> Looking at <https://lore.kernel.org/all/b2fb88db-009e-4b38-dc3d-5ce9163257de@samsung.com/>,
> it should be Andrzej Hajda <andrzej.hajda@intel.com>. I've added.
>
>
> > On Thu, Oct 28, 2021 at 10:58 AM Philip Chen <philipchen@chromium.org> wrote:
> > >
> > > Fit ps8640 driver into runtime power management framework:
> > >
> > > First, break _poweron() to 3 parts: (1) turn on power and wait for
> > > ps8640's internal MCU to finish init (2) check panel HPD (which is
> > > proxied by GPIO9) (3) the other configs. As runtime_resume() can be
> > > called before panel is powered, we only add (1) to _resume() and leave
> > > (2)(3) to _pre_enable(). We also add (2) to _aux_transfer() as we want
> > > to ensure panel HPD is asserted before we start AUX CH transactions.
> > >
> > > Second, the original driver has a mysterious delay of 50 ms between (2)
> > > and (3). Since Parade's support can't explain what the delay is for,
> > > and we don't see removing the delay break any boards at hand, remove
> > > the delay to fit into this driver change.
> > >
> > > In addition, rename "powered" to "pre_enabled" and don't check for it
> > > in the pm_runtime calls. The pm_runtime calls are already refcounted
> > > so there's no reason to check there. The other user of "powered",
> > > _get_edid(), only cares if pre_enable() has already been called.
> > >
> > > Lastly, change some existing DRM_...() logging to dev_...() along the
> > > way, since DRM_...() seem to be deprecated in [1].
> > >
> > > [1] https://patchwork.freedesktop.org/patch/454760/
> > >
> > > Signed-off-by: Philip Chen <philipchen@chromium.org>
> > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> > > ---
> > > In v3, I added pm_suspend_ignore_children() in ps8640_probe().
> > > Also, I moved the change of "put_sync_suspend" from patch 2/2 to here.
> > > But I forgot to mention both changes. So edit v3 change log retroactively.
> > >
> > > In v4, I moved the change of "ps8640_ensure_hpd" return data type
> > > from patch 2/2 to here. But I forgot to mention it. So edit v4 change log
> > > retroactively.
> > >
> > > Changes in v5:
> > > - Move the implementation of _runtime_disable() around to resolve merge
> > >   conflict when rebasing.
> > > - Improve the document for how autosuspend_delay is picked.
>
> The new text looks good to me, thanks!
>
> Since this is from @chromium.org and only reviewed-by @chromium.org
> people, I'll plan to give it a 2-week snooze to give others ample time
> to comment on these two patches. If 2 weeks pass w/ no comments then
> I'll land to drm-misc-next. If someone gives an Ack and/or Reviewed-by
> then I'll likely land sooner.

My 2-week snooze went off, so this is now pushed to drm-misc-next
fixing a small whitespace warning that the dim tool complained about.

e9d9f9582c3d drm/bridge: parade-ps8640: Populate devices on aux-bus
826cff3f7ebb drm/bridge: parade-ps8640: Enable runtime power management

-Doug