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

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	dri-devel@lists.freedesktop.org,
	linux-renesas-soc@vger.kernel.org,
	Kieran Bingham <kieran.bingham@ideasonboard.com>
Subject: Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
Date: Mon, 3 Oct 2022 09:01:38 +0200	[thread overview]
Message-ID: <CAMuHMdUV7TY=mfTS6ShOqPbT8JAC-nQFmUWdsyGtP86Op+0SaQ@mail.gmail.com> (raw)
In-Reply-To: <b1425f66-29cf-ce13-fe29-895cd2140c77@ideasonboard.com>

Hi Tomi,

On Mon, Oct 3, 2022 at 8:56 AM Tomi Valkeinen
<tomi.valkeinen@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@intel.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@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...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds