[PATCH 0/8] drm: writeback: clean up writeback connector initialization

[PATCH 0/8] drm: writeback: clean up writeback connector initialization

[Dmitry Baryshkov]

Drivers using drm_writeback_connector_init() / _with_encoder() don't
perform cleanup in a manner similar to drmm_writeback_connector_init()
(see drm_writeback_connector_cleanup()). Migrate all existing drivers
to use drmm_writeback_connector_init(), drop
drm_writeback_connector_init() and drm_writeback_connector::encoder
(it's unused afterwards).

This series leaves former drm_writeback_connector_init_with_encoder()
(renamed to drm_writeback_connector_init as a non-managed counterpart
for drmm_writeback_connector_init()). It is supposed to be used by
drivers which can not use drmm functions (like Intel). However I think
it would be better to drop it completely.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Dmitry Baryshkov (8):
      drm/amd/display: use drmm_writeback_connector_init()
      drm/komeda: use drmm_writeback_connector_init()
      drm/mali: use drmm_writeback_connector_init()
      drm/msm/dpu: use drmm_writeback_connector_init()
      drm/msm/dpu: use drmm_writeback_connector_init()
      drm/vc4: use drmm_writeback_connector_init()
      drm: writeback: drop excess connector initialization functions
      drm: writeback: rename drm_writeback_connector_init_with_encoder()

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  2 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c   | 18 ++++--
 .../drm/arm/display/komeda/komeda_wb_connector.c   | 30 ++++++----
 drivers/gpu/drm/arm/malidp_mw.c                    | 25 ++++----
 drivers/gpu/drm/drm_writeback.c                    | 69 +++-------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c      | 10 +---
 .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c    | 23 +++++---
 drivers/gpu/drm/vc4/vc4_txp.c                      |  9 ++-
 include/drm/drm_writeback.h                        | 22 +------
 9 files changed, 77 insertions(+), 131 deletions(-)
---
base-commit: 94f208ff622b09309358abaf26d7acca0c318fae
change-id: 20250801-wb-drop-encoder-97a0c75bd5d7

Best regards,
-- 
With best wishes
Dmitry

[PATCH 1/8] drm/amd/display: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c    |  2 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c | 18 +++++++++++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 096b23ad4845d365dad707deed4e3e84b1f9945d..a9948e8d67a6e9556d21090b5cdd101258d62480 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10003,7 +10003,7 @@ static void dm_set_writeback(struct amdgpu_display_manager *dm,
 		return;
 	}
 
-	acrtc = to_amdgpu_crtc(wb_conn->encoder.crtc);
+	acrtc = to_amdgpu_crtc(crtc_state->base.crtc);
 	if (!acrtc) {
 		drm_err(adev_to_drm(adev), "no amdgpu_crtc found\n");
 		kfree(wb_info);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
index d9527c05fc878ef6916782ff9fde847813938461..80c37487ca77c0494eaf769c5b9a3c53486aa52d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
@@ -171,7 +171,6 @@ static const struct drm_encoder_helper_funcs amdgpu_dm_wb_encoder_helper_funcs =
 
 static const struct drm_connector_funcs amdgpu_dm_wb_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
 	.reset = amdgpu_dm_connector_funcs_reset,
 	.atomic_duplicate_state = amdgpu_dm_connector_atomic_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -190,17 +189,26 @@ int amdgpu_dm_wb_connector_init(struct amdgpu_display_manager *dm,
 	struct dc *dc = dm->dc;
 	struct dc_link *link = dc_get_link_at_index(dc, link_index);
 	int res = 0;
+	struct drm_encoder *encoder;
+
+	encoder = drmm_plain_encoder_alloc(&dm->adev->ddev, NULL,
+					   DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (IS_ERR(encoder))
+		return PTR_ERR(encoder);
+
+	drm_encoder_helper_add(encoder, &amdgpu_dm_wb_encoder_helper_funcs);
+
+	encoder->possible_crtcs = amdgpu_dm_get_encoder_crtc_mask(dm->adev);
 
 	wbcon->link = link;
 
 	drm_connector_helper_add(&wbcon->base.base, &amdgpu_dm_wb_conn_helper_funcs);
 
-	res = drm_writeback_connector_init(&dm->adev->ddev, &wbcon->base,
+	res = drmm_writeback_connector_init(&dm->adev->ddev, &wbcon->base,
 					    &amdgpu_dm_wb_connector_funcs,
-					    &amdgpu_dm_wb_encoder_helper_funcs,
+					    encoder,
 					    amdgpu_dm_wb_formats,
-					    ARRAY_SIZE(amdgpu_dm_wb_formats),
-					    amdgpu_dm_get_encoder_crtc_mask(dm->adev));
+					    ARRAY_SIZE(amdgpu_dm_wb_formats));
 
 	if (res)
 		return res;

-- 
2.39.5

[PATCH 2/8] drm/komeda: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 .../drm/arm/display/komeda/komeda_wb_connector.c   | 30 ++++++++++++----------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index 875cdbff18c9dc97e736049feaa8a1fe1bcc82ff..210841b4453abfe60cecc1cfc79a84298be3e684 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -5,6 +5,7 @@
  *
  */
 #include <drm/drm_framebuffer.h>
+#include <drm/drm_managed.h>
 #include "komeda_dev.h"
 #include "komeda_kms.h"
 
@@ -121,17 +122,10 @@ komeda_wb_connector_fill_modes(struct drm_connector *connector,
 	return 0;
 }
 
-static void komeda_wb_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_cleanup(connector);
-	kfree(to_kconn(to_wb_conn(connector)));
-}
-
 static const struct drm_connector_funcs komeda_wb_connector_funcs = {
 	.reset			= drm_atomic_helper_connector_reset,
 	.detect			= komeda_wb_connector_detect,
 	.fill_modes		= komeda_wb_connector_fill_modes,
-	.destroy		= komeda_wb_connector_destroy,
 	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
 };
@@ -143,13 +137,15 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
 	struct komeda_wb_connector *kwb_conn;
 	struct drm_writeback_connector *wb_conn;
 	struct drm_display_info *info;
+	struct drm_encoder *encoder;
+
 	u32 *formats, n_formats = 0;
 	int err;
 
 	if (!kcrtc->master->wb_layer)
 		return 0;
 
-	kwb_conn = kzalloc(sizeof(*kwb_conn), GFP_KERNEL);
+	kwb_conn = drmm_kzalloc(&kms->base, sizeof(*kwb_conn), GFP_KERNEL);
 	if (!kwb_conn)
 		return -ENOMEM;
 
@@ -165,11 +161,19 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
 		return -ENOMEM;
 	}
 
-	err = drm_writeback_connector_init(&kms->base, wb_conn,
-					   &komeda_wb_connector_funcs,
-					   &komeda_wb_encoder_helper_funcs,
-					   formats, n_formats,
-					   BIT(drm_crtc_index(&kcrtc->base)));
+	encoder = drmm_plain_encoder_alloc(&kms->base, NULL,
+					   DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (IS_ERR(encoder))
+		return PTR_ERR(encoder);
+
+	drm_encoder_helper_add(encoder, &komeda_wb_encoder_helper_funcs);
+
+	encoder->possible_crtcs = BIT(drm_crtc_index(&kcrtc->base));
+
+	err = drmm_writeback_connector_init(&kms->base, wb_conn,
+					    &komeda_wb_connector_funcs,
+					    encoder,
+					    formats, n_formats);
 	komeda_put_fourcc_list(formats);
 	if (err) {
 		kfree(kwb_conn);

-- 
2.39.5

[PATCH 3/8] drm/mali: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/arm/malidp_mw.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index 600af5ad81b15d0c30f9f79588f40cca07437ed8..182275c0c29cd8527c85dfb3e7317561da392c46 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -84,11 +84,6 @@ malidp_mw_connector_detect(struct drm_connector *connector, bool force)
 	return connector_status_connected;
 }
 
-static void malidp_mw_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_cleanup(connector);
-}
-
 static struct drm_connector_state *
 malidp_mw_connector_duplicate_state(struct drm_connector *connector)
 {
@@ -114,7 +109,6 @@ static const struct drm_connector_funcs malidp_mw_connector_funcs = {
 	.reset = malidp_mw_connector_reset,
 	.detect = malidp_mw_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = malidp_mw_connector_destroy,
 	.atomic_duplicate_state = malidp_mw_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
@@ -211,6 +205,7 @@ static u32 *get_writeback_formats(struct malidp_drm *malidp, int *n_formats)
 int malidp_mw_connector_init(struct drm_device *drm)
 {
 	struct malidp_drm *malidp = drm_to_malidp(drm);
+	struct drm_encoder *encoder;
 	u32 *formats;
 	int ret, n_formats;
 
@@ -224,11 +219,19 @@ int malidp_mw_connector_init(struct drm_device *drm)
 	if (!formats)
 		return -ENOMEM;
 
-	ret = drm_writeback_connector_init(drm, &malidp->mw_connector,
-					   &malidp_mw_connector_funcs,
-					   &malidp_mw_encoder_helper_funcs,
-					   formats, n_formats,
-					   1 << drm_crtc_index(&malidp->crtc));
+	encoder = drmm_plain_encoder_alloc(drm, NULL, DRM_MODE_ENCODER_VIRTUAL,
+					   NULL);
+	if (IS_ERR(encoder))
+		return PTR_ERR(encoder);
+
+	drm_encoder_helper_add(encoder, &malidp_mw_encoder_helper_funcs);
+
+	encoder->possible_crtcs = 1 << drm_crtc_index(&malidp->crtc);
+
+	ret = drmm_writeback_connector_init(drm, &malidp->mw_connector,
+					    &malidp_mw_connector_funcs,
+					    encoder,
+					    formats, n_formats);
 	kfree(formats);
 	if (ret)
 		return ret;

-- 
2.39.5

[PATCH 4/8] drm/msm/dpu: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
index 8ff496082902b1ee713e806140f39b4730ed256a..cd73468e369a93c50303db2a7d4499bcb17be5d1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
@@ -80,7 +80,6 @@ static int dpu_wb_conn_atomic_check(struct drm_connector *connector,
 static const struct drm_connector_funcs dpu_wb_conn_funcs = {
 	.reset = drm_atomic_helper_connector_reset,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
@@ -131,12 +130,9 @@ int dpu_writeback_init(struct drm_device *dev, struct drm_encoder *enc,
 
 	drm_connector_helper_add(&dpu_wb_conn->base.base, &dpu_wb_conn_helper_funcs);
 
-	/* DPU initializes the encoder and sets it up completely for writeback
-	 * cases and hence should use the new API drm_writeback_connector_init_with_encoder
-	 * to initialize the writeback connector
-	 */
-	rc = drm_writeback_connector_init_with_encoder(dev, &dpu_wb_conn->base, enc,
-			&dpu_wb_conn_funcs, format_list, num_formats);
+	rc = drmm_writeback_connector_init(dev, &dpu_wb_conn->base,
+					   &dpu_wb_conn_funcs, enc,
+					   format_list, num_formats);
 
 	if (!rc)
 		dpu_wb_conn->wb_enc = enc;

-- 
2.39.5

[PATCH 5/8] drm/msm/dpu: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c    | 23 +++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
index 8cd37d7b8ae281cbc1fd8cbb243c621174517e23..9986a10e8114680e9da48986f4ca3ce6ec66b8cb 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
@@ -134,7 +134,6 @@ static void rcar_du_wb_conn_reset(struct drm_connector *connector)
 static const struct drm_connector_funcs rcar_du_wb_conn_funcs = {
 	.reset = rcar_du_wb_conn_reset,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
 	.atomic_duplicate_state = rcar_du_wb_conn_duplicate_state,
 	.atomic_destroy_state = rcar_du_wb_conn_destroy_state,
 };
@@ -202,15 +201,25 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu,
 {
 	struct drm_writeback_connector *wb_conn = &rcrtc->writeback;
 
+	struct drm_encoder *encoder;
+
+	encoder = drmm_plain_encoder_alloc(&rcdu->ddev, NULL,
+					   DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (IS_ERR(encoder))
+		return PTR_ERR(encoder);
+
+	drm_encoder_helper_add(encoder, &rcar_du_wb_enc_helper_funcs);
+
+	encoder->possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);
+
 	drm_connector_helper_add(&wb_conn->base,
 				 &rcar_du_wb_conn_helper_funcs);
 
-	return drm_writeback_connector_init(&rcdu->ddev, wb_conn,
-					    &rcar_du_wb_conn_funcs,
-					    &rcar_du_wb_enc_helper_funcs,
-					    writeback_formats,
-					    ARRAY_SIZE(writeback_formats),
-					    1 << drm_crtc_index(&rcrtc->crtc));
+	return drmm_writeback_connector_init(&rcdu->ddev, wb_conn,
+					     &rcar_du_wb_conn_funcs,
+					     encoder,
+					     writeback_formats,
+					     ARRAY_SIZE(writeback_formats));
 }
 
 void rcar_du_writeback_setup(struct rcar_du_crtc *rcrtc,

-- 
2.39.5

[PATCH 6/8] drm/vc4: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/vc4/vc4_txp.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index 42acac05fe47861ced226a4f64661f545e21ddb5..fef4e4ee47cf41d40de47c5201f0349dc36c56bd 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -377,7 +377,6 @@ vc4_txp_connector_detect(struct drm_connector *connector, bool force)
 static const struct drm_connector_funcs vc4_txp_connector_funcs = {
 	.detect = vc4_txp_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
 	.reset = drm_atomic_helper_connector_reset,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -601,10 +600,10 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data)
 
 	drm_connector_helper_add(&txp->connector.base,
 				 &vc4_txp_connector_helper_funcs);
-	ret = drm_writeback_connector_init_with_encoder(drm, &txp->connector,
-							encoder,
-							&vc4_txp_connector_funcs,
-							drm_fmts, ARRAY_SIZE(drm_fmts));
+	ret = drmm_writeback_connector_init(drm, &txp->connector,
+					    &vc4_txp_connector_funcs,
+					    encoder,
+					    drm_fmts, ARRAY_SIZE(drm_fmts));
 	if (ret)
 		return ret;
 

-- 
2.39.5

[PATCH 7/8] drm: writeback: drop excess connector initialization functions

[Dmitry Baryshkov]

Now as all drivers have been converted to
drmm_writeback_connector_init(), drop drm_writeback_connector_init() and
drm_writeback_connector::encoder field, they are unused now.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/drm_writeback.c | 55 -----------------------------------------
 include/drm/drm_writeback.h     | 18 --------------
 2 files changed, 73 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 95b8a2e4bda69230591b50be15d14d0b3692373b..1a01df91b2c5868e158d489b782f4c57c61a272c 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -142,61 +142,6 @@ static int create_writeback_properties(struct drm_device *dev)
 	return 0;
 }
 
-static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
-	.destroy = drm_encoder_cleanup,
-};
-
-/**
- * drm_writeback_connector_init - Initialize a writeback connector and its properties
- * @dev: DRM device
- * @wb_connector: Writeback connector to initialize
- * @con_funcs: Connector funcs vtable
- * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
- * @formats: Array of supported pixel formats for the writeback engine
- * @n_formats: Length of the formats array
- * @possible_crtcs: possible crtcs for the internal writeback encoder
- *
- * This function creates the writeback-connector-specific properties if they
- * have not been already created, initializes the connector as
- * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
- * values. It will also create an internal encoder associated with the
- * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
- * the encoder helper.
- *
- * Drivers should always use this function instead of drm_connector_init() to
- * set up writeback connectors.
- *
- * Returns: 0 on success, or a negative error code
- */
-int drm_writeback_connector_init(struct drm_device *dev,
-				 struct drm_writeback_connector *wb_connector,
-				 const struct drm_connector_funcs *con_funcs,
-				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
-				 const u32 *formats, int n_formats,
-				 u32 possible_crtcs)
-{
-	int ret = 0;
-
-	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
-
-	wb_connector->encoder.possible_crtcs = possible_crtcs;
-
-	ret = drm_encoder_init(dev, &wb_connector->encoder,
-			       &drm_writeback_encoder_funcs,
-			       DRM_MODE_ENCODER_VIRTUAL, NULL);
-	if (ret)
-		return ret;
-
-	ret = drm_writeback_connector_init_with_encoder(dev, wb_connector, &wb_connector->encoder,
-			con_funcs, formats, n_formats);
-
-	if (ret)
-		drm_encoder_cleanup(&wb_connector->encoder);
-
-	return ret;
-}
-EXPORT_SYMBOL(drm_writeback_connector_init);
-
 static void delete_writeback_properties(struct drm_device *dev)
 {
 	if (dev->mode_config.writeback_pixel_formats_property) {
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index c380a7b8f55a3616fa070c037d5cc653b0061fe6..879ca103320cc225ffb3687419088361315535fc 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -24,17 +24,6 @@ struct drm_writeback_connector {
 	 */
 	struct drm_connector base;
 
-	/**
-	 * @encoder: Internal encoder used by the connector to fulfill
-	 * the DRM framework requirements. The users of the
-	 * @drm_writeback_connector control the behaviour of the @encoder
-	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
-	 * function.
-	 * For users of drm_writeback_connector_init_with_encoder(), this field
-	 * is not valid as the encoder is managed within their drivers.
-	 */
-	struct drm_encoder encoder;
-
 	/**
 	 * @pixel_formats_blob_ptr:
 	 *
@@ -148,13 +137,6 @@ drm_connector_to_writeback(struct drm_connector *connector)
 	return container_of(connector, struct drm_writeback_connector, base);
 }
 
-int drm_writeback_connector_init(struct drm_device *dev,
-				 struct drm_writeback_connector *wb_connector,
-				 const struct drm_connector_funcs *con_funcs,
-				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
-				 const u32 *formats, int n_formats,
-				 u32 possible_crtcs);
-
 int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
 				struct drm_writeback_connector *wb_connector,
 				struct drm_encoder *enc,

-- 
2.39.5

[PATCH 8/8] drm: writeback: rename drm_writeback_connector_init_with_encoder()

[Dmitry Baryshkov]

Rename drm_writeback_connector_init_with_encoder() to
drm_writeback_connector_init() and adapt its interface to follow
drmm_writeback_connector_init().

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/drm_writeback.c | 14 +++++++-------
 include/drm/drm_writeback.h     | 10 +++++-----
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 1a01df91b2c5868e158d489b782f4c57c61a272c..ec2575c4c21b7449707b0595322e2202a0cf9865 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -235,7 +235,7 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
 }
 
 /**
- * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
+ * drm_writeback_connector_init - Initialize a writeback connector with
  * a custom encoder
  *
  * @dev: DRM device
@@ -263,11 +263,11 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
  *
  * Returns: 0 on success, or a negative error code
  */
-int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
-					      struct drm_writeback_connector *wb_connector,
-					      struct drm_encoder *enc,
-					      const struct drm_connector_funcs *con_funcs,
-					      const u32 *formats, int n_formats)
+int drm_writeback_connector_init(struct drm_device *dev,
+				 struct drm_writeback_connector *wb_connector,
+				 const struct drm_connector_funcs *con_funcs,
+				 struct drm_encoder *enc,
+				 const u32 *formats, int n_formats)
 {
 	struct drm_connector *connector = &wb_connector->base;
 	int ret;
@@ -284,7 +284,7 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
 
 	return ret;
 }
-EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
+EXPORT_SYMBOL(drm_writeback_connector_init);
 
 /**
  * drm_writeback_connector_cleanup - Cleanup the writeback connector
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index 879ca103320cc225ffb3687419088361315535fc..958466a05e604b387722610fc11f9e841316d21b 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -137,11 +137,11 @@ drm_connector_to_writeback(struct drm_connector *connector)
 	return container_of(connector, struct drm_writeback_connector, base);
 }
 
-int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
-				struct drm_writeback_connector *wb_connector,
-				struct drm_encoder *enc,
-				const struct drm_connector_funcs *con_funcs, const u32 *formats,
-				int n_formats);
+int drm_writeback_connector_init(struct drm_device *dev,
+				 struct drm_writeback_connector *wb_connector,
+				 const struct drm_connector_funcs *con_funcs,
+				 struct drm_encoder *enc,
+				 const u32 *formats, int n_formats);
 
 int drmm_writeback_connector_init(struct drm_device *dev,
 				  struct drm_writeback_connector *wb_connector,

-- 
2.39.5

Re: [PATCH 5/8] drm/msm/dpu: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

On Fri, Aug 01, 2025 at 04:51:13PM +0300, Dmitry Baryshkov wrote:
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback
> connector instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c    | 23 +++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> index 8cd37d7b8ae281cbc1fd8cbb243c621174517e23..9986a10e8114680e9da48986f4ca3ce6ec66b8cb 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> @@ -134,7 +134,6 @@ static void rcar_du_wb_conn_reset(struct drm_connector *connector)

And I used wrong subject prefix here... I will fix it for v2 after
getting feedback on the series.


-- 
With best wishes
Dmitry

Re: [PATCH 5/8] drm/msm/dpu: use drmm_writeback_connector_init()

[Geert Uytterhoeven]

s@msm/dpu@renesas/r-car@ in the Subject.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH 8/8] drm: writeback: rename drm_writeback_connector_init_with_encoder()

[Louis Chauvet]

Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> Rename drm_writeback_connector_init_with_encoder() to
> drm_writeback_connector_init() and adapt its interface to follow
> drmm_writeback_connector_init().
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>   drivers/gpu/drm/drm_writeback.c | 14 +++++++-------
>   include/drm/drm_writeback.h     | 10 +++++-----
>   2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index 1a01df91b2c5868e158d489b782f4c57c61a272c..ec2575c4c21b7449707b0595322e2202a0cf9865 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -235,7 +235,7 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
>   }
>   
>   /**
> - * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
> + * drm_writeback_connector_init - Initialize a writeback connector with
>    * a custom encoder

If I understood correctly your series you want to reduce the usage of 
non-drmm writeback, so maybe we can add a comment to direct poeple to 
drmm variant to avoid new usage of this API?

With or without this:

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

>    *
>    * @dev: DRM device
> @@ -263,11 +263,11 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
>    *
>    * Returns: 0 on success, or a negative error code
>    */
> -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> -					      struct drm_writeback_connector *wb_connector,
> -					      struct drm_encoder *enc,
> -					      const struct drm_connector_funcs *con_funcs,
> -					      const u32 *formats, int n_formats)
> +int drm_writeback_connector_init(struct drm_device *dev,
> +				 struct drm_writeback_connector *wb_connector,
> +				 const struct drm_connector_funcs *con_funcs,
> +				 struct drm_encoder *enc,
> +				 const u32 *formats, int n_formats)
>   {
>   	struct drm_connector *connector = &wb_connector->base;
>   	int ret;
> @@ -284,7 +284,7 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
>   
>   	return ret;
>   }
> -EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
> +EXPORT_SYMBOL(drm_writeback_connector_init);
>   
>   /**
>    * drm_writeback_connector_cleanup - Cleanup the writeback connector
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index 879ca103320cc225ffb3687419088361315535fc..958466a05e604b387722610fc11f9e841316d21b 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -137,11 +137,11 @@ drm_connector_to_writeback(struct drm_connector *connector)
>   	return container_of(connector, struct drm_writeback_connector, base);
>   }
>   
> -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> -				struct drm_writeback_connector *wb_connector,
> -				struct drm_encoder *enc,
> -				const struct drm_connector_funcs *con_funcs, const u32 *formats,
> -				int n_formats);
> +int drm_writeback_connector_init(struct drm_device *dev,
> +				 struct drm_writeback_connector *wb_connector,
> +				 const struct drm_connector_funcs *con_funcs,
> +				 struct drm_encoder *enc,
> +				 const u32 *formats, int n_formats);
>   
>   int drmm_writeback_connector_init(struct drm_device *dev,
>   				  struct drm_writeback_connector *wb_connector,
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/8] drm/amd/display: use drmm_writeback_connector_init()

[Louis Chauvet]

Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback
> connector instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c    |  2 +-
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c | 18 +++++++++++++-----
>   2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 096b23ad4845d365dad707deed4e3e84b1f9945d..a9948e8d67a6e9556d21090b5cdd101258d62480 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -10003,7 +10003,7 @@ static void dm_set_writeback(struct amdgpu_display_manager *dm,
>   		return;
>   	}
>   
> -	acrtc = to_amdgpu_crtc(wb_conn->encoder.crtc);
> +	acrtc = to_amdgpu_crtc(crtc_state->base.crtc);
>   	if (!acrtc) {
>   		drm_err(adev_to_drm(adev), "no amdgpu_crtc found\n");
>   		kfree(wb_info);
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
> index d9527c05fc878ef6916782ff9fde847813938461..80c37487ca77c0494eaf769c5b9a3c53486aa52d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
> @@ -171,7 +171,6 @@ static const struct drm_encoder_helper_funcs amdgpu_dm_wb_encoder_helper_funcs =
>   
>   static const struct drm_connector_funcs amdgpu_dm_wb_connector_funcs = {
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = drm_connector_cleanup,
>   	.reset = amdgpu_dm_connector_funcs_reset,
>   	.atomic_duplicate_state = amdgpu_dm_connector_atomic_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> @@ -190,17 +189,26 @@ int amdgpu_dm_wb_connector_init(struct amdgpu_display_manager *dm,
>   	struct dc *dc = dm->dc;
>   	struct dc_link *link = dc_get_link_at_index(dc, link_index);
>   	int res = 0;
> +	struct drm_encoder *encoder;
> +
> +	encoder = drmm_plain_encoder_alloc(&dm->adev->ddev, NULL,
> +					   DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (IS_ERR(encoder))
> +		return PTR_ERR(encoder);
> +
> +	drm_encoder_helper_add(encoder, &amdgpu_dm_wb_encoder_helper_funcs);
> +
> +	encoder->possible_crtcs = amdgpu_dm_get_encoder_crtc_mask(dm->adev);
>   
>   	wbcon->link = link;
>   
>   	drm_connector_helper_add(&wbcon->base.base, &amdgpu_dm_wb_conn_helper_funcs);
>   
> -	res = drm_writeback_connector_init(&dm->adev->ddev, &wbcon->base,
> +	res = drmm_writeback_connector_init(&dm->adev->ddev, &wbcon->base,
>   					    &amdgpu_dm_wb_connector_funcs,
> -					    &amdgpu_dm_wb_encoder_helper_funcs,
> +					    encoder,
>   					    amdgpu_dm_wb_formats,
> -					    ARRAY_SIZE(amdgpu_dm_wb_formats),
> -					    amdgpu_dm_get_encoder_crtc_mask(dm->adev));
> +					    ARRAY_SIZE(amdgpu_dm_wb_formats));
>   
>   	if (res)
>   		return res;
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/8] drm/mali: use drmm_writeback_connector_init()

[Louis Chauvet]

Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback
> connector instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>   drivers/gpu/drm/arm/malidp_mw.c | 25 ++++++++++++++-----------
>   1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
> index 600af5ad81b15d0c30f9f79588f40cca07437ed8..182275c0c29cd8527c85dfb3e7317561da392c46 100644
> --- a/drivers/gpu/drm/arm/malidp_mw.c
> +++ b/drivers/gpu/drm/arm/malidp_mw.c
> @@ -84,11 +84,6 @@ malidp_mw_connector_detect(struct drm_connector *connector, bool force)
>   	return connector_status_connected;
>   }
>   
> -static void malidp_mw_connector_destroy(struct drm_connector *connector)
> -{
> -	drm_connector_cleanup(connector);
> -}
> -
>   static struct drm_connector_state *
>   malidp_mw_connector_duplicate_state(struct drm_connector *connector)
>   {
> @@ -114,7 +109,6 @@ static const struct drm_connector_funcs malidp_mw_connector_funcs = {
>   	.reset = malidp_mw_connector_reset,
>   	.detect = malidp_mw_connector_detect,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = malidp_mw_connector_destroy,
>   	.atomic_duplicate_state = malidp_mw_connector_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
> @@ -211,6 +205,7 @@ static u32 *get_writeback_formats(struct malidp_drm *malidp, int *n_formats)
>   int malidp_mw_connector_init(struct drm_device *drm)
>   {
>   	struct malidp_drm *malidp = drm_to_malidp(drm);
> +	struct drm_encoder *encoder;
>   	u32 *formats;
>   	int ret, n_formats;
>   
> @@ -224,11 +219,19 @@ int malidp_mw_connector_init(struct drm_device *drm)
>   	if (!formats)
>   		return -ENOMEM;
>   
> -	ret = drm_writeback_connector_init(drm, &malidp->mw_connector,
> -					   &malidp_mw_connector_funcs,
> -					   &malidp_mw_encoder_helper_funcs,
> -					   formats, n_formats,
> -					   1 << drm_crtc_index(&malidp->crtc));
> +	encoder = drmm_plain_encoder_alloc(drm, NULL, DRM_MODE_ENCODER_VIRTUAL,
> +					   NULL);
> +	if (IS_ERR(encoder))
> +		return PTR_ERR(encoder);
> +
> +	drm_encoder_helper_add(encoder, &malidp_mw_encoder_helper_funcs);
> +
> +	encoder->possible_crtcs = 1 << drm_crtc_index(&malidp->crtc);

drm_crtc_mask?

With this:

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> +
> +	ret = drmm_writeback_connector_init(drm, &malidp->mw_connector,
> +					    &malidp_mw_connector_funcs,
> +					    encoder,
> +					    formats, n_formats);
>   	kfree(formats);
>   	if (ret)
>   		return ret;
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 6/8] drm/vc4: use drmm_writeback_connector_init()

[Louis Chauvet]

Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback
> connector instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
>   drivers/gpu/drm/vc4/vc4_txp.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
> index 42acac05fe47861ced226a4f64661f545e21ddb5..fef4e4ee47cf41d40de47c5201f0349dc36c56bd 100644
> --- a/drivers/gpu/drm/vc4/vc4_txp.c
> +++ b/drivers/gpu/drm/vc4/vc4_txp.c
> @@ -377,7 +377,6 @@ vc4_txp_connector_detect(struct drm_connector *connector, bool force)
>   static const struct drm_connector_funcs vc4_txp_connector_funcs = {
>   	.detect = vc4_txp_connector_detect,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = drm_connector_cleanup,
>   	.reset = drm_atomic_helper_connector_reset,
>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> @@ -601,10 +600,10 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data)
>   
>   	drm_connector_helper_add(&txp->connector.base,
>   				 &vc4_txp_connector_helper_funcs);
> -	ret = drm_writeback_connector_init_with_encoder(drm, &txp->connector,
> -							encoder,
> -							&vc4_txp_connector_funcs,
> -							drm_fmts, ARRAY_SIZE(drm_fmts));
> +	ret = drmm_writeback_connector_init(drm, &txp->connector,
> +					    &vc4_txp_connector_funcs,
> +					    encoder,
> +					    drm_fmts, ARRAY_SIZE(drm_fmts));
>   	if (ret)
>   		return ret;
>   
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 5/8] drm/msm/dpu: use drmm_writeback_connector_init()

[Louis Chauvet]

Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback
> connector instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>   .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c    | 23 +++++++++++++++-------
>   1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> index 8cd37d7b8ae281cbc1fd8cbb243c621174517e23..9986a10e8114680e9da48986f4ca3ce6ec66b8cb 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> @@ -134,7 +134,6 @@ static void rcar_du_wb_conn_reset(struct drm_connector *connector)
>   static const struct drm_connector_funcs rcar_du_wb_conn_funcs = {
>   	.reset = rcar_du_wb_conn_reset,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = drm_connector_cleanup,
>   	.atomic_duplicate_state = rcar_du_wb_conn_duplicate_state,
>   	.atomic_destroy_state = rcar_du_wb_conn_destroy_state,
>   };
> @@ -202,15 +201,25 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu,
>   {
>   	struct drm_writeback_connector *wb_conn = &rcrtc->writeback;
>   
> +	struct drm_encoder *encoder;
> +
> +	encoder = drmm_plain_encoder_alloc(&rcdu->ddev, NULL,
> +					   DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (IS_ERR(encoder))
> +		return PTR_ERR(encoder);
> +
> +	drm_encoder_helper_add(encoder, &rcar_du_wb_enc_helper_funcs);
> +
> +	encoder->possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);

drm_crtc_mask?

With this:

Reviewed-by: Louis Chauvet <louis.chauvet>

> +
>   	drm_connector_helper_add(&wb_conn->base,
>   				 &rcar_du_wb_conn_helper_funcs);
>   
> -	return drm_writeback_connector_init(&rcdu->ddev, wb_conn,
> -					    &rcar_du_wb_conn_funcs,
> -					    &rcar_du_wb_enc_helper_funcs,
> -					    writeback_formats,
> -					    ARRAY_SIZE(writeback_formats),
> -					    1 << drm_crtc_index(&rcrtc->crtc));
> +	return drmm_writeback_connector_init(&rcdu->ddev, wb_conn,
> +					     &rcar_du_wb_conn_funcs,
> +					     encoder,
> +					     writeback_formats,
> +					     ARRAY_SIZE(writeback_formats));
>   }
>   
>   void rcar_du_writeback_setup(struct rcar_du_crtc *rcrtc,
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 7/8] drm: writeback: drop excess connector initialization functions

[Louis Chauvet]

Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> Now as all drivers have been converted to
> drmm_writeback_connector_init(), drop drm_writeback_connector_init() and
> drm_writeback_connector::encoder field, they are unused now.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
>   drivers/gpu/drm/drm_writeback.c | 55 -----------------------------------------
>   include/drm/drm_writeback.h     | 18 --------------
>   2 files changed, 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index 95b8a2e4bda69230591b50be15d14d0b3692373b..1a01df91b2c5868e158d489b782f4c57c61a272c 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -142,61 +142,6 @@ static int create_writeback_properties(struct drm_device *dev)
>   	return 0;
>   }
>   
> -static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
> -	.destroy = drm_encoder_cleanup,
> -};
> -
> -/**
> - * drm_writeback_connector_init - Initialize a writeback connector and its properties
> - * @dev: DRM device
> - * @wb_connector: Writeback connector to initialize
> - * @con_funcs: Connector funcs vtable
> - * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
> - * @formats: Array of supported pixel formats for the writeback engine
> - * @n_formats: Length of the formats array
> - * @possible_crtcs: possible crtcs for the internal writeback encoder
> - *
> - * This function creates the writeback-connector-specific properties if they
> - * have not been already created, initializes the connector as
> - * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
> - * values. It will also create an internal encoder associated with the
> - * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
> - * the encoder helper.
> - *
> - * Drivers should always use this function instead of drm_connector_init() to
> - * set up writeback connectors.
> - *
> - * Returns: 0 on success, or a negative error code
> - */
> -int drm_writeback_connector_init(struct drm_device *dev,
> -				 struct drm_writeback_connector *wb_connector,
> -				 const struct drm_connector_funcs *con_funcs,
> -				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
> -				 const u32 *formats, int n_formats,
> -				 u32 possible_crtcs)
> -{
> -	int ret = 0;
> -
> -	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
> -
> -	wb_connector->encoder.possible_crtcs = possible_crtcs;
> -
> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
> -			       &drm_writeback_encoder_funcs,
> -			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> -	if (ret)
> -		return ret;
> -
> -	ret = drm_writeback_connector_init_with_encoder(dev, wb_connector, &wb_connector->encoder,
> -			con_funcs, formats, n_formats);
> -
> -	if (ret)
> -		drm_encoder_cleanup(&wb_connector->encoder);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(drm_writeback_connector_init);
> -
>   static void delete_writeback_properties(struct drm_device *dev)
>   {
>   	if (dev->mode_config.writeback_pixel_formats_property) {
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index c380a7b8f55a3616fa070c037d5cc653b0061fe6..879ca103320cc225ffb3687419088361315535fc 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -24,17 +24,6 @@ struct drm_writeback_connector {
>   	 */
>   	struct drm_connector base;
>   
> -	/**
> -	 * @encoder: Internal encoder used by the connector to fulfill
> -	 * the DRM framework requirements. The users of the
> -	 * @drm_writeback_connector control the behaviour of the @encoder
> -	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
> -	 * function.
> -	 * For users of drm_writeback_connector_init_with_encoder(), this field
> -	 * is not valid as the encoder is managed within their drivers.
> -	 */
> -	struct drm_encoder encoder;
> -
>   	/**
>   	 * @pixel_formats_blob_ptr:
>   	 *
> @@ -148,13 +137,6 @@ drm_connector_to_writeback(struct drm_connector *connector)
>   	return container_of(connector, struct drm_writeback_connector, base);
>   }
>   
> -int drm_writeback_connector_init(struct drm_device *dev,
> -				 struct drm_writeback_connector *wb_connector,
> -				 const struct drm_connector_funcs *con_funcs,
> -				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
> -				 const u32 *formats, int n_formats,
> -				 u32 possible_crtcs);
> -
>   int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
>   				struct drm_writeback_connector *wb_connector,
>   				struct drm_encoder *enc,
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 4/8] drm/msm/dpu: use drmm_writeback_connector_init()

[Louis Chauvet]

Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback
> connector instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> index 8ff496082902b1ee713e806140f39b4730ed256a..cd73468e369a93c50303db2a7d4499bcb17be5d1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> @@ -80,7 +80,6 @@ static int dpu_wb_conn_atomic_check(struct drm_connector *connector,
>   static const struct drm_connector_funcs dpu_wb_conn_funcs = {
>   	.reset = drm_atomic_helper_connector_reset,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = drm_connector_cleanup,
>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
> @@ -131,12 +130,9 @@ int dpu_writeback_init(struct drm_device *dev, struct drm_encoder *enc,
>   
>   	drm_connector_helper_add(&dpu_wb_conn->base.base, &dpu_wb_conn_helper_funcs);
>   
> -	/* DPU initializes the encoder and sets it up completely for writeback
> -	 * cases and hence should use the new API drm_writeback_connector_init_with_encoder
> -	 * to initialize the writeback connector
> -	 */
> -	rc = drm_writeback_connector_init_with_encoder(dev, &dpu_wb_conn->base, enc,
> -			&dpu_wb_conn_funcs, format_list, num_formats);
> +	rc = drmm_writeback_connector_init(dev, &dpu_wb_conn->base,
> +					   &dpu_wb_conn_funcs, enc,
> +					   format_list, num_formats);
>   
>   	if (!rc)
>   		dpu_wb_conn->wb_enc = enc;
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/8] drm/komeda: use drmm_writeback_connector_init()

[Louis Chauvet]

Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback
> connector instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>   .../drm/arm/display/komeda/komeda_wb_connector.c   | 30 ++++++++++++----------
>   1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> index 875cdbff18c9dc97e736049feaa8a1fe1bcc82ff..210841b4453abfe60cecc1cfc79a84298be3e684 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> @@ -5,6 +5,7 @@
>    *
>    */
>   #include <drm/drm_framebuffer.h>
> +#include <drm/drm_managed.h>
>   #include "komeda_dev.h"
>   #include "komeda_kms.h"
>   
> @@ -121,17 +122,10 @@ komeda_wb_connector_fill_modes(struct drm_connector *connector,
>   	return 0;
>   }
>   
> -static void komeda_wb_connector_destroy(struct drm_connector *connector)
> -{
> -	drm_connector_cleanup(connector);
> -	kfree(to_kconn(to_wb_conn(connector)));
> -}
> -
>   static const struct drm_connector_funcs komeda_wb_connector_funcs = {
>   	.reset			= drm_atomic_helper_connector_reset,
>   	.detect			= komeda_wb_connector_detect,
>   	.fill_modes		= komeda_wb_connector_fill_modes,
> -	.destroy		= komeda_wb_connector_destroy,
>   	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
>   	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
>   };
> @@ -143,13 +137,15 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
>   	struct komeda_wb_connector *kwb_conn;
>   	struct drm_writeback_connector *wb_conn;
>   	struct drm_display_info *info;
> +	struct drm_encoder *encoder;
> +
>   	u32 *formats, n_formats = 0;
>   	int err;
>   
>   	if (!kcrtc->master->wb_layer)
>   		return 0;
>   
> -	kwb_conn = kzalloc(sizeof(*kwb_conn), GFP_KERNEL);
> +	kwb_conn = drmm_kzalloc(&kms->base, sizeof(*kwb_conn), GFP_KERNEL);
>   	if (!kwb_conn)
>   		return -ENOMEM;
>   
> @@ -165,11 +161,19 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
>   		return -ENOMEM;
>   	}
>   
> -	err = drm_writeback_connector_init(&kms->base, wb_conn,
> -					   &komeda_wb_connector_funcs,
> -					   &komeda_wb_encoder_helper_funcs,
> -					   formats, n_formats,
> -					   BIT(drm_crtc_index(&kcrtc->base)));
> +	encoder = drmm_plain_encoder_alloc(&kms->base, NULL,
> +					   DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (IS_ERR(encoder))
> +		return PTR_ERR(encoder);
> +
> +	drm_encoder_helper_add(encoder, &komeda_wb_encoder_helper_funcs);
> +
> +	encoder->possible_crtcs = BIT(drm_crtc_index(&kcrtc->base));

Maybe use drm_crtc_mask?

With this:

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> +
> +	err = drmm_writeback_connector_init(&kms->base, wb_conn,
> +					    &komeda_wb_connector_funcs,
> +					    encoder,
> +					    formats, n_formats);
>   	komeda_put_fourcc_list(formats);
>   	if (err) {
>   		kfree(kwb_conn);
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [PATCH 7/8] drm: writeback: drop excess connector initialization functions

[Kandpal, Suraj]

> Subject: Re: [PATCH 7/8] drm: writeback: drop excess connector initialization
> functions

This should be drm/writeback 

Regards,
Suraj Kandpal

> 
> 
> 
> Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> > Now as all drivers have been converted to
> > drmm_writeback_connector_init(), drop drm_writeback_connector_init()
> > and drm_writeback_connector::encoder field, they are unused now.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> 
> Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
> 
> > ---
> >   drivers/gpu/drm/drm_writeback.c | 55 -----------------------------------------
> >   include/drm/drm_writeback.h     | 18 --------------
> >   2 files changed, 73 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_writeback.c
> > b/drivers/gpu/drm/drm_writeback.c index
> >
> 95b8a2e4bda69230591b50be15d14d0b3692373b..1a01df91b2c5868e158d489
> b782f
> > 4c57c61a272c 100644
> > --- a/drivers/gpu/drm/drm_writeback.c
> > +++ b/drivers/gpu/drm/drm_writeback.c
> > @@ -142,61 +142,6 @@ static int create_writeback_properties(struct
> drm_device *dev)
> >   	return 0;
> >   }
> >
> > -static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
> > -	.destroy = drm_encoder_cleanup,
> > -};
> > -
> > -/**
> > - * drm_writeback_connector_init - Initialize a writeback connector
> > and its properties
> > - * @dev: DRM device
> > - * @wb_connector: Writeback connector to initialize
> > - * @con_funcs: Connector funcs vtable
> > - * @enc_helper_funcs: Encoder helper funcs vtable to be used by the
> > internal encoder
> > - * @formats: Array of supported pixel formats for the writeback
> > engine
> > - * @n_formats: Length of the formats array
> > - * @possible_crtcs: possible crtcs for the internal writeback encoder
> > - *
> > - * This function creates the writeback-connector-specific properties
> > if they
> > - * have not been already created, initializes the connector as
> > - * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the
> > property
> > - * values. It will also create an internal encoder associated with
> > the
> > - * drm_writeback_connector and set it to use the @enc_helper_funcs
> > vtable for
> > - * the encoder helper.
> > - *
> > - * Drivers should always use this function instead of
> > drm_connector_init() to
> > - * set up writeback connectors.
> > - *
> > - * Returns: 0 on success, or a negative error code
> > - */
> > -int drm_writeback_connector_init(struct drm_device *dev,
> > -				 struct drm_writeback_connector
> *wb_connector,
> > -				 const struct drm_connector_funcs
> *con_funcs,
> > -				 const struct drm_encoder_helper_funcs
> *enc_helper_funcs,
> > -				 const u32 *formats, int n_formats,
> > -				 u32 possible_crtcs)
> > -{
> > -	int ret = 0;
> > -
> > -	drm_encoder_helper_add(&wb_connector->encoder,
> enc_helper_funcs);
> > -
> > -	wb_connector->encoder.possible_crtcs = possible_crtcs;
> > -
> > -	ret = drm_encoder_init(dev, &wb_connector->encoder,
> > -			       &drm_writeback_encoder_funcs,
> > -			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> > -	if (ret)
> > -		return ret;
> > -
> > -	ret = drm_writeback_connector_init_with_encoder(dev,
> wb_connector, &wb_connector->encoder,
> > -			con_funcs, formats, n_formats);
> > -
> > -	if (ret)
> > -		drm_encoder_cleanup(&wb_connector->encoder);
> > -
> > -	return ret;
> > -}
> > -EXPORT_SYMBOL(drm_writeback_connector_init);
> > -
> >   static void delete_writeback_properties(struct drm_device *dev)
> >   {
> >   	if (dev->mode_config.writeback_pixel_formats_property) { diff --git
> > a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h index
> >
> c380a7b8f55a3616fa070c037d5cc653b0061fe6..879ca103320cc225ffb36874190
> 8
> > 8361315535fc 100644
> > --- a/include/drm/drm_writeback.h
> > +++ b/include/drm/drm_writeback.h
> > @@ -24,17 +24,6 @@ struct drm_writeback_connector {
> >   	 */
> >   	struct drm_connector base;
> >
> > -	/**
> > -	 * @encoder: Internal encoder used by the connector to fulfill
> > -	 * the DRM framework requirements. The users of the
> > -	 * @drm_writeback_connector control the behaviour of the @encoder
> > -	 * by passing the @enc_funcs parameter to
> drm_writeback_connector_init()
> > -	 * function.
> > -	 * For users of drm_writeback_connector_init_with_encoder(), this
> field
> > -	 * is not valid as the encoder is managed within their drivers.
> > -	 */
> > -	struct drm_encoder encoder;
> > -
> >   	/**
> >   	 * @pixel_formats_blob_ptr:
> >   	 *
> > @@ -148,13 +137,6 @@ drm_connector_to_writeback(struct drm_connector
> *connector)
> >   	return container_of(connector, struct drm_writeback_connector, base);
> >   }
> >
> > -int drm_writeback_connector_init(struct drm_device *dev,
> > -				 struct drm_writeback_connector
> *wb_connector,
> > -				 const struct drm_connector_funcs
> *con_funcs,
> > -				 const struct drm_encoder_helper_funcs
> *enc_helper_funcs,
> > -				 const u32 *formats, int n_formats,
> > -				 u32 possible_crtcs);
> > -
> >   int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> >   				struct drm_writeback_connector
> *wb_connector,
> >   				struct drm_encoder *enc,
> >
> 
> --
> Louis Chauvet, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

RE: [PATCH 8/8] drm: writeback: rename drm_writeback_connector_init_with_encoder()

[Kandpal, Suraj]

> Subject: Re: [PATCH 8/8] drm: writeback: rename
> drm_writeback_connector_init_with_encoder()

Same here drm/writeback

Regards,
Suraj Kandpal

> 
> 
> 
> Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> > Rename drm_writeback_connector_init_with_encoder() to
> > drm_writeback_connector_init() and adapt its interface to follow
> > drmm_writeback_connector_init().
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > ---
> >   drivers/gpu/drm/drm_writeback.c | 14 +++++++-------
> >   include/drm/drm_writeback.h     | 10 +++++-----
> >   2 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_writeback.c
> > b/drivers/gpu/drm/drm_writeback.c index
> >
> 1a01df91b2c5868e158d489b782f4c57c61a272c..ec2575c4c21b7449707b05953
> 22e
> > 2202a0cf9865 100644
> > --- a/drivers/gpu/drm/drm_writeback.c
> > +++ b/drivers/gpu/drm/drm_writeback.c
> > @@ -235,7 +235,7 @@ static int __drm_writeback_connector_init(struct
> drm_device *dev,
> >   }
> >
> >   /**
> > - * drm_writeback_connector_init_with_encoder - Initialize a writeback
> > connector with
> > + * drm_writeback_connector_init - Initialize a writeback connector
> > + with
> >    * a custom encoder
> 
> If I understood correctly your series you want to reduce the usage of non-
> drmm writeback, so maybe we can add a comment to direct poeple to drmm
> variant to avoid new usage of this API?
> 
> With or without this:
> 
> Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
> 
> >    *
> >    * @dev: DRM device
> > @@ -263,11 +263,11 @@ static int __drm_writeback_connector_init(struct
> drm_device *dev,
> >    *
> >    * Returns: 0 on success, or a negative error code
> >    */
> > -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > -					      struct drm_writeback_connector
> *wb_connector,
> > -					      struct drm_encoder *enc,
> > -					      const struct drm_connector_funcs
> *con_funcs,
> > -					      const u32 *formats, int n_formats)
> > +int drm_writeback_connector_init(struct drm_device *dev,
> > +				 struct drm_writeback_connector
> *wb_connector,
> > +				 const struct drm_connector_funcs
> *con_funcs,
> > +				 struct drm_encoder *enc,
> > +				 const u32 *formats, int n_formats)
> >   {
> >   	struct drm_connector *connector = &wb_connector->base;
> >   	int ret;
> > @@ -284,7 +284,7 @@ int
> > drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> >
> >   	return ret;
> >   }
> > -EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
> > +EXPORT_SYMBOL(drm_writeback_connector_init);
> >
> >   /**
> >    * drm_writeback_connector_cleanup - Cleanup the writeback connector
> > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> > index
> >
> 879ca103320cc225ffb3687419088361315535fc..958466a05e604b387722610fc1
> 1f
> > 9e841316d21b 100644
> > --- a/include/drm/drm_writeback.h
> > +++ b/include/drm/drm_writeback.h
> > @@ -137,11 +137,11 @@ drm_connector_to_writeback(struct
> drm_connector *connector)
> >   	return container_of(connector, struct drm_writeback_connector, base);
> >   }
> >
> > -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > -				struct drm_writeback_connector
> *wb_connector,
> > -				struct drm_encoder *enc,
> > -				const struct drm_connector_funcs
> *con_funcs, const u32 *formats,
> > -				int n_formats);
> > +int drm_writeback_connector_init(struct drm_device *dev,
> > +				 struct drm_writeback_connector
> *wb_connector,
> > +				 const struct drm_connector_funcs
> *con_funcs,
> > +				 struct drm_encoder *enc,
> > +				 const u32 *formats, int n_formats);
> >
> >   int drmm_writeback_connector_init(struct drm_device *dev,
> >   				  struct drm_writeback_connector
> *wb_connector,
> >
> 
> --
> Louis Chauvet, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 7/8] drm: writeback: drop excess connector initialization functions

[Dmitry Baryshkov]

On 01/08/2025 17:22, Kandpal, Suraj wrote:
>> Subject: Re: [PATCH 7/8] drm: writeback: drop excess connector initialization
>> functions
> 
> This should be drm/writeback

No:

$ git log --oneline --no-merges next/master 
drivers/gpu/drm/drm_writeback.c
fb721b2c35b1 drm: writeback: Fix drm_writeback_connector_cleanup signature
09cba36cc840 drm: Include <linux/export.h>
ddd147d91d50 drm: writeback: Fix kernel doc name
ff3881cc6a58 drm: writeback: Fix use after free in 
drm_writeback_connector_cleanup()
1914ba2b91ea drm: writeback: Create drmm variants for 
drm_writeback_connector initialization
2f3f4a73631b drm: writeback: Add missing cleanup in case of 
initialization failure
135d8fc7af44 drm: writeback: Create an helper for 
drm_writeback_connector initialization
02c50fa60ca5 drm/writeback: remove pointless enable_signaling implementation
720cf96d8fec drm: Drop drm_framebuffer.h from drm_crtc.h
7933aecffa28 drm: introduce drm_writeback_connector_init_with_encoder() API
57b8280a0a41 drm: allow passing possible_crtcs to 
drm_writeback_connector_init()
38d6fd406aaa drm/writeback: don't set fence->ops to default
b1066a123538 drm: Clear the fence pointer when writeback job signaled
0500c04ea14a drm: drop use of drmP.h in drm/*
9d2230dc1351 drm: writeback: Add job prepare and cleanup operations
e482ae9b5fdc drm: writeback: Fix leak of writeback job
97eb9eaeb95b drm: writeback: Cleanup job ownership handling when queuing job
71a5cb3eb758 drm: writeback: Fix doc that says connector should be 
disconnected
cde4c44d8769 drm: drop _mode_ from drm_mode_connector_attach_encoder
73915b2b1f25 drm/writeback: Fix the "overview" section of the doc
b13cc8dd5884 drm: writeback: Add out-fences for writeback connectors
935774cd71fe drm: Add writeback connector type



-- 
With best wishes
Dmitry

Re: [PATCH 0/8] drm: writeback: clean up writeback connector initialization

[Liviu Dudau]

Hi,

On Fri, Aug 01, 2025 at 04:51:08PM +0300, Dmitry Baryshkov wrote:
> Drivers using drm_writeback_connector_init() / _with_encoder() don't
> perform cleanup in a manner similar to drmm_writeback_connector_init()
> (see drm_writeback_connector_cleanup()). Migrate all existing drivers
> to use drmm_writeback_connector_init(), drop
> drm_writeback_connector_init() and drm_writeback_connector::encoder
> (it's unused afterwards).
> 
> This series leaves former drm_writeback_connector_init_with_encoder()
> (renamed to drm_writeback_connector_init as a non-managed counterpart
> for drmm_writeback_connector_init()). It is supposed to be used by
> drivers which can not use drmm functions (like Intel). However I think
> it would be better to drop it completely.

The intent of _init_with_encoder() was to be a special case for drivers
that use their own specific encoder and the rest use the generic function
that creates the virtual encoder inside the call. The API for
_init_with_encoder() was actually introduced 4 years after the original
patch, so that should give a hint.

drmm_writeback_connector_init() is more like _init_with_encoder() and
I don't remember reviewing it, so I'm not sure why that was considered
to be the better behaviour for the managed version. Now you're moving
all the drivers to the managed version and you have to duplicate code
in each driver to create the ENCODER_VIRTUAL encoder.

I'm not against the changes being made in the series, I just want to
see a better justification on why _init_with_encoder() behaviour is
better than the previous default that you're removing.

Best regards,
Liviu


> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> Dmitry Baryshkov (8):
>       drm/amd/display: use drmm_writeback_connector_init()
>       drm/komeda: use drmm_writeback_connector_init()
>       drm/mali: use drmm_writeback_connector_init()
>       drm/msm/dpu: use drmm_writeback_connector_init()
>       drm/msm/dpu: use drmm_writeback_connector_init()
>       drm/vc4: use drmm_writeback_connector_init()
>       drm: writeback: drop excess connector initialization functions
>       drm: writeback: rename drm_writeback_connector_init_with_encoder()
> 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  2 +-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c   | 18 ++++--
>  .../drm/arm/display/komeda/komeda_wb_connector.c   | 30 ++++++----
>  drivers/gpu/drm/arm/malidp_mw.c                    | 25 ++++----
>  drivers/gpu/drm/drm_writeback.c                    | 69 +++-------------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c      | 10 +---
>  .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c    | 23 +++++---
>  drivers/gpu/drm/vc4/vc4_txp.c                      |  9 ++-
>  include/drm/drm_writeback.h                        | 22 +------
>  9 files changed, 77 insertions(+), 131 deletions(-)
> ---
> base-commit: 94f208ff622b09309358abaf26d7acca0c318fae
> change-id: 20250801-wb-drop-encoder-97a0c75bd5d7
> 
> Best regards,
> -- 
> With best wishes
> Dmitry
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 0/8] drm: writeback: clean up writeback connector initialization

[Dmitry Baryshkov]

On Mon, Aug 04, 2025 at 03:43:12PM +0100, Liviu Dudau wrote:
> Hi,
> 
> On Fri, Aug 01, 2025 at 04:51:08PM +0300, Dmitry Baryshkov wrote:
> > Drivers using drm_writeback_connector_init() / _with_encoder() don't
> > perform cleanup in a manner similar to drmm_writeback_connector_init()
> > (see drm_writeback_connector_cleanup()). Migrate all existing drivers
> > to use drmm_writeback_connector_init(), drop
> > drm_writeback_connector_init() and drm_writeback_connector::encoder
> > (it's unused afterwards).
> > 
> > This series leaves former drm_writeback_connector_init_with_encoder()
> > (renamed to drm_writeback_connector_init as a non-managed counterpart
> > for drmm_writeback_connector_init()). It is supposed to be used by
> > drivers which can not use drmm functions (like Intel). However I think
> > it would be better to drop it completely.
> 
> The intent of _init_with_encoder() was to be a special case for drivers
> that use their own specific encoder and the rest use the generic function
> that creates the virtual encoder inside the call. The API for
> _init_with_encoder() was actually introduced 4 years after the original
> patch, so that should give a hint.
> 
> drmm_writeback_connector_init() is more like _init_with_encoder() and
> I don't remember reviewing it, so I'm not sure why that was considered
> to be the better behaviour for the managed version. Now you're moving
> all the drivers to the managed version and you have to duplicate code
> in each driver to create the ENCODER_VIRTUAL encoder.

This follows e.g. the process of deprecating drm_simple_* /
drm_simple_encoder. The drivers are expected to open code empty encoder
handling on their own.

> I'm not against the changes being made in the series, I just want to
> see a better justification on why _init_with_encoder() behaviour is
> better than the previous default that you're removing.

This was triggered by the discussion of Intel writeback patchset, see
the threads for first three patches of [1]. We have an optional
non-pointer field inside drm_writeback_connector, which can be left
uninitialized (or zero-filled). I have checked and the drivers are not
actually using the embedded connector for anything after linking it to
the drm_connector. So, by removing the encoder from the
drm_writeback_connector structure we are tying a loose end.

[1] https://lore.kernel.org/dri-devel/20250725050409.2687242-1-suraj.kandpal@intel.com/

-- 
With best wishes
Dmitry

RE: [PATCH 1/8] drm/amd/display: use drmm_writeback_connector_init()

[Kandpal, Suraj]

> Subject: [PATCH 1/8] drm/amd/display: use drmm_writeback_connector_init()
> 
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback connector
> instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

I'll be sending a RFC version of code reworking the writeback structure to sit inside the
drm_connector structure and see how that fares with everyone so that we can go ahead with the
redesign

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c    |  2 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c | 18
> +++++++++++++-----
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index
> 096b23ad4845d365dad707deed4e3e84b1f9945d..a9948e8d67a6e9556d21090
> b5cdd101258d62480 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -10003,7 +10003,7 @@ static void dm_set_writeback(struct
> amdgpu_display_manager *dm,
>  		return;
>  	}
> 
> -	acrtc = to_amdgpu_crtc(wb_conn->encoder.crtc);
> +	acrtc = to_amdgpu_crtc(crtc_state->base.crtc);
>  	if (!acrtc) {
>  		drm_err(adev_to_drm(adev), "no amdgpu_crtc found\n");
>  		kfree(wb_info);
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
> index
> d9527c05fc878ef6916782ff9fde847813938461..80c37487ca77c0494eaf769c5b9
> a3c53486aa52d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
> @@ -171,7 +171,6 @@ static const struct drm_encoder_helper_funcs
> amdgpu_dm_wb_encoder_helper_funcs =
> 
>  static const struct drm_connector_funcs amdgpu_dm_wb_connector_funcs = {
>  	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = drm_connector_cleanup,
>  	.reset = amdgpu_dm_connector_funcs_reset,
>  	.atomic_duplicate_state =
> amdgpu_dm_connector_atomic_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> @@ -190,17 +189,26 @@ int amdgpu_dm_wb_connector_init(struct
> amdgpu_display_manager *dm,
>  	struct dc *dc = dm->dc;
>  	struct dc_link *link = dc_get_link_at_index(dc, link_index);
>  	int res = 0;
> +	struct drm_encoder *encoder;
> +
> +	encoder = drmm_plain_encoder_alloc(&dm->adev->ddev, NULL,
> +					   DRM_MODE_ENCODER_VIRTUAL,
> NULL);
> +	if (IS_ERR(encoder))
> +		return PTR_ERR(encoder);
> +
> +	drm_encoder_helper_add(encoder,
> &amdgpu_dm_wb_encoder_helper_funcs);
> +
> +	encoder->possible_crtcs = amdgpu_dm_get_encoder_crtc_mask(dm-
> >adev);
> 
>  	wbcon->link = link;
> 
>  	drm_connector_helper_add(&wbcon->base.base,
> &amdgpu_dm_wb_conn_helper_funcs);
> 
> -	res = drm_writeback_connector_init(&dm->adev->ddev, &wbcon-
> >base,
> +	res = drmm_writeback_connector_init(&dm->adev->ddev, &wbcon-
> >base,
> 
> &amdgpu_dm_wb_connector_funcs,
> -
> &amdgpu_dm_wb_encoder_helper_funcs,
> +					    encoder,
>  					    amdgpu_dm_wb_formats,
> -
> ARRAY_SIZE(amdgpu_dm_wb_formats),
> -
> amdgpu_dm_get_encoder_crtc_mask(dm->adev));
> +
> ARRAY_SIZE(amdgpu_dm_wb_formats));
> 
>  	if (res)
>  		return res;
> 
> --
> 2.39.5

RE: [PATCH 2/8] drm/komeda: use drmm_writeback_connector_init()

[Kandpal, Suraj]

> Subject: [PATCH 2/8] drm/komeda: use drmm_writeback_connector_init()
> 
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback connector
> instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  .../drm/arm/display/komeda/komeda_wb_connector.c   | 30 ++++++++++++----
> ------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> index
> 875cdbff18c9dc97e736049feaa8a1fe1bcc82ff..210841b4453abfe60cecc1cfc79a8
> 4298be3e684 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> @@ -5,6 +5,7 @@
>   *
>   */
>  #include <drm/drm_framebuffer.h>
> +#include <drm/drm_managed.h>
>  #include "komeda_dev.h"
>  #include "komeda_kms.h"
> 
> @@ -121,17 +122,10 @@ komeda_wb_connector_fill_modes(struct
> drm_connector *connector,
>  	return 0;
>  }
> 
> -static void komeda_wb_connector_destroy(struct drm_connector *connector)
> -{
> -	drm_connector_cleanup(connector);
> -	kfree(to_kconn(to_wb_conn(connector)));
> -}
> -
>  static const struct drm_connector_funcs komeda_wb_connector_funcs = {
>  	.reset			= drm_atomic_helper_connector_reset,
>  	.detect			= komeda_wb_connector_detect,
>  	.fill_modes		= komeda_wb_connector_fill_modes,
> -	.destroy		= komeda_wb_connector_destroy,
>  	.atomic_duplicate_state	=
> drm_atomic_helper_connector_duplicate_state,
>  	.atomic_destroy_state	=
> drm_atomic_helper_connector_destroy_state,
>  };
> @@ -143,13 +137,15 @@ static int komeda_wb_connector_add(struct
> komeda_kms_dev *kms,
>  	struct komeda_wb_connector *kwb_conn;
>  	struct drm_writeback_connector *wb_conn;
>  	struct drm_display_info *info;
> +	struct drm_encoder *encoder;
> +
>  	u32 *formats, n_formats = 0;
>  	int err;
> 
>  	if (!kcrtc->master->wb_layer)
>  		return 0;
> 
> -	kwb_conn = kzalloc(sizeof(*kwb_conn), GFP_KERNEL);
> +	kwb_conn = drmm_kzalloc(&kms->base, sizeof(*kwb_conn),
> GFP_KERNEL);
>  	if (!kwb_conn)
>  		return -ENOMEM;
> 
> @@ -165,11 +161,19 @@ static int komeda_wb_connector_add(struct
> komeda_kms_dev *kms,
>  		return -ENOMEM;
>  	}
> 
> -	err = drm_writeback_connector_init(&kms->base, wb_conn,
> -					   &komeda_wb_connector_funcs,
> -
> &komeda_wb_encoder_helper_funcs,
> -					   formats, n_formats,
> -					   BIT(drm_crtc_index(&kcrtc->base)));
> +	encoder = drmm_plain_encoder_alloc(&kms->base, NULL,
> +					   DRM_MODE_ENCODER_VIRTUAL,
> NULL);
> +	if (IS_ERR(encoder))
> +		return PTR_ERR(encoder);
> +
> +	drm_encoder_helper_add(encoder,
> &komeda_wb_encoder_helper_funcs);
> +
> +	encoder->possible_crtcs = BIT(drm_crtc_index(&kcrtc->base));
> +
> +	err = drmm_writeback_connector_init(&kms->base, wb_conn,
> +					    &komeda_wb_connector_funcs,
> +					    encoder,
> +					    formats, n_formats);
>  	komeda_put_fourcc_list(formats);
>  	if (err) {
>  		kfree(kwb_conn);
> 
> --
> 2.39.5

RE: [PATCH 4/8] drm/msm/dpu: use drmm_writeback_connector_init()

[Kandpal, Suraj]

> Subject: [PATCH 4/8] drm/msm/dpu: use drmm_writeback_connector_init()
> 
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback connector
> instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> index
> 8ff496082902b1ee713e806140f39b4730ed256a..cd73468e369a93c50303db2a7
> d4499bcb17be5d1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> @@ -80,7 +80,6 @@ static int dpu_wb_conn_atomic_check(struct
> drm_connector *connector,  static const struct drm_connector_funcs
> dpu_wb_conn_funcs = {
>  	.reset = drm_atomic_helper_connector_reset,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = drm_connector_cleanup,
>  	.atomic_duplicate_state =
> drm_atomic_helper_connector_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
> @@ -131,12 +130,9 @@ int dpu_writeback_init(struct drm_device *dev, struct
> drm_encoder *enc,
> 
>  	drm_connector_helper_add(&dpu_wb_conn->base.base,
> &dpu_wb_conn_helper_funcs);
> 
> -	/* DPU initializes the encoder and sets it up completely for writeback
> -	 * cases and hence should use the new API
> drm_writeback_connector_init_with_encoder
> -	 * to initialize the writeback connector
> -	 */
> -	rc = drm_writeback_connector_init_with_encoder(dev,
> &dpu_wb_conn->base, enc,
> -			&dpu_wb_conn_funcs, format_list, num_formats);
> +	rc = drmm_writeback_connector_init(dev, &dpu_wb_conn->base,
> +					   &dpu_wb_conn_funcs, enc,
> +					   format_list, num_formats);
> 
>  	if (!rc)
>  		dpu_wb_conn->wb_enc = enc;
> 
> --
> 2.39.5

RE: [PATCH 6/8] drm/vc4: use drmm_writeback_connector_init()

[Kandpal, Suraj]

> Subject: [PATCH 6/8] drm/vc4: use drmm_writeback_connector_init()
> 
> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback connector
> instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/vc4/vc4_txp.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
> index
> 42acac05fe47861ced226a4f64661f545e21ddb5..fef4e4ee47cf41d40de47c5201f
> 0349dc36c56bd 100644
> --- a/drivers/gpu/drm/vc4/vc4_txp.c
> +++ b/drivers/gpu/drm/vc4/vc4_txp.c
> @@ -377,7 +377,6 @@ vc4_txp_connector_detect(struct drm_connector
> *connector, bool force)  static const struct drm_connector_funcs
> vc4_txp_connector_funcs = {
>  	.detect = vc4_txp_connector_detect,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = drm_connector_cleanup,
>  	.reset = drm_atomic_helper_connector_reset,
>  	.atomic_duplicate_state =
> drm_atomic_helper_connector_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> @@ -601,10 +600,10 @@ static int vc4_txp_bind(struct device *dev, struct
> device *master, void *data)
> 
>  	drm_connector_helper_add(&txp->connector.base,
>  				 &vc4_txp_connector_helper_funcs);
> -	ret = drm_writeback_connector_init_with_encoder(drm, &txp-
> >connector,
> -							encoder,
> -
> 	&vc4_txp_connector_funcs,
> -							drm_fmts,
> ARRAY_SIZE(drm_fmts));
> +	ret = drmm_writeback_connector_init(drm, &txp->connector,
> +					    &vc4_txp_connector_funcs,
> +					    encoder,
> +					    drm_fmts, ARRAY_SIZE(drm_fmts));
>  	if (ret)
>  		return ret;
> 
> 
> --
> 2.39.5

RE: [PATCH 7/8] drm: writeback: drop excess connector initialization functions

[Kandpal, Suraj]

> Subject: [PATCH 7/8] drm: writeback: drop excess connector initialization
> functions
> 
> Now as all drivers have been converted to drmm_writeback_connector_init(),
> drop drm_writeback_connector_init() and drm_writeback_connector::encoder
> field, they are unused now.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/drm_writeback.c | 55 -----------------------------------------
>  include/drm/drm_writeback.h     | 18 --------------
>  2 files changed, 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_writeback.c
> b/drivers/gpu/drm/drm_writeback.c index
> 95b8a2e4bda69230591b50be15d14d0b3692373b..1a01df91b2c5868e158d489
> b782f4c57c61a272c 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -142,61 +142,6 @@ static int create_writeback_properties(struct
> drm_device *dev)
>  	return 0;
>  }
> 
> -static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
> -	.destroy = drm_encoder_cleanup,
> -};
> -
> -/**
> - * drm_writeback_connector_init - Initialize a writeback connector and its
> properties
> - * @dev: DRM device
> - * @wb_connector: Writeback connector to initialize
> - * @con_funcs: Connector funcs vtable
> - * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal
> encoder
> - * @formats: Array of supported pixel formats for the writeback engine
> - * @n_formats: Length of the formats array
> - * @possible_crtcs: possible crtcs for the internal writeback encoder
> - *
> - * This function creates the writeback-connector-specific properties if they
> - * have not been already created, initializes the connector as
> - * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the
> property
> - * values. It will also create an internal encoder associated with the
> - * drm_writeback_connector and set it to use the @enc_helper_funcs vtable
> for
> - * the encoder helper.
> - *
> - * Drivers should always use this function instead of drm_connector_init() to
> - * set up writeback connectors.
> - *
> - * Returns: 0 on success, or a negative error code
> - */
> -int drm_writeback_connector_init(struct drm_device *dev,
> -				 struct drm_writeback_connector
> *wb_connector,
> -				 const struct drm_connector_funcs
> *con_funcs,
> -				 const struct drm_encoder_helper_funcs
> *enc_helper_funcs,
> -				 const u32 *formats, int n_formats,
> -				 u32 possible_crtcs)
> -{
> -	int ret = 0;
> -
> -	drm_encoder_helper_add(&wb_connector->encoder,
> enc_helper_funcs);
> -
> -	wb_connector->encoder.possible_crtcs = possible_crtcs;
> -
> -	ret = drm_encoder_init(dev, &wb_connector->encoder,
> -			       &drm_writeback_encoder_funcs,
> -			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> -	if (ret)
> -		return ret;
> -
> -	ret = drm_writeback_connector_init_with_encoder(dev,
> wb_connector, &wb_connector->encoder,
> -			con_funcs, formats, n_formats);
> -
> -	if (ret)
> -		drm_encoder_cleanup(&wb_connector->encoder);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(drm_writeback_connector_init);
> -
>  static void delete_writeback_properties(struct drm_device *dev)  {
>  	if (dev->mode_config.writeback_pixel_formats_property) { diff --git
> a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h index
> c380a7b8f55a3616fa070c037d5cc653b0061fe6..879ca103320cc225ffb36874190
> 88361315535fc 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -24,17 +24,6 @@ struct drm_writeback_connector {
>  	 */
>  	struct drm_connector base;
> 
> -	/**
> -	 * @encoder: Internal encoder used by the connector to fulfill
> -	 * the DRM framework requirements. The users of the
> -	 * @drm_writeback_connector control the behaviour of the @encoder
> -	 * by passing the @enc_funcs parameter to
> drm_writeback_connector_init()
> -	 * function.
> -	 * For users of drm_writeback_connector_init_with_encoder(), this
> field
> -	 * is not valid as the encoder is managed within their drivers.
> -	 */
> -	struct drm_encoder encoder;
> -
>  	/**
>  	 * @pixel_formats_blob_ptr:
>  	 *
> @@ -148,13 +137,6 @@ drm_connector_to_writeback(struct drm_connector
> *connector)
>  	return container_of(connector, struct drm_writeback_connector, base);
> }
> 
> -int drm_writeback_connector_init(struct drm_device *dev,
> -				 struct drm_writeback_connector
> *wb_connector,
> -				 const struct drm_connector_funcs
> *con_funcs,
> -				 const struct drm_encoder_helper_funcs
> *enc_helper_funcs,
> -				 const u32 *formats, int n_formats,
> -				 u32 possible_crtcs);
> -
>  int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
>  				struct drm_writeback_connector
> *wb_connector,
>  				struct drm_encoder *enc,
> 
> --
> 2.39.5

RE: [PATCH 8/8] drm: writeback: rename drm_writeback_connector_init_with_encoder()

[Kandpal, Suraj]

> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> > ---
> >   drivers/gpu/drm/drm_writeback.c | 14 +++++++-------
> >   include/drm/drm_writeback.h     | 10 +++++-----
> >   2 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_writeback.c
> > b/drivers/gpu/drm/drm_writeback.c index
> >
> 1a01df91b2c5868e158d489b782f4c57c61a272c..ec2575c4c21b7449707b05953
> 22e
> > 2202a0cf9865 100644
> > --- a/drivers/gpu/drm/drm_writeback.c
> > +++ b/drivers/gpu/drm/drm_writeback.c
> > @@ -235,7 +235,7 @@ static int __drm_writeback_connector_init(struct
> drm_device *dev,
> >   }
> >
> >   /**
> > - * drm_writeback_connector_init_with_encoder - Initialize a writeback
> > connector with
> > + * drm_writeback_connector_init - Initialize a writeback connector
> > + with
> >    * a custom encoder
> 
> If I understood correctly your series you want to reduce the usage of non-
> drmm writeback, so maybe we can add a comment to direct poeple to drmm
> variant to avoid new usage of this API?
> 
> With or without this:
> 
> Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
> 
> >    *
> >    * @dev: DRM device
> > @@ -263,11 +263,11 @@ static int __drm_writeback_connector_init(struct
> drm_device *dev,
> >    *
> >    * Returns: 0 on success, or a negative error code
> >    */
> > -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > -					      struct drm_writeback_connector
> *wb_connector,
> > -					      struct drm_encoder *enc,
> > -					      const struct drm_connector_funcs
> *con_funcs,
> > -					      const u32 *formats, int n_formats)
> > +int drm_writeback_connector_init(struct drm_device *dev,
> > +				 struct drm_writeback_connector
> *wb_connector,
> > +				 const struct drm_connector_funcs
> *con_funcs,
> > +				 struct drm_encoder *enc,
> > +				 const u32 *formats, int n_formats)
> >   {
> >   	struct drm_connector *connector = &wb_connector->base;
> >   	int ret;
> > @@ -284,7 +284,7 @@ int
> > drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> >
> >   	return ret;
> >   }
> > -EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
> > +EXPORT_SYMBOL(drm_writeback_connector_init);
> >
> >   /**
> >    * drm_writeback_connector_cleanup - Cleanup the writeback connector
> > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> > index
> >
> 879ca103320cc225ffb3687419088361315535fc..958466a05e604b387722610fc1
> 1f
> > 9e841316d21b 100644
> > --- a/include/drm/drm_writeback.h
> > +++ b/include/drm/drm_writeback.h
> > @@ -137,11 +137,11 @@ drm_connector_to_writeback(struct
> drm_connector *connector)
> >   	return container_of(connector, struct drm_writeback_connector, base);
> >   }
> >
> > -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > -				struct drm_writeback_connector
> *wb_connector,
> > -				struct drm_encoder *enc,
> > -				const struct drm_connector_funcs
> *con_funcs, const u32 *formats,
> > -				int n_formats);
> > +int drm_writeback_connector_init(struct drm_device *dev,
> > +				 struct drm_writeback_connector
> *wb_connector,
> > +				 const struct drm_connector_funcs
> *con_funcs,
> > +				 struct drm_encoder *enc,
> > +				 const u32 *formats, int n_formats);
> >
> >   int drmm_writeback_connector_init(struct drm_device *dev,
> >   				  struct drm_writeback_connector
> *wb_connector,
> >
> 
> --
> Louis Chauvet, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

RE: [PATCH 3/8] drm/mali: use drmm_writeback_connector_init()

[Kandpal, Suraj]

> Subject: Re: [PATCH 3/8] drm/mali: use drmm_writeback_connector_init()
> 
> 
> 
> Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> > Use drmm_plain_encoder_alloc() to allocate simple encoder and
> > drmm_writeback_connector_init() in order to initialize writeback
> > connector instance.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> > ---
> >   drivers/gpu/drm/arm/malidp_mw.c | 25 ++++++++++++++-----------
> >   1 file changed, 14 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/malidp_mw.c
> > b/drivers/gpu/drm/arm/malidp_mw.c index
> >
> 600af5ad81b15d0c30f9f79588f40cca07437ed8..182275c0c29cd8527c85dfb3e7
> 31
> > 7561da392c46 100644
> > --- a/drivers/gpu/drm/arm/malidp_mw.c
> > +++ b/drivers/gpu/drm/arm/malidp_mw.c
> > @@ -84,11 +84,6 @@ malidp_mw_connector_detect(struct drm_connector
> *connector, bool force)
> >   	return connector_status_connected;
> >   }
> >
> > -static void malidp_mw_connector_destroy(struct drm_connector
> > *connector) -{
> > -	drm_connector_cleanup(connector);
> > -}
> > -
> >   static struct drm_connector_state *
> >   malidp_mw_connector_duplicate_state(struct drm_connector *connector)
> >   {
> > @@ -114,7 +109,6 @@ static const struct drm_connector_funcs
> malidp_mw_connector_funcs = {
> >   	.reset = malidp_mw_connector_reset,
> >   	.detect = malidp_mw_connector_detect,
> >   	.fill_modes = drm_helper_probe_single_connector_modes,
> > -	.destroy = malidp_mw_connector_destroy,
> >   	.atomic_duplicate_state = malidp_mw_connector_duplicate_state,
> >   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> >   };
> > @@ -211,6 +205,7 @@ static u32 *get_writeback_formats(struct malidp_drm
> *malidp, int *n_formats)
> >   int malidp_mw_connector_init(struct drm_device *drm)
> >   {
> >   	struct malidp_drm *malidp = drm_to_malidp(drm);
> > +	struct drm_encoder *encoder;
> >   	u32 *formats;
> >   	int ret, n_formats;
> >
> > @@ -224,11 +219,19 @@ int malidp_mw_connector_init(struct drm_device
> *drm)
> >   	if (!formats)
> >   		return -ENOMEM;
> >
> > -	ret = drm_writeback_connector_init(drm, &malidp->mw_connector,
> > -					   &malidp_mw_connector_funcs,
> > -
> &malidp_mw_encoder_helper_funcs,
> > -					   formats, n_formats,
> > -					   1 << drm_crtc_index(&malidp-
> >crtc));
> > +	encoder = drmm_plain_encoder_alloc(drm, NULL,
> DRM_MODE_ENCODER_VIRTUAL,
> > +					   NULL);
> > +	if (IS_ERR(encoder))
> > +		return PTR_ERR(encoder);
> > +
> > +	drm_encoder_helper_add(encoder,
> &malidp_mw_encoder_helper_funcs);
> > +
> > +	encoder->possible_crtcs = 1 << drm_crtc_index(&malidp->crtc);
> 
> drm_crtc_mask?
> 
> With this:
> 
> Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
> 
> > +
> > +	ret = drmm_writeback_connector_init(drm, &malidp->mw_connector,
> > +					    &malidp_mw_connector_funcs,
> > +					    encoder,
> > +					    formats, n_formats);
> >   	kfree(formats);
> >   	if (ret)
> >   		return ret;
> >
> 
> --
> Louis Chauvet, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

RE: [PATCH 8/8] drm: writeback: rename drm_writeback_connector_init_with_encoder()

[Kandpal, Suraj]

> -----Original Message-----
> From: Kandpal, Suraj
> Sent: Friday, August 8, 2025 10:35 AM
> To: Louis Chauvet <louis.chauvet@bootlin.com>; Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com>; Jani Nikula
> <jani.nikula@linux.intel.com>; Harry Wentland <harry.wentland@amd.com>;
> Leo Li <sunpeng.li@amd.com>; Rodrigo Siqueira <siqueira@igalia.com>; Alex
> Deucher <alexander.deucher@amd.com>; Christian König
> <christian.koenig@amd.com>; David Airlie <airlied@gmail.com>; Simona Vetter
> <simona@ffwll.ch>; Liviu Dudau <liviu.dudau@arm.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>;
> Thomas Zimmermann <tzimmermann@suse.de>; Rob Clark
> <robin.clark@oss.qualcomm.com>; Dmitry Baryshkov <lumag@kernel.org>;
> Abhinav Kumar <abhinav.kumar@linux.dev>; Jessica Zhang
> <jessica.zhang@oss.qualcomm.com>; Sean Paul <sean@poorly.run>; Marijn
> Suijten <marijn.suijten@somainline.org>; Laurent Pinchart
> <laurent.pinchart+renesas@ideasonboard.com>; Tomi Valkeinen
> <tomi.valkeinen+renesas@ideasonboard.com>; Kieran Bingham
> <kieran.bingham+renesas@ideasonboard.com>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Magnus Damm <magnus.damm@gmail.com>;
> Dave Stevenson <dave.stevenson@raspberrypi.com>; Maíra Canal
> <mcanal@igalia.com>; Raspberry Pi Kernel Maintenance <kernel-
> list@raspberrypi.com>
> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org;
> freedreno@lists.freedesktop.org; linux-renesas-soc@vger.kernel.org
> Subject: RE: [PATCH 8/8] drm: writeback: rename
> drm_writeback_connector_init_with_encoder()
> 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> 
> LGTM,
> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

One thing I noticed was after the Rename both drm_writeback_connector_init
And drmm_writeback_connector_init have identical comments both allowing custom encoders
To be used now is that what we were aiming for with the only difference being the 
ret = drmm_add_action_or_reset(dev, drm_writeback_connector_cleanup,
                                       wb_connector);
call ?

Regards,
Suraj Kandpal

> 
> > > ---
> > >   drivers/gpu/drm/drm_writeback.c | 14 +++++++-------
> > >   include/drm/drm_writeback.h     | 10 +++++-----
> > >   2 files changed, 12 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_writeback.c
> > > b/drivers/gpu/drm/drm_writeback.c index
> > >
> >
> 1a01df91b2c5868e158d489b782f4c57c61a272c..ec2575c4c21b7449707b05953
> > 22e
> > > 2202a0cf9865 100644
> > > --- a/drivers/gpu/drm/drm_writeback.c
> > > +++ b/drivers/gpu/drm/drm_writeback.c
> > > @@ -235,7 +235,7 @@ static int __drm_writeback_connector_init(struct
> > drm_device *dev,
> > >   }
> > >
> > >   /**
> > > - * drm_writeback_connector_init_with_encoder - Initialize a
> > > writeback connector with
> > > + * drm_writeback_connector_init - Initialize a writeback connector
> > > + with
> > >    * a custom encoder
> >
> > If I understood correctly your series you want to reduce the usage of
> > non- drmm writeback, so maybe we can add a comment to direct poeple to
> > drmm variant to avoid new usage of this API?
> >
> > With or without this:
> >
> > Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
> >
> > >    *
> > >    * @dev: DRM device
> > > @@ -263,11 +263,11 @@ static int
> > > __drm_writeback_connector_init(struct
> > drm_device *dev,
> > >    *
> > >    * Returns: 0 on success, or a negative error code
> > >    */
> > > -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > > -					      struct drm_writeback_connector
> > *wb_connector,
> > > -					      struct drm_encoder *enc,
> > > -					      const struct drm_connector_funcs
> > *con_funcs,
> > > -					      const u32 *formats, int n_formats)
> > > +int drm_writeback_connector_init(struct drm_device *dev,
> > > +				 struct drm_writeback_connector
> > *wb_connector,
> > > +				 const struct drm_connector_funcs
> > *con_funcs,
> > > +				 struct drm_encoder *enc,
> > > +				 const u32 *formats, int n_formats)
> > >   {
> > >   	struct drm_connector *connector = &wb_connector->base;
> > >   	int ret;
> > > @@ -284,7 +284,7 @@ int
> > > drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > >
> > >   	return ret;
> > >   }
> > > -EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
> > > +EXPORT_SYMBOL(drm_writeback_connector_init);
> > >
> > >   /**
> > >    * drm_writeback_connector_cleanup - Cleanup the writeback
> > > connector diff --git a/include/drm/drm_writeback.h
> > > b/include/drm/drm_writeback.h index
> > >
> >
> 879ca103320cc225ffb3687419088361315535fc..958466a05e604b387722610fc1
> > 1f
> > > 9e841316d21b 100644
> > > --- a/include/drm/drm_writeback.h
> > > +++ b/include/drm/drm_writeback.h
> > > @@ -137,11 +137,11 @@ drm_connector_to_writeback(struct
> > drm_connector *connector)
> > >   	return container_of(connector, struct drm_writeback_connector, base);
> > >   }
> > >
> > > -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > > -				struct drm_writeback_connector
> > *wb_connector,
> > > -				struct drm_encoder *enc,
> > > -				const struct drm_connector_funcs
> > *con_funcs, const u32 *formats,
> > > -				int n_formats);
> > > +int drm_writeback_connector_init(struct drm_device *dev,
> > > +				 struct drm_writeback_connector
> > *wb_connector,
> > > +				 const struct drm_connector_funcs
> > *con_funcs,
> > > +				 struct drm_encoder *enc,
> > > +				 const u32 *formats, int n_formats);
> > >
> > >   int drmm_writeback_connector_init(struct drm_device *dev,
> > >   				  struct drm_writeback_connector
> > *wb_connector,
> > >
> >
> > --
> > Louis Chauvet, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com

Re: [PATCH 5/8] drm/msm/dpu: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

On Fri, Aug 01, 2025 at 04:08:37PM +0200, Louis Chauvet wrote:
> 
> 
> Le 01/08/2025 à 15:51, Dmitry Baryshkov a écrit :
> > Use drmm_plain_encoder_alloc() to allocate simple encoder and
> > drmm_writeback_connector_init() in order to initialize writeback
> > connector instance.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > ---
> >   .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c    | 23 +++++++++++++++-------
> >   1 file changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> > index 8cd37d7b8ae281cbc1fd8cbb243c621174517e23..9986a10e8114680e9da48986f4ca3ce6ec66b8cb 100644
> > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> > @@ -134,7 +134,6 @@ static void rcar_du_wb_conn_reset(struct drm_connector *connector)
> >   static const struct drm_connector_funcs rcar_du_wb_conn_funcs = {
> >   	.reset = rcar_du_wb_conn_reset,
> >   	.fill_modes = drm_helper_probe_single_connector_modes,
> > -	.destroy = drm_connector_cleanup,
> >   	.atomic_duplicate_state = rcar_du_wb_conn_duplicate_state,
> >   	.atomic_destroy_state = rcar_du_wb_conn_destroy_state,
> >   };
> > @@ -202,15 +201,25 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu,
> >   {
> >   	struct drm_writeback_connector *wb_conn = &rcrtc->writeback;
> > +	struct drm_encoder *encoder;
> > +
> > +	encoder = drmm_plain_encoder_alloc(&rcdu->ddev, NULL,
> > +					   DRM_MODE_ENCODER_VIRTUAL, NULL);
> > +	if (IS_ERR(encoder))
> > +		return PTR_ERR(encoder);
> > +
> > +	drm_encoder_helper_add(encoder, &rcar_du_wb_enc_helper_funcs);
> > +
> > +	encoder->possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);
> 
> drm_crtc_mask?

I was just moving the code, but I think this kind of change is fine

> 
> With this:
> 
> Reviewed-by: Louis Chauvet <louis.chauvet>
> 
> > +
> >   	drm_connector_helper_add(&wb_conn->base,
> >   				 &rcar_du_wb_conn_helper_funcs);
> > -	return drm_writeback_connector_init(&rcdu->ddev, wb_conn,
> > -					    &rcar_du_wb_conn_funcs,
> > -					    &rcar_du_wb_enc_helper_funcs,
> > -					    writeback_formats,
> > -					    ARRAY_SIZE(writeback_formats),
> > -					    1 << drm_crtc_index(&rcrtc->crtc));
> > +	return drmm_writeback_connector_init(&rcdu->ddev, wb_conn,
> > +					     &rcar_du_wb_conn_funcs,
> > +					     encoder,
> > +					     writeback_formats,
> > +					     ARRAY_SIZE(writeback_formats));
> >   }
> >   void rcar_du_writeback_setup(struct rcar_du_crtc *rcrtc,
> > 
> 
> -- 
> Louis Chauvet, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

-- 
With best wishes
Dmitry

Re: [PATCH 8/8] drm: writeback: rename drm_writeback_connector_init_with_encoder()

[Dmitry Baryshkov]

On Fri, Aug 08, 2025 at 05:24:19AM +0000, Kandpal, Suraj wrote:
> 
> 
> > -----Original Message-----
> > From: Kandpal, Suraj
> > Sent: Friday, August 8, 2025 10:35 AM
[...]
> > Subject: RE: [PATCH 8/8] drm: writeback: rename
> > drm_writeback_connector_init_with_encoder()

Please fix your email setup to not include this splats.

> > 
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > 
> > LGTM,
> > Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
> 
> One thing I noticed was after the Rename both drm_writeback_connector_init
> And drmm_writeback_connector_init have identical comments both allowing custom encoders
> To be used now is that what we were aiming for with the only difference being the 
> ret = drmm_add_action_or_reset(dev, drm_writeback_connector_cleanup,
>                                        wb_connector);
> call ?

No, there is also a difference in drm_connector_init() vs
drmm_connector_init().


-- 
With best wishes
Dmitry