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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/10/2022 10:01, Geert Uytterhoeven wrote:
Hi Tomi,

On Mon, Oct 3, 2022 at 8:56 AM Tomi Valkeinen
<tomi.valkeinen@xxxxxxxxxxxxxxxx> 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?

 Tomi



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux