[PATCH v7 0/7] drm/vkms: Switch all vkms object to DRM managed objects

[PATCH v7 0/7] drm/vkms: Switch all vkms object to DRM managed objects

[Louis Chauvet]

To simplify the memory managment, this series replace all manual drm
object managment by drm-managed one. This way the VKMS code don't have to
manage it directly and the DRM core will handle the object destruction.

No functional changes are intended in this series.

PATCH 1/7: Migrate connector managment to drmm
PATCH 2/7: Migrate encoder managment to drmm
PATCH 3/7: Migrate connector management to drm
PATCH 4/7: Create a helper to initialize drm_writeback_connector (common
           part between drmm and normal variants)
PATCH 5/7: Ensure the proper clean of drm_writeback_connector after a
           failure during init
PATCH 6/7: Create the drmm initialization for drm_writeback_connector
PATCH 7/7: Migrate writeback connector management to drm

For the drmm_writeback helpers, you can find some discussions here [3].

[3]:https://lore.kernel.org/all/20240906-writeback-drmm-v1-1-01ede328182c@bootlin.com/

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
Changes in v7:
- Add Reviewed-by
- Merge PATCH 4/8 and 5/8 as the function were statics
- PATCH 6/7: Remove optional parameter for encoder
- PATCH 7/7: Create dedicated encoder for vkms writeback connector
- Removed useless cleanup in PATCH 6/8
- Link to v6: https://lore.kernel.org/r/20241230-google-vkms-managed-v6-0-15c7d65cd63b@bootlin.com

Changes in v6:
- Added R-by
- Splited the drmm_writeback_connector init in multiple commits + rework 
  how it is done. This time it should not change the behavior of existing 
  drivers
- Link to v5: https://lore.kernel.org/r/20241122-google-vkms-managed-v5-0-1ab60403e960@bootlin.com

Changes in v5:
- Rebased on drm-misc-next
- Link to v4: https://lore.kernel.org/r/20241010-google-vkms-managed-v4-0-ed04a62ad2e3@bootlin.com

Changes in v4:
- No changes for the managed part
- Add the patch to introduce drmm_writeback helpers
- Link to v3: https://lore.kernel.org/r/20240912-google-vkms-managed-v3-0-7708d6ad262d@bootlin.com

Changes in v3:
- As suggested by Maxime, split the managed and the dynamic allocation
  parts in different series
- To reduce the diff in this series, extract the "remove crtc index" part,
  see https://lore.kernel.org/all/20240906-vkms-remove-index-v1-1-3cfedd8ccb2f@bootlin.com/
- Link to v2: https://lore.kernel.org/r/20240827-google-vkms-managed-v2-0-f41104553aeb@bootlin.com

Changes in v2:
- Applied comments from José
- Extract the rename vkms_output -> vkms_crtc to avoid useless changes in
  the last commit
- Extract the rename to_vkms_crtc_state to
  drm_crtc_state_to_vkms_crtc_state to avoid useless changes in last
  commit
- Extract the drm_mode_crtc_set_gamma_size result check in its own commit
- Rebased on drm-misc/drm-misc-next
- Link to v1: https://lore.kernel.org/r/20240814-google-vkms-managed-v1-0-7ab8b8921103@bootlin.com

---
Louis Chauvet (7):
      drm/vkms: Switch to managed for connector
      drm/vkms: Switch to managed for encoder
      drm/vkms: Switch to managed for crtc
      drm: writeback: Create an helper for drm_writeback_connector initialization
      drm: writeback: Add missing cleanup in case of initialization failure
      drm: writeback: Create drmm variants for drm_writeback_connector initialization
      drm/vkms: Switch to managed for writeback connector

 drivers/gpu/drm/drm_writeback.c       | 185 +++++++++++++++++++++++++++++-----
 drivers/gpu/drm/vkms/vkms_crtc.c      |  14 +++
 drivers/gpu/drm/vkms/vkms_drv.c       |   9 --
 drivers/gpu/drm/vkms/vkms_drv.h       |   3 +-
 drivers/gpu/drm/vkms/vkms_output.c    |  26 ++---
 drivers/gpu/drm/vkms/vkms_writeback.c |  21 ++--
 include/drm/drm_writeback.h           |   6 ++
 7 files changed, 201 insertions(+), 63 deletions(-)
---
base-commit: f8a2397baf041a5cee408b082334bb09c7e161df
change-id: 20240521-google-vkms-managed-4aec99461a77

Best regards,
-- 
Louis Chauvet <louis.chauvet@bootlin.com>

[PATCH v7 1/7] drm/vkms: Switch to managed for connector

[Louis Chauvet]

The current VKMS driver uses non-managed function to create connectors. It
is not an issue yet, but in order to support multiple devices easily,
convert this code to use drm and device managed helpers.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_output.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 8f4bd5aef087b459d37d0cbbf90fe0145090917a..570823ecb28f589e6323036590ec05a2f633bc9b 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -3,11 +3,11 @@
 #include "vkms_drv.h"
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_probe_helper.h>
 
 static const struct drm_connector_funcs vkms_connector_funcs = {
 	.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,
@@ -75,8 +75,8 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 		}
 	}
 
-	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
-				 DRM_MODE_CONNECTOR_VIRTUAL);
+	ret = drmm_connector_init(dev, connector, &vkms_connector_funcs,
+				  DRM_MODE_CONNECTOR_VIRTUAL, NULL);
 	if (ret) {
 		DRM_ERROR("Failed to init connector\n");
 		return ret;
@@ -88,7 +88,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 			       DRM_MODE_ENCODER_VIRTUAL, NULL);
 	if (ret) {
 		DRM_ERROR("Failed to init encoder\n");
-		goto err_encoder;
+		return ret;
 	}
 	encoder->possible_crtcs = drm_crtc_mask(crtc);
 
@@ -110,9 +110,5 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 
 err_attach:
 	drm_encoder_cleanup(encoder);
-
-err_encoder:
-	drm_connector_cleanup(connector);
-
 	return ret;
 }

-- 
2.47.1

[PATCH v7 2/7] drm/vkms: Switch to managed for encoder

[Louis Chauvet]

