[PATCH v2 0/2] drm/nouveau: remove drm_encoder

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface
@ 2024-12-15 10:19 Dmitry Baryshkov
  2024-12-15 10:19 ` [PATCH v2 1/2] drm/nouveau: incorporate I2C TV encoder drivers Dmitry Baryshkov
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2024-12-15 10:19 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich
  Cc: dri-devel, linux-kernel, nouveau, Laurent Pinchart

The nouveau driver is the only user of the drm_encoder_slave interface.
Demote it from KMS helpers module to the nouveau driver itself, moving
corresponding I2C encoders to be handled by nouveau driver too.

Ideally those two drivers should be converted to the drm_bridge
interface, but it's unclear if it's worth spending time on that.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Changes in v2:
- Renamed symbols in defconfig (Laurent)
- Added missing Kbuild file (Laurent, LKP)
- Renamed guarding defines in include files.
- Dropped mentions of two removed functions.
- Link to v1: https://lore.kernel.org/r/20241214-nouveau-encoder-slave-v1-0-beda767472e3@linaro.org

---
Dmitry Baryshkov (2):
      drm/nouveau: incorporate I2C TV encoder drivers
      drm/nouveau: vendor in drm_encoder_slave API

 arch/arm/configs/multi_v7_defconfig                |   4 +-
 arch/parisc/configs/generic-32bit_defconfig        |   4 +-
 arch/parisc/configs/generic-64bit_defconfig        |   4 +-
 drivers/gpu/drm/Makefile                           |   1 -
 drivers/gpu/drm/i2c/Kconfig                        |  18 ----
 drivers/gpu/drm/i2c/Makefile                       |   6 --
 drivers/gpu/drm/nouveau/Kconfig                    |  20 ++++
 drivers/gpu/drm/nouveau/dispnv04/Kbuild            |   3 +
 drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  12 +--
 drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild        |   5 +
 .../drm/{ => nouveau/dispnv04}/i2c/ch7006_drv.c    |  30 +++---
 .../drm/{ => nouveau/dispnv04}/i2c/ch7006_mode.c   |   8 +-
 .../drm/{ => nouveau/dispnv04}/i2c/ch7006_priv.h   |  11 ++-
 .../drm/{ => nouveau/dispnv04}/i2c/sil164_drv.c    |  33 ++++---
 .../dispnv04/nouveau_i2c_encoder.c}                |  85 +++++-----------
 drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  20 ++--
 drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |   4 +-
 .../gpu/drm/nouveau/include}/i2c/ch7006.h          |   4 +-
 .../gpu/drm/nouveau/include/i2c/encoder_i2c.h      | 109 ++++++++-------------
 .../gpu/drm/nouveau/include}/i2c/sil164.h          |   4 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c        |   6 +-
 drivers/gpu/drm/nouveau/nouveau_encoder.h          |  13 +--
 22 files changed, 172 insertions(+), 232 deletions(-)
---
base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f
change-id: 20241214-nouveau-encoder-slave-a6dd422fa4a9

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


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

* [PATCH v2 1/2] drm/nouveau: incorporate I2C TV encoder drivers
  2024-12-15 10:19 [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface Dmitry Baryshkov
@ 2024-12-15 10:19 ` Dmitry Baryshkov
  2024-12-16 13:43   ` Danilo Krummrich
  2024-12-15 10:19 ` [PATCH v2 2/2] drm/nouveau: vendor in drm_encoder_slave API Dmitry Baryshkov
  2024-12-16 11:45 ` [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface Danilo Krummrich
  2 siblings, 1 reply; 12+ messages in thread
From: Dmitry Baryshkov @ 2024-12-15 10:19 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich
  Cc: dri-devel, linux-kernel, nouveau, Laurent Pinchart

Chrontel CH7006 and Silicon Image sil164 drivers use drm_encoder_slave
interface which is being used only by the nouveau driver. It doesn't
make sense to keep this interface inside the DRM subsystem. In
preparation to moving this set of helpers to the nouveau driver, move
the only two I2C driver that use that interface to the nouveau driver
too.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm/configs/multi_v7_defconfig                  |  4 ++--
 arch/parisc/configs/generic-32bit_defconfig          |  4 ++--
 arch/parisc/configs/generic-64bit_defconfig          |  4 ++--
 drivers/gpu/drm/i2c/Kconfig                          | 18 ------------------
 drivers/gpu/drm/i2c/Makefile                         |  6 ------
 drivers/gpu/drm/nouveau/Kconfig                      | 20 ++++++++++++++++++++
 drivers/gpu/drm/nouveau/dispnv04/Kbuild              |  2 ++
 drivers/gpu/drm/nouveau/dispnv04/dfp.c               |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild          |  5 +++++
 .../gpu/drm/{ => nouveau/dispnv04}/i2c/ch7006_drv.c  |  0
 .../gpu/drm/{ => nouveau/dispnv04}/i2c/ch7006_mode.c |  0
 .../gpu/drm/{ => nouveau/dispnv04}/i2c/ch7006_priv.h |  7 ++++---
 .../gpu/drm/{ => nouveau/dispnv04}/i2c/sil164_drv.c  |  3 ++-
 drivers/gpu/drm/nouveau/dispnv04/tvnv04.c            |  2 +-
 .../gpu/drm/nouveau/include}/i2c/ch7006.h            |  4 ++--
 .../gpu/drm/nouveau/include}/i2c/sil164.h            |  4 ++--
 16 files changed, 45 insertions(+), 40 deletions(-)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 9d4336261e450ce5bf99a1aa53e603bd7c0037bb..7fb24a4fb9f697c0c726204d0ba3754e87000e6a 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -713,10 +713,10 @@ CONFIG_VIDEO_ADV7604_CEC=y
 CONFIG_VIDEO_ML86V7667=m
 CONFIG_IMX_IPUV3_CORE=m
 CONFIG_DRM=y
-# CONFIG_DRM_I2C_CH7006 is not set
-# CONFIG_DRM_I2C_SIL164 is not set
 CONFIG_DRM_I2C_NXP_TDA998X=m
 CONFIG_DRM_NOUVEAU=m
+# CONFIG_DRM_NOUVEAU_CH7006 is not set
+# CONFIG_DRM_NOUVEAU_SIL164 is not set
 CONFIG_DRM_EXYNOS=m
 CONFIG_DRM_EXYNOS_FIMD=y
 CONFIG_DRM_EXYNOS_MIXER=y
diff --git a/arch/parisc/configs/generic-32bit_defconfig b/arch/parisc/configs/generic-32bit_defconfig
index 5ce258f3fffaf0e3aac5f8f5450dd65bad305879..f5fffc24c3bc5baf0d77b845e3ac77fae49b276e 100644
--- a/arch/parisc/configs/generic-32bit_defconfig
+++ b/arch/parisc/configs/generic-32bit_defconfig
@@ -132,11 +132,11 @@ CONFIG_I2C=y
 CONFIG_HWMON=m
 CONFIG_DRM=m
 CONFIG_DRM_DISPLAY_DP_AUX_CEC=y
-# CONFIG_DRM_I2C_CH7006 is not set
-# CONFIG_DRM_I2C_SIL164 is not set
 CONFIG_DRM_RADEON=m
 CONFIG_DRM_NOUVEAU=m
 # CONFIG_DRM_NOUVEAU_BACKLIGHT is not set
+# CONFIG_DRM_NOUVEAU_CH7006 is not set
+# CONFIG_DRM_NOUVEAU_SIL164 is not set
 CONFIG_DRM_VGEM=m
 CONFIG_DRM_UDL=m
 CONFIG_DRM_MGAG200=m
diff --git a/arch/parisc/configs/generic-64bit_defconfig b/arch/parisc/configs/generic-64bit_defconfig
index 19a804860ed5b355f82396c2314bd0d8f3fb768a..704f68fbf960fb865206c8f8ab1751c9f0db6de3 100644
--- a/arch/parisc/configs/generic-64bit_defconfig
+++ b/arch/parisc/configs/generic-64bit_defconfig
@@ -193,11 +193,11 @@ CONFIG_MEDIA_SUPPORT=m
 CONFIG_AGP=y
 CONFIG_AGP_PARISC=y
 CONFIG_DRM=y
-# CONFIG_DRM_I2C_CH7006 is not set
-# CONFIG_DRM_I2C_SIL164 is not set
 CONFIG_DRM_RADEON=y
 CONFIG_DRM_NOUVEAU=m
 # CONFIG_DRM_NOUVEAU_BACKLIGHT is not set
+# CONFIG_DRM_NOUVEAU_CH7006 is not set
+# CONFIG_DRM_NOUVEAU_SIL164 is not set
 CONFIG_DRM_MGAG200=m
 CONFIG_FB=y
 CONFIG_FB_PM2=m
diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
index 6f19e1c35e30b0e595c1a60628a6b8cf313fcabc..d5200f67958e68a8ec73401f1d3b79cbe0aa303d 100644
--- a/drivers/gpu/drm/i2c/Kconfig
+++ b/drivers/gpu/drm/i2c/Kconfig
@@ -2,24 +2,6 @@
 menu "I2C encoder or helper chips"
      depends on DRM && DRM_KMS_HELPER && I2C
 
-config DRM_I2C_CH7006
-	tristate "Chrontel ch7006 TV encoder"
-	default m if DRM_NOUVEAU
-	help
-	  Support for Chrontel ch7006 and similar TV encoders, found
-	  on some nVidia video cards.
-
-	  This driver is currently only useful if you're also using
-	  the nouveau driver.
-
-config DRM_I2C_SIL164
-	tristate "Silicon Image sil164 TMDS transmitter"
-	default m if DRM_NOUVEAU
-	help
-	  Support for sil164 and similar single-link (or dual-link
-	  when used in pairs) TMDS transmitters, used in some nVidia
-	  video cards.
-
 config DRM_I2C_NXP_TDA998X
 	tristate "NXP Semiconductors TDA998X HDMI encoder"
 	default m if DRM_TILCDC
diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
index a962f6f085686674ed33010345730db776815ebe..31fd35527d99d7eb23851d290175a3ff0c756772 100644
--- a/drivers/gpu/drm/i2c/Makefile
+++ b/drivers/gpu/drm/i2c/Makefile
@@ -1,10 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
-ch7006-y := ch7006_drv.o ch7006_mode.o
-obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o
-
-sil164-y := sil164_drv.o
-obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o
-
 tda998x-y := tda998x_drv.o
 obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
 obj-$(CONFIG_DRM_I2C_NXP_TDA9950) += tda9950.o
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index ce840300578d8a4011c448b61caf830cef3805bf..4cffac26f90ae6130ef30ba389b2a8c9b732058c 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -109,3 +109,23 @@ config DRM_NOUVEAU_GSP_DEFAULT
 	help
 	  Say Y here if you want to use the GSP codepaths by default on
 	  Turing and Ampere GPUs.
+
+config DRM_NOUVEAU_CH7006
+	tristate "Chrontel ch7006 TV encoder"
+	depends on DRM_NOUVEAU
+	default m
+	help
+	  Support for Chrontel ch7006 and similar TV encoders, found
+	  on some nVidia video cards.
+
+	  This driver is currently only useful if you're also using
+	  the nouveau driver.
+
+config DRM_NOUVEAU_SIL164
+	tristate "Silicon Image sil164 TMDS transmitter"
+	depends on DRM_NOUVEAU
+	default m
+	help
+	  Support for sil164 and similar single-link (or dual-link
+	  when used in pairs) TMDS transmitters, used in some nVidia
+	  video cards.
diff --git a/drivers/gpu/drm/nouveau/dispnv04/Kbuild b/drivers/gpu/drm/nouveau/dispnv04/Kbuild
index 975c4e2269366d57e928eedbbbc669d24744379f..949802882ebd53c15e124c218a092af9693d36bc 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/Kbuild
+++ b/drivers/gpu/drm/nouveau/dispnv04/Kbuild
@@ -10,3 +10,5 @@ nouveau-y += dispnv04/overlay.o
 nouveau-y += dispnv04/tvmodesnv17.o
 nouveau-y += dispnv04/tvnv04.o
 nouveau-y += dispnv04/tvnv17.o
+
+include $(src)/dispnv04/i2c/Kbuild
diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
index 504c421aa176ef3d944592a0109cb72e21fd47b7..28a42ab5cb900ebe8a526e154f9e90598333356c 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
@@ -35,7 +35,7 @@
 #include "hw.h"
 #include "nvreg.h"
 
-#include <drm/i2c/sil164.h>
+#include <i2c/sil164.h>
 
 #include <subdev/i2c.h>
 
diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild b/drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild
new file mode 100644
index 0000000000000000000000000000000000000000..3fddfc97bcb399ef3821c6065e5868363883ac74
--- /dev/null
+++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild
@@ -0,0 +1,5 @@
+ch7006-y := dispnv04/i2c/ch7006_drv.o dispnv04/i2c/ch7006_mode.o
+obj-$(CONFIG_DRM_NOUVEAU_CH7006) += ch7006.o
+
+sil164-y := dispnv04/i2c/sil164_drv.o
+obj-$(CONFIG_DRM_NOUVEAU_SIL164) += sil164.o
diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
similarity index 100%
rename from drivers/gpu/drm/i2c/ch7006_drv.c
rename to drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
diff --git a/drivers/gpu/drm/i2c/ch7006_mode.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
similarity index 100%
rename from drivers/gpu/drm/i2c/ch7006_mode.c
rename to drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
diff --git a/drivers/gpu/drm/i2c/ch7006_priv.h b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
similarity index 99%
rename from drivers/gpu/drm/i2c/ch7006_priv.h
rename to drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
index 052bdc48a339df47073ab305f224f96c8630d66c..8136f8cd8f9b859ccf915e295c783f9fc8321c2e 100644
--- a/drivers/gpu/drm/i2c/ch7006_priv.h
+++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
@@ -24,12 +24,13 @@
  *
  */
 
-#ifndef __DRM_I2C_CH7006_PRIV_H__
-#define __DRM_I2C_CH7006_PRIV_H__
+#ifndef __NOUVEAU_I2C_CH7006_PRIV_H__
+#define __NOUVEAU_I2C_CH7006_PRIV_H__
 
 #include <drm/drm_encoder_slave.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/i2c/ch7006.h>
+
+#include <i2c/ch7006.h>
 
 typedef int64_t fixed;
 #define fixed1 (1LL << 32)
diff --git a/drivers/gpu/drm/i2c/sil164_drv.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
similarity index 99%
rename from drivers/gpu/drm/i2c/sil164_drv.c
rename to drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
index ff23422727fce290a188e495d343e32bc2c373ec..74fc961c0d0de06f1fe8dd93d351452cd20cead7 100644
--- a/drivers/gpu/drm/i2c/sil164_drv.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
@@ -30,7 +30,8 @@
 #include <drm/drm_encoder_slave.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/i2c/sil164.h>
+
+#include <i2c/sil164.h>
 
 struct sil164_priv {
 	struct sil164_encoder_params config;
diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
index d3014027a8122be499b85459b038fdcce5800720..67f3e0ac0e109b223ca8ec4ddc4e688247373b2e 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
@@ -32,7 +32,7 @@
 #include "hw.h"
 #include <drm/drm_modeset_helper_vtables.h>
 
-#include <drm/i2c/ch7006.h>
+#include <i2c/ch7006.h>
 
 static struct nvkm_i2c_bus_probe nv04_tv_encoder_info[] = {
 	{
diff --git a/include/drm/i2c/ch7006.h b/drivers/gpu/drm/nouveau/include/i2c/ch7006.h
similarity index 97%
rename from include/drm/i2c/ch7006.h
rename to drivers/gpu/drm/nouveau/include/i2c/ch7006.h
index 5305b9797f938626c8f6c464ddc9bf424a39b4a0..1a6fa405f85b2a0d0f9a4d1c786defc527fa1d3b 100644
--- a/include/drm/i2c/ch7006.h
+++ b/drivers/gpu/drm/nouveau/include/i2c/ch7006.h
@@ -24,8 +24,8 @@
  *
  */
 
-#ifndef __DRM_I2C_CH7006_H__
-#define __DRM_I2C_CH7006_H__
+#ifndef __NOUVEAU_I2C_CH7006_H__
+#define __NOUVEAU_I2C_CH7006_H__
 
 /**
  * struct ch7006_encoder_params
diff --git a/include/drm/i2c/sil164.h b/drivers/gpu/drm/nouveau/include/i2c/sil164.h
similarity index 96%
rename from include/drm/i2c/sil164.h
rename to drivers/gpu/drm/nouveau/include/i2c/sil164.h
index ddf248693c8be8809777723c272f82af8d334c99..b86750d7abe1c2e7142eac32898398475fd42531 100644
--- a/include/drm/i2c/sil164.h
+++ b/drivers/gpu/drm/nouveau/include/i2c/sil164.h
@@ -24,8 +24,8 @@
  *
  */
 
-#ifndef __DRM_I2C_SIL164_H__
-#define __DRM_I2C_SIL164_H__
+#ifndef __NOUVEAU_I2C_SIL164_H__
+#define __NOUVEAU_I2C_SIL164_H__
 
 /**
  * struct sil164_encoder_params

-- 
2.39.5


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

* Re: [PATCH v2 1/2] drm/nouveau: incorporate I2C TV encoder drivers
  2024-12-15 10:19 ` [PATCH v2 1/2] drm/nouveau: incorporate I2C TV encoder drivers Dmitry Baryshkov
@ 2024-12-16 13:43   ` Danilo Krummrich
  0 siblings, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2024-12-16 13:43 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Karol Herbst, Lyude Paul, dri-devel, linux-kernel,
	nouveau, Laurent Pinchart

On Sun, Dec 15, 2024 at 12:19:23PM +0200, Dmitry Baryshkov wrote:
> Chrontel CH7006 and Silicon Image sil164 drivers use drm_encoder_slave
> interface which is being used only by the nouveau driver. It doesn't
> make sense to keep this interface inside the DRM subsystem. In
> preparation to moving this set of helpers to the nouveau driver, move
> the only two I2C driver that use that interface to the nouveau driver
> too.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm/configs/multi_v7_defconfig                  |  4 ++--
>  arch/parisc/configs/generic-32bit_defconfig          |  4 ++--
>  arch/parisc/configs/generic-64bit_defconfig          |  4 ++--
>  drivers/gpu/drm/i2c/Kconfig                          | 18 ------------------
>  drivers/gpu/drm/i2c/Makefile                         |  6 ------
>  drivers/gpu/drm/nouveau/Kconfig                      | 20 ++++++++++++++++++++
>  drivers/gpu/drm/nouveau/dispnv04/Kbuild              |  2 ++
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c               |  2 +-
>  drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild          |  5 +++++
>  .../gpu/drm/{ => nouveau/dispnv04}/i2c/ch7006_drv.c  |  0
>  .../gpu/drm/{ => nouveau/dispnv04}/i2c/ch7006_mode.c |  0
>  .../gpu/drm/{ => nouveau/dispnv04}/i2c/ch7006_priv.h |  7 ++++---
>  .../gpu/drm/{ => nouveau/dispnv04}/i2c/sil164_drv.c  |  3 ++-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c            |  2 +-
>  .../gpu/drm/nouveau/include}/i2c/ch7006.h            |  4 ++--
>  .../gpu/drm/nouveau/include}/i2c/sil164.h            |  4 ++--

Please move this one (and the one above) to include/dispnv04/i2c/ch7006.h

>  16 files changed, 45 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 9d4336261e450ce5bf99a1aa53e603bd7c0037bb..7fb24a4fb9f697c0c726204d0ba3754e87000e6a 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -713,10 +713,10 @@ CONFIG_VIDEO_ADV7604_CEC=y
>  CONFIG_VIDEO_ML86V7667=m
>  CONFIG_IMX_IPUV3_CORE=m
>  CONFIG_DRM=y
> -# CONFIG_DRM_I2C_CH7006 is not set
> -# CONFIG_DRM_I2C_SIL164 is not set
>  CONFIG_DRM_I2C_NXP_TDA998X=m
>  CONFIG_DRM_NOUVEAU=m
> +# CONFIG_DRM_NOUVEAU_CH7006 is not set
> +# CONFIG_DRM_NOUVEAU_SIL164 is not set
>  CONFIG_DRM_EXYNOS=m
>  CONFIG_DRM_EXYNOS_FIMD=y
>  CONFIG_DRM_EXYNOS_MIXER=y
> diff --git a/arch/parisc/configs/generic-32bit_defconfig b/arch/parisc/configs/generic-32bit_defconfig
> index 5ce258f3fffaf0e3aac5f8f5450dd65bad305879..f5fffc24c3bc5baf0d77b845e3ac77fae49b276e 100644
> --- a/arch/parisc/configs/generic-32bit_defconfig
> +++ b/arch/parisc/configs/generic-32bit_defconfig
> @@ -132,11 +132,11 @@ CONFIG_I2C=y
>  CONFIG_HWMON=m
>  CONFIG_DRM=m
>  CONFIG_DRM_DISPLAY_DP_AUX_CEC=y
> -# CONFIG_DRM_I2C_CH7006 is not set
> -# CONFIG_DRM_I2C_SIL164 is not set
>  CONFIG_DRM_RADEON=m
>  CONFIG_DRM_NOUVEAU=m
>  # CONFIG_DRM_NOUVEAU_BACKLIGHT is not set
> +# CONFIG_DRM_NOUVEAU_CH7006 is not set
> +# CONFIG_DRM_NOUVEAU_SIL164 is not set
>  CONFIG_DRM_VGEM=m
>  CONFIG_DRM_UDL=m
>  CONFIG_DRM_MGAG200=m
> diff --git a/arch/parisc/configs/generic-64bit_defconfig b/arch/parisc/configs/generic-64bit_defconfig
> index 19a804860ed5b355f82396c2314bd0d8f3fb768a..704f68fbf960fb865206c8f8ab1751c9f0db6de3 100644
> --- a/arch/parisc/configs/generic-64bit_defconfig
> +++ b/arch/parisc/configs/generic-64bit_defconfig
> @@ -193,11 +193,11 @@ CONFIG_MEDIA_SUPPORT=m
>  CONFIG_AGP=y
>  CONFIG_AGP_PARISC=y
>  CONFIG_DRM=y
> -# CONFIG_DRM_I2C_CH7006 is not set
> -# CONFIG_DRM_I2C_SIL164 is not set
>  CONFIG_DRM_RADEON=y
>  CONFIG_DRM_NOUVEAU=m
>  # CONFIG_DRM_NOUVEAU_BACKLIGHT is not set
> +# CONFIG_DRM_NOUVEAU_CH7006 is not set
> +# CONFIG_DRM_NOUVEAU_SIL164 is not set
>  CONFIG_DRM_MGAG200=m
>  CONFIG_FB=y
>  CONFIG_FB_PM2=m
> diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
> index 6f19e1c35e30b0e595c1a60628a6b8cf313fcabc..d5200f67958e68a8ec73401f1d3b79cbe0aa303d 100644
> --- a/drivers/gpu/drm/i2c/Kconfig
> +++ b/drivers/gpu/drm/i2c/Kconfig
> @@ -2,24 +2,6 @@
>  menu "I2C encoder or helper chips"
>       depends on DRM && DRM_KMS_HELPER && I2C
>  
> -config DRM_I2C_CH7006
> -	tristate "Chrontel ch7006 TV encoder"
> -	default m if DRM_NOUVEAU
> -	help
> -	  Support for Chrontel ch7006 and similar TV encoders, found
> -	  on some nVidia video cards.
> -
> -	  This driver is currently only useful if you're also using
> -	  the nouveau driver.
> -
> -config DRM_I2C_SIL164
> -	tristate "Silicon Image sil164 TMDS transmitter"
> -	default m if DRM_NOUVEAU
> -	help
> -	  Support for sil164 and similar single-link (or dual-link
> -	  when used in pairs) TMDS transmitters, used in some nVidia
> -	  video cards.
> -
>  config DRM_I2C_NXP_TDA998X
>  	tristate "NXP Semiconductors TDA998X HDMI encoder"
>  	default m if DRM_TILCDC
> diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
> index a962f6f085686674ed33010345730db776815ebe..31fd35527d99d7eb23851d290175a3ff0c756772 100644
> --- a/drivers/gpu/drm/i2c/Makefile
> +++ b/drivers/gpu/drm/i2c/Makefile
> @@ -1,10 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
> -ch7006-y := ch7006_drv.o ch7006_mode.o
> -obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o
> -
> -sil164-y := sil164_drv.o
> -obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o
> -
>  tda998x-y := tda998x_drv.o
>  obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
>  obj-$(CONFIG_DRM_I2C_NXP_TDA9950) += tda9950.o
> diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
> index ce840300578d8a4011c448b61caf830cef3805bf..4cffac26f90ae6130ef30ba389b2a8c9b732058c 100644
> --- a/drivers/gpu/drm/nouveau/Kconfig
> +++ b/drivers/gpu/drm/nouveau/Kconfig
> @@ -109,3 +109,23 @@ config DRM_NOUVEAU_GSP_DEFAULT
>  	help
>  	  Say Y here if you want to use the GSP codepaths by default on
>  	  Turing and Ampere GPUs.
> +
> +config DRM_NOUVEAU_CH7006
> +	tristate "Chrontel ch7006 TV encoder"
> +	depends on DRM_NOUVEAU
> +	default m
> +	help
> +	  Support for Chrontel ch7006 and similar TV encoders, found

This help text should probably end right after "TV encoders".

> +	  on some nVidia video cards.
> +
> +	  This driver is currently only useful if you're also using
> +	  the nouveau driver.
> +
> +config DRM_NOUVEAU_SIL164
> +	tristate "Silicon Image sil164 TMDS transmitter"
> +	depends on DRM_NOUVEAU
> +	default m
> +	help
> +	  Support for sil164 and similar single-link (or dual-link
> +	  when used in pairs) TMDS transmitters, used in some nVidia
> +	  video cards.

I think we can remove everything after "TMDS transmitters".

> diff --git a/drivers/gpu/drm/nouveau/dispnv04/Kbuild b/drivers/gpu/drm/nouveau/dispnv04/Kbuild
> index 975c4e2269366d57e928eedbbbc669d24744379f..949802882ebd53c15e124c218a092af9693d36bc 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/Kbuild
> +++ b/drivers/gpu/drm/nouveau/dispnv04/Kbuild
> @@ -10,3 +10,5 @@ nouveau-y += dispnv04/overlay.o
>  nouveau-y += dispnv04/tvmodesnv17.o
>  nouveau-y += dispnv04/tvnv04.o
>  nouveau-y += dispnv04/tvnv17.o
> +
> +include $(src)/dispnv04/i2c/Kbuild
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> index 504c421aa176ef3d944592a0109cb72e21fd47b7..28a42ab5cb900ebe8a526e154f9e90598333356c 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> @@ -35,7 +35,7 @@
>  #include "hw.h"
>  #include "nvreg.h"
>  
> -#include <drm/i2c/sil164.h>
> +#include <i2c/sil164.h>
>  
>  #include <subdev/i2c.h>
>  
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild b/drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild
> new file mode 100644
> index 0000000000000000000000000000000000000000..3fddfc97bcb399ef3821c6065e5868363883ac74
> --- /dev/null
> +++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild
> @@ -0,0 +1,5 @@
> +ch7006-y := dispnv04/i2c/ch7006_drv.o dispnv04/i2c/ch7006_mode.o
> +obj-$(CONFIG_DRM_NOUVEAU_CH7006) += ch7006.o
> +
> +sil164-y := dispnv04/i2c/sil164_drv.o
> +obj-$(CONFIG_DRM_NOUVEAU_SIL164) += sil164.o
> diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
> similarity index 100%
> rename from drivers/gpu/drm/i2c/ch7006_drv.c
> rename to drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
> diff --git a/drivers/gpu/drm/i2c/ch7006_mode.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
> similarity index 100%
> rename from drivers/gpu/drm/i2c/ch7006_mode.c
> rename to drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
> diff --git a/drivers/gpu/drm/i2c/ch7006_priv.h b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
> similarity index 99%
> rename from drivers/gpu/drm/i2c/ch7006_priv.h
> rename to drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
> index 052bdc48a339df47073ab305f224f96c8630d66c..8136f8cd8f9b859ccf915e295c783f9fc8321c2e 100644
> --- a/drivers/gpu/drm/i2c/ch7006_priv.h
> +++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
> @@ -24,12 +24,13 @@
>   *
>   */
>  
> -#ifndef __DRM_I2C_CH7006_PRIV_H__
> -#define __DRM_I2C_CH7006_PRIV_H__
> +#ifndef __NOUVEAU_I2C_CH7006_PRIV_H__
> +#define __NOUVEAU_I2C_CH7006_PRIV_H__
>  
>  #include <drm/drm_encoder_slave.h>
>  #include <drm/drm_probe_helper.h>
> -#include <drm/i2c/ch7006.h>
> +
> +#include <i2c/ch7006.h>
>  
>  typedef int64_t fixed;
>  #define fixed1 (1LL << 32)
> diff --git a/drivers/gpu/drm/i2c/sil164_drv.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
> similarity index 99%
> rename from drivers/gpu/drm/i2c/sil164_drv.c
> rename to drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
> index ff23422727fce290a188e495d343e32bc2c373ec..74fc961c0d0de06f1fe8dd93d351452cd20cead7 100644
> --- a/drivers/gpu/drm/i2c/sil164_drv.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
> @@ -30,7 +30,8 @@
>  #include <drm/drm_encoder_slave.h>
>  #include <drm/drm_print.h>
>  #include <drm/drm_probe_helper.h>
> -#include <drm/i2c/sil164.h>
> +
> +#include <i2c/sil164.h>
>  
>  struct sil164_priv {
>  	struct sil164_encoder_params config;
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
> index d3014027a8122be499b85459b038fdcce5800720..67f3e0ac0e109b223ca8ec4ddc4e688247373b2e 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
> @@ -32,7 +32,7 @@
>  #include "hw.h"
>  #include <drm/drm_modeset_helper_vtables.h>
>  
> -#include <drm/i2c/ch7006.h>
> +#include <i2c/ch7006.h>
>  
>  static struct nvkm_i2c_bus_probe nv04_tv_encoder_info[] = {
>  	{
> diff --git a/include/drm/i2c/ch7006.h b/drivers/gpu/drm/nouveau/include/i2c/ch7006.h
> similarity index 97%
> rename from include/drm/i2c/ch7006.h
> rename to drivers/gpu/drm/nouveau/include/i2c/ch7006.h
> index 5305b9797f938626c8f6c464ddc9bf424a39b4a0..1a6fa405f85b2a0d0f9a4d1c786defc527fa1d3b 100644
> --- a/include/drm/i2c/ch7006.h
> +++ b/drivers/gpu/drm/nouveau/include/i2c/ch7006.h
> @@ -24,8 +24,8 @@
>   *
>   */
>  
> -#ifndef __DRM_I2C_CH7006_H__
> -#define __DRM_I2C_CH7006_H__
> +#ifndef __NOUVEAU_I2C_CH7006_H__
> +#define __NOUVEAU_I2C_CH7006_H__
>  
>  /**
>   * struct ch7006_encoder_params
> diff --git a/include/drm/i2c/sil164.h b/drivers/gpu/drm/nouveau/include/i2c/sil164.h
> similarity index 96%
> rename from include/drm/i2c/sil164.h
> rename to drivers/gpu/drm/nouveau/include/i2c/sil164.h
> index ddf248693c8be8809777723c272f82af8d334c99..b86750d7abe1c2e7142eac32898398475fd42531 100644
> --- a/include/drm/i2c/sil164.h
> +++ b/drivers/gpu/drm/nouveau/include/i2c/sil164.h
> @@ -24,8 +24,8 @@
>   *
>   */
>  
> -#ifndef __DRM_I2C_SIL164_H__
> -#define __DRM_I2C_SIL164_H__
> +#ifndef __NOUVEAU_I2C_SIL164_H__
> +#define __NOUVEAU_I2C_SIL164_H__
>  
>  /**
>   * struct sil164_encoder_params
> 
> -- 
> 2.39.5
> 

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

* [PATCH v2 2/2] drm/nouveau: vendor in drm_encoder_slave API
  2024-12-15 10:19 [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface Dmitry Baryshkov
  2024-12-15 10:19 ` [PATCH v2 1/2] drm/nouveau: incorporate I2C TV encoder drivers Dmitry Baryshkov
@ 2024-12-15 10:19 ` Dmitry Baryshkov
  2024-12-16 13:50   ` Danilo Krummrich
  2024-12-16 11:45 ` [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface Danilo Krummrich
  2 siblings, 1 reply; 12+ messages in thread
From: Dmitry Baryshkov @ 2024-12-15 10:19 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich
  Cc: dri-devel, linux-kernel, nouveau, Laurent Pinchart

Nouveau driver is the only user of the drm_encoder_slave API. Rework
necessary bits of drm_encoder_slave into the nouveau_i2c_encoder API and
drop drm_encoder_slave.c from the DRM KMS helper.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/Makefile                           |   1 -
 drivers/gpu/drm/nouveau/dispnv04/Kbuild            |   1 +
 drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  10 +-
 drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c  |  30 +++---
 drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c |   8 +-
 drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h |   4 +-
 drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c  |  30 +++---
 .../dispnv04/nouveau_i2c_encoder.c}                |  85 +++++-----------
 drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  18 ++--
 drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |   4 +-
 .../gpu/drm/nouveau/include/i2c/encoder_i2c.h      | 109 ++++++++-------------
 drivers/gpu/drm/nouveau/nouveau_connector.c        |   6 +-
 drivers/gpu/drm/nouveau/nouveau_encoder.h          |  13 +--
 13 files changed, 127 insertions(+), 192 deletions(-)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 19fb370fbc56772077973c864df71e4b8e0bf99b..85af94bb907d6cdbad7078e2667426e479b1069f 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -135,7 +135,6 @@ drm_kms_helper-y := \
 	drm_atomic_state_helper.o \
 	drm_crtc_helper.o \
 	drm_damage_helper.o \
-	drm_encoder_slave.o \
 	drm_flip_work.o \
 	drm_format_helper.o \
 	drm_gem_atomic_helper.o \
diff --git a/drivers/gpu/drm/nouveau/dispnv04/Kbuild b/drivers/gpu/drm/nouveau/dispnv04/Kbuild
index 949802882ebd53c15e124c218a092af9693d36bc..4c7bc6bb81b325ac22286372b0a3e4f1390e78be 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/Kbuild
+++ b/drivers/gpu/drm/nouveau/dispnv04/Kbuild
@@ -6,6 +6,7 @@ nouveau-y += dispnv04/dac.o
 nouveau-y += dispnv04/dfp.o
 nouveau-y += dispnv04/disp.o
 nouveau-y += dispnv04/hw.o
+nouveau-y += dispnv04/nouveau_i2c_encoder.o
 nouveau-y += dispnv04/overlay.o
 nouveau-y += dispnv04/tvmodesnv17.o
 nouveau-y += dispnv04/tvnv04.o
diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
index 28a42ab5cb900ebe8a526e154f9e90598333356c..e211602298a5fb9f513203720b3ee5d37cd21122 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
@@ -171,7 +171,7 @@ static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
 	list_for_each_entry(slave, &dev->mode_config.encoder_list, head) {
 		struct dcb_output *slave_dcb = nouveau_encoder(slave)->dcb;
 
-		if (slave_dcb->type == DCB_OUTPUT_TMDS && get_slave_funcs(slave) &&
+		if (slave_dcb->type == DCB_OUTPUT_TMDS && get_encoder_i2c_funcs(slave) &&
 		    slave_dcb->tmdsconf.slave_addr == dcb->tmdsconf.slave_addr)
 			return slave;
 	}
@@ -473,7 +473,7 @@ static void nv04_dfp_commit(struct drm_encoder *encoder)
 	/* Init external transmitters */
 	slave_encoder = get_tmds_slave(encoder);
 	if (slave_encoder)
-		get_slave_funcs(slave_encoder)->mode_set(
+		get_encoder_i2c_funcs(slave_encoder)->mode_set(
 			slave_encoder, &nv_encoder->mode, &nv_encoder->mode);
 
 	helper->dpms(encoder, DRM_MODE_DPMS_ON);
@@ -614,8 +614,8 @@ static void nv04_dfp_destroy(struct drm_encoder *encoder)
 {
 	struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
 
-	if (get_slave_funcs(encoder))
-		get_slave_funcs(encoder)->destroy(encoder);
+	if (get_encoder_i2c_funcs(encoder))
+		get_encoder_i2c_funcs(encoder)->destroy(encoder);
 
 	drm_encoder_cleanup(encoder);
 	kfree(nv_encoder);
@@ -649,7 +649,7 @@ static void nv04_tmds_slave_init(struct drm_encoder *encoder)
 	if (type < 0)
 		return;
 
-	drm_i2c_encoder_init(dev, to_encoder_slave(encoder),
+	nouveau_i2c_encoder_init(dev, to_encoder_i2c(encoder),
 			     &bus->i2c, &info[type].dev);
 }
 
diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
index 131512a5f3bd996ad1e2eb869ffa09837daba0c7..c9249b58e65459ae61898a246c36da2c3bfe0844 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
@@ -47,14 +47,14 @@ static void ch7006_encoder_destroy(struct drm_encoder *encoder)
 	drm_property_destroy(encoder->dev, priv->scale_property);
 
 	kfree(priv);
-	to_encoder_slave(encoder)->slave_priv = NULL;
+	to_encoder_i2c(encoder)->encoder_i2c_priv = NULL;
 
-	drm_i2c_encoder_destroy(encoder);
+	nouveau_i2c_encoder_destroy(encoder);
 }
 
 static void  ch7006_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 	struct ch7006_state *state = &priv->state;
 
@@ -71,7 +71,7 @@ static void  ch7006_encoder_dpms(struct drm_encoder *encoder, int mode)
 
 static void ch7006_encoder_save(struct drm_encoder *encoder)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 
 	ch7006_dbg(client, "\n");
@@ -81,7 +81,7 @@ static void ch7006_encoder_save(struct drm_encoder *encoder)
 
 static void ch7006_encoder_restore(struct drm_encoder *encoder)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 
 	ch7006_dbg(client, "\n");
@@ -116,7 +116,7 @@ static void ch7006_encoder_mode_set(struct drm_encoder *encoder,
 				     struct drm_display_mode *drm_mode,
 				     struct drm_display_mode *adjusted_mode)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 	struct ch7006_encoder_params *params = &priv->params;
 	struct ch7006_state *state = &priv->state;
@@ -179,7 +179,7 @@ static void ch7006_encoder_mode_set(struct drm_encoder *encoder,
 static enum drm_connector_status ch7006_encoder_detect(struct drm_encoder *encoder,
 						       struct drm_connector *connector)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 	struct ch7006_state *state = &priv->state;
 	int det;
@@ -285,7 +285,7 @@ static int ch7006_encoder_set_property(struct drm_encoder *encoder,
 				       struct drm_property *property,
 				       uint64_t val)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 	struct ch7006_state *state = &priv->state;
 	struct drm_mode_config *conf = &encoder->dev->mode_config;
@@ -370,7 +370,7 @@ static int ch7006_encoder_set_property(struct drm_encoder *encoder,
 	return 0;
 }
 
-static const struct drm_encoder_slave_funcs ch7006_encoder_funcs = {
+static const struct nouveau_i2c_encoder_funcs ch7006_encoder_funcs = {
 	.set_config = ch7006_encoder_set_config,
 	.destroy = ch7006_encoder_destroy,
 	.dpms = ch7006_encoder_dpms,
@@ -437,7 +437,7 @@ static int ch7006_resume(struct device *dev)
 
 static int ch7006_encoder_init(struct i2c_client *client,
 			       struct drm_device *dev,
-			       struct drm_encoder_slave *encoder)
+			       struct nouveau_i2c_encoder *encoder)
 {
 	struct ch7006_priv *priv;
 	int i;
@@ -448,8 +448,8 @@ static int ch7006_encoder_init(struct i2c_client *client,
 	if (!priv)
 		return -ENOMEM;
 
-	encoder->slave_priv = priv;
-	encoder->slave_funcs = &ch7006_encoder_funcs;
+	encoder->encoder_i2c_priv = priv;
+	encoder->encoder_i2c_funcs = &ch7006_encoder_funcs;
 
 	priv->norm = TV_NORM_PAL;
 	priv->select_subconnector = DRM_MODE_SUBCONNECTOR_Automatic;
@@ -495,7 +495,7 @@ static const struct dev_pm_ops ch7006_pm_ops = {
 	.resume = ch7006_resume,
 };
 
-static struct drm_i2c_encoder_driver ch7006_driver = {
+static struct nouveau_i2c_encoder_driver ch7006_driver = {
 	.i2c_driver = {
 		.probe = ch7006_probe,
 		.remove = ch7006_remove,
@@ -516,12 +516,12 @@ static struct drm_i2c_encoder_driver ch7006_driver = {
 
 static int __init ch7006_init(void)
 {
-	return drm_i2c_encoder_register(THIS_MODULE, &ch7006_driver);
+	return i2c_add_driver(&ch7006_driver.i2c_driver);
 }
 
 static void __exit ch7006_exit(void)
 {
-	drm_i2c_encoder_unregister(&ch7006_driver);
+	i2c_del_driver(&ch7006_driver.i2c_driver);
 }
 
 int ch7006_debug;
diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
index 6afe6d0ee6306db57c3e3bafe2bf1b0b1b49aea5..e58d94451959a2afc01f570ea620d8e6721cb7af 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
@@ -198,7 +198,7 @@ const struct ch7006_mode *ch7006_lookup_mode(struct drm_encoder *encoder,
 
 void ch7006_setup_levels(struct drm_encoder *encoder)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 	uint8_t *regs = priv->state.regs;
 	const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
@@ -229,7 +229,7 @@ void ch7006_setup_levels(struct drm_encoder *encoder)
 
 void ch7006_setup_subcarrier(struct drm_encoder *encoder)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 	struct ch7006_state *state = &priv->state;
 	const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
@@ -253,7 +253,7 @@ void ch7006_setup_subcarrier(struct drm_encoder *encoder)
 
 void ch7006_setup_pll(struct drm_encoder *encoder)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 	uint8_t *regs = priv->state.regs;
 	const struct ch7006_mode *mode = priv->mode;
@@ -324,7 +324,7 @@ void ch7006_setup_power_state(struct drm_encoder *encoder)
 
 void ch7006_setup_properties(struct drm_encoder *encoder)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 	struct ch7006_priv *priv = to_ch7006_priv(encoder);
 	struct ch7006_state *state = &priv->state;
 	const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
index 8136f8cd8f9b859ccf915e295c783f9fc8321c2e..70c4fc8a181035b8960a8fcce12b5a1ad76e58b7 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
+++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
@@ -27,9 +27,9 @@
 #ifndef __NOUVEAU_I2C_CH7006_PRIV_H__
 #define __NOUVEAU_I2C_CH7006_PRIV_H__
 
-#include <drm/drm_encoder_slave.h>
 #include <drm/drm_probe_helper.h>
 
+#include <i2c/encoder_i2c.h>
 #include <i2c/ch7006.h>
 
 typedef int64_t fixed;
@@ -100,7 +100,7 @@ struct ch7006_priv {
 };
 
 #define to_ch7006_priv(x) \
-	((struct ch7006_priv *)to_encoder_slave(x)->slave_priv)
+	((struct ch7006_priv *)to_encoder_i2c(x)->encoder_i2c_priv)
 
 extern int ch7006_debug;
 extern char *ch7006_tv_norm;
diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
index 74fc961c0d0de06f1fe8dd93d351452cd20cead7..80758e796b5d794f8e05c1b68b3383e1af3d0df5 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
@@ -27,10 +27,10 @@
 #include <linux/module.h>
 
 #include <drm/drm_drv.h>
-#include <drm/drm_encoder_slave.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 
+#include <i2c/encoder_i2c.h>
 #include <i2c/sil164.h>
 
 struct sil164_priv {
@@ -42,7 +42,7 @@ struct sil164_priv {
 };
 
 #define to_sil164_priv(x) \
-	((struct sil164_priv *)to_encoder_slave(x)->slave_priv)
+	((struct sil164_priv *)to_encoder_i2c(x)->encoder_i2c_priv)
 
 #define sil164_dbg(client, format, ...) do {				\
 		if (drm_debug_enabled(DRM_UT_KMS))			\
@@ -222,7 +222,7 @@ sil164_encoder_dpms(struct drm_encoder *encoder, int mode)
 	bool on = (mode == DRM_MODE_DPMS_ON);
 	bool duallink = (on && encoder->crtc->mode.clock > 165000);
 
-	sil164_set_power_state(drm_i2c_encoder_get_client(encoder), on);
+	sil164_set_power_state(nouveau_i2c_encoder_get_client(encoder), on);
 
 	if (priv->duallink_slave)
 		sil164_set_power_state(priv->duallink_slave, duallink);
@@ -233,7 +233,7 @@ sil164_encoder_save(struct drm_encoder *encoder)
 {
 	struct sil164_priv *priv = to_sil164_priv(encoder);
 
-	sil164_save_state(drm_i2c_encoder_get_client(encoder),
+	sil164_save_state(nouveau_i2c_encoder_get_client(encoder),
 			  priv->saved_state);
 
 	if (priv->duallink_slave)
@@ -246,7 +246,7 @@ sil164_encoder_restore(struct drm_encoder *encoder)
 {
 	struct sil164_priv *priv = to_sil164_priv(encoder);
 
-	sil164_restore_state(drm_i2c_encoder_get_client(encoder),
+	sil164_restore_state(nouveau_i2c_encoder_get_client(encoder),
 			     priv->saved_state);
 
 	if (priv->duallink_slave)
@@ -278,7 +278,7 @@ sil164_encoder_mode_set(struct drm_encoder *encoder,
 	struct sil164_priv *priv = to_sil164_priv(encoder);
 	bool duallink = adjusted_mode->clock > 165000;
 
-	sil164_init_state(drm_i2c_encoder_get_client(encoder),
+	sil164_init_state(nouveau_i2c_encoder_get_client(encoder),
 			  &priv->config, duallink);
 
 	if (priv->duallink_slave)
@@ -292,7 +292,7 @@ static enum drm_connector_status
 sil164_encoder_detect(struct drm_encoder *encoder,
 		      struct drm_connector *connector)
 {
-	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
 
 	if (sil164_read(client, SIL164_DETECT) & SIL164_DETECT_HOTPLUG_STAT)
 		return connector_status_connected;
@@ -331,10 +331,10 @@ sil164_encoder_destroy(struct drm_encoder *encoder)
 	i2c_unregister_device(priv->duallink_slave);
 
 	kfree(priv);
-	drm_i2c_encoder_destroy(encoder);
+	nouveau_i2c_encoder_destroy(encoder);
 }
 
-static const struct drm_encoder_slave_funcs sil164_encoder_funcs = {
+static const struct nouveau_i2c_encoder_funcs sil164_encoder_funcs = {
 	.set_config = sil164_encoder_set_config,
 	.destroy = sil164_encoder_destroy,
 	.dpms = sil164_encoder_dpms,
@@ -394,7 +394,7 @@ sil164_detect_slave(struct i2c_client *client)
 static int
 sil164_encoder_init(struct i2c_client *client,
 		    struct drm_device *dev,
-		    struct drm_encoder_slave *encoder)
+		    struct nouveau_i2c_encoder *encoder)
 {
 	struct sil164_priv *priv;
 	struct i2c_client *slave_client;
@@ -403,8 +403,8 @@ sil164_encoder_init(struct i2c_client *client,
 	if (!priv)
 		return -ENOMEM;
 
-	encoder->slave_priv = priv;
-	encoder->slave_funcs = &sil164_encoder_funcs;
+	encoder->encoder_i2c_priv = priv;
+	encoder->encoder_i2c_funcs = &sil164_encoder_funcs;
 
 	slave_client = sil164_detect_slave(client);
 	if (!IS_ERR(slave_client))
@@ -419,7 +419,7 @@ static const struct i2c_device_id sil164_ids[] = {
 };
 MODULE_DEVICE_TABLE(i2c, sil164_ids);
 
-static struct drm_i2c_encoder_driver sil164_driver = {
+static struct nouveau_i2c_encoder_driver sil164_driver = {
 	.i2c_driver = {
 		.probe = sil164_probe,
 		.driver = {
@@ -435,13 +435,13 @@ static struct drm_i2c_encoder_driver sil164_driver = {
 static int __init
 sil164_init(void)
 {
-	return drm_i2c_encoder_register(THIS_MODULE, &sil164_driver);
+	return i2c_add_driver(&sil164_driver.i2c_driver);
 }
 
 static void __exit
 sil164_exit(void)
 {
-	drm_i2c_encoder_unregister(&sil164_driver);
+	i2c_del_driver(&sil164_driver.i2c_driver);
 }
 
 MODULE_AUTHOR("Francisco Jerez <currojerez@riseup.net>");
diff --git a/drivers/gpu/drm/drm_encoder_slave.c b/drivers/gpu/drm/nouveau/dispnv04/nouveau_i2c_encoder.c
similarity index 57%
rename from drivers/gpu/drm/drm_encoder_slave.c
rename to drivers/gpu/drm/nouveau/dispnv04/nouveau_i2c_encoder.c
index e464429d32df6e2eeb541d92c5a486b4ee7a21ec..58270719cc3db6a2a0205196bf18aba139da2df4 100644
--- a/drivers/gpu/drm/drm_encoder_slave.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/nouveau_i2c_encoder.c
@@ -26,10 +26,10 @@
 
 #include <linux/module.h>
 
-#include <drm/drm_encoder_slave.h>
+#include <i2c/encoder_i2c.h>
 
 /**
- * drm_i2c_encoder_init - Initialize an I2C slave encoder
+ * nouveau_i2c_encoder_init - Initialize an I2C slave encoder
  * @dev:	DRM device.
  * @encoder:    Encoder to be attached to the I2C device. You aren't
  *		required to have called drm_encoder_init() before.
@@ -40,7 +40,7 @@
  *
  * Create an I2C device on the specified bus (the module containing its
  * driver is transparently loaded) and attach it to the specified
- * &drm_encoder_slave. The @slave_funcs field will be initialized with
+ * &nouveau_i2c_encoder. The @encoder_i2c_funcs field will be initialized with
  * the hooks provided by the slave driver.
  *
  * If @info.platform_data is non-NULL it will be used as the initial
@@ -49,14 +49,14 @@
  * Returns 0 on success or a negative errno on failure, in particular,
  * -ENODEV is returned when no matching driver is found.
  */
-int drm_i2c_encoder_init(struct drm_device *dev,
-			 struct drm_encoder_slave *encoder,
+int nouveau_i2c_encoder_init(struct drm_device *dev,
+			 struct nouveau_i2c_encoder *encoder,
 			 struct i2c_adapter *adap,
 			 const struct i2c_board_info *info)
 {
 	struct module *module = NULL;
 	struct i2c_client *client;
-	struct drm_i2c_encoder_driver *encoder_drv;
+	struct nouveau_i2c_encoder_driver *encoder_drv;
 	int err = 0;
 
 	request_module("%s%s", I2C_MODULE_PREFIX, info->type);
@@ -73,16 +73,16 @@ int drm_i2c_encoder_init(struct drm_device *dev,
 		goto fail_unregister;
 	}
 
-	encoder->bus_priv = client;
+	encoder->i2c_client = client;
 
-	encoder_drv = to_drm_i2c_encoder_driver(to_i2c_driver(client->dev.driver));
+	encoder_drv = to_nouveau_i2c_encoder_driver(to_i2c_driver(client->dev.driver));
 
 	err = encoder_drv->encoder_init(client, dev, encoder);
 	if (err)
 		goto fail_module_put;
 
 	if (info->platform_data)
-		encoder->slave_funcs->set_config(&encoder->base,
+		encoder->encoder_i2c_funcs->set_config(&encoder->base,
 						 info->platform_data);
 
 	return 0;
@@ -93,90 +93,53 @@ int drm_i2c_encoder_init(struct drm_device *dev,
 	i2c_unregister_device(client);
 	return err;
 }
-EXPORT_SYMBOL(drm_i2c_encoder_init);
 
 /**
- * drm_i2c_encoder_destroy - Unregister the I2C device backing an encoder
+ * nouveau_i2c_encoder_destroy - Unregister the I2C device backing an encoder
  * @drm_encoder:	Encoder to be unregistered.
  *
  * This should be called from the @destroy method of an I2C slave
  * encoder driver once I2C access is no longer needed.
  */
-void drm_i2c_encoder_destroy(struct drm_encoder *drm_encoder)
+void nouveau_i2c_encoder_destroy(struct drm_encoder *drm_encoder)
 {
-	struct drm_encoder_slave *encoder = to_encoder_slave(drm_encoder);
-	struct i2c_client *client = drm_i2c_encoder_get_client(drm_encoder);
+	struct nouveau_i2c_encoder *encoder = to_encoder_i2c(drm_encoder);
+	struct i2c_client *client = nouveau_i2c_encoder_get_client(drm_encoder);
 	struct module *module = client->dev.driver->owner;
 
 	i2c_unregister_device(client);
-	encoder->bus_priv = NULL;
+	encoder->i2c_client = NULL;
 
 	module_put(module);
 }
-EXPORT_SYMBOL(drm_i2c_encoder_destroy);
+EXPORT_SYMBOL(nouveau_i2c_encoder_destroy);
 
 /*
  * Wrapper fxns which can be plugged in to drm_encoder_helper_funcs:
  */
 
-static inline const struct drm_encoder_slave_funcs *
-get_slave_funcs(struct drm_encoder *enc)
-{
-	return to_encoder_slave(enc)->slave_funcs;
-}
-
-void drm_i2c_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-	get_slave_funcs(encoder)->dpms(encoder, mode);
-}
-EXPORT_SYMBOL(drm_i2c_encoder_dpms);
-
-bool drm_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
+bool nouveau_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
 		const struct drm_display_mode *mode,
 		struct drm_display_mode *adjusted_mode)
 {
-	if (!get_slave_funcs(encoder)->mode_fixup)
+	if (!get_encoder_i2c_funcs(encoder)->mode_fixup)
 		return true;
 
-	return get_slave_funcs(encoder)->mode_fixup(encoder, mode, adjusted_mode);
-}
-EXPORT_SYMBOL(drm_i2c_encoder_mode_fixup);
-
-void drm_i2c_encoder_prepare(struct drm_encoder *encoder)
-{
-	drm_i2c_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
-}
-EXPORT_SYMBOL(drm_i2c_encoder_prepare);
-
-void drm_i2c_encoder_commit(struct drm_encoder *encoder)
-{
-	drm_i2c_encoder_dpms(encoder, DRM_MODE_DPMS_ON);
-}
-EXPORT_SYMBOL(drm_i2c_encoder_commit);
-
-void drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
-		struct drm_display_mode *mode,
-		struct drm_display_mode *adjusted_mode)
-{
-	get_slave_funcs(encoder)->mode_set(encoder, mode, adjusted_mode);
+	return get_encoder_i2c_funcs(encoder)->mode_fixup(encoder, mode, adjusted_mode);
 }
-EXPORT_SYMBOL(drm_i2c_encoder_mode_set);
 
-enum drm_connector_status drm_i2c_encoder_detect(struct drm_encoder *encoder,
+enum drm_connector_status nouveau_i2c_encoder_detect(struct drm_encoder *encoder,
 	    struct drm_connector *connector)
 {
-	return get_slave_funcs(encoder)->detect(encoder, connector);
+	return get_encoder_i2c_funcs(encoder)->detect(encoder, connector);
 }
-EXPORT_SYMBOL(drm_i2c_encoder_detect);
 
-void drm_i2c_encoder_save(struct drm_encoder *encoder)
+void nouveau_i2c_encoder_save(struct drm_encoder *encoder)
 {
-	get_slave_funcs(encoder)->save(encoder);
+	get_encoder_i2c_funcs(encoder)->save(encoder);
 }
-EXPORT_SYMBOL(drm_i2c_encoder_save);
 
-void drm_i2c_encoder_restore(struct drm_encoder *encoder)
+void nouveau_i2c_encoder_restore(struct drm_encoder *encoder)
 {
-	get_slave_funcs(encoder)->restore(encoder);
+	get_encoder_i2c_funcs(encoder)->restore(encoder);
 }
-EXPORT_SYMBOL(drm_i2c_encoder_restore);
diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
index 67f3e0ac0e109b223ca8ec4ddc4e688247373b2e..0ce369d7b773ac3753fe907acdee3b402644cc0e 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
@@ -99,7 +99,7 @@ static void nv04_tv_dpms(struct drm_encoder *encoder, int mode)
 
 	NVWriteRAMDAC(dev, 0, NV_PRAMDAC_PLL_COEFF_SELECT, state->pllsel);
 
-	get_slave_funcs(encoder)->dpms(encoder, mode);
+	get_encoder_i2c_funcs(encoder)->dpms(encoder, mode);
 }
 
 static void nv04_tv_bind(struct drm_device *dev, int head, bool bind)
@@ -158,7 +158,7 @@ static void nv04_tv_mode_set(struct drm_encoder *encoder,
 	regp->tv_vskew = 1;
 	regp->tv_vsync_delay = 1;
 
-	get_slave_funcs(encoder)->mode_set(encoder, mode, adjusted_mode);
+	get_encoder_i2c_funcs(encoder)->mode_set(encoder, mode, adjusted_mode);
 }
 
 static void nv04_tv_commit(struct drm_encoder *encoder)
@@ -178,7 +178,7 @@ static void nv04_tv_commit(struct drm_encoder *encoder)
 
 static void nv04_tv_destroy(struct drm_encoder *encoder)
 {
-	get_slave_funcs(encoder)->destroy(encoder);
+	get_encoder_i2c_funcs(encoder)->destroy(encoder);
 	drm_encoder_cleanup(encoder);
 
 	kfree(encoder->helper_private);
@@ -191,11 +191,11 @@ static const struct drm_encoder_funcs nv04_tv_funcs = {
 
 static const struct drm_encoder_helper_funcs nv04_tv_helper_funcs = {
 	.dpms = nv04_tv_dpms,
-	.mode_fixup = drm_i2c_encoder_mode_fixup,
+	.mode_fixup = nouveau_i2c_encoder_mode_fixup,
 	.prepare = nv04_tv_prepare,
 	.commit = nv04_tv_commit,
 	.mode_set = nv04_tv_mode_set,
-	.detect = drm_i2c_encoder_detect,
+	.detect = nouveau_i2c_encoder_detect,
 };
 
 int
@@ -226,8 +226,8 @@ nv04_tv_create(struct drm_connector *connector, struct dcb_output *entry)
 			 NULL);
 	drm_encoder_helper_add(encoder, &nv04_tv_helper_funcs);
 
-	nv_encoder->enc_save = drm_i2c_encoder_save;
-	nv_encoder->enc_restore = drm_i2c_encoder_restore;
+	nv_encoder->enc_save = nouveau_i2c_encoder_save;
+	nv_encoder->enc_restore = nouveau_i2c_encoder_restore;
 
 	encoder->possible_crtcs = entry->heads;
 	encoder->possible_clones = 0;
@@ -235,14 +235,14 @@ nv04_tv_create(struct drm_connector *connector, struct dcb_output *entry)
 	nv_encoder->or = ffs(entry->or) - 1;
 
 	/* Run the slave-specific initialization */
-	ret = drm_i2c_encoder_init(dev, to_encoder_slave(encoder),
+	ret = nouveau_i2c_encoder_init(dev, to_encoder_i2c(encoder),
 				   &bus->i2c,
 				   &nv04_tv_encoder_info[type].dev);
 	if (ret < 0)
 		goto fail_cleanup;
 
 	/* Attach it to the specified connector. */
-	get_slave_funcs(encoder)->create_resources(encoder, connector);
+	get_encoder_i2c_funcs(encoder)->create_resources(encoder, connector);
 	drm_connector_attach_encoder(connector, encoder);
 
 	return 0;
diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
index 3ecb101d23e949b753b873d24eec01ad6fe7f5d6..50326691444322f282729e108b4ab88bd9cbc14a 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
@@ -779,7 +779,7 @@ static const struct drm_encoder_helper_funcs nv17_tv_helper_funcs = {
 	.detect = nv17_tv_detect,
 };
 
-static const struct drm_encoder_slave_funcs nv17_tv_slave_funcs = {
+static const struct nouveau_i2c_encoder_funcs nv17_tv_encoder_i2c_funcs = {
 	.get_modes = nv17_tv_get_modes,
 	.mode_valid = nv17_tv_mode_valid,
 	.create_resources = nv17_tv_create_resources,
@@ -818,7 +818,7 @@ nv17_tv_create(struct drm_connector *connector, struct dcb_output *entry)
 	drm_encoder_init(dev, encoder, &nv17_tv_funcs, DRM_MODE_ENCODER_TVDAC,
 			 NULL);
 	drm_encoder_helper_add(encoder, &nv17_tv_helper_funcs);
-	to_encoder_slave(encoder)->slave_funcs = &nv17_tv_slave_funcs;
+	to_encoder_i2c(encoder)->encoder_i2c_funcs = &nv17_tv_encoder_i2c_funcs;
 
 	tv_enc->base.enc_save = nv17_tv_save;
 	tv_enc->base.enc_restore = nv17_tv_restore;
diff --git a/include/drm/drm_encoder_slave.h b/drivers/gpu/drm/nouveau/include/i2c/encoder_i2c.h
similarity index 62%
rename from include/drm/drm_encoder_slave.h
rename to drivers/gpu/drm/nouveau/include/i2c/encoder_i2c.h
index 49172166a164474f43e4afb2eeeb3cde8ae7c61a..f12f44415b79c336e6ecfa163cc780a0bcbc5ca5 100644
--- a/include/drm/drm_encoder_slave.h
+++ b/drivers/gpu/drm/nouveau/include/i2c/encoder_i2c.h
@@ -24,8 +24,8 @@
  *
  */
 
-#ifndef __DRM_ENCODER_SLAVE_H__
-#define __DRM_ENCODER_SLAVE_H__
+#ifndef __NOUVEAU_ENCODER_I2C_H__
+#define __NOUVEAU_ENCODER_I2C_H__
 
 #include <linux/i2c.h>
 
@@ -33,7 +33,7 @@
 #include <drm/drm_encoder.h>
 
 /**
- * struct drm_encoder_slave_funcs - Entry points exposed by a slave encoder driver
+ * struct nouveau_i2c_encoder_funcs - Entry points exposed by a I2C encoder driver
  *
  * Most of its members are analogous to the function pointers in
  * &drm_encoder_helper_funcs and they can optionally be used to
@@ -41,7 +41,7 @@
  * @set_property) will typically be wrapped around and only be called
  * if the encoder is the currently selected one for the connector.
  */
-struct drm_encoder_slave_funcs {
+struct nouveau_i2c_encoder_funcs {
 	/**
 	 * @set_config: Initialize any encoder-specific modesetting parameters.
 	 * The meaning of the @params parameter is implementation dependent. It
@@ -58,24 +58,23 @@ struct drm_encoder_slave_funcs {
 	void (*destroy)(struct drm_encoder *encoder);
 
 	/**
-	 * @dpms: Analogous to &drm_encoder_helper_funcs @dpms callback. Wrapped
-	 * by drm_i2c_encoder_dpms().
+	 * @dpms: Analogous to &drm_encoder_helper_funcs @dpms callback.
 	 */
 	void (*dpms)(struct drm_encoder *encoder, int mode);
 
 	/**
-	 * @save: Save state. Wrapped by drm_i2c_encoder_save().
+	 * @save: Save state. Wrapped by nouveau_i2c_encoder_save().
 	 */
 	void (*save)(struct drm_encoder *encoder);
 
 	/**
-	 * @restore: Restore state. Wrapped by drm_i2c_encoder_restore().
+	 * @restore: Restore state. Wrapped by nouveau_i2c_encoder_restore().
 	 */
 	void (*restore)(struct drm_encoder *encoder);
 
 	/**
 	 * @mode_fixup: Analogous to &drm_encoder_helper_funcs @mode_fixup
-	 * callback. Wrapped by drm_i2c_encoder_mode_fixup().
+	 * callback. Wrapped by nouveau_i2c_encoder_mode_fixup().
 	 */
 	bool (*mode_fixup)(struct drm_encoder *encoder,
 			   const struct drm_display_mode *mode,
@@ -88,7 +87,7 @@ struct drm_encoder_slave_funcs {
 			  struct drm_display_mode *mode);
 	/**
 	 * @mode_set: Analogous to &drm_encoder_helper_funcs @mode_set
-	 * callback. Wrapped by drm_i2c_encoder_mode_set().
+	 * callback.
 	 */
 	void (*mode_set)(struct drm_encoder *encoder,
 			 struct drm_display_mode *mode,
@@ -96,7 +95,7 @@ struct drm_encoder_slave_funcs {
 
 	/**
 	 * @detect: Analogous to &drm_encoder_helper_funcs @detect
-	 * callback. Wrapped by drm_i2c_encoder_detect().
+	 * callback. Wrapped by nouveau_i2c_encoder_detect().
 	 */
 	enum drm_connector_status (*detect)(struct drm_encoder *encoder,
 					    struct drm_connector *connector);
@@ -120,54 +119,59 @@ struct drm_encoder_slave_funcs {
 };
 
 /**
- * struct drm_encoder_slave - Slave encoder struct
+ * struct nouveau_i2c_encoder - I2C encoder struct
  *
- * A &drm_encoder_slave has two sets of callbacks, @slave_funcs and the
+ * A &nouveau_i2c_encoder has two sets of callbacks, @encoder_i2c_funcs and the
  * ones in @base. The former are never actually called by the common
  * CRTC code, it's just a convenience for splitting the encoder
  * functions in an upper, GPU-specific layer and a (hopefully)
  * GPU-agnostic lower layer: It's the GPU driver responsibility to
- * call the slave methods when appropriate.
+ * call the nouveau_i2c_encoder methods when appropriate.
  *
- * drm_i2c_encoder_init() provides a way to get an implementation of
+ * nouveau_i2c_encoder_init() provides a way to get an implementation of
  * this.
  */
-struct drm_encoder_slave {
+struct nouveau_i2c_encoder {
 	/**
 	 * @base: DRM encoder object.
 	 */
 	struct drm_encoder base;
 
 	/**
-	 * @slave_funcs: Slave encoder callbacks.
+	 * @encoder_i2c_funcs: I2C encoder callbacks.
 	 */
-	const struct drm_encoder_slave_funcs *slave_funcs;
+	const struct nouveau_i2c_encoder_funcs *encoder_i2c_funcs;
 
 	/**
-	 * @slave_priv: Slave encoder private data.
+	 * @encoder_i2c_priv: I2C encoder private data.
 	 */
-	void *slave_priv;
+	void *encoder_i2c_priv;
 
 	/**
-	 * @bus_priv: Bus specific data.
+	 * @i2c_client: corresponding I2C client structure
 	 */
-	void *bus_priv;
+	struct i2c_client *i2c_client;
 };
-#define to_encoder_slave(x) container_of((x), struct drm_encoder_slave, base)
+#define to_encoder_i2c(x) container_of((x), struct nouveau_i2c_encoder, base)
 
-int drm_i2c_encoder_init(struct drm_device *dev,
-			 struct drm_encoder_slave *encoder,
+int nouveau_i2c_encoder_init(struct drm_device *dev,
+			 struct nouveau_i2c_encoder *encoder,
 			 struct i2c_adapter *adap,
 			 const struct i2c_board_info *info);
 
+static inline const struct nouveau_i2c_encoder_funcs *
+get_encoder_i2c_funcs(struct drm_encoder *enc)
+{
+	return to_encoder_i2c(enc)->encoder_i2c_funcs;
+}
 
 /**
- * struct drm_i2c_encoder_driver
+ * struct nouveau_i2c_encoder_driver
  *
  * Describes a device driver for an encoder connected to the GPU through an I2C
  * bus.
  */
-struct drm_i2c_encoder_driver {
+struct nouveau_i2c_encoder_driver {
 	/**
 	 * @i2c_driver: I2C device driver description.
 	 */
@@ -175,67 +179,40 @@ struct drm_i2c_encoder_driver {
 
 	/**
 	 * @encoder_init: Callback to allocate any per-encoder data structures
-	 * and to initialize the @slave_funcs and (optionally) @slave_priv
+	 * and to initialize the @encoder_i2c_funcs and (optionally) @encoder_i2c_priv
 	 * members of @encoder.
 	 */
 	int (*encoder_init)(struct i2c_client *client,
 			    struct drm_device *dev,
-			    struct drm_encoder_slave *encoder);
+			    struct nouveau_i2c_encoder *encoder);
 
 };
-#define to_drm_i2c_encoder_driver(x) container_of((x),			\
-						  struct drm_i2c_encoder_driver, \
+#define to_nouveau_i2c_encoder_driver(x) container_of((x),			\
+						  struct nouveau_i2c_encoder_driver, \
 						  i2c_driver)
 
 /**
- * drm_i2c_encoder_get_client - Get the I2C client corresponding to an encoder
+ * nouveau_i2c_encoder_get_client - Get the I2C client corresponding to an encoder
  * @encoder: The encoder
  */
-static inline struct i2c_client *drm_i2c_encoder_get_client(struct drm_encoder *encoder)
-{
-	return (struct i2c_client *)to_encoder_slave(encoder)->bus_priv;
-}
-
-/**
- * drm_i2c_encoder_register - Register an I2C encoder driver
- * @owner:	Module containing the driver.
- * @driver:	Driver to be registered.
- */
-static inline int drm_i2c_encoder_register(struct module *owner,
-					   struct drm_i2c_encoder_driver *driver)
+static inline struct i2c_client *nouveau_i2c_encoder_get_client(struct drm_encoder *encoder)
 {
-	return i2c_register_driver(owner, &driver->i2c_driver);
+	return to_encoder_i2c(encoder)->i2c_client;
 }
 
-/**
- * drm_i2c_encoder_unregister - Unregister an I2C encoder driver
- * @driver:	Driver to be unregistered.
- */
-static inline void drm_i2c_encoder_unregister(struct drm_i2c_encoder_driver *driver)
-{
-	i2c_del_driver(&driver->i2c_driver);
-}
-
-void drm_i2c_encoder_destroy(struct drm_encoder *encoder);
-
+void nouveau_i2c_encoder_destroy(struct drm_encoder *encoder);
 
 /*
  * Wrapper fxns which can be plugged in to drm_encoder_helper_funcs:
  */
 
-void drm_i2c_encoder_dpms(struct drm_encoder *encoder, int mode);
-bool drm_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
+bool nouveau_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
 		const struct drm_display_mode *mode,
 		struct drm_display_mode *adjusted_mode);
-void drm_i2c_encoder_prepare(struct drm_encoder *encoder);
-void drm_i2c_encoder_commit(struct drm_encoder *encoder);
-void drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
-		struct drm_display_mode *mode,
-		struct drm_display_mode *adjusted_mode);
-enum drm_connector_status drm_i2c_encoder_detect(struct drm_encoder *encoder,
+enum drm_connector_status nouveau_i2c_encoder_detect(struct drm_encoder *encoder,
 	    struct drm_connector *connector);
-void drm_i2c_encoder_save(struct drm_encoder *encoder);
-void drm_i2c_encoder_restore(struct drm_encoder *encoder);
+void nouveau_i2c_encoder_save(struct drm_encoder *encoder);
+void nouveau_i2c_encoder_restore(struct drm_encoder *encoder);
 
 
 #endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 8d5c9c74cbb90927036cee75f173f687617f14ec..c69a57d8d9ec456de510e5c1bb195c9dcc91b595 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -797,7 +797,7 @@ nouveau_connector_set_property(struct drm_connector *connector,
 						    property, value);
 	if (ret) {
 		if (nv_encoder && nv_encoder->dcb->type == DCB_OUTPUT_TV)
-			return get_slave_funcs(encoder)->set_property(
+			return get_encoder_i2c_funcs(encoder)->set_property(
 				encoder, connector, property, value);
 		return ret;
 	}
@@ -1015,7 +1015,7 @@ nouveau_connector_get_modes(struct drm_connector *connector)
 		nouveau_connector_detect_depth(connector);
 
 	if (nv_encoder->dcb->type == DCB_OUTPUT_TV)
-		ret = get_slave_funcs(encoder)->get_modes(encoder, connector);
+		ret = get_encoder_i2c_funcs(encoder)->get_modes(encoder, connector);
 
 	if (nv_connector->type == DCB_CONNECTOR_LVDS ||
 	    nv_connector->type == DCB_CONNECTOR_LVDS_SPWG ||
@@ -1100,7 +1100,7 @@ nouveau_connector_mode_valid(struct drm_connector *connector,
 			max_clock = 350000;
 		break;
 	case DCB_OUTPUT_TV:
-		return get_slave_funcs(encoder)->mode_valid(encoder, mode);
+		return get_encoder_i2c_funcs(encoder)->mode_valid(encoder, mode);
 	case DCB_OUTPUT_DP:
 		return nv50_dp_mode_valid(nv_encoder, mode, NULL);
 	default:
diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h b/drivers/gpu/drm/nouveau/nouveau_encoder.h
index 333042fc493faec360b8d9713ab687836b986431..a7d06564f4c46cb766d6907f47ded3f8a7f5d33f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
+++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
@@ -31,7 +31,8 @@
 
 #include <drm/display/drm_dp_helper.h>
 #include <drm/display/drm_dp_mst_helper.h>
-#include <drm/drm_encoder_slave.h>
+
+#include <i2c/encoder_i2c.h>
 
 #include "dispnv04/disp.h"
 
@@ -43,7 +44,7 @@ struct nouveau_connector;
 struct nvkm_i2c_port;
 
 struct nouveau_encoder {
-	struct drm_encoder_slave base;
+	struct nouveau_i2c_encoder base;
 
 	struct dcb_output *dcb;
 	struct nvif_outp outp;
@@ -137,7 +138,7 @@ find_encoder(struct drm_connector *connector, int type);
 
 static inline struct nouveau_encoder *nouveau_encoder(struct drm_encoder *enc)
 {
-	struct drm_encoder_slave *slave = to_encoder_slave(enc);
+	struct nouveau_i2c_encoder *slave = to_encoder_i2c(enc);
 
 	return container_of(slave, struct nouveau_encoder, base);
 }
@@ -147,12 +148,6 @@ static inline struct drm_encoder *to_drm_encoder(struct nouveau_encoder *enc)
 	return &enc->base.base;
 }
 
-static inline const struct drm_encoder_slave_funcs *
-get_slave_funcs(struct drm_encoder *enc)
-{
-	return to_encoder_slave(enc)->slave_funcs;
-}
-
 /* nouveau_dp.c */
 enum nouveau_dp_status {
 	NOUVEAU_DP_NONE,

-- 
2.39.5


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

* Re: [PATCH v2 2/2] drm/nouveau: vendor in drm_encoder_slave API
  2024-12-15 10:19 ` [PATCH v2 2/2] drm/nouveau: vendor in drm_encoder_slave API Dmitry Baryshkov
