DRM bridge drivers are now attaching their DSI device at probe time, which requires us to register our DSI host in order to let the bridge to probe: this recently started producing an endless -EPROBE_DEFER loop on some machines that are using external bridges, like the parade-ps8640, found on the ACER Chromebook R13.
Now that the DSI hosts/devices probe sequence is documented, we can do adjustments to the mtk_dsi driver as to both fix now and make sure to avoid this situation in the future: for this, following what is documented in drm_bridge.c, move the mtk_dsi component_add() to the mtk_dsi_ops.attach callback and delete it in the detach callback; keeping in mind that we are registering a drm_bridge for our DSI, which is only used/attached if the DSI Host is bound, it wouldn't make sense to keep adding our bridge at probe time (as it would be useless to have it if mtk_dsi_ops.attach() fails!), so also move that one to the dsi host attach function (and remove it in detach).
Signed-off-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com Reviewed-by: Andrzej Hajda andrzej.hajda@intel.com --- drivers/gpu/drm/mediatek/mtk_dsi.c | 167 +++++++++++++++-------------- 1 file changed, 84 insertions(+), 83 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 5d90d2eb0019..bced4c7d668e 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -786,18 +786,101 @@ void mtk_dsi_ddp_stop(struct device *dev) mtk_dsi_poweroff(dsi); }
+static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi) +{ + int ret; + + ret = drm_simple_encoder_init(drm, &dsi->encoder, + DRM_MODE_ENCODER_DSI); + if (ret) { + DRM_ERROR("Failed to encoder init to drm\n"); + return ret; + } + + dsi->encoder.possible_crtcs = mtk_drm_find_possible_crtc_by_comp(drm, dsi->host.dev); + + ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, + DRM_BRIDGE_ATTACH_NO_CONNECTOR); + if (ret) + goto err_cleanup_encoder; + + dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder); + if (IS_ERR(dsi->connector)) { + DRM_ERROR("Unable to create bridge connector\n"); + ret = PTR_ERR(dsi->connector); + goto err_cleanup_encoder; + } + drm_connector_attach_encoder(dsi->connector, &dsi->encoder); + + return 0; + +err_cleanup_encoder: + drm_encoder_cleanup(&dsi->encoder); + return ret; +} + +static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) +{ + int ret; + struct drm_device *drm = data; + struct mtk_dsi *dsi = dev_get_drvdata(dev); + + ret = mtk_dsi_encoder_init(drm, dsi); + if (ret) + return ret; + + return device_reset_optional(dev); +} + +static void mtk_dsi_unbind(struct device *dev, struct device *master, + void *data) +{ + struct mtk_dsi *dsi = dev_get_drvdata(dev); + + drm_encoder_cleanup(&dsi->encoder); +} + +static const struct component_ops mtk_dsi_component_ops = { + .bind = mtk_dsi_bind, + .unbind = mtk_dsi_unbind, +}; + static int mtk_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *device) { struct mtk_dsi *dsi = host_to_dsi(host); + struct device *dev = host->dev; + int ret;
dsi->lanes = device->lanes; dsi->format = device->format; dsi->mode_flags = device->mode_flags; + dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0); + if (IS_ERR(dsi->next_bridge)) + return PTR_ERR(dsi->next_bridge); + + drm_bridge_add(&dsi->bridge); + + ret = component_add(host->dev, &mtk_dsi_component_ops); + if (ret) { + DRM_ERROR("failed to add dsi_host component: %d\n", ret); + drm_bridge_remove(&dsi->bridge); + return ret; + }
return 0; }
+static int mtk_dsi_host_detach(struct mipi_dsi_host *host, + struct mipi_dsi_device *device) +{ + struct mtk_dsi *dsi = host_to_dsi(host); + + component_del(host->dev, &mtk_dsi_component_ops); + drm_bridge_remove(&dsi->bridge); + return 0; +} + static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi) { int ret; @@ -938,73 +1021,14 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
static const struct mipi_dsi_host_ops mtk_dsi_ops = { .attach = mtk_dsi_host_attach, + .detach = mtk_dsi_host_detach, .transfer = mtk_dsi_host_transfer, };
-static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi) -{ - int ret; - - ret = drm_simple_encoder_init(drm, &dsi->encoder, - DRM_MODE_ENCODER_DSI); - if (ret) { - DRM_ERROR("Failed to encoder init to drm\n"); - return ret; - } - - dsi->encoder.possible_crtcs = mtk_drm_find_possible_crtc_by_comp(drm, dsi->host.dev); - - ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, - DRM_BRIDGE_ATTACH_NO_CONNECTOR); - if (ret) - goto err_cleanup_encoder; - - dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder); - if (IS_ERR(dsi->connector)) { - DRM_ERROR("Unable to create bridge connector\n"); - ret = PTR_ERR(dsi->connector); - goto err_cleanup_encoder; - } - drm_connector_attach_encoder(dsi->connector, &dsi->encoder); - - return 0; - -err_cleanup_encoder: - drm_encoder_cleanup(&dsi->encoder); - return ret; -} - -static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) -{ - int ret; - struct drm_device *drm = data; - struct mtk_dsi *dsi = dev_get_drvdata(dev); - - ret = mtk_dsi_encoder_init(drm, dsi); - if (ret) - return ret; - - return device_reset_optional(dev); -} - -static void mtk_dsi_unbind(struct device *dev, struct device *master, - void *data) -{ - struct mtk_dsi *dsi = dev_get_drvdata(dev); - - drm_encoder_cleanup(&dsi->encoder); -} - -static const struct component_ops mtk_dsi_component_ops = { - .bind = mtk_dsi_bind, - .unbind = mtk_dsi_unbind, -}; - static int mtk_dsi_probe(struct platform_device *pdev) { struct mtk_dsi *dsi; struct device *dev = &pdev->dev; - struct drm_panel *panel; struct resource *regs; int irq_num; int ret; @@ -1021,19 +1045,6 @@ static int mtk_dsi_probe(struct platform_device *pdev) return ret; }
- ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, - &panel, &dsi->next_bridge); - if (ret) - goto err_unregister_host; - - if (panel) { - dsi->next_bridge = devm_drm_panel_bridge_add(dev, panel); - if (IS_ERR(dsi->next_bridge)) { - ret = PTR_ERR(dsi->next_bridge); - goto err_unregister_host; - } - } - dsi->driver_data = of_device_get_match_data(dev);
dsi->engine_clk = devm_clk_get(dev, "engine"); @@ -1098,14 +1109,6 @@ static int mtk_dsi_probe(struct platform_device *pdev) dsi->bridge.of_node = dev->of_node; dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
- drm_bridge_add(&dsi->bridge); - - ret = component_add(&pdev->dev, &mtk_dsi_component_ops); - if (ret) { - dev_err(&pdev->dev, "failed to add component: %d\n", ret); - goto err_unregister_host; - } - return 0;
err_unregister_host: @@ -1118,8 +1121,6 @@ static int mtk_dsi_remove(struct platform_device *pdev) struct mtk_dsi *dsi = platform_get_drvdata(pdev);
mtk_output_dsi_disable(dsi); - drm_bridge_remove(&dsi->bridge); - component_del(&pdev->dev, &mtk_dsi_component_ops); mipi_dsi_host_unregister(&dsi->host);
return 0;
Hi, Angelo:
On Tue, 2022-01-04 at 10:59 +0100, AngeloGioacchino Del Regno wrote:
DRM bridge drivers are now attaching their DSI device at probe time, which requires us to register our DSI host in order to let the bridge to probe: this recently started producing an endless -EPROBE_DEFER loop on some machines that are using external bridges, like the parade-ps8640, found on the ACER Chromebook R13.
Now that the DSI hosts/devices probe sequence is documented, we can do adjustments to the mtk_dsi driver as to both fix now and make sure to avoid this situation in the future: for this, following what is documented in drm_bridge.c, move the mtk_dsi component_add() to the mtk_dsi_ops.attach callback and delete it in the detach callback; keeping in mind that we are registering a drm_bridge for our DSI, which is only used/attached if the DSI Host is bound, it wouldn't make sense to keep adding our bridge at probe time (as it would be useless to have it if mtk_dsi_ops.attach() fails!), so also move that one to the dsi host attach function (and remove it in detach).
This patch looks good to me, but please add 'Fixes' tag to note to which patch this patch want to fix.
Regards, CK
Signed-off-by: AngeloGioacchino Del Regno < angelogioacchino.delregno@collabora.com> Reviewed-by: Andrzej Hajda andrzej.hajda@intel.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 167 +++++++++++++++----------
1 file changed, 84 insertions(+), 83 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 5d90d2eb0019..bced4c7d668e 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -786,18 +786,101 @@ void mtk_dsi_ddp_stop(struct device *dev) mtk_dsi_poweroff(dsi); }
+static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi) +{
- int ret;
- ret = drm_simple_encoder_init(drm, &dsi->encoder,
DRM_MODE_ENCODER_DSI);- if (ret) {
DRM_ERROR("Failed to encoder init to drm\n");return ret;- }
- dsi->encoder.possible_crtcs =
mtk_drm_find_possible_crtc_by_comp(drm, dsi->host.dev);
- ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);- if (ret)
goto err_cleanup_encoder;- dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder);
- if (IS_ERR(dsi->connector)) {
DRM_ERROR("Unable to create bridge connector\n");ret = PTR_ERR(dsi->connector);goto err_cleanup_encoder;- }
- drm_connector_attach_encoder(dsi->connector, &dsi->encoder);
- return 0;
+err_cleanup_encoder:
- drm_encoder_cleanup(&dsi->encoder);
- return ret;
+}
+static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) +{
- int ret;
- struct drm_device *drm = data;
- struct mtk_dsi *dsi = dev_get_drvdata(dev);
- ret = mtk_dsi_encoder_init(drm, dsi);
- if (ret)
return ret;- return device_reset_optional(dev);
+}
+static void mtk_dsi_unbind(struct device *dev, struct device *master,
void *data)+{
- struct mtk_dsi *dsi = dev_get_drvdata(dev);
- drm_encoder_cleanup(&dsi->encoder);
+}
+static const struct component_ops mtk_dsi_component_ops = {
- .bind = mtk_dsi_bind,
- .unbind = mtk_dsi_unbind,
+};
static int mtk_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *device) { struct mtk_dsi *dsi = host_to_dsi(host);
struct device *dev = host->dev;
int ret;
dsi->lanes = device->lanes; dsi->format = device->format; dsi->mode_flags = device->mode_flags;
dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0,
0);
- if (IS_ERR(dsi->next_bridge))
return PTR_ERR(dsi->next_bridge);- drm_bridge_add(&dsi->bridge);
- ret = component_add(host->dev, &mtk_dsi_component_ops);
- if (ret) {
DRM_ERROR("failed to add dsi_host component: %d\n",ret);
drm_bridge_remove(&dsi->bridge);return ret;}
return 0;
}
+static int mtk_dsi_host_detach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)+{
- struct mtk_dsi *dsi = host_to_dsi(host);
- component_del(host->dev, &mtk_dsi_component_ops);
- drm_bridge_remove(&dsi->bridge);
- return 0;
+}
static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi) { int ret; @@ -938,73 +1021,14 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
static const struct mipi_dsi_host_ops mtk_dsi_ops = { .attach = mtk_dsi_host_attach,
- .detach = mtk_dsi_host_detach, .transfer = mtk_dsi_host_transfer,
};
-static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi) -{
- int ret;
- ret = drm_simple_encoder_init(drm, &dsi->encoder,
DRM_MODE_ENCODER_DSI);- if (ret) {
DRM_ERROR("Failed to encoder init to drm\n");return ret;- }
- dsi->encoder.possible_crtcs =
mtk_drm_find_possible_crtc_by_comp(drm, dsi->host.dev);
- ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);- if (ret)
goto err_cleanup_encoder;- dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder);
- if (IS_ERR(dsi->connector)) {
DRM_ERROR("Unable to create bridge connector\n");ret = PTR_ERR(dsi->connector);goto err_cleanup_encoder;- }
- drm_connector_attach_encoder(dsi->connector, &dsi->encoder);
- return 0;
-err_cleanup_encoder:
- drm_encoder_cleanup(&dsi->encoder);
- return ret;
-}
-static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) -{
- int ret;
- struct drm_device *drm = data;
- struct mtk_dsi *dsi = dev_get_drvdata(dev);
- ret = mtk_dsi_encoder_init(drm, dsi);
- if (ret)
return ret;- return device_reset_optional(dev);
-}
-static void mtk_dsi_unbind(struct device *dev, struct device *master,
void *data)-{
- struct mtk_dsi *dsi = dev_get_drvdata(dev);
- drm_encoder_cleanup(&dsi->encoder);
-}
-static const struct component_ops mtk_dsi_component_ops = {
- .bind = mtk_dsi_bind,
- .unbind = mtk_dsi_unbind,
-};
static int mtk_dsi_probe(struct platform_device *pdev) { struct mtk_dsi *dsi; struct device *dev = &pdev->dev;
- struct drm_panel *panel; struct resource *regs; int irq_num; int ret;
@@ -1021,19 +1045,6 @@ static int mtk_dsi_probe(struct platform_device *pdev) return ret; }
- ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
&panel, &dsi->next_bridge);- if (ret)
goto err_unregister_host;- if (panel) {
dsi->next_bridge = devm_drm_panel_bridge_add(dev,panel);
if (IS_ERR(dsi->next_bridge)) {ret = PTR_ERR(dsi->next_bridge);goto err_unregister_host;}}
dsi->driver_data = of_device_get_match_data(dev);
dsi->engine_clk = devm_clk_get(dev, "engine");
@@ -1098,14 +1109,6 @@ static int mtk_dsi_probe(struct platform_device *pdev) dsi->bridge.of_node = dev->of_node; dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
- drm_bridge_add(&dsi->bridge);
- ret = component_add(&pdev->dev, &mtk_dsi_component_ops);
- if (ret) {
dev_err(&pdev->dev, "failed to add component: %d\n",ret);
goto err_unregister_host;- }
- return 0;
err_unregister_host: @@ -1118,8 +1121,6 @@ static int mtk_dsi_remove(struct platform_device *pdev) struct mtk_dsi *dsi = platform_get_drvdata(pdev);
mtk_output_dsi_disable(dsi);
drm_bridge_remove(&dsi->bridge);
component_del(&pdev->dev, &mtk_dsi_component_ops); mipi_dsi_host_unregister(&dsi->host);
return 0;
Il 06/01/22 06:22, CK Hu ha scritto:
Hi, Angelo:
On Tue, 2022-01-04 at 10:59 +0100, AngeloGioacchino Del Regno wrote:
DRM bridge drivers are now attaching their DSI device at probe time, which requires us to register our DSI host in order to let the bridge to probe: this recently started producing an endless -EPROBE_DEFER loop on some machines that are using external bridges, like the parade-ps8640, found on the ACER Chromebook R13.
Now that the DSI hosts/devices probe sequence is documented, we can do adjustments to the mtk_dsi driver as to both fix now and make sure to avoid this situation in the future: for this, following what is documented in drm_bridge.c, move the mtk_dsi component_add() to the mtk_dsi_ops.attach callback and delete it in the detach callback; keeping in mind that we are registering a drm_bridge for our DSI, which is only used/attached if the DSI Host is bound, it wouldn't make sense to keep adding our bridge at probe time (as it would be useless to have it if mtk_dsi_ops.attach() fails!), so also move that one to the dsi host attach function (and remove it in detach).
This patch looks good to me, but please add 'Fixes' tag to note to which patch this patch want to fix.
Regards, CK
Hello, unfortunately I couldn't find a valid commit for a Fixes tag. This started being an issue at some point, when the DRM API was changed to adhere to the documented probe sequence: the MediaTek DSI driver was not the only one that got broken/affected by these changes.
If you have any advice on which commit should be tagged, I'm open for any kind of suggestion.
Thanks, - Angelo
On Tue, Jan 4, 2022 at 3:30 PM AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com wrote:
DRM bridge drivers are now attaching their DSI device at probe time, which requires us to register our DSI host in order to let the bridge to probe: this recently started producing an endless -EPROBE_DEFER loop on some machines that are using external bridges, like the parade-ps8640, found on the ACER Chromebook R13.
Now that the DSI hosts/devices probe sequence is documented, we can do adjustments to the mtk_dsi driver as to both fix now and make sure to avoid this situation in the future: for this, following what is documented in drm_bridge.c, move the mtk_dsi component_add() to the mtk_dsi_ops.attach callback and delete it in the detach callback; keeping in mind that we are registering a drm_bridge for our DSI, which is only used/attached if the DSI Host is bound, it wouldn't make sense to keep adding our bridge at probe time (as it would be useless to have it if mtk_dsi_ops.attach() fails!), so also move that one to the dsi host attach function (and remove it in detach).
Signed-off-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com Reviewed-by: Andrzej Hajda andrzej.hajda@intel.com
Eventually I've observed similar issue on other Component based DSI controllers, hence
Reviewed-by: Jagan Teki jagan@amarulasolutions.com
Il 12/01/22 08:09, Jagan Teki ha scritto:
On Tue, Jan 4, 2022 at 3:30 PM AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com wrote:
DRM bridge drivers are now attaching their DSI device at probe time, which requires us to register our DSI host in order to let the bridge to probe: this recently started producing an endless -EPROBE_DEFER loop on some machines that are using external bridges, like the parade-ps8640, found on the ACER Chromebook R13.
Now that the DSI hosts/devices probe sequence is documented, we can do adjustments to the mtk_dsi driver as to both fix now and make sure to avoid this situation in the future: for this, following what is documented in drm_bridge.c, move the mtk_dsi component_add() to the mtk_dsi_ops.attach callback and delete it in the detach callback; keeping in mind that we are registering a drm_bridge for our DSI, which is only used/attached if the DSI Host is bound, it wouldn't make sense to keep adding our bridge at probe time (as it would be useless to have it if mtk_dsi_ops.attach() fails!), so also move that one to the dsi host attach function (and remove it in detach).
Signed-off-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com Reviewed-by: Andrzej Hajda andrzej.hajda@intel.com
Eventually I've observed similar issue on other Component based DSI controllers, hence
Reviewed-by: Jagan Teki jagan@amarulasolutions.com
Hello dri-devel, can you please pick this patch?
All MediaTek platforms are broken in v5.17 without this one.
Thanks, Angelo
Hi, Angelo:
AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com 於 2022年1月27日 週四 下午6:32寫道：
Il 12/01/22 08:09, Jagan Teki ha scritto:
On Tue, Jan 4, 2022 at 3:30 PM AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com wrote:
DRM bridge drivers are now attaching their DSI device at probe time, which requires us to register our DSI host in order to let the bridge to probe: this recently started producing an endless -EPROBE_DEFER loop on some machines that are using external bridges, like the parade-ps8640, found on the ACER Chromebook R13.
Now that the DSI hosts/devices probe sequence is documented, we can do adjustments to the mtk_dsi driver as to both fix now and make sure to avoid this situation in the future: for this, following what is documented in drm_bridge.c, move the mtk_dsi component_add() to the mtk_dsi_ops.attach callback and delete it in the detach callback; keeping in mind that we are registering a drm_bridge for our DSI, which is only used/attached if the DSI Host is bound, it wouldn't make sense to keep adding our bridge at probe time (as it would be useless to have it if mtk_dsi_ops.attach() fails!), so also move that one to the dsi host attach function (and remove it in detach).
Signed-off-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com Reviewed-by: Andrzej Hajda andrzej.hajda@intel.com
Eventually I've observed similar issue on other Component based DSI controllers, hence
Reviewed-by: Jagan Teki jagan@amarulasolutions.com
Hello dri-devel, can you please pick this patch?
All MediaTek platforms are broken in v5.17 without this one.
Please add Fixes tag [1] to this patch.
[1] https://www.kernel.org/doc/html/v5.16/process/submitting-patches.html
Regards, Chun-Kuang.
Thanks, Angelo
dri-devel@lists.freedesktop.org
-
AngeloGioacchino Del Regno -
Chun-Kuang Hu -
CK Hu -
Jagan Teki