* [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
@ 2022-10-18 18:18 Randy Dunlap
2022-11-03 6:06 ` Randy Dunlap
2022-11-09 14:11 ` Laurent Pinchart
0 siblings, 2 replies; 9+ messages in thread
From: Randy Dunlap @ 2022-10-18 18:18 UTC (permalink / raw)
To: linux-kernel
Cc: Laurent Pinchart, Randy Dunlap, dri-devel, linux-renesas-soc,
Kieran Bingham, Tomi Valkeinen, LUU HOAI
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
2022-10-18 18:18 [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI Randy Dunlap
@ 2022-11-03 6:06 ` Randy Dunlap
2022-11-03 10:59 ` Kieran Bingham
2022-11-09 14:11 ` Laurent Pinchart
1 sibling, 1 reply; 9+ messages in thread
From: Randy Dunlap @ 2022-11-03 6:06 UTC (permalink / raw)
To: linux-kernel
Cc: Laurent Pinchart, dri-devel, linux-renesas-soc, Kieran Bingham,
Tomi Valkeinen, LUU HOAI
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
2022-11-03 6:06 ` Randy Dunlap
@ 2022-11-03 10:59 ` Kieran Bingham
2022-11-03 11:53 ` Javier Martinez Canillas
0 siblings, 1 reply; 9+ messages in thread
From: Kieran Bingham @ 2022-11-03 10:59 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel
Cc: Laurent Pinchart, dri-devel, linux-renesas-soc, Tomi Valkeinen, LUU HOAI
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
2022-11-03 10:59 ` Kieran Bingham
@ 2022-11-03 11:53 ` Javier Martinez Canillas
2022-11-03 11:56 ` Javier Martinez Canillas
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2022-11-03 11:53 UTC (permalink / raw)
To: Kieran Bingham, Randy Dunlap, linux-kernel
Cc: linux-renesas-soc, Laurent Pinchart, LUU HOAI, dri-devel, Tomi Valkeinen
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
2022-11-03 11:53 ` Javier Martinez Canillas
@ 2022-11-03 11:56 ` Javier Martinez Canillas
2022-11-03 11:59 ` Kieran Bingham
2022-11-03 16:26 ` Randy Dunlap
2 siblings, 0 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2022-11-03 11:56 UTC (permalink / raw)
To: Kieran Bingham, Randy Dunlap, linux-kernel
Cc: linux-renesas-soc, Laurent Pinchart, LUU HOAI, dri-devel, Tomi Valkeinen
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
2022-11-03 11:53 ` Javier Martinez Canillas
2022-11-03 11:56 ` Javier Martinez Canillas
@ 2022-11-03 11:59 ` Kieran Bingham
2022-11-03 16:26 ` Randy Dunlap
2 siblings, 0 replies; 9+ messages in thread
From: Kieran Bingham @ 2022-11-03 11:59 UTC (permalink / raw)
To: Javier Martinez Canillas, Randy Dunlap, linux-kernel
Cc: linux-renesas-soc, Laurent Pinchart, LUU HOAI, dri-devel, Tomi Valkeinen
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
2022-11-03 11:53 ` Javier Martinez Canillas
2022-11-03 11:56 ` Javier Martinez Canillas
2022-11-03 11:59 ` Kieran Bingham
@ 2022-11-03 16:26 ` Randy Dunlap
2022-11-03 18:31 ` Javier Martinez Canillas
2 siblings, 1 reply; 9+ messages in thread
From: Randy Dunlap @ 2022-11-03 16:26 UTC (permalink / raw)
To: Javier Martinez Canillas, Kieran Bingham, linux-kernel
Cc: linux-renesas-soc, Laurent Pinchart, LUU HOAI, dri-devel, Tomi Valkeinen
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
2022-11-03 16:26 ` Randy Dunlap
@ 2022-11-03 18:31 ` Javier Martinez Canillas
0 siblings, 0 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2022-11-03 18:31 UTC (permalink / raw)
To: Randy Dunlap, Kieran Bingham, linux-kernel
Cc: linux-renesas-soc, Laurent Pinchart, LUU HOAI, dri-devel, Tomi Valkeinen
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
2022-10-18 18:18 [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI Randy Dunlap
2022-11-03 6:06 ` Randy Dunlap
@ 2022-11-09 14:11 ` Laurent Pinchart
1 sibling, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2022-11-09 14:11 UTC (permalink / raw)
To: Randy Dunlap
Cc: linux-kernel, dri-devel, linux-renesas-soc, Kieran Bingham,
Tomi Valkeinen, LUU HOAI
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-11-09 14:12 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-18 18:18 [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI Randy Dunlap
2022-11-03 6:06 ` Randy Dunlap
2022-11-03 10:59 ` Kieran Bingham
2022-11-03 11:53 ` Javier Martinez Canillas
2022-11-03 11:56 ` Javier Martinez Canillas
2022-11-03 11:59 ` Kieran Bingham
2022-11-03 16:26 ` Randy Dunlap
2022-11-03 18:31 ` Javier Martinez Canillas
2022-11-09 14:11 ` Laurent Pinchart
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).