linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] drm: Restore helper usability
@ 2024-04-22 10:30 Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 01/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2" Geert Uytterhoeven
                   ` (11 more replies)
  0 siblings, 12 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

	Hi all,

As discussed on IRC with Maxime and Arnd, this series reverts the
conversion of select to depends for various DRM helpers in series
"[PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to
depends on"[1], and various fixes for it.  This conversion introduced a
big usability issue when configuring a kernel and enabling DRM drivers
that use DRM helper code: as drivers now depend on helpers, the user
needs to know which helpers to enable, before the driver he is
interested even becomes visible.  The user should not need to know that,
and drivers should select the helpers they need.

Hence revert back to what we had before, where drivers selected the
helpers (and any of their dependencies, if they can be met) they need.
In general, when a symbol selects another symbol, it should just make
sure the dependencies of the target symbol are met, which may mean
adding dependencies to the source symbol.

Thanks for applying!

[1] https://lore.kernel.org/r/20240327-kms-kconfig-helpers-v3-0-eafee11b84b3@kernel.org/

Geert Uytterhoeven (11):
  Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2"
  Revert "drm/display: Select DRM_KMS_HELPER for DP helpers"
  Revert "drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable"
  Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies"
  Revert "drm: Switch DRM_DISPLAY_HDMI_HELPER to depends on"
  Revert "drm: Switch DRM_DISPLAY_HDCP_HELPER to depends on"
  Revert "drm: Switch DRM_DISPLAY_DP_HELPER to depends on"
  Revert "drm: Switch DRM_DISPLAY_DP_AUX_BUS to depends on"
  Revert "drm: Switch DRM_DISPLAY_HELPER to depends on"
  Revert "drm: Make drivers depends on DRM_DW_HDMI"
  Revert "drm/display: Make all helpers visible and switch to depends
    on"

 drivers/gpu/drm/Kconfig                 |  8 +++----
 drivers/gpu/drm/amd/amdgpu/Kconfig      | 12 ++++------
 drivers/gpu/drm/bridge/Kconfig          | 28 +++++++++++-----------
 drivers/gpu/drm/bridge/analogix/Kconfig | 18 +++++++-------
 drivers/gpu/drm/bridge/cadence/Kconfig  |  8 +++----
 drivers/gpu/drm/bridge/imx/Kconfig      |  4 ++--
 drivers/gpu/drm/bridge/synopsys/Kconfig |  6 ++---
 drivers/gpu/drm/display/Kconfig         | 32 ++++++++++---------------
 drivers/gpu/drm/exynos/Kconfig          |  4 ++--
 drivers/gpu/drm/i915/Kconfig            |  8 +++----
 drivers/gpu/drm/imx/ipuv3/Kconfig       |  5 ++--
 drivers/gpu/drm/ingenic/Kconfig         |  2 +-
 drivers/gpu/drm/mediatek/Kconfig        |  6 ++---
 drivers/gpu/drm/meson/Kconfig           |  2 +-
 drivers/gpu/drm/msm/Kconfig             |  8 +++----
 drivers/gpu/drm/nouveau/Kconfig         | 10 ++++----
 drivers/gpu/drm/panel/Kconfig           | 32 ++++++++++++-------------
 drivers/gpu/drm/radeon/Kconfig          |  8 +++----
 drivers/gpu/drm/renesas/rcar-du/Kconfig |  2 +-
 drivers/gpu/drm/rockchip/Kconfig        | 10 ++++----
 drivers/gpu/drm/sun4i/Kconfig           |  2 +-
 drivers/gpu/drm/tegra/Kconfig           |  8 +++----
 drivers/gpu/drm/vc4/Kconfig             | 10 ++++----
 drivers/gpu/drm/xe/Kconfig              | 13 ++++------
 drivers/gpu/drm/xlnx/Kconfig            |  8 +++----
 25 files changed, 116 insertions(+), 138 deletions(-)

-- 
2.34.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 01/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 02/11] Revert "drm/display: Select DRM_KMS_HELPER for DP helpers" Geert Uytterhoeven
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit a57e191ebbaa0363dbf352cc37447c2230573e29, as the
commits it fixes will be reverted, too.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/bridge/analogix/Kconfig | 2 +-
 drivers/gpu/drm/rockchip/Kconfig        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
index 5b564fded6d6c8e7..12bfea53bf24b2c2 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -28,7 +28,7 @@ config DRM_ANALOGIX_ANX78XX
 
 config DRM_ANALOGIX_DP
 	tristate
-	depends on DRM_DISPLAY_HELPER
+	depends on DRM
 
 config DRM_ANALOGIX_ANX7625
 	tristate "Analogix Anx7625 MIPI to DP interface support"
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 4c7072e6e34eafb5..4b4ad75032fda17e 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -36,7 +36,7 @@ config ROCKCHIP_VOP2
 config ROCKCHIP_ANALOGIX_DP
 	bool "Rockchip specific extensions for Analogix DP driver"
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HELPER=y || (DRM_DISPLAY_HELPER=m && DRM_ROCKCHIP=m)
+	depends on DRM_DISPLAY_HELPER
 	depends on ROCKCHIP_VOP
 	help
 	  This selects support for Rockchip SoC specific extensions
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 02/11] Revert "drm/display: Select DRM_KMS_HELPER for DP helpers"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 01/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2" Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 03/11] Revert "drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable" Geert Uytterhoeven
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit 7fa678cc0a5648b5ea28629a2d21b9d4b6ac8f56, as the
commit it fixes will be reverted, too.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/display/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index a38962a556c2e847..01f2a231aa5f04bd 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -39,7 +39,6 @@ config DRM_DISPLAY_DP_AUX_CHARDEV
 config DRM_DISPLAY_DP_HELPER
 	bool "DRM DisplayPort Helpers"
 	depends on DRM_DISPLAY_HELPER
-	select DRM_KMS_HELPER
 	default y
 	help
 	  DRM display helpers for DisplayPort.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 03/11] Revert "drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 01/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2" Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 02/11] Revert "drm/display: Select DRM_KMS_HELPER for DP helpers" Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 04/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies" Geert Uytterhoeven
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit 0209df3b4731516fe77638bfc52ba2e9629c67cd, as the
commit it fixes (which is BTW not the commit in the Fixes: tag!) will be
reverted, too.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/bridge/synopsys/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig
index 1252fd30d4a4b461..387f5bd86089fb5c 100644
--- a/drivers/gpu/drm/bridge/synopsys/Kconfig
+++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_DW_HDMI
-	tristate "Synopsys Designware HDMI TX Controller"
+	tristate
 	depends on DRM_DISPLAY_HDMI_HELPER
 	depends on DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 04/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2024-04-22 10:30 ` [PATCH 03/11] Revert "drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable" Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 05/11] Revert "drm: Switch DRM_DISPLAY_HDMI_HELPER to depends on" Geert Uytterhoeven
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit d1ef8fc18be6adbbffdee06fbb5b33699e2852be, as the
commit it fixes will be reverted, too.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/exynos/Kconfig   | 2 +-
 drivers/gpu/drm/rockchip/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 58cd772207413e94..6a26a0b8eff2c021 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -68,7 +68,7 @@ config DRM_EXYNOS_DP
 	bool "Exynos specific extensions for Analogix DP driver"
 	depends on DRM_EXYNOS_FIMD || DRM_EXYNOS7_DECON
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HELPER=y || (DRM_DISPLAY_HELPER=m && DRM_EXYNOS=m)
+	depends on DRM_DISPLAY_HELPER
 	select DRM_ANALOGIX_DP
 	default DRM_EXYNOS
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 4b4ad75032fda17e..4b49a14758fe0412 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -46,7 +46,7 @@ config ROCKCHIP_ANALOGIX_DP
 config ROCKCHIP_CDN_DP
 	bool "Rockchip cdn DP"
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HELPER=y || (DRM_DISPLAY_HELPER=m && DRM_ROCKCHIP=m)
+	depends on DRM_DISPLAY_HELPER
 	depends on EXTCON=y || (EXTCON=m && DRM_ROCKCHIP=m)
 	help
 	  This selects support for Rockchip SoC specific extensions
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 05/11] Revert "drm: Switch DRM_DISPLAY_HDMI_HELPER to depends on"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
                   ` (3 preceding siblings ...)
  2024-04-22 10:30 ` [PATCH 04/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies" Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 06/11] Revert "drm: Switch DRM_DISPLAY_HDCP_HELPER " Geert Uytterhoeven
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit f6d2dc03fa8546b284dd8c1af027d9fac5725921, as helper
code should always be selected by the driver that needs it, for the
convenience of the final user configuring a kernel.

The user who configures a kernel should not need to know which helpers
are needed for the driver he is interested in.  Making a driver depend
on helper code means that the user needs to know which helpers to enable
first, which is very user-unfriendly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/amd/amdgpu/Kconfig      | 2 +-
 drivers/gpu/drm/bridge/synopsys/Kconfig | 2 +-
 drivers/gpu/drm/display/Kconfig         | 1 -
 drivers/gpu/drm/i915/Kconfig            | 2 +-
 drivers/gpu/drm/nouveau/Kconfig         | 2 +-
 drivers/gpu/drm/tegra/Kconfig           | 2 +-
 drivers/gpu/drm/vc4/Kconfig             | 2 +-
 drivers/gpu/drm/xe/Kconfig              | 2 +-
 8 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig
index b0365cc1374ee50a..1662dc49f18ed11e 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -5,12 +5,12 @@ config DRM_AMDGPU
 	depends on DRM
 	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HDCP_HELPER
-	depends on DRM_DISPLAY_HDMI_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on MMU
 	depends on PCI
 	depends on !UML
 	select FW_LOADER
+	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
 	select DRM_SCHED
 	select DRM_TTM
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig
index 387f5bd86089fb5c..f366ece471462a70 100644
--- a/drivers/gpu/drm/bridge/synopsys/Kconfig
+++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_DW_HDMI
 	tristate
-	depends on DRM_DISPLAY_HDMI_HELPER
 	depends on DRM_DISPLAY_HELPER
+	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_MMIO
 	select CEC_CORE if CEC_NOTIFIER
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 01f2a231aa5f04bd..d65f1a37c08c7cf8 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -74,6 +74,5 @@ config DRM_DISPLAY_HDCP_HELPER
 config DRM_DISPLAY_HDMI_HELPER
 	bool "DRM HDMI Helpers"
 	depends on DRM_DISPLAY_HELPER
-	default y
 	help
 	  DRM display helpers for HDMI.
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 4f0d18a16b0f4f99..87ef8c4d72a53768 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -4,7 +4,6 @@ config DRM_I915
 	depends on DRM
 	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HDCP_HELPER
-	depends on DRM_DISPLAY_HDMI_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on X86 && PCI
 	depends on !PREEMPT_RT
@@ -14,6 +13,7 @@ config DRM_I915
 	# the shmem_readpage() which depends upon tmpfs
 	select SHMEM
 	select TMPFS
+	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
 	select DRM_PANEL
 	select DRM_MIPI_DSI
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 4c10b400658c519c..7cc305b2826d6abc 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -3,12 +3,12 @@ config DRM_NOUVEAU
 	tristate "Nouveau (NVIDIA) cards"
 	depends on DRM
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HDMI_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on PCI
 	depends on MMU
 	select IOMMU_API
 	select FW_LOADER
+	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
 	select DRM_TTM
 	select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index 6974caa99ece94e1..bb6e35261f1144b1 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -6,9 +6,9 @@ config DRM_TEGRA
 	depends on DRM
 	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HDMI_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 4801f8b64d3d70cd..98772a6b5bf0df54 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -4,13 +4,13 @@ config DRM_VC4
 	depends on ARCH_BCM || ARCH_BCM2835 || COMPILE_TEST
 	depends on COMMON_CLK
 	depends on DRM
-	depends on DRM_DISPLAY_HDMI_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on PM
 	# Make sure not 'y' when RASPBERRYPI_FIRMWARE is 'm'. This can only
 	# happen when COMPILE_TEST=y, hence the added !RASPBERRYPI_FIRMWARE.
 	depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
 	depends on SND && SND_SOC
+	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
 	select DRM_GEM_DMA_HELPER
 	select DRM_PANEL_BRIDGE
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index bfa0e9d4bd64f83b..1fa8ef75823c7a6b 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -5,7 +5,6 @@ config DRM_XE
 	depends on DRM
 	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HDCP_HELPER
-	depends on DRM_DISPLAY_HDMI_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on MMU
 	depends on PCI
@@ -20,6 +19,7 @@ config DRM_XE
 	select DRM_KUNIT_TEST_HELPERS if DRM_XE_KUNIT_TEST != n
 	select DRM_PANEL
 	select DRM_SUBALLOC_HELPER
+	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_MIPI_DSI
 	select RELAY
 	select IRQ_WORK
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 06/11] Revert "drm: Switch DRM_DISPLAY_HDCP_HELPER to depends on"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
                   ` (4 preceding siblings ...)
  2024-04-22 10:30 ` [PATCH 05/11] Revert "drm: Switch DRM_DISPLAY_HDMI_HELPER to depends on" Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 07/11] Revert "drm: Switch DRM_DISPLAY_DP_HELPER " Geert Uytterhoeven
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit 3166e7e6d935caaef07605a5c90773fbf9ffeaf4, as helper
code should always be selected by the driver that needs it, for the
convenience of the final user configuring a kernel.