@ 2024-12-16 13:50   ` Danilo Krummrich
  0 siblings, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2024-12-16 13:50 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Karol Herbst, Lyude Paul, dri-devel, linux-kernel,
	nouveau, Laurent Pinchart

On Sun, Dec 15, 2024 at 12:19:24PM +0200, Dmitry Baryshkov wrote:
> Nouveau driver is the only user of the drm_encoder_slave API. Rework
> necessary bits of drm_encoder_slave into the nouveau_i2c_encoder API and
> drop drm_encoder_slave.c from the DRM KMS helper.

Please fix the CHECK warnings produced by `./scripts/checkpatch.pl --strict`.

> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/Makefile                           |   1 -
>  drivers/gpu/drm/nouveau/dispnv04/Kbuild            |   1 +
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  10 +-
>  drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c  |  30 +++---
>  drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c |   8 +-
>  drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h |   4 +-
>  drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c  |  30 +++---
>  .../dispnv04/nouveau_i2c_encoder.c}                |  85 +++++-----------
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  18 ++--
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |   4 +-
>  .../gpu/drm/nouveau/include/i2c/encoder_i2c.h      | 109 ++++++++-------------
>  drivers/gpu/drm/nouveau/nouveau_connector.c        |   6 +-
>  drivers/gpu/drm/nouveau/nouveau_encoder.h          |  13 +--
>  13 files changed, 127 insertions(+), 192 deletions(-)
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 19fb370fbc56772077973c864df71e4b8e0bf99b..85af94bb907d6cdbad7078e2667426e479b1069f 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -135,7 +135,6 @@ drm_kms_helper-y := \
>  	drm_atomic_state_helper.o \
>  	drm_crtc_helper.o \
>  	drm_damage_helper.o \
> -	drm_encoder_slave.o \
>  	drm_flip_work.o \
>  	drm_format_helper.o \
>  	drm_gem_atomic_helper.o \
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/Kbuild b/drivers/gpu/drm/nouveau/dispnv04/Kbuild
> index 949802882ebd53c15e124c218a092af9693d36bc..4c7bc6bb81b325ac22286372b0a3e4f1390e78be 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/Kbuild
> +++ b/drivers/gpu/drm/nouveau/dispnv04/Kbuild
> @@ -6,6 +6,7 @@ nouveau-y += dispnv04/dac.o
>  nouveau-y += dispnv04/dfp.o
>  nouveau-y += dispnv04/disp.o
>  nouveau-y += dispnv04/hw.o
> +nouveau-y += dispnv04/nouveau_i2c_encoder.o
>  nouveau-y += dispnv04/overlay.o
>  nouveau-y += dispnv04/tvmodesnv17.o
>  nouveau-y += dispnv04/tvnv04.o
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> index 28a42ab5cb900ebe8a526e154f9e90598333356c..e211602298a5fb9f513203720b3ee5d37cd21122 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> @@ -171,7 +171,7 @@ static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
>  	list_for_each_entry(slave, &dev->mode_config.encoder_list, head) {
>  		struct dcb_output *slave_dcb = nouveau_encoder(slave)->dcb;
>  
> -		if (slave_dcb->type == DCB_OUTPUT_TMDS && get_slave_funcs(slave) &&
> +		if (slave_dcb->type == DCB_OUTPUT_TMDS && get_encoder_i2c_funcs(slave) &&
>  		    slave_dcb->tmdsconf.slave_addr == dcb->tmdsconf.slave_addr)
>  			return slave;
>  	}
> @@ -473,7 +473,7 @@ static void nv04_dfp_commit(struct drm_encoder *encoder)
>  	/* Init external transmitters */
>  	slave_encoder = get_tmds_slave(encoder);
>  	if (slave_encoder)
> -		get_slave_funcs(slave_encoder)->mode_set(
> +		get_encoder_i2c_funcs(slave_encoder)->mode_set(
>  			slave_encoder, &nv_encoder->mode, &nv_encoder->mode);
>  
>  	helper->dpms(encoder, DRM_MODE_DPMS_ON);
> @@ -614,8 +614,8 @@ static void nv04_dfp_destroy(struct drm_encoder *encoder)
>  {
>  	struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
>  
> -	if (get_slave_funcs(encoder))
> -		get_slave_funcs(encoder)->destroy(encoder);
> +	if (get_encoder_i2c_funcs(encoder))
> +		get_encoder_i2c_funcs(encoder)->destroy(encoder);
>  
>  	drm_encoder_cleanup(encoder);
>  	kfree(nv_encoder);
> @@ -649,7 +649,7 @@ static void nv04_tmds_slave_init(struct drm_encoder *encoder)
>  	if (type < 0)
>  		return;
>  
> -	drm_i2c_encoder_init(dev, to_encoder_slave(encoder),
> +	nouveau_i2c_encoder_init(dev, to_encoder_i2c(encoder),
>  			     &bus->i2c, &info[type].dev);
>  }
>  
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
> index 131512a5f3bd996ad1e2eb869ffa09837daba0c7..c9249b58e65459ae61898a246c36da2c3bfe0844 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_drv.c
> @@ -47,14 +47,14 @@ static void ch7006_encoder_destroy(struct drm_encoder *encoder)
>  	drm_property_destroy(encoder->dev, priv->scale_property);
>  
>  	kfree(priv);
> -	to_encoder_slave(encoder)->slave_priv = NULL;
> +	to_encoder_i2c(encoder)->encoder_i2c_priv = NULL;
>  
> -	drm_i2c_encoder_destroy(encoder);
> +	nouveau_i2c_encoder_destroy(encoder);
>  }
>  
>  static void  ch7006_encoder_dpms(struct drm_encoder *encoder, int mode)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  	struct ch7006_state *state = &priv->state;
>  
> @@ -71,7 +71,7 @@ static void  ch7006_encoder_dpms(struct drm_encoder *encoder, int mode)
>  
>  static void ch7006_encoder_save(struct drm_encoder *encoder)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  
>  	ch7006_dbg(client, "\n");
> @@ -81,7 +81,7 @@ static void ch7006_encoder_save(struct drm_encoder *encoder)
>  
>  static void ch7006_encoder_restore(struct drm_encoder *encoder)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  
>  	ch7006_dbg(client, "\n");
> @@ -116,7 +116,7 @@ static void ch7006_encoder_mode_set(struct drm_encoder *encoder,
>  				     struct drm_display_mode *drm_mode,
>  				     struct drm_display_mode *adjusted_mode)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  	struct ch7006_encoder_params *params = &priv->params;
>  	struct ch7006_state *state = &priv->state;
> @@ -179,7 +179,7 @@ static void ch7006_encoder_mode_set(struct drm_encoder *encoder,
>  static enum drm_connector_status ch7006_encoder_detect(struct drm_encoder *encoder,
>  						       struct drm_connector *connector)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  	struct ch7006_state *state = &priv->state;
>  	int det;
> @@ -285,7 +285,7 @@ static int ch7006_encoder_set_property(struct drm_encoder *encoder,
>  				       struct drm_property *property,
>  				       uint64_t val)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  	struct ch7006_state *state = &priv->state;
>  	struct drm_mode_config *conf = &encoder->dev->mode_config;
> @@ -370,7 +370,7 @@ static int ch7006_encoder_set_property(struct drm_encoder *encoder,
>  	return 0;
>  }
>  
> -static const struct drm_encoder_slave_funcs ch7006_encoder_funcs = {
> +static const struct nouveau_i2c_encoder_funcs ch7006_encoder_funcs = {
>  	.set_config = ch7006_encoder_set_config,
>  	.destroy = ch7006_encoder_destroy,
>  	.dpms = ch7006_encoder_dpms,
> @@ -437,7 +437,7 @@ static int ch7006_resume(struct device *dev)
>  
>  static int ch7006_encoder_init(struct i2c_client *client,
>  			       struct drm_device *dev,
> -			       struct drm_encoder_slave *encoder)
> +			       struct nouveau_i2c_encoder *encoder)
>  {
>  	struct ch7006_priv *priv;
>  	int i;
> @@ -448,8 +448,8 @@ static int ch7006_encoder_init(struct i2c_client *client,
>  	if (!priv)
>  		return -ENOMEM;
>  
> -	encoder->slave_priv = priv;
> -	encoder->slave_funcs = &ch7006_encoder_funcs;
> +	encoder->encoder_i2c_priv = priv;
> +	encoder->encoder_i2c_funcs = &ch7006_encoder_funcs;
>  
>  	priv->norm = TV_NORM_PAL;
>  	priv->select_subconnector = DRM_MODE_SUBCONNECTOR_Automatic;
> @@ -495,7 +495,7 @@ static const struct dev_pm_ops ch7006_pm_ops = {
>  	.resume = ch7006_resume,
>  };
>  
> -static struct drm_i2c_encoder_driver ch7006_driver = {
> +static struct nouveau_i2c_encoder_driver ch7006_driver = {
>  	.i2c_driver = {
>  		.probe = ch7006_probe,
>  		.remove = ch7006_remove,
> @@ -516,12 +516,12 @@ static struct drm_i2c_encoder_driver ch7006_driver = {
>  
>  static int __init ch7006_init(void)
>  {
> -	return drm_i2c_encoder_register(THIS_MODULE, &ch7006_driver);
> +	return i2c_add_driver(&ch7006_driver.i2c_driver);
>  }
>  
>  static void __exit ch7006_exit(void)
>  {
> -	drm_i2c_encoder_unregister(&ch7006_driver);
> +	i2c_del_driver(&ch7006_driver.i2c_driver);
>  }
>  
>  int ch7006_debug;
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
> index 6afe6d0ee6306db57c3e3bafe2bf1b0b1b49aea5..e58d94451959a2afc01f570ea620d8e6721cb7af 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_mode.c
> @@ -198,7 +198,7 @@ const struct ch7006_mode *ch7006_lookup_mode(struct drm_encoder *encoder,
>  
>  void ch7006_setup_levels(struct drm_encoder *encoder)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  	uint8_t *regs = priv->state.regs;
>  	const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
> @@ -229,7 +229,7 @@ void ch7006_setup_levels(struct drm_encoder *encoder)
>  
>  void ch7006_setup_subcarrier(struct drm_encoder *encoder)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  	struct ch7006_state *state = &priv->state;
>  	const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
> @@ -253,7 +253,7 @@ void ch7006_setup_subcarrier(struct drm_encoder *encoder)
>  
>  void ch7006_setup_pll(struct drm_encoder *encoder)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  	uint8_t *regs = priv->state.regs;
>  	const struct ch7006_mode *mode = priv->mode;
> @@ -324,7 +324,7 @@ void ch7006_setup_power_state(struct drm_encoder *encoder)
>  
>  void ch7006_setup_properties(struct drm_encoder *encoder)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  	struct ch7006_priv *priv = to_ch7006_priv(encoder);
>  	struct ch7006_state *state = &priv->state;
>  	const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
> index 8136f8cd8f9b859ccf915e295c783f9fc8321c2e..70c4fc8a181035b8960a8fcce12b5a1ad76e58b7 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
> +++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/ch7006_priv.h
> @@ -27,9 +27,9 @@
>  #ifndef __NOUVEAU_I2C_CH7006_PRIV_H__
>  #define __NOUVEAU_I2C_CH7006_PRIV_H__
>  
> -#include <drm/drm_encoder_slave.h>
>  #include <drm/drm_probe_helper.h>
>  
> +#include <i2c/encoder_i2c.h>
>  #include <i2c/ch7006.h>
>  
>  typedef int64_t fixed;
> @@ -100,7 +100,7 @@ struct ch7006_priv {
>  };
>  
>  #define to_ch7006_priv(x) \
> -	((struct ch7006_priv *)to_encoder_slave(x)->slave_priv)
> +	((struct ch7006_priv *)to_encoder_i2c(x)->encoder_i2c_priv)
>  
>  extern int ch7006_debug;
>  extern char *ch7006_tv_norm;
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c b/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
> index 74fc961c0d0de06f1fe8dd93d351452cd20cead7..80758e796b5d794f8e05c1b68b3383e1af3d0df5 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/i2c/sil164_drv.c
> @@ -27,10 +27,10 @@
>  #include <linux/module.h>
>  
>  #include <drm/drm_drv.h>
> -#include <drm/drm_encoder_slave.h>
>  #include <drm/drm_print.h>
>  #include <drm/drm_probe_helper.h>
>  
> +#include <i2c/encoder_i2c.h>
>  #include <i2c/sil164.h>
>  
>  struct sil164_priv {
> @@ -42,7 +42,7 @@ struct sil164_priv {
>  };
>  
>  #define to_sil164_priv(x) \
> -	((struct sil164_priv *)to_encoder_slave(x)->slave_priv)
> +	((struct sil164_priv *)to_encoder_i2c(x)->encoder_i2c_priv)
>  
>  #define sil164_dbg(client, format, ...) do {				\
>  		if (drm_debug_enabled(DRM_UT_KMS))			\
> @@ -222,7 +222,7 @@ sil164_encoder_dpms(struct drm_encoder *encoder, int mode)
>  	bool on = (mode == DRM_MODE_DPMS_ON);
>  	bool duallink = (on && encoder->crtc->mode.clock > 165000);
>  
> -	sil164_set_power_state(drm_i2c_encoder_get_client(encoder), on);
> +	sil164_set_power_state(nouveau_i2c_encoder_get_client(encoder), on);
>  
>  	if (priv->duallink_slave)
>  		sil164_set_power_state(priv->duallink_slave, duallink);
> @@ -233,7 +233,7 @@ sil164_encoder_save(struct drm_encoder *encoder)
>  {
>  	struct sil164_priv *priv = to_sil164_priv(encoder);
>  
> -	sil164_save_state(drm_i2c_encoder_get_client(encoder),
> +	sil164_save_state(nouveau_i2c_encoder_get_client(encoder),
>  			  priv->saved_state);
>  
>  	if (priv->duallink_slave)
> @@ -246,7 +246,7 @@ sil164_encoder_restore(struct drm_encoder *encoder)
>  {
>  	struct sil164_priv *priv = to_sil164_priv(encoder);
>  
> -	sil164_restore_state(drm_i2c_encoder_get_client(encoder),
> +	sil164_restore_state(nouveau_i2c_encoder_get_client(encoder),
>  			     priv->saved_state);
>  
>  	if (priv->duallink_slave)
> @@ -278,7 +278,7 @@ sil164_encoder_mode_set(struct drm_encoder *encoder,
>  	struct sil164_priv *priv = to_sil164_priv(encoder);
>  	bool duallink = adjusted_mode->clock > 165000;
>  
> -	sil164_init_state(drm_i2c_encoder_get_client(encoder),
> +	sil164_init_state(nouveau_i2c_encoder_get_client(encoder),
>  			  &priv->config, duallink);
>  
>  	if (priv->duallink_slave)
> @@ -292,7 +292,7 @@ static enum drm_connector_status
>  sil164_encoder_detect(struct drm_encoder *encoder,
>  		      struct drm_connector *connector)
>  {
> -	struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(encoder);
>  
>  	if (sil164_read(client, SIL164_DETECT) & SIL164_DETECT_HOTPLUG_STAT)
>  		return connector_status_connected;
> @@ -331,10 +331,10 @@ sil164_encoder_destroy(struct drm_encoder *encoder)
>  	i2c_unregister_device(priv->duallink_slave);
>  
>  	kfree(priv);
> -	drm_i2c_encoder_destroy(encoder);
> +	nouveau_i2c_encoder_destroy(encoder);
>  }
>  
> -static const struct drm_encoder_slave_funcs sil164_encoder_funcs = {
> +static const struct nouveau_i2c_encoder_funcs sil164_encoder_funcs = {
>  	.set_config = sil164_encoder_set_config,
>  	.destroy = sil164_encoder_destroy,
>  	.dpms = sil164_encoder_dpms,
> @@ -394,7 +394,7 @@ sil164_detect_slave(struct i2c_client *client)
>  static int
>  sil164_encoder_init(struct i2c_client *client,
>  		    struct drm_device *dev,
> -		    struct drm_encoder_slave *encoder)
> +		    struct nouveau_i2c_encoder *encoder)
>  {
>  	struct sil164_priv *priv;
>  	struct i2c_client *slave_client;
> @@ -403,8 +403,8 @@ sil164_encoder_init(struct i2c_client *client,
>  	if (!priv)
>  		return -ENOMEM;
>  
> -	encoder->slave_priv = priv;
> -	encoder->slave_funcs = &sil164_encoder_funcs;
> +	encoder->encoder_i2c_priv = priv;
> +	encoder->encoder_i2c_funcs = &sil164_encoder_funcs;
>  
>  	slave_client = sil164_detect_slave(client);
>  	if (!IS_ERR(slave_client))
> @@ -419,7 +419,7 @@ static const struct i2c_device_id sil164_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, sil164_ids);
>  
> -static struct drm_i2c_encoder_driver sil164_driver = {
> +static struct nouveau_i2c_encoder_driver sil164_driver = {
>  	.i2c_driver = {
>  		.probe = sil164_probe,
>  		.driver = {
> @@ -435,13 +435,13 @@ static struct drm_i2c_encoder_driver sil164_driver = {
>  static int __init
>  sil164_init(void)
>  {
> -	return drm_i2c_encoder_register(THIS_MODULE, &sil164_driver);
> +	return i2c_add_driver(&sil164_driver.i2c_driver);
>  }
>  
>  static void __exit
>  sil164_exit(void)
>  {
> -	drm_i2c_encoder_unregister(&sil164_driver);
> +	i2c_del_driver(&sil164_driver.i2c_driver);
>  }
>  
>  MODULE_AUTHOR("Francisco Jerez <currojerez@riseup.net>");
> diff --git a/drivers/gpu/drm/drm_encoder_slave.c b/drivers/gpu/drm/nouveau/dispnv04/nouveau_i2c_encoder.c
> similarity index 57%
> rename from drivers/gpu/drm/drm_encoder_slave.c
> rename to drivers/gpu/drm/nouveau/dispnv04/nouveau_i2c_encoder.c
> index e464429d32df6e2eeb541d92c5a486b4ee7a21ec..58270719cc3db6a2a0205196bf18aba139da2df4 100644
> --- a/drivers/gpu/drm/drm_encoder_slave.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/nouveau_i2c_encoder.c
> @@ -26,10 +26,10 @@
>  
>  #include <linux/module.h>
>  
> -#include <drm/drm_encoder_slave.h>
> +#include <i2c/encoder_i2c.h>
>  
>  /**
> - * drm_i2c_encoder_init - Initialize an I2C slave encoder
> + * nouveau_i2c_encoder_init - Initialize an I2C slave encoder
>   * @dev:	DRM device.
>   * @encoder:    Encoder to be attached to the I2C device. You aren't
>   *		required to have called drm_encoder_init() before.
> @@ -40,7 +40,7 @@
>   *
>   * Create an I2C device on the specified bus (the module containing its
>   * driver is transparently loaded) and attach it to the specified
> - * &drm_encoder_slave. The @slave_funcs field will be initialized with
> + * &nouveau_i2c_encoder. The @encoder_i2c_funcs field will be initialized with
>   * the hooks provided by the slave driver.
>   *
>   * If @info.platform_data is non-NULL it will be used as the initial
> @@ -49,14 +49,14 @@
>   * Returns 0 on success or a negative errno on failure, in particular,
>   * -ENODEV is returned when no matching driver is found.
>   */
> -int drm_i2c_encoder_init(struct drm_device *dev,
> -			 struct drm_encoder_slave *encoder,
> +int nouveau_i2c_encoder_init(struct drm_device *dev,
> +			 struct nouveau_i2c_encoder *encoder,
>  			 struct i2c_adapter *adap,
>  			 const struct i2c_board_info *info)
>  {
>  	struct module *module = NULL;
>  	struct i2c_client *client;
> -	struct drm_i2c_encoder_driver *encoder_drv;
> +	struct nouveau_i2c_encoder_driver *encoder_drv;
>  	int err = 0;
>  
>  	request_module("%s%s", I2C_MODULE_PREFIX, info->type);
> @@ -73,16 +73,16 @@ int drm_i2c_encoder_init(struct drm_device *dev,
>  		goto fail_unregister;
>  	}
>  
> -	encoder->bus_priv = client;
> +	encoder->i2c_client = client;
>  
> -	encoder_drv = to_drm_i2c_encoder_driver(to_i2c_driver(client->dev.driver));
> +	encoder_drv = to_nouveau_i2c_encoder_driver(to_i2c_driver(client->dev.driver));
>  
>  	err = encoder_drv->encoder_init(client, dev, encoder);
>  	if (err)
>  		goto fail_module_put;
>  
>  	if (info->platform_data)
> -		encoder->slave_funcs->set_config(&encoder->base,
> +		encoder->encoder_i2c_funcs->set_config(&encoder->base,
>  						 info->platform_data);
>  
>  	return 0;
> @@ -93,90 +93,53 @@ int drm_i2c_encoder_init(struct drm_device *dev,
>  	i2c_unregister_device(client);
>  	return err;
>  }
> -EXPORT_SYMBOL(drm_i2c_encoder_init);
>  
>  /**
> - * drm_i2c_encoder_destroy - Unregister the I2C device backing an encoder
> + * nouveau_i2c_encoder_destroy - Unregister the I2C device backing an encoder
>   * @drm_encoder:	Encoder to be unregistered.
>   *
>   * This should be called from the @destroy method of an I2C slave
>   * encoder driver once I2C access is no longer needed.
>   */
> -void drm_i2c_encoder_destroy(struct drm_encoder *drm_encoder)
> +void nouveau_i2c_encoder_destroy(struct drm_encoder *drm_encoder)
>  {
> -	struct drm_encoder_slave *encoder = to_encoder_slave(drm_encoder);
> -	struct i2c_client *client = drm_i2c_encoder_get_client(drm_encoder);
> +	struct nouveau_i2c_encoder *encoder = to_encoder_i2c(drm_encoder);
> +	struct i2c_client *client = nouveau_i2c_encoder_get_client(drm_encoder);
>  	struct module *module = client->dev.driver->owner;
>  
>  	i2c_unregister_device(client);
> -	encoder->bus_priv = NULL;
> +	encoder->i2c_client = NULL;
>  
>  	module_put(module);
>  }
> -EXPORT_SYMBOL(drm_i2c_encoder_destroy);
> +EXPORT_SYMBOL(nouveau_i2c_encoder_destroy);
>  
>  /*
>   * Wrapper fxns which can be plugged in to drm_encoder_helper_funcs:
>   */
>  
> -static inline const struct drm_encoder_slave_funcs *
> -get_slave_funcs(struct drm_encoder *enc)
> -{
> -	return to_encoder_slave(enc)->slave_funcs;
> -}
> -
> -void drm_i2c_encoder_dpms(struct drm_encoder *encoder, int mode)
> -{
> -	get_slave_funcs(encoder)->dpms(encoder, mode);
> -}
> -EXPORT_SYMBOL(drm_i2c_encoder_dpms);
> -
> -bool drm_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
> +bool nouveau_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
>  		const struct drm_display_mode *mode,
>  		struct drm_display_mode *adjusted_mode)
>  {
> -	if (!get_slave_funcs(encoder)->mode_fixup)
> +	if (!get_encoder_i2c_funcs(encoder)->mode_fixup)
>  		return true;
>  
> -	return get_slave_funcs(encoder)->mode_fixup(encoder, mode, adjusted_mode);
> -}
> -EXPORT_SYMBOL(drm_i2c_encoder_mode_fixup);
> -
> -void drm_i2c_encoder_prepare(struct drm_encoder *encoder)
> -{
> -	drm_i2c_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
> -}
> -EXPORT_SYMBOL(drm_i2c_encoder_prepare);
> -
> -void drm_i2c_encoder_commit(struct drm_encoder *encoder)
> -{
> -	drm_i2c_encoder_dpms(encoder, DRM_MODE_DPMS_ON);
> -}
> -EXPORT_SYMBOL(drm_i2c_encoder_commit);
> -
> -void drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
> -		struct drm_display_mode *mode,
> -		struct drm_display_mode *adjusted_mode)
> -{
> -	get_slave_funcs(encoder)->mode_set(encoder, mode, adjusted_mode);
> +	return get_encoder_i2c_funcs(encoder)->mode_fixup(encoder, mode, adjusted_mode);
>  }
> -EXPORT_SYMBOL(drm_i2c_encoder_mode_set);
>  
> -enum drm_connector_status drm_i2c_encoder_detect(struct drm_encoder *encoder,
> +enum drm_connector_status nouveau_i2c_encoder_detect(struct drm_encoder *encoder,
>  	    struct drm_connector *connector)
>  {
> -	return get_slave_funcs(encoder)->detect(encoder, connector);
> +	return get_encoder_i2c_funcs(encoder)->detect(encoder, connector);
>  }
> -EXPORT_SYMBOL(drm_i2c_encoder_detect);
>  
> -void drm_i2c_encoder_save(struct drm_encoder *encoder)
> +void nouveau_i2c_encoder_save(struct drm_encoder *encoder)
>  {
> -	get_slave_funcs(encoder)->save(encoder);
> +	get_encoder_i2c_funcs(encoder)->save(encoder);
>  }
> -EXPORT_SYMBOL(drm_i2c_encoder_save);
>  
> -void drm_i2c_encoder_restore(struct drm_encoder *encoder)
> +void nouveau_i2c_encoder_restore(struct drm_encoder *encoder)
>  {
> -	get_slave_funcs(encoder)->restore(encoder);
> +	get_encoder_i2c_funcs(encoder)->restore(encoder);
>  }
> -EXPORT_SYMBOL(drm_i2c_encoder_restore);
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
> index 67f3e0ac0e109b223ca8ec4ddc4e688247373b2e..0ce369d7b773ac3753fe907acdee3b402644cc0e 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
> @@ -99,7 +99,7 @@ static void nv04_tv_dpms(struct drm_encoder *encoder, int mode)
>  
>  	NVWriteRAMDAC(dev, 0, NV_PRAMDAC_PLL_COEFF_SELECT, state->pllsel);
>  
> -	get_slave_funcs(encoder)->dpms(encoder, mode);
> +	get_encoder_i2c_funcs(encoder)->dpms(encoder, mode);
>  }
>  
>  static void nv04_tv_bind(struct drm_device *dev, int head, bool bind)
> @@ -158,7 +158,7 @@ static void nv04_tv_mode_set(struct drm_encoder *encoder,
>  	regp->tv_vskew = 1;
>  	regp->tv_vsync_delay = 1;
>  
> -	get_slave_funcs(encoder)->mode_set(encoder, mode, adjusted_mode);
> +	get_encoder_i2c_funcs(encoder)->mode_set(encoder, mode, adjusted_mode);
>  }
>  
>  static void nv04_tv_commit(struct drm_encoder *encoder)
> @@ -178,7 +178,7 @@ static void nv04_tv_commit(struct drm_encoder *encoder)
>  
>  static void nv04_tv_destroy(struct drm_encoder *encoder)
>  {
> -	get_slave_funcs(encoder)->destroy(encoder);
> +	get_encoder_i2c_funcs(encoder)->destroy(encoder);
>  	drm_encoder_cleanup(encoder);
>  
>  	kfree(encoder->helper_private);
> @@ -191,11 +191,11 @@ static const struct drm_encoder_funcs nv04_tv_funcs = {
>  
>  static const struct drm_encoder_helper_funcs nv04_tv_helper_funcs = {
>  	.dpms = nv04_tv_dpms,
> -	.mode_fixup = drm_i2c_encoder_mode_fixup,
> +	.mode_fixup = nouveau_i2c_encoder_mode_fixup,
>  	.prepare = nv04_tv_prepare,
>  	.commit = nv04_tv_commit,
>  	.mode_set = nv04_tv_mode_set,
> -	.detect = drm_i2c_encoder_detect,
> +	.detect = nouveau_i2c_encoder_detect,
>  };
>  
>  int
> @@ -226,8 +226,8 @@ nv04_tv_create(struct drm_connector *connector, struct dcb_output *entry)
>  			 NULL);
>  	drm_encoder_helper_add(encoder, &nv04_tv_helper_funcs);
>  
> -	nv_encoder->enc_save = drm_i2c_encoder_save;
> -	nv_encoder->enc_restore = drm_i2c_encoder_restore;
> +	nv_encoder->enc_save = nouveau_i2c_encoder_save;
> +	nv_encoder->enc_restore = nouveau_i2c_encoder_restore;
>  
>  	encoder->possible_crtcs = entry->heads;
>  	encoder->possible_clones = 0;
> @@ -235,14 +235,14 @@ nv04_tv_create(struct drm_connector *connector, struct dcb_output *entry)
>  	nv_encoder->or = ffs(entry->or) - 1;
>  
>  	/* Run the slave-specific initialization */
> -	ret = drm_i2c_encoder_init(dev, to_encoder_slave(encoder),
> +	ret = nouveau_i2c_encoder_init(dev, to_encoder_i2c(encoder),
>  				   &bus->i2c,
>  				   &nv04_tv_encoder_info[type].dev);
>  	if (ret < 0)
>  		goto fail_cleanup;
>  
>  	/* Attach it to the specified connector. */
> -	get_slave_funcs(encoder)->create_resources(encoder, connector);
> +	get_encoder_i2c_funcs(encoder)->create_resources(encoder, connector);
>  	drm_connector_attach_encoder(connector, encoder);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
> index 3ecb101d23e949b753b873d24eec01ad6fe7f5d6..50326691444322f282729e108b4ab88bd9cbc14a 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
> @@ -779,7 +779,7 @@ static const struct drm_encoder_helper_funcs nv17_tv_helper_funcs = {
>  	.detect = nv17_tv_detect,
>  };
>  
> -static const struct drm_encoder_slave_funcs nv17_tv_slave_funcs = {
> +static const struct nouveau_i2c_encoder_funcs nv17_tv_encoder_i2c_funcs = {
>  	.get_modes = nv17_tv_get_modes,
>  	.mode_valid = nv17_tv_mode_valid,
>  	.create_resources = nv17_tv_create_resources,
> @@ -818,7 +818,7 @@ nv17_tv_create(struct drm_connector *connector, struct dcb_output *entry)
>  	drm_encoder_init(dev, encoder, &nv17_tv_funcs, DRM_MODE_ENCODER_TVDAC,
>  			 NULL);
>  	drm_encoder_helper_add(encoder, &nv17_tv_helper_funcs);
> -	to_encoder_slave(encoder)->slave_funcs = &nv17_tv_slave_funcs;
> +	to_encoder_i2c(encoder)->encoder_i2c_funcs = &nv17_tv_encoder_i2c_funcs;
>  
>  	tv_enc->base.enc_save = nv17_tv_save;
>  	tv_enc->base.enc_restore = nv17_tv_restore;
> diff --git a/include/drm/drm_encoder_slave.h b/drivers/gpu/drm/nouveau/include/i2c/encoder_i2c.h
> similarity index 62%
> rename from include/drm/drm_encoder_slave.h
> rename to drivers/gpu/drm/nouveau/include/i2c/encoder_i2c.h
> index 49172166a164474f43e4afb2eeeb3cde8ae7c61a..f12f44415b79c336e6ecfa163cc780a0bcbc5ca5 100644
> --- a/include/drm/drm_encoder_slave.h
> +++ b/drivers/gpu/drm/nouveau/include/i2c/encoder_i2c.h
> @@ -24,8 +24,8 @@
>   *
>   */
>  
> -#ifndef __DRM_ENCODER_SLAVE_H__
> -#define __DRM_ENCODER_SLAVE_H__
> +#ifndef __NOUVEAU_ENCODER_I2C_H__
> +#define __NOUVEAU_ENCODER_I2C_H__
>  
>  #include <linux/i2c.h>
>  
> @@ -33,7 +33,7 @@
>  #include <drm/drm_encoder.h>
>  
>  /**
> - * struct drm_encoder_slave_funcs - Entry points exposed by a slave encoder driver
> + * struct nouveau_i2c_encoder_funcs - Entry points exposed by a I2C encoder driver
>   *
>   * Most of its members are analogous to the function pointers in
>   * &drm_encoder_helper_funcs and they can optionally be used to
> @@ -41,7 +41,7 @@
>   * @set_property) will typically be wrapped around and only be called
>   * if the encoder is the currently selected one for the connector.
>   */
> -struct drm_encoder_slave_funcs {
> +struct nouveau_i2c_encoder_funcs {
>  	/**
>  	 * @set_config: Initialize any encoder-specific modesetting parameters.
>  	 * The meaning of the @params parameter is implementation dependent. It
> @@ -58,24 +58,23 @@ struct drm_encoder_slave_funcs {
>  	void (*destroy)(struct drm_encoder *encoder);
>  
>  	/**
> -	 * @dpms: Analogous to &drm_encoder_helper_funcs @dpms callback. Wrapped
> -	 * by drm_i2c_encoder_dpms().
> +	 * @dpms: Analogous to &drm_encoder_helper_funcs @dpms callback.
>  	 */
>  	void (*dpms)(struct drm_encoder *encoder, int mode);
>  
>  	/**
> -	 * @save: Save state. Wrapped by drm_i2c_encoder_save().
> +	 * @save: Save state. Wrapped by nouveau_i2c_encoder_save().
>  	 */
>  	void (*save)(struct drm_encoder *encoder);
>  
>  	/**
> -	 * @restore: Restore state. Wrapped by drm_i2c_encoder_restore().
> +	 * @restore: Restore state. Wrapped by nouveau_i2c_encoder_restore().
>  	 */
>  	void (*restore)(struct drm_encoder *encoder);
>  
>  	/**
>  	 * @mode_fixup: Analogous to &drm_encoder_helper_funcs @mode_fixup
> -	 * callback. Wrapped by drm_i2c_encoder_mode_fixup().
> +	 * callback. Wrapped by nouveau_i2c_encoder_mode_fixup().
>  	 */
>  	bool (*mode_fixup)(struct drm_encoder *encoder,
>  			   const struct drm_display_mode *mode,
> @@ -88,7 +87,7 @@ struct drm_encoder_slave_funcs {
>  			  struct drm_display_mode *mode);
>  	/**
>  	 * @mode_set: Analogous to &drm_encoder_helper_funcs @mode_set
> -	 * callback. Wrapped by drm_i2c_encoder_mode_set().
> +	 * callback.
>  	 */
>  	void (*mode_set)(struct drm_encoder *encoder,
>  			 struct drm_display_mode *mode,
> @@ -96,7 +95,7 @@ struct drm_encoder_slave_funcs {
>  
>  	/**
>  	 * @detect: Analogous to &drm_encoder_helper_funcs @detect
> -	 * callback. Wrapped by drm_i2c_encoder_detect().
> +	 * callback. Wrapped by nouveau_i2c_encoder_detect().
>  	 */
>  	enum drm_connector_status (*detect)(struct drm_encoder *encoder,
>  					    struct drm_connector *connector);
> @@ -120,54 +119,59 @@ struct drm_encoder_slave_funcs {
>  };
>  
>  /**
> - * struct drm_encoder_slave - Slave encoder struct
> + * struct nouveau_i2c_encoder - I2C encoder struct
>   *
> - * A &drm_encoder_slave has two sets of callbacks, @slave_funcs and the
> + * A &nouveau_i2c_encoder has two sets of callbacks, @encoder_i2c_funcs and the
>   * ones in @base. The former are never actually called by the common
>   * CRTC code, it's just a convenience for splitting the encoder
>   * functions in an upper, GPU-specific layer and a (hopefully)
>   * GPU-agnostic lower layer: It's the GPU driver responsibility to
> - * call the slave methods when appropriate.
> + * call the nouveau_i2c_encoder methods when appropriate.
>   *
> - * drm_i2c_encoder_init() provides a way to get an implementation of
> + * nouveau_i2c_encoder_init() provides a way to get an implementation of
>   * this.
>   */
> -struct drm_encoder_slave {
> +struct nouveau_i2c_encoder {
>  	/**
>  	 * @base: DRM encoder object.
>  	 */
>  	struct drm_encoder base;
>  
>  	/**
> -	 * @slave_funcs: Slave encoder callbacks.
> +	 * @encoder_i2c_funcs: I2C encoder callbacks.
>  	 */
> -	const struct drm_encoder_slave_funcs *slave_funcs;
> +	const struct nouveau_i2c_encoder_funcs *encoder_i2c_funcs;
>  
>  	/**
> -	 * @slave_priv: Slave encoder private data.
> +	 * @encoder_i2c_priv: I2C encoder private data.
>  	 */
> -	void *slave_priv;
> +	void *encoder_i2c_priv;
>  
>  	/**
> -	 * @bus_priv: Bus specific data.
> +	 * @i2c_client: corresponding I2C client structure
>  	 */
> -	void *bus_priv;
> +	struct i2c_client *i2c_client;
>  };
> -#define to_encoder_slave(x) container_of((x), struct drm_encoder_slave, base)
> +#define to_encoder_i2c(x) container_of((x), struct nouveau_i2c_encoder, base)
>  
> -int drm_i2c_encoder_init(struct drm_device *dev,
> -			 struct drm_encoder_slave *encoder,
> +int nouveau_i2c_encoder_init(struct drm_device *dev,
> +			 struct nouveau_i2c_encoder *encoder,
>  			 struct i2c_adapter *adap,
>  			 const struct i2c_board_info *info);
>  
> +static inline const struct nouveau_i2c_encoder_funcs *
> +get_encoder_i2c_funcs(struct drm_encoder *enc)
> +{
> +	return to_encoder_i2c(enc)->encoder_i2c_funcs;
> +}
>  
>  /**
> - * struct drm_i2c_encoder_driver
> + * struct nouveau_i2c_encoder_driver
>   *
>   * Describes a device driver for an encoder connected to the GPU through an I2C
>   * bus.
>   */
> -struct drm_i2c_encoder_driver {
> +struct nouveau_i2c_encoder_driver {
>  	/**
>  	 * @i2c_driver: I2C device driver description.
>  	 */
> @@ -175,67 +179,40 @@ struct drm_i2c_encoder_driver {
>  
>  	/**
>  	 * @encoder_init: Callback to allocate any per-encoder data structures
> -	 * and to initialize the @slave_funcs and (optionally) @slave_priv
> +	 * and to initialize the @encoder_i2c_funcs and (optionally) @encoder_i2c_priv
>  	 * members of @encoder.
>  	 */
>  	int (*encoder_init)(struct i2c_client *client,
>  			    struct drm_device *dev,
> -			    struct drm_encoder_slave *encoder);
> +			    struct nouveau_i2c_encoder *encoder);
>  
>  };
> -#define to_drm_i2c_encoder_driver(x) container_of((x),			\
> -						  struct drm_i2c_encoder_driver, \
> +#define to_nouveau_i2c_encoder_driver(x) container_of((x),			\
> +						  struct nouveau_i2c_encoder_driver, \
>  						  i2c_driver)
>  
>  /**
> - * drm_i2c_encoder_get_client - Get the I2C client corresponding to an encoder
> + * nouveau_i2c_encoder_get_client - Get the I2C client corresponding to an encoder
>   * @encoder: The encoder
>   */
> -static inline struct i2c_client *drm_i2c_encoder_get_client(struct drm_encoder *encoder)
> -{
> -	return (struct i2c_client *)to_encoder_slave(encoder)->bus_priv;
> -}
> -
> -/**
> - * drm_i2c_encoder_register - Register an I2C encoder driver
> - * @owner:	Module containing the driver.
> - * @driver:	Driver to be registered.
> - */
> -static inline int drm_i2c_encoder_register(struct module *owner,
> -					   struct drm_i2c_encoder_driver *driver)
> +static inline struct i2c_client *nouveau_i2c_encoder_get_client(struct drm_encoder *encoder)
>  {
> -	return i2c_register_driver(owner, &driver->i2c_driver);
> +	return to_encoder_i2c(encoder)->i2c_client;
>  }
>  
> -/**
> - * drm_i2c_encoder_unregister - Unregister an I2C encoder driver
> - * @driver:	Driver to be unregistered.
> - */
> -static inline void drm_i2c_encoder_unregister(struct drm_i2c_encoder_driver *driver)
> -{
> -	i2c_del_driver(&driver->i2c_driver);
> -}
> -
> -void drm_i2c_encoder_destroy(struct drm_encoder *encoder);
> -
> +void nouveau_i2c_encoder_destroy(struct drm_encoder *encoder);
>  
>  /*
>   * Wrapper fxns which can be plugged in to drm_encoder_helper_funcs:
>   */
>  
> -void drm_i2c_encoder_dpms(struct drm_encoder *encoder, int mode);
> -bool drm_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
> +bool nouveau_i2c_encoder_mode_fixup(struct drm_encoder *encoder,
>  		const struct drm_display_mode *mode,
>  		struct drm_display_mode *adjusted_mode);
> -void drm_i2c_encoder_prepare(struct drm_encoder *encoder);
> -void drm_i2c_encoder_commit(struct drm_encoder *encoder);
> -void drm_i2c_encoder_mode_set(struct drm_encoder *encoder,
> -		struct drm_display_mode *mode,
> -		struct drm_display_mode *adjusted_mode);
> -enum drm_connector_status drm_i2c_encoder_detect(struct drm_encoder *encoder,
> +enum drm_connector_status nouveau_i2c_encoder_detect(struct drm_encoder *encoder,
>  	    struct drm_connector *connector);
> -void drm_i2c_encoder_save(struct drm_encoder *encoder);
> -void drm_i2c_encoder_restore(struct drm_encoder *encoder);
> +void nouveau_i2c_encoder_save(struct drm_encoder *encoder);
> +void nouveau_i2c_encoder_restore(struct drm_encoder *encoder);
>  
>  
>  #endif
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 8d5c9c74cbb90927036cee75f173f687617f14ec..c69a57d8d9ec456de510e5c1bb195c9dcc91b595 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -797,7 +797,7 @@ nouveau_connector_set_property(struct drm_connector *connector,
>  						    property, value);
>  	if (ret) {
>  		if (nv_encoder && nv_encoder->dcb->type == DCB_OUTPUT_TV)
> -			return get_slave_funcs(encoder)->set_property(
> +			return get_encoder_i2c_funcs(encoder)->set_property(
>  				encoder, connector, property, value);
>  		return ret;
>  	}
> @@ -1015,7 +1015,7 @@ nouveau_connector_get_modes(struct drm_connector *connector)
>  		nouveau_connector_detect_depth(connector);
>  
>  	if (nv_encoder->dcb->type == DCB_OUTPUT_TV)
> -		ret = get_slave_funcs(encoder)->get_modes(encoder, connector);
> +		ret = get_encoder_i2c_funcs(encoder)->get_modes(encoder, connector);
>  
>  	if (nv_connector->type == DCB_CONNECTOR_LVDS ||
>  	    nv_connector->type == DCB_CONNECTOR_LVDS_SPWG ||
> @@ -1100,7 +1100,7 @@ nouveau_connector_mode_valid(struct drm_connector *connector,
>  			max_clock = 350000;
>  		break;
>  	case DCB_OUTPUT_TV:
> -		return get_slave_funcs(encoder)->mode_valid(encoder, mode);
> +		return get_encoder_i2c_funcs(encoder)->mode_valid(encoder, mode);
>  	case DCB_OUTPUT_DP:
>  		return nv50_dp_mode_valid(nv_encoder, mode, NULL);
>  	default:
> diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h b/drivers/gpu/drm/nouveau/nouveau_encoder.h
> index 333042fc493faec360b8d9713ab687836b986431..a7d06564f4c46cb766d6907f47ded3f8a7f5d33f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
> @@ -31,7 +31,8 @@
>  
>  #include <drm/display/drm_dp_helper.h>
>  #include <drm/display/drm_dp_mst_helper.h>
> -#include <drm/drm_encoder_slave.h>
> +
> +#include <i2c/encoder_i2c.h>
>  
>  #include "dispnv04/disp.h"
>  
> @@ -43,7 +44,7 @@ struct nouveau_connector;
>  struct nvkm_i2c_port;
>  
>  struct nouveau_encoder {
> -	struct drm_encoder_slave base;
> +	struct nouveau_i2c_encoder base;
>  
>  	struct dcb_output *dcb;
>  	struct nvif_outp outp;
> @@ -137,7 +138,7 @@ find_encoder(struct drm_connector *connector, int type);
>  
>  static inline struct nouveau_encoder *nouveau_encoder(struct drm_encoder *enc)
>  {
> -	struct drm_encoder_slave *slave = to_encoder_slave(enc);
> +	struct nouveau_i2c_encoder *slave = to_encoder_i2c(enc);
>  
>  	return container_of(slave, struct nouveau_encoder, base);
>  }
> @@ -147,12 +148,6 @@ static inline struct drm_encoder *to_drm_encoder(struct nouveau_encoder *enc)
>  	return &enc->base.base;
>  }
>  
> -static inline const struct drm_encoder_slave_funcs *
> -get_slave_funcs(struct drm_encoder *enc)
> -{
> -	return to_encoder_slave(enc)->slave_funcs;
> -}
> -
>  /* nouveau_dp.c */
>  enum nouveau_dp_status {
>  	NOUVEAU_DP_NONE,
> 
> -- 
> 2.39.5
> 

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

* Re: [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface
  2024-12-15 10:19 [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface Dmitry Baryshkov
  2024-12-15 10:19 ` [PATCH v2 1/2] drm/nouveau: incorporate I2C TV encoder drivers Dmitry Baryshkov
  2024-12-15 10:19 ` [PATCH v2 2/2] drm/nouveau: vendor in drm_encoder_slave API Dmitry Baryshkov
@ 2024-12-16 11:45 ` Danilo Krummrich
  2024-12-16 12:11   ` Dmitry Baryshkov
  2 siblings, 1 reply; 12+ messages in thread
From: Danilo Krummrich @ 2024-12-16 11:45 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Karol Herbst, Lyude Paul, dri-devel, linux-kernel,
	nouveau, Laurent Pinchart

On Sun, Dec 15, 2024 at 12:19:22PM +0200, Dmitry Baryshkov wrote:
> The nouveau driver is the only user of the drm_encoder_slave interface.
> Demote it from KMS helpers module to the nouveau driver itself, moving
> corresponding I2C encoders to be handled by nouveau driver too.

I understand nouveau is the only driver using this interface (and the
corresponding i2c encoders).

However, I'm not quite seeing the advantage of folding the interface (including
the two i2c drivers) into nouveau. I don't think this legacy interface does harm
the subsystem in any way / does prevent the subsystem from moving forward.

Can't we just keep it as it is?

> 
> Ideally those two drivers should be converted to the drm_bridge
> interface, but it's unclear if it's worth spending time on that.

Probably not.

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> Changes in v2:
> - Renamed symbols in defconfig (Laurent)
> - Added missing Kbuild file (Laurent, LKP)
> - Renamed guarding defines in include files.
> - Dropped mentions of two removed functions.
> - Link to v1: https://lore.kernel.org/r/20241214-nouveau-encoder-slave-v1-0-beda767472e3@linaro.org
> 
> ---
> Dmitry Baryshkov (2):
>       drm/nouveau: incorporate I2C TV encoder drivers
>       drm/nouveau: vendor in drm_encoder_slave API
> 
>  arch/arm/configs/multi_v7_defconfig                |   4 +-
>  arch/parisc/configs/generic-32bit_defconfig        |   4 +-
>  arch/parisc/configs/generic-64bit_defconfig        |   4 +-
>  drivers/gpu/drm/Makefile                           |   1 -
>  drivers/gpu/drm/i2c/Kconfig                        |  18 ----
>  drivers/gpu/drm/i2c/Makefile                       |   6 --
>  drivers/gpu/drm/nouveau/Kconfig                    |  20 ++++
>  drivers/gpu/drm/nouveau/dispnv04/Kbuild            |   3 +
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  12 +--
>  drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild        |   5 +
>  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_drv.c    |  30 +++---
>  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_mode.c   |   8 +-
>  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_priv.h   |  11 ++-
>  .../drm/{ => nouveau/dispnv04}/i2c/sil164_drv.c    |  33 ++++---
>  .../dispnv04/nouveau_i2c_encoder.c}                |  85 +++++-----------
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  20 ++--
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |   4 +-
>  .../gpu/drm/nouveau/include}/i2c/ch7006.h          |   4 +-
>  .../gpu/drm/nouveau/include/i2c/encoder_i2c.h      | 109 ++++++++-------------
>  .../gpu/drm/nouveau/include}/i2c/sil164.h          |   4 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c        |   6 +-
>  drivers/gpu/drm/nouveau/nouveau_encoder.h          |  13 +--
>  22 files changed, 172 insertions(+), 232 deletions(-)
> ---
> base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f
> change-id: 20241214-nouveau-encoder-slave-a6dd422fa4a9
> 
> Best regards,
> -- 
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 

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

* Re: [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface
  2024-12-16 11:45 ` [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface Danilo Krummrich
@ 2024-12-16 12:11   ` Dmitry Baryshkov
  2024-12-16 12:16     ` Laurent Pinchart
  0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Baryshkov @ 2024-12-16 12:11 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Karol Herbst, Lyude Paul, dri-devel, linux-kernel,
	nouveau, Laurent Pinchart

On Mon, Dec 16, 2024 at 12:45:15PM +0100, Danilo Krummrich wrote:
> On Sun, Dec 15, 2024 at 12:19:22PM +0200, Dmitry Baryshkov wrote:
> > The nouveau driver is the only user of the drm_encoder_slave interface.
> > Demote it from KMS helpers module to the nouveau driver itself, moving
> > corresponding I2C encoders to be handled by nouveau driver too.
> 
> I understand nouveau is the only driver using this interface (and the
> corresponding i2c encoders).
> 
> However, I'm not quite seeing the advantage of folding the interface (including
> the two i2c drivers) into nouveau. I don't think this legacy interface does harm
> the subsystem in any way / does prevent the subsystem from moving forward.
> 
> Can't we just keep it as it is?

Well, drm_encoder_slave is a part of the DRM KMS helpers module, so it
take (a little bit) of space on every system. The nouveau situation
isn't unique, other drivers (i915, ast) also incorporate the code for
I2C backends. For the further discussion see the thread starting from
Laurent's email ([1]).

[1] https://lore.kernel.org/all/20241117205426.GE12409@pendragon.ideasonboard.com/

> 
> > 
> > Ideally those two drivers should be converted to the drm_bridge
> > interface, but it's unclear if it's worth spending time on that.
> 
> Probably not.
> 
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> > Changes in v2:
> > - Renamed symbols in defconfig (Laurent)
> > - Added missing Kbuild file (Laurent, LKP)
> > - Renamed guarding defines in include files.
> > - Dropped mentions of two removed functions.
> > - Link to v1: https://lore.kernel.org/r/20241214-nouveau-encoder-slave-v1-0-beda767472e3@linaro.org
> > 
> > ---
> > Dmitry Baryshkov (2):
> >       drm/nouveau: incorporate I2C TV encoder drivers
> >       drm/nouveau: vendor in drm_encoder_slave API
> > 
> >  arch/arm/configs/multi_v7_defconfig                |   4 +-
> >  arch/parisc/configs/generic-32bit_defconfig        |   4 +-
> >  arch/parisc/configs/generic-64bit_defconfig        |   4 +-
> >  drivers/gpu/drm/Makefile                           |   1 -
> >  drivers/gpu/drm/i2c/Kconfig                        |  18 ----
> >  drivers/gpu/drm/i2c/Makefile                       |   6 --
> >  drivers/gpu/drm/nouveau/Kconfig                    |  20 ++++
> >  drivers/gpu/drm/nouveau/dispnv04/Kbuild            |   3 +
> >  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  12 +--
> >  drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild        |   5 +
> >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_drv.c    |  30 +++---
> >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_mode.c   |   8 +-
> >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_priv.h   |  11 ++-
> >  .../drm/{ => nouveau/dispnv04}/i2c/sil164_drv.c    |  33 ++++---
> >  .../dispnv04/nouveau_i2c_encoder.c}                |  85 +++++-----------
> >  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  20 ++--
> >  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |   4 +-
> >  .../gpu/drm/nouveau/include}/i2c/ch7006.h          |   4 +-
> >  .../gpu/drm/nouveau/include/i2c/encoder_i2c.h      | 109 ++++++++-------------
> >  .../gpu/drm/nouveau/include}/i2c/sil164.h          |   4 +-
> >  drivers/gpu/drm/nouveau/nouveau_connector.c        |   6 +-
> >  drivers/gpu/drm/nouveau/nouveau_encoder.h          |  13 +--
> >  22 files changed, 172 insertions(+), 232 deletions(-)
> > ---
> > base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f
> > change-id: 20241214-nouveau-encoder-slave-a6dd422fa4a9
> > 
> > Best regards,
> > -- 
> > Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > 

