[PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI

[Tomi Valkeinen]

On 03/10/2022 10:01, Geert Uytterhoeven wrote:
> Hi Tomi,
> 
> On Mon, Oct 3, 2022 at 8:56 AM Tomi Valkeinen
> <tomi.valkeinen at ideasonboard.com> 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 at intel.com>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
>>> ---
>>>    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@glider.be/
> 
> 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?

  Tomi