| Message ID | 20180525155030.3667352-7-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 25, 2018 at 5:50 PM, Arnd Bergmann <arnd@arndb.de> wrote: > The DRM panel bridge code is built into the kms helpers module, so we > get a link error when trying to use it from a built-in driver while the > kms helper is a loadable module: > > drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': > lvds-encoder.c:(.text+0x124): undefined reference to `devm_drm_panel_bridge_add' > > This adds a the same dependency in the lvds-encoder that we use for all > the other users of the panel bridge. I did not bisect the problem, but > from inspection it seems to date back to the patch that separated out > the panel bridge from lvds encoder. > > Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge.") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
Hi Arnd, Thank you for the patch. On Friday, 25 May 2018 18:50:14 EEST Arnd Bergmann wrote: > The DRM panel bridge code is built into the kms helpers module, so we > get a link error when trying to use it from a built-in driver while the > kms helper is a loadable module: > > drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': > lvds-encoder.c:(.text+0x124): undefined reference to > `devm_drm_panel_bridge_add' > > This adds a the same dependency in the lvds-encoder that we use for all > the other users of the panel bridge. I did not bisect the problem, but > from inspection it seems to date back to the patch that separated out > the panel bridge from lvds encoder. > > Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the > lvds-encoder bridge.") Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/gpu/drm/bridge/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 6caa47834194..cf47bfa7a050 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -46,6 +46,7 @@ config DRM_DUMB_VGA_DAC > config DRM_LVDS_ENCODER > tristate "Transparent parallel to LVDS encoder support" > depends on OF > + select DRM_KMS_HELPER > select DRM_PANEL_BRIDGE > help > Support for transparent parallel to LVDS encoders that don't require Wouldn't it be better to apply the following ? config DRM_PANEL_BRIDGE def_bool y depends on DRM_BRIDGE - depends on DRM_KMS_HELPER + select DRM_KMS_HELPER select DRM_PANEL help DRM bridge wrapper of DRM panels Otherwise you'll potentially have to patch every user of DRM_PANEL_BRIDGE as done in this patch.
On Mon, May 28, 2018 at 10:02 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Arnd, > > Thank you for the patch. > > On Friday, 25 May 2018 18:50:14 EEST Arnd Bergmann wrote: >> The DRM panel bridge code is built into the kms helpers module, so we >> get a link error when trying to use it from a built-in driver while the >> kms helper is a loadable module: >> >> drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': >> lvds-encoder.c:(.text+0x124): undefined reference to >> `devm_drm_panel_bridge_add' >> >> This adds a the same dependency in the lvds-encoder that we use for all >> the other users of the panel bridge. I did not bisect the problem, but >> from inspection it seems to date back to the patch that separated out >> the panel bridge from lvds encoder. >> >> Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the >> lvds-encoder bridge.") Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> drivers/gpu/drm/bridge/Kconfig | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig >> index 6caa47834194..cf47bfa7a050 100644 >> --- a/drivers/gpu/drm/bridge/Kconfig >> +++ b/drivers/gpu/drm/bridge/Kconfig >> @@ -46,6 +46,7 @@ config DRM_DUMB_VGA_DAC >> config DRM_LVDS_ENCODER >> tristate "Transparent parallel to LVDS encoder support" >> depends on OF >> + select DRM_KMS_HELPER >> select DRM_PANEL_BRIDGE >> help >> Support for transparent parallel to LVDS encoders that don't require > > Wouldn't it be better to apply the following ? > > config DRM_PANEL_BRIDGE > def_bool y > depends on DRM_BRIDGE > - depends on DRM_KMS_HELPER > + select DRM_KMS_HELPER > select DRM_PANEL > help > DRM bridge wrapper of DRM panels > > Otherwise you'll potentially have to patch every user of DRM_PANEL_BRIDGE as > done in this patch. Select isn't recursive, so this won't work unfortunately :-/ -Daniel > -- > Regards, > > Laurent Pinchart > > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, May 28, 2018 at 10:06 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, May 28, 2018 at 10:02 AM, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: >> Hi Arnd, >> >> Thank you for the patch. >> >> On Friday, 25 May 2018 18:50:14 EEST Arnd Bergmann wrote: >>> The DRM panel bridge code is built into the kms helpers module, so we >>> get a link error when trying to use it from a built-in driver while the >>> kms helper is a loadable module: >>> >>> drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': >>> lvds-encoder.c:(.text+0x124): undefined reference to >>> `devm_drm_panel_bridge_add' >>> >>> This adds a the same dependency in the lvds-encoder that we use for all >>> the other users of the panel bridge. I did not bisect the problem, but >>> from inspection it seems to date back to the patch that separated out >>> the panel bridge from lvds encoder. >>> >>> Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the >>> lvds-encoder bridge.") Signed-off-by: Arnd Bergmann <arnd@arndb.de> >>> --- >>> drivers/gpu/drm/bridge/Kconfig | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig >>> index 6caa47834194..cf47bfa7a050 100644 >>> --- a/drivers/gpu/drm/bridge/Kconfig >>> +++ b/drivers/gpu/drm/bridge/Kconfig >>> @@ -46,6 +46,7 @@ config DRM_DUMB_VGA_DAC >>> config DRM_LVDS_ENCODER >>> tristate "Transparent parallel to LVDS encoder support" >>> depends on OF >>> + select DRM_KMS_HELPER >>> select DRM_PANEL_BRIDGE >>> help >>> Support for transparent parallel to LVDS encoders that don't require >> >> Wouldn't it be better to apply the following ? >> >> config DRM_PANEL_BRIDGE >> def_bool y >> depends on DRM_BRIDGE >> - depends on DRM_KMS_HELPER >> + select DRM_KMS_HELPER >> select DRM_PANEL >> help >> DRM bridge wrapper of DRM panels >> >> Otherwise you'll potentially have to patch every user of DRM_PANEL_BRIDGE as >> done in this patch. > > Select isn't recursive, so this won't work unfortunately :-/ The problem is a bit different: select *is* recursive, which is part of the reason we normally try to avoid it (it gets hard to disable certain symbols or turn them into modules when there are lots of things selecting them). However, DRM_PANEL_BRIDGE is a silent 'bool' symbol that is always enabled when DRM_BRIDGE is enabled. Making it 'select DRM_KMS_HELPER' would lead to DRM_KMS_HELPER always being built-in even if all other DRM drivers are configured as loadable modules! Note these Makefile line in drivers/gpu/drm: drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o The intention is definitely that drm_kms_helper can be a loadable module, (it cannot be built-in when CONFIG_DRM=m) and the panel bridge is simply a component that gets linked into it, as of commit 123387d5efa6 ("drm/bridge: Build the panel wrapper in drm_kms_helper"). Arnd
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 6caa47834194..cf47bfa7a050 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -46,6 +46,7 @@ config DRM_DUMB_VGA_DAC config DRM_LVDS_ENCODER tristate "Transparent parallel to LVDS encoder support" depends on OF + select DRM_KMS_HELPER select DRM_PANEL_BRIDGE help Support for transparent parallel to LVDS encoders that don't require
The DRM panel bridge code is built into the kms helpers module, so we get a link error when trying to use it from a built-in driver while the kms helper is a loadable module: drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': lvds-encoder.c:(.text+0x124): undefined reference to `devm_drm_panel_bridge_add' This adds a the same dependency in the lvds-encoder that we use for all the other users of the panel bridge. I did not bisect the problem, but from inspection it seems to date back to the patch that separated out the panel bridge from lvds encoder. Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge.") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/gpu/drm/bridge/Kconfig | 1 + 1 file changed, 1 insertion(+)