-- 
With best wishes
Dmitry

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

* Re: [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface
  2024-12-16 12:11   ` Dmitry Baryshkov
@ 2024-12-16 12:16     ` Laurent Pinchart
  2024-12-16 12:41       ` Danilo Krummrich
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2024-12-16 12:16 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Karol Herbst,
	Lyude Paul, dri-devel, linux-kernel, nouveau

On Mon, Dec 16, 2024 at 02:11:41PM +0200, Dmitry Baryshkov wrote:
> On Mon, Dec 16, 2024 at 12:45:15PM +0100, Danilo Krummrich wrote:
> > On Sun, Dec 15, 2024 at 12:19:22PM +0200, Dmitry Baryshkov wrote:
> > > The nouveau driver is the only user of the drm_encoder_slave interface.
> > > Demote it from KMS helpers module to the nouveau driver itself, moving
> > > corresponding I2C encoders to be handled by nouveau driver too.
> > 
> > I understand nouveau is the only driver using this interface (and the
> > corresponding i2c encoders).
> > 
> > However, I'm not quite seeing the advantage of folding the interface (including
> > the two i2c drivers) into nouveau. I don't think this legacy interface does harm
> > the subsystem in any way / does prevent the subsystem from moving forward.
> > 
> > Can't we just keep it as it is?
> 
> Well, drm_encoder_slave is a part of the DRM KMS helpers module, so it
> take (a little bit) of space on every system. The nouveau situation
> isn't unique, other drivers (i915, ast) also incorporate the code for
> I2C backends. For the further discussion see the thread starting from
> Laurent's email ([1]).
> 
> [1] https://lore.kernel.org/all/20241117205426.GE12409@pendragon.ideasonboard.com/

It's also a question of whether maintenance of this code based used by
the nouveau driver only should be the responsibility of the drm-misc
community or the nouveau driver maintainers.

> > > Ideally those two drivers should be converted to the drm_bridge
> > > interface, but it's unclear if it's worth spending time on that.
> > 
> > Probably not.
> > 
> > > 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > > Changes in v2:
> > > - Renamed symbols in defconfig (Laurent)
> > > - Added missing Kbuild file (Laurent, LKP)
> > > - Renamed guarding defines in include files.
> > > - Dropped mentions of two removed functions.
> > > - Link to v1: https://lore.kernel.org/r/20241214-nouveau-encoder-slave-v1-0-beda767472e3@linaro.org
> > > 
> > > ---
> > > Dmitry Baryshkov (2):
> > >       drm/nouveau: incorporate I2C TV encoder drivers
> > >       drm/nouveau: vendor in drm_encoder_slave API
> > > 
> > >  arch/arm/configs/multi_v7_defconfig                |   4 +-
> > >  arch/parisc/configs/generic-32bit_defconfig        |   4 +-
> > >  arch/parisc/configs/generic-64bit_defconfig        |   4 +-
> > >  drivers/gpu/drm/Makefile                           |   1 -
> > >  drivers/gpu/drm/i2c/Kconfig                        |  18 ----
> > >  drivers/gpu/drm/i2c/Makefile                       |   6 --
> > >  drivers/gpu/drm/nouveau/Kconfig                    |  20 ++++
> > >  drivers/gpu/drm/nouveau/dispnv04/Kbuild            |   3 +
> > >  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  12 +--
> > >  drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild        |   5 +
> > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_drv.c    |  30 +++---
> > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_mode.c   |   8 +-
> > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_priv.h   |  11 ++-
> > >  .../drm/{ => nouveau/dispnv04}/i2c/sil164_drv.c    |  33 ++++---
> > >  .../dispnv04/nouveau_i2c_encoder.c}                |  85 +++++-----------
> > >  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  20 ++--
> > >  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |   4 +-
> > >  .../gpu/drm/nouveau/include}/i2c/ch7006.h          |   4 +-
> > >  .../gpu/drm/nouveau/include/i2c/encoder_i2c.h      | 109 ++++++++-------------
> > >  .../gpu/drm/nouveau/include}/i2c/sil164.h          |   4 +-
> > >  drivers/gpu/drm/nouveau/nouveau_connector.c        |   6 +-
> > >  drivers/gpu/drm/nouveau/nouveau_encoder.h          |  13 +--
> > >  22 files changed, 172 insertions(+), 232 deletions(-)
> > > ---
> > > base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f
> > > change-id: 20241214-nouveau-encoder-slave-a6dd422fa4a9

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface
  2024-12-16 12:16     ` Laurent Pinchart
@ 2024-12-16 12:41       ` Danilo Krummrich
  2024-12-16 12:58         ` Dmitry Baryshkov
  0 siblings, 1 reply; 12+ messages in thread
From: Danilo Krummrich @ 2024-12-16 12:41 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Dmitry Baryshkov, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Karol Herbst,
	Lyude Paul, dri-devel, linux-kernel, nouveau

On Mon, Dec 16, 2024 at 02:16:51PM +0200, Laurent Pinchart wrote:
> On Mon, Dec 16, 2024 at 02:11:41PM +0200, Dmitry Baryshkov wrote:
> > On Mon, Dec 16, 2024 at 12:45:15PM +0100, Danilo Krummrich wrote:
> > > On Sun, Dec 15, 2024 at 12:19:22PM +0200, Dmitry Baryshkov wrote:
> > > > The nouveau driver is the only user of the drm_encoder_slave interface.
> > > > Demote it from KMS helpers module to the nouveau driver itself, moving
> > > > corresponding I2C encoders to be handled by nouveau driver too.
> > > 
> > > I understand nouveau is the only driver using this interface (and the
> > > corresponding i2c encoders).
> > > 
> > > However, I'm not quite seeing the advantage of folding the interface (including
> > > the two i2c drivers) into nouveau. I don't think this legacy interface does harm
> > > the subsystem in any way / does prevent the subsystem from moving forward.
> > > 
> > > Can't we just keep it as it is?
> > 
> > Well, drm_encoder_slave is a part of the DRM KMS helpers module, so it
> > take (a little bit) of space on every system. The nouveau situation
> > isn't unique, other drivers (i915, ast) also incorporate the code for
> > I2C backends. For the further discussion see the thread starting from
> > Laurent's email ([1]).
> > 
> > [1] https://lore.kernel.org/all/20241117205426.GE12409@pendragon.ideasonboard.com/