The current VKMS driver uses non-managed function to create encoders. It
is not an issue yet, but in order to support multiple devices easily,
convert this code to use drm and device managed helpers.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_output.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 570823ecb28f589e6323036590ec05a2f633bc9b..ab9affa75b66ce9f00fe025052439405206144ec 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -13,10 +13,6 @@ static const struct drm_connector_funcs vkms_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static const struct drm_encoder_funcs vkms_encoder_funcs = {
-	.destroy = drm_encoder_cleanup,
-};
-
 static int vkms_conn_get_modes(struct drm_connector *connector)
 {
 	int count;
@@ -84,8 +80,8 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 
 	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
 
-	ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
-			       DRM_MODE_ENCODER_VIRTUAL, NULL);
+	ret = drmm_encoder_init(dev, encoder, NULL,
+				DRM_MODE_ENCODER_VIRTUAL, NULL);
 	if (ret) {
 		DRM_ERROR("Failed to init encoder\n");
 		return ret;
@@ -95,7 +91,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 	ret = drm_connector_attach_encoder(connector, encoder);
 	if (ret) {
 		DRM_ERROR("Failed to attach connector to encoder\n");
-		goto err_attach;
+		return ret;
 	}
 
 	if (vkmsdev->config->writeback) {
@@ -108,7 +104,5 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 
 	return 0;
 
-err_attach:
-	drm_encoder_cleanup(encoder);
 	return ret;
 }

-- 
2.47.1

[PATCH v7 3/7] drm/vkms: Switch to managed for crtc

[Louis Chauvet]

The current VKMS driver uses managed function to create crtc, but
don't use it to properly clean the crtc workqueue. It is not an
issue yet, but in order to support multiple devices easily,
convert this code to use drm and device managed helpers.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 14 ++++++++++++++
 drivers/gpu/drm/vkms/vkms_drv.c  |  9 ---------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 28a57ae109fcc05af3fe74f94518c462c09119e3..ace8d293f7da611110c1e117b6cf2f3c9e9b4381 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -6,6 +6,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
+#include <drm/drm_managed.h>
 
 #include "vkms_drv.h"
 
@@ -270,6 +271,14 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
 	.atomic_disable	= vkms_crtc_atomic_disable,
 };
 
+static void vkms_crtc_destroy_workqueue(struct drm_device *dev,
+					void *raw_vkms_out)
+{
+	struct vkms_output *vkms_out = raw_vkms_out;
+
+	destroy_workqueue(vkms_out->composer_workq);
+}
+
 int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 		   struct drm_plane *primary, struct drm_plane *cursor)
 {
@@ -300,5 +309,10 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 	if (!vkms_out->composer_workq)
 		return -ENOMEM;
 
+	ret = drmm_add_action_or_reset(dev, vkms_crtc_destroy_workqueue,
+				       vkms_out);
+	if (ret)
+		return ret;
+
 	return ret;
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index e0409aba93496932b32a130ebb608ee53b1a9c59..7c142bfc3bd9de9556621db3e7f570dc0a4fab3a 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -53,14 +53,6 @@ MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
 
 DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
 
-static void vkms_release(struct drm_device *dev)
-{
-	struct vkms_device *vkms = drm_device_to_vkms_device(dev);
-
-	if (vkms->output.composer_workq)
-		destroy_workqueue(vkms->output.composer_workq);
-}
-
 static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
 {
 	struct drm_device *dev = old_state->dev;
@@ -108,7 +100,6 @@ static const struct drm_debugfs_info vkms_config_debugfs_list[] = {
 
 static const struct drm_driver vkms_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
-	.release		= vkms_release,
 	.fops			= &vkms_driver_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
 	DRM_FBDEV_SHMEM_DRIVER_OPS,

-- 
2.47.1

[PATCH v7 4/7] drm: writeback: Create an helper for drm_writeback_connector initialization

[Louis Chauvet]

As the old drm and the new drmm variants of drm_writeback_connector
requires almost the same initialization, create an internal helper to do
most of the initialization work.

Currently there is no cleanup function for writeback connectors. To allows
implementation of drmm variant of writeback connector, create a cleanup
function that can be used to properly remove all the writeback-specific
properties and allocations.

This also introduce an helper to cleanup only the drm_writeback_connector
properties, so it can be used during initialization to cleanup in case of
failure.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/drm_writeback.c | 130 ++++++++++++++++++++++++++++++++--------
 1 file changed, 104 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 33a3c98a962d1ec49ac4b353902036cf74290ae6..494400b09796d37ed89145da45d5f1e029632de5 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -15,6 +15,7 @@
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_framebuffer.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_property.h>
 #include <drm/drm_writeback.h>
@@ -140,6 +141,22 @@ static int create_writeback_properties(struct drm_device *dev)
 	return 0;
 }
 
+static void delete_writeback_properties(struct drm_device *dev)
+{
+	if (dev->mode_config.writeback_pixel_formats_property) {
+		drm_property_destroy(dev, dev->mode_config.writeback_pixel_formats_property);
+		dev->mode_config.writeback_pixel_formats_property = NULL;
+	}
+	if (dev->mode_config.writeback_out_fence_ptr_property) {
+		drm_property_destroy(dev, dev->mode_config.writeback_out_fence_ptr_property);
+		dev->mode_config.writeback_out_fence_ptr_property = NULL;
+	}
+	if (dev->mode_config.writeback_fb_id_property) {
+		drm_property_destroy(dev, dev->mode_config.writeback_fb_id_property);
+		dev->mode_config.writeback_fb_id_property = NULL;
+	}
+}
+
 static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
 	.destroy = drm_encoder_cleanup,
 };
@@ -202,7 +219,6 @@ EXPORT_SYMBOL(drm_writeback_connector_init);
  * @dev: DRM device
  * @wb_connector: Writeback connector to initialize
  * @enc: handle to the already initialized drm encoder
- * @con_funcs: Connector funcs vtable
  * @formats: Array of supported pixel formats for the writeback engine
  * @n_formats: Length of the formats array
  *
@@ -218,41 +234,31 @@ EXPORT_SYMBOL(drm_writeback_connector_init);
  * assigning the encoder helper functions, possible_crtcs and any other encoder
  * specific operation.
  *
- * Drivers should always use this function instead of drm_connector_init() to
- * set up writeback connectors if they want to manage themselves the lifetime of the
- * associated encoder.
- *
  * 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)
+static int __drm_writeback_connector_init(struct drm_device *dev,
+					  struct drm_writeback_connector *wb_connector,
+					  struct drm_encoder *enc, const u32 *formats,
+					  int n_formats)
 {
-	struct drm_property_blob *blob;
 	struct drm_connector *connector = &wb_connector->base;
 	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_property_blob *blob;
 	int ret = create_writeback_properties(dev);
 
 	if (ret != 0)
 		return ret;
 
-	blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
-					formats);
-	if (IS_ERR(blob))
-		return PTR_ERR(blob);
-
-
 	connector->interlace_allowed = 0;
 
-	ret = drm_connector_init(dev, connector, con_funcs,
-				 DRM_MODE_CONNECTOR_WRITEBACK);
-	if (ret)
-		goto connector_fail;
-
 	ret = drm_connector_attach_encoder(connector, enc);
 	if (ret)
-		goto attach_fail;
+		return ret;
+
+	blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
+					formats);
+	if (IS_ERR(blob))
+		return PTR_ERR(blob);
 
 	INIT_LIST_HEAD(&wb_connector->job_queue);
 	spin_lock_init(&wb_connector->job_lock);
