On Fri, 5 Apr 2019 at 11:20, Noralf Trønnes <nor...@tronnes.org> wrote:
>
>
>
> Den 05.04.2019 07.28, skrev Joel Stanley:
> > When building this driver for architectures where CMA is not available.
> >
> > Fixes: 4f2a8f5898ec ("drm: Add ASPEED GFX driver")
> > Reported-by: Stephen Rothwell <s...@canb.auug.org.au>
> > Reported-by: kernel test robot <l...@intel.com>
> > Signed-off-by: Joel Stanley <j...@jms.id.au>
> > ---
> > This fixes the build break.
> >
> > Another question is if we need to select this at all, as we have a
> > reserved memory region to allocate from. I am not familiar with this
> > area, so advice is welcome.
> >
> > drivers/gpu/drm/aspeed/Kconfig | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/aspeed/Kconfig b/drivers/gpu/drm/aspeed/Kconfig
> > index 42b74d18a41b..1775fb85ec74 100644
> > --- a/drivers/gpu/drm/aspeed/Kconfig
> > +++ b/drivers/gpu/drm/aspeed/Kconfig
> > @@ -4,8 +4,8 @@ config DRM_ASPEED_GFX
> > select DRM_KMS_HELPER
> > select DRM_KMS_CMA_HELPER
> > select DRM_PANEL
> > - select DMA_CMA
> > - select CMA
> > + select DMA_CMA if HAVE_DMA_CONTIGUOUS
> > + select CMA if HAVE_DMA_CONTIGUOUS
>
> I'm no expert on this, but the rule is that you should not select
> options that are user visible which both of these are. The user should
> select them. So I believe that you should remove them.
>
> Docs: Documentation/kbuild/kconfig-language.txt
I agree with your point, I was following what etnaviv did in this case.
As it's written the driver requires CMA to be enabled, or else it
fails to probe:
[ 3.398247] aspeed_gfx 1e6e6000.display: failed to initialize
reserved mem: -22
[ 3.410471] aspeed_gfx: probe of 1e6e6000.display failed with error -22
It was suggested by Michael that 'imply' might be appropriate here.
However, in my testing 'imply' does not set CMA=y . If I start from
both CMA and ASPEED_GFX disabled, and then use menuconfig to enable
ASPEED_GFX, I expected CMA to be enabled. It does not.
In v2[1] of this patch I make the driver depend on ARCH_ASPEED, which
being ARM means it has CMA available. The other case is when enabling
COMPILE_TEST, where the test for HAVE_DMA_CONTIGUOUS will stop the
build break.
CMA only depends on MMU, so we should be safe to select it. I'm open
to other suggstions though.
Cheers,
Joel
[1]
https://lore.kernel.org/linux-arm-kernel/20190405081117.27339-1-j...@jms.id.au/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
