linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
@ 2022-10-01 22:03 Laurent Pinchart
  2022-10-03  6:52 ` Tomi Valkeinen
  2022-11-14  9:05 ` Geert Uytterhoeven
  0 siblings, 2 replies; 8+ messages in thread
From: Laurent Pinchart @ 2022-10-01 22:03 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-renesas-soc, Tomi Valkeinen, Kieran Bingham

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
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
  2022-10-01 22:03 [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI Laurent Pinchart
@ 2022-10-03  6:52 ` Tomi Valkeinen
  2022-10-03  7:01   ` Geert Uytterhoeven
  2022-11-14  9:05 ` Geert Uytterhoeven
  1 sibling, 1 reply; 8+ messages in thread
From: Tomi Valkeinen @ 2022-10-03  6:52 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel; +Cc: linux-renesas-soc, Kieran Bingham

Hi,

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.

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
  2022-10-03  6:52 ` Tomi Valkeinen
@ 2022-10-03  7:01   ` Geert Uytterhoeven
  2022-10-03  7:31     ` Tomi Valkeinen
  0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2022-10-03  7:01 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Laurent Pinchart, dri-devel, linux-renesas-soc, Kieran Bingham

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
  2022-10-03  7:01   ` Geert Uytterhoeven
@ 2022-10-03  7:31     ` Tomi Valkeinen
  2022-10-03 12:26       ` Laurent Pinchart
  0 siblings, 1 reply; 8+ messages in thread
From: Tomi Valkeinen @ 2022-10-03  7:31 UTC (permalink / raw)
  To: Geert Uytterhoeven, Laurent Pinchart
  Cc: dri-devel, linux-renesas-soc, Kieran Bingham

On 03/10/2022 10:01, Geert Uytterhoeven wrote:
> 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...

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
  2022-10-03  7:31     ` Tomi Valkeinen
@ 2022-10-03 12:26       ` Laurent Pinchart
  0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2022-10-03 12:26 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Geert Uytterhoeven, dri-devel, linux-renesas-soc, Kieran Bingham

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@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...
> 
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
  2022-10-01 22:03 [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI Laurent Pinchart
  2022-10-03  6:52 ` Tomi Valkeinen
@ 2022-11-14  9:05 ` Geert Uytterhoeven
  2022-11-14 14:21   ` Laurent Pinchart
  1 sibling, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2022-11-14  9:05 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, linux-renesas-soc, Tomi Valkeinen, Kieran Bingham

Hi Laurent,

On Sun, Oct 2, 2022 at 12:06 AM Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> 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>

Thanks for your patch, which is now commit a830a15678593948
("drm: rcar-du: Fix Kconfig dependency between RCAR_DU
and RCAR_MIPI_DSI") in v6.1-rc5.

> --- 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

This means this driver is now enabled by default on systems that do not
have the MIPI DSI Encoder (e.g. R-Car Gen2), and that we should probably
disable it explicitly in shmobile_defconfig.  Is that intentional?

> +       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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
  2022-11-14  9:05 ` Geert Uytterhoeven
@ 2022-11-14 14:21   ` Laurent Pinchart
  2022-11-14 14:52     ` Geert Uytterhoeven
  0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2022-11-14 14:21 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: dri-devel, linux-renesas-soc, Tomi Valkeinen, Kieran Bingham

Hi Geert,

On Mon, Nov 14, 2022 at 10:05:58AM +0100, Geert Uytterhoeven wrote:
> On Sun, Oct 2, 2022 at 12:06 AM 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>
> 
> Thanks for your patch, which is now commit a830a15678593948
> ("drm: rcar-du: Fix Kconfig dependency between RCAR_DU
> and RCAR_MIPI_DSI") in v6.1-rc5.
> 
> > --- 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
> 
> This means this driver is now enabled by default on systems that do not
> have the MIPI DSI Encoder (e.g. R-Car Gen2), and that we should probably
> disable it explicitly in shmobile_defconfig.  Is that intentional?

I don't think so, no. Would you like to send a patch ? If so, it should
enable the option in relevant defconfig files.

> > +       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

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
  2022-11-14 14:21   ` Laurent Pinchart
@ 2022-11-14 14:52     ` Geert Uytterhoeven
  0 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2022-11-14 14:52 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, linux-renesas-soc, Tomi Valkeinen, Kieran Bingham

 Hi Laurent,

On Mon, Nov 14, 2022 at 3:22 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Mon, Nov 14, 2022 at 10:05:58AM +0100, Geert Uytterhoeven wrote:
> > On Sun, Oct 2, 2022 at 12:06 AM 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>
> >
> > Thanks for your patch, which is now commit a830a15678593948
> > ("drm: rcar-du: Fix Kconfig dependency between RCAR_DU
> > and RCAR_MIPI_DSI") in v6.1-rc5.
> >
> > > --- 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
> >
> > This means this driver is now enabled by default on systems that do not
> > have the MIPI DSI Encoder (e.g. R-Car Gen2), and that we should probably
> > disable it explicitly in shmobile_defconfig.  Is that intentional?
>
> I don't think so, no. Would you like to send a patch ? If so, it should
> enable the option in relevant defconfig files.

You mean just drop the "default DRM_RCAR_DU" here?

I'm wondering if we can solve this in a consistent way.
Currently we have:

    config DRM_RCAR_USE_CMM default DRM_RCAR_DU
    config DRM_RCAR_DW_HDMI default n
    config DRM_RCAR_USE_LVDS default DRM_RCAR_DU
    config DRM_RCAR_MIPI_DSI default n
    config DRM_RCAR_VSP default y if ARM64

HDMI is only used on R-Car Gen3 parts
MIPI_DSI is only used on R-Car Gen4 parts
LVDS is used on R-Car Gen2 and Gen3 parts

Thoughts?

> > > +       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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-11-14 14:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-01 22:03 [PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI Laurent Pinchart
2022-10-03  6:52 ` Tomi Valkeinen
2022-10-03  7:01   ` Geert Uytterhoeven
2022-10-03  7:31     ` Tomi Valkeinen
2022-10-03 12:26       ` Laurent Pinchart
2022-11-14  9:05 ` Geert Uytterhoeven
2022-11-14 14:21   ` Laurent Pinchart
2022-11-14 14:52     ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).