@@ -275,15 +281,87 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
 	wb_connector->pixel_formats_blob_ptr = blob;
 
 	return 0;
+}
+
+/**
+ * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
+ * a custom encoder
+ *
+ * @dev: DRM device
+ * @wb_connector: Writeback connector to initialize
+ * @enc: handle to the already initialized drm encoder
+ * @con_funcs: Connector funcs vtable
+ * @formats: Array of supported pixel formats for the writeback engine
+ * @n_formats: Length of the formats array
+ *
+ * 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.
+ *
+ * This function assumes that the drm_writeback_connector's encoder has already been
+ * created and initialized before invoking this function.
+ *
+ * In addition, this function also assumes that callers of this API will manage
+ * assigning the encoder helper functions, possible_crtcs and any other encoder
+ * specific operation.
+ *
+ * Drivers should always use this function instead of drm_connector_init() to
+ * set up writeback connectors if they want to manage themselves the lifetime of the
+ * associated encoder.
+ *
+ * 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)
+{
+	struct drm_property_blob *blob;
+	struct drm_connector *connector = &wb_connector->base;
+	int ret;
+
+	ret = drm_connector_init(dev, connector, con_funcs,
+				 DRM_MODE_CONNECTOR_WRITEBACK);
+	if (ret)
+		return ret;
+
+	ret = __drm_writeback_connector_init(dev, wb_connector, enc, formats,
+					     n_formats);
+	if (ret)
+		drm_connector_cleanup(connector);
 
-attach_fail:
-	drm_connector_cleanup(connector);
-connector_fail:
-	drm_property_blob_put(blob);
 	return ret;
 }
 EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
 
+/**
+ * drm_writeback_connector_cleanup - Cleanup the writeback connector
+ * @dev: DRM device
+ * @wb_connector: Pointer to the writeback connector to clean up
+ *
+ * This will decrement the reference counter of blobs and destroy properties. It
+ * will also clean the remaining jobs in this writeback connector. Caution: This helper will not
+ * clean up the attached encoder and the drm_connector.
+ */
+static void drm_writeback_connector_cleanup(struct drm_device *dev,
+					    struct drm_writeback_connector *wb_connector)
+{
+	unsigned long flags;
+	struct drm_writeback_job *pos, *n;
+
+	delete_writeback_properties(dev);
+	drm_property_blob_put(wb_connector->pixel_formats_blob_ptr);
+
+	spin_lock_irqsave(&wb_connector->job_lock, flags);
+	list_for_each_entry_safe(pos, n, &wb_connector->job_queue, list_entry) {
+		drm_writeback_cleanup_job(pos);
+		list_del(&pos->list_entry);
+	}
+	spin_unlock_irqrestore(&wb_connector->job_lock, flags);
+}
+
 int drm_writeback_set_fb(struct drm_connector_state *conn_state,
 			 struct drm_framebuffer *fb)
 {

-- 
2.47.1

[PATCH v7 5/7] drm: writeback: Add missing cleanup in case of initialization failure

[Louis Chauvet]

The current implementation of drm_writeback_connector initialization does
not properly clean up all resources in case of failure (allocated
properties and possible_encoders). Add this cleaning in case of failure.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/drm_writeback.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 494400b09796d37ed89145da45d5f1e029632de5..b767184289222353489b21416a3329352c3bdfa0 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -247,18 +247,20 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
 	int ret = create_writeback_properties(dev);
 
 	if (ret != 0)
-		return ret;
+		goto failed_properties;
 
 	connector->interlace_allowed = 0;
 
 	ret = drm_connector_attach_encoder(connector, enc);
 	if (ret)
-		return ret;
+		goto failed_properties;
 
 	blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
 					formats);
-	if (IS_ERR(blob))
-		return PTR_ERR(blob);
+	if (IS_ERR(blob)) {
+		ret = PTR_ERR(blob);
+		goto failed_properties;
+	}
 
 	INIT_LIST_HEAD(&wb_connector->job_queue);
 	spin_lock_init(&wb_connector->job_lock);
@@ -281,6 +283,9 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
 	wb_connector->pixel_formats_blob_ptr = blob;
 
 	return 0;
+failed_properties:
+	delete_writeback_properties(dev);
+	return ret;
 }
 
 /**

-- 
2.47.1

[PATCH v7 6/7] drm: writeback: Create drmm variants for drm_writeback_connector initialization

[Louis Chauvet]

To allows driver to only use drmm objects, add helper to create
drm_writeback_connectors with automated lifetime management.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/drm_writeback.c | 48 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_writeback.h     |  6 ++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index b767184289222353489b21416a3329352c3bdfa0..1d0f35ff96396ee6e887ac987d6075e122e89b1a 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -367,6 +367,54 @@ static void drm_writeback_connector_cleanup(struct drm_device *dev,
 	spin_unlock_irqrestore(&wb_connector->job_lock, flags);
 }
 
+/**
+ * drmm_writeback_connector_init - Initialize a writeback connector with
+ * a custom encoder
+ *
+ * @dev: DRM device
+ * @wb_connector: Writeback connector to initialize
+ * @con_funcs: Connector funcs vtable
+ * @enc: Encoder to connect this writeback connector
+ * @formats: Array of supported pixel formats for the writeback engine
+ * @n_formats: Length of the formats array
+ *
+ * This function initialize a writeback connector and register its cleanup.
+ *
+ * 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.
+ *
+ * Returns: 0 on success, or a negative error code
+ */
+int drmm_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;
+
+	ret = drmm_connector_init(dev, connector, con_funcs,
+				  DRM_MODE_CONNECTOR_WRITEBACK, NULL);
+	if (ret)
+		return ret;
+
+	ret = __drm_writeback_connector_init(dev, wb_connector, enc, formats,
+					     n_formats);
+	if (ret)
+		return ret;
+
+	ret = drmm_add_action_or_reset(dev, (void *)drm_writeback_connector_cleanup,
+				       wb_connector);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL(drmm_writeback_connector_init);
+
 int drm_writeback_set_fb(struct drm_connector_state *conn_state,
 			 struct drm_framebuffer *fb)
 {
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index 17e576c80169a820e8d5587b229b2cc2ee369a18..c380a7b8f55a3616fa070c037d5cc653b0061fe6 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -161,6 +161,12 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
 				const struct drm_connector_funcs *con_funcs, const u32 *formats,
 				int n_formats);
 
+int drmm_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 drm_writeback_set_fb(struct drm_connector_state *conn_state,
 			 struct drm_framebuffer *fb);
 

-- 
2.47.1

[PATCH v7 7/7] drm/vkms: Switch to managed for writeback connector

[Louis Chauvet]

The current VKMS driver uses non-managed function to create
writeback connectors. It is not an issue yet, but in order
to support multiple devices easily, convert this code to
use drm and device managed helpers.

