Hi Rob,
Thank you for the patch.
On Sun, Jun 30, 2019 at 08:01:43AM -0700, Rob Clark wrote:
From: Rob Clark robdclark@chromium.org
Request the enable gpio ASIS to avoid disabling bridge during probe, if already enabled. And if already enabled, defer enabling runpm until attach to avoid cutting off the power to the bridge.
Once we get to attach, we know panel and drm driver are probed successfully, so at this point it i s safe to enable runpm and reset the bridge. If we do it earlier, we kill efifb (in the case that panel or drm driver do not probe successfully, giving the user no way to see what is going on.
Signed-off-by: Rob Clark robdclark@chromium.org
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 7a046bcdd81b..8bdc33576992 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -257,6 +257,12 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge) .node = NULL, };
- if (gpiod_get_value(pdata->enable_gpio)) {
pm_runtime_enable(pdata->dev);
Does this need to be balanced with a pm_runtime_disable() call ? Bridges can be attached and detached at runtime when reloading the display controller drivers, so you need to ensure that detach/re-attach cycles work.
ti_sn_bridge_resume(pdata->dev);ti_sn_bridge_suspend(pdata->dev);- }
- ret = drm_connector_init(bridge->dev, &pdata->connector, &ti_sn_bridge_connector_funcs, DRM_MODE_CONNECTOR_eDP);
@@ -813,7 +819,7 @@ static int ti_sn_bridge_probe(struct i2c_client *client, dev_set_drvdata(&client->dev, pdata); pdata->enable_gpio = devm_gpiod_get(pdata->dev, "enable",
GPIOD_OUT_LOW);
GPIOD_ASIS);@@ -843,7 +849,9 @@ static int ti_sn_bridge_probe(struct i2c_client *client, if (ret) return ret;
- pm_runtime_enable(pdata->dev);
- if (!gpiod_get_value(pdata->enable_gpio)) {
pm_runtime_enable(pdata->dev);- }
If I understand the issue correctly, this is part of an effort to avoid disabling a potentially display output until we get as close as possible to display handover, right ? Is there a drawback in always enabling runtime PM when the bridge is attached instead of at probe time ? I think we need to come up with a set of rules for bridge driver authors, otherwise we'll end up with incompatible expectations of bridge drivers and display controller drivers.
i2c_set_clientdata(client, pdata);