| Message ID | 20170721205625.3375683-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Arnd, On Fri, 2017-07-21 at 22:56 +0200, Arnd Bergmann wrote: > The new PRE/PRG driver code causes a link failure when DRM is disabled: > > drivers/gpu/ipu-v3/ipu-pre.o: In function `ipu_pre_configure': > ipu-pre.c:(.text.ipu_pre_configure+0x18): undefined reference to `drm_format_info' > drivers/gpu/ipu-v3/ipu-prg.o: In function `ipu_prg_format_supported': > ipu-prg.c:(.text.ipu_prg_format_supported+0x8): undefined reference to `drm_format_info' > > Adding a Kconfig dependency on DRM means we don't run into this problem > any more. This might not be the best solution though, as the ipu seems > to have been intentionally kept separate from DRM in the past. > > Fixes: ea9c260514c1 ("gpu: ipu-v3: add driver for Prefetch Resolve Gasket") > Link: https://patchwork.kernel.org/patch/9636665/ > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > Originally sent on March 21, but got no reply for it. Resending unchanged > as it is still needed in v4.13-rc1 thank you for fix and for the resend. I have the original patch in my inbox, I'm sorry I overlooked it. I would still like to keep the ipu-v3 driver buildable without DRM enabled. For now, I have applied your patch as is. regards Philipp
On Mon, Jul 24, 2017 at 10:05 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > On Fri, 2017-07-21 at 22:56 +0200, Arnd Bergmann wrote: >> The new PRE/PRG driver code causes a link failure when DRM is disabled: >> >> drivers/gpu/ipu-v3/ipu-pre.o: In function `ipu_pre_configure': >> ipu-pre.c:(.text.ipu_pre_configure+0x18): undefined reference to `drm_format_info' >> drivers/gpu/ipu-v3/ipu-prg.o: In function ` ': >> ipu-prg.c:(.text.ipu_prg_format_supported+0x8): undefined reference to `drm_format_info' >> >> Adding a Kconfig dependency on DRM means we don't run into this problem >> any more. This might not be the best solution though, as the ipu seems >> to have been intentionally kept separate from DRM in the past. >> >> Fixes: ea9c260514c1 ("gpu: ipu-v3: add driver for Prefetch Resolve Gasket") >> Link: https://patchwork.kernel.org/patch/9636665/ >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> Originally sent on March 21, but got no reply for it. Resending unchanged >> as it is still needed in v4.13-rc1 > > thank you for fix and for the resend. I have the original patch in my > inbox, I'm sorry I overlooked it. > > I would still like to keep the ipu-v3 driver buildable without DRM > enabled. For now, I have applied your patch as is. Ok, thanks! I'm pretty sure we can find a way to solve it so that you don't depend on DRM, but we should discuss what that would look like. Are you mainly interested in being able to build-test without DRM, or do you actually want the ipu_pre_configure() and ipu_prg_format_supported() functions to work correctly in that case? If you only need build-testing, you could have a simple wrapper like const struct drm_format_info *ipu_format_info(u32 format) { static const struct drm_format_info invalid = {}; if (!IS_REACHABLE(CONFIG_DRM)) return &invalid; return drm_format_info(format); } The more useful way to solve it would be more work, we could for instance move (parts of) drivers/gpu/drm/drm_fourcc.c into lib/fourcc.c and have it built whenever at least DRM or IPU_v3 are enabled. Arnd
On Tue, 2017-07-25 at 09:33 +0200, Arnd Bergmann wrote: > On Mon, Jul 24, 2017 at 10:05 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > > On Fri, 2017-07-21 at 22:56 +0200, Arnd Bergmann wrote: > >> The new PRE/PRG driver code causes a link failure when DRM is disabled: > >> > >> drivers/gpu/ipu-v3/ipu-pre.o: In function `ipu_pre_configure': > >> ipu-pre.c:(.text.ipu_pre_configure+0x18): undefined reference to `drm_format_info' > >> drivers/gpu/ipu-v3/ipu-prg.o: In function ` ': > >> ipu-prg.c:(.text.ipu_prg_format_supported+0x8): undefined reference to `drm_format_info' > >> > >> Adding a Kconfig dependency on DRM means we don't run into this problem > >> any more. This might not be the best solution though, as the ipu seems > >> to have been intentionally kept separate from DRM in the past. > >> > >> Fixes: ea9c260514c1 ("gpu: ipu-v3: add driver for Prefetch Resolve Gasket") > >> Link: https://patchwork.kernel.org/patch/9636665/ > >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > >> --- > >> Originally sent on March 21, but got no reply for it. Resending unchanged > >> as it is still needed in v4.13-rc1 > > > > thank you for fix and for the resend. I have the original patch in my > > inbox, I'm sorry I overlooked it. > > > > I would still like to keep the ipu-v3 driver buildable without DRM > > enabled. For now, I have applied your patch as is. > > Ok, thanks! > > I'm pretty sure we can find a way to solve it so that you don't depend > on DRM, but we should discuss what that would look like. > > Are you mainly interested in being able to build-test without DRM, Both that, and I'd like to keep the ability to disable DRM for devices that only need the video capture parts of the IPUv3 to work. An example would be a network connected camera without display or need for GPU processing. > or do you actually want the ipu_pre_configure() and > ipu_prg_format_supported() functions to work correctly in that case? In both cases the PRE and PRG don't have to be usable, as their only function is pixel data prefetching for the display path. > If you only need build-testing, you could have a simple wrapper like > > const struct drm_format_info *ipu_format_info(u32 format) > { > static const struct drm_format_info invalid = {}; > > if (!IS_REACHABLE(CONFIG_DRM)) > return &invalid; > > return drm_format_info(format); > } That should work fine. Both ipu_prg_format_supported and ipu_prg_channel_configure are only ever called by DRM code. > The more useful way to solve it would be more work, we could for > instance move (parts of) drivers/gpu/drm/drm_fourcc.c into > lib/fourcc.c and have it built whenever at least DRM or IPU_v3 > are enabled. I don't think this is necessary, at least for this case. regards Philipp
On Tue, Jul 25, 2017 at 10:03 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > On Tue, 2017-07-25 at 09:33 +0200, Arnd Bergmann wrote: >> On Mon, Jul 24, 2017 at 10:05 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: >> > On Fri, 2017-07-21 at 22:56 +0200, Arnd Bergmann wrote: >> If you only need build-testing, you could have a simple wrapper like >> >> const struct drm_format_info *ipu_format_info(u32 format) >> { >> static const struct drm_format_info invalid = {}; >> >> if (!IS_REACHABLE(CONFIG_DRM)) >> return &invalid; >> >> return drm_format_info(format); >> } > > That should work fine. Both ipu_prg_format_supported and > ipu_prg_channel_configure are only ever called by DRM code. Ok, in that case, I think we should just make the compilation of PRE and PRG conditional and leave them out when DRM is not built-in (be aware of the case of DRM=m, IPU=y). Arnd
On Tue, 2017-07-25 at 13:35 +0200, Arnd Bergmann wrote: > On Tue, Jul 25, 2017 at 10:03 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > > On Tue, 2017-07-25 at 09:33 +0200, Arnd Bergmann wrote: > >> On Mon, Jul 24, 2017 at 10:05 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > >> > On Fri, 2017-07-21 at 22:56 +0200, Arnd Bergmann wrote: > > >> If you only need build-testing, you could have a simple wrapper like > >> > >> const struct drm_format_info *ipu_format_info(u32 format) > >> { > >> static const struct drm_format_info invalid = {}; > >> > >> if (!IS_REACHABLE(CONFIG_DRM)) > >> return &invalid; > >> > >> return drm_format_info(format); > >> } > > > > That should work fine. Both ipu_prg_format_supported and > > ipu_prg_channel_configure are only ever called by DRM code. > > Ok, in that case, I think we should just make the compilation > of PRE and PRG conditional and leave them out when DRM > is not built-in (be aware of the case of DRM=m, IPU=y). Maybe we should just: config IMX_IPUV3_CORE depends on DRM || !DRM # if DRM=m, this can't be 'y' regards Philipp
On Tue, Jul 25, 2017 at 1:52 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > On Tue, 2017-07-25 at 13:35 +0200, Arnd Bergmann wrote: >> On Tue, Jul 25, 2017 at 10:03 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: >> > On Tue, 2017-07-25 at 09:33 +0200, Arnd Bergmann wrote: >> >> On Mon, Jul 24, 2017 at 10:05 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: >> >> > On Fri, 2017-07-21 at 22:56 +0200, Arnd Bergmann wrote: >> >> >> If you only need build-testing, you could have a simple wrapper like >> >> >> >> const struct drm_format_info *ipu_format_info(u32 format) >> >> { >> >> static const struct drm_format_info invalid = {}; >> >> >> >> if (!IS_REACHABLE(CONFIG_DRM)) >> >> return &invalid; >> >> >> >> return drm_format_info(format); >> >> } >> > >> > That should work fine. Both ipu_prg_format_supported and >> > ipu_prg_channel_configure are only ever called by DRM code. >> >> Ok, in that case, I think we should just make the compilation >> of PRE and PRG conditional and leave them out when DRM >> is not built-in (be aware of the case of DRM=m, IPU=y). > > Maybe we should just: > > config IMX_IPUV3_CORE > depends on DRM || !DRM # if DRM=m, this can't be 'y' Yes, that would solve that case. I see now that the DRM=n case is already handled by 30310c835f3e ("gpu: ipu-v3: don't depend on DRM being enabled"), so that is probably sufficient. I've added that change to my randconfig tree now and will give it some more testing. If you don't hear back from me, please assume that it works, and send that version upstream. Arnd
diff --git a/drivers/gpu/ipu-v3/Kconfig b/drivers/gpu/ipu-v3/Kconfig index 08766c6e7856..f8b200801408 100644 --- a/drivers/gpu/ipu-v3/Kconfig +++ b/drivers/gpu/ipu-v3/Kconfig @@ -1,6 +1,7 @@ config IMX_IPUV3_CORE tristate "IPUv3 core support" depends on SOC_IMX5 || SOC_IMX6Q || ARCH_MULTIPLATFORM + depends on DRM select GENERIC_IRQ_CHIP help Choose this if you have a i.MX5/6 system and want to use the Image
The new PRE/PRG driver code causes a link failure when DRM is disabled: drivers/gpu/ipu-v3/ipu-pre.o: In function `ipu_pre_configure': ipu-pre.c:(.text.ipu_pre_configure+0x18): undefined reference to `drm_format_info' drivers/gpu/ipu-v3/ipu-prg.o: In function `ipu_prg_format_supported': ipu-prg.c:(.text.ipu_prg_format_supported+0x8): undefined reference to `drm_format_info' Adding a Kconfig dependency on DRM means we don't run into this problem any more. This might not be the best solution though, as the ipu seems to have been intentionally kept separate from DRM in the past. Fixes: ea9c260514c1 ("gpu: ipu-v3: add driver for Prefetch Resolve Gasket") Link: https://patchwork.kernel.org/patch/9636665/ Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- Originally sent on March 21, but got no reply for it. Resending unchanged as it is still needed in v4.13-rc1 --- drivers/gpu/ipu-v3/Kconfig | 1 + 1 file changed, 1 insertion(+)