Re: [PATCH v1 2/2] drivers/gpu/drm: don't select DMA_CMA or CMA from aspeed or etnaviv

On Thu, Apr 8, 2021 at 2:00 PM David Hildenbrand [off-list ref] wrote:

On 08.04.21 13:44, Arnd Bergmann wrote:

quoted

On Thu, Apr 8, 2021 at 1:00 PM David Hildenbrand [off-list ref] wrote:

quoted

quoted

It is a somewhat awkward way to say "prevent this symbol from
being =y if the dependency is =m".

What would be the right thing to do in the case here then to achieve the
"if DRMA_ASPEED_GFX is enabled, also enable DMA_CMA id possible"?

One approach could be to have for DMA_CMA

default y if DRMA_ASPEED_GFX

but it feels like the wrong way to tackle this.

I'm still not sure what you are trying to achieve. Is the idea only to provide
a useful default for DMA_CMA depending on which drivers are enabled?

"Random drivers should not override a user configuration of core knobs
(e.g., CONFIG_DMA_CMA=n)."

Let's assume I'm a distribution and want to set CONFIG_CMA=n or want to
set CONFIG_DMA_CMA=n with CONFIG_CMA=y; there is no way to do that with
e.g., DRMA_ASPEED_GFX=y because it will always override my (user!)
setting -- even though it doesn't really always need it. Using "select"
is the problem here.

I agree on the part of removing the 'select' if we don't need it. The
part I couldn't figure out was what the 'imply' is supposed to help with.
Most other users that added imply tried (and failed) to fix a build problem.

quoted

This is something you could do using a hidden helper symbol like

config DRMA_ASPEED_GFX
        bool "Aspeed display driver"
        select DRM_WANT_CMA

config DRM_WANT_CMA
        bool
        help
           Select this from any driver that benefits from CMA being enabled

config DMA_CMA
        bool "Use CMA helpers for DRM"
        default DRM_WANT_CMA

          Arnd

That's precisely what I had first, with an additional "WANT_CMA" --  but
looking at the number of such existing options (I was able to spot 1 !)
I wondered if there is a better approach to achieve the same; "imply"
sounded like a good candidate.

I can probably find a couple more, but regardless of how many others
exist, this would be a much clearer way of doing it than 'imply' since it
has none of the ambiguity and misuse problems.

I think the reason we don't see more is that generally speaking, those
defaults are widely ignored anyway. You almost always start out with
a defconfig file that contains everything you know you need, and then
you add bits to that. Having the default in any form only helps to
make that defconfig file one line shorter, while requiring other users
to add another line to turn it off when they do not want it.

         Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel