Random drivers should not override a user configuration of core knobs
(e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
which depends on CMA, if possible; however, these drivers also have to
tolerate if DMA_CMA is not available/functioning, for example, if no CMA
area for DMA_CMA use has been setup via "cma=X". In the worst case, the
driver cannot do it's job properly in some configurations.
For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and DMA_CMA if
available") documents
While this is no build dependency, etnaviv will only work correctly
on most systems if CMA and DMA_CMA are enabled. Select both options
if available to avoid users ending up with a non-working GPU due to
a lacking kernel config.
So etnaviv really wants to have DMA_CMA, however, can deal with some cases
where it is not available.
Let's introduce WANT_DMA_CMA and use it in most cases where drivers
select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
of recursive dependency issues).
We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
With this change, distributions can disable CONFIG_CMA or
CONFIG_DMA_CMA, without it silently getting enabled again by random
drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
and CONFIG_DMA_CMA if they are unspecified and any driver is around that
selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
DRM_KMS_CMA_HELPER.
For example, if any driver selects WANT_DMA_CMA and we do a
"make olddefconfig":
1. With "# CONFIG_CMA is not set" and no specification of
"CONFIG_DMA_CMA"
-> CONFIG_DMA_CMA won't be part of .config
2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
-> CONFIG_DMA_CMA will be removed from .config
Note: drivers/remoteproc seems to be special; commit c51e882cd711
("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains that
there is a real dependency to DMA_CMA for it to work; leave that dependency
in place and don't convert it to a soft dependency.
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Joel Stanley <[email protected]>
Cc: Andrew Jeffery <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Russell King <[email protected]>
Cc: Christian Gmeiner <[email protected]>
Cc: Paul Cercueil <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Marek Szyprowski <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Cc: Eric Anholt <[email protected]>
Cc: Michal Simek <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: "Alexander A. Klimov" <[email protected]>
Cc: Peter Collingbourne <[email protected]>
Cc: Suman Anna <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: David Hildenbrand <[email protected]>
---
Let's see if this approach is better for soft dependencies (and if we
actually have some hard dependencies in there). This is the follow-up
of
https://lkml.kernel.org/r/[email protected]
https://lkml.kernel.org/r/[email protected]
I was wondering if it would make sense in some drivers to warn if either
CONFIG_DMA_CMA is not available or if DRM_CMA has not been configured
properly - just to give people a heads up that something might more likely
go wrong; that would, however, be future work.
v2 -> v3:
- Don't use "imply" but instead use a new WANT_DMA_CMA and make the default
of CMA and DMA_CMA depend on it.
- Also adjust ingenic, mcde, tve200; these sound like soft dependencies as
well (although DMA_CMA is really desired)
v1 -> v2:
- Fix DRM_CMA -> DMA_CMA
---
drivers/gpu/drm/Kconfig | 2 ++
drivers/gpu/drm/aspeed/Kconfig | 2 --
drivers/gpu/drm/etnaviv/Kconfig | 3 +--
drivers/gpu/drm/ingenic/Kconfig | 1 -
drivers/gpu/drm/mcde/Kconfig | 1 -
drivers/gpu/drm/tve200/Kconfig | 1 -
drivers/video/fbdev/Kconfig | 2 +-
kernel/dma/Kconfig | 7 +++++++
mm/Kconfig | 1 +
9 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 85b79a7fee63..6f9989adfa93 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -201,12 +201,14 @@ config DRM_TTM_HELPER
config DRM_GEM_CMA_HELPER
bool
depends on DRM
+ select WANT_DMA_CMA
help
Choose this if you need the GEM CMA helper functions
config DRM_KMS_CMA_HELPER
bool
depends on DRM
+ select WANT_DMA_CMA
select DRM_GEM_CMA_HELPER
help
Choose this if you need the KMS CMA helper functions
diff --git a/drivers/gpu/drm/aspeed/Kconfig b/drivers/gpu/drm/aspeed/Kconfig
index 5e95bcea43e9..e5ff33f85f21 100644
--- a/drivers/gpu/drm/aspeed/Kconfig
+++ b/drivers/gpu/drm/aspeed/Kconfig
@@ -6,8 +6,6 @@ config DRM_ASPEED_GFX
depends on MMU
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
- select DMA_CMA if HAVE_DMA_CONTIGUOUS
- select CMA if HAVE_DMA_CONTIGUOUS
select MFD_SYSCON
help
Chose this option if you have an ASPEED AST2500 SOC Display
diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
index faa7fc68b009..a3e7649b44a7 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -9,8 +9,7 @@ config DRM_ETNAVIV
select THERMAL if DRM_ETNAVIV_THERMAL
select TMPFS
select WANT_DEV_COREDUMP
- select CMA if HAVE_DMA_CONTIGUOUS
- select DMA_CMA if HAVE_DMA_CONTIGUOUS
+ select WANT_DMA_CMA
select DRM_SCHED
help
DRM driver for Vivante GPUs.
diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig
index 3b57f8be007c..156b11b7bbb8 100644
--- a/drivers/gpu/drm/ingenic/Kconfig
+++ b/drivers/gpu/drm/ingenic/Kconfig
@@ -2,7 +2,6 @@ config DRM_INGENIC
tristate "DRM Support for Ingenic SoCs"
depends on MIPS || COMPILE_TEST
depends on DRM
- depends on CMA
depends on OF
depends on COMMON_CLK
select DRM_BRIDGE
diff --git a/drivers/gpu/drm/mcde/Kconfig b/drivers/gpu/drm/mcde/Kconfig
index 71c689b573c9..217d54c4babc 100644
--- a/drivers/gpu/drm/mcde/Kconfig
+++ b/drivers/gpu/drm/mcde/Kconfig
@@ -1,7 +1,6 @@
config DRM_MCDE
tristate "DRM Support for ST-Ericsson MCDE (Multichannel Display Engine)"
depends on DRM
- depends on CMA
depends on ARM || COMPILE_TEST
depends on OF
depends on COMMON_CLK
diff --git a/drivers/gpu/drm/tve200/Kconfig b/drivers/gpu/drm/tve200/Kconfig
index e2d163c74ed6..d04b7322c770 100644
--- a/drivers/gpu/drm/tve200/Kconfig
+++ b/drivers/gpu/drm/tve200/Kconfig
@@ -2,7 +2,6 @@
config DRM_TVE200
tristate "DRM Support for Faraday TV Encoder TVE200"
depends on DRM
- depends on CMA
depends on ARM || COMPILE_TEST
depends on OF
select DRM_BRIDGE
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 4f02db65dede..e8acd4f77d41 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2186,7 +2186,7 @@ config FB_HYPERV
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
select FB_DEFERRED_IO
- select DMA_CMA if HAVE_DMA_CONTIGUOUS && CMA
+ select WANT_DMA_CMA
help
This framebuffer driver supports Microsoft Hyper-V Synthetic Video.
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 77b405508743..928f16d2461d 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -103,8 +103,15 @@ config DMA_DIRECT_REMAP
select DMA_REMAP
select DMA_COHERENT_POOL
+config WANT_DMA_CMA
+ bool
+ help
+ Drivers should "select" this option if they desire to use the
+ DMA_CMA mechanism.
+
config DMA_CMA
bool "DMA Contiguous Memory Allocator"
+ default y if WANT_DMA_CMA
depends on HAVE_DMA_CONTIGUOUS && CMA
help
This enables the Contiguous Memory Allocator which allows drivers
diff --git a/mm/Kconfig b/mm/Kconfig
index 24c045b24b95..169598ee56b1 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -485,6 +485,7 @@ config FRONTSWAP
config CMA
bool "Contiguous Memory Allocator"
+ default y if WANT_DMA_CMA
depends on MMU
select MIGRATION
select MEMORY_ISOLATION
--
2.30.2
Am Freitag, dem 09.04.2021 um 13:20 +0200 schrieb David Hildenbrand:
> Random drivers should not override a user configuration of core knobs
> (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
> which depends on CMA, if possible; however, these drivers also have to
> tolerate if DMA_CMA is not available/functioning, for example, if no CMA
> area for DMA_CMA use has been setup via "cma=X". In the worst case, the
> driver cannot do it's job properly in some configurations.
>
> For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and DMA_CMA if
> available") documents
> While this is no build dependency, etnaviv will only work correctly
> on most systems if CMA and DMA_CMA are enabled. Select both options
> if available to avoid users ending up with a non-working GPU due to
> a lacking kernel config.
> So etnaviv really wants to have DMA_CMA, however, can deal with some cases
> where it is not available.
>
> Let's introduce WANT_DMA_CMA and use it in most cases where drivers
> select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
> of recursive dependency issues).
>
> We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
>
> With this change, distributions can disable CONFIG_CMA or
> CONFIG_DMA_CMA, without it silently getting enabled again by random
> drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
> and CONFIG_DMA_CMA if they are unspecified and any driver is around that
> selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER.
>
> For example, if any driver selects WANT_DMA_CMA and we do a
> "make olddefconfig":
>
> 1. With "# CONFIG_CMA is not set" and no specification of
> "CONFIG_DMA_CMA"
>
> -> CONFIG_DMA_CMA won't be part of .config
>
> 2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
>
> Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
> DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
>
> 3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
>
> -> CONFIG_DMA_CMA will be removed from .config
>
> Note: drivers/remoteproc seems to be special; commit c51e882cd711
> ("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains that
> there is a real dependency to DMA_CMA for it to work; leave that dependency
> in place and don't convert it to a soft dependency.
Hm, to me this sounds much like the reasoning for the etnaviv
dependency. There is no actual build dependency, as the allocations are
done through the DMA API, but for the allocations to succeed you most
likely want CMA to be enabled. But that's just an observation from the
outside, I have no real clue about the remoteproc drivers.
As far as the etnaviv changes are concerned:
Acked-by: Lucas Stach <[email protected]>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Thomas Zimmermann <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Joel Stanley <[email protected]>
> Cc: Andrew Jeffery <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Christian Gmeiner <[email protected]>
> Cc: Paul Cercueil <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Marek Szyprowski <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Mike Rapoport <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Bartlomiej Zolnierkiewicz <[email protected]>
> Cc: Eric Anholt <[email protected]>
> Cc: Michal Simek <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: "Alexander A. Klimov" <[email protected]>
> Cc: Peter Collingbourne <[email protected]>
> Cc: Suman Anna <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: David Hildenbrand <[email protected]>
> ---
>
> Let's see if this approach is better for soft dependencies (and if we
> actually have some hard dependencies in there). This is the follow-up
> of
> https://lkml.kernel.org/r/[email protected]
> https://lkml.kernel.org/r/[email protected]
>
> I was wondering if it would make sense in some drivers to warn if either
> CONFIG_DMA_CMA is not available or if DRM_CMA has not been configured
> properly - just to give people a heads up that something might more likely
> go wrong; that would, however, be future work.
>
> v2 -> v3:
> - Don't use "imply" but instead use a new WANT_DMA_CMA and make the default
> of CMA and DMA_CMA depend on it.
> - Also adjust ingenic, mcde, tve200; these sound like soft dependencies as
> well (although DMA_CMA is really desired)
>
> v1 -> v2:
> - Fix DRM_CMA -> DMA_CMA
>
> ---
> drivers/gpu/drm/Kconfig | 2 ++
> drivers/gpu/drm/aspeed/Kconfig | 2 --
> drivers/gpu/drm/etnaviv/Kconfig | 3 +--
> drivers/gpu/drm/ingenic/Kconfig | 1 -
> drivers/gpu/drm/mcde/Kconfig | 1 -
> drivers/gpu/drm/tve200/Kconfig | 1 -
> drivers/video/fbdev/Kconfig | 2 +-
> kernel/dma/Kconfig | 7 +++++++
> mm/Kconfig | 1 +
> 9 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 85b79a7fee63..6f9989adfa93 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -201,12 +201,14 @@ config DRM_TTM_HELPER
> config DRM_GEM_CMA_HELPER
> bool
> depends on DRM
> + select WANT_DMA_CMA
> help
> Choose this if you need the GEM CMA helper functions
>
>
>
>
> config DRM_KMS_CMA_HELPER
> bool
> depends on DRM
> + select WANT_DMA_CMA
> select DRM_GEM_CMA_HELPER
> help
> Choose this if you need the KMS CMA helper functions
> diff --git a/drivers/gpu/drm/aspeed/Kconfig b/drivers/gpu/drm/aspeed/Kconfig
> index 5e95bcea43e9..e5ff33f85f21 100644
> --- a/drivers/gpu/drm/aspeed/Kconfig
> +++ b/drivers/gpu/drm/aspeed/Kconfig
> @@ -6,8 +6,6 @@ config DRM_ASPEED_GFX
> depends on MMU
> select DRM_KMS_HELPER
> select DRM_KMS_CMA_HELPER
> - select DMA_CMA if HAVE_DMA_CONTIGUOUS
> - select CMA if HAVE_DMA_CONTIGUOUS
> select MFD_SYSCON
> help
> Chose this option if you have an ASPEED AST2500 SOC Display
> diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
> index faa7fc68b009..a3e7649b44a7 100644
> --- a/drivers/gpu/drm/etnaviv/Kconfig
> +++ b/drivers/gpu/drm/etnaviv/Kconfig
> @@ -9,8 +9,7 @@ config DRM_ETNAVIV
> select THERMAL if DRM_ETNAVIV_THERMAL
> select TMPFS
> select WANT_DEV_COREDUMP
> - select CMA if HAVE_DMA_CONTIGUOUS
> - select DMA_CMA if HAVE_DMA_CONTIGUOUS
> + select WANT_DMA_CMA
> select DRM_SCHED
> help
> DRM driver for Vivante GPUs.
> diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig
> index 3b57f8be007c..156b11b7bbb8 100644
> --- a/drivers/gpu/drm/ingenic/Kconfig
> +++ b/drivers/gpu/drm/ingenic/Kconfig
> @@ -2,7 +2,6 @@ config DRM_INGENIC
> tristate "DRM Support for Ingenic SoCs"
> depends on MIPS || COMPILE_TEST
> depends on DRM
> - depends on CMA
> depends on OF
> depends on COMMON_CLK
> select DRM_BRIDGE
> diff --git a/drivers/gpu/drm/mcde/Kconfig b/drivers/gpu/drm/mcde/Kconfig
> index 71c689b573c9..217d54c4babc 100644
> --- a/drivers/gpu/drm/mcde/Kconfig
> +++ b/drivers/gpu/drm/mcde/Kconfig
> @@ -1,7 +1,6 @@
> config DRM_MCDE
> tristate "DRM Support for ST-Ericsson MCDE (Multichannel Display Engine)"
> depends on DRM
> - depends on CMA
> depends on ARM || COMPILE_TEST
> depends on OF
> depends on COMMON_CLK
> diff --git a/drivers/gpu/drm/tve200/Kconfig b/drivers/gpu/drm/tve200/Kconfig
> index e2d163c74ed6..d04b7322c770 100644
> --- a/drivers/gpu/drm/tve200/Kconfig
> +++ b/drivers/gpu/drm/tve200/Kconfig
> @@ -2,7 +2,6 @@
> config DRM_TVE200
> tristate "DRM Support for Faraday TV Encoder TVE200"
> depends on DRM
> - depends on CMA
> depends on ARM || COMPILE_TEST
> depends on OF
> select DRM_BRIDGE
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index 4f02db65dede..e8acd4f77d41 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -2186,7 +2186,7 @@ config FB_HYPERV
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> select FB_DEFERRED_IO
> - select DMA_CMA if HAVE_DMA_CONTIGUOUS && CMA
> + select WANT_DMA_CMA
> help
> This framebuffer driver supports Microsoft Hyper-V Synthetic Video.
>
>
>
>
> diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
> index 77b405508743..928f16d2461d 100644
> --- a/kernel/dma/Kconfig
> +++ b/kernel/dma/Kconfig
> @@ -103,8 +103,15 @@ config DMA_DIRECT_REMAP
> select DMA_REMAP
> select DMA_COHERENT_POOL
>
>
>
>
> +config WANT_DMA_CMA
> + bool
> + help
> + Drivers should "select" this option if they desire to use the
> + DMA_CMA mechanism.
> +
> config DMA_CMA
> bool "DMA Contiguous Memory Allocator"
> + default y if WANT_DMA_CMA
> depends on HAVE_DMA_CONTIGUOUS && CMA
> help
> This enables the Contiguous Memory Allocator which allows drivers
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 24c045b24b95..169598ee56b1 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -485,6 +485,7 @@ config FRONTSWAP
>
>
>
>
> config CMA
> bool "Contiguous Memory Allocator"
> + default y if WANT_DMA_CMA
> depends on MMU
> select MIGRATION
> select MEMORY_ISOLATION
On Fri, Apr 9, 2021 at 1:20 PM David Hildenbrand <[email protected]> wrote:
> Random drivers should not override a user configuration of core knobs
> (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
> which depends on CMA, if possible; however, these drivers also have to
> tolerate if DMA_CMA is not available/functioning, for example, if no CMA
> area for DMA_CMA use has been setup via "cma=X". In the worst case, the
> driver cannot do it's job properly in some configurations.
Looks good to me. At least a lot better than what we have.
Reviewed-by: Linus Walleij <[email protected]>
> Let's see if this approach is better for soft dependencies (and if we
> actually have some hard dependencies in there). This is the follow-up
> of
> https://lkml.kernel.org/r/[email protected]
> https://lkml.kernel.org/r/[email protected]
You can just add these to the commit message with Link:
when applying so people can easily find the discussion from the
commit.
> I was wondering if it would make sense in some drivers to warn if either
> CONFIG_DMA_CMA is not available or if DRM_CMA has not been configured
> properly - just to give people a heads up that something might more likely
> go wrong; that would, however, be future work.
I think the frameworks (DRM_*_CMA_HELPER)
should pr_info something about it so the individual drivers
don't have to sanity check their entire world.
Yours,
Linus Walleij
On Fri, Apr 9, 2021 at 1:21 PM David Hildenbrand <[email protected]> wrote:
>
> Random drivers should not override a user configuration of core knobs
> (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
> which depends on CMA, if possible; however, these drivers also have to
> tolerate if DMA_CMA is not available/functioning, for example, if no CMA
> area for DMA_CMA use has been setup via "cma=X". In the worst case, the
> driver cannot do it's job properly in some configurations.
>
> For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and DMA_CMA if
> available") documents
> While this is no build dependency, etnaviv will only work correctly
> on most systems if CMA and DMA_CMA are enabled. Select both options
> if available to avoid users ending up with a non-working GPU due to
> a lacking kernel config.
> So etnaviv really wants to have DMA_CMA, however, can deal with some cases
> where it is not available.
>
> Let's introduce WANT_DMA_CMA and use it in most cases where drivers
> select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
> of recursive dependency issues).
>
> We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
>
> With this change, distributions can disable CONFIG_CMA or
> CONFIG_DMA_CMA, without it silently getting enabled again by random
> drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
> and CONFIG_DMA_CMA if they are unspecified and any driver is around that
> selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
> DRM_KMS_CMA_HELPER.
>
> For example, if any driver selects WANT_DMA_CMA and we do a
> "make olddefconfig":
>
> 1. With "# CONFIG_CMA is not set" and no specification of
> "CONFIG_DMA_CMA"
>
> -> CONFIG_DMA_CMA won't be part of .config
>
> 2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
>
> Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
> DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
>
> 3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
>
> -> CONFIG_DMA_CMA will be removed from .config
>
> Note: drivers/remoteproc seems to be special; commit c51e882cd711
> ("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains that
> there is a real dependency to DMA_CMA for it to work; leave that dependency
> in place and don't convert it to a soft dependency.
I don't think this dependency is fundamentally different from the others,
though davinci machines tend to have less memory than a lot of the
other machines, so it's more likely to fail without CMA.
Regardless of this,
Reviewed-by: Arnd Bergmann <[email protected]>
On 09.04.21 15:35, Arnd Bergmann wrote:
> On Fri, Apr 9, 2021 at 1:21 PM David Hildenbrand <[email protected]> wrote:
>>
>> Random drivers should not override a user configuration of core knobs
>> (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
>> which depends on CMA, if possible; however, these drivers also have to
>> tolerate if DMA_CMA is not available/functioning, for example, if no CMA
>> area for DMA_CMA use has been setup via "cma=X". In the worst case, the
>> driver cannot do it's job properly in some configurations.
>>
>> For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and DMA_CMA if
>> available") documents
>> While this is no build dependency, etnaviv will only work correctly
>> on most systems if CMA and DMA_CMA are enabled. Select both options
>> if available to avoid users ending up with a non-working GPU due to
>> a lacking kernel config.
>> So etnaviv really wants to have DMA_CMA, however, can deal with some cases
>> where it is not available.
>>
>> Let's introduce WANT_DMA_CMA and use it in most cases where drivers
>> select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
>> of recursive dependency issues).
>>
>> We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
>> DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
>>
>> With this change, distributions can disable CONFIG_CMA or
>> CONFIG_DMA_CMA, without it silently getting enabled again by random
>> drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
>> and CONFIG_DMA_CMA if they are unspecified and any driver is around that
>> selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
>> DRM_KMS_CMA_HELPER.
>>
>> For example, if any driver selects WANT_DMA_CMA and we do a
>> "make olddefconfig":
>>
>> 1. With "# CONFIG_CMA is not set" and no specification of
>> "CONFIG_DMA_CMA"
>>
>> -> CONFIG_DMA_CMA won't be part of .config
>>
>> 2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
>>
>> Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
>> DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
>>
>> 3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
>>
>> -> CONFIG_DMA_CMA will be removed from .config
>>
>> Note: drivers/remoteproc seems to be special; commit c51e882cd711
>> ("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains that
>> there is a real dependency to DMA_CMA for it to work; leave that dependency
>> in place and don't convert it to a soft dependency.
>
> I don't think this dependency is fundamentally different from the others,
> though davinci machines tend to have less memory than a lot of the
> other machines, so it's more likely to fail without CMA.
>
I was also unsure - and Lucas had similar thoughts. If you want, I can
send a v4 also taking care of this.
Thanks!
> Regardless of this,
>
> Reviewed-by: Arnd Bergmann <[email protected]>
>
--
Thanks,
David / dhildenb
On 2021-04-09 14:39, David Hildenbrand wrote:
> On 09.04.21 15:35, Arnd Bergmann wrote:
>> On Fri, Apr 9, 2021 at 1:21 PM David Hildenbrand <[email protected]>
>> wrote:
>>>
>>> Random drivers should not override a user configuration of core knobs
>>> (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
>>> which depends on CMA, if possible; however, these drivers also have to
>>> tolerate if DMA_CMA is not available/functioning, for example, if no CMA
>>> area for DMA_CMA use has been setup via "cma=X". In the worst case, the
>>> driver cannot do it's job properly in some configurations.
>>>
>>> For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and
>>> DMA_CMA if
>>> available") documents
>>> While this is no build dependency, etnaviv will only work
>>> correctly
>>> on most systems if CMA and DMA_CMA are enabled. Select both
>>> options
>>> if available to avoid users ending up with a non-working GPU
>>> due to
>>> a lacking kernel config.
>>> So etnaviv really wants to have DMA_CMA, however, can deal with some
>>> cases
>>> where it is not available.
>>>
>>> Let's introduce WANT_DMA_CMA and use it in most cases where drivers
>>> select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
>>> of recursive dependency issues).
>>>
>>> We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
>>> DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
>>>
>>> With this change, distributions can disable CONFIG_CMA or
>>> CONFIG_DMA_CMA, without it silently getting enabled again by random
>>> drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
>>> and CONFIG_DMA_CMA if they are unspecified and any driver is around that
>>> selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
>>> DRM_KMS_CMA_HELPER.
>>>
>>> For example, if any driver selects WANT_DMA_CMA and we do a
>>> "make olddefconfig":
>>>
>>> 1. With "# CONFIG_CMA is not set" and no specification of
>>> "CONFIG_DMA_CMA"
>>>
>>> -> CONFIG_DMA_CMA won't be part of .config
>>>
>>> 2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
>>>
>>> Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
>>> DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
>>>
>>> 3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
>>>
>>> -> CONFIG_DMA_CMA will be removed from .config
>>>
>>> Note: drivers/remoteproc seems to be special; commit c51e882cd711
>>> ("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains
>>> that
>>> there is a real dependency to DMA_CMA for it to work; leave that
>>> dependency
>>> in place and don't convert it to a soft dependency.
>>
>> I don't think this dependency is fundamentally different from the others,
>> though davinci machines tend to have less memory than a lot of the
>> other machines, so it's more likely to fail without CMA.
>>
>
> I was also unsure - and Lucas had similar thoughts. If you want, I can
> send a v4 also taking care of this.
TBH I think it should all just be removed. DMA_CMA is effectively an
internal feature of the DMA API, and drivers which simply use the DMA
API shouldn't really be trying to assume *how* things might be allocated
at runtime - CMA is hardly the only way. Platform-level assumptions
about the presence or not of IOMMUs, memory carveouts, etc., and whether
it even matters - e.g. a device with a tiny LCD may only need display
buffers which still fit in a regular MAX_ORDER allocation - could go in
platform-specific configs, but I really don't think they belong at the
generic subsystem level.
We already have various examples like I2S drivers that won't even probe
without a dmaengine provider being present, or host controller drivers
which are useless without their corresponding phy driver (and I'm
guessing you can probably also do higher-level things like include the
block layer but omit all filesystem drivers). I don't believe it's
Kconfig's job to try to guess whether a given configuration is *useful*,
only to enforce that's it's valid to build.
Robin.
On 12.04.21 15:12, Robin Murphy wrote:
> On 2021-04-09 14:39, David Hildenbrand wrote:
>> On 09.04.21 15:35, Arnd Bergmann wrote:
>>> On Fri, Apr 9, 2021 at 1:21 PM David Hildenbrand <[email protected]>
>>> wrote:
>>>>
>>>> Random drivers should not override a user configuration of core knobs
>>>> (e.g., CONFIG_DMA_CMA=n). Applicable drivers would like to use DMA_CMA,
>>>> which depends on CMA, if possible; however, these drivers also have to
>>>> tolerate if DMA_CMA is not available/functioning, for example, if no CMA
>>>> area for DMA_CMA use has been setup via "cma=X". In the worst case, the
>>>> driver cannot do it's job properly in some configurations.
>>>>
>>>> For example, commit 63f5677544b3 ("drm/etnaviv: select CMA and
>>>> DMA_CMA if
>>>> available") documents
>>>> While this is no build dependency, etnaviv will only work
>>>> correctly
>>>> on most systems if CMA and DMA_CMA are enabled. Select both
>>>> options
>>>> if available to avoid users ending up with a non-working GPU
>>>> due to
>>>> a lacking kernel config.
>>>> So etnaviv really wants to have DMA_CMA, however, can deal with some
>>>> cases
>>>> where it is not available.
>>>>
>>>> Let's introduce WANT_DMA_CMA and use it in most cases where drivers
>>>> select CMA/DMA_CMA, or depend on DMA_CMA (in a wrong way via CMA because
>>>> of recursive dependency issues).
>>>>
>>>> We'll assume that any driver that selects DRM_GEM_CMA_HELPER or
>>>> DRM_KMS_CMA_HELPER would like to use DMA_CMA if possible.
>>>>
>>>> With this change, distributions can disable CONFIG_CMA or
>>>> CONFIG_DMA_CMA, without it silently getting enabled again by random
>>>> drivers. Also, we'll now automatically try to enabled both, CONFIG_CMA
>>>> and CONFIG_DMA_CMA if they are unspecified and any driver is around that
>>>> selects WANT_DMA_CMA -- also implicitly via DRM_GEM_CMA_HELPER or
>>>> DRM_KMS_CMA_HELPER.
>>>>
>>>> For example, if any driver selects WANT_DMA_CMA and we do a
>>>> "make olddefconfig":
>>>>
>>>> 1. With "# CONFIG_CMA is not set" and no specification of
>>>> "CONFIG_DMA_CMA"
>>>>
>>>> -> CONFIG_DMA_CMA won't be part of .config
>>>>
>>>> 2. With no specification of CONFIG_CMA or CONFIG_DMA_CMA
>>>>
>>>> Contiguous Memory Allocator (CMA) [Y/n/?] (NEW)
>>>> DMA Contiguous Memory Allocator (DMA_CMA) [Y/n/?] (NEW)
>>>>
>>>> 3. With "# CONFIG_CMA is not set" and "# CONFIG_DMA_CMA is not set"
>>>>
>>>> -> CONFIG_DMA_CMA will be removed from .config
>>>>
>>>> Note: drivers/remoteproc seems to be special; commit c51e882cd711
>>>> ("remoteproc/davinci: Update Kconfig to depend on DMA_CMA") explains
>>>> that
>>>> there is a real dependency to DMA_CMA for it to work; leave that
>>>> dependency
>>>> in place and don't convert it to a soft dependency.
>>>
>>> I don't think this dependency is fundamentally different from the others,
>>> though davinci machines tend to have less memory than a lot of the
>>> other machines, so it's more likely to fail without CMA.
>>>
>>
>> I was also unsure - and Lucas had similar thoughts. If you want, I can
>> send a v4 also taking care of this.
>
> TBH I think it should all just be removed. DMA_CMA is effectively an
> internal feature of the DMA API, and drivers which simply use the DMA
> API shouldn't really be trying to assume *how* things might be allocated
> at runtime - CMA is hardly the only way. Platform-level assumptions
> about the presence or not of IOMMUs, memory carveouts, etc., and whether
> it even matters - e.g. a device with a tiny LCD may only need display
> buffers which still fit in a regular MAX_ORDER allocation - could go in
> platform-specific configs, but I really don't think they belong at the
> generic subsystem level.
>
> We already have various examples like I2S drivers that won't even probe
> without a dmaengine provider being present, or host controller drivers
> which are useless without their corresponding phy driver (and I'm
> guessing you can probably also do higher-level things like include the
> block layer but omit all filesystem drivers). I don't believe it's
> Kconfig's job to try to guess whether a given configuration is *useful*,
> only to enforce that's it's valid to build.
That would mean: if it's not a built-time dependency, don't mention it
in Kconfig.
If that were true, why do we have have defaults modeled in Kconfig then?
IMHO, some part of Kconfig is to give you sane defaults.
--
Thanks,
David / dhildenb