From be5c6b177229c5cb40f3fef785250afbfd669175 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Sat, 18 Mar 2023 20:07:50 +0100 Subject: drm/bridge: fsl-ldb: Convert to platform remove callback returning void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is (mostly) ignored and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new() which already returns void. Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König Reviewed-by: Laurent Pinchart Signed-off-by: Neil Armstrong Link: https://patchwork.freedesktop.org/patch/msgid/20230318190804.234610-6-u.kleine-koenig@pengutronix.de --- drivers/gpu/drm/bridge/fsl-ldb.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/bridge/fsl-ldb.c') diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c index 6bac160b395b..450b352914f4 100644 --- a/drivers/gpu/drm/bridge/fsl-ldb.c +++ b/drivers/gpu/drm/bridge/fsl-ldb.c @@ -347,13 +347,11 @@ static int fsl_ldb_probe(struct platform_device *pdev) return 0; } -static int fsl_ldb_remove(struct platform_device *pdev) +static void fsl_ldb_remove(struct platform_device *pdev) { struct fsl_ldb *fsl_ldb = platform_get_drvdata(pdev); drm_bridge_remove(&fsl_ldb->bridge); - - return 0; } static const struct of_device_id fsl_ldb_match[] = { @@ -367,7 +365,7 @@ MODULE_DEVICE_TABLE(of, fsl_ldb_match); static struct platform_driver fsl_ldb_driver = { .probe = fsl_ldb_probe, - .remove = fsl_ldb_remove, + .remove_new = fsl_ldb_remove, .driver = { .name = "fsl-ldb", .of_match_table = fsl_ldb_match, -- cgit From 8cc0b604f2345c304b7ddc828b880f4f3f21816a Mon Sep 17 00:00:00 2001 From: Luca Ceresoli Date: Wed, 5 Apr 2023 10:10:56 +0200 Subject: drm: bridge: ldb: add missing \n in dev_warn() string dev_warn() and similar require a training \n. Signed-off-by: Luca Ceresoli Reviewed-by: Robert Foss Signed-off-by: Robert Foss Link: https://patchwork.freedesktop.org/patch/msgid/20230405081058.2347130-1-luca.ceresoli@bootlin.com --- drivers/gpu/drm/bridge/fsl-ldb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/bridge/fsl-ldb.c') diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c index 450b352914f4..81b84a2ee6bb 100644 --- a/drivers/gpu/drm/bridge/fsl-ldb.c +++ b/drivers/gpu/drm/bridge/fsl-ldb.c @@ -170,7 +170,7 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge, configured_link_freq = clk_get_rate(fsl_ldb->clk); if (configured_link_freq != requested_link_freq) - dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz", + dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz\n", configured_link_freq, requested_link_freq); -- cgit From e09220f42b5c28dae51a3cf2810afcb4fad9909d Mon Sep 17 00:00:00 2001 From: Luca Ceresoli Date: Wed, 5 Apr 2023 10:10:57 +0200 Subject: drm: bridge: ldb: add support for using channel 1 only The LDB driver currently checks whether dual mode is used, otherwise it assumes only channel 0 is in use. Add support for using only channel 1. In device tree terms, this means linking port 2 only. Doing this cleanly requires changing the logic of the probe functions from this: 1. use of_graph_get_remote_node() on port 1 to find the panel 2. use drm_of_lvds_get_dual_link_pixel_order() to detect dual mode to this: 1. use of_graph_get_remote_node() twice to find remote ports 2. reuse the result of the above to know whether each channel is enabled and to find the panel 3. if (both channels as enabled) use drm_of_lvds_get_dual_link_pixel_order() to detect dual mode Also add a dev_dbg() to log the detected mode and log an error in case no panel was found (no channel enabled). Signed-off-by: Luca Ceresoli Reviewed-by: Marek Vasut Signed-off-by: Robert Foss Link: https://patchwork.freedesktop.org/patch/msgid/20230405081058.2347130-2-luca.ceresoli@bootlin.com --- drivers/gpu/drm/bridge/fsl-ldb.c | 101 +++++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 42 deletions(-) (limited to 'drivers/gpu/drm/bridge/fsl-ldb.c') diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c index 81b84a2ee6bb..682623369498 100644 --- a/drivers/gpu/drm/bridge/fsl-ldb.c +++ b/drivers/gpu/drm/bridge/fsl-ldb.c @@ -84,10 +84,16 @@ struct fsl_ldb { struct drm_bridge *panel_bridge; struct clk *clk; struct regmap *regmap; - bool lvds_dual_link; const struct fsl_ldb_devdata *devdata; + bool ch0_enabled; + bool ch1_enabled; }; +static bool fsl_ldb_is_dual(const struct fsl_ldb *fsl_ldb) +{ + return (fsl_ldb->ch0_enabled && fsl_ldb->ch1_enabled); +} + static inline struct fsl_ldb *to_fsl_ldb(struct drm_bridge *bridge) { return container_of(bridge, struct fsl_ldb, bridge); @@ -95,7 +101,7 @@ static inline struct fsl_ldb *to_fsl_ldb(struct drm_bridge *bridge) static unsigned long fsl_ldb_link_frequency(struct fsl_ldb *fsl_ldb, int clock) { - if (fsl_ldb->lvds_dual_link) + if (fsl_ldb_is_dual(fsl_ldb)) return clock * 3500; else return clock * 7000; @@ -177,28 +183,21 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge, clk_prepare_enable(fsl_ldb->clk); /* Program LDB_CTRL */ - reg = LDB_CTRL_CH0_ENABLE; + reg = (fsl_ldb->ch0_enabled ? LDB_CTRL_CH0_ENABLE : 0) | + (fsl_ldb->ch1_enabled ? LDB_CTRL_CH1_ENABLE : 0) | + (fsl_ldb_is_dual(fsl_ldb) ? LDB_CTRL_SPLIT_MODE : 0); - if (fsl_ldb->lvds_dual_link) - reg |= LDB_CTRL_CH1_ENABLE | LDB_CTRL_SPLIT_MODE; + if (lvds_format_24bpp) + reg |= (fsl_ldb->ch0_enabled ? LDB_CTRL_CH0_DATA_WIDTH : 0) | + (fsl_ldb->ch1_enabled ? LDB_CTRL_CH1_DATA_WIDTH : 0); - if (lvds_format_24bpp) { - reg |= LDB_CTRL_CH0_DATA_WIDTH; - if (fsl_ldb->lvds_dual_link) - reg |= LDB_CTRL_CH1_DATA_WIDTH; - } + if (lvds_format_jeida) + reg |= (fsl_ldb->ch0_enabled ? LDB_CTRL_CH0_BIT_MAPPING : 0) | + (fsl_ldb->ch1_enabled ? LDB_CTRL_CH1_BIT_MAPPING : 0); - if (lvds_format_jeida) { - reg |= LDB_CTRL_CH0_BIT_MAPPING; - if (fsl_ldb->lvds_dual_link) - reg |= LDB_CTRL_CH1_BIT_MAPPING; - } - - if (mode->flags & DRM_MODE_FLAG_PVSYNC) { - reg |= LDB_CTRL_DI0_VSYNC_POLARITY; - if (fsl_ldb->lvds_dual_link) - reg |= LDB_CTRL_DI1_VSYNC_POLARITY; - } + if (mode->flags & DRM_MODE_FLAG_PVSYNC) + reg |= (fsl_ldb->ch0_enabled ? LDB_CTRL_DI0_VSYNC_POLARITY : 0) | + (fsl_ldb->ch1_enabled ? LDB_CTRL_DI1_VSYNC_POLARITY : 0); regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->ldb_ctrl, reg); @@ -210,9 +209,8 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge, /* Wait for VBG to stabilize. */ usleep_range(15, 20); - reg |= LVDS_CTRL_CH0_EN; - if (fsl_ldb->lvds_dual_link) - reg |= LVDS_CTRL_CH1_EN; + reg |= (fsl_ldb->ch0_enabled ? LVDS_CTRL_CH0_EN : 0) | + (fsl_ldb->ch1_enabled ? LVDS_CTRL_CH1_EN : 0); regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg); } @@ -265,7 +263,7 @@ fsl_ldb_mode_valid(struct drm_bridge *bridge, { struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge); - if (mode->clock > (fsl_ldb->lvds_dual_link ? 160000 : 80000)) + if (mode->clock > (fsl_ldb_is_dual(fsl_ldb) ? 160000 : 80000)) return MODE_CLOCK_HIGH; return MODE_OK; @@ -286,7 +284,7 @@ static int fsl_ldb_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct device_node *panel_node; - struct device_node *port1, *port2; + struct device_node *remote1, *remote2; struct drm_panel *panel; struct fsl_ldb *fsl_ldb; int dual_link; @@ -311,10 +309,23 @@ static int fsl_ldb_probe(struct platform_device *pdev) if (IS_ERR(fsl_ldb->regmap)) return PTR_ERR(fsl_ldb->regmap); - /* Locate the panel DT node. */ - panel_node = of_graph_get_remote_node(dev->of_node, 1, 0); - if (!panel_node) - return -ENXIO; + /* Locate the remote ports and the panel node */ + remote1 = of_graph_get_remote_node(dev->of_node, 1, 0); + remote2 = of_graph_get_remote_node(dev->of_node, 2, 0); + fsl_ldb->ch0_enabled = (remote1 != NULL); + fsl_ldb->ch1_enabled = (remote2 != NULL); + panel_node = of_node_get(remote1 ? remote1 : remote2); + of_node_put(remote1); + of_node_put(remote2); + + if (!fsl_ldb->ch0_enabled && !fsl_ldb->ch1_enabled) { + of_node_put(panel_node); + return dev_err_probe(dev, -ENXIO, "No panel node found"); + } + + dev_dbg(dev, "Using %s\n", + fsl_ldb_is_dual(fsl_ldb) ? "dual-link mode" : + fsl_ldb->ch0_enabled ? "channel 0" : "channel 1"); panel = of_drm_find_panel(panel_node); of_node_put(panel_node); @@ -325,20 +336,26 @@ static int fsl_ldb_probe(struct platform_device *pdev) if (IS_ERR(fsl_ldb->panel_bridge)) return PTR_ERR(fsl_ldb->panel_bridge); - /* Determine whether this is dual-link configuration */ - port1 = of_graph_get_port_by_id(dev->of_node, 1); - port2 = of_graph_get_port_by_id(dev->of_node, 2); - dual_link = drm_of_lvds_get_dual_link_pixel_order(port1, port2); - of_node_put(port1); - of_node_put(port2); - if (dual_link == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) { - dev_err(dev, "LVDS channel pixel swap not supported.\n"); - return -EINVAL; - } + if (fsl_ldb_is_dual(fsl_ldb)) { + struct device_node *port1, *port2; - if (dual_link == DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS) - fsl_ldb->lvds_dual_link = true; + port1 = of_graph_get_port_by_id(dev->of_node, 1); + port2 = of_graph_get_port_by_id(dev->of_node, 2); + dual_link = drm_of_lvds_get_dual_link_pixel_order(port1, port2); + of_node_put(port1); + of_node_put(port2); + + if (dual_link < 0) + return dev_err_probe(dev, dual_link, + "Error getting dual link configuration\n"); + + /* Only DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS is supported */ + if (dual_link == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) { + dev_err(dev, "LVDS channel pixel swap not supported.\n"); + return -EINVAL; + } + } platform_set_drvdata(pdev, fsl_ldb); -- cgit