With CONFIG_DRM_IMX8QM_LDB=m and CONFIG_DRM_IMX8QXP_LDB=y (or vice
versa), imx-ldb-helper.o is linked to a module and also to vmlinux
even though the expected CFLAGS are different between builtins and
modules.
This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
Fixing mixed module-builtin objects").
Turn helpers in imx-ldb-helper.c into inline functions.
Signed-off-by: Masahiro Yamada <[email protected]>
---
drivers/gpu/drm/bridge/imx/Makefile | 4 +-
drivers/gpu/drm/bridge/imx/imx-ldb-helper.c | 221 --------------------
drivers/gpu/drm/bridge/imx/imx-ldb-helper.h | 213 +++++++++++++++++--
3 files changed, 197 insertions(+), 241 deletions(-)
delete mode 100644 drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
index aa90ec8d5433..64b93009376a 100644
--- a/drivers/gpu/drm/bridge/imx/Makefile
+++ b/drivers/gpu/drm/bridge/imx/Makefile
@@ -1,7 +1,7 @@
-imx8qm-ldb-objs := imx-ldb-helper.o imx8qm-ldb-drv.o
+imx8qm-ldb-objs := imx8qm-ldb-drv.o
obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o
-imx8qxp-ldb-objs := imx-ldb-helper.o imx8qxp-ldb-drv.o
+imx8qxp-ldb-objs := imx8qxp-ldb-drv.o
obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o
obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
deleted file mode 100644
index 7338b84bc83d..000000000000
--- a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
+++ /dev/null
@@ -1,221 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright (C) 2012 Sascha Hauer, Pengutronix
- * Copyright 2019,2020,2022 NXP
- */
-
-#include <linux/media-bus-format.h>
-#include <linux/mfd/syscon.h>
-#include <linux/of.h>
-#include <linux/regmap.h>
-
-#include <drm/drm_bridge.h>
-#include <drm/drm_of.h>
-#include <drm/drm_print.h>
-
-#include "imx-ldb-helper.h"
-
-bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch)
-{
- return ldb_ch->link_type == LDB_CH_SINGLE_LINK;
-}
-
-bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch)
-{
- return ldb_ch->link_type == LDB_CH_DUAL_LINK_EVEN_ODD_PIXELS ||
- ldb_ch->link_type == LDB_CH_DUAL_LINK_ODD_EVEN_PIXELS;
-}
-
-int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
- struct drm_bridge_state *bridge_state,
- struct drm_crtc_state *crtc_state,
- struct drm_connector_state *conn_state)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
-
- ldb_ch->in_bus_format = bridge_state->input_bus_cfg.format;
- ldb_ch->out_bus_format = bridge_state->output_bus_cfg.format;
-
- return 0;
-}
-
-void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
- const struct drm_display_mode *mode,
- const struct drm_display_mode *adjusted_mode)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
- struct ldb *ldb = ldb_ch->ldb;
- bool is_split = ldb_channel_is_split_link(ldb_ch);
-
- if (is_split)
- ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;
-
- switch (ldb_ch->out_bus_format) {
- case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
- break;
- case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
- if (ldb_ch->chno == 0 || is_split)
- ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
- if (ldb_ch->chno == 1 || is_split)
- ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
- break;
- case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
- if (ldb_ch->chno == 0 || is_split)
- ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
- LDB_BIT_MAP_CH0_JEIDA;
- if (ldb_ch->chno == 1 || is_split)
- ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
- LDB_BIT_MAP_CH1_JEIDA;
- break;
- }
-}
-
-void ldb_bridge_enable_helper(struct drm_bridge *bridge)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
- struct ldb *ldb = ldb_ch->ldb;
-
- /*
- * Platform specific bridge drivers should set ldb_ctrl properly
- * for the enablement, so just write the ctrl_reg here.
- */
- regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
-}
-
-void ldb_bridge_disable_helper(struct drm_bridge *bridge)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
- struct ldb *ldb = ldb_ch->ldb;
- bool is_split = ldb_channel_is_split_link(ldb_ch);
-
- if (ldb_ch->chno == 0 || is_split)
- ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK;
- if (ldb_ch->chno == 1 || is_split)
- ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;
-
- regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
-}
-
-int ldb_bridge_attach_helper(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
- struct ldb *ldb = ldb_ch->ldb;
-
- if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
- DRM_DEV_ERROR(ldb->dev,
- "do not support creating a drm_connector\n");
- return -EINVAL;
- }
-
- if (!bridge->encoder) {
- DRM_DEV_ERROR(ldb->dev, "missing encoder\n");
- return -ENODEV;
- }
-
- return drm_bridge_attach(bridge->encoder,
- ldb_ch->next_bridge, bridge,
- DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-}
-
-int ldb_init_helper(struct ldb *ldb)
-{
- struct device *dev = ldb->dev;
- struct device_node *np = dev->of_node;
- struct device_node *child;
- int ret;
- u32 i;
-
- ldb->regmap = syscon_node_to_regmap(np->parent);
- if (IS_ERR(ldb->regmap)) {
- ret = PTR_ERR(ldb->regmap);
- if (ret != -EPROBE_DEFER)
- DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
- return ret;
- }
-
- for_each_available_child_of_node(np, child) {
- struct ldb_channel *ldb_ch;
-
- ret = of_property_read_u32(child, "reg", &i);
- if (ret || i > MAX_LDB_CHAN_NUM - 1) {
- ret = -EINVAL;
- DRM_DEV_ERROR(dev,
- "invalid channel node address: %u\n", i);
- of_node_put(child);
- return ret;
- }
-
- ldb_ch = ldb->channel[i];
- ldb_ch->ldb = ldb;
- ldb_ch->chno = i;
- ldb_ch->is_available = true;
- ldb_ch->np = child;
-
- ldb->available_ch_cnt++;
- }
-
- return 0;
-}
-
-int ldb_find_next_bridge_helper(struct ldb *ldb)
-{
- struct device *dev = ldb->dev;
- struct ldb_channel *ldb_ch;
- int ret, i;
-
- for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
- ldb_ch = ldb->channel[i];
-
- if (!ldb_ch->is_available)
- continue;
-
- ldb_ch->next_bridge = devm_drm_of_get_bridge(dev, ldb_ch->np,
- 1, 0);
- if (IS_ERR(ldb_ch->next_bridge)) {
- ret = PTR_ERR(ldb_ch->next_bridge);
- if (ret != -EPROBE_DEFER)
- DRM_DEV_ERROR(dev,
- "failed to get next bridge: %d\n",
- ret);
- return ret;
- }
- }
-
- return 0;
-}
-
-void ldb_add_bridge_helper(struct ldb *ldb,
- const struct drm_bridge_funcs *bridge_funcs)
-{
- struct ldb_channel *ldb_ch;
- int i;
-
- for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
- ldb_ch = ldb->channel[i];
-
- if (!ldb_ch->is_available)
- continue;
-
- ldb_ch->bridge.driver_private = ldb_ch;
- ldb_ch->bridge.funcs = bridge_funcs;
- ldb_ch->bridge.of_node = ldb_ch->np;
-
- drm_bridge_add(&ldb_ch->bridge);
- }
-}
-
-void ldb_remove_bridge_helper(struct ldb *ldb)
-{
- struct ldb_channel *ldb_ch;
- int i;
-
- for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
- ldb_ch = ldb->channel[i];
-
- if (!ldb_ch->is_available)
- continue;
-
- drm_bridge_remove(&ldb_ch->bridge);
- }
-}
diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
index a0a5cde27fbc..42e9b4aa8399 100644
--- a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
+++ b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
@@ -65,32 +65,209 @@ struct ldb {
#define bridge_to_ldb_ch(b) container_of(b, struct ldb_channel, bridge)
-bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch);
-bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch);
+static inline bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch)
+{
+ return ldb_ch->link_type == LDB_CH_SINGLE_LINK;
+}
-int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
- struct drm_bridge_state *bridge_state,
- struct drm_crtc_state *crtc_state,
- struct drm_connector_state *conn_state);
+static inline bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch)
+{
+ return ldb_ch->link_type == LDB_CH_DUAL_LINK_EVEN_ODD_PIXELS ||
+ ldb_ch->link_type == LDB_CH_DUAL_LINK_ODD_EVEN_PIXELS;
+}
-void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
- const struct drm_display_mode *mode,
- const struct drm_display_mode *adjusted_mode);
+static inline int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;
-void ldb_bridge_enable_helper(struct drm_bridge *bridge);
+ ldb_ch->in_bus_format = bridge_state->input_bus_cfg.format;
+ ldb_ch->out_bus_format = bridge_state->output_bus_cfg.format;
-void ldb_bridge_disable_helper(struct drm_bridge *bridge);
+ return 0;
+}
-int ldb_bridge_attach_helper(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags);
+static inline void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;
+ struct ldb *ldb = ldb_ch->ldb;
+ bool is_split = ldb_channel_is_split_link(ldb_ch);
-int ldb_init_helper(struct ldb *ldb);
+ if (is_split)
+ ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;
-int ldb_find_next_bridge_helper(struct ldb *ldb);
+ switch (ldb_ch->out_bus_format) {
+ case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+ if (ldb_ch->chno == 0 || is_split)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
+ if (ldb_ch->chno == 1 || is_split)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+ if (ldb_ch->chno == 0 || is_split)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
+ LDB_BIT_MAP_CH0_JEIDA;
+ if (ldb_ch->chno == 1 || is_split)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
+ LDB_BIT_MAP_CH1_JEIDA;
+ break;
+ }
+}
-void ldb_add_bridge_helper(struct ldb *ldb,
- const struct drm_bridge_funcs *bridge_funcs);
+static inline void ldb_bridge_enable_helper(struct drm_bridge *bridge)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;
+ struct ldb *ldb = ldb_ch->ldb;
-void ldb_remove_bridge_helper(struct ldb *ldb);
+ /*
+ * Platform specific bridge drivers should set ldb_ctrl properly
+ * for the enablement, so just write the ctrl_reg here.
+ */
+ regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
+}
+
+static inline void ldb_bridge_disable_helper(struct drm_bridge *bridge)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;
+ struct ldb *ldb = ldb_ch->ldb;
+ bool is_split = ldb_channel_is_split_link(ldb_ch);
+
+ if (ldb_ch->chno == 0 || is_split)
+ ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK;
+ if (ldb_ch->chno == 1 || is_split)
+ ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;
+
+ regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
+}
+
+static inline int ldb_bridge_attach_helper(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;
+ struct ldb *ldb = ldb_ch->ldb;
+
+ if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
+ DRM_DEV_ERROR(ldb->dev,
+ "do not support creating a drm_connector\n");
+ return -EINVAL;
+ }
+
+ if (!bridge->encoder) {
+ DRM_DEV_ERROR(ldb->dev, "missing encoder\n");
+ return -ENODEV;
+ }
+
+ return drm_bridge_attach(bridge->encoder,
+ ldb_ch->next_bridge, bridge,
+ DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+}
+
+static inline int ldb_init_helper(struct ldb *ldb)
+{
+ struct device *dev = ldb->dev;
+ struct device_node *np = dev->of_node;
+ struct device_node *child;
+ int ret;
+ u32 i;
+
+ ldb->regmap = syscon_node_to_regmap(np->parent);
+ if (IS_ERR(ldb->regmap)) {
+ ret = PTR_ERR(ldb->regmap);
+ if (ret != -EPROBE_DEFER)
+ DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
+ return ret;
+ }
+
+ for_each_available_child_of_node(np, child) {
+ struct ldb_channel *ldb_ch;
+
+ ret = of_property_read_u32(child, "reg", &i);
+ if (ret || i > MAX_LDB_CHAN_NUM - 1) {
+ ret = -EINVAL;
+ DRM_DEV_ERROR(dev,
+ "invalid channel node address: %u\n", i);
+ of_node_put(child);
+ return ret;
+ }
+
+ ldb_ch = ldb->channel[i];
+ ldb_ch->ldb = ldb;
+ ldb_ch->chno = i;
+ ldb_ch->is_available = true;
+ ldb_ch->np = child;
+
+ ldb->available_ch_cnt++;
+ }
+
+ return 0;
+}
+
+static inline int ldb_find_next_bridge_helper(struct ldb *ldb)
+{
+ struct device *dev = ldb->dev;
+ struct ldb_channel *ldb_ch;
+ int ret, i;
+
+ for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
+ ldb_ch = ldb->channel[i];
+
+ if (!ldb_ch->is_available)
+ continue;
+
+ ldb_ch->next_bridge = devm_drm_of_get_bridge(dev, ldb_ch->np,
+ 1, 0);
+ if (IS_ERR(ldb_ch->next_bridge)) {
+ ret = PTR_ERR(ldb_ch->next_bridge);
+ if (ret != -EPROBE_DEFER)
+ DRM_DEV_ERROR(dev,
+ "failed to get next bridge: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static inline void ldb_add_bridge_helper(struct ldb *ldb,
+ const struct drm_bridge_funcs *bridge_funcs)
+{
+ struct ldb_channel *ldb_ch;
+ int i;
+
+ for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
+ ldb_ch = ldb->channel[i];
+
+ if (!ldb_ch->is_available)
+ continue;
+
+ ldb_ch->bridge.driver_private = ldb_ch;
+ ldb_ch->bridge.funcs = bridge_funcs;
+ ldb_ch->bridge.of_node = ldb_ch->np;
+
+ drm_bridge_add(&ldb_ch->bridge);
+ }
+}
+
+static inline void ldb_remove_bridge_helper(struct ldb *ldb)
+{
+ struct ldb_channel *ldb_ch;
+ int i;
+
+ for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
+ ldb_ch = ldb->channel[i];
+
+ if (!ldb_ch->is_available)
+ continue;
+
+ drm_bridge_remove(&ldb_ch->bridge);
+ }
+}
#endif /* __IMX_LDB_HELPER__ */
--
2.39.2
With the previous fix, these modules are built from a single C file.
Rename the source files so they match the module names.
Signed-off-by: Masahiro Yamada <[email protected]>
---
drivers/gpu/drm/bridge/imx/Makefile | 4 ----
drivers/gpu/drm/bridge/imx/{imx8qm-ldb-drv.c => imx8qm-ldb.c} | 0
.../gpu/drm/bridge/imx/{imx8qxp-ldb-drv.c => imx8qxp-ldb.c} | 0
3 files changed, 4 deletions(-)
rename drivers/gpu/drm/bridge/imx/{imx8qm-ldb-drv.c => imx8qm-ldb.c} (100%)
rename drivers/gpu/drm/bridge/imx/{imx8qxp-ldb-drv.c => imx8qxp-ldb.c} (100%)
diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
index 64b93009376a..c102443f7286 100644
--- a/drivers/gpu/drm/bridge/imx/Makefile
+++ b/drivers/gpu/drm/bridge/imx/Makefile
@@ -1,9 +1,5 @@
-imx8qm-ldb-objs := imx8qm-ldb-drv.o
obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o
-
-imx8qxp-ldb-objs := imx8qxp-ldb-drv.o
obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o
-
obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK) += imx8qxp-pixel-link.o
obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK_TO_DPI) += imx8qxp-pxl2dpi.o
diff --git a/drivers/gpu/drm/bridge/imx/imx8qm-ldb-drv.c b/drivers/gpu/drm/bridge/imx/imx8qm-ldb.c
similarity index 100%
rename from drivers/gpu/drm/bridge/imx/imx8qm-ldb-drv.c
rename to drivers/gpu/drm/bridge/imx/imx8qm-ldb.c
diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
similarity index 100%
rename from drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c
rename to drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
--
2.39.2
Hi Yamada-san,
Thank you for the patch.
On Sun, Jun 04, 2023 at 02:07:46AM +0900, Masahiro Yamada wrote:
> With CONFIG_DRM_IMX8QM_LDB=m and CONFIG_DRM_IMX8QXP_LDB=y (or vice
> versa), imx-ldb-helper.o is linked to a module and also to vmlinux
> even though the expected CFLAGS are different between builtins and
> modules.
>
> This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
> Fixing mixed module-builtin objects").
>
> Turn helpers in imx-ldb-helper.c into inline functions.
Wouldn't it be better to turn it into a module ? It could then be
built-in for the above configuration, are compiled as a module when all
its users are module as well.
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> drivers/gpu/drm/bridge/imx/Makefile | 4 +-
> drivers/gpu/drm/bridge/imx/imx-ldb-helper.c | 221 --------------------
> drivers/gpu/drm/bridge/imx/imx-ldb-helper.h | 213 +++++++++++++++++--
> 3 files changed, 197 insertions(+), 241 deletions(-)
> delete mode 100644 drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
>
> diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
> index aa90ec8d5433..64b93009376a 100644
> --- a/drivers/gpu/drm/bridge/imx/Makefile
> +++ b/drivers/gpu/drm/bridge/imx/Makefile
> @@ -1,7 +1,7 @@
> -imx8qm-ldb-objs := imx-ldb-helper.o imx8qm-ldb-drv.o
> +imx8qm-ldb-objs := imx8qm-ldb-drv.o
> obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o
>
> -imx8qxp-ldb-objs := imx-ldb-helper.o imx8qxp-ldb-drv.o
> +imx8qxp-ldb-objs := imx8qxp-ldb-drv.o
> obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o
>
> obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
> diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
> deleted file mode 100644
> index 7338b84bc83d..000000000000
> --- a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
> +++ /dev/null
> @@ -1,221 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Copyright (C) 2012 Sascha Hauer, Pengutronix
> - * Copyright 2019,2020,2022 NXP
> - */
> -
> -#include <linux/media-bus-format.h>
> -#include <linux/mfd/syscon.h>
> -#include <linux/of.h>
> -#include <linux/regmap.h>
> -
> -#include <drm/drm_bridge.h>
> -#include <drm/drm_of.h>
> -#include <drm/drm_print.h>
> -
> -#include "imx-ldb-helper.h"
> -
> -bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch)
> -{
> - return ldb_ch->link_type == LDB_CH_SINGLE_LINK;
> -}
> -
> -bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch)
> -{
> - return ldb_ch->link_type == LDB_CH_DUAL_LINK_EVEN_ODD_PIXELS ||
> - ldb_ch->link_type == LDB_CH_DUAL_LINK_ODD_EVEN_PIXELS;
> -}
> -
> -int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
> - struct drm_bridge_state *bridge_state,
> - struct drm_crtc_state *crtc_state,
> - struct drm_connector_state *conn_state)
> -{
> - struct ldb_channel *ldb_ch = bridge->driver_private;
> -
> - ldb_ch->in_bus_format = bridge_state->input_bus_cfg.format;
> - ldb_ch->out_bus_format = bridge_state->output_bus_cfg.format;
> -
> - return 0;
> -}
> -
> -void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
> - const struct drm_display_mode *mode,
> - const struct drm_display_mode *adjusted_mode)
> -{
> - struct ldb_channel *ldb_ch = bridge->driver_private;
> - struct ldb *ldb = ldb_ch->ldb;
> - bool is_split = ldb_channel_is_split_link(ldb_ch);
> -
> - if (is_split)
> - ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;
> -
> - switch (ldb_ch->out_bus_format) {
> - case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> - break;
> - case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> - if (ldb_ch->chno == 0 || is_split)
> - ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
> - if (ldb_ch->chno == 1 || is_split)
> - ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
> - break;
> - case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> - if (ldb_ch->chno == 0 || is_split)
> - ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
> - LDB_BIT_MAP_CH0_JEIDA;
> - if (ldb_ch->chno == 1 || is_split)
> - ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
> - LDB_BIT_MAP_CH1_JEIDA;
> - break;
> - }
> -}
> -
> -void ldb_bridge_enable_helper(struct drm_bridge *bridge)
> -{
> - struct ldb_channel *ldb_ch = bridge->driver_private;
> - struct ldb *ldb = ldb_ch->ldb;
> -
> - /*
> - * Platform specific bridge drivers should set ldb_ctrl properly
> - * for the enablement, so just write the ctrl_reg here.
> - */
> - regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
> -}
> -
> -void ldb_bridge_disable_helper(struct drm_bridge *bridge)
> -{
> - struct ldb_channel *ldb_ch = bridge->driver_private;
> - struct ldb *ldb = ldb_ch->ldb;
> - bool is_split = ldb_channel_is_split_link(ldb_ch);
> -
> - if (ldb_ch->chno == 0 || is_split)
> - ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK;
> - if (ldb_ch->chno == 1 || is_split)
> - ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;
> -
> - regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
> -}
> -
> -int ldb_bridge_attach_helper(struct drm_bridge *bridge,
> - enum drm_bridge_attach_flags flags)
> -{
> - struct ldb_channel *ldb_ch = bridge->driver_private;
> - struct ldb *ldb = ldb_ch->ldb;
> -
> - if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> - DRM_DEV_ERROR(ldb->dev,
> - "do not support creating a drm_connector\n");
> - return -EINVAL;
> - }
> -
> - if (!bridge->encoder) {
> - DRM_DEV_ERROR(ldb->dev, "missing encoder\n");
> - return -ENODEV;
> - }
> -
> - return drm_bridge_attach(bridge->encoder,
> - ldb_ch->next_bridge, bridge,
> - DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> -}
> -
> -int ldb_init_helper(struct ldb *ldb)
> -{
> - struct device *dev = ldb->dev;
> - struct device_node *np = dev->of_node;
> - struct device_node *child;
> - int ret;
> - u32 i;
> -
> - ldb->regmap = syscon_node_to_regmap(np->parent);
> - if (IS_ERR(ldb->regmap)) {
> - ret = PTR_ERR(ldb->regmap);
> - if (ret != -EPROBE_DEFER)
> - DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
> - return ret;
> - }
> -
> - for_each_available_child_of_node(np, child) {
> - struct ldb_channel *ldb_ch;
> -
> - ret = of_property_read_u32(child, "reg", &i);
> - if (ret || i > MAX_LDB_CHAN_NUM - 1) {
> - ret = -EINVAL;
> - DRM_DEV_ERROR(dev,
> - "invalid channel node address: %u\n", i);
> - of_node_put(child);
> - return ret;
> - }
> -
> - ldb_ch = ldb->channel[i];
> - ldb_ch->ldb = ldb;
> - ldb_ch->chno = i;
> - ldb_ch->is_available = true;
> - ldb_ch->np = child;
> -
> - ldb->available_ch_cnt++;
> - }
> -
> - return 0;
> -}
> -
> -int ldb_find_next_bridge_helper(struct ldb *ldb)
> -{
> - struct device *dev = ldb->dev;
> - struct ldb_channel *ldb_ch;
> - int ret, i;
> -
> - for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
> - ldb_ch = ldb->channel[i];
> -
> - if (!ldb_ch->is_available)
> - continue;
> -
> - ldb_ch->next_bridge = devm_drm_of_get_bridge(dev, ldb_ch->np,
> - 1, 0);
> - if (IS_ERR(ldb_ch->next_bridge)) {
> - ret = PTR_ERR(ldb_ch->next_bridge);
> - if (ret != -EPROBE_DEFER)
> - DRM_DEV_ERROR(dev,
> - "failed to get next bridge: %d\n",
> - ret);
> - return ret;
> - }
> - }
> -
> - return 0;
> -}
> -
> -void ldb_add_bridge_helper(struct ldb *ldb,
> - const struct drm_bridge_funcs *bridge_funcs)
> -{
> - struct ldb_channel *ldb_ch;
> - int i;
> -
> - for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
> - ldb_ch = ldb->channel[i];
> -
> - if (!ldb_ch->is_available)
> - continue;
> -
> - ldb_ch->bridge.driver_private = ldb_ch;
> - ldb_ch->bridge.funcs = bridge_funcs;
> - ldb_ch->bridge.of_node = ldb_ch->np;
> -
> - drm_bridge_add(&ldb_ch->bridge);
> - }
> -}
> -
> -void ldb_remove_bridge_helper(struct ldb *ldb)
> -{
> - struct ldb_channel *ldb_ch;
> - int i;
> -
> - for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
> - ldb_ch = ldb->channel[i];
> -
> - if (!ldb_ch->is_available)
> - continue;
> -
> - drm_bridge_remove(&ldb_ch->bridge);
> - }
> -}
> diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
> index a0a5cde27fbc..42e9b4aa8399 100644
> --- a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
> +++ b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
> @@ -65,32 +65,209 @@ struct ldb {
>
> #define bridge_to_ldb_ch(b) container_of(b, struct ldb_channel, bridge)
>
> -bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch);
> -bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch);
> +static inline bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch)
> +{
> + return ldb_ch->link_type == LDB_CH_SINGLE_LINK;
> +}
>
> -int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
> - struct drm_bridge_state *bridge_state,
> - struct drm_crtc_state *crtc_state,
> - struct drm_connector_state *conn_state);
> +static inline bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch)
> +{
> + return ldb_ch->link_type == LDB_CH_DUAL_LINK_EVEN_ODD_PIXELS ||
> + ldb_ch->link_type == LDB_CH_DUAL_LINK_ODD_EVEN_PIXELS;
> +}
>
> -void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
> - const struct drm_display_mode *mode,
> - const struct drm_display_mode *adjusted_mode);
> +static inline int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> +{
> + struct ldb_channel *ldb_ch = bridge->driver_private;
>
> -void ldb_bridge_enable_helper(struct drm_bridge *bridge);
> + ldb_ch->in_bus_format = bridge_state->input_bus_cfg.format;
> + ldb_ch->out_bus_format = bridge_state->output_bus_cfg.format;
>
> -void ldb_bridge_disable_helper(struct drm_bridge *bridge);
> + return 0;
> +}
>
> -int ldb_bridge_attach_helper(struct drm_bridge *bridge,
> - enum drm_bridge_attach_flags flags);
> +static inline void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
> + const struct drm_display_mode *mode,
> + const struct drm_display_mode *adjusted_mode)
> +{
> + struct ldb_channel *ldb_ch = bridge->driver_private;
> + struct ldb *ldb = ldb_ch->ldb;
> + bool is_split = ldb_channel_is_split_link(ldb_ch);
>
> -int ldb_init_helper(struct ldb *ldb);
> + if (is_split)
> + ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;
>
> -int ldb_find_next_bridge_helper(struct ldb *ldb);
> + switch (ldb_ch->out_bus_format) {
> + case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> + break;
> + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> + if (ldb_ch->chno == 0 || is_split)
> + ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
> + if (ldb_ch->chno == 1 || is_split)
> + ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
> + break;
> + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> + if (ldb_ch->chno == 0 || is_split)
> + ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
> + LDB_BIT_MAP_CH0_JEIDA;
> + if (ldb_ch->chno == 1 || is_split)
> + ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
> + LDB_BIT_MAP_CH1_JEIDA;
> + break;
> + }
> +}
>
> -void ldb_add_bridge_helper(struct ldb *ldb,
> - const struct drm_bridge_funcs *bridge_funcs);
> +static inline void ldb_bridge_enable_helper(struct drm_bridge *bridge)
> +{
> + struct ldb_channel *ldb_ch = bridge->driver_private;
> + struct ldb *ldb = ldb_ch->ldb;
>
> -void ldb_remove_bridge_helper(struct ldb *ldb);
> + /*
> + * Platform specific bridge drivers should set ldb_ctrl properly
> + * for the enablement, so just write the ctrl_reg here.
> + */
> + regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
> +}
> +
> +static inline void ldb_bridge_disable_helper(struct drm_bridge *bridge)
> +{
> + struct ldb_channel *ldb_ch = bridge->driver_private;
> + struct ldb *ldb = ldb_ch->ldb;
> + bool is_split = ldb_channel_is_split_link(ldb_ch);
> +
> + if (ldb_ch->chno == 0 || is_split)
> + ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK;
> + if (ldb_ch->chno == 1 || is_split)
> + ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;
> +
> + regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
> +}
> +
> +static inline int ldb_bridge_attach_helper(struct drm_bridge *bridge,
> + enum drm_bridge_attach_flags flags)
> +{
> + struct ldb_channel *ldb_ch = bridge->driver_private;
> + struct ldb *ldb = ldb_ch->ldb;
> +
> + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> + DRM_DEV_ERROR(ldb->dev,
> + "do not support creating a drm_connector\n");
> + return -EINVAL;
> + }
> +
> + if (!bridge->encoder) {
> + DRM_DEV_ERROR(ldb->dev, "missing encoder\n");
> + return -ENODEV;
> + }
> +
> + return drm_bridge_attach(bridge->encoder,
> + ldb_ch->next_bridge, bridge,
> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> +}
> +
> +static inline int ldb_init_helper(struct ldb *ldb)
> +{
> + struct device *dev = ldb->dev;
> + struct device_node *np = dev->of_node;
> + struct device_node *child;
> + int ret;
> + u32 i;
> +
> + ldb->regmap = syscon_node_to_regmap(np->parent);
> + if (IS_ERR(ldb->regmap)) {
> + ret = PTR_ERR(ldb->regmap);
> + if (ret != -EPROBE_DEFER)
> + DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
> + return ret;
> + }
> +
> + for_each_available_child_of_node(np, child) {
> + struct ldb_channel *ldb_ch;
> +
> + ret = of_property_read_u32(child, "reg", &i);
> + if (ret || i > MAX_LDB_CHAN_NUM - 1) {
> + ret = -EINVAL;
> + DRM_DEV_ERROR(dev,
> + "invalid channel node address: %u\n", i);
> + of_node_put(child);
> + return ret;
> + }
> +
> + ldb_ch = ldb->channel[i];
> + ldb_ch->ldb = ldb;
> + ldb_ch->chno = i;
> + ldb_ch->is_available = true;
> + ldb_ch->np = child;
> +
> + ldb->available_ch_cnt++;
> + }
> +
> + return 0;
> +}
> +
> +static inline int ldb_find_next_bridge_helper(struct ldb *ldb)
> +{
> + struct device *dev = ldb->dev;
> + struct ldb_channel *ldb_ch;
> + int ret, i;
> +
> + for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
> + ldb_ch = ldb->channel[i];
> +
> + if (!ldb_ch->is_available)
> + continue;
> +
> + ldb_ch->next_bridge = devm_drm_of_get_bridge(dev, ldb_ch->np,
> + 1, 0);
> + if (IS_ERR(ldb_ch->next_bridge)) {
> + ret = PTR_ERR(ldb_ch->next_bridge);
> + if (ret != -EPROBE_DEFER)
> + DRM_DEV_ERROR(dev,
> + "failed to get next bridge: %d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static inline void ldb_add_bridge_helper(struct ldb *ldb,
> + const struct drm_bridge_funcs *bridge_funcs)
> +{
> + struct ldb_channel *ldb_ch;
> + int i;
> +
> + for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
> + ldb_ch = ldb->channel[i];
> +
> + if (!ldb_ch->is_available)
> + continue;
> +
> + ldb_ch->bridge.driver_private = ldb_ch;
> + ldb_ch->bridge.funcs = bridge_funcs;
> + ldb_ch->bridge.of_node = ldb_ch->np;
> +
> + drm_bridge_add(&ldb_ch->bridge);
> + }
> +}
> +
> +static inline void ldb_remove_bridge_helper(struct ldb *ldb)
> +{
> + struct ldb_channel *ldb_ch;
> + int i;
> +
> + for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
> + ldb_ch = ldb->channel[i];
> +
> + if (!ldb_ch->is_available)
> + continue;
> +
> + drm_bridge_remove(&ldb_ch->bridge);
> + }
> +}
>
> #endif /* __IMX_LDB_HELPER__ */
--
Regards,
Laurent Pinchart
On Sun, Jun 4, 2023 at 1:52 PM Laurent Pinchart
<[email protected]> wrote:
>
> Hi Yamada-san,
>
> Thank you for the patch.
>
> On Sun, Jun 04, 2023 at 02:07:46AM +0900, Masahiro Yamada wrote:
> > With CONFIG_DRM_IMX8QM_LDB=m and CONFIG_DRM_IMX8QXP_LDB=y (or vice
> > versa), imx-ldb-helper.o is linked to a module and also to vmlinux
> > even though the expected CFLAGS are different between builtins and
> > modules.
> >
> > This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
> > Fixing mixed module-builtin objects").
> >
> > Turn helpers in imx-ldb-helper.c into inline functions.
>
> Wouldn't it be better to turn it into a module ? It could then be
> built-in for the above configuration, are compiled as a module when all
> its users are module as well.
Yes, two ways to fix it.
inline line functions vs a separate module
I do not have a strong opinion.
I sent v2.
https://lore.kernel.org/lkml/[email protected]/T/#t
Please pick a preferred one.
--
Best Regards
Masahiro Yamada