Hi Tomi,
On Mon, Oct 03, 2022 at 10:31:24AM +0300, Tomi Valkeinen wrote:
> On 03/10/2022 10:01, Geert Uytterhoeven wrote:
> > On Mon, Oct 3, 2022 at 8:56 AM Tomi Valkeinen wrote:
> >> On 02/10/2022 01:03, Laurent Pinchart wrote:
> >>> When the R-Car MIPI DSI driver was added, it was a standalone encoder
> >>> driver without any dependency to or from the R-Car DU driver. Commit
> >>> 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence") then
> >>> added a direct call from the DU driver to the MIPI DSI driver, without
> >>> updating Kconfig to take the new dependency into account. Fix it the
> >>> same way that the LVDS encoder is handled.
> >>>
> >>> Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
> >>> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> >>> ---
> >>> drivers/gpu/drm/rcar-du/Kconfig | 13 +++++++++----
> >>> 1 file changed, 9 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> >>> index c959e8c6be7d..fd2c2eaee26b 100644
> >>> --- a/drivers/gpu/drm/rcar-du/Kconfig
> >>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> >>> @@ -44,12 +44,17 @@ config DRM_RCAR_LVDS
> >>> select OF_FLATTREE
> >>> select OF_OVERLAY
> >>>
> >>> +config DRM_RCAR_USE_MIPI_DSI
> >>> + bool "R-Car DU MIPI DSI Encoder Support"
> >>> + depends on DRM_BRIDGE && OF
> >>> + default DRM_RCAR_DU
> >>> + help
> >>> + Enable support for the R-Car Display Unit embedded MIPI DSI encoders.
> >>> +
> >>> config DRM_RCAR_MIPI_DSI
> >>> - tristate "R-Car DU MIPI DSI Encoder Support"
> >>> - depends on DRM && DRM_BRIDGE && OF
> >>> + def_tristate DRM_RCAR_DU
> >>> + depends on DRM_RCAR_USE_MIPI_DSI
> >>> select DRM_MIPI_DSI
> >>> - help
> >>> - Enable support for the R-Car Display Unit embedded MIPI DSI encoders.
> >>>
> >>> config DRM_RCAR_VSP
> >>> bool "R-Car DU VSP Compositor Support" if ARM
> >>>
> >>> base-commit: 7860d720a84c74b2761c6b7995392a798ab0a3cb
> >>
> >> Interesting dependency issue. Took me a while to understand it =).
> >>
> >> But is there a reason to not have "depends on DRM_RCAR_DU" for
> >> DRM_RCAR_USE_MIPI_DSI and DRM_RCAR_USE_LVDS? Now the menu items are
> >> available even if RCAR_DU is n. That's also the case for
> >> DRM_RCAR_DW_HDMI, but I'm not sure if that's supposed to be usable even
> >> without RCAR_DU.
> >
> > See
> > https://lore.kernel.org/linux-renesas-soc/cover.1639559338.git.geert+renesas@xxxxxxxxx/
> >
> > Didn't get to implement the suggested improvements yet...
>
> Ok, looks good.
>
> But I started to wonder, for LVDS and DSI (maybe for HDMI too), what's
> the point of modules here...
>
> If RCAR_DU, LVDS and DSI are compiled as modules, you can load either
> LVDS or DSI module, but those won't do anything alone. So in practice
> you always need to load RCAR_DU module too. But if LVDS or DSI are
> enabled in the kconfig, you _have_ to load those before RCAR_DU.
>
> In other words, you can never, e.g. load RCAR_DU and LVDS, but not DSI,
> if all three are enabled.
>
> So why not just compile LVDS and DSI into the RCAR_DU module,
> simplifying the dependencies, removing this issue altogether?
Patches are welcome :-) I'd do that for v6.2 though, not as a v6.1 fix.
--
Regards,
Laurent Pinchart
