| From 57c8b030bf429ee3d7ed2863a42eed13cd31d6bc Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Thu, 18 May 2023 15:39:02 -0400 |
| Subject: drm/bridge: anx7625: Prevent endless probe loop |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: NĂcolas F. R. A. Prado <nfraprado@collabora.com> |
| |
| [ Upstream commit 1464e48d69ab7a50a377c9d39f5e5eb3cee2722e ] |
| |
| During probe, the driver registers i2c dummy devices and populates the |
| aux bus, which registers a device for the panel. After doing that, the |
| driver can still defer probe if needed. This ordering of operations is |
| troublesome however, because the deferred probe work will retry probing |
| all pending devices every time a new device is registered. Therefore, if |
| modules need to be loaded in order to satisfy the dependencies for this |
| driver to complete probe, the kernel will stall, since it'll keep trying |
| to probe the anx7625 driver, but never succeed, given that modules would |
| only be loaded after the deferred probe work completes. |
| |
| Two changes are required to avoid this issue: |
| * Move of_find_mipi_dsi_host_by_node(), which can defer probe, to before |
| anx7625_register_i2c_dummy_clients() and |
| devm_of_dp_aux_populate_ep_devices(), which register devices. |
| * Make use of the done_probing callback when populating the aux bus, |
| so that the bridge registration is only done after the panel is |
| probed. This is required because the panel might need to defer probe, |
| but the aux bus population needs the i2c dummy devices working, so |
| this call couldn't just be moved to an earlier point in probe. |
| One caveat is that if the panel is described outside the aux bus, the |
| probe loop issue can still happen, but we don't have a way to avoid |
| it in that case since there's no callback available. |
| |
| With this patch applied, it's possible to boot on |
| mt8192-asurada-spherion with |
| |
| CONFIG_DRM_ANALOGIX_ANX7625=y |
| CONFIG_MTK_MMSYS=m |
| CONFIG_BACKLIGHT_PWM=y |
| |
| and also with |
| |
| CONFIG_DRM_ANALOGIX_ANX7625=y |
| CONFIG_MTK_MMSYS=y |
| CONFIG_BACKLIGHT_PWM=m |
| |
| Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid through aux channel") |
| Fixes: 269332997a16 ("drm/bridge: anx7625: Return -EPROBE_DEFER if the dsi host was not found") |
| Reported-by: "kernelci.org bot" <bot@kernelci.org> |
| Signed-off-by: NĂcolas F. R. A. Prado <nfraprado@collabora.com> |
| Reviewed-by: Robert Foss <rfoss@kernel.org> |
| Signed-off-by: Robert Foss <rfoss@kernel.org> |
| Link: https://patchwork.freedesktop.org/patch/msgid/20230518193902.891121-1-nfraprado@collabora.com |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/gpu/drm/bridge/analogix/anx7625.c | 128 +++++++++++++++------- |
| 1 file changed, 88 insertions(+), 40 deletions(-) |
| |
| diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c |
| index 6846199a2ee14..9e387c3e9b696 100644 |
| --- a/drivers/gpu/drm/bridge/analogix/anx7625.c |
| +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c |
| @@ -1687,6 +1687,14 @@ static int anx7625_parse_dt(struct device *dev, |
| if (of_property_read_bool(np, "analogix,audio-enable")) |
| pdata->audio_en = 1; |
| |
| + return 0; |
| +} |
| + |
| +static int anx7625_parse_dt_panel(struct device *dev, |
| + struct anx7625_platform_data *pdata) |
| +{ |
| + struct device_node *np = dev->of_node; |
| + |
| pdata->panel_bridge = devm_drm_of_get_bridge(dev, np, 1, 0); |
| if (IS_ERR(pdata->panel_bridge)) { |
| if (PTR_ERR(pdata->panel_bridge) == -ENODEV) { |
| @@ -2032,7 +2040,7 @@ static int anx7625_register_audio(struct device *dev, struct anx7625_data *ctx) |
| return 0; |
| } |
| |
| -static int anx7625_attach_dsi(struct anx7625_data *ctx) |
| +static int anx7625_setup_dsi_device(struct anx7625_data *ctx) |
| { |
| struct mipi_dsi_device *dsi; |
| struct device *dev = &ctx->client->dev; |
| @@ -2042,9 +2050,6 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx) |
| .channel = 0, |
| .node = NULL, |
| }; |
| - int ret; |
| - |
| - DRM_DEV_DEBUG_DRIVER(dev, "attach dsi\n"); |
| |
| host = of_find_mipi_dsi_host_by_node(ctx->pdata.mipi_host_node); |
| if (!host) { |
| @@ -2065,14 +2070,24 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx) |
| MIPI_DSI_MODE_VIDEO_HSE | |
| MIPI_DSI_HS_PKT_END_ALIGNED; |
| |
| - ret = devm_mipi_dsi_attach(dev, dsi); |
| + ctx->dsi = dsi; |
| + |
| + return 0; |
| +} |
| + |
| +static int anx7625_attach_dsi(struct anx7625_data *ctx) |
| +{ |
| + struct device *dev = &ctx->client->dev; |
| + int ret; |
| + |
| + DRM_DEV_DEBUG_DRIVER(dev, "attach dsi\n"); |
| + |
| + ret = devm_mipi_dsi_attach(dev, ctx->dsi); |
| if (ret) { |
| DRM_DEV_ERROR(dev, "fail to attach dsi to host.\n"); |
| return ret; |
| } |
| |
| - ctx->dsi = dsi; |
| - |
| DRM_DEV_DEBUG_DRIVER(dev, "attach dsi succeeded.\n"); |
| |
| return 0; |
| @@ -2560,6 +2575,40 @@ static void anx7625_runtime_disable(void *data) |
| pm_runtime_disable(data); |
| } |
| |
| +static int anx7625_link_bridge(struct drm_dp_aux *aux) |
| +{ |
| + struct anx7625_data *platform = container_of(aux, struct anx7625_data, aux); |
| + struct device *dev = aux->dev; |
| + int ret; |
| + |
| + ret = anx7625_parse_dt_panel(dev, &platform->pdata); |
| + if (ret) { |
| + DRM_DEV_ERROR(dev, "fail to parse DT for panel : %d\n", ret); |
| + return ret; |
| + } |
| + |
| + platform->bridge.funcs = &anx7625_bridge_funcs; |
| + platform->bridge.of_node = dev->of_node; |
| + if (!anx7625_of_panel_on_aux_bus(dev)) |
| + platform->bridge.ops |= DRM_BRIDGE_OP_EDID; |
| + if (!platform->pdata.panel_bridge) |
| + platform->bridge.ops |= DRM_BRIDGE_OP_HPD | |
| + DRM_BRIDGE_OP_DETECT; |
| + platform->bridge.type = platform->pdata.panel_bridge ? |
| + DRM_MODE_CONNECTOR_eDP : |
| + DRM_MODE_CONNECTOR_DisplayPort; |
| + |
| + drm_bridge_add(&platform->bridge); |
| + |
| + if (!platform->pdata.is_dpi) { |
| + ret = anx7625_attach_dsi(platform); |
| + if (ret) |
| + drm_bridge_remove(&platform->bridge); |
| + } |
| + |
| + return ret; |
| +} |
| + |
| static int anx7625_i2c_probe(struct i2c_client *client) |
| { |
| struct anx7625_data *platform; |
| @@ -2634,6 +2683,24 @@ static int anx7625_i2c_probe(struct i2c_client *client) |
| platform->aux.wait_hpd_asserted = anx7625_wait_hpd_asserted; |
| drm_dp_aux_init(&platform->aux); |
| |
| + ret = anx7625_parse_dt(dev, pdata); |
| + if (ret) { |
| + if (ret != -EPROBE_DEFER) |
| + DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret); |
| + goto free_wq; |
| + } |
| + |
| + if (!platform->pdata.is_dpi) { |
| + ret = anx7625_setup_dsi_device(platform); |
| + if (ret < 0) |
| + goto free_wq; |
| + } |
| + |
| + /* |
| + * Registering the i2c devices will retrigger deferred probe, so it |
| + * needs to be done after calls that might return EPROBE_DEFER, |
| + * otherwise we can get an infinite loop. |
| + */ |
| if (anx7625_register_i2c_dummy_clients(platform, client) != 0) { |
| ret = -ENOMEM; |
| DRM_DEV_ERROR(dev, "fail to reserve I2C bus.\n"); |
| @@ -2648,13 +2715,21 @@ static int anx7625_i2c_probe(struct i2c_client *client) |
| if (ret) |
| goto free_wq; |
| |
| - devm_of_dp_aux_populate_ep_devices(&platform->aux); |
| - |
| - ret = anx7625_parse_dt(dev, pdata); |
| + /* |
| + * Populating the aux bus will retrigger deferred probe, so it needs to |
| + * be done after calls that might return EPROBE_DEFER, otherwise we can |
| + * get an infinite loop. |
| + */ |
| + ret = devm_of_dp_aux_populate_bus(&platform->aux, anx7625_link_bridge); |
| if (ret) { |
| - if (ret != -EPROBE_DEFER) |
| - DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret); |
| - goto free_wq; |
| + if (ret != -ENODEV) { |
| + DRM_DEV_ERROR(dev, "failed to populate aux bus : %d\n", ret); |
| + goto free_wq; |
| + } |
| + |
| + ret = anx7625_link_bridge(&platform->aux); |
| + if (ret) |
| + goto free_wq; |
| } |
| |
| if (!platform->pdata.low_power_mode) { |
| @@ -2667,27 +2742,6 @@ static int anx7625_i2c_probe(struct i2c_client *client) |
| if (platform->pdata.intp_irq) |
| queue_work(platform->workqueue, &platform->work); |
| |
| - platform->bridge.funcs = &anx7625_bridge_funcs; |
| - platform->bridge.of_node = client->dev.of_node; |
| - if (!anx7625_of_panel_on_aux_bus(&client->dev)) |
| - platform->bridge.ops |= DRM_BRIDGE_OP_EDID; |
| - if (!platform->pdata.panel_bridge) |
| - platform->bridge.ops |= DRM_BRIDGE_OP_HPD | |
| - DRM_BRIDGE_OP_DETECT; |
| - platform->bridge.type = platform->pdata.panel_bridge ? |
| - DRM_MODE_CONNECTOR_eDP : |
| - DRM_MODE_CONNECTOR_DisplayPort; |
| - |
| - drm_bridge_add(&platform->bridge); |
| - |
| - if (!platform->pdata.is_dpi) { |
| - ret = anx7625_attach_dsi(platform); |
| - if (ret) { |
| - DRM_DEV_ERROR(dev, "Fail to attach to dsi : %d\n", ret); |
| - goto unregister_bridge; |
| - } |
| - } |
| - |
| if (platform->pdata.audio_en) |
| anx7625_register_audio(dev, platform); |
| |
| @@ -2695,12 +2749,6 @@ static int anx7625_i2c_probe(struct i2c_client *client) |
| |
| return 0; |
| |
| -unregister_bridge: |
| - drm_bridge_remove(&platform->bridge); |
| - |
| - if (!platform->pdata.low_power_mode) |
| - pm_runtime_put_sync_suspend(&client->dev); |
| - |
| free_wq: |
| if (platform->workqueue) |
| destroy_workqueue(platform->workqueue); |
| -- |
| 2.39.2 |
| |