Hello,
This is a new attempt at fixing the "panel is missing" issue (described in this thread [1]). I lost track of Eric's proposal, but I recently proposed to address this problem through a new ->detect() hook in the panel_funcs interface [2], which was rejected.
So here is a new version based on the feedback I had from Daniel, Thierry and Rob.
The idea is to allow of_drm_find_panel() to return -ENODEV and let the DRM driver decide what to do with that (silently ignore the missing component and register the DRM device, or fail to register the DRM device).
Patch 1 changes the semantic of of_drm_find_panel() so that it returns an ERR_PTR() instead of NULL when the panel is not found. This way we'll be able to differentiate the "panel is missing" from "panel has not been probed yet" errors.
Patch 2 and 3 are adding new tests in of_drm_find_panel() and drm_of_find_panel_or_bridge() to return -ENODEV when the status property of the DT node is not set to "okay".
Patch 4 is patching the VC4 DSI encoder driver to gracefully handle the -ENODEV case and allow the registration of the DRM device when the DSI device is disabled.
Note that patch 6 which was modifying the panel status prop from the I2C driver has been dropped because I'm not sure yet how to solve the "force probe of deferred-probe devices even if no new devices have been bound to drivers" problem. Anyway, even without this patch, the series still makes sense to handle the case where devices are described in the DT but marked "disabled" (either at compilation time or tweaked by the bootloader).
Regards,
Boris
Changes in v3: - Dropped patch 1 since it's been acked by Thierry and should be applied soon (either through the drm-tegra or drm-misc tree) - Dropped patch 6 because we are still discussing who should mark the device "disabled" or "fail" and how we should trigger the re-probe of deferred-probe devices in this case
Changes in v2: - Everything :-)
[1]https://lists.freedesktop.org/archives/dri-devel/2017-November/157688.html [2]https://www.spinics.net/lists/dri-devel/msg174808.html
*** BLURB HERE ***
Boris Brezillon (4): drm/panel: Make of_drm_find_panel() return an ERR_PTR() instead of NULL drm/panel: Let of_drm_find_panel() return -ENODEV when the panel is disabled drm/of: Make drm_of_find_panel_or_bridge() fail when the device is disabled drm/vc4: Support the case where the DSI device is disabled
drivers/gpu/drm/bridge/cdns-dsi.c | 2 +- drivers/gpu/drm/bridge/lvds-encoder.c | 4 ++-- drivers/gpu/drm/drm_of.c | 9 ++++++++- drivers/gpu/drm/drm_panel.c | 11 +++++++++-- drivers/gpu/drm/exynos/exynos_dp.c | 6 ++++-- drivers/gpu/drm/exynos/exynos_drm_dpi.c | 4 ++-- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 3 +++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 5 +++-- drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 4 ++-- drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 5 ++++- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++-- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 2 +- drivers/gpu/drm/sti/sti_dvo.c | 4 +++- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 ++-- drivers/gpu/drm/tegra/dsi.c | 3 +++ drivers/gpu/drm/tegra/output.c | 4 ++-- drivers/gpu/drm/vc4/vc4_dsi.c | 15 +++++++++++++-- include/drm/drm_panel.h | 2 +- 19 files changed, 66 insertions(+), 27 deletions(-)
Right now, the DRM panel logic returns NULL when a panel pointing to the passed OF node is not present in the list of registered panels.
Most drivers interpret this NULL value as -EPROBE_DEFER, but we are about to modify the semantic of of_drm_find_panel() and let the framework return -ENODEV when the device node we're pointing to has a status property that is not equal to "okay" or "ok".
Let's first patch the of_drm_find_panel() implementation to return ERR_PTR(-EPROBE_DEFER) instead of NULL and patch all callers to replace the '!panel' check by an 'IS_ERR(panel)' one.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com --- Changes in v3: - Rework error handling in a few drivers as suggested by Thierry - Return -ENODEV when CONFIG_OF or CONFIG_DRM_PANEL are not enabled
Changes in v2: - New commit --- drivers/gpu/drm/bridge/cdns-dsi.c | 2 +- drivers/gpu/drm/bridge/lvds-encoder.c | 4 ++-- drivers/gpu/drm/drm_of.c | 4 +++- drivers/gpu/drm/drm_panel.c | 6 ++++-- drivers/gpu/drm/exynos/exynos_dp.c | 6 ++++-- drivers/gpu/drm/exynos/exynos_drm_dpi.c | 4 ++-- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 3 +++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 5 +++-- drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 4 ++-- drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 5 ++++- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++-- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 2 +- drivers/gpu/drm/sti/sti_dvo.c | 4 +++- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 ++-- drivers/gpu/drm/tegra/dsi.c | 3 +++ drivers/gpu/drm/tegra/output.c | 4 ++-- include/drm/drm_panel.h | 2 +- 18 files changed, 43 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c index c255fc3e1be5..2c5991cf5397 100644 --- a/drivers/gpu/drm/bridge/cdns-dsi.c +++ b/drivers/gpu/drm/bridge/cdns-dsi.c @@ -1152,7 +1152,7 @@ static int cdns_dsi_attach(struct mipi_dsi_host *host, np = of_node_get(dev->dev.of_node);
panel = of_drm_find_panel(np); - if (panel) { + if (!IS_ERR(panel)) { bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DSI); } else { bridge = of_drm_find_bridge(dev->dev.of_node); diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c index 75b0d3f6e4de..f56c92f7af7c 100644 --- a/drivers/gpu/drm/bridge/lvds-encoder.c +++ b/drivers/gpu/drm/bridge/lvds-encoder.c @@ -68,9 +68,9 @@ static int lvds_encoder_probe(struct platform_device *pdev)
panel = of_drm_find_panel(panel_node); of_node_put(panel_node); - if (!panel) { + if (IS_ERR(panel)) { dev_dbg(&pdev->dev, "panel not found, deferring probe\n"); - return -EPROBE_DEFER; + return PTR_ERR(panel); }
lvds_encoder->panel_bridge = diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index 1fe122461298..81d3f76c43f3 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -240,8 +240,10 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
if (panel) { *panel = of_drm_find_panel(remote); - if (*panel) + if (!IS_ERR(*panel)) ret = 0; + else + *panel = NULL; }
/* No panel found yet, check for a bridge next. */ diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index 308d442a531b..49642d79df8f 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -135,7 +135,9 @@ EXPORT_SYMBOL(drm_panel_detach); * tree node. If a matching panel is found, return a pointer to it. * * Return: A pointer to the panel registered for the specified device tree - * node or NULL if no panel matching the device tree node can be found. + * node or an ERR_PTR() if no panel matching the device tree node can be found. + * The only error that can be reported is -EPROBE_DEFER, meaning that the panel + * device has not been probed yet, and the caller should retry later. */ struct drm_panel *of_drm_find_panel(const struct device_node *np) { @@ -151,7 +153,7 @@ struct drm_panel *of_drm_find_panel(const struct device_node *np) }
mutex_unlock(&panel_lock); - return NULL; + return ERR_PTR(-EPROBE_DEFER); } EXPORT_SYMBOL(of_drm_find_panel); #endif diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c index 86330f396784..af7ab1ceb50f 100644 --- a/drivers/gpu/drm/exynos/exynos_dp.c +++ b/drivers/gpu/drm/exynos/exynos_dp.c @@ -232,9 +232,11 @@ static int exynos_dp_probe(struct platform_device *pdev) np = of_parse_phandle(dev->of_node, "panel", 0); if (np) { dp->plat_data.panel = of_drm_find_panel(np); + of_node_put(np); - if (!dp->plat_data.panel) - return -EPROBE_DEFER; + if (IS_ERR(dp->plat_data.panel)) + return PTR_ERR(dp->plat_data.panel); + goto out; }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c index 66945e0dc57f..5887e8522b70 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c @@ -240,8 +240,8 @@ struct drm_encoder *exynos_dpi_probe(struct device *dev)
if (ctx->panel_node) { ctx->panel = of_drm_find_panel(ctx->panel_node); - if (!ctx->panel) - return ERR_PTR(-EPROBE_DEFER); + if (IS_ERR(ctx->panel)) + return ERR_CAST(ctx->panel); }
return &ctx->encoder; diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index 7904ffa9abfb..27d40da77181 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1539,6 +1539,9 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, dsi->format = device->format; dsi->mode_flags = device->mode_flags; dsi->panel = of_drm_find_panel(device->dev.of_node); + if (IS_ERR(dsi->panel)) + dsi->panel = NULL; + if (dsi->panel) { drm_panel_attach(dsi->panel, &dsi->connector); dsi->connector.status = connector_status_connected; diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c index c54806d08dd7..681e2a07d03b 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c @@ -148,8 +148,9 @@ int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev) if (panel_node) { fsl_dev->connector.panel = of_drm_find_panel(panel_node); of_node_put(panel_node); - if (!fsl_dev->connector.panel) - return -EPROBE_DEFER; + if (IS_ERR(fsl_dev->connector.panel)) + return PTR_ERR(fsl_dev->connector.panel); + return fsl_dcu_attach_panel(fsl_dev, fsl_dev->connector.panel); }
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c index 4a645926edb7..2bfb39082f54 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c @@ -341,7 +341,7 @@ static void mdp4_lcdc_encoder_disable(struct drm_encoder *encoder) mdp4_write(mdp4_kms, REG_MDP4_LCDC_ENABLE, 0);
panel = of_drm_find_panel(mdp4_lcdc_encoder->panel_node); - if (panel) { + if (!IS_ERR(panel)) { drm_panel_disable(panel); drm_panel_unprepare(panel); } @@ -410,7 +410,7 @@ static void mdp4_lcdc_encoder_enable(struct drm_encoder *encoder) dev_err(dev->dev, "failed to enable lcdc_clk: %d\n", ret);
panel = of_drm_find_panel(mdp4_lcdc_encoder->panel_node); - if (panel) { + if (!IS_ERR(panel)) { drm_panel_prepare(panel); drm_panel_enable(panel); } diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c index e3b1c86b7aae..32fba5664b0e 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c @@ -34,9 +34,12 @@ static enum drm_connector_status mdp4_lvds_connector_detect( struct mdp4_lvds_connector *mdp4_lvds_connector = to_mdp4_lvds_connector(connector);
- if (!mdp4_lvds_connector->panel) + if (!mdp4_lvds_connector->panel) { mdp4_lvds_connector->panel = of_drm_find_panel(mdp4_lvds_connector->panel_node); + if (IS_ERR(mdp4_lvds_connector->panel)) + mdp4_lvds_connector->panel = NULL; + }
return mdp4_lvds_connector->panel ? connector_status_connected : diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 7a03a9489708..fffc80b73966 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1881,7 +1881,7 @@ int msm_dsi_host_register(struct mipi_dsi_host *host, bool check_defer) * output */ if (check_defer && msm_host->device_node) { - if (!of_drm_find_panel(msm_host->device_node)) + if (IS_ERR(of_drm_find_panel(msm_host->device_node))) if (!of_drm_find_bridge(msm_host->device_node)) return -EPROBE_DEFER; } diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 3d2d3bbd1342..ce7fb00909c4 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -431,8 +431,8 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) ret = -EPROBE_DEFER; } else { lvds->panel = of_drm_find_panel(remote); - if (!lvds->panel) - ret = -EPROBE_DEFER; + if (IS_ERR(lvds->panel)) + ret = PTR_ERR(lvds->panel); }
done: diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index d53d5a09547f..01642aaf6127 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -595,7 +595,7 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host, dsi->format = device->format; dsi->mode_flags = device->mode_flags; dsi->panel = of_drm_find_panel(device->dev.of_node); - if (dsi->panel) + if (!IS_ERR(dsi->panel)) return drm_panel_attach(dsi->panel, &dsi->connector);
return -EINVAL; diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c index a5979cd25cc7..030da55a8d30 100644 --- a/drivers/gpu/drm/sti/sti_dvo.c +++ b/drivers/gpu/drm/sti/sti_dvo.c @@ -387,7 +387,9 @@ sti_dvo_connector_detect(struct drm_connector *connector, bool force)
if (!dvo->panel) { dvo->panel = of_drm_find_panel(dvo->panel_node); - if (dvo->panel) + if (IS_ERR(dvo->panel)) + dvo->panel = NULL; + else drm_panel_attach(dvo->panel, connector); }
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index bfbf761f0c1d..ce388d7cebaa 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -812,8 +812,8 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
dsi->device = device; dsi->panel = of_drm_find_panel(device->dev.of_node); - if (!dsi->panel) - return -EINVAL; + if (IS_ERR(dsi->panel)) + return PTR_ERR(dsi->panel);
dev_info(host->dev, "Attached device %s\n", device->name);
diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c index 87c5d89bc9ba..ad88ec230329 100644 --- a/drivers/gpu/drm/tegra/dsi.c +++ b/drivers/gpu/drm/tegra/dsi.c @@ -1411,6 +1411,9 @@ static int tegra_dsi_host_attach(struct mipi_dsi_host *host, struct tegra_output *output = &dsi->output;
output->panel = of_drm_find_panel(device->dev.of_node); + if (IS_ERR(output->panel)) + output->panel = NULL; + if (output->panel && output->connector.dev) { drm_panel_attach(output->panel, &output->connector); drm_helper_hpd_irq_event(output->connector.dev); diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c index 676fd394836f..c8e3cd633b5a 100644 --- a/drivers/gpu/drm/tegra/output.c +++ b/drivers/gpu/drm/tegra/output.c @@ -111,8 +111,8 @@ int tegra_output_probe(struct tegra_output *output) if (panel) { output->panel = of_drm_find_panel(panel); of_node_put(panel); - if (!output->panel) - return -EPROBE_DEFER; + if (IS_ERR(output->panel)) + return PTR_ERR(output->panel); }
output->edid = of_get_property(output->of_node, "nvidia,edid", &size); diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 14ac240a1f64..777814755fa6 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -199,7 +199,7 @@ struct drm_panel *of_drm_find_panel(const struct device_node *np); #else static inline struct drm_panel *of_drm_find_panel(const struct device_node *np) { - return NULL; + return ERR_PTR(-ENODEV); } #endif
DT nodes might be present in the DT but with a status property set to "disabled" or "fail". In this case, we should not return -EPROBE_DEFER when the caller asks for a drm_panel instance. Return -ENODEV instead.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com Reviewed-by: Thierry Reding treding@nvidia.com Acked-by: Thierry Reding treding@nvidia.com --- Changes in v3: - Add Thierry's A-b/R-b
Changes in v2: - New commit --- drivers/gpu/drm/drm_panel.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index 49642d79df8f..1a59c9bd2327 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -136,13 +136,18 @@ EXPORT_SYMBOL(drm_panel_detach); * * Return: A pointer to the panel registered for the specified device tree * node or an ERR_PTR() if no panel matching the device tree node can be found. - * The only error that can be reported is -EPROBE_DEFER, meaning that the panel - * device has not been probed yet, and the caller should retry later. + * Possible error codes returned by this function: + * - EPROBE_DEFER: the panel device has not been probed yet, and the caller + * should retry later + * - ENODEV: the device is not available (status != "okay" or "ok") */ struct drm_panel *of_drm_find_panel(const struct device_node *np) { struct drm_panel *panel;
+ if (!of_device_is_available(np)) + return ERR_PTR(-ENODEV); + mutex_lock(&panel_lock);
list_for_each_entry(panel, &panel_list, list) {
There's no point searching for a drm_bridge or drm_panel if the OF node we're pointing has a status property that is not "okay" or "ok". Just return -ENODEV in this case.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com Reviewed-by: Thierry Reding treding@nvidia.com Acked-by: Thierry Reding treding@nvidia.com --- Changes in v3: - Add Thierry's A-b/R-b
Changes in v2: - New commit --- drivers/gpu/drm/drm_of.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index 81d3f76c43f3..7678100fd772 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -238,6 +238,11 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, if (!remote) return -ENODEV;
+ if (!of_device_is_available(remote)) { + of_node_put(remote); + return -ENODEV; + } + if (panel) { *panel = of_drm_find_panel(remote); if (!IS_ERR(*panel))
Having a device with a status property != "okay" in the DT is a valid use case, and we should not prevent the registration of the DRM device when the DSI device connected to the DSI controller is disabled.
Consider the ENODEV return code as a valid result and do not expose the DSI encoder/connector when it happens.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com --- Changes in v3: - None
Changes in v2: - New commit --- drivers/gpu/drm/vc4/vc4_dsi.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 8aa897835118..db2f137f8b7b 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1606,8 +1606,18 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, &dsi->bridge); - if (ret) + if (ret) { + /* If the bridge or panel pointed by dev->of_node is not + * enabled, just return 0 here so that we don't prevent the DRM + * dev from being registered. Of course that means the DSI + * encoder won't be exposed, but that's not a problem since + * nothing is connected to it. + */ + if (ret == -ENODEV) + return 0; + return ret; + }
if (panel) { dsi->bridge = devm_drm_panel_bridge_add(dev, panel, @@ -1652,7 +1662,8 @@ static void vc4_dsi_unbind(struct device *dev, struct device *master, struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_dsi *dsi = dev_get_drvdata(dev);
- pm_runtime_disable(dev); + if (dsi->bridge) + pm_runtime_disable(dev);
vc4_dsi_encoder_destroy(dsi->encoder);
Boris Brezillon boris.brezillon@bootlin.com writes:
Having a device with a status property != "okay" in the DT is a valid use case, and we should not prevent the registration of the DRM device when the DSI device connected to the DSI controller is disabled.
Consider the ENODEV return code as a valid result and do not expose the DSI encoder/connector when it happens.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com
Reviewed-by: Eric Anholt eric@anholt.net
On Wed, May 09, 2018 at 03:00:38PM +0200, Boris Brezillon wrote:
Hello,
This is a new attempt at fixing the "panel is missing" issue (described in this thread [1]). I lost track of Eric's proposal, but I recently proposed to address this problem through a new ->detect() hook in the panel_funcs interface [2], which was rejected.
So here is a new version based on the feedback I had from Daniel, Thierry and Rob.
The idea is to allow of_drm_find_panel() to return -ENODEV and let the DRM driver decide what to do with that (silently ignore the missing component and register the DRM device, or fail to register the DRM device).
Patch 1 changes the semantic of of_drm_find_panel() so that it returns an ERR_PTR() instead of NULL when the panel is not found. This way we'll be able to differentiate the "panel is missing" from "panel has not been probed yet" errors.
Patch 2 and 3 are adding new tests in of_drm_find_panel() and drm_of_find_panel_or_bridge() to return -ENODEV when the status property of the DT node is not set to "okay".
Patch 4 is patching the VC4 DSI encoder driver to gracefully handle the -ENODEV case and allow the registration of the DRM device when the DSI device is disabled.
Note that patch 6 which was modifying the panel status prop from the I2C driver has been dropped because I'm not sure yet how to solve the "force probe of deferred-probe devices even if no new devices have been bound to drivers" problem. Anyway, even without this patch, the series still makes sense to handle the case where devices are described in the DT but marked "disabled" (either at compilation time or tweaked by the bootloader).
Regards,
Boris
Changes in v3:
- Dropped patch 1 since it's been acked by Thierry and should be applied soon (either through the drm-tegra or drm-misc tree)
- Dropped patch 6 because we are still discussing who should mark the device "disabled" or "fail" and how we should trigger the re-probe of deferred-probe devices in this case
Changes in v2:
- Everything :-)
[1]https://lists.freedesktop.org/archives/dri-devel/2017-November/157688.html [2]https://www.spinics.net/lists/dri-devel/msg174808.html
I don't exactly remember what we decided should be the merge path for this, but I suspect someone else was supposed to pick it up because I ended up acking these patches. However, since this hasn't been applied yet, I decided to go ahead and apply this to drm-misc-next.
Thierry
Thierry Reding thierry.reding@gmail.com writes:
[ Unknown signature status ] On Wed, May 09, 2018 at 03:00:38PM +0200, Boris Brezillon wrote:
Hello,
This is a new attempt at fixing the "panel is missing" issue (described in this thread [1]). I lost track of Eric's proposal, but I recently proposed to address this problem through a new ->detect() hook in the panel_funcs interface [2], which was rejected.
So here is a new version based on the feedback I had from Daniel, Thierry and Rob.
The idea is to allow of_drm_find_panel() to return -ENODEV and let the DRM driver decide what to do with that (silently ignore the missing component and register the DRM device, or fail to register the DRM device).
Patch 1 changes the semantic of of_drm_find_panel() so that it returns an ERR_PTR() instead of NULL when the panel is not found. This way we'll be able to differentiate the "panel is missing" from "panel has not been probed yet" errors.
Patch 2 and 3 are adding new tests in of_drm_find_panel() and drm_of_find_panel_or_bridge() to return -ENODEV when the status property of the DT node is not set to "okay".
Patch 4 is patching the VC4 DSI encoder driver to gracefully handle the -ENODEV case and allow the registration of the DRM device when the DSI device is disabled.
Note that patch 6 which was modifying the panel status prop from the I2C driver has been dropped because I'm not sure yet how to solve the "force probe of deferred-probe devices even if no new devices have been bound to drivers" problem. Anyway, even without this patch, the series still makes sense to handle the case where devices are described in the DT but marked "disabled" (either at compilation time or tweaked by the bootloader).
Regards,
Boris
Changes in v3:
- Dropped patch 1 since it's been acked by Thierry and should be applied soon (either through the drm-tegra or drm-misc tree)
- Dropped patch 6 because we are still discussing who should mark the device "disabled" or "fail" and how we should trigger the re-probe of deferred-probe devices in this case
Changes in v2:
- Everything :-)
[1]https://lists.freedesktop.org/archives/dri-devel/2017-November/157688.html [2]https://www.spinics.net/lists/dri-devel/msg174808.html
I don't exactly remember what we decided should be the merge path for this, but I suspect someone else was supposed to pick it up because I ended up acking these patches. However, since this hasn't been applied yet, I decided to go ahead and apply this to drm-misc-next.
Since you'd been engaged on previous versions, I was waiting for an ack on patch 1. Thanks for taking care of this now!
dri-devel@lists.freedesktop.org
-
Boris Brezillon -
Eric Anholt -
Thierry Reding