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

[Philipp Zabel]

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@xxxxxxxxxxxxxx> 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@xxxxxxxx>
> >> ---
> >> 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