diff mbox

[RESEND] gpu: ipu-v3: add DRM dependency

Message ID 20170721205625.3375683-1-arnd@arndb.de
State Accepted
Commit 2406b296a3a80ba1c78707f205556f2388d474ff
Headers show

Commit Message

Arnd Bergmann July 21, 2017, 8:56 p.m. UTC
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(+)

-- 
2.9.0

Comments

Philipp Zabel July 24, 2017, 8:05 a.m. UTC | #1
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
Philipp Zabel July 25, 2017, 8:03 a.m. UTC | #2
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
Philipp Zabel July 25, 2017, 11:52 a.m. UTC | #3
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
diff mbox

Patch

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