The drm_encoder_slave code it's rather small, but I guess this can be used as
argument for both, keeping it where it is and moving it.

If you want to move it to nouveau, I'm not going to object. But please fold the
helper code, such that we aren't left with unused functions and unnecessary
function pointer indirections through struct drm_encoder_slave_funcs.

> 
> It's also a question of whether maintenance of this code based used by
> the nouveau driver only should be the responsibility of the drm-misc
> community or the nouveau driver maintainers.

Good question. It's common infrastructure; do we expect / require the last user
of such infrastructure to take ownership?

> 
> > > > Ideally those two drivers should be converted to the drm_bridge
> > > > interface, but it's unclear if it's worth spending time on that.
> > > 
> > > Probably not.
> > > 
> > > > 
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > ---
> > > > Changes in v2:
> > > > - Renamed symbols in defconfig (Laurent)
> > > > - Added missing Kbuild file (Laurent, LKP)
> > > > - Renamed guarding defines in include files.
> > > > - Dropped mentions of two removed functions.
> > > > - Link to v1: https://lore.kernel.org/r/20241214-nouveau-encoder-slave-v1-0-beda767472e3@linaro.org
> > > > 
> > > > ---
> > > > Dmitry Baryshkov (2):
> > > >       drm/nouveau: incorporate I2C TV encoder drivers
> > > >       drm/nouveau: vendor in drm_encoder_slave API
> > > > 
> > > >  arch/arm/configs/multi_v7_defconfig                |   4 +-
> > > >  arch/parisc/configs/generic-32bit_defconfig        |   4 +-
> > > >  arch/parisc/configs/generic-64bit_defconfig        |   4 +-
> > > >  drivers/gpu/drm/Makefile                           |   1 -
> > > >  drivers/gpu/drm/i2c/Kconfig                        |  18 ----
> > > >  drivers/gpu/drm/i2c/Makefile                       |   6 --
> > > >  drivers/gpu/drm/nouveau/Kconfig                    |  20 ++++
> > > >  drivers/gpu/drm/nouveau/dispnv04/Kbuild            |   3 +
> > > >  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  12 +--
> > > >  drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild        |   5 +
> > > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_drv.c    |  30 +++---
> > > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_mode.c   |   8 +-
> > > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_priv.h   |  11 ++-
> > > >  .../drm/{ => nouveau/dispnv04}/i2c/sil164_drv.c    |  33 ++++---
> > > >  .../dispnv04/nouveau_i2c_encoder.c}                |  85 +++++-----------
> > > >  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  20 ++--
> > > >  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |   4 +-
> > > >  .../gpu/drm/nouveau/include}/i2c/ch7006.h          |   4 +-
> > > >  .../gpu/drm/nouveau/include/i2c/encoder_i2c.h      | 109 ++++++++-------------
> > > >  .../gpu/drm/nouveau/include}/i2c/sil164.h          |   4 +-
> > > >  drivers/gpu/drm/nouveau/nouveau_connector.c        |   6 +-
> > > >  drivers/gpu/drm/nouveau/nouveau_encoder.h          |  13 +--
> > > >  22 files changed, 172 insertions(+), 232 deletions(-)
> > > > ---
> > > > base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f
> > > > change-id: 20241214-nouveau-encoder-slave-a6dd422fa4a9
> 
> -- 
> Regards,
> 
> Laurent Pinchart

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

