Message ID | 20221018181828.19528-1-rdunlap@infradead.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp2098006wrs; Tue, 18 Oct 2022 11:20:21 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5i+OBsy8ehssvLPb3kCuTIdxJ2uxZUu8JkoySryRKNdun+Luu5pk8+VhOzki/H7aDd/+Bg X-Received: by 2002:a17:902:d4c5:b0:183:6e51:503 with SMTP id o5-20020a170902d4c500b001836e510503mr4282145plg.84.1666117221192; Tue, 18 Oct 2022 11:20:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666117221; cv=none; d=google.com; s=arc-20160816; b=eWZdrUnVSuCvWWKkp16fOf+ZplByqlbioyJj1+Vj1zjg9aUMghe3xGbS9wxySi/ZPR 7oOpghRkoWGUjfCmni1oRI1aLnQc0hInmNvZuT8nOL66cvsvZb+CfQpMIku88ZYz19J/ B77ZafmaID3P70Ixhw7kXIX2LSxoQ9VYJXgZH0jZS7dgtGMc6L1cC3BArtn724L69jtm IJfA5oqnk2QpjQTp9MT75uXGCJZE5AozrsN/ETHEIohDpT4Wnld1R1kaNRf98d2My43a mjMa+wKD4RfrLIe+XLHPPzHZyF4ai5qOkZs12NVEyeT5mfXgSqlGPrq283wiiAE9wFPb 6R3g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=u1jSsa7KqxmCrkOFvnCR72ZfQLGaAXjkoUkUU1lr3G8=; b=U8DDt0/1B8pZ5bwgA6WKBrqdeAr0V1juiOQzz+a8k/GvY7V30roQp0dbw5tdZO5s9L VW17Sl7lRNiI5S+3OgR5NXtEaXvoyHOozwYDa/GNoUCDwcY4gEjiLnB9Miw6nh83IywT 4jt9aOVx7jgouIvl9EOOFEQT3NHz4vxDMhVATW+Ec4p+JE+uygo5CTSsK3TBjqAb0ljF C5Vu/CgPGUxaOaAmHRnNc6JavNPr+05TrcbKi0qL31qoU3GXm+NO0TJkiCmV/peKCvEr YxxtU+eLM0YgKliSKN9iQqsGTmEpknwTASQ8ED/WgnZ8QJChvNnthJ4cFKAIL7kHBLoM qAkw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@infradead.org header.s=casper.20170209 header.b=KJi6nSw4; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id r27-20020a63441b000000b0045497c0ba94si15576891pga.635.2022.10.18.11.20.07; Tue, 18 Oct 2022 11:20:21 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@infradead.org header.s=casper.20170209 header.b=KJi6nSw4; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229975AbiJRSSo (ORCPT <rfc822;toshivichauhan@gmail.com> + 99 others); Tue, 18 Oct 2022 14:18:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33388 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229846AbiJRSSm (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 18 Oct 2022 14:18:42 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 91DE180F4E; Tue, 18 Oct 2022 11:18:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Content-Transfer-Encoding:MIME-Version: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:In-Reply-To:References; bh=u1jSsa7KqxmCrkOFvnCR72ZfQLGaAXjkoUkUU1lr3G8=; b=KJi6nSw4Ag2zEO7s5ov0ONErxv fGEmJ9nNV38vIpdnD8BPWG8CEY/cFTNQ8G2Lyt0q3Qbn+JcyFAkEC09BjAGh6JLQhmm31W0Vidb2g rqe1daF/U/g1Z+D1X9f3jelTyP8XHMVgd5h1qDpf2MfEbDsA2KLSx4LVt9VS8nwBNmRo5DE/2F2uL xNS8KFJtysjer8qzcMw8dcB1va8LBi45n2sSKJF3f142famaY51K9ky6UgVOxVAyZ6AMI6PkF4whT +kGGGTsmnq9BOab4VZofipUqypVmeZcDzWICjJ951bpTSx9B27+EqObVuANkLjFczLufynTP+vXxH dOcDlTYw==; Received: from [2601:1c2:d80:3110::a2e7] (helo=casper.infradead.org) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1okrAk-00AvHD-HN; Tue, 18 Oct 2022 18:18:40 +0000 From: Randy Dunlap <rdunlap@infradead.org> To: linux-kernel@vger.kernel.org Cc: Randy Dunlap <rdunlap@infradead.org>, Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>, Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>, LUU HOAI <hoai.luu.ub@renesas.com>, dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch> Subject: [PATCH] drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI Date: Tue, 18 Oct 2022 11:18:28 -0700 Message-Id: <20221018181828.19528-1-rdunlap@infradead.org> X-Mailer: git-send-email 2.38.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747050531328643481?= X-GMAIL-MSGID: =?utf-8?q?1747050531328643481?= |
Series |
drm: rcar_du: DRM_RCAR_DU optionally depends on RCAR_MIPI_DSI
|
|
Commit Message
Randy Dunlap
Oct. 18, 2022, 6:18 p.m. UTC
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(+)
Comments
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
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
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
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 >
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 >
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.
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.
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
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