Quoting Philip Chen (2021-10-26 12:11:09)
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
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.
drivers/gpu/drm/bridge/parade-ps8640.c | 186 +++++++++++++++---------- 1 file changed, 115 insertions(+), 71 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c index 3aaa90913bf8..ac42a3473770 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 void ps8640_ensure_hpd(struct ps8640 *ps_bridge)
static int ps8640_ensure_hpd?
+{
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];
Otherwise
Reviewed-by: Stephen Boyd swboyd@chromium.org