Reviewed-by: José Expósito <jose.exposito89@gmail.com>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_drv.h       |  3 ++-
 drivers/gpu/drm/vkms/vkms_output.c    |  2 +-
 drivers/gpu/drm/vkms/vkms_writeback.c | 21 +++++++++++++--------
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 00541eff3d1b0aa4b374fb94c8fe34932df31509..46ac36aebb27ce8d9018224735007c1b3fe7d0a5 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -179,6 +179,7 @@ struct vkms_output {
 	struct drm_encoder encoder;
 	struct drm_connector connector;
 	struct drm_writeback_connector wb_connector;
+	struct drm_encoder wb_encoder;
 	struct hrtimer vblank_hrtimer;
 	ktime_t period_ns;
 	struct workqueue_struct *composer_workq;
@@ -275,6 +276,6 @@ void vkms_set_composer(struct vkms_output *out, bool enabled);
 void vkms_writeback_row(struct vkms_writeback_job *wb, const struct line_buffer *src_buffer, int y);
 
 /* Writeback */
-int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
+int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc);
 
 #endif /* _VKMS_DRV_H_ */
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index ab9affa75b66ce9f00fe025052439405206144ec..de817e2794860f9071a71b3631460691e0d73a85 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -95,7 +95,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 	}
 
 	if (vkmsdev->config->writeback) {
-		writeback = vkms_enable_writeback_connector(vkmsdev);
+		writeback = vkms_enable_writeback_connector(vkmsdev, crtc);
 		if (writeback)
 			DRM_ERROR("Failed to init writeback connector\n");
 	}
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
index 79918b44fedd7ae2451d1d530fc6d5aabf2d99a3..981975c2b0a0c75e4a3aceca2a965f5876ae0a8f 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -24,7 +24,6 @@ static const u32 vkms_wb_formats[] = {
 
 static const struct drm_connector_funcs vkms_wb_connector_funcs = {
 	.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,
@@ -163,16 +162,22 @@ static const struct drm_connector_helper_funcs vkms_wb_conn_helper_funcs = {
 	.atomic_check = vkms_wb_atomic_check,
 };
 
-int vkms_enable_writeback_connector(struct vkms_device *vkmsdev)
+int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc)
 {
 	struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
+	int ret;
+
+	ret = drmm_encoder_init(&vkmsdev->drm, &vkmsdev->output.wb_encoder,
+				NULL, DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (ret)
+		return ret;
+	vkmsdev->output.wb_encoder.possible_crtcs |= drm_crtc_mask(crtc);
 
 	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
 
-	return drm_writeback_connector_init(&vkmsdev->drm, wb,
-					    &vkms_wb_connector_funcs,
-					    NULL,
-					    vkms_wb_formats,
-					    ARRAY_SIZE(vkms_wb_formats),
-					    1);
+	return drmm_writeback_connector_init(&vkmsdev->drm, wb,
+					     &vkms_wb_connector_funcs,
+					     &vkmsdev->output.wb_encoder,
+					     vkms_wb_formats,
+					     ARRAY_SIZE(vkms_wb_formats));
 }

-- 
2.47.1

Re: [PATCH v7 7/7] drm/vkms: Switch to managed for writeback connector

[Louis Chauvet]

On 13/01/25 - 18:09, Louis Chauvet wrote:
> The current VKMS driver uses non-managed function to create
> writeback connectors. It is not an issue yet, but in order
> to support multiple devices easily, convert this code to
> use drm and device managed helpers.
> 
> Reviewed-by: José Expósito <jose.exposito89@gmail.com>

Hi,

Sorry José, I forgot to remove your Reviewed-by, the changes made here are 
not trivials. Can I keep it or do you have any comments ?

Sorry,
Louis Chauvet

> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  drivers/gpu/drm/vkms/vkms_drv.h       |  3 ++-
>  drivers/gpu/drm/vkms/vkms_output.c    |  2 +-
>  drivers/gpu/drm/vkms/vkms_writeback.c | 21 +++++++++++++--------
>  3 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 00541eff3d1b0aa4b374fb94c8fe34932df31509..46ac36aebb27ce8d9018224735007c1b3fe7d0a5 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -179,6 +179,7 @@ struct vkms_output {
>  	struct drm_encoder encoder;
>  	struct drm_connector connector;
>  	struct drm_writeback_connector wb_connector;
> +	struct drm_encoder wb_encoder;
>  	struct hrtimer vblank_hrtimer;
>  	ktime_t period_ns;
>  	struct workqueue_struct *composer_workq;
> @@ -275,6 +276,6 @@ void vkms_set_composer(struct vkms_output *out, bool enabled);
>  void vkms_writeback_row(struct vkms_writeback_job *wb, const struct line_buffer *src_buffer, int y);
>  
>  /* Writeback */
> -int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
> +int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc);
>  
>  #endif /* _VKMS_DRV_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index ab9affa75b66ce9f00fe025052439405206144ec..de817e2794860f9071a71b3631460691e0d73a85 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -95,7 +95,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>  	}
>  
>  	if (vkmsdev->config->writeback) {
> -		writeback = vkms_enable_writeback_connector(vkmsdev);
> +		writeback = vkms_enable_writeback_connector(vkmsdev, crtc);
>  		if (writeback)
>  			DRM_ERROR("Failed to init writeback connector\n");
>  	}
> diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
> index 79918b44fedd7ae2451d1d530fc6d5aabf2d99a3..981975c2b0a0c75e4a3aceca2a965f5876ae0a8f 100644
> --- a/drivers/gpu/drm/vkms/vkms_writeback.c
> +++ b/drivers/gpu/drm/vkms/vkms_writeback.c
> @@ -24,7 +24,6 @@ static const u32 vkms_wb_formats[] = {
>  
>  static const struct drm_connector_funcs vkms_wb_connector_funcs = {
>  	.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,
> @@ -163,16 +162,22 @@ static const struct drm_connector_helper_funcs vkms_wb_conn_helper_funcs = {
>  	.atomic_check = vkms_wb_atomic_check,
>  };
>  
> -int vkms_enable_writeback_connector(struct vkms_device *vkmsdev)
> +int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc)
>  {
>  	struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
> +	int ret;
> +
> +	ret = drmm_encoder_init(&vkmsdev->drm, &vkmsdev->output.wb_encoder,
> +				NULL, DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (ret)
> +		return ret;
> +	vkmsdev->output.wb_encoder.possible_crtcs |= drm_crtc_mask(crtc);
>  
>  	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
>  
> -	return drm_writeback_connector_init(&vkmsdev->drm, wb,
> -					    &vkms_wb_connector_funcs,
> -					    NULL,
> -					    vkms_wb_formats,
> -					    ARRAY_SIZE(vkms_wb_formats),
> -					    1);
> +	return drmm_writeback_connector_init(&vkmsdev->drm, wb,
> +					     &vkms_wb_connector_funcs,
> +					     &vkmsdev->output.wb_encoder,
> +					     vkms_wb_formats,
> +					     ARRAY_SIZE(vkms_wb_formats));
>  }
> 
> -- 
> 2.47.1
> 

Re: [PATCH v7 1/7] drm/vkms: Switch to managed for connector

[Thomas Zimmermann]

Am 13.01.25 um 18:09 schrieb Louis Chauvet:
> The current VKMS driver uses non-managed function to create connectors. It
> is not an issue yet, but in order to support multiple devices easily,
> convert this code to use drm and device managed helpers.
>
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> Reviewed-by: Maíra Canal <mcanal@igalia.com>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/vkms/vkms_output.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 8f4bd5aef087b459d37d0cbbf90fe0145090917a..570823ecb28f589e6323036590ec05a2f633bc9b 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -3,11 +3,11 @@
>   #include "vkms_drv.h"
>   #include <drm/drm_atomic_helper.h>
>   #include <drm/drm_edid.h>
> +#include <drm/drm_managed.h>
>   #include <drm/drm_probe_helper.h>
>   
>   static const struct drm_connector_funcs vkms_connector_funcs = {
>   	.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,
> @@ -75,8 +75,8 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>   		}
>   	}
>   
> -	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
> -				 DRM_MODE_CONNECTOR_VIRTUAL);
> +	ret = drmm_connector_init(dev, connector, &vkms_connector_funcs,
> +				  DRM_MODE_CONNECTOR_VIRTUAL, NULL);
>   	if (ret) {
>   		DRM_ERROR("Failed to init connector\n");
>   		return ret;
> @@ -88,7 +88,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>   			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>   	if (ret) {
>   		DRM_ERROR("Failed to init encoder\n");
> -		goto err_encoder;
> +		return ret;
>   	}
>   	encoder->possible_crtcs = drm_crtc_mask(crtc);
>   
> @@ -110,9 +110,5 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>   
>   err_attach:
>   	drm_encoder_cleanup(encoder);
> -
> -err_encoder:
> -	drm_connector_cleanup(connector);
> -
>   	return ret;
>   }
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v7 2/7] drm/vkms: Switch to managed for encoder

[Thomas Zimmermann]

Am 13.01.25 um 18:09 schrieb Louis Chauvet:
> The current VKMS driver uses non-managed function to create encoders. It
> is not an issue yet, but in order to support multiple devices easily,
> convert this code to use drm and device managed helpers.
>
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> Reviewed-by: Maíra Canal <mcanal@igalia.com>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/vkms/vkms_output.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 570823ecb28f589e6323036590ec05a2f633bc9b..ab9affa75b66ce9f00fe025052439405206144ec 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -13,10 +13,6 @@ static const struct drm_connector_funcs vkms_connector_funcs = {
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>   };
>   
> -static const struct drm_encoder_funcs vkms_encoder_funcs = {
> -	.destroy = drm_encoder_cleanup,
> -};
> -
>   static int vkms_conn_get_modes(struct drm_connector *connector)
>   {
>   	int count;
> @@ -84,8 +80,8 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>   
>   	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
>   
> -	ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
> -			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	ret = drmm_encoder_init(dev, encoder, NULL,
> +				DRM_MODE_ENCODER_VIRTUAL, NULL);
>   	if (ret) {
>   		DRM_ERROR("Failed to init encoder\n");
>   		return ret;
> @@ -95,7 +91,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>   	ret = drm_connector_attach_encoder(connector, encoder);
>   	if (ret) {
>   		DRM_ERROR("Failed to attach connector to encoder\n");
> -		goto err_attach;
> +		return ret;
>   	}
>   
>   	if (vkmsdev->config->writeback) {
> @@ -108,7 +104,5 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>   
>   	return 0;
>   
> -err_attach:
> -	drm_encoder_cleanup(encoder);
>   	return ret;
>   }
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v7 3/7] drm/vkms: Switch to managed for crtc

[Thomas Zimmermann]

Hi


Am 13.01.25 um 18:09 schrieb Louis Chauvet:
> The current VKMS driver uses managed function to create crtc, but
> don't use it to properly clean the crtc workqueue. It is not an
> issue yet, but in order to support multiple devices easily,
> convert this code to use drm and device managed helpers.
>
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> Reviewed-by: Maíra Canal <mcanal@igalia.com>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>   drivers/gpu/drm/vkms/vkms_crtc.c | 14 ++++++++++++++
>   drivers/gpu/drm/vkms/vkms_drv.c  |  9 ---------
>   2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 28a57ae109fcc05af3fe74f94518c462c09119e3..ace8d293f7da611110c1e117b6cf2f3c9e9b4381 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -6,6 +6,7 @@
>   #include <drm/drm_atomic_helper.h>
>   #include <drm/drm_probe_helper.h>
>   #include <drm/drm_vblank.h>
> +#include <drm/drm_managed.h>

Alphabetical order please.

>   
>   #include "vkms_drv.h"
>   
> @@ -270,6 +271,14 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
>   	.atomic_disable	= vkms_crtc_atomic_disable,
>   };
>   
> +static void vkms_crtc_destroy_workqueue(struct drm_device *dev,
> +					void *raw_vkms_out)
> +{
> +	struct vkms_output *vkms_out = raw_vkms_out;
> +
> +	destroy_workqueue(vkms_out->composer_workq);
> +}
> +

This could be implemented in drm_managed.c. 
drmm_alloc_ordered_workqueue() would call alloc_ordered_workqueue() and 
set up the managed callback as well.

Best regards
Thomas

>   int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>   		   struct drm_plane *primary, struct drm_plane *cursor)
>   {
> @@ -300,5 +309,10 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>   	if (!vkms_out->composer_workq)
>   		return -ENOMEM;
>   
> +	ret = drmm_add_action_or_reset(dev, vkms_crtc_destroy_workqueue,
> +				       vkms_out);
> +	if (ret)
> +		return ret;
> +
>   	return ret;
>   }
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index e0409aba93496932b32a130ebb608ee53b1a9c59..7c142bfc3bd9de9556621db3e7f570dc0a4fab3a 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -53,14 +53,6 @@ MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
>   
>   DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
>   
> -static void vkms_release(struct drm_device *dev)
> -{
> -	struct vkms_device *vkms = drm_device_to_vkms_device(dev);
> -
> -	if (vkms->output.composer_workq)
> -		destroy_workqueue(vkms->output.composer_workq);
> -}
> -
>   static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
>   {
>   	struct drm_device *dev = old_state->dev;
> @@ -108,7 +100,6 @@ static const struct drm_debugfs_info vkms_config_debugfs_list[] = {
>   
>   static const struct drm_driver vkms_driver = {
>   	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
> -	.release		= vkms_release,
>   	.fops			= &vkms_driver_fops,
>   	DRM_GEM_SHMEM_DRIVER_OPS,
>   	DRM_FBDEV_SHMEM_DRIVER_OPS,
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v7 3/7] drm/vkms: Switch to managed for crtc

[Louis Chauvet]

On 14/01/25 - 10:06, Thomas Zimmermann wrote:
> Hi
> 
> 
> Am 13.01.25 um 18:09 schrieb Louis Chauvet:
> > The current VKMS driver uses managed function to create crtc, but
> > don't use it to properly clean the crtc workqueue. It is not an
> > issue yet, but in order to support multiple devices easily,
> > convert this code to use drm and device managed helpers.
> > 
> > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> > Reviewed-by: Maíra Canal <mcanal@igalia.com>
> > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> > ---
> >   drivers/gpu/drm/vkms/vkms_crtc.c | 14 ++++++++++++++
> >   drivers/gpu/drm/vkms/vkms_drv.c  |  9 ---------
> >   2 files changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > index 28a57ae109fcc05af3fe74f94518c462c09119e3..ace8d293f7da611110c1e117b6cf2f3c9e9b4381 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -6,6 +6,7 @@
> >   #include <drm/drm_atomic_helper.h>
> >   #include <drm/drm_probe_helper.h>
> >   #include <drm/drm_vblank.h>
> > +#include <drm/drm_managed.h>
> 
> Alphabetical order please.
> 
> >   #include "vkms_drv.h"
> > @@ -270,6 +271,14 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
> >   	.atomic_disable	= vkms_crtc_atomic_disable,
> >   };
> > +static void vkms_crtc_destroy_workqueue(struct drm_device *dev,
> > +					void *raw_vkms_out)
> > +{
> > +	struct vkms_output *vkms_out = raw_vkms_out;
> > +
> > +	destroy_workqueue(vkms_out->composer_workq);
> > +}
> > +
> 
> This could be implemented in drm_managed.c. drmm_alloc_ordered_workqueue()
> would call alloc_ordered_workqueue() and set up the managed callback as
> well.

Hello Thomas,

Thanks for this review. For the next iteration, I will add the macro 
drmm_alloc_ordered_workqueue:

void __drmm_destroy_workqueue(struct drm_device *device, void* res)
{
	struct workqueue_struct *wq = res;

	destroy_workqueue(wq);
}
EXPORT_SYMBOL(__drmm_destroy_workqueue);

#define drmm_alloc_ordered_workqueue(dev, fmt, flags, args...)					\
	({											\
		struct workqueue_struct *wq = alloc_ordered_workqueue(fmt, flags, ##args);	\
		wq ? ({										\
			int ret = drmm_add_action_or_reset(dev, __drmm_destroy_workqueue, wq);	\
			ret ? ERR_PTR(ret) : wq;						\
		}) :										\
			wq;									\
	})

Besides this, is there anything else you noticed that I should change 
for the next iteration in the remaining patches?

Thanks,
Louis Chauvet

> Best regards
> Thomas
> 
> >   int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> >   		   struct drm_plane *primary, struct drm_plane *cursor)
> >   {
> > @@ -300,5 +309,10 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> >   	if (!vkms_out->composer_workq)
> >   		return -ENOMEM;
> > +	ret = drmm_add_action_or_reset(dev, vkms_crtc_destroy_workqueue,
> > +				       vkms_out);
> > +	if (ret)
> > +		return ret;
> > +
> >   	return ret;
> >   }
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > index e0409aba93496932b32a130ebb608ee53b1a9c59..7c142bfc3bd9de9556621db3e7f570dc0a4fab3a 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -53,14 +53,6 @@ MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
> >   DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
> > -static void vkms_release(struct drm_device *dev)
> > -{
> > -	struct vkms_device *vkms = drm_device_to_vkms_device(dev);
> > -
> > -	if (vkms->output.composer_workq)
> > -		destroy_workqueue(vkms->output.composer_workq);
> > -}
> > -
> >   static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
> >   {
> >   	struct drm_device *dev = old_state->dev;
> > @@ -108,7 +100,6 @@ static const struct drm_debugfs_info vkms_config_debugfs_list[] = {
> >   static const struct drm_driver vkms_driver = {
> >   	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
> > -	.release		= vkms_release,
> >   	.fops			= &vkms_driver_fops,
> >   	DRM_GEM_SHMEM_DRIVER_OPS,
> >   	DRM_FBDEV_SHMEM_DRIVER_OPS,
> > 
> 
> -- 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
> 

Re: [PATCH v7 3/7] drm/vkms: Switch to managed for crtc

[Thomas Zimmermann]

Hi


Am 14.01.25 um 14:19 schrieb Louis Chauvet:
> On 14/01/25 - 10:06, Thomas Zimmermann wrote:
>> Hi
>>
>>
>> Am 13.01.25 um 18:09 schrieb Louis Chauvet:
>>> The current VKMS driver uses managed function to create crtc, but
>>> don't use it to properly clean the crtc workqueue. It is not an
>>> issue yet, but in order to support multiple devices easily,
>>> convert this code to use drm and device managed helpers.
>>>
>>> Reviewed-by: Maxime Ripard <mripard@kernel.org>
>>> Reviewed-by: Maíra Canal <mcanal@igalia.com>
>>> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
>>> ---
>>>    drivers/gpu/drm/vkms/vkms_crtc.c | 14 ++++++++++++++
>>>    drivers/gpu/drm/vkms/vkms_drv.c  |  9 ---------
>>>    2 files changed, 14 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
>>> index 28a57ae109fcc05af3fe74f94518c462c09119e3..ace8d293f7da611110c1e117b6cf2f3c9e9b4381 100644
>>> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
>>> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
>>> @@ -6,6 +6,7 @@
>>>    #include <drm/drm_atomic_helper.h>
>>>    #include <drm/drm_probe_helper.h>
>>>    #include <drm/drm_vblank.h>
>>> +#include <drm/drm_managed.h>
>> Alphabetical order please.
>>
>>>    #include "vkms_drv.h"
>>> @@ -270,6 +271,14 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
>>>    	.atomic_disable	= vkms_crtc_atomic_disable,
>>>    };
>>> +static void vkms_crtc_destroy_workqueue(struct drm_device *dev,
>>> +					void *raw_vkms_out)
>>> +{
>>> +	struct vkms_output *vkms_out = raw_vkms_out;
>>> +
>>> +	destroy_workqueue(vkms_out->composer_workq);
>>> +}
>>> +
>> This could be implemented in drm_managed.c. drmm_alloc_ordered_workqueue()
>> would call alloc_ordered_workqueue() and set up the managed callback as
>> well.
> Hello Thomas,
>
> Thanks for this review. For the next iteration, I will add the macro
> drmm_alloc_ordered_workqueue:
>
> void __drmm_destroy_workqueue(struct drm_device *device, void* res)
> {
> 	struct workqueue_struct *wq = res;
>
> 	destroy_workqueue(wq);
> }
> EXPORT_SYMBOL(__drmm_destroy_workqueue);
>
> #define drmm_alloc_ordered_workqueue(dev, fmt, flags, args...)					\
> 	({											\
> 		struct workqueue_struct *wq = alloc_ordered_workqueue(fmt, flags, ##args);	\
> 		wq ? ({										\
> 			int ret = drmm_add_action_or_reset(dev, __drmm_destroy_workqueue, wq);	\
> 			ret ? ERR_PTR(ret) : wq;						\
> 		}) :										\
> 			wq;									\
> 	})

Great. There are quite a few work queues in DRM drivers. Hopefully 
soemone else will find this useful. With this change and the fixed 
include sorting, you can add my R-b to this patch.

>
> Besides this, is there anything else you noticed that I should change
> for the next iteration in the remaining patches?

I've looked through the other patches and they look good to me. Feel 
free to add Acked-by: Thomas Zimmermann <tzimmermann@suse.de> to patches 
4 to 7.

Best regards
Thomas

>
> Thanks,
> Louis Chauvet
>
>> Best regards
>> Thomas
>>
>>>    int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>>>    		   struct drm_plane *primary, struct drm_plane *cursor)
>>>    {
>>> @@ -300,5 +309,10 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>>>    	if (!vkms_out->composer_workq)
>>>    		return -ENOMEM;
>>> +	ret = drmm_add_action_or_reset(dev, vkms_crtc_destroy_workqueue,
>>> +				       vkms_out);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>>    	return ret;
>>>    }
>>> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
>>> index e0409aba93496932b32a130ebb608ee53b1a9c59..7c142bfc3bd9de9556621db3e7f570dc0a4fab3a 100644
>>> --- a/drivers/gpu/drm/vkms/vkms_drv.c
>>> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
>>> @@ -53,14 +53,6 @@ MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
>>>    DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
>>> -static void vkms_release(struct drm_device *dev)
>>> -{
>>> -	struct vkms_device *vkms = drm_device_to_vkms_device(dev);
>>> -
>>> -	if (vkms->output.composer_workq)
>>> -		destroy_workqueue(vkms->output.composer_workq);
>>> -}
>>> -
>>>    static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
>>>    {
>>>    	struct drm_device *dev = old_state->dev;
>>> @@ -108,7 +100,6 @@ static const struct drm_debugfs_info vkms_config_debugfs_list[] = {
>>>    static const struct drm_driver vkms_driver = {
>>>    	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
>>> -	.release		= vkms_release,
>>>    	.fops			= &vkms_driver_fops,
>>>    	DRM_GEM_SHMEM_DRIVER_OPS,
>>>    	DRM_FBDEV_SHMEM_DRIVER_OPS,
>>>
>> -- 
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v7 4/7] drm: writeback: Create an helper for drm_writeback_connector initialization

[kernel test robot]

Hi Louis,

kernel test robot noticed the following build warnings:

[auto build test WARNING on f8a2397baf041a5cee408b082334bb09c7e161df]

url:    https://github.com/intel-lab-lkp/linux/commits/Louis-Chauvet/drm-vkms-Switch-to-managed-for-connector/20250114-011112
base:   f8a2397baf041a5cee408b082334bb09c7e161df
patch link:    https://lore.kernel.org/r/20250113-google-vkms-managed-v7-4-4f81d1893e0b%40bootlin.com
patch subject: [PATCH v7 4/7] drm: writeback: Create an helper for drm_writeback_connector initialization
config: i386-randconfig-014-20250115 (https://download.01.org/0day-ci/archive/20250115/202501150938.a3MspUwj-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250115/202501150938.a3MspUwj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501150938.a3MspUwj-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/drm_writeback.c: In function 'drm_writeback_connector_init_with_encoder':
>> drivers/gpu/drm/drm_writeback.c:321:35: warning: unused variable 'blob' [-Wunused-variable]
     321 |         struct drm_property_blob *blob;
         |                                   ^~~~
   drivers/gpu/drm/drm_writeback.c: At top level:
   drivers/gpu/drm/drm_writeback.c:348:13: warning: 'drm_writeback_connector_cleanup' defined but not used [-Wunused-function]
     348 | static void drm_writeback_connector_cleanup(struct drm_device *dev,
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
>> drivers/gpu/drm/drm_writeback.c:243: warning: expecting prototype for drm_writeback_connector_init_with_encoder(). Prototype was for __drm_writeback_connector_init() instead


vim +/blob +321 drivers/gpu/drm/drm_writeback.c

   214	
   215	/**
   216	 * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
   217	 * a custom encoder
   218	 *
   219	 * @dev: DRM device
   220	 * @wb_connector: Writeback connector to initialize
   221	 * @enc: handle to the already initialized drm encoder
   222	 * @formats: Array of supported pixel formats for the writeback engine
   223	 * @n_formats: Length of the formats array
   224	 *
   225	 * This function creates the writeback-connector-specific properties if they
   226	 * have not been already created, initializes the connector as
   227	 * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
   228	 * values.
   229	 *
   230	 * This function assumes that the drm_writeback_connector's encoder has already been
   231	 * created and initialized before invoking this function.
   232	 *
   233	 * In addition, this function also assumes that callers of this API will manage
   234	 * assigning the encoder helper functions, possible_crtcs and any other encoder
   235	 * specific operation.
   236	 *
   237	 * Returns: 0 on success, or a negative error code
   238	 */
   239	static int __drm_writeback_connector_init(struct drm_device *dev,
   240						  struct drm_writeback_connector *wb_connector,
   241						  struct drm_encoder *enc, const u32 *formats,
   242						  int n_formats)
 > 243	{
   244		struct drm_connector *connector = &wb_connector->base;
   245		struct drm_mode_config *config = &dev->mode_config;
   246		struct drm_property_blob *blob;
   247		int ret = create_writeback_properties(dev);
   248	
   249		if (ret != 0)
   250			return ret;
   251	
   252		connector->interlace_allowed = 0;
   253	
   254		ret = drm_connector_attach_encoder(connector, enc);
   255		if (ret)
   256			return ret;
   257	
   258		blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
   259						formats);
   260		if (IS_ERR(blob))
   261			return PTR_ERR(blob);
   262	
   263		INIT_LIST_HEAD(&wb_connector->job_queue);
   264		spin_lock_init(&wb_connector->job_lock);
   265	
   266		wb_connector->fence_context = dma_fence_context_alloc(1);
   267		spin_lock_init(&wb_connector->fence_lock);
   268		snprintf(wb_connector->timeline_name,
   269			 sizeof(wb_connector->timeline_name),
   270			 "CONNECTOR:%d-%s", connector->base.id, connector->name);
   271	
   272		drm_object_attach_property(&connector->base,
   273					   config->writeback_out_fence_ptr_property, 0);
   274	
   275		drm_object_attach_property(&connector->base,
   276					   config->writeback_fb_id_property, 0);
   277	
   278		drm_object_attach_property(&connector->base,
   279					   config->writeback_pixel_formats_property,
   280					   blob->base.id);
   281		wb_connector->pixel_formats_blob_ptr = blob;
   282	
   283		return 0;
   284	}
   285	
   286	/**
   287	 * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
   288	 * a custom encoder
   289	 *
   290	 * @dev: DRM device
   291	 * @wb_connector: Writeback connector to initialize
   292	 * @enc: handle to the already initialized drm encoder
   293	 * @con_funcs: Connector funcs vtable
   294	 * @formats: Array of supported pixel formats for the writeback engine
   295	 * @n_formats: Length of the formats array
   296	 *
   297	 * This function creates the writeback-connector-specific properties if they
   298	 * have not been already created, initializes the connector as
   299	 * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
   300	 * values.
   301	 *
   302	 * This function assumes that the drm_writeback_connector's encoder has already been
   303	 * created and initialized before invoking this function.
   304	 *
   305	 * In addition, this function also assumes that callers of this API will manage
   306	 * assigning the encoder helper functions, possible_crtcs and any other encoder
   307	 * specific operation.
   308	 *
   309	 * Drivers should always use this function instead of drm_connector_init() to
   310	 * set up writeback connectors if they want to manage themselves the lifetime of the
   311	 * associated encoder.
   312	 *
   313	 * Returns: 0 on success, or a negative error code
   314	 */
   315	int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
   316						      struct drm_writeback_connector *wb_connector,
   317						      struct drm_encoder *enc,
   318						      const struct drm_connector_funcs *con_funcs,
   319						      const u32 *formats, int n_formats)
   320	{
 > 321		struct drm_property_blob *blob;
   322		struct drm_connector *connector = &wb_connector->base;
   323		int ret;
   324	
   325		ret = drm_connector_init(dev, connector, con_funcs,
   326					 DRM_MODE_CONNECTOR_WRITEBACK);
   327		if (ret)
   328			return ret;
   329	
   330		ret = __drm_writeback_connector_init(dev, wb_connector, enc, formats,
   331						     n_formats);
   332		if (ret)
   333			drm_connector_cleanup(connector);
   334	
   335		return ret;
   336	}
   337	EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
   338	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v7 7/7] drm/vkms: Switch to managed for writeback connector

[José Expósito]

On Mon, Jan 13, 2025 at 06:12:03PM +0100, Louis Chauvet wrote:
> On 13/01/25 - 18:09, Louis Chauvet wrote:
> > The current VKMS driver uses non-managed function to create
> > writeback connectors. It is not an issue yet, but in order
> > to support multiple devices easily, convert this code to
> > use drm and device managed helpers.
> > 
> > Reviewed-by: José Expósito <jose.exposito89@gmail.com>
> 
> Hi,
> 
> Sorry José, I forgot to remove your Reviewed-by, the changes made here are 
> not trivials. Can I keep it or do you have any comments ?

Hi Louis,

No problem, feel free to keep it. I had a look to v8 and its looking
great, sending a few review-by to that version.

I'll try to rebase my branch and run all the automated tests I have
been writen just in case they catch a bug.

Best wishes,
Jose
 
> Sorry,
> Louis Chauvet
> 
> > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> > ---
> >  drivers/gpu/drm/vkms/vkms_drv.h       |  3 ++-
> >  drivers/gpu/drm/vkms/vkms_output.c    |  2 +-
> >  drivers/gpu/drm/vkms/vkms_writeback.c | 21 +++++++++++++--------
> >  3 files changed, 16 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > index 00541eff3d1b0aa4b374fb94c8fe34932df31509..46ac36aebb27ce8d9018224735007c1b3fe7d0a5 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -179,6 +179,7 @@ struct vkms_output {
> >  	struct drm_encoder encoder;
> >  	struct drm_connector connector;
> >  	struct drm_writeback_connector wb_connector;
> > +	struct drm_encoder wb_encoder;
> >  	struct hrtimer vblank_hrtimer;
> >  	ktime_t period_ns;
> >  	struct workqueue_struct *composer_workq;
> > @@ -275,6 +276,6 @@ void vkms_set_composer(struct vkms_output *out, bool enabled);
> >  void vkms_writeback_row(struct vkms_writeback_job *wb, const struct line_buffer *src_buffer, int y);
> >  
> >  /* Writeback */
> > -int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
> > +int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc);
> >  
> >  #endif /* _VKMS_DRV_H_ */
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> > index ab9affa75b66ce9f00fe025052439405206144ec..de817e2794860f9071a71b3631460691e0d73a85 100644
> > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -95,7 +95,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> >  	}
> >  
> >  	if (vkmsdev->config->writeback) {
> > -		writeback = vkms_enable_writeback_connector(vkmsdev);
> > +		writeback = vkms_enable_writeback_connector(vkmsdev, crtc);
> >  		if (writeback)
> >  			DRM_ERROR("Failed to init writeback connector\n");
> >  	}
> > diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
> > index 79918b44fedd7ae2451d1d530fc6d5aabf2d99a3..981975c2b0a0c75e4a3aceca2a965f5876ae0a8f 100644
> > --- a/drivers/gpu/drm/vkms/vkms_writeback.c
> > +++ b/drivers/gpu/drm/vkms/vkms_writeback.c
> > @@ -24,7 +24,6 @@ static const u32 vkms_wb_formats[] = {
> >  
> >  static const struct drm_connector_funcs vkms_wb_connector_funcs = {
> >  	.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,
> > @@ -163,16 +162,22 @@ static const struct drm_connector_helper_funcs vkms_wb_conn_helper_funcs = {
> >  	.atomic_check = vkms_wb_atomic_check,
> >  };
> >  
> > -int vkms_enable_writeback_connector(struct vkms_device *vkmsdev)
> > +int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc)
> >  {
> >  	struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
> > +	int ret;
> > +
> > +	ret = drmm_encoder_init(&vkmsdev->drm, &vkmsdev->output.wb_encoder,
> > +				NULL, DRM_MODE_ENCODER_VIRTUAL, NULL);
> > +	if (ret)
> > +		return ret;
> > +	vkmsdev->output.wb_encoder.possible_crtcs |= drm_crtc_mask(crtc);
> >  
> >  	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
> >  
> > -	return drm_writeback_connector_init(&vkmsdev->drm, wb,
> > -					    &vkms_wb_connector_funcs,
> > -					    NULL,
> > -					    vkms_wb_formats,
> > -					    ARRAY_SIZE(vkms_wb_formats),
> > -					    1);
> > +	return drmm_writeback_connector_init(&vkmsdev->drm, wb,
> > +					     &vkms_wb_connector_funcs,
> > +					     &vkmsdev->output.wb_encoder,
> > +					     vkms_wb_formats,
> > +					     ARRAY_SIZE(vkms_wb_formats));
> >  }
> > 
> > -- 
> > 2.47.1
> >