* Re: [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface
  2024-12-16 12:41       ` Danilo Krummrich
@ 2024-12-16 12:58         ` Dmitry Baryshkov
  2024-12-16 13:30           ` Danilo Krummrich
  0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Baryshkov @ 2024-12-16 12:58 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Karol Herbst,
	Lyude Paul, dri-devel, linux-kernel, nouveau

On Mon, Dec 16, 2024 at 01:41:56PM +0100, Danilo Krummrich wrote:
> On Mon, Dec 16, 2024 at 02:16:51PM +0200, Laurent Pinchart wrote:
> > On Mon, Dec 16, 2024 at 02:11:41PM +0200, Dmitry Baryshkov wrote:
> > > On Mon, Dec 16, 2024 at 12:45:15PM +0100, Danilo Krummrich wrote:
> > > > On Sun, Dec 15, 2024 at 12:19:22PM +0200, Dmitry Baryshkov wrote:
> > > > > The nouveau driver is the only user of the drm_encoder_slave interface.
> > > > > Demote it from KMS helpers module to the nouveau driver itself, moving
> > > > > corresponding I2C encoders to be handled by nouveau driver too.
> > > > 
> > > > I understand nouveau is the only driver using this interface (and the
> > > > corresponding i2c encoders).
> > > > 
> > > > However, I'm not quite seeing the advantage of folding the interface (including
> > > > the two i2c drivers) into nouveau. I don't think this legacy interface does harm
> > > > the subsystem in any way / does prevent the subsystem from moving forward.
> > > > 
> > > > Can't we just keep it as it is?
> > > 
> > > Well, drm_encoder_slave is a part of the DRM KMS helpers module, so it
> > > take (a little bit) of space on every system. The nouveau situation
> > > isn't unique, other drivers (i915, ast) also incorporate the code for
> > > I2C backends. For the further discussion see the thread starting from
> > > Laurent's email ([1]).
> > > 
> > > [1] https://lore.kernel.org/all/20241117205426.GE12409@pendragon.ideasonboard.com/
> 
> The drm_encoder_slave code it's rather small, but I guess this can be used as
> argument for both, keeping it where it is and moving it.
> 
> If you want to move it to nouveau, I'm not going to object. But please fold the
> helper code, such that we aren't left with unused functions and unnecessary
> function pointer indirections through struct drm_encoder_slave_funcs.

This is more or less what I've done. Or would you prefer to keep the
wrapping functions that just execute the callback? I can change the
patchset accordingly.

> 
> > 
> > It's also a question of whether maintenance of this code based used by
> > the nouveau driver only should be the responsibility of the drm-misc
> > community or the nouveau driver maintainers.
> 
> Good question. It's common infrastructure; do we expect / require the last user
> of such infrastructure to take ownership?

Unfortunately it's more like 'the only one' :-( In other words, if we
were expecting other users, there would not be such a move. But
hopefully all new drivers will use bridges infrastructure.

> 
> > 
> > > > > Ideally those two drivers should be converted to the drm_bridge
> > > > > interface, but it's unclear if it's worth spending time on that.
> > > > 
> > > > Probably not.
> > > > 
> > > > > 
> > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > ---
> > > > > Changes in v2:
> > > > > - Renamed symbols in defconfig (Laurent)
> > > > > - Added missing Kbuild file (Laurent, LKP)
> > > > > - Renamed guarding defines in include files.
> > > > > - Dropped mentions of two removed functions.
> > > > > - Link to v1: https://lore.kernel.org/r/20241214-nouveau-encoder-slave-v1-0-beda767472e3@linaro.org
> > > > > 
> > > > > ---
> > > > > Dmitry Baryshkov (2):
> > > > >       drm/nouveau: incorporate I2C TV encoder drivers
> > > > >       drm/nouveau: vendor in drm_encoder_slave API
> > > > > 
> > > > >  arch/arm/configs/multi_v7_defconfig                |   4 +-
> > > > >  arch/parisc/configs/generic-32bit_defconfig        |   4 +-
> > > > >  arch/parisc/configs/generic-64bit_defconfig        |   4 +-
> > > > >  drivers/gpu/drm/Makefile                           |   1 -
> > > > >  drivers/gpu/drm/i2c/Kconfig                        |  18 ----
> > > > >  drivers/gpu/drm/i2c/Makefile                       |   6 --
> > > > >  drivers/gpu/drm/nouveau/Kconfig                    |  20 ++++
> > > > >  drivers/gpu/drm/nouveau/dispnv04/Kbuild            |   3 +
> > > > >  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  12 +--
> > > > >  drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild        |   5 +
> > > > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_drv.c    |  30 +++---
> > > > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_mode.c   |   8 +-
> > > > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_priv.h   |  11 ++-
> > > > >  .../drm/{ => nouveau/dispnv04}/i2c/sil164_drv.c    |  33 ++++---
> > > > >  .../dispnv04/nouveau_i2c_encoder.c}                |  85 +++++-----------
> > > > >  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  20 ++--
> > > > >  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |   4 +-
> > > > >  .../gpu/drm/nouveau/include}/i2c/ch7006.h          |   4 +-
> > > > >  .../gpu/drm/nouveau/include/i2c/encoder_i2c.h      | 109 ++++++++-------------
> > > > >  .../gpu/drm/nouveau/include}/i2c/sil164.h          |   4 +-
> > > > >  drivers/gpu/drm/nouveau/nouveau_connector.c        |   6 +-
> > > > >  drivers/gpu/drm/nouveau/nouveau_encoder.h          |  13 +--
> > > > >  22 files changed, 172 insertions(+), 232 deletions(-)
> > > > > ---
> > > > > base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f
> > > > > change-id: 20241214-nouveau-encoder-slave-a6dd422fa4a9
> > 
> > -- 
> > Regards,
> > 
> > Laurent Pinchart

-- 
With best wishes
Dmitry

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

* Re: [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface
  2024-12-16 12:58         ` Dmitry Baryshkov
@ 2024-12-16 13:30           ` Danilo Krummrich
  2024-12-16 14:15             ` Dmitry Baryshkov
  0 siblings, 1 reply; 12+ messages in thread
From: Danilo Krummrich @ 2024-12-16 13:30 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Karol Herbst,
	Lyude Paul, dri-devel, linux-kernel, nouveau

On Mon, Dec 16, 2024 at 02:58:59PM +0200, Dmitry Baryshkov wrote:
> On Mon, Dec 16, 2024 at 01:41:56PM +0100, Danilo Krummrich wrote:
> > On Mon, Dec 16, 2024 at 02:16:51PM +0200, Laurent Pinchart wrote:
> > > On Mon, Dec 16, 2024 at 02:11:41PM +0200, Dmitry Baryshkov wrote:
> > > > On Mon, Dec 16, 2024 at 12:45:15PM +0100, Danilo Krummrich wrote:
> > > > > On Sun, Dec 15, 2024 at 12:19:22PM +0200, Dmitry Baryshkov wrote:
> > > > > > The nouveau driver is the only user of the drm_encoder_slave interface.
> > > > > > Demote it from KMS helpers module to the nouveau driver itself, moving
> > > > > > corresponding I2C encoders to be handled by nouveau driver too.
> > > > > 
> > > > > I understand nouveau is the only driver using this interface (and the
> > > > > corresponding i2c encoders).
> > > > > 
> > > > > However, I'm not quite seeing the advantage of folding the interface (including
> > > > > the two i2c drivers) into nouveau. I don't think this legacy interface does harm
> > > > > the subsystem in any way / does prevent the subsystem from moving forward.
> > > > > 
> > > > > Can't we just keep it as it is?
> > > > 
> > > > Well, drm_encoder_slave is a part of the DRM KMS helpers module, so it
> > > > take (a little bit) of space on every system. The nouveau situation
> > > > isn't unique, other drivers (i915, ast) also incorporate the code for
> > > > I2C backends. For the further discussion see the thread starting from
> > > > Laurent's email ([1]).
> > > > 
> > > > [1] https://lore.kernel.org/all/20241117205426.GE12409@pendragon.ideasonboard.com/
> > 
> > The drm_encoder_slave code it's rather small, but I guess this can be used as
> > argument for both, keeping it where it is and moving it.
> > 
> > If you want to move it to nouveau, I'm not going to object. But please fold the
> > helper code, such that we aren't left with unused functions and unnecessary
> > function pointer indirections through struct drm_encoder_slave_funcs.
> 
> This is more or less what I've done. Or would you prefer to keep the
> wrapping functions that just execute the callback? I can change the
> patchset accordingly.

No, I think it's good indeed -- st a first glance it looked like there's more to
get rid of.

There are just a few more nits, I'll go ahead and add comments in the
corresponding patches.

> 
> > 
> > > 
> > > It's also a question of whether maintenance of this code based used by
> > > the nouveau driver only should be the responsibility of the drm-misc
> > > community or the nouveau driver maintainers.
> > 
> > Good question. It's common infrastructure; do we expect / require the last user
> > of such infrastructure to take ownership?
> 
> Unfortunately it's more like 'the only one' :-( In other words, if we

I can't see a major difference between "last one" and "only one" in this
context.

> were expecting other users, there would not be such a move. But
> hopefully all new drivers will use bridges infrastructure.

Agreed, but I don't think it answers my question.

> 
> > 
> > > 
> > > > > > Ideally those two drivers should be converted to the drm_bridge
> > > > > > interface, but it's unclear if it's worth spending time on that.
> > > > > 
> > > > > Probably not.
> > > > > 
> > > > > > 
> > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > ---
> > > > > > Changes in v2:
> > > > > > - Renamed symbols in defconfig (Laurent)
> > > > > > - Added missing Kbuild file (Laurent, LKP)
> > > > > > - Renamed guarding defines in include files.
> > > > > > - Dropped mentions of two removed functions.
> > > > > > - Link to v1: https://lore.kernel.org/r/20241214-nouveau-encoder-slave-v1-0-beda767472e3@linaro.org
> > > > > > 
> > > > > > ---
> > > > > > Dmitry Baryshkov (2):
> > > > > >       drm/nouveau: incorporate I2C TV encoder drivers
> > > > > >       drm/nouveau: vendor in drm_encoder_slave API
> > > > > > 
> > > > > >  arch/arm/configs/multi_v7_defconfig                |   4 +-
> > > > > >  arch/parisc/configs/generic-32bit_defconfig        |   4 +-
> > > > > >  arch/parisc/configs/generic-64bit_defconfig        |   4 +-
> > > > > >  drivers/gpu/drm/Makefile                           |   1 -
> > > > > >  drivers/gpu/drm/i2c/Kconfig                        |  18 ----
> > > > > >  drivers/gpu/drm/i2c/Makefile                       |   6 --
> > > > > >  drivers/gpu/drm/nouveau/Kconfig                    |  20 ++++
> > > > > >  drivers/gpu/drm/nouveau/dispnv04/Kbuild            |   3 +
> > > > > >  drivers/gpu/drm/nouveau/dispnv04/dfp.c             |  12 +--
> > > > > >  drivers/gpu/drm/nouveau/dispnv04/i2c/Kbuild        |   5 +
> > > > > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_drv.c    |  30 +++---
> > > > > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_mode.c   |   8 +-
> > > > > >  .../drm/{ => nouveau/dispnv04}/i2c/ch7006_priv.h   |  11 ++-
> > > > > >  .../drm/{ => nouveau/dispnv04}/i2c/sil164_drv.c    |  33 ++++---
> > > > > >  .../dispnv04/nouveau_i2c_encoder.c}                |  85 +++++-----------
> > > > > >  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c          |  20 ++--
> > > > > >  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c          |   4 +-
> > > > > >  .../gpu/drm/nouveau/include}/i2c/ch7006.h          |   4 +-
> > > > > >  .../gpu/drm/nouveau/include/i2c/encoder_i2c.h      | 109 ++++++++-------------
> > > > > >  .../gpu/drm/nouveau/include}/i2c/sil164.h          |   4 +-
> > > > > >  drivers/gpu/drm/nouveau/nouveau_connector.c        |   6 +-
> > > > > >  drivers/gpu/drm/nouveau/nouveau_encoder.h          |  13 +--
> > > > > >  22 files changed, 172 insertions(+), 232 deletions(-)
> > > > > > ---
> > > > > > base-commit: 4176cf5c5651c33769de83bb61b0287f4ec7719f
> > > > > > change-id: 20241214-nouveau-encoder-slave-a6dd422fa4a9
> > > 
> > > -- 
> > > Regards,
> > > 
> > > Laurent Pinchart
> 
> -- 
> With best wishes
> Dmitry

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

* Re: [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface
  2024-12-16 13:30           ` Danilo Krummrich
@ 2024-12-16 14:15             ` Dmitry Baryshkov
  0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2024-12-16 14:15 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Karol Herbst,
	Lyude Paul, dri-devel, linux-kernel, nouveau

On Mon, Dec 16, 2024 at 02:30:55PM +0100, Danilo Krummrich wrote:
> On Mon, Dec 16, 2024 at 02:58:59PM +0200, Dmitry Baryshkov wrote:
> > On Mon, Dec 16, 2024 at 01:41:56PM +0100, Danilo Krummrich wrote:
> > > On Mon, Dec 16, 2024 at 02:16:51PM +0200, Laurent Pinchart wrote:
> > > > On Mon, Dec 16, 2024 at 02:11:41PM +0200, Dmitry Baryshkov wrote:
> > > > > On Mon, Dec 16, 2024 at 12:45:15PM +0100, Danilo Krummrich wrote:
> > > > > > On Sun, Dec 15, 2024 at 12:19:22PM +0200, Dmitry Baryshkov wrote:
> > > > > > > The nouveau driver is the only user of the drm_encoder_slave interface.
> > > > > > > Demote it from KMS helpers module to the nouveau driver itself, moving
> > > > > > > corresponding I2C encoders to be handled by nouveau driver too.
> > > > > > 
> > > > > > I understand nouveau is the only driver using this interface (and the
> > > > > > corresponding i2c encoders).
> > > > > > 
> > > > > > However, I'm not quite seeing the advantage of folding the interface (including
> > > > > > the two i2c drivers) into nouveau. I don't think this legacy interface does harm
> > > > > > the subsystem in any way / does prevent the subsystem from moving forward.
> > > > > > 
> > > > > > Can't we just keep it as it is?
> > > > > 
> > > > > Well, drm_encoder_slave is a part of the DRM KMS helpers module, so it
> > > > > take (a little bit) of space on every system. The nouveau situation
> > > > > isn't unique, other drivers (i915, ast) also incorporate the code for
> > > > > I2C backends. For the further discussion see the thread starting from
> > > > > Laurent's email ([1]).
> > > > > 
> > > > > [1] https://lore.kernel.org/all/20241117205426.GE12409@pendragon.ideasonboard.com/
> > > 
> > > The drm_encoder_slave code it's rather small, but I guess this can be used as
> > > argument for both, keeping it where it is and moving it.
> > > 
> > > If you want to move it to nouveau, I'm not going to object. But please fold the
> > > helper code, such that we aren't left with unused functions and unnecessary
> > > function pointer indirections through struct drm_encoder_slave_funcs.
> > 
> > This is more or less what I've done. Or would you prefer to keep the
> > wrapping functions that just execute the callback? I can change the
> > patchset accordingly.
> 
> No, I think it's good indeed -- st a first glance it looked like there's more to
> get rid of.
> 
> There are just a few more nits, I'll go ahead and add comments in the
> corresponding patches.
> 
> > 
> > > 
> > > > 
> > > > It's also a question of whether maintenance of this code based used by
> > > > the nouveau driver only should be the responsibility of the drm-misc
> > > > community or the nouveau driver maintainers.
> > > 
> > > Good question. It's common infrastructure; do we expect / require the last user
> > > of such infrastructure to take ownership?
> > 
> > Unfortunately it's more like 'the only one' :-( In other words, if we
> 
> I can't see a major difference between "last one" and "only one" in this
> context.

I was thinkinkg between "the last one" and "the only one, there will be
more in future". Excuse me.

> 
> > were expecting other users, there would not be such a move. But
> > hopefully all new drivers will use bridges infrastructure.
> 
> Agreed, but I don't think it answers my question.

Anyway, yes (in my humble opinion). it's usually easier to push bits of
infrasrucure towards the last user. Frequently it allows us to simplify
both the framework and the last user.

-- 
With best wishes
Dmitry

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

end of thread, other threads:[~2024-12-16 14:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-15 10:19 [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface Dmitry Baryshkov
2024-12-15 10:19 ` [PATCH v2 1/2] drm/nouveau: incorporate I2C TV encoder drivers Dmitry Baryshkov
2024-12-16 13:43   ` Danilo Krummrich
2024-12-15 10:19 ` [PATCH v2 2/2] drm/nouveau: vendor in drm_encoder_slave API Dmitry Baryshkov
2024-12-16 13:50   ` Danilo Krummrich
2024-12-16 11:45 ` [PATCH v2 0/2] drm/nouveau: remove drm_encoder_slave interface Danilo Krummrich
2024-12-16 12:11   ` Dmitry Baryshkov
2024-12-16 12:16     ` Laurent Pinchart
2024-12-16 12:41       ` Danilo Krummrich
2024-12-16 12:58         ` Dmitry Baryshkov
2024-12-16 13:30           ` Danilo Krummrich
2024-12-16 14:15             ` Dmitry Baryshkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox