This function is the pendant of drm_of_find_panel_or_bridge() to remove a previously allocated panel_bridge. Given a specific port and endpoint it remove the panel bridge. Since drm_panel_bridge_remove() will check that bridge parameter is not NULL and is a real drm_panel_bridge and no a simple bridge it is safe to call it directly.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org Reviewed-by: Philippe Cornu philippe.cornu@st.com Tested-by: Philippe Cornu philippe.cornu@st.com --- drivers/gpu/drm/drm_of.c | 33 +++++++++++++++++++++++++++++++++ include/drm/drm_of.h | 8 ++++++++ 2 files changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index 8dafbdf..6b54f17 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -260,3 +260,36 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, return ret; } EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge); + +#ifdef CONFIG_DRM_PANEL_BRIDGE +/* + * drm_of_panel_bridge_remove - remove panel bridge + * @np: device tree node containing panel bridge output ports + * + * Remove the panel bridge of a given DT node's port and endpoint number + * + * Returns zero if successful, or one of the standard error codes if it fails. + */ +int drm_of_panel_bridge_remove(const struct device_node *np, + int port, int endpoint) +{ + struct drm_bridge *bridge; + struct device_node *remote; + + remote = of_graph_get_remote_node(np, port, endpoint); + if (!remote) + return -ENODEV; + + bridge = of_drm_find_bridge(remote); + drm_panel_bridge_remove(bridge); + + return 0; +} +#else +int drm_of_panel_bridge_remove(const struct device_node *np, + int port, int endpoint) +{ + return -EINVAL; +} +#endif +EXPORT_SYMBOL_GPL(drm_of_panel_bridge_remove); diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h index 104dd51..390966e 100644 --- a/include/drm/drm_of.h +++ b/include/drm/drm_of.h @@ -29,6 +29,8 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, int port, int endpoint, struct drm_panel **panel, struct drm_bridge **bridge); +int drm_of_panel_bridge_remove(const struct device_node *np, + int port, int endpoint); #else static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev, struct device_node *port) @@ -65,6 +67,12 @@ static inline int drm_of_find_panel_or_bridge(const struct device_node *np, { return -EINVAL; } + +static inline int drm_of_panel_bridge_remove(const struct device_node *np, + int port, int endpoint) +{ + return -EINVAL; +} #endif
static inline int drm_of_encoder_active_endpoint_id(struct device_node *node,
With a call to drm_of_panel_bridge_remove() we could remove the bridge without store it in ldtc internal driver structure.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org Reviewed-by: Philippe Cornu philippe.cornu@st.com Tested-by: Philippe Cornu philippe.cornu@st.com --- drivers/gpu/drm/stm/ltdc.c | 16 +++++----------- drivers/gpu/drm/stm/ltdc.h | 2 -- 2 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index d394a03..735c908 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -791,9 +791,8 @@ static const struct drm_encoder_funcs ltdc_encoder_funcs = { .destroy = drm_encoder_cleanup, };
-static int ltdc_encoder_init(struct drm_device *ddev) +static int ltdc_encoder_init(struct drm_device *ddev, struct drm_bridge *bridge) { - struct ltdc_device *ldev = ddev->dev_private; struct drm_encoder *encoder; int ret;
@@ -807,7 +806,7 @@ static int ltdc_encoder_init(struct drm_device *ddev) drm_encoder_init(ddev, encoder, <dc_encoder_funcs, DRM_MODE_ENCODER_DPI, NULL);
- ret = drm_bridge_attach(encoder, ldev->bridge, NULL); + ret = drm_bridge_attach(encoder, bridge, NULL); if (ret) { drm_encoder_cleanup(encoder); return -EINVAL; @@ -936,12 +935,9 @@ int ltdc_load(struct drm_device *ddev) ret = PTR_ERR(bridge); goto err; } - ldev->is_panel_bridge = true; }
- ldev->bridge = bridge; - - ret = ltdc_encoder_init(ddev); + ret = ltdc_encoder_init(ddev, bridge); if (ret) { DRM_ERROR("Failed to init encoder\n"); goto err; @@ -972,8 +968,7 @@ int ltdc_load(struct drm_device *ddev) return 0;
err: - if (ldev->is_panel_bridge) - drm_panel_bridge_remove(bridge); + drm_panel_bridge_remove(bridge);
clk_disable_unprepare(ldev->pixel_clk);
@@ -986,8 +981,7 @@ void ltdc_unload(struct drm_device *ddev)
DRM_DEBUG_DRIVER("\n");
- if (ldev->is_panel_bridge) - drm_panel_bridge_remove(ldev->bridge); + drm_of_panel_bridge_remove(ddev->dev->of_node, 0, 0);
clk_disable_unprepare(ldev->pixel_clk); } diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h index bc6d6f6..ae43755 100644 --- a/drivers/gpu/drm/stm/ltdc.h +++ b/drivers/gpu/drm/stm/ltdc.h @@ -24,8 +24,6 @@ struct ltdc_device { struct drm_fbdev_cma *fbdev; void __iomem *regs; struct clk *pixel_clk; /* lcd pixel clock */ - struct drm_bridge *bridge; - bool is_panel_bridge; struct mutex err_lock; /* protecting error_status */ struct ltdc_caps caps; u32 error_status;
With a call to drm_of_panel_bridge_remove() we could remove the bridge without store it in vc4_dpi internal driver structure.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/drm/vc4/vc4_dpi.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index 519cefe..72c9dbd 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -97,8 +97,6 @@ struct vc4_dpi {
struct drm_encoder *encoder; struct drm_connector *connector; - struct drm_bridge *bridge; - bool is_panel_bridge;
void __iomem *regs;
@@ -251,10 +249,11 @@ static int vc4_dpi_init_bridge(struct vc4_dpi *dpi) { struct device *dev = &dpi->pdev->dev; struct drm_panel *panel; + struct drm_bridge *bridge; int ret;
ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, - &panel, &dpi->bridge); + &panel, &bridge); if (ret) { /* If nothing was connected in the DT, that's not an * error. @@ -265,13 +264,10 @@ static int vc4_dpi_init_bridge(struct vc4_dpi *dpi) return ret; }
- if (panel) { - dpi->bridge = drm_panel_bridge_add(panel, - DRM_MODE_CONNECTOR_DPI); - dpi->is_panel_bridge = true; - } + if (panel) + bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DPI);
- return drm_bridge_attach(dpi->encoder, dpi->bridge, NULL); + return drm_bridge_attach(dpi->encoder, bridge, NULL); }
static int vc4_dpi_bind(struct device *dev, struct device *master, void *data) @@ -352,8 +348,7 @@ static void vc4_dpi_unbind(struct device *dev, struct device *master, struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_dpi *dpi = dev_get_drvdata(dev);
- if (dpi->is_panel_bridge) - drm_panel_bridge_remove(dpi->bridge); + drm_of_panel_bridge_remove(dev->of_node, 0, 0);
drm_encoder_cleanup(dpi->encoder);
2017-10-02 11:34 GMT+02:00 Benjamin Gaignard benjamin.gaignard@linaro.org:
With a call to drm_of_panel_bridge_remove() we could remove the bridge without store it in vc4_dpi internal driver structure.
+ Eric to get his point of view on that
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/gpu/drm/vc4/vc4_dpi.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index 519cefe..72c9dbd 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -97,8 +97,6 @@ struct vc4_dpi {
struct drm_encoder *encoder; struct drm_connector *connector;
struct drm_bridge *bridge;
bool is_panel_bridge; void __iomem *regs;
@@ -251,10 +249,11 @@ static int vc4_dpi_init_bridge(struct vc4_dpi *dpi) { struct device *dev = &dpi->pdev->dev; struct drm_panel *panel;
struct drm_bridge *bridge; int ret; ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
&panel, &dpi->bridge);
&panel, &bridge); if (ret) { /* If nothing was connected in the DT, that's not an * error.
@@ -265,13 +264,10 @@ static int vc4_dpi_init_bridge(struct vc4_dpi *dpi) return ret; }
if (panel) {
dpi->bridge = drm_panel_bridge_add(panel,
DRM_MODE_CONNECTOR_DPI);
dpi->is_panel_bridge = true;
}
if (panel)
bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DPI);
return drm_bridge_attach(dpi->encoder, dpi->bridge, NULL);
return drm_bridge_attach(dpi->encoder, bridge, NULL);
}
static int vc4_dpi_bind(struct device *dev, struct device *master, void *data) @@ -352,8 +348,7 @@ static void vc4_dpi_unbind(struct device *dev, struct device *master, struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_dpi *dpi = dev_get_drvdata(dev);
if (dpi->is_panel_bridge)
drm_panel_bridge_remove(dpi->bridge);
drm_of_panel_bridge_remove(dev->of_node, 0, 0); drm_encoder_cleanup(dpi->encoder);
-- 2.7.4
Benjamin Gaignard benjamin.gaignard@linaro.org writes:
With a call to drm_of_panel_bridge_remove() we could remove the bridge without store it in vc4_dpi internal driver structure.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
Reviewed-by: Eric Anholt eric@anholt.net
Thanks for doing this cleanup!
When using drm_of_panel_bridge_remove() we can simplify the code and remove is_panel_bridge from dw_mipi_dsi structure.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org Reviewed-by: Philippe Cornu philippe.cornu@st.com Tested-by: Philippe Cornu philippe.cornu@st.com --- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index f4f633a..d9cca4f 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c @@ -221,7 +221,6 @@ struct dw_mipi_dsi { struct drm_bridge bridge; struct mipi_dsi_host dsi_host; struct drm_bridge *panel_bridge; - bool is_panel_bridge; struct device *dev; void __iomem *base;
@@ -297,7 +296,6 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host, bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DSI); if (IS_ERR(bridge)) return PTR_ERR(bridge); - dsi->is_panel_bridge = true; }
dsi->panel_bridge = bridge; @@ -312,8 +310,7 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host *host, { struct dw_mipi_dsi *dsi = host_to_dsi(host);
- if (dsi->is_panel_bridge) - drm_panel_bridge_remove(dsi->panel_bridge); + drm_of_panel_bridge_remove(host->dev->of_node, 1, 0);
drm_bridge_remove(&dsi->bridge);
On 2 October 2017 at 10:34, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
When using drm_of_panel_bridge_remove() we can simplify the code and remove is_panel_bridge from dw_mipi_dsi structure.
As the previous patches remove the struct drm_bridge pointer, it might be worth mentioning why that cannot be done here?
If you agree, please don't resent the patches - just amend before pushing.
Thanks Emil
2017-10-03 11:24 GMT+02:00 Emil Velikov emil.l.velikov@gmail.com:
On 2 October 2017 at 10:34, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:
When using drm_of_panel_bridge_remove() we can simplify the code and remove is_panel_bridge from dw_mipi_dsi structure.
As the previous patches remove the struct drm_bridge pointer, it might be worth mentioning why that cannot be done here?
If you agree, please don't resent the patches - just amend before pushing.
Will add that: Because drm_bridge pointer is used in attach and post_disable functions it can't be removed from internal driver structure .
Thanks Emil
dri-devel@lists.freedesktop.org