The user who configures a kernel should not need to know which helpers
are needed for the driver he is interested in.  Making a driver depend
on helper code means that the user needs to know which helpers to enable
first, which is very user-unfriendly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/amd/amdgpu/Kconfig      | 2 +-
 drivers/gpu/drm/bridge/Kconfig          | 2 +-
 drivers/gpu/drm/bridge/analogix/Kconfig | 2 +-
 drivers/gpu/drm/bridge/cadence/Kconfig  | 2 +-
 drivers/gpu/drm/display/Kconfig         | 1 -
 drivers/gpu/drm/i915/Kconfig            | 2 +-
 drivers/gpu/drm/xe/Kconfig              | 2 +-
 7 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig
index 1662dc49f18ed11e..ba09121e7debb247 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -4,13 +4,13 @@ config DRM_AMDGPU
 	tristate "AMD GPU"
 	depends on DRM
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HDCP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on MMU
 	depends on PCI
 	depends on !UML
 	select FW_LOADER
 	select DRM_DISPLAY_HDMI_HELPER
+	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_KMS_HELPER
 	select DRM_SCHED
 	select DRM_TTM
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index d1fbf8796fea8dbe..ad38cd201b006251 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -94,9 +94,9 @@ config DRM_ITE_IT6505
 	tristate "ITE IT6505 DisplayPort bridge"
 	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HDCP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_KMS_HELPER
 	select EXTCON
 	select CRYPTO
diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
index 12bfea53bf24b2c2..9659df6718de6db4 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -35,9 +35,9 @@ config DRM_ANALOGIX_ANX7625
 	depends on DRM
 	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HDCP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_MIPI_DSI
 	help
 	  ANX7625 is an ultra-low power 4K mobile HD transmitter
diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig b/drivers/gpu/drm/bridge/cadence/Kconfig
index 7817f6f56607141f..3480fd4d0a5f76e4 100644
--- a/drivers/gpu/drm/bridge/cadence/Kconfig
+++ b/drivers/gpu/drm/bridge/cadence/Kconfig
@@ -24,9 +24,9 @@ endif
 config DRM_CDNS_MHDP8546
 	tristate "Cadence DPI/DP bridge"
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HDCP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_KMS_HELPER
 	select DRM_PANEL_BRIDGE
 	help
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index d65f1a37c08c7cf8..9801f47a3704530d 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -67,7 +67,6 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG
 config DRM_DISPLAY_HDCP_HELPER
 	bool "DRM HDCD Helpers"
 	depends on DRM_DISPLAY_HELPER
-	default y
 	help
 	  DRM display helpers for HDCP.
 
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 87ef8c4d72a53768..dbde4e29d93a79dc 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -3,7 +3,6 @@ config DRM_I915
 	tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
 	depends on DRM
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HDCP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on X86 && PCI
 	depends on !PREEMPT_RT
@@ -13,6 +12,7 @@ config DRM_I915
 	# the shmem_readpage() which depends upon tmpfs
 	select SHMEM
 	select TMPFS
+	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 1fa8ef75823c7a6b..02da2faf5ae33bdb 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -4,7 +4,6 @@ config DRM_XE
 	depends on (m || (y && KUNIT=y))
 	depends on DRM
 	depends on DRM_DISPLAY_DP_HELPER
-	depends on DRM_DISPLAY_HDCP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on MMU
 	depends on PCI
@@ -19,6 +18,7 @@ config DRM_XE
 	select DRM_KUNIT_TEST_HELPERS if DRM_XE_KUNIT_TEST != n
 	select DRM_PANEL
 	select DRM_SUBALLOC_HELPER
+	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_MIPI_DSI
 	select RELAY
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 07/11] Revert "drm: Switch DRM_DISPLAY_DP_HELPER to depends on"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
                   ` (5 preceding siblings ...)
  2024-04-22 10:30 ` [PATCH 06/11] Revert "drm: Switch DRM_DISPLAY_HDCP_HELPER " Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 08/11] Revert "drm: Switch DRM_DISPLAY_DP_AUX_BUS " Geert Uytterhoeven
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit 0323287de87d7e6e9c22c57d7440aa353a2298d0, as helper
code should always be selected by the driver that needs it, for the
convenience of the final user configuring a kernel.

The user who configures a kernel should not need to know which helpers
are needed for the driver he is interested in.  Making a driver depend
on helper code means that the user needs to know which helpers to enable
first, which is very user-unfriendly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/Kconfig                 |  2 +-
 drivers/gpu/drm/amd/amdgpu/Kconfig      |  2 +-
 drivers/gpu/drm/bridge/Kconfig          | 10 +++++-----
 drivers/gpu/drm/bridge/analogix/Kconfig |  6 +++---
 drivers/gpu/drm/bridge/cadence/Kconfig  |  2 +-
 drivers/gpu/drm/display/Kconfig         |  1 -
 drivers/gpu/drm/exynos/Kconfig          |  2 +-
 drivers/gpu/drm/i915/Kconfig            |  2 +-
 drivers/gpu/drm/mediatek/Kconfig        |  2 +-
 drivers/gpu/drm/msm/Kconfig             |  2 +-
 drivers/gpu/drm/nouveau/Kconfig         |  2 +-
 drivers/gpu/drm/panel/Kconfig           |  8 ++++----
 drivers/gpu/drm/radeon/Kconfig          |  2 +-
 drivers/gpu/drm/rockchip/Kconfig        |  4 ++--
 drivers/gpu/drm/tegra/Kconfig           |  2 +-
 drivers/gpu/drm/xe/Kconfig              |  2 +-
 drivers/gpu/drm/xlnx/Kconfig            |  2 +-
 17 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 959b19a0410188d9..33792ca3eeb7ae8d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -75,11 +75,11 @@ config DRM_KUNIT_TEST_HELPERS
 config DRM_KUNIT_TEST
 	tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS
 	depends on DRM
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on KUNIT
 	depends on MMU
 	select DRM_BUDDY
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_EXEC
 	select DRM_EXPORT_FOR_TESTS if m
 	select DRM_GEM_SHMEM_HELPER
diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig
index ba09121e7debb247..cf931b94a1889746 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -3,12 +3,12 @@
 config DRM_AMDGPU
 	tristate "AMD GPU"
 	depends on DRM
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on MMU
 	depends on PCI
 	depends on !UML
 	select FW_LOADER
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index ad38cd201b006251..6015e2e4c2f620cf 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -93,9 +93,9 @@ config DRM_FSL_LDB
 config DRM_ITE_IT6505
 	tristate "ITE IT6505 DisplayPort bridge"
 	depends on DRM_DISPLAY_DP_AUX_BUS
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_KMS_HELPER
 	select EXTCON
@@ -226,9 +226,9 @@ config DRM_PARADE_PS8622
 config DRM_PARADE_PS8640
 	tristate "Parade PS8640 MIPI DSI to eDP Converter"
 	depends on DRM_DISPLAY_DP_AUX_BUS
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
 	select DRM_PANEL
@@ -312,9 +312,9 @@ config DRM_TOSHIBA_TC358764
 
 config DRM_TOSHIBA_TC358767
 	tristate "Toshiba TC358767 eDP bridge"
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	select DRM_MIPI_DSI
@@ -335,9 +335,9 @@ config DRM_TOSHIBA_TC358768
 
 config DRM_TOSHIBA_TC358775
 	tristate "Toshiba TC358775 DSI/LVDS bridge"
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	select DRM_PANEL
@@ -381,9 +381,9 @@ config DRM_TI_SN65DSI83
 config DRM_TI_SN65DSI86
 	tristate "TI SN65DSI86 DSI to eDP bridge"
 	depends on DRM_DISPLAY_DP_AUX_BUS
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
index 9659df6718de6db4..ec98c94535736c0a 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -1,10 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_ANALOGIX_ANX6345
 	tristate "Analogix ANX6345 bridge"
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_ANALOGIX_DP
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	help
@@ -15,9 +15,9 @@ config DRM_ANALOGIX_ANX6345
 
 config DRM_ANALOGIX_ANX78XX
 	tristate "Analogix ANX78XX bridge"
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	select DRM_ANALOGIX_DP
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	help
@@ -34,9 +34,9 @@ config DRM_ANALOGIX_ANX7625
 	tristate "Analogix Anx7625 MIPI to DP interface support"
 	depends on DRM
 	depends on DRM_DISPLAY_DP_AUX_BUS
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_MIPI_DSI
 	help
diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig b/drivers/gpu/drm/bridge/cadence/Kconfig
index 3480fd4d0a5f76e4..20143afded40e437 100644
--- a/drivers/gpu/drm/bridge/cadence/Kconfig
+++ b/drivers/gpu/drm/bridge/cadence/Kconfig
@@ -23,9 +23,9 @@ endif
 
 config DRM_CDNS_MHDP8546
 	tristate "Cadence DPI/DP bridge"
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_KMS_HELPER
 	select DRM_PANEL_BRIDGE
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 9801f47a3704530d..0cd4396914224226 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -39,7 +39,6 @@ config DRM_DISPLAY_DP_AUX_CHARDEV
 config DRM_DISPLAY_DP_HELPER
 	bool "DRM DisplayPort Helpers"
 	depends on DRM_DISPLAY_HELPER
-	default y
 	help
 	  DRM display helpers for DisplayPort.
 
diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 6a26a0b8eff2c021..4b0183bf221c8bb2 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -67,9 +67,9 @@ config DRM_EXYNOS_DSI
 config DRM_EXYNOS_DP
 	bool "Exynos specific extensions for Analogix DP driver"
 	depends on DRM_EXYNOS_FIMD || DRM_EXYNOS7_DECON
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	select DRM_ANALOGIX_DP
+	select DRM_DISPLAY_DP_HELPER
 	default DRM_EXYNOS
 	select DRM_PANEL
 	help
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index dbde4e29d93a79dc..43183a68a09557b7 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -2,7 +2,6 @@
 config DRM_I915
 	tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
 	depends on DRM
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on X86 && PCI
 	depends on !PREEMPT_RT
@@ -12,6 +11,7 @@ config DRM_I915
 	# the shmem_readpage() which depends upon tmpfs
 	select SHMEM
 	select TMPFS
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index 6caab8d4d4e0f5f8..2add54486ac4ab11 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -23,10 +23,10 @@ config DRM_MEDIATEK
 config DRM_MEDIATEK_DP
 	tristate "DRM DPTX Support for MediaTek SoCs"
 	depends on DRM_DISPLAY_DP_AUX_BUS
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on DRM_MEDIATEK
 	select PHY_MTK_DP
+	select DRM_DISPLAY_DP_HELPER
 	help
 	  DRM/KMS Display Port driver for MediaTek SoCs.
 
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index f7708590583e7377..28a898722ace789b 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -6,7 +6,6 @@ config DRM_MSM
 	depends on COMMON_CLK
 	depends on DRM
 	depends on DRM_DISPLAY_DP_AUX_BUS
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on IOMMU_SUPPORT
 	depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
@@ -17,6 +16,7 @@ config DRM_MSM
 	select IOMMU_IO_PGTABLE
 	select QCOM_MDT_LOADER if ARCH_QCOM
 	select REGULATOR
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_EXEC
 	select DRM_KMS_HELPER
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 7cc305b2826d6abc..5ac852b816db262b 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -2,12 +2,12 @@
 config DRM_NOUVEAU
 	tristate "Nouveau (NVIDIA) cards"
 	depends on DRM
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on PCI
 	depends on MMU
 	select IOMMU_API
 	select FW_LOADER
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
 	select DRM_TTM
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index ab67789e59a2e7a6..ed7949bebad812c9 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -545,10 +545,10 @@ config DRM_PANEL_RAYDIUM_RM68200
 config DRM_PANEL_RAYDIUM_RM692E5
 	tristate "Raydium RM692E5-based DSI panel"
 	depends on BACKLIGHT_CLASS_DEVICE
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on DRM_MIPI_DSI
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	help
 	  Say Y here if you want to enable support for Raydium RM692E5-based
 	  display panels, such as the one found in the Fairphone 5 smartphone.
@@ -572,10 +572,10 @@ config DRM_PANEL_SAMSUNG_ATNA33XC20
 	tristate "Samsung ATNA33XC20 eDP panel"
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on DRM_DISPLAY_DP_AUX_BUS
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	depends on PM
+	select DRM_DISPLAY_DP_HELPER
 	help
 	  DRM panel driver for the Samsung ATNA33XC20 panel. This panel can't
 	  be handled by the DRM_PANEL_SIMPLE driver because its power
@@ -812,11 +812,11 @@ config DRM_PANEL_EDP
 	tristate "support for simple Embedded DisplayPort panels"
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on DRM_DISPLAY_DP_AUX_BUS
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	depends on PM
 	select VIDEOMODE_HELPERS
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_KMS_HELPER
 	help
 	  DRM panel driver for dumb eDP panels that need at most a regulator and
@@ -891,10 +891,10 @@ config DRM_PANEL_TRULY_NT35597_WQXGA
 config DRM_PANEL_VISIONOX_R66451
 	tristate "Visionox R66451"
 	depends on BACKLIGHT_CLASS_DEVICE
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on DRM_MIPI_DSI
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	help
 	  Say Y here if you want to enable support for Visionox
 	  R66451 1080x2340 AMOLED DSI panel.
diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig
index 18c867219a706a17..07d330450f05f899 100644
--- a/drivers/gpu/drm/radeon/Kconfig
+++ b/drivers/gpu/drm/radeon/Kconfig
@@ -4,11 +4,11 @@ config DRM_RADEON
 	tristate "ATI Radeon"
 	depends on AGP || !AGP
 	depends on DRM
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on PCI
 	depends on MMU
 	select FW_LOADER
