While it works, for the most part, to assume that the panel has finished probing when devm_of_dp_aux_populate_ep_devices() returns, it's a bit fragile. This is talked about at length in commit a1e3667a9835 ("drm/bridge: ti-sn65dsi86: Promote the AUX channel to its own sub-dev").
When reviewing the ps8640 code, I managed to convince myself that it was OK not to worry about it there and that maybe it wasn't really _that_ fragile. However, it turns out that it really is. Simply hardcoding panel_edp_probe() to return -EPROBE_DEFER was enough to put the boot process into an infinite loop. I believe this manages to trip the same issues that we used to trip with the main MSM code where something about our actions trigger Linux to re-probe previously deferred devices right away and each time we try again we re-trigger Linux to re-probe.
It's really not that crazy to just break the probe up. Let's use the new helpers introduced in the patch ("drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly") to break the driver into two.
With this change, the device still boots (though obviously the panel doesn't come up) if I force panel-edp to always return -EPROBE_DEFER.
Signed-off-by: Douglas Anderson dianders@chromium.org ---
drivers/gpu/drm/bridge/parade-ps8640.c | 66 +++++++++++++++----------- 1 file changed, 39 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c index 9766cbbd62ad..96e883985608 100644 --- a/drivers/gpu/drm/bridge/parade-ps8640.c +++ b/drivers/gpu/drm/bridge/parade-ps8640.c @@ -93,6 +93,7 @@ enum ps8640_vdo_control { };
struct ps8640 { + struct dp_aux_ep_client ep_client; struct drm_bridge bridge; struct drm_bridge *panel_bridge; struct drm_dp_aux aux; @@ -584,10 +585,36 @@ static int ps8640_bridge_host_attach(struct device *dev, struct ps8640 *ps_bridg return 0; }
+static int ps8640_bridge_probe(struct device *clientdev, struct dp_aux_ep_client *client) +{ + struct ps8640 *ps_bridge = container_of(client, struct ps8640, ep_client); + struct device_node *np = clientdev->of_node; + int ret; + + /* port@1 is ps8640 output port */ + ps_bridge->panel_bridge = devm_drm_of_get_bridge(clientdev, np, 1, 0); + 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(clientdev, ps_bridge); + if (ret) + drm_bridge_remove(&ps_bridge->bridge); + + return ret; +} + +static void ps8640_bridge_remove(struct device *clientdev, struct dp_aux_ep_client *client) +{ + struct ps8640 *ps_bridge = container_of(client, struct ps8640, ep_client); + + drm_bridge_remove(&ps_bridge->bridge); +} + static int ps8640_probe(struct i2c_client *client) { struct device *dev = &client->dev; - struct device_node *np = dev->of_node; struct ps8640 *ps_bridge; int ret; u32 i; @@ -672,31 +699,17 @@ static int ps8640_probe(struct i2c_client *client)
devm_of_dp_aux_populate_ep_devices(&ps_bridge->aux);
- /* port@1 is ps8640 output port */ - ps_bridge->panel_bridge = devm_drm_of_get_bridge(dev, np, 1, 0); - 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); - if (ret) - goto err_bridge_remove; - - return 0; - -err_bridge_remove: - drm_bridge_remove(&ps_bridge->bridge); - return ret; -} - -static int ps8640_remove(struct i2c_client *client) -{ - struct ps8640 *ps_bridge = i2c_get_clientdata(client); - - drm_bridge_remove(&ps_bridge->bridge); - - return 0; + /* + * Create a sub-device and kick off a probe to it. This effectively + * breaks our probe in two and lets the first half complete even if + * the second half might return -EPROBE_DEFER. If we didn't do this + * then if a panel isn't immediately ready we'd delete it right away + * and never give it a chance to finish. + */ + ps_bridge->ep_client.probe = ps8640_bridge_probe; + ps_bridge->ep_client.remove = ps8640_bridge_remove; + ps_bridge->ep_client.aux = &ps_bridge->aux; + return devm_dp_aux_register_ep_client(&ps_bridge->ep_client); }
static const struct of_device_id ps8640_match[] = { @@ -707,7 +720,6 @@ MODULE_DEVICE_TABLE(of, ps8640_match);
static struct i2c_driver ps8640_driver = { .probe_new = ps8640_probe, - .remove = ps8640_remove, .driver = { .name = "ps8640", .of_match_table = ps8640_match,