Re: [PATCH] drm: rcar-du: fix linker undefined references

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Randy,

On Fri, Apr 23, 2021 at 03:02:27PM -0700, Randy Dunlap wrote:
> On 4/23/21 2:56 PM, Randy Dunlap wrote:
> > On 4/23/21 2:46 PM, Laurent Pinchart wrote:
> >> On Fri, Apr 23, 2021 at 02:37:27PM -0700, Randy Dunlap wrote:
> >>> When DRM_RCAR_DU=y and DRM_RCAR_LVDS=m, there are several build errors
> >>> as reported by 'kernel test robot'. These can be corrected by changing
> >>> "imply" to "select".
> >>>
> >>> In looking at this, the same problem (build errors) happens when
> >>> DRM_RCAR_DU=y and DRM_RCAR_CMM=m, so again change the "imply" to
> >>> "select" here as well.
> >>>
> >>> These 2 changes fix the following 8 build/link errors:
> >>>
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
> >>> `rcar_du_crtc_atomic_enable':
> >>> rcar_du_crtc.c:(.text+0x1be8): undefined reference to 
> >>> `rcar_lvds_clk_enable'
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
> >>> `rcar_du_crtc_atomic_disable':
> >>> rcar_du_crtc.c:(.text+0x2438): undefined reference to 
> >>> `rcar_lvds_clk_disable'
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_drv.o: in function 
> >>> `rcar_du_init':
> >>> rcar_du_drv.c:(.init.text+0x14): undefined reference to 
> >>> `rcar_du_of_init'
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_encoder.o: in 
> >>> function `rcar_du_encoder_init':
> >>> rcar_du_encoder.c:(.text+0x1d4): undefined reference to 
> >>> `rcar_lvds_dual_link'
> >>>
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
> >>> `rcar_du_cmm_setup':
> >>> rcar_du_crtc.c:(.text+0x380): undefined reference to `rcar_cmm_setup'
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
> >>> `rcar_du_crtc_atomic_enable':
> >>> rcar_du_crtc.c:(.text+0x1c08): undefined reference to `rcar_cmm_enable'
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
> >>> `rcar_du_crtc_atomic_disable':
> >>> rcar_du_crtc.c:(.text+0x231c): undefined reference to `rcar_cmm_disable'
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_kms.o: in function 
> >>> `rcar_du_modeset_init':
> >>> rcar_du_kms.c:(.text+0xd08): undefined reference to `rcar_cmm_init'
> >>>
> >>> Fixes: e08e934d6c28 ("drm: rcar-du: Add support for CMM")
> >>> Fixes: 02f2b30032c1 ("drm: rcar-du: lvds: Add API to enable/disable 
> >>> clock output")
> >>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> >>> Reported-by: kernel test robot <lkp@intel.com>
> >>> Cc: Masahiro Yamada <masahiroy@kernel.org>
> >>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >>> Cc: dri-devel@lists.freedesktop.org
> >>> Cc: linux-renesas-soc@vger.kernel.org
> >>> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >>> ---
> >>>   drivers/gpu/drm/rcar-du/Kconfig |    4 ++--
> >>>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> --- linux-next-20210420.orig/drivers/gpu/drm/rcar-du/Kconfig
> >>> +++ linux-next-20210420/drivers/gpu/drm/rcar-du/Kconfig
> >>> @@ -4,8 +4,8 @@ config DRM_RCAR_DU
> >>>       depends on DRM && OF
> >>>       depends on ARM || ARM64
> >>>       depends on ARCH_RENESAS || COMPILE_TEST
> >>> -    imply DRM_RCAR_CMM
> >>> -    imply DRM_RCAR_LVDS
> >>> +    select DRM_RCAR_CMM
> >>> +    select DRM_RCAR_LVDS
> >>
> >> No all platforms that integrate a DU have CMM and/or LVDS support, so we
> >> shouldn't select the automatically.
> >>
> >> Would
> >>
> >>     depends on DRM_RCAR_CMM || DRM_RCAR_CMM=n
> >>     depends on DRM_RCAR_LVDS || DRM_RCAR_LVDS=n
> > 
> > Looks good; I like it, but kconfig does not:
> > 
> > $ xbuild_arm64.sh  oldconfig
> > make[1]: Entering directory 
> > '/home/rdunlap/lnx/next/linux-next-20210420/ARM64'
> >    GEN     Makefile
> > drivers/gpu/drm/rcar-du/Kconfig:2:error: recursive dependency detected!
> > drivers/gpu/drm/rcar-du/Kconfig:2:    symbol DRM_RCAR_DU depends on 
> > DRM_RCAR_CMM
> > drivers/gpu/drm/rcar-du/Kconfig:17:    symbol DRM_RCAR_CMM depends on 
> > DRM_RCAR_DU
> > For a resolution refer to Documentation/kbuild/kconfig-language.rst
> > subsection "Kconfig recursive dependency limitations"
> > 
> >> work ? I thought that's what "imply" meant, but it seems I got it wrong.
> >>
> >>>       select DRM_KMS_HELPER
> >>>       select DRM_KMS_CMA_HELPER
> >>>       select DRM_GEM_CMA_HELPER
> 
> There is some documentation (kconfig-language.rst) that says that when a 
> driver Kconfig is using mixed builtin and loadable modules, the driver 
> should be using IS_REACHABLE(). The example also uses "imply", so maybe
> we should/could try something like that:
> If CMM is REACHBLE(), make the calls.
> If LVDS is READABLE(), make the calls.
> 
> I can look into that some more if you want me to.
> Or I'll be happy to let you do so. :)

If you have time to have a look, I'd be very happy about that. I can't
promise to repay it with a drink at the next conference, but I hope that
will change sooner than later :-)

-- 
Regards,

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