+	select DRM_DISPLAY_DP_HELPER
         select DRM_KMS_HELPER
 	select DRM_SUBALLOC_HELPER
         select DRM_TTM
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 4b49a14758fe0412..b72c0bbf346da5fa 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -35,9 +35,9 @@ config ROCKCHIP_VOP2
 
 config ROCKCHIP_ANALOGIX_DP
 	bool "Rockchip specific extensions for Analogix DP driver"
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on ROCKCHIP_VOP
+	select DRM_DISPLAY_DP_HELPER
 	help
 	  This selects support for Rockchip SoC specific extensions
 	  for the Analogix Core DP driver. If you want to enable DP
@@ -45,9 +45,9 @@ config ROCKCHIP_ANALOGIX_DP
 
 config ROCKCHIP_CDN_DP
 	bool "Rockchip cdn DP"
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on EXTCON=y || (EXTCON=m && DRM_ROCKCHIP=m)
+	select DRM_DISPLAY_DP_HELPER
 	help
 	  This selects support for Rockchip SoC specific extensions
 	  for the cdn DP driver. If you want to enable Dp on
diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index bb6e35261f1144b1..e0385d175ec6d266 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -5,9 +5,9 @@ config DRM_TEGRA
 	depends on COMMON_CLK
 	depends on DRM
 	depends on DRM_DISPLAY_DP_AUX_BUS
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 02da2faf5ae33bdb..be29e5cd5215e842 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -3,7 +3,6 @@ config DRM_XE
 	tristate "Intel Xe Graphics"
 	depends on (m || (y && KUNIT=y))
 	depends on DRM
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on MMU
 	depends on PCI
@@ -18,6 +17,7 @@ config DRM_XE
 	select DRM_KUNIT_TEST_HELPERS if DRM_XE_KUNIT_TEST != n
 	select DRM_PANEL
 	select DRM_SUBALLOC_HELPER
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_MIPI_DSI
diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
index 41d753b14ccd9574..7a14a8c2e7be9e59 100644
--- a/drivers/gpu/drm/xlnx/Kconfig
+++ b/drivers/gpu/drm/xlnx/Kconfig
@@ -4,12 +4,12 @@ config DRM_ZYNQMP_DPSUB
 	depends on COMMON_CLK
 	depends on DMADEVICES
 	depends on DRM
-	depends on DRM_DISPLAY_DP_HELPER
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	depends on PHY_XILINX_ZYNQMP
 	depends on XILINX_ZYNQMP_DPDMA
 	select DMA_ENGINE
+	select DRM_DISPLAY_DP_HELPER
 	select DRM_GEM_DMA_HELPER
 	select DRM_KMS_HELPER
 	select GENERIC_PHY
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 08/11] Revert "drm: Switch DRM_DISPLAY_DP_AUX_BUS to depends on"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
                   ` (6 preceding siblings ...)
  2024-04-22 10:30 ` [PATCH 07/11] Revert "drm: Switch DRM_DISPLAY_DP_HELPER " Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 09/11] Revert "drm: Switch DRM_DISPLAY_HELPER " Geert Uytterhoeven
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit 4d15125d7fe637f401e64e33c99513adf6586fdd, as helper
code should always be selected by the driver that needs it, for the
convenience of the final user configuring a kernel.

The user who configures a kernel should not need to know which helpers
are needed for the driver he is interested in.  Making a driver depend
on helper code means that the user needs to know which helpers to enable
first, which is very user-unfriendly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/bridge/Kconfig          | 6 +++---
 drivers/gpu/drm/bridge/analogix/Kconfig | 2 +-
 drivers/gpu/drm/display/Kconfig         | 1 -
 drivers/gpu/drm/mediatek/Kconfig        | 2 +-
 drivers/gpu/drm/msm/Kconfig             | 2 +-
 drivers/gpu/drm/panel/Kconfig           | 4 ++--
 drivers/gpu/drm/tegra/Kconfig           | 2 +-
 7 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 6015e2e4c2f620cf..a51ad2b3a0fb01e2 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -92,11 +92,11 @@ config DRM_FSL_LDB
 
 config DRM_ITE_IT6505
 	tristate "ITE IT6505 DisplayPort bridge"
-	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
+	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	select EXTCON
 	select CRYPTO
@@ -225,10 +225,10 @@ config DRM_PARADE_PS8622
 
 config DRM_PARADE_PS8640
 	tristate "Parade PS8640 MIPI DSI to eDP Converter"
-	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
 	select DRM_PANEL
@@ -380,7 +380,6 @@ config DRM_TI_SN65DSI83
 
 config DRM_TI_SN65DSI86
 	tristate "TI SN65DSI86 DSI to eDP bridge"
-	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
@@ -389,6 +388,7 @@ config DRM_TI_SN65DSI86
 	select DRM_PANEL
 	select DRM_MIPI_DSI
 	select AUXILIARY_BUS
+	select DRM_DISPLAY_DP_AUX_BUS
 	help
 	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
 
diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
index ec98c94535736c0a..16d18dde483ae9c4 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -33,11 +33,11 @@ config DRM_ANALOGIX_DP
 config DRM_ANALOGIX_ANX7625
 	tristate "Analogix Anx7625 MIPI to DP interface support"
 	depends on DRM
-	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
+	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_MIPI_DSI
 	help
 	  ANX7625 is an ultra-low power 4K mobile HD transmitter
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 0cd4396914224226..cffa2acdbc6c0988 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -11,7 +11,6 @@ config DRM_DISPLAY_DP_AUX_BUS
 	tristate "DRM DisplayPort AUX bus support"
 	depends on DRM
 	depends on OF || COMPILE_TEST
-	default y
 
 config DRM_DISPLAY_DP_AUX_CEC
 	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index 2add54486ac4ab11..50bb28327f65fbf5 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -22,11 +22,11 @@ config DRM_MEDIATEK
 
 config DRM_MEDIATEK_DP
 	tristate "DRM DPTX Support for MediaTek SoCs"
-	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_HELPER
 	depends on DRM_MEDIATEK
 	select PHY_MTK_DP
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_DP_AUX_BUS
 	help
 	  DRM/KMS Display Port driver for MediaTek SoCs.
 
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 28a898722ace789b..2055266506e5adf0 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -5,7 +5,6 @@ config DRM_MSM
 	depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST
 	depends on COMMON_CLK
 	depends on DRM
-	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_HELPER
 	depends on IOMMU_SUPPORT
 	depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
@@ -16,6 +15,7 @@ config DRM_MSM
 	select IOMMU_IO_PGTABLE
 	select QCOM_MDT_LOADER if ARCH_QCOM
 	select REGULATOR
+	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_EXEC
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index ed7949bebad812c9..5073c45481d55821 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -571,11 +571,11 @@ config DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01
 config DRM_PANEL_SAMSUNG_ATNA33XC20
 	tristate "Samsung ATNA33XC20 eDP panel"
 	depends on BACKLIGHT_CLASS_DEVICE
-	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	depends on PM
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_DP_AUX_BUS
 	help
 	  DRM panel driver for the Samsung ATNA33XC20 panel. This panel can't
 	  be handled by the DRM_PANEL_SIMPLE driver because its power
@@ -811,12 +811,12 @@ config DRM_PANEL_STARTEK_KD070FHFID015
 config DRM_PANEL_EDP
 	tristate "support for simple Embedded DisplayPort panels"
 	depends on BACKLIGHT_CLASS_DEVICE
-	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	depends on PM
 	select VIDEOMODE_HELPERS
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	help
 	  DRM panel driver for dumb eDP panels that need at most a regulator and
diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index e0385d175ec6d266..44381ee6ea9e36f7 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -4,11 +4,11 @@ config DRM_TEGRA
 	depends on ARCH_TEGRA || COMPILE_TEST
 	depends on COMMON_CLK
 	depends on DRM
-	depends on DRM_DISPLAY_DP_AUX_BUS
 	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
+	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
 	select DRM_PANEL
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 09/11] Revert "drm: Switch DRM_DISPLAY_HELPER to depends on"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
                   ` (7 preceding siblings ...)
  2024-04-22 10:30 ` [PATCH 08/11] Revert "drm: Switch DRM_DISPLAY_DP_AUX_BUS " Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 10/11] Revert "drm: Make drivers depends on DRM_DW_HDMI" Geert Uytterhoeven
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit e075e496f516bf92bc0cbaf94d64e8d4a6b58321, as helper
code should always be selected by the driver that needs it, for the
convenience of the final user configuring a kernel.

The user who configures a kernel should not need to know which helpers
are needed for the driver he is interested in.  Making a driver depend
on helper code means that the user needs to know which helpers to enable
first, which is very user-unfriendly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/Kconfig                 |  6 ++----
 drivers/gpu/drm/amd/amdgpu/Kconfig      |  6 ++----
 drivers/gpu/drm/bridge/Kconfig          | 10 +++++-----
 drivers/gpu/drm/bridge/analogix/Kconfig |  6 +++---
 drivers/gpu/drm/bridge/cadence/Kconfig  |  4 ++--
 drivers/gpu/drm/bridge/synopsys/Kconfig |  2 +-
 drivers/gpu/drm/display/Kconfig         |  1 -
 drivers/gpu/drm/exynos/Kconfig          |  2 +-
 drivers/gpu/drm/i915/Kconfig            |  2 +-
 drivers/gpu/drm/mediatek/Kconfig        |  2 +-
 drivers/gpu/drm/msm/Kconfig             |  4 ++--
 drivers/gpu/drm/nouveau/Kconfig         |  6 ++----
 drivers/gpu/drm/panel/Kconfig           | 20 ++++++++++----------
 drivers/gpu/drm/radeon/Kconfig          |  6 ++----
 drivers/gpu/drm/rockchip/Kconfig        |  4 ++--
 drivers/gpu/drm/tegra/Kconfig           |  2 +-
 drivers/gpu/drm/vc4/Kconfig             |  8 ++++----
 drivers/gpu/drm/xe/Kconfig              |  7 ++-----
 drivers/gpu/drm/xlnx/Kconfig            |  6 ++----
 19 files changed, 45 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 33792ca3eeb7ae8d..bf4020915e299861 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -74,12 +74,10 @@ config DRM_KUNIT_TEST_HELPERS
 
 config DRM_KUNIT_TEST
 	tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
-	depends on KUNIT
-	depends on MMU
+	depends on DRM && KUNIT && MMU
 	select DRM_BUDDY
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_EXEC
 	select DRM_EXPORT_FOR_TESTS if m
 	select DRM_GEM_SHMEM_HELPER
diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig
index cf931b94a1889746..22d88f8ef5279a0f 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -2,15 +2,13 @@
 
 config DRM_AMDGPU
 	tristate "AMD GPU"
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
-	depends on MMU
-	depends on PCI
+	depends on DRM && PCI && MMU
 	depends on !UML
 	select FW_LOADER
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select DRM_SCHED
 	select DRM_TTM
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index a51ad2b3a0fb01e2..f71d57216ae0602a 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -92,10 +92,10 @@ config DRM_FSL_LDB
 
 config DRM_ITE_IT6505
 	tristate "ITE IT6505 DisplayPort bridge"
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	select EXTCON
@@ -225,9 +225,9 @@ config DRM_PARADE_PS8622
 
 config DRM_PARADE_PS8640
 	tristate "Parade PS8640 MIPI DSI to eDP Converter"
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
@@ -312,9 +312,9 @@ config DRM_TOSHIBA_TC358764
 
 config DRM_TOSHIBA_TC358767
 	tristate "Toshiba TC358767 eDP bridge"
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	select DRM_MIPI_DSI
@@ -335,9 +335,9 @@ config DRM_TOSHIBA_TC358768
 
 config DRM_TOSHIBA_TC358775
 	tristate "Toshiba TC358775 DSI/LVDS bridge"
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	select DRM_PANEL
@@ -380,9 +380,9 @@ config DRM_TI_SN65DSI83
 
 config DRM_TI_SN65DSI86
 	tristate "TI SN65DSI86 DSI to eDP bridge"
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
index 16d18dde483ae9c4..4846b2e9be7c2a5d 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -1,10 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_ANALOGIX_ANX6345
 	tristate "Analogix ANX6345 bridge"
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_ANALOGIX_DP
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	help
@@ -15,9 +15,9 @@ config DRM_ANALOGIX_ANX6345
 
 config DRM_ANALOGIX_ANX78XX
 	tristate "Analogix ANX78XX bridge"
-	depends on DRM_DISPLAY_HELPER
 	select DRM_ANALOGIX_DP
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	help
@@ -33,10 +33,10 @@ config DRM_ANALOGIX_DP
 config DRM_ANALOGIX_ANX7625
 	tristate "Analogix Anx7625 MIPI to DP interface support"
 	depends on DRM
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_MIPI_DSI
 	help
diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig b/drivers/gpu/drm/bridge/cadence/Kconfig
index 20143afded40e437..cced81633ddcda03 100644
--- a/drivers/gpu/drm/bridge/cadence/Kconfig
+++ b/drivers/gpu/drm/bridge/cadence/Kconfig
@@ -23,12 +23,12 @@ endif
 
 config DRM_CDNS_MHDP8546
 	tristate "Cadence DPI/DP bridge"
-	depends on DRM_DISPLAY_HELPER
-	depends on OF
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select DRM_PANEL_BRIDGE
+	depends on OF
 	help
 	  Support Cadence DPI to DP bridge. This is an internal
 	  bridge and is meant to be directly embedded in a SoC.
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig
index f366ece471462a70..15fc182d05ef02e6 100644
--- a/drivers/gpu/drm/bridge/synopsys/Kconfig
+++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_DW_HDMI
 	tristate
-	depends on DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_MMIO
 	select CEC_CORE if CEC_NOTIFIER
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index cffa2acdbc6c0988..c77e7f85bd674dc9 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -3,7 +3,6 @@
 config DRM_DISPLAY_HELPER
 	tristate "DRM Display Helpers"
 	depends on DRM
-	default y
 	help
 	  DRM helpers for display adapters.
 
diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 4b0183bf221c8bb2..733b109a509525f7 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -4,6 +4,7 @@ config DRM_EXYNOS
 	depends on OF && DRM && COMMON_CLK
 	depends on ARCH_S3C64XX || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
 	depends on MMU
+	select DRM_DISPLAY_HELPER if DRM_EXYNOS_DP
 	select DRM_KMS_HELPER
 	select VIDEOMODE_HELPERS
 	select FB_DMAMEM_HELPERS if DRM_FBDEV_EMULATION
@@ -67,7 +68,6 @@ config DRM_EXYNOS_DSI
 config DRM_EXYNOS_DP
 	bool "Exynos specific extensions for Analogix DP driver"
 	depends on DRM_EXYNOS_FIMD || DRM_EXYNOS7_DECON
-	depends on DRM_DISPLAY_HELPER
 	select DRM_ANALOGIX_DP
 	select DRM_DISPLAY_DP_HELPER
 	default DRM_EXYNOS
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 43183a68a09557b7..5932024f8f9547e5 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -2,7 +2,6 @@
 config DRM_I915
 	tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
 	depends on DRM
-	depends on DRM_DISPLAY_HELPER
 	depends on X86 && PCI
 	depends on !PREEMPT_RT
 	select INTEL_GTT if X86
@@ -14,6 +13,7 @@ config DRM_I915
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select DRM_PANEL
 	select DRM_MIPI_DSI
diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index 50bb28327f65fbf5..96cbe020f493ab19 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -22,9 +22,9 @@ config DRM_MEDIATEK
 
 config DRM_MEDIATEK_DP
 	tristate "DRM DPTX Support for MediaTek SoCs"
-	depends on DRM_DISPLAY_HELPER
 	depends on DRM_MEDIATEK
 	select PHY_MTK_DP
+	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_DP_AUX_BUS
 	help
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 2055266506e5adf0..27d72ed8b3896b64 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -2,10 +2,9 @@
 
 config DRM_MSM
 	tristate "MSM DRM"
+	depends on DRM
 	depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST
 	depends on COMMON_CLK
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
 	depends on IOMMU_SUPPORT
 	depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
 	depends on QCOM_OCMEM || QCOM_OCMEM=n
@@ -17,6 +16,7 @@ config DRM_MSM
 	select REGULATOR
 	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_EXEC
 	select DRM_KMS_HELPER
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 5ac852b816db262b..ceef470c9fbfcfb0 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -1,14 +1,12 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_NOUVEAU
 	tristate "Nouveau (NVIDIA) cards"
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
-	depends on PCI
-	depends on MMU
+	depends on DRM && PCI && MMU
 	select IOMMU_API
 	select FW_LOADER
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select DRM_TTM
 	select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 5073c45481d55821..1b70ebba9571d29f 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -544,11 +544,11 @@ config DRM_PANEL_RAYDIUM_RM68200
 
 config DRM_PANEL_RAYDIUM_RM692E5
 	tristate "Raydium RM692E5-based DSI panel"
-	depends on BACKLIGHT_CLASS_DEVICE
-	depends on DRM_DISPLAY_HELPER
-	depends on DRM_MIPI_DSI
 	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	help
 	  Say Y here if you want to enable support for Raydium RM692E5-based
 	  display panels, such as the one found in the Fairphone 5 smartphone.
@@ -570,11 +570,11 @@ config DRM_PANEL_SAMSUNG_S6E88A0_AMS452EF01
 
 config DRM_PANEL_SAMSUNG_ATNA33XC20
 	tristate "Samsung ATNA33XC20 eDP panel"
-	depends on BACKLIGHT_CLASS_DEVICE
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	depends on BACKLIGHT_CLASS_DEVICE
 	depends on PM
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_AUX_BUS
 	help
 	  DRM panel driver for the Samsung ATNA33XC20 panel. This panel can't
@@ -810,12 +810,12 @@ config DRM_PANEL_STARTEK_KD070FHFID015
 
 config DRM_PANEL_EDP
 	tristate "support for simple Embedded DisplayPort panels"
-	depends on BACKLIGHT_CLASS_DEVICE
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
+	depends on BACKLIGHT_CLASS_DEVICE
 	depends on PM
 	select VIDEOMODE_HELPERS
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	help
@@ -890,11 +890,11 @@ config DRM_PANEL_TRULY_NT35597_WQXGA
 
 config DRM_PANEL_VISIONOX_R66451
 	tristate "Visionox R66451"
-	depends on BACKLIGHT_CLASS_DEVICE
-	depends on DRM_DISPLAY_HELPER
-	depends on DRM_MIPI_DSI
 	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	help
 	  Say Y here if you want to enable support for Visionox
 	  R66451 1080x2340 AMOLED DSI panel.
diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig
index 07d330450f05f899..f98356be0af288f5 100644
--- a/drivers/gpu/drm/radeon/Kconfig
+++ b/drivers/gpu/drm/radeon/Kconfig
@@ -2,13 +2,11 @@
 
 config DRM_RADEON
 	tristate "ATI Radeon"
+	depends on DRM && PCI && MMU
 	depends on AGP || !AGP
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
-	depends on PCI
-	depends on MMU
 	select FW_LOADER
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
         select DRM_KMS_HELPER
 	select DRM_SUBALLOC_HELPER
         select DRM_TTM
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index b72c0bbf346da5fa..0d5260e10f272d3b 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -35,8 +35,8 @@ config ROCKCHIP_VOP2
 
 config ROCKCHIP_ANALOGIX_DP
 	bool "Rockchip specific extensions for Analogix DP driver"
-	depends on DRM_DISPLAY_HELPER
 	depends on ROCKCHIP_VOP
+	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_HELPER
 	help
 	  This selects support for Rockchip SoC specific extensions
@@ -45,8 +45,8 @@ config ROCKCHIP_ANALOGIX_DP
 
 config ROCKCHIP_CDN_DP
 	bool "Rockchip cdn DP"
-	depends on DRM_DISPLAY_HELPER
 	depends on EXTCON=y || (EXTCON=m && DRM_ROCKCHIP=m)
+	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_HELPER
 	help
 	  This selects support for Rockchip SoC specific extensions
diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index 44381ee6ea9e36f7..782f51d3044af1fc 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -4,10 +4,10 @@ config DRM_TEGRA
 	depends on ARCH_TEGRA || COMPILE_TEST
 	depends on COMMON_CLK
 	depends on DRM
-	depends on DRM_DISPLAY_HELPER
 	depends on OF
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 98772a6b5bf0df54..91dcf8d174d6c6e1 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -2,15 +2,15 @@
 config DRM_VC4
 	tristate "Broadcom VC4 Graphics"
 	depends on ARCH_BCM || ARCH_BCM2835 || COMPILE_TEST
-	depends on COMMON_CLK
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
-	depends on PM
 	# Make sure not 'y' when RASPBERRYPI_FIRMWARE is 'm'. This can only
 	# happen when COMPILE_TEST=y, hence the added !RASPBERRYPI_FIRMWARE.
 	depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
+	depends on DRM
 	depends on SND && SND_SOC
+	depends on COMMON_CLK
+	depends on PM
 	select DRM_DISPLAY_HDMI_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select DRM_GEM_DMA_HELPER
 	select DRM_PANEL_BRIDGE
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index be29e5cd5215e842..1a556d087e63c30f 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -1,11 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_XE
 	tristate "Intel Xe Graphics"
-	depends on (m || (y && KUNIT=y))
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
-	depends on MMU
-	depends on PCI
+	depends on DRM && PCI && MMU && (m || (y && KUNIT=y))
 	select INTERVAL_TREE
 	# we need shmfs for the swappable backing store, and in particular
 	# the shmem_readpage() which depends upon tmpfs
@@ -20,6 +16,7 @@ config DRM_XE
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_MIPI_DSI
 	select RELAY
 	select IRQ_WORK
diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
index 7a14a8c2e7be9e59..68ee897de9d75884 100644
--- a/drivers/gpu/drm/xlnx/Kconfig
+++ b/drivers/gpu/drm/xlnx/Kconfig
@@ -1,15 +1,13 @@
 config DRM_ZYNQMP_DPSUB
 	tristate "ZynqMP DisplayPort Controller Driver"
 	depends on ARCH_ZYNQMP || COMPILE_TEST
-	depends on COMMON_CLK
+	depends on COMMON_CLK && DRM && OF
 	depends on DMADEVICES
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
-	depends on OF
 	depends on PHY_XILINX_ZYNQMP
 	depends on XILINX_ZYNQMP_DPDMA
 	select DMA_ENGINE
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_GEM_DMA_HELPER
 	select DRM_KMS_HELPER
 	select GENERIC_PHY
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 10/11] Revert "drm: Make drivers depends on DRM_DW_HDMI"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
                   ` (8 preceding siblings ...)
  2024-04-22 10:30 ` [PATCH 09/11] Revert "drm: Switch DRM_DISPLAY_HELPER " Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 10:30 ` [PATCH 11/11] Revert "drm/display: Make all helpers visible and switch to depends on" Geert Uytterhoeven
  2024-04-22 11:50 ` [PATCH 00/11] drm: Restore helper usability Jani Nikula
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit c0e0f139354c01e0213204e4a96e7076e5a3e396, as helper
code should always be selected by the driver that needs it, for the
convenience of the final user configuring a kernel.

The user who configures a kernel should not need to know which helpers
are needed for the driver he is interested in.  Making a driver depend
on helper code means that the user needs to know which helpers to enable
first, which is very user-unfriendly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/bridge/imx/Kconfig      | 4 ++--
 drivers/gpu/drm/imx/ipuv3/Kconfig       | 5 ++---
 drivers/gpu/drm/ingenic/Kconfig         | 2 +-
 drivers/gpu/drm/meson/Kconfig           | 2 +-
 drivers/gpu/drm/renesas/rcar-du/Kconfig | 2 +-
 drivers/gpu/drm/rockchip/Kconfig        | 2 +-
 drivers/gpu/drm/sun4i/Kconfig           | 2 +-
 7 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
index 7687ed652df5b9d3..5965e8027529a19c 100644
--- a/drivers/gpu/drm/bridge/imx/Kconfig
+++ b/drivers/gpu/drm/bridge/imx/Kconfig
@@ -5,9 +5,9 @@ config DRM_IMX_LDB_HELPER
 
 config DRM_IMX8MP_DW_HDMI_BRIDGE
 	tristate "Freescale i.MX8MP HDMI-TX bridge support"
-	depends on COMMON_CLK
-	depends on DRM_DW_HDMI
 	depends on OF
+	depends on COMMON_CLK
+	select DRM_DW_HDMI
 	select DRM_IMX8MP_HDMI_PVI
 	select PHY_FSL_SAMSUNG_HDMI_PHY
 	help
diff --git a/drivers/gpu/drm/imx/ipuv3/Kconfig b/drivers/gpu/drm/imx/ipuv3/Kconfig
index 5d810ac02171d9c5..bacf0655ebaf3f5f 100644
--- a/drivers/gpu/drm/imx/ipuv3/Kconfig
+++ b/drivers/gpu/drm/imx/ipuv3/Kconfig
@@ -35,8 +35,7 @@ config DRM_IMX_LDB
 
 config DRM_IMX_HDMI
 	tristate "Freescale i.MX DRM HDMI"
-	depends on DRM_DW_HDMI
-	depends on DRM_IMX
-	depends on OF
+	select DRM_DW_HDMI
+	depends on DRM_IMX && OF
 	help
 	  Choose this if you want to use HDMI on i.MX6.
diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig
index 23effeb2ac721749..3db117c5edd9161b 100644
--- a/drivers/gpu/drm/ingenic/Kconfig
+++ b/drivers/gpu/drm/ingenic/Kconfig
@@ -27,8 +27,8 @@ config DRM_INGENIC_IPU
 
 config DRM_INGENIC_DW_HDMI
 	tristate "Ingenic specific support for Synopsys DW HDMI"
-	depends on DRM_DW_HDMI
 	depends on MACH_JZ4780
+	select DRM_DW_HDMI
 	help
 	  Choose this option to enable Synopsys DesignWare HDMI based driver.
 	  If you want to enable HDMI on Ingenic JZ4780 based SoC, you should
diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig
index 5520b9e3f0107c4d..615fdd0ce41b433a 100644
--- a/drivers/gpu/drm/meson/Kconfig
+++ b/drivers/gpu/drm/meson/Kconfig
@@ -13,9 +13,9 @@ config DRM_MESON
 
 config DRM_MESON_DW_HDMI
 	tristate "HDMI Synopsys Controller support for Amlogic Meson Display"
-	depends on DRM_DW_HDMI
 	depends on DRM_MESON
 	default y if DRM_MESON
+	select DRM_DW_HDMI
 	imply DRM_DW_HDMI_I2S_AUDIO
 
 config DRM_MESON_DW_MIPI_DSI
diff --git a/drivers/gpu/drm/renesas/rcar-du/Kconfig b/drivers/gpu/drm/renesas/rcar-du/Kconfig
index 2dc739db2ba33b99..53c356aed5d52070 100644
--- a/drivers/gpu/drm/renesas/rcar-du/Kconfig
+++ b/drivers/gpu/drm/renesas/rcar-du/Kconfig
@@ -25,8 +25,8 @@ config DRM_RCAR_CMM
 config DRM_RCAR_DW_HDMI
 	tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
 	depends on DRM && OF
-	depends on DRM_DW_HDMI
 	depends on DRM_RCAR_DU || COMPILE_TEST
+	select DRM_DW_HDMI
 	help
 	  Enable support for R-Car Gen3 or RZ/G2 internal HDMI encoder.
 
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 0d5260e10f272d3b..1bf3e2829cd07b6b 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -7,6 +7,7 @@ config DRM_ROCKCHIP
 	select DRM_PANEL
 	select VIDEOMODE_HELPERS
 	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
+	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
 	select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
 	select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
 	select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI
@@ -56,7 +57,6 @@ config ROCKCHIP_CDN_DP
 
 config ROCKCHIP_DW_HDMI
 	bool "Rockchip specific extensions for Synopsys DW HDMI"
-	depends on DRM_DW_HDMI
 	help
 	  This selects support for Rockchip SoC specific extensions
 	  for the Synopsys DesignWare HDMI driver. If you want to
diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index 5b19c7cb7b7ebf53..4741d9f6544c201b 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -57,8 +57,8 @@ config DRM_SUN6I_DSI
 config DRM_SUN8I_DW_HDMI
 	tristate "Support for Allwinner version of DesignWare HDMI"
 	depends on DRM_SUN4I
-	depends on DRM_DW_HDMI
 	default DRM_SUN4I
+	select DRM_DW_HDMI
 	help
 	  Choose this option if you have an Allwinner SoC with the
 	  DesignWare HDMI controller. SoCs that support HDMI and
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 11/11] Revert "drm/display: Make all helpers visible and switch to depends on"
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
                   ` (9 preceding siblings ...)
  2024-04-22 10:30 ` [PATCH 10/11] Revert "drm: Make drivers depends on DRM_DW_HDMI" Geert Uytterhoeven
@ 2024-04-22 10:30 ` Geert Uytterhoeven
  2024-04-22 11:50 ` [PATCH 00/11] drm: Restore helper usability Jani Nikula
  11 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 10:30 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jani Nikula, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

This reverts commit d674858ff979550a0e97b4ac766f2640f0d9d7e7, as helper
code should always be selected by the driver that needs it, for the
convenience of the final user configuring a kernel.

The user who configures a kernel should not need to know which helpers
are needed for the driver he is interested in.  Making a driver depend
on helper code means that the user needs to know which helpers to enable
first, which is very user-unfriendly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/display/Kconfig | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index c77e7f85bd674dc9..864a6488bfdf1499 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -1,21 +1,20 @@
 # SPDX-License-Identifier: MIT
 
 config DRM_DISPLAY_HELPER
-	tristate "DRM Display Helpers"
+	tristate
 	depends on DRM
 	help
 	  DRM helpers for display adapters.
 
 config DRM_DISPLAY_DP_AUX_BUS
-	tristate "DRM DisplayPort AUX bus support"
+	tristate
 	depends on DRM
 	depends on OF || COMPILE_TEST
 
 config DRM_DISPLAY_DP_AUX_CEC
 	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
-	depends on DRM_DISPLAY_DP_HELPER
+	depends on DRM && DRM_DISPLAY_HELPER
+	select DRM_DISPLAY_DP_HELPER
 	select CEC_CORE
 	help
 	  Choose this option if you want to enable HDMI CEC support for
@@ -25,24 +24,23 @@ config DRM_DISPLAY_DP_AUX_CEC
 	  that do support this they often do not hook up the CEC pin.
 
 config DRM_DISPLAY_DP_AUX_CHARDEV
-	bool "DRM DisplayPort AUX Interface"
-	depends on DRM
-	depends on DRM_DISPLAY_HELPER
-	depends on DRM_DISPLAY_DP_HELPER
+	bool "DRM DP AUX Interface"
+	depends on DRM && DRM_DISPLAY_HELPER
+	select DRM_DISPLAY_DP_HELPER
 	help
 	  Choose this option to enable a /dev/drm_dp_auxN node that allows to
 	  read and write values to arbitrary DPCD registers on the DP aux
 	  channel.
 
 config DRM_DISPLAY_DP_HELPER
-	bool "DRM DisplayPort Helpers"
+	bool
 	depends on DRM_DISPLAY_HELPER
 	help
 	  DRM display helpers for DisplayPort.
 
 config DRM_DISPLAY_DP_TUNNEL
-	bool "DRM DisplayPort tunnels support"
-	depends on DRM_DISPLAY_DP_HELPER
+	bool
+	select DRM_DISPLAY_DP_HELPER
 	help
 	  Enable support for DisplayPort tunnels. This allows drivers to use
 	  DP tunnel features like the Bandwidth Allocation mode to maximize the
@@ -62,13 +60,13 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG
 	  If in doubt, say "N".
 
 config DRM_DISPLAY_HDCP_HELPER
-	bool "DRM HDCD Helpers"
+	bool
 	depends on DRM_DISPLAY_HELPER
 	help
 	  DRM display helpers for HDCP.
 
 config DRM_DISPLAY_HDMI_HELPER
-	bool "DRM HDMI Helpers"
+	bool
 	depends on DRM_DISPLAY_HELPER
 	help
 	  DRM display helpers for HDMI.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
                   ` (10 preceding siblings ...)
  2024-04-22 10:30 ` [PATCH 11/11] Revert "drm/display: Make all helpers visible and switch to depends on" Geert Uytterhoeven
@ 2024-04-22 11:50 ` Jani Nikula
  2024-04-22 12:27   ` Dmitry Baryshkov
  2024-04-22 12:30   ` Arnd Bergmann
  11 siblings, 2 replies; 25+ messages in thread
From: Jani Nikula @ 2024-04-22 11:50 UTC (permalink / raw)
  To: Geert Uytterhoeven, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter, Arnd Bergmann
  Cc: dri-devel, linux-kernel, linux-renesas-soc, Geert Uytterhoeven

On Mon, 22 Apr 2024, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> 	Hi all,
>
> As discussed on IRC with Maxime and Arnd, this series reverts the
> conversion of select to depends for various DRM helpers in series
> "[PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to
> depends on"[1], and various fixes for it.  This conversion introduced a
> big usability issue when configuring a kernel and enabling DRM drivers
> that use DRM helper code: as drivers now depend on helpers, the user
> needs to know which helpers to enable, before the driver he is
> interested even becomes visible.  The user should not need to know that,
> and drivers should select the helpers they need.
>
> Hence revert back to what we had before, where drivers selected the
> helpers (and any of their dependencies, if they can be met) they need.
> In general, when a symbol selects another symbol, it should just make
> sure the dependencies of the target symbol are met, which may mean
> adding dependencies to the source symbol.

I still disagree with this, because fundamentally the source symbol
really should not have to care about the dependencies of the target
symbol.

That said, I'm not going to keep arguing against this. Whatever.


BR,
Jani.


>
> Thanks for applying!
>
> [1] https://lore.kernel.org/r/20240327-kms-kconfig-helpers-v3-0-eafee11b84b3@kernel.org/
>
> Geert Uytterhoeven (11):
>   Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2"
>   Revert "drm/display: Select DRM_KMS_HELPER for DP helpers"
>   Revert "drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable"
>   Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies"
>   Revert "drm: Switch DRM_DISPLAY_HDMI_HELPER to depends on"
>   Revert "drm: Switch DRM_DISPLAY_HDCP_HELPER to depends on"
>   Revert "drm: Switch DRM_DISPLAY_DP_HELPER to depends on"
>   Revert "drm: Switch DRM_DISPLAY_DP_AUX_BUS to depends on"
>   Revert "drm: Switch DRM_DISPLAY_HELPER to depends on"
>   Revert "drm: Make drivers depends on DRM_DW_HDMI"
>   Revert "drm/display: Make all helpers visible and switch to depends
>     on"
>
>  drivers/gpu/drm/Kconfig                 |  8 +++----
>  drivers/gpu/drm/amd/amdgpu/Kconfig      | 12 ++++------
>  drivers/gpu/drm/bridge/Kconfig          | 28 +++++++++++-----------
>  drivers/gpu/drm/bridge/analogix/Kconfig | 18 +++++++-------
>  drivers/gpu/drm/bridge/cadence/Kconfig  |  8 +++----
>  drivers/gpu/drm/bridge/imx/Kconfig      |  4 ++--
>  drivers/gpu/drm/bridge/synopsys/Kconfig |  6 ++---
>  drivers/gpu/drm/display/Kconfig         | 32 ++++++++++---------------
>  drivers/gpu/drm/exynos/Kconfig          |  4 ++--
>  drivers/gpu/drm/i915/Kconfig            |  8 +++----
>  drivers/gpu/drm/imx/ipuv3/Kconfig       |  5 ++--
>  drivers/gpu/drm/ingenic/Kconfig         |  2 +-
>  drivers/gpu/drm/mediatek/Kconfig        |  6 ++---
>  drivers/gpu/drm/meson/Kconfig           |  2 +-
>  drivers/gpu/drm/msm/Kconfig             |  8 +++----
>  drivers/gpu/drm/nouveau/Kconfig         | 10 ++++----
>  drivers/gpu/drm/panel/Kconfig           | 32 ++++++++++++-------------
>  drivers/gpu/drm/radeon/Kconfig          |  8 +++----
>  drivers/gpu/drm/renesas/rcar-du/Kconfig |  2 +-
>  drivers/gpu/drm/rockchip/Kconfig        | 10 ++++----
>  drivers/gpu/drm/sun4i/Kconfig           |  2 +-
>  drivers/gpu/drm/tegra/Kconfig           |  8 +++----
>  drivers/gpu/drm/vc4/Kconfig             | 10 ++++----
>  drivers/gpu/drm/xe/Kconfig              | 13 ++++------
>  drivers/gpu/drm/xlnx/Kconfig            |  8 +++----
>  25 files changed, 116 insertions(+), 138 deletions(-)

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 11:50 ` [PATCH 00/11] drm: Restore helper usability Jani Nikula
@ 2024-04-22 12:27   ` Dmitry Baryshkov
  2024-04-22 12:30   ` Arnd Bergmann
  1 sibling, 0 replies; 25+ messages in thread
From: Dmitry Baryshkov @ 2024-04-22 12:27 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Geert Uytterhoeven, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter, Arnd Bergmann,
	dri-devel, linux-kernel, linux-renesas-soc

On Mon, Apr 22, 2024 at 02:50:09PM +0300, Jani Nikula wrote:
> On Mon, 22 Apr 2024, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> > 	Hi all,
> >
> > As discussed on IRC with Maxime and Arnd, this series reverts the
> > conversion of select to depends for various DRM helpers in series
> > "[PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to
> > depends on"[1], and various fixes for it.  This conversion introduced a
> > big usability issue when configuring a kernel and enabling DRM drivers
> > that use DRM helper code: as drivers now depend on helpers, the user
> > needs to know which helpers to enable, before the driver he is
> > interested even becomes visible.  The user should not need to know that,
> > and drivers should select the helpers they need.
> >
> > Hence revert back to what we had before, where drivers selected the
> > helpers (and any of their dependencies, if they can be met) they need.
> > In general, when a symbol selects another symbol, it should just make
> > sure the dependencies of the target symbol are met, which may mean
> > adding dependencies to the source symbol.
> 
> I still disagree with this, because fundamentally the source symbol
> really should not have to care about the dependencies of the target
> symbol.

I'd agree with you, but if the driver depends on helpers it becomes
ridiculous. So, it seems, we need a different solution for the original
problem.

> That said, I'm not going to keep arguing against this. Whatever.
> 
> 
> BR,
> Jani.
> 
> 
> >
> > Thanks for applying!
> >
> > [1] https://lore.kernel.org/r/20240327-kms-kconfig-helpers-v3-0-eafee11b84b3@kernel.org/
> >
> > Geert Uytterhoeven (11):
> >   Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2"
> >   Revert "drm/display: Select DRM_KMS_HELPER for DP helpers"
> >   Revert "drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable"
> >   Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies"
> >   Revert "drm: Switch DRM_DISPLAY_HDMI_HELPER to depends on"
> >   Revert "drm: Switch DRM_DISPLAY_HDCP_HELPER to depends on"
> >   Revert "drm: Switch DRM_DISPLAY_DP_HELPER to depends on"
> >   Revert "drm: Switch DRM_DISPLAY_DP_AUX_BUS to depends on"
> >   Revert "drm: Switch DRM_DISPLAY_HELPER to depends on"
> >   Revert "drm: Make drivers depends on DRM_DW_HDMI"
> >   Revert "drm/display: Make all helpers visible and switch to depends
> >     on"
> >
> >  drivers/gpu/drm/Kconfig                 |  8 +++----
> >  drivers/gpu/drm/amd/amdgpu/Kconfig      | 12 ++++------
> >  drivers/gpu/drm/bridge/Kconfig          | 28 +++++++++++-----------
> >  drivers/gpu/drm/bridge/analogix/Kconfig | 18 +++++++-------
> >  drivers/gpu/drm/bridge/cadence/Kconfig  |  8 +++----
> >  drivers/gpu/drm/bridge/imx/Kconfig      |  4 ++--
> >  drivers/gpu/drm/bridge/synopsys/Kconfig |  6 ++---
> >  drivers/gpu/drm/display/Kconfig         | 32 ++++++++++---------------
> >  drivers/gpu/drm/exynos/Kconfig          |  4 ++--
> >  drivers/gpu/drm/i915/Kconfig            |  8 +++----
> >  drivers/gpu/drm/imx/ipuv3/Kconfig       |  5 ++--
> >  drivers/gpu/drm/ingenic/Kconfig         |  2 +-
> >  drivers/gpu/drm/mediatek/Kconfig        |  6 ++---
> >  drivers/gpu/drm/meson/Kconfig           |  2 +-
> >  drivers/gpu/drm/msm/Kconfig             |  8 +++----
> >  drivers/gpu/drm/nouveau/Kconfig         | 10 ++++----
> >  drivers/gpu/drm/panel/Kconfig           | 32 ++++++++++++-------------
> >  drivers/gpu/drm/radeon/Kconfig          |  8 +++----
> >  drivers/gpu/drm/renesas/rcar-du/Kconfig |  2 +-
> >  drivers/gpu/drm/rockchip/Kconfig        | 10 ++++----
> >  drivers/gpu/drm/sun4i/Kconfig           |  2 +-
> >  drivers/gpu/drm/tegra/Kconfig           |  8 +++----
> >  drivers/gpu/drm/vc4/Kconfig             | 10 ++++----
> >  drivers/gpu/drm/xe/Kconfig              | 13 ++++------
> >  drivers/gpu/drm/xlnx/Kconfig            |  8 +++----
> >  25 files changed, 116 insertions(+), 138 deletions(-)
> 
> -- 
> Jani Nikula, Intel

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 11:50 ` [PATCH 00/11] drm: Restore helper usability Jani Nikula
  2024-04-22 12:27   ` Dmitry Baryshkov
@ 2024-04-22 12:30   ` Arnd Bergmann
  2024-04-22 13:28     ` Jani Nikula
  1 sibling, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2024-04-22 12:30 UTC (permalink / raw)
  To: Jani Nikula, Geert Uytterhoeven, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Dave Airlie, Daniel Vetter
  Cc: dri-devel, linux-kernel, Linux-Renesas

On Mon, Apr 22, 2024, at 13:50, Jani Nikula wrote:
> On Mon, 22 Apr 2024, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>> 	Hi all,
>>
>> As discussed on IRC with Maxime and Arnd, this series reverts the
>> conversion of select to depends for various DRM helpers in series
>> "[PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to
>> depends on"[1], and various fixes for it.  This conversion introduced a
>> big usability issue when configuring a kernel and enabling DRM drivers
>> that use DRM helper code: as drivers now depend on helpers, the user
>> needs to know which helpers to enable, before the driver he is
>> interested even becomes visible.  The user should not need to know that,
>> and drivers should select the helpers they need.
>>
>> Hence revert back to what we had before, where drivers selected the
>> helpers (and any of their dependencies, if they can be met) they need.
>> In general, when a symbol selects another symbol, it should just make
>> sure the dependencies of the target symbol are met, which may mean
>> adding dependencies to the source symbol.

Thanks for doing this.

Acked-by: Arnd Bergmann <arnd@arndb.de>

> I still disagree with this, because fundamentally the source symbol
> really should not have to care about the dependencies of the target
> symbol.

Sorry you missed the IRC discussion on #armlinux, we should have
included you as well since you applied the original patch.

I think the reason for this revert is a bit more nuanced than
just the usability problem. Sorry if I'm dragging this out too
much, but I want to be sure that two points come across:

1. There is a semantic problem that is mostly subjective, but
   with the naming as "helper", I generally expect it as a hidden
   symbol that gets selected by its users, while calling same module
   "feature" would be something that is user-enabled and that
   other modules depend on. Both ways are commonly used in the
   kernel and are not mistakes on their own.

2. Using "select" on user visible symbols that have dependencies
   is a common source for bugs, and this is is a problem in
   drivers/gpu/drm more than elsewhere in the kernel, as these
   drivers traditionally select entire subsystems or drivers
   (I2C, VIRTIO, INPUT, ACPI_WMI, BACKLIGHT_CLASS_DEVICE,
   POWER_SUPPLY, SND_PCM, INTERCONNECT, ...). This regularly
   leads to circular dependencies and we should fix all of them.
   The display helpers however don't have this problem because
   they do not have any dependencies outside of drivers/gpu/

        Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 12:30   ` Arnd Bergmann
@ 2024-04-22 13:28     ` Jani Nikula
  2024-04-22 13:54       ` Arnd Bergmann
  0 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2024-04-22 13:28 UTC (permalink / raw)
  To: Arnd Bergmann, Geert Uytterhoeven, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Dave Airlie, Daniel Vetter
  Cc: dri-devel, linux-kernel, Linux-Renesas

On Mon, 22 Apr 2024, "Arnd Bergmann" <arnd@arndb.de> wrote:
> On Mon, Apr 22, 2024, at 13:50, Jani Nikula wrote:
>> On Mon, 22 Apr 2024, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>>> 	Hi all,
>>>
>>> As discussed on IRC with Maxime and Arnd, this series reverts the
>>> conversion of select to depends for various DRM helpers in series
>>> "[PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to
>>> depends on"[1], and various fixes for it.  This conversion introduced a
>>> big usability issue when configuring a kernel and enabling DRM drivers
>>> that use DRM helper code: as drivers now depend on helpers, the user
>>> needs to know which helpers to enable, before the driver he is
>>> interested even becomes visible.  The user should not need to know that,
>>> and drivers should select the helpers they need.
>>>
>>> Hence revert back to what we had before, where drivers selected the
>>> helpers (and any of their dependencies, if they can be met) they need.
>>> In general, when a symbol selects another symbol, it should just make
>>> sure the dependencies of the target symbol are met, which may mean
>>> adding dependencies to the source symbol.
>
> Thanks for doing this.
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
>
>> I still disagree with this, because fundamentally the source symbol
>> really should not have to care about the dependencies of the target
>> symbol.
>
> Sorry you missed the IRC discussion on #armlinux, we should have
> included you as well since you applied the original patch.
>
> I think the reason for this revert is a bit more nuanced than
> just the usability problem. Sorry if I'm dragging this out too
> much, but I want to be sure that two points come across:
>
> 1. There is a semantic problem that is mostly subjective, but
>    with the naming as "helper", I generally expect it as a hidden
>    symbol that gets selected by its users, while calling same module
>    "feature" would be something that is user-enabled and that
>    other modules depend on. Both ways are commonly used in the
>    kernel and are not mistakes on their own.

Fair enough. I believe for (optional) "feature" the common pattern would
then be depends on FEATURE || FEATURE=n.

> 2. Using "select" on user visible symbols that have dependencies
>    is a common source for bugs, and this is is a problem in
>    drivers/gpu/drm more than elsewhere in the kernel, as these
>    drivers traditionally select entire subsystems or drivers
>    (I2C, VIRTIO, INPUT, ACPI_WMI, BACKLIGHT_CLASS_DEVICE,
>    POWER_SUPPLY, SND_PCM, INTERCONNECT, ...). This regularly
>    leads to circular dependencies and we should fix all of them.

What annoys me is that the fixes tend to fall in two categories:

- Play catch with selecting the dependencies of the selected
  symbols. "depends on" handles this recursively, while select does
  not. There is no end to this, it just goes on and on, as the
  dependencies of the selected symbols change over time. Often the
  selects require unintuitive if patterns that are about the
  implementation details of the symbol being selected.

- Brush the invalid configs under the rug by using IS_REACHABLE(),
  switching from a loud link time failure to a silent runtime
  failure. (I regularly reject patches adding IS_REACHABLE() to i915,
  because from my pov e.g. i915=y backlight=m is an invalid
  configuration that the user shouldn't have to debug at runtime. But I
  can't express that in kconfig while everyone selects backlight.)

If you have other ideas how these should be fixed, I'm all ears.

>    The display helpers however don't have this problem because
>    they do not have any dependencies outside of drivers/gpu/

Fair enough, though I think they still suffer from some of them having
dependencies. (Wasn't this how the original patches and the debate all
got started?)


BR,
Jani.


-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 13:28     ` Jani Nikula
@ 2024-04-22 13:54       ` Arnd Bergmann
  2024-04-22 16:58         ` Geert Uytterhoeven
  2024-04-22 17:00         ` Jani Nikula
  0 siblings, 2 replies; 25+ messages in thread
From: Arnd Bergmann @ 2024-04-22 13:54 UTC (permalink / raw)
  To: Jani Nikula, Geert Uytterhoeven, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Dave Airlie, Daniel Vetter
  Cc: dri-devel, linux-kernel, Linux-Renesas

On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote:
> On Mon, 22 Apr 2024, "Arnd Bergmann" <arnd@arndb.de> wrote:
>> On Mon, Apr 22, 2024, at 13:50, Jani Nikula wrote:
>>
>>> I still disagree with this, because fundamentally the source symbol
>>> really should not have to care about the dependencies of the target
>>> symbol.
>>
>> Sorry you missed the IRC discussion on #armlinux, we should have
>> included you as well since you applied the original patch.
>>
>> I think the reason for this revert is a bit more nuanced than
>> just the usability problem. Sorry if I'm dragging this out too
>> much, but I want to be sure that two points come across:
>>
>> 1. There is a semantic problem that is mostly subjective, but
>>    with the naming as "helper", I generally expect it as a hidden
>>    symbol that gets selected by its users, while calling same module
>>    "feature" would be something that is user-enabled and that
>>    other modules depend on. Both ways are commonly used in the
>>    kernel and are not mistakes on their own.
>
> Fair enough. I believe for (optional) "feature" the common pattern would
> then be depends on FEATURE || FEATURE=n.
>
>> 2. Using "select" on user visible symbols that have dependencies
>>    is a common source for bugs, and this is is a problem in
>>    drivers/gpu/drm more than elsewhere in the kernel, as these
>>    drivers traditionally select entire subsystems or drivers
>>    (I2C, VIRTIO, INPUT, ACPI_WMI, BACKLIGHT_CLASS_DEVICE,
>>    POWER_SUPPLY, SND_PCM, INTERCONNECT, ...). This regularly
>>    leads to circular dependencies and we should fix all of them.
>
> What annoys me is that the fixes tend to fall in two categories:
>
> - Play catch with selecting the dependencies of the selected
>   symbols. "depends on" handles this recursively, while select does
>   not.

I'm not sure where this misunderstanding comes from, as you
seem to be repeating the same incorrect assumption about
how select works that Maxime wrote in his changelog. To clarify,
this works exactly as one would expect:

config HELPER_A
       tristate

config HELPER_B
       tristate
       select HELPER_A

config DRIVER
       tristate "Turn on the driver and the helpers it uses"
       select HELPER_B # this recursively selects HELPER_A

Whereas this one is broken:

config FEATURE_A
       tristate "user visible if I2C is enabled"
       depends on I2C

config HELPER_B
       tristate # hidden
       select FEATURE_A

config DRIVER
       tristate "This driver is broken if I2C is disabled"
       select HELPER_B

>   There is no end to this, it just goes on and on, as the
>   dependencies of the selected symbols change over time. Often the
>   selects require unintuitive if patterns that are about the
>   implementation details of the symbol being selected.

Agreed, that is the problem I frequently face with drivers/gpu/drm,
and most of the time it can only be solved by rewriting the whole
system to not select user-visible symbol at all.

Using 'depends on' by itself is unfortunately not enough to
avoid /all/ the problems. See e.g. today's failure

config DRM_DISPLAY_HELPER
       tristate "DRM Display Helpers"
       default y

config DRM_DISPLAY_DP_HELPER
       bool "DRM DisplayPort Helpers"
       depends on DRM_DISPLAY_HELPER

config DRM_PANEL_LG_SW43408
       tristate "LG SW43408 panel"
       depends on DRM_DISPLAY_DP_HELPER

This version is still broken for DRM_DISPLAY_HELPER=m,
DRM_DISPLAY_DP_HELPER=m, DRM_PANEL_LG_SW43408=y because
the dependency on the bool symbol is not enough to
ensure that DRM_DISPLAY_HELPER is also built-in, so you
still need explicit dependencies on both
DRM_DISPLAY_HELPER and DRM_DISPLAY_DP_HELPER in the users.

This can be solved by making DRM_DISPLAY_DP_HELPER a
tristate symbol and adjusting the #ifdef checks and
Makefile logic accordingly, which is exactly what you'd
need to do to make it work with 'select' as well.

> - Brush the invalid configs under the rug by using IS_REACHABLE(),
>   switching from a loud link time failure to a silent runtime
>   failure. (I regularly reject patches adding IS_REACHABLE() to i915,
>   because from my pov e.g. i915=y backlight=m is an invalid
>   configuration that the user shouldn't have to debug at runtime. But I
>   can't express that in kconfig while everyone selects backlight.)

Thanks a lot for rejecting the IS_REACHABLE() patches, it is
indeed the worst way to handle those (I know, as I introduced
IS_REACHABLE() only to replace open-coded versions of the same,
not to have it as a feature or to use it in new code).

> If you have other ideas how these should be fixed, I'm all ears.
>
>>    The display helpers however don't have this problem because
>>    they do not have any dependencies outside of drivers/gpu/
>
> Fair enough, though I think they still suffer from some of them having
> dependencies. (Wasn't this how the original patches and the debate all
> got started?)

I believe that Maxime said on IRC that he only did the patches
originally because he expected problems with them based on his
understanding on how Kconfig works. I'm not aware of any
particular problem here.

Let me know if you see a problem with any of the symbols that
Geert has proposed for reverting, and I'll try to find a solution.
In my randconfig test environment, I have several patches that
I sent in the past to clean up the ACPI_VIDEO, I2C, BACKLIGHT and
LED dependencies to stop using 'select' as I could not otherwise
get nouveau, i915 and xe to build reliably, but that means I
may be missing some of the other problems.

     Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 13:54       ` Arnd Bergmann
@ 2024-04-22 16:58         ` Geert Uytterhoeven
  2024-04-22 17:14           ` Jani Nikula
  2024-04-22 18:23           ` Arnd Bergmann
  2024-04-22 17:00         ` Jani Nikula
  1 sibling, 2 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 16:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jani Nikula, Geert Uytterhoeven, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Dave Airlie, Daniel Vetter,
	dri-devel, linux-kernel, Linux-Renesas, Masahiro Yamada,
	linux-kbuild

Hi Arnd,

CC kbuild

On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote:
> > On Mon, 22 Apr 2024, "Arnd Bergmann" <arnd@arndb.de> wrote:
> >> 2. Using "select" on user visible symbols that have dependencies
> >>    is a common source for bugs, and this is is a problem in
> >>    drivers/gpu/drm more than elsewhere in the kernel, as these
> >>    drivers traditionally select entire subsystems or drivers
> >>    (I2C, VIRTIO, INPUT, ACPI_WMI, BACKLIGHT_CLASS_DEVICE,
> >>    POWER_SUPPLY, SND_PCM, INTERCONNECT, ...). This regularly
> >>    leads to circular dependencies and we should fix all of them.
> >
> > What annoys me is that the fixes tend to fall in two categories:
> >
> > - Play catch with selecting the dependencies of the selected
> >   symbols. "depends on" handles this recursively, while select does
> >   not.
>
> I'm not sure where this misunderstanding comes from, as you
> seem to be repeating the same incorrect assumption about
> how select works that Maxime wrote in his changelog. To clarify,
> this works exactly as one would expect:
>
> config HELPER_A
>        tristate
>
> config HELPER_B
>        tristate
>        select HELPER_A
>
> config DRIVER
>        tristate "Turn on the driver and the helpers it uses"
>        select HELPER_B # this recursively selects HELPER_A
>
> Whereas this one is broken:
>
> config FEATURE_A
>        tristate "user visible if I2C is enabled"
>        depends on I2C
>
> config HELPER_B
>        tristate # hidden
>        select FEATURE_A
>
> config DRIVER
>        tristate "This driver is broken if I2C is disabled"
>        select HELPER_B

So the DRIVER section should gain a "depends on I2C" statement.

Yamada-san: would it be difficult to modify Kconfig to ignore symbols
like DRIVER that select other symbols with unmet dependencies?
Currently it already warns about that.

