[PATCH v3 0/4] drm/panel: Handle the "panel is missing" case properly

[PATCH v3 0/4] drm/panel: Handle the "panel is missing" case properly

[Boris Brezillon]

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(-)

-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 1/4] drm/panel: Make of_drm_find_panel() return an ERR_PTR() instead of NULL

[Boris Brezillon]

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
 
-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 2/4] drm/panel: Let of_drm_find_panel() return -ENODEV when the panel is disabled

[Boris Brezillon]

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) {
-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 3/4] drm/of: Make drm_of_find_panel_or_bridge() fail when the device is disabled

[Boris Brezillon]

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))
-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 4/4] drm/vc4: Support the case where the DSI device is disabled

[Boris Brezillon]

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);
 
-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/4] drm/vc4: Support the case where the DSI device is disabled

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 493 bytes --]

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>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 0/4] drm/panel: Handle the "panel is missing" case properly

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2601 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 0/4] drm/panel: Handle the "panel is missing" case properly

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 2863 bytes --]

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!

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel