[PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[Randy Dunlap]

When CONFIG_DRM_RCAR_DU=y and CONFIG_DRM_RCAR_MIPI_DSI=m, calls
from the builtin driver to the mipi driver fail due to linker
errors.
Since the RCAR_MIPI_DSI driver is not always required, fix the
build error by making DRM_RCAR_DU optionally depend on the
RCAR_MIPI_DSI Kconfig symbol. This prevents the problematic
kconfig combination without requiring that RCAR_MIPI_DSI always
be enabled.

aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_enable':
rcar_du_crtc.c:(.text+0x3a18): undefined reference to `rcar_mipi_dsi_pclk_enable'
aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_disable':
rcar_du_crtc.c:(.text+0x47cc): undefined reference to `rcar_mipi_dsi_pclk_disable'

Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: LUU HOAI <hoai.luu.ub@renesas.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/rcar-du/Kconfig |    1 +
 1 file changed, 1 insertion(+)

diff -- a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -4,6 +4,7 @@ config DRM_RCAR_DU
 	depends on DRM && OF
 	depends on ARM || ARM64
 	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on DRM_RCAR_MIPI_DSI || DRM_RCAR_MIPI_DSI=n
 	select DRM_KMS_HELPER
 	select DRM_GEM_DMA_HELPER
 	select VIDEOMODE_HELPERS

Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[Randy Dunlap]

ping. I have verified (on linux-next-20221103) that this is still needed.
Thanks.

On 10/18/22 11:18, Randy Dunlap wrote:
> When CONFIG_DRM_RCAR_DU=y and CONFIG_DRM_RCAR_MIPI_DSI=m, calls
> from the builtin driver to the mipi driver fail due to linker
> errors.
> Since the RCAR_MIPI_DSI driver is not always required, fix the
> build error by making DRM_RCAR_DU optionally depend on the
> RCAR_MIPI_DSI Kconfig symbol. This prevents the problematic
> kconfig combination without requiring that RCAR_MIPI_DSI always
> be enabled.
> 
> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_enable':
> rcar_du_crtc.c:(.text+0x3a18): undefined reference to `rcar_mipi_dsi_pclk_enable'
> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_disable':
> rcar_du_crtc.c:(.text+0x47cc): undefined reference to `rcar_mipi_dsi_pclk_disable'
> 
> Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: LUU HOAI <hoai.luu.ub@renesas.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
>  drivers/gpu/drm/rcar-du/Kconfig |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff -- a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -4,6 +4,7 @@ config DRM_RCAR_DU
>  	depends on DRM && OF
>  	depends on ARM || ARM64
>  	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on DRM_RCAR_MIPI_DSI || DRM_RCAR_MIPI_DSI=n
>  	select DRM_KMS_HELPER
>  	select DRM_GEM_DMA_HELPER
>  	select VIDEOMODE_HELPERS

-- 
~Randy

Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[Kieran Bingham]

Hi Randy,

Quoting Randy Dunlap (2022-11-03 06:06:45)
> ping. I have verified (on linux-next-20221103) that this is still needed.
> Thanks.
> 
> On 10/18/22 11:18, Randy Dunlap wrote:
> > When CONFIG_DRM_RCAR_DU=y and CONFIG_DRM_RCAR_MIPI_DSI=m, calls
> > from the builtin driver to the mipi driver fail due to linker
> > errors.
> > Since the RCAR_MIPI_DSI driver is not always required, fix the
> > build error by making DRM_RCAR_DU optionally depend on the
> > RCAR_MIPI_DSI Kconfig symbol. This prevents the problematic
> > kconfig combination without requiring that RCAR_MIPI_DSI always
> > be enabled.
> > 
> > aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_enable':
> > rcar_du_crtc.c:(.text+0x3a18): undefined reference to `rcar_mipi_dsi_pclk_enable'
> > aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_disable':
> > rcar_du_crtc.c:(.text+0x47cc): undefined reference to `rcar_mipi_dsi_pclk_disable'
> > 
> > Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > Cc: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > Cc: LUU HOAI <hoai.luu.ub@renesas.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-renesas-soc@vger.kernel.org
> > Cc: David Airlie <airlied@gmail.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > ---
> >  drivers/gpu/drm/rcar-du/Kconfig |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff -- a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -4,6 +4,7 @@ config DRM_RCAR_DU
> >       depends on DRM && OF
> >       depends on ARM || ARM64
> >       depends on ARCH_RENESAS || COMPILE_TEST
> > +     depends on DRM_RCAR_MIPI_DSI || DRM_RCAR_MIPI_DSI=n

Please forgive my ignorance, but I don't understand how this works.
Could you explain what this is doing please?

I know you've explained above that it fixes it to optionally depend on
DRM_RCAR_MIPI_DSI ... but it's not making sense to me.

To me - this is saying we depend on DRM_RCAR_MIPI_DSI being enabled, or
not being enabled ... ? Which is like saying if (0 || 1) ?

I'm guessing I'm missing something obvious :-S

--
Kieran


> >       select DRM_KMS_HELPER
> >       select DRM_GEM_DMA_HELPER
> >       select VIDEOMODE_HELPERS
> 
> -- 
> ~Randy

Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[Javier Martinez Canillas]

Hello Kieran,

On 11/3/22 11:59, Kieran Bingham wrote:
> Hi Randy,
> 
> Quoting Randy Dunlap (2022-11-03 06:06:45)
>> ping. I have verified (on linux-next-20221103) that this is still needed.
>> Thanks.
>>
>> On 10/18/22 11:18, Randy Dunlap wrote:
>>> When CONFIG_DRM_RCAR_DU=y and CONFIG_DRM_RCAR_MIPI_DSI=m, calls
>>> from the builtin driver to the mipi driver fail due to linker
>>> errors.
>>> Since the RCAR_MIPI_DSI driver is not always required, fix the
>>> build error by making DRM_RCAR_DU optionally depend on the
>>> RCAR_MIPI_DSI Kconfig symbol. This prevents the problematic
>>> kconfig combination without requiring that RCAR_MIPI_DSI always
>>> be enabled.
>>>
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_enable':
>>> rcar_du_crtc.c:(.text+0x3a18): undefined reference to `rcar_mipi_dsi_pclk_enable'
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_disable':
>>> rcar_du_crtc.c:(.text+0x47cc): undefined reference to `rcar_mipi_dsi_pclk_disable'
>>>
>>> Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>> Cc: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>> Cc: LUU HOAI <hoai.luu.ub@renesas.com>
>>> Cc: dri-devel@lists.freedesktop.org
>>> Cc: linux-renesas-soc@vger.kernel.org
>>> Cc: David Airlie <airlied@gmail.com>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> ---
>>>  drivers/gpu/drm/rcar-du/Kconfig |    1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff -- a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>> @@ -4,6 +4,7 @@ config DRM_RCAR_DU
>>>       depends on DRM && OF
>>>       depends on ARM || ARM64
>>>       depends on ARCH_RENESAS || COMPILE_TEST
>>> +     depends on DRM_RCAR_MIPI_DSI || DRM_RCAR_MIPI_DSI=n
> 
> Please forgive my ignorance, but I don't understand how this works.
> Could you explain what this is doing please?
> 
> I know you've explained above that it fixes it to optionally depend on
> DRM_RCAR_MIPI_DSI ... but it's not making sense to me.
> 
> To me - this is saying we depend on DRM_RCAR_MIPI_DSI being enabled, or
> not being enabled ... ? Which is like saying if (0 || 1) ?
> 
> I'm guessing I'm missing something obvious :-S
>

What this Kconfig expression is saying is that it depends on DRM_RCAR_MIPI_DSI=y
if DRM_RCAR_DU=y and DRM_RCAR_MIPI_DSI=m if DRM_RCAR_DU=m. But that the it can
also be satisfied if is not set DRM_RCAR_MIPI_DSI.

This is usually used to make sure that you don't end with a configuration where
DRM_RCAR_MIPI_DSI=y and DRM_RCAR_DU=m or DRM_RCAR_MIPI_DSI=m and DRM_RCAR_DU=y.

Randy, I think that it's more idiomatic though to it express as following:

depends on DRM_RCAR_MIPI_DSI || !DRM_RCAR_MIPI_DSI

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[Javier Martinez Canillas]

On 11/3/22 12:53, Javier Martinez Canillas wrote:

[...]

>>
> 
> What this Kconfig expression is saying is that it depends on DRM_RCAR_MIPI_DSI=y
> if DRM_RCAR_DU=y and DRM_RCAR_MIPI_DSI=m if DRM_RCAR_DU=m. But that the it can

It should had been s/and/or here but you get the idea.

> also be satisfied if is not set DRM_RCAR_MIPI_DSI.
> 
> This is usually used to make sure that you don't end with a configuration where
> DRM_RCAR_MIPI_DSI=y and DRM_RCAR_DU=m or DRM_RCAR_MIPI_DSI=m and DRM_RCAR_DU=y.
> 
> Randy, I think that it's more idiomatic though to it express as following:
> 
> depends on DRM_RCAR_MIPI_DSI || !DRM_RCAR_MIPI_DSI
> 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[Kieran Bingham]

Quoting Javier Martinez Canillas (2022-11-03 11:53:14)
> Hello Kieran,
> 
> On 11/3/22 11:59, Kieran Bingham wrote:
> > Hi Randy,
> > 
> > Quoting Randy Dunlap (2022-11-03 06:06:45)
> >> ping. I have verified (on linux-next-20221103) that this is still needed.
> >> Thanks.
> >>
> >> On 10/18/22 11:18, Randy Dunlap wrote:
> >>> When CONFIG_DRM_RCAR_DU=y and CONFIG_DRM_RCAR_MIPI_DSI=m, calls
> >>> from the builtin driver to the mipi driver fail due to linker
> >>> errors.
> >>> Since the RCAR_MIPI_DSI driver is not always required, fix the
> >>> build error by making DRM_RCAR_DU optionally depend on the
> >>> RCAR_MIPI_DSI Kconfig symbol. This prevents the problematic
> >>> kconfig combination without requiring that RCAR_MIPI_DSI always
> >>> be enabled.
> >>>
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_enable':
> >>> rcar_du_crtc.c:(.text+0x3a18): undefined reference to `rcar_mipi_dsi_pclk_enable'
> >>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_disable':
> >>> rcar_du_crtc.c:(.text+0x47cc): undefined reference to `rcar_mipi_dsi_pclk_disable'
> >>>
> >>> Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
> >>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> >>> Cc: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >>> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >>> Cc: LUU HOAI <hoai.luu.ub@renesas.com>
> >>> Cc: dri-devel@lists.freedesktop.org
> >>> Cc: linux-renesas-soc@vger.kernel.org
> >>> Cc: David Airlie <airlied@gmail.com>
> >>> Cc: Daniel Vetter <daniel@ffwll.ch>
> >>> ---
> >>>  drivers/gpu/drm/rcar-du/Kconfig |    1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff -- a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> >>> --- a/drivers/gpu/drm/rcar-du/Kconfig
> >>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> >>> @@ -4,6 +4,7 @@ config DRM_RCAR_DU
> >>>       depends on DRM && OF
> >>>       depends on ARM || ARM64
> >>>       depends on ARCH_RENESAS || COMPILE_TEST
> >>> +     depends on DRM_RCAR_MIPI_DSI || DRM_RCAR_MIPI_DSI=n
> > 
> > Please forgive my ignorance, but I don't understand how this works.
> > Could you explain what this is doing please?
> > 
> > I know you've explained above that it fixes it to optionally depend on
> > DRM_RCAR_MIPI_DSI ... but it's not making sense to me.
> > 
> > To me - this is saying we depend on DRM_RCAR_MIPI_DSI being enabled, or
> > not being enabled ... ? Which is like saying if (0 || 1) ?
> > 
> > I'm guessing I'm missing something obvious :-S
> >
> 
> What this Kconfig expression is saying is that it depends on DRM_RCAR_MIPI_DSI=y
> if DRM_RCAR_DU=y and DRM_RCAR_MIPI_DSI=m if DRM_RCAR_DU=m. But that the it can
> also be satisfied if is not set DRM_RCAR_MIPI_DSI.
> 
> This is usually used to make sure that you don't end with a configuration where
> DRM_RCAR_MIPI_DSI=y and DRM_RCAR_DU=m or DRM_RCAR_MIPI_DSI=m and DRM_RCAR_DU=y.
> 
> Randy, I think that it's more idiomatic though to it express as following:
> 
> depends on DRM_RCAR_MIPI_DSI || !DRM_RCAR_MIPI_DSI

Ok - thanks, so it's the module part that breaks. I never build modules,
always builtin - so it doesn't hit me ;-)

Anyway - it certainly makes sense now I think so either as posted, or
with the idiomatic proposal from Javier:

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> 
> -- 
> Best regards,
> 
> Javier Martinez Canillas
> Core Platforms
> Red Hat
>

Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[Randy Dunlap]

Hi,

On 11/3/22 04:53, Javier Martinez Canillas wrote:
> Hello Kieran,
> 
> On 11/3/22 11:59, Kieran Bingham wrote:
>> Hi Randy,
>>
>> Quoting Randy Dunlap (2022-11-03 06:06:45)
>>> ping. I have verified (on linux-next-20221103) that this is still needed.
>>> Thanks.
>>>
>>> On 10/18/22 11:18, Randy Dunlap wrote:
>>>> When CONFIG_DRM_RCAR_DU=y and CONFIG_DRM_RCAR_MIPI_DSI=m, calls
>>>> from the builtin driver to the mipi driver fail due to linker
>>>> errors.
>>>> Since the RCAR_MIPI_DSI driver is not always required, fix the
>>>> build error by making DRM_RCAR_DU optionally depend on the
>>>> RCAR_MIPI_DSI Kconfig symbol. This prevents the problematic
>>>> kconfig combination without requiring that RCAR_MIPI_DSI always
>>>> be enabled.
>>>>
>>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_enable':
>>>> rcar_du_crtc.c:(.text+0x3a18): undefined reference to `rcar_mipi_dsi_pclk_enable'
>>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_disable':
>>>> rcar_du_crtc.c:(.text+0x47cc): undefined reference to `rcar_mipi_dsi_pclk_disable'
>>>>
>>>> Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>> Cc: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>>> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>> Cc: LUU HOAI <hoai.luu.ub@renesas.com>
>>>> Cc: dri-devel@lists.freedesktop.org
>>>> Cc: linux-renesas-soc@vger.kernel.org
>>>> Cc: David Airlie <airlied@gmail.com>
>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>> ---
>>>>  drivers/gpu/drm/rcar-du/Kconfig |    1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff -- a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
>>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>>> @@ -4,6 +4,7 @@ config DRM_RCAR_DU
>>>>       depends on DRM && OF
>>>>       depends on ARM || ARM64
>>>>       depends on ARCH_RENESAS || COMPILE_TEST
>>>> +     depends on DRM_RCAR_MIPI_DSI || DRM_RCAR_MIPI_DSI=n
>>
>> Please forgive my ignorance, but I don't understand how this works.
>> Could you explain what this is doing please?
>>
>> I know you've explained above that it fixes it to optionally depend on
>> DRM_RCAR_MIPI_DSI ... but it's not making sense to me.
>>
>> To me - this is saying we depend on DRM_RCAR_MIPI_DSI being enabled, or
>> not being enabled ... ? Which is like saying if (0 || 1) ?
>>
>> I'm guessing I'm missing something obvious :-S

It's kconfig tristate symbols (y/n/m), not boolean. :)

> What this Kconfig expression is saying is that it depends on DRM_RCAR_MIPI_DSI=y
> if DRM_RCAR_DU=y and DRM_RCAR_MIPI_DSI=m if DRM_RCAR_DU=m. But that the it can
> also be satisfied if is not set DRM_RCAR_MIPI_DSI.
> 
> This is usually used to make sure that you don't end with a configuration where
> DRM_RCAR_MIPI_DSI=y and DRM_RCAR_DU=m or DRM_RCAR_MIPI_DSI=m and DRM_RCAR_DU=y.
> 
> Randy, I think that it's more idiomatic though to it express as following:
> 
> depends on DRM_RCAR_MIPI_DSI || !DRM_RCAR_MIPI_DSI

I count just over 200 of each idiom (but my grep strings could be
too crude). I guess that you want a v2 with that change?

Thanks.
-- 
~Randy

Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[Javier Martinez Canillas]

On 11/3/22 17:26, Randy Dunlap wrote:

[...]

>>
>> Randy, I think that it's more idiomatic though to it express as following:
>>
>> depends on DRM_RCAR_MIPI_DSI || !DRM_RCAR_MIPI_DSI
> 
> I count just over 200 of each idiom (but my grep strings could be
> too crude). I guess that you want a v2 with that change?
> 

I believe Kieran was happy with either so no objections from
me. I don't have a strong opinion, I just thought the latter
was more idiomatic but you said that both are used alike.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI

[Laurent Pinchart]

Hi Randy,

Thank you for the patch.

On Tue, Oct 18, 2022 at 11:18:28AM -0700, Randy Dunlap wrote:
> When CONFIG_DRM_RCAR_DU=y and CONFIG_DRM_RCAR_MIPI_DSI=m, calls
> from the builtin driver to the mipi driver fail due to linker
> errors.
> Since the RCAR_MIPI_DSI driver is not always required, fix the
> build error by making DRM_RCAR_DU optionally depend on the
> RCAR_MIPI_DSI Kconfig symbol. This prevents the problematic
> kconfig combination without requiring that RCAR_MIPI_DSI always
> be enabled.
> 
> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_enable':
> rcar_du_crtc.c:(.text+0x3a18): undefined reference to `rcar_mipi_dsi_pclk_enable'
> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function `rcar_du_crtc_atomic_disable':
> rcar_du_crtc.c:(.text+0x47cc): undefined reference to `rcar_mipi_dsi_pclk_disable'

I've already posted a fix, see

https://lore.kernel.org/dri-devel/20221001220342.5828-1-laurent.pinchart+renesas@ideasonboard.com/

It aligns with how the LVDS encoder driver is handled, so I would prefer
that. I will send a pull request shortly, as a v6.1 fix.

> Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: LUU HOAI <hoai.luu.ub@renesas.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
>  drivers/gpu/drm/rcar-du/Kconfig |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff -- a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -4,6 +4,7 @@ config DRM_RCAR_DU
>  	depends on DRM && OF
>  	depends on ARM || ARM64
>  	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on DRM_RCAR_MIPI_DSI || DRM_RCAR_MIPI_DSI=n
>  	select DRM_KMS_HELPER
>  	select DRM_GEM_DMA_HELPER
>  	select VIDEOMODE_HELPERS

-- 
Regards,

Laurent Pinchart