Hi Kieran and Emil, On Fri, 22 May 2020 at 11:43, Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> wrote: > > Hi Daniel, > > On 18/05/2020 21:16, Daniel Gomez wrote: > > Select DRM_KMS_HELPER dependency. > > > > Build error when DRM_KMS_HELPER is not selected: > > > > drivers/gpu/drm/rcar-du/rcar_lvds.o:(.rodata+0xd48): undefined reference to `drm_atomic_helper_bridge_duplicate_state' > > drivers/gpu/drm/rcar-du/rcar_lvds.o:(.rodata+0xd50): undefined reference to `drm_atomic_helper_bridge_destroy_state' > > drivers/gpu/drm/rcar-du/rcar_lvds.o:(.rodata+0xd70): undefined reference to `drm_atomic_helper_bridge_reset' > > drivers/gpu/drm/rcar-du/rcar_lvds.o:(.rodata+0xdc8): undefined reference to `drm_atomic_helper_connector_reset' > > drivers/gpu/drm/rcar-du/rcar_lvds.o:(.rodata+0xde0): undefined reference to `drm_helper_probe_single_connector_modes' > > drivers/gpu/drm/rcar-du/rcar_lvds.o:(.rodata+0xe08): undefined reference to `drm_atomic_helper_connector_duplicate_state' > > drivers/gpu/drm/rcar-du/rcar_lvds.o:(.rodata+0xe10): undefined reference to `drm_atomic_helper_connector_destroy_state' > > > > Looking at the files in rcar_du that utilise drm_atomic_helpers... > > git grep -l drm_atomic_helper_ drivers/gpu/drm/rcar-du/ > drivers/gpu/drm/rcar-du/rcar_du_crtc.c > drivers/gpu/drm/rcar-du/rcar_du_kms.c > drivers/gpu/drm/rcar-du/rcar_du_plane.c > drivers/gpu/drm/rcar-du/rcar_du_vsp.c > drivers/gpu/drm/rcar-du/rcar_du_writeback.c > drivers/gpu/drm/rcar-du/rcar_lvds.c > > of those, these are configurable: > > rcar_du_vsp.c # DRM_RCAR_VSP > rcar_du_writeback.c # DRM_RCAR_WRITEBACK > rcar_lvds.c # DRM_RCAR_LVDS > > But VSP and WRITEBACK are already implicitly dependant upon DRM_RCAR_DU > because they get built into the DU module. > > So indeed, only the RCAR_LVDS is a separate module, using those helpers, > so I think a select is a reasonable fix. > > I would also ask whether DRM_RCAR_LVDS should depend upon DRM_RCAR_DU > though as well. > > There is no linkage requirement, as it's a standalone bridge driver from > what I can see, but I don't think it serves much purpose without the DU? I have actually spotted when using arch/arm/configs/multi_v7_defconfig where DRM_RCAR_DU is built as module. But when I was reviewing it I was able to compile RCAR_LVDS=y with DRM_RCAR_DU=n. Also, according to https://patchwork.kernel.org/patch/10159063/, the LVDS encoders used to be described as part of the DU but not after the patch. So, I assume it can be used without the DU but not completely sure. > > Anyway, even if it's just for compile testing maybe, the select here > should be fine. > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > > > Signed-off-by: Daniel Gomez <dagmcr@xxxxxxxxx> > > --- > > drivers/gpu/drm/rcar-du/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig > > index 0919f1f159a4..f65d1489dc50 100644 > > --- a/drivers/gpu/drm/rcar-du/Kconfig > > +++ b/drivers/gpu/drm/rcar-du/Kconfig > > @@ -31,6 +31,7 @@ config DRM_RCAR_DW_HDMI > > config DRM_RCAR_LVDS > > tristate "R-Car DU LVDS Encoder Support" > > depends on DRM && DRM_BRIDGE && OF > > + select DRM_KMS_HELPER > > select DRM_PANEL > > select OF_FLATTREE > > select OF_OVERLAY > > >