Handling this implicitly (instead of the current explict "depends
on") would have the disadvantage though: a user who is not aware of
the implicit dependency may wonder why DRIVER is invisible in his
config interface.

>
> >   There is no end to this, it just goes on and on, as the
> >   dependencies of the selected symbols change over time. Often the
> >   selects require unintuitive if patterns that are about the
> >   implementation details of the symbol being selected.
>
> Agreed, that is the problem I frequently face with drivers/gpu/drm,
> and most of the time it can only be solved by rewriting the whole
> system to not select user-visible symbol at all.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 13:54       ` Arnd Bergmann
  2024-04-22 16:58         ` Geert Uytterhoeven
@ 2024-04-22 17:00         ` Jani Nikula
  2024-04-22 18:11           ` Geert Uytterhoeven
  1 sibling, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2024-04-22 17:00 UTC (permalink / raw)
  To: Arnd Bergmann, Geert Uytterhoeven, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Dave Airlie, Daniel Vetter
  Cc: dri-devel, linux-kernel, Linux-Renesas

On Mon, 22 Apr 2024, "Arnd Bergmann" <arnd@arndb.de> wrote:
> On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote:
>> On Mon, 22 Apr 2024, "Arnd Bergmann" <arnd@arndb.de> wrote:
>>> On Mon, Apr 22, 2024, at 13:50, Jani Nikula wrote:
>>>
>>>> I still disagree with this, because fundamentally the source symbol
>>>> really should not have to care about the dependencies of the target
>>>> symbol.
>>>
>>> Sorry you missed the IRC discussion on #armlinux, we should have
>>> included you as well since you applied the original patch.
>>>
>>> I think the reason for this revert is a bit more nuanced than
>>> just the usability problem. Sorry if I'm dragging this out too
>>> much, but I want to be sure that two points come across:
>>>
>>> 1. There is a semantic problem that is mostly subjective, but
>>>    with the naming as "helper", I generally expect it as a hidden
>>>    symbol that gets selected by its users, while calling same module
>>>    "feature" would be something that is user-enabled and that
>>>    other modules depend on. Both ways are commonly used in the
>>>    kernel and are not mistakes on their own.
>>
>> Fair enough. I believe for (optional) "feature" the common pattern would
>> then be depends on FEATURE || FEATURE=n.
>>
>>> 2. Using "select" on user visible symbols that have dependencies
>>>    is a common source for bugs, and this is is a problem in
>>>    drivers/gpu/drm more than elsewhere in the kernel, as these
>>>    drivers traditionally select entire subsystems or drivers
>>>    (I2C, VIRTIO, INPUT, ACPI_WMI, BACKLIGHT_CLASS_DEVICE,
>>>    POWER_SUPPLY, SND_PCM, INTERCONNECT, ...). This regularly
>>>    leads to circular dependencies and we should fix all of them.
>>
>> What annoys me is that the fixes tend to fall in two categories:
>>
>> - Play catch with selecting the dependencies of the selected
>>   symbols. "depends on" handles this recursively, while select does
>>   not.
>
> I'm not sure where this misunderstanding comes from, as you
> seem to be repeating the same incorrect assumption about
> how select works that Maxime wrote in his changelog. To clarify,
> this works exactly as one would expect:
>
> config HELPER_A
>        tristate
>
> config HELPER_B
>        tristate
>        select HELPER_A
>
> config DRIVER
>        tristate "Turn on the driver and the helpers it uses"
>        select HELPER_B # this recursively selects HELPER_A
>
> Whereas this one is broken:
>
> config FEATURE_A
>        tristate "user visible if I2C is enabled"
>        depends on I2C
>
> config HELPER_B
>        tristate # hidden
>        select FEATURE_A
>
> config DRIVER
>        tristate "This driver is broken if I2C is disabled"
>        select HELPER_B

This case is really what I was referring to, although I was sloppy with
words there. I understand that select does work recursively for selects.

>>   There is no end to this, it just goes on and on, as the
>>   dependencies of the selected symbols change over time. Often the
>>   selects require unintuitive if patterns that are about the
>>   implementation details of the symbol being selected.
>
> Agreed, that is the problem I frequently face with drivers/gpu/drm,
> and most of the time it can only be solved by rewriting the whole
> system to not select user-visible symbol at all.
>
> Using 'depends on' by itself is unfortunately not enough to
> avoid /all/ the problems. See e.g. today's failure
>
> config DRM_DISPLAY_HELPER
>        tristate "DRM Display Helpers"
>        default y
>
> config DRM_DISPLAY_DP_HELPER
>        bool "DRM DisplayPort Helpers"
>        depends on DRM_DISPLAY_HELPER
>
> config DRM_PANEL_LG_SW43408
>        tristate "LG SW43408 panel"
>        depends on DRM_DISPLAY_DP_HELPER
>
> This version is still broken for DRM_DISPLAY_HELPER=m,
> DRM_DISPLAY_DP_HELPER=m, DRM_PANEL_LG_SW43408=y because
> the dependency on the bool symbol is not enough to
> ensure that DRM_DISPLAY_HELPER is also built-in, so you
> still need explicit dependencies on both
> DRM_DISPLAY_HELPER and DRM_DISPLAY_DP_HELPER in the users.
>
> This can be solved by making DRM_DISPLAY_DP_HELPER a
> tristate symbol and adjusting the #ifdef checks and
> Makefile logic accordingly, which is exactly what you'd
> need to do to make it work with 'select' as well.

So bool is kind of problematic for depends on and select even when it's
not really used for describing builtin vs. no, but rather yes vs. no?

>> - Brush the invalid configs under the rug by using IS_REACHABLE(),
>>   switching from a loud link time failure to a silent runtime
>>   failure. (I regularly reject patches adding IS_REACHABLE() to i915,
>>   because from my pov e.g. i915=y backlight=m is an invalid
>>   configuration that the user shouldn't have to debug at runtime. But I
>>   can't express that in kconfig while everyone selects backlight.)
>
> Thanks a lot for rejecting the IS_REACHABLE() patches, it is
> indeed the worst way to handle those (I know, as I introduced
> IS_REACHABLE() only to replace open-coded versions of the same,
> not to have it as a feature or to use it in new code).
>
>> If you have other ideas how these should be fixed, I'm all ears.
>>
>>>    The display helpers however don't have this problem because
>>>    they do not have any dependencies outside of drivers/gpu/
>>
>> Fair enough, though I think they still suffer from some of them having
>> dependencies. (Wasn't this how the original patches and the debate all
>> got started?)
>
> I believe that Maxime said on IRC that he only did the patches
> originally because he expected problems with them based on his
> understanding on how Kconfig works. I'm not aware of any
> particular problem here.
>
> Let me know if you see a problem with any of the symbols that
> Geert has proposed for reverting, and I'll try to find a solution.

I think there will still be some things that select and some other
things that depends on DRM_DISPLAY_HELPER, for example. Usually that's a
recipe for kconfig failures down the line. (Is it ever a good idea?)

For example, after this series, we'll (again) have:

config DRM_DISPLAY_DP_AUX_CEC
	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
	depends on DRM && DRM_DISPLAY_HELPER
	select DRM_DISPLAY_DP_HELPER
	select CEC_CORE

Should we now also drop the help from DRM_DISPLAY_HELPER, and select it
everywhere, including here, and in DRM_DISPLAY_HDCP_HELPER and
DRM_DISPLAY_HDMI_HELPER?

> In my randconfig test environment, I have several patches that
> I sent in the past to clean up the ACPI_VIDEO, I2C, BACKLIGHT and
> LED dependencies to stop using 'select' as I could not otherwise
> get nouveau, i915 and xe to build reliably, but that means I
> may be missing some of the other problems.

Yeah, these seem to be the really problematic ones. I admit I may have
been misguided in insisting the same approach to the helpers that should
be used here; at least the helpers should be easier to fix without
selecting the target symbol dependencies in the source symbols.

BR,
Jani.


-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 16:58         ` Geert Uytterhoeven
@ 2024-04-22 17:14           ` Jani Nikula
  2024-04-22 18:02             ` Geert Uytterhoeven
  2024-04-22 18:23           ` Arnd Bergmann
  1 sibling, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2024-04-22 17:14 UTC (permalink / raw)
  To: Geert Uytterhoeven, Arnd Bergmann
  Cc: Geert Uytterhoeven, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Dave Airlie, Daniel Vetter, dri-devel,
	linux-kernel, Linux-Renesas, Masahiro Yamada, linux-kbuild

On Mon, 22 Apr 2024, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Arnd,
>
> CC kbuild
>
> On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> I'm not sure where this misunderstanding comes from, as you
>> seem to be repeating the same incorrect assumption about
>> how select works that Maxime wrote in his changelog. To clarify,
>> this works exactly as one would expect:
>>
>> config HELPER_A
>>        tristate
>>
>> config HELPER_B
>>        tristate
>>        select HELPER_A
>>
>> config DRIVER
>>        tristate "Turn on the driver and the helpers it uses"
>>        select HELPER_B # this recursively selects HELPER_A
>>
>> Whereas this one is broken:
>>
>> config FEATURE_A
>>        tristate "user visible if I2C is enabled"
>>        depends on I2C
>>
>> config HELPER_B
>>        tristate # hidden
>>        select FEATURE_A
>>
>> config DRIVER
>>        tristate "This driver is broken if I2C is disabled"
>>        select HELPER_B
>
> So the DRIVER section should gain a "depends on I2C" statement.

Why should DRIVER have to care that HELPER_B needs either FEATURE_A or
I2C? It should only have to care about HELPER_B. And if the dependencies
of FEATURE_A or HELPER_B later change, that's their business, not
DRIVER's.


BR,
Jani.

>
> Yamada-san: would it be difficult to modify Kconfig to ignore symbols
> like DRIVER that select other symbols with unmet dependencies?
> Currently it already warns about that.
>
> Handling this implicitly (instead of the current explict "depends
> on") would have the disadvantage though: a user who is not aware of
> the implicit dependency may wonder why DRIVER is invisible in his
> config interface.

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 17:14           ` Jani Nikula
@ 2024-04-22 18:02             ` Geert Uytterhoeven
  0 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 18:02 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Arnd Bergmann, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Dave Airlie, Daniel Vetter, dri-devel,
	linux-kernel, Linux-Renesas, Masahiro Yamada, linux-kbuild

Hi Jani,

On Mon, Apr 22, 2024 at 7:15 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 22 Apr 2024, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >> I'm not sure where this misunderstanding comes from, as you
> >> seem to be repeating the same incorrect assumption about
> >> how select works that Maxime wrote in his changelog. To clarify,
> >> this works exactly as one would expect:
> >>
> >> config HELPER_A
> >>        tristate
> >>
> >> config HELPER_B
> >>        tristate
> >>        select HELPER_A
> >>
> >> config DRIVER
> >>        tristate "Turn on the driver and the helpers it uses"
> >>        select HELPER_B # this recursively selects HELPER_A
> >>
> >> Whereas this one is broken:
> >>
> >> config FEATURE_A
> >>        tristate "user visible if I2C is enabled"
> >>        depends on I2C
> >>
> >> config HELPER_B
> >>        tristate # hidden
> >>        select FEATURE_A
> >>
> >> config DRIVER
> >>        tristate "This driver is broken if I2C is disabled"
> >>        select HELPER_B
> >
> > So the DRIVER section should gain a "depends on I2C" statement.
>
> Why should DRIVER have to care that HELPER_B needs either FEATURE_A or
> I2C? It should only have to care about HELPER_B. And if the dependencies
> of FEATURE_A or HELPER_B later change, that's their business, not
> DRIVER's.

That's correct. But currently the dependency on I2C is not handled
automatically.

> > Yamada-san: would it be difficult to modify Kconfig to ignore symbols
> > like DRIVER that select other symbols with unmet dependencies?
> > Currently it already warns about that.
> >
> > Handling this implicitly (instead of the current explict "depends
> > on") would have the disadvantage though: a user who is not aware of
> > the implicit dependency may wonder why DRIVER is invisible in his
> > config interface.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 17:00         ` Jani Nikula
@ 2024-04-22 18:11           ` Geert Uytterhoeven
  0 siblings, 0 replies; 25+ messages in thread
From: Geert Uytterhoeven @ 2024-04-22 18:11 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Arnd Bergmann, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Dave Airlie, Daniel Vetter, dri-devel,
	linux-kernel, Linux-Renesas, Masahiro Yamada, linux-kbuild

Hi Jani,

CC kbuild

On Mon, Apr 22, 2024 at 7:00 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 22 Apr 2024, "Arnd Bergmann" <arnd@arndb.de> wrote:
> > I'm not sure where this misunderstanding comes from, as you
> > seem to be repeating the same incorrect assumption about
> > how select works that Maxime wrote in his changelog. To clarify,
> > this works exactly as one would expect:
> >
> > config HELPER_A
> >        tristate
> >
> > config HELPER_B
> >        tristate
> >        select HELPER_A
> >
> > config DRIVER
> >        tristate "Turn on the driver and the helpers it uses"
> >        select HELPER_B # this recursively selects HELPER_A
> >
> > Whereas this one is broken:
> >
> > config FEATURE_A
> >        tristate "user visible if I2C is enabled"
> >        depends on I2C
> >
> > config HELPER_B
> >        tristate # hidden
> >        select FEATURE_A
> >
> > config DRIVER
> >        tristate "This driver is broken if I2C is disabled"
> >        select HELPER_B
>
> This case is really what I was referring to, although I was sloppy with
> words there. I understand that select does work recursively for selects.
>
> >>   There is no end to this, it just goes on and on, as the
> >>   dependencies of the selected symbols change over time. Often the
> >>   selects require unintuitive if patterns that are about the
> >>   implementation details of the symbol being selected.
> >
> > Agreed, that is the problem I frequently face with drivers/gpu/drm,
> > and most of the time it can only be solved by rewriting the whole
> > system to not select user-visible symbol at all.
> >
> > Using 'depends on' by itself is unfortunately not enough to
> > avoid /all/ the problems. See e.g. today's failure
> >
> > config DRM_DISPLAY_HELPER
> >        tristate "DRM Display Helpers"
> >        default y
> >
> > config DRM_DISPLAY_DP_HELPER
> >        bool "DRM DisplayPort Helpers"
> >        depends on DRM_DISPLAY_HELPER
> >
> > config DRM_PANEL_LG_SW43408
> >        tristate "LG SW43408 panel"
> >        depends on DRM_DISPLAY_DP_HELPER
> >
> > This version is still broken for DRM_DISPLAY_HELPER=m,
> > DRM_DISPLAY_DP_HELPER=m, DRM_PANEL_LG_SW43408=y because
> > the dependency on the bool symbol is not enough to
> > ensure that DRM_DISPLAY_HELPER is also built-in, so you
> > still need explicit dependencies on both
> > DRM_DISPLAY_HELPER and DRM_DISPLAY_DP_HELPER in the users.
> >
> > This can be solved by making DRM_DISPLAY_DP_HELPER a
> > tristate symbol and adjusting the #ifdef checks and
> > Makefile logic accordingly, which is exactly what you'd
> > need to do to make it work with 'select' as well.
>
> So bool is kind of problematic for depends on and select even when it's
> not really used for describing builtin vs. no, but rather yes vs. no?

Yes, the underlying issue is that bool is used for two different things:
  A. To enable a driver module that can be only built-in,
  B. To enable an option or feature of a driver or subsystem.

Without this distinction, dependencies cannot be auto-propagated 100%
correctly.  Fixing that would require introducing a third type (and possibly
renaming the existing ones to end up with 3 good names).

Actually two types could work:
  1. driver,
  2. option,
as case A is just a driver that can only be built-in (i.e. "depends on y",
which is similar to the behavior with CONFIG_MODULES=n).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 16:58         ` Geert Uytterhoeven
  2024-04-22 17:14           ` Jani Nikula
@ 2024-04-22 18:23           ` Arnd Bergmann
  2024-04-22 19:42             ` Masahiro Yamada
  1 sibling, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2024-04-22 18:23 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jani Nikula, Geert Uytterhoeven, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Dave Airlie, Daniel Vetter,
	dri-devel, linux-kernel, Linux-Renesas, Masahiro Yamada,
	linux-kbuild

On Mon, Apr 22, 2024, at 18:58, Geert Uytterhoeven wrote:
> On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote:
>> Whereas this one is broken:
>>
>> config FEATURE_A
>>        tristate "user visible if I2C is enabled"
>>        depends on I2C
>>
>> config HELPER_B
>>        tristate # hidden
>>        select FEATURE_A
>>
>> config DRIVER
>>        tristate "This driver is broken if I2C is disabled"
>>        select HELPER_B
>
> So the DRIVER section should gain a "depends on I2C" statement.

That is of course the common workaround, but my point was
that nothing should ever 'select I2C' or any of the other
subsystems that are user visible.

> Yamada-san: would it be difficult to modify Kconfig to ignore symbols
> like DRIVER that select other symbols with unmet dependencies?
> Currently it already warns about that.
>
> Handling this implicitly (instead of the current explict "depends
> on") would have the disadvantage though: a user who is not aware of
> the implicit dependency may wonder why DRIVER is invisible in his
> config interface.

I think hiding this would make it much harder to get anything
right. The symbols in question are almost all ones that should
be enabled in normal configs, and the 'make menuconfig' help
doesn't make it too hard to figure things out normally, we just
have to find a way to avoid regressions when converting things
to 'depends on' that used an incorrect 'select'.

     Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 18:23           ` Arnd Bergmann
@ 2024-04-22 19:42             ` Masahiro Yamada
  2024-04-22 20:46               ` Arnd Bergmann
  0 siblings, 1 reply; 25+ messages in thread
From: Masahiro Yamada @ 2024-04-22 19:42 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Geert Uytterhoeven, Jani Nikula, Geert Uytterhoeven,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Dave Airlie,
	Daniel Vetter, dri-devel, linux-kernel, Linux-Renesas,
	linux-kbuild

On Tue, Apr 23, 2024 at 3:24 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Apr 22, 2024, at 18:58, Geert Uytterhoeven wrote:
> > On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >> On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote:
> >> Whereas this one is broken:
> >>
> >> config FEATURE_A
> >>        tristate "user visible if I2C is enabled"
> >>        depends on I2C
> >>
> >> config HELPER_B
> >>        tristate # hidden
> >>        select FEATURE_A
> >>
> >> config DRIVER
> >>        tristate "This driver is broken if I2C is disabled"
> >>        select HELPER_B
> >
> > So the DRIVER section should gain a "depends on I2C" statement.
>
> That is of course the common workaround, but my point was
> that nothing should ever 'select I2C' or any of the other
> subsystems that are user visible.
>
> > Yamada-san: would it be difficult to modify Kconfig to ignore symbols
> > like DRIVER that select other symbols with unmet dependencies?
> > Currently it already warns about that.
> >
> > Handling this implicitly (instead of the current explict "depends
> > on") would have the disadvantage though: a user who is not aware of
> > the implicit dependency may wonder why DRIVER is invisible in his
> > config interface.
>
> I think hiding this would make it much harder to get anything
> right. The symbols in question are almost all ones that should
> be enabled in normal configs, and the 'make menuconfig' help
> doesn't make it too hard to figure things out normally, we just
> have to find a way to avoid regressions when converting things
> to 'depends on' that used an incorrect 'select'.
>
>      Arnd



I am confused because you repeatedly discussed
the missing I2C dependency.


Are you talking about DRM drivers,
or is it just "an example" in general?



DRM selects I2C.

https://github.com/torvalds/linux/blob/v6.9-rc4/drivers/gpu/drm/Kconfig#L16



If you make sure individual DRM drivers depend on DRM,
none of them can be enabled without I2C.



Currently, this is not guaranteed just because
DRM folks do not know how to use the "menuconfig" syntax.



The "menuconfig" makes sense only when it is
followed by "if".




diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 5a0c476361c3..6984b3fea271 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -29,6 +29,8 @@ menuconfig DRM
          details.  You should also select and configure AGP
          (/dev/agpgart) support if it is available for your platform.

+if DRM
+
 config DRM_MIPI_DBI
        tristate
        depends on DRM
@@ -414,3 +416,5 @@ config DRM_LIB_RANDOM
 config DRM_PRIVACY_SCREEN
        bool
        default n
+
+endif










--
Best Regards
Masahiro Yamada

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/11] drm: Restore helper usability
  2024-04-22 19:42             ` Masahiro Yamada
@ 2024-04-22 20:46               ` Arnd Bergmann
  0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2024-04-22 20:46 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Geert Uytterhoeven, Jani Nikula, Geert Uytterhoeven,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Dave Airlie,
	Daniel Vetter, dri-devel, linux-kernel, Linux-Renesas,
	linux-kbuild

On Mon, Apr 22, 2024, at 21:42, Masahiro Yamada wrote:
> On Tue, Apr 23, 2024 at 3:24 AM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, Apr 22, 2024, at 18:58, Geert Uytterhoeven wrote:
>> > On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> >> On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote:
>>
>> I think hiding this would make it much harder to get anything
>> right. The symbols in question are almost all ones that should
>> be enabled in normal configs, and the 'make menuconfig' help
>> doesn't make it too hard to figure things out normally, we just
>> have to find a way to avoid regressions when converting things
>> to 'depends on' that used an incorrect 'select'.
>
> I am confused because you repeatedly discussed
> the missing I2C dependency.
>
>
> Are you talking about DRM drivers,
> or is it just "an example" in general?
>
>
>
> DRM selects I2C.

It's a prominent example because I2C ends up showing
up in most circular dependencies. I forgot that CONFIG_DRM
itself selects this one, but this is clearly part of the
overall problem:

$ git grep -w 'select I2C' | wc -l
35
$ git grep -w 'depends on I2C' | wc -l
1068

Attempting to clean up some of the incorrect 'select'
statements, such as changing DRM_NOUVEAU to 'depends on
ACPI_VIDEO' instead of 'select' tends to run into
another 'select I2C' such as this one:

drivers/i2c/Kconfig:8:	symbol I2C is selected by DRM_NOUVEAU
drivers/gpu/drm/nouveau/Kconfig:2:	symbol DRM_NOUVEAU depends on ACPI_VIDEO
drivers/acpi/Kconfig:214:	symbol ACPI_VIDEO depends on BACKLIGHT_CLASS_DEVICE
drivers/video/backlight/Kconfig:136:	symbol BACKLIGHT_CLASS_DEVICE is selected by FB_BACKLIGHT
drivers/video/fbdev/core/Kconfig:184:	symbol FB_BACKLIGHT is selected by HT16K33
drivers/auxdisplay/Kconfig:490:	symbol HT16K33 depends on I2C

Again, I2C was probably not the best example for an urgent problem
as it ends up being selected unconditionally anyway, but
I think ACPI_VIDEO and BACKLIGHT_CLASS_DEVICE are the ones that
we should stop selecting.

> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 5a0c476361c3..6984b3fea271 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -29,6 +29,8 @@ menuconfig DRM
>           details.  You should also select and configure AGP
>           (/dev/agpgart) support if it is available for your platform.
> 
> +if DRM
> +
>  config DRM_MIPI_DBI
>         tristate
>         depends on DRM
> @@ -414,3 +416,5 @@ config DRM_LIB_RANDOM
>  config DRM_PRIVACY_SCREEN
>         bool
>        default n
> +
> +endif

This is a probably good idea (aside from DRM_PANEL_ORIENTATION_QUIRKS,
which needs to be moved out of the section), but seems completely
unrelated to the issue of selecting too many symbols.

     Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2024-04-22 20:46 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-22 10:30 [PATCH 00/11] drm: Restore helper usability Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 01/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2" Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 02/11] Revert "drm/display: Select DRM_KMS_HELPER for DP helpers" Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 03/11] Revert "drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable" Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 04/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies" Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 05/11] Revert "drm: Switch DRM_DISPLAY_HDMI_HELPER to depends on" Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 06/11] Revert "drm: Switch DRM_DISPLAY_HDCP_HELPER " Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 07/11] Revert "drm: Switch DRM_DISPLAY_DP_HELPER " Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 08/11] Revert "drm: Switch DRM_DISPLAY_DP_AUX_BUS " Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 09/11] Revert "drm: Switch DRM_DISPLAY_HELPER " Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 10/11] Revert "drm: Make drivers depends on DRM_DW_HDMI" Geert Uytterhoeven
2024-04-22 10:30 ` [PATCH 11/11] Revert "drm/display: Make all helpers visible and switch to depends on" Geert Uytterhoeven
2024-04-22 11:50 ` [PATCH 00/11] drm: Restore helper usability Jani Nikula
2024-04-22 12:27   ` Dmitry Baryshkov
2024-04-22 12:30   ` Arnd Bergmann
2024-04-22 13:28     ` Jani Nikula
2024-04-22 13:54       ` Arnd Bergmann
2024-04-22 16:58         ` Geert Uytterhoeven
2024-04-22 17:14           ` Jani Nikula
2024-04-22 18:02             ` Geert Uytterhoeven
2024-04-22 18:23           ` Arnd Bergmann
2024-04-22 19:42             ` Masahiro Yamada
2024-04-22 20:46               ` Arnd Bergmann
2024-04-22 17:00         ` Jani Nikula
2024-04-22 18:11           ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).