[PATCH 0/3] drm/vkms: Switch to allocated for drm objects

[PATCH 0/3] drm/vkms: Switch to allocated for drm objects

[Louis Chauvet]

Specific allocations for each DRM object is not strictly needed in VKMS 
right now, but in order to implement dynamic configuration of VKMS 
(configFS), it will be easier to have one allocation per DRM object.

There is no need for a dynamic allocation for the writeback connector as 
there can only be one per CRTC

No functionnal changes are intented in this series.

This series requires [1] to switch vkms objects to drm-managed objects.

[1]: https://lore.kernel.org/all/20240912-google-vkms-managed-v3-0-7708d6ad262d@bootlin.com/

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
Louis Chauvet (3):
      drm/vkms: Switch to dynamic allocation for connector
      drm/vkms: Switch to dynamic allocation for encoder
      drm/vkms: Switch to dynamic allocation for CRTC

 drivers/gpu/drm/vkms/vkms_crtc.c      | 28 +++++++++++---------
 drivers/gpu/drm/vkms/vkms_drv.h       | 11 ++++----
 drivers/gpu/drm/vkms/vkms_output.c    | 48 ++++++++++++++++++++++-------------
 drivers/gpu/drm/vkms/vkms_writeback.c | 17 +++++++------
 4 files changed, 61 insertions(+), 43 deletions(-)
---
base-commit: d2194256049910d286cd6c308c2689df521d8842
change-id: 20240909-b4-vkms-allocated-9f5508f4444a
prerequisite-message-id: <20240906-writeback-drmm-v1-1-01ede328182c@bootlin.com>
prerequisite-patch-id: 93bfa5c36385932ea291789faa7356639d9e4bfc
prerequisite-message-id: <20240906-vkms-remove-index-v1-1-3cfedd8ccb2f@bootlin.com>
prerequisite-patch-id: 130816a16434cafa13b7a2b629398a20782be3a6
prerequisite-message-id: 20240912-google-vkms-managed-v3-0-7708d6ad262d@bootlin.com
prerequisite-patch-id: ec4384a6e16b63691ccc54a71d304722e33443bc
prerequisite-patch-id: 4c4bbad8522d8184fb3e6695a98fdf34c2bf784c
prerequisite-patch-id: 05df4db485c1454c432e776421b056e3a9c51d7e
prerequisite-patch-id: 30a1e033fa43241ca6a43006fd4f29f8e9217224

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

[PATCH 1/3] drm/vkms: Switch to dynamic allocation for connector

[Louis Chauvet]

A specific allocation for the connector is not strictly necessary
at this point, but in order to implement dynamic configuration of
VKMS (configFS), it will be easier to have one allocation per
connector.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_drv.h    | 1 -
 drivers/gpu/drm/vkms/vkms_output.c | 9 ++++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index dac063f11dcd751865cc79d3c2466d113b8e79c7..938f369dba7ab82b55c656ac6ccf2dfe5a11f9e6 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -99,7 +99,6 @@ struct vkms_crtc_state {
 struct vkms_output {
 	struct drm_crtc crtc;
 	struct drm_encoder encoder;
-	struct drm_connector connector;
 	struct drm_writeback_connector wb_connector;
 	struct hrtimer vblank_hrtimer;
 	ktime_t period_ns;
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 2226ba1972f3ff51483abacac45ee8914be0c94a..a0331181ab0e369d711aee0974df4859844c6549 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -31,7 +31,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 {
 	struct vkms_output *output = &vkmsdev->output;
 	struct drm_device *dev = &vkmsdev->drm;
-	struct drm_connector *connector = &output->connector;
+	struct drm_connector *connector;
 	struct drm_encoder *encoder = &output->encoder;
 	struct drm_crtc *crtc = &output->crtc;
 	struct vkms_plane *primary, *overlay, *cursor = NULL;
@@ -62,6 +62,13 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 		}
 	}
 
+	connector = drmm_kzalloc(dev, sizeof(*connector), GFP_KERNEL);
+	if (!connector) {
+		DRM_ERROR("Failed to allocate connector\n");
+		ret = -ENOMEM;
+		goto err_connector;
+	}
+
 	ret = drmm_connector_init(dev, connector, &vkms_connector_funcs,
 				  DRM_MODE_CONNECTOR_VIRTUAL, NULL);
 	if (ret) {

-- 
2.44.2

[PATCH 2/3] drm/vkms: Switch to dynamic allocation for encoder

[Louis Chauvet]

A specific allocation for the encoder is not strictly necessary  at this
point, but in order to implement dynamic configuration of VKMS (configFS),
it will be easier to have one allocation per encoder.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_drv.h    | 1 -
 drivers/gpu/drm/vkms/vkms_output.c | 8 +++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 938f369dba7ab82b55c656ac6ccf2dfe5a11f9e6..972aee6853f2b29909291e33652f68740fdc9dbc 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -98,7 +98,6 @@ struct vkms_crtc_state {
 
 struct vkms_output {
 	struct drm_crtc crtc;
-	struct drm_encoder encoder;
 	struct drm_writeback_connector wb_connector;
 	struct hrtimer vblank_hrtimer;
 	ktime_t period_ns;
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index a0331181ab0e369d711aee0974df4859844c6549..60d5365f8d41b8f20da489cfb9dbc85eb9df4916 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -32,7 +32,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 	struct vkms_output *output = &vkmsdev->output;
 	struct drm_device *dev = &vkmsdev->drm;
 	struct drm_connector *connector;
-	struct drm_encoder *encoder = &output->encoder;
+	struct drm_encoder *encoder;
 	struct drm_crtc *crtc = &output->crtc;
 	struct vkms_plane *primary, *overlay, *cursor = NULL;
 	int ret;
@@ -78,6 +78,12 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 
 	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
 
+	encoder = drmm_kzalloc(dev, sizeof(*encoder), GFP_KERNEL);
+	if (!encoder) {
+		DRM_ERROR("Failed to allocate encoder\n");
+		ret = -ENOMEM;
+		goto err_connector;
+	}
 	ret = drmm_encoder_init(dev, encoder, NULL,
 				DRM_MODE_ENCODER_VIRTUAL, NULL);
 	if (ret) {

-- 
2.44.2

[PATCH 3/3] drm/vkms: Switch to dynamic allocation for CRTC

[Louis Chauvet]

specific allocation for the CRTC is not strictly necessary at this point,
but in order to implement dynamic configuration of VKMS (configFS), it
will be easier to have one allocation per CRTC.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_crtc.c      | 28 ++++++++++++++-----------
 drivers/gpu/drm/vkms/vkms_drv.h       |  9 ++++----
 drivers/gpu/drm/vkms/vkms_output.c    | 39 ++++++++++++++++++-----------------
 drivers/gpu/drm/vkms/vkms_writeback.c | 17 ++++++++-------
 4 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 821b9ac746083630116e05c1cf8e3dc2424ac66a..1169eb5a5e521fb42b1af85082425cd71aa2be4d 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -88,7 +88,7 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
-	struct vkms_output *output = &vkmsdev->output;
+	struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
 	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
 
 	if (!READ_ONCE(vblank->enabled)) {
@@ -281,19 +281,23 @@ static void vkms_crtc_destroy_workqueue(struct drm_device *dev,
 	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)
+struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *primary,
+				   struct drm_plane *cursor)
 {
-	struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc);
+	struct vkms_output *vkms_out;
+	struct drm_crtc *crtc;
 	int ret;
 
-	ret = drmm_crtc_init_with_planes(dev, crtc, primary, cursor,
-					 &vkms_crtc_funcs, NULL);
-	if (ret) {
-		DRM_ERROR("Failed to init CRTC\n");
-		return ret;
+	vkms_out = drmm_crtc_alloc_with_planes(dev, struct vkms_output, crtc,
+					       primary, cursor,
+					       &vkms_crtc_funcs, NULL);
+	if (IS_ERR(vkms_out)) {
+		DRM_DEV_ERROR(dev->dev, "Failed to init CRTC\n");
+		return vkms_out;
 	}
 
+	crtc = &vkms_out->crtc;
+
 	drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
 
 	drm_mode_crtc_set_gamma_size(crtc, VKMS_LUT_SIZE);
@@ -304,12 +308,12 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 
 	vkms_out->composer_workq = alloc_ordered_workqueue("vkms_composer", 0);
 	if (!vkms_out->composer_workq)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	ret = drmm_add_action_or_reset(dev, vkms_crtc_destroy_workqueue,
 				       vkms_out);
 	if (ret)
-		return ret;
+		return ERR_PTR(ret);
 
-	return ret;
+	return vkms_out;
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 972aee6853f2b29909291e33652f68740fdc9dbc..a97164c0c2d62c4b6bb5641d09b3607a742cf585 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -126,7 +126,6 @@ struct vkms_config {
 struct vkms_device {
 	struct drm_device drm;
 	struct platform_device *platform;
-	struct vkms_output output;
 	const struct vkms_config *config;
 };
 
@@ -143,8 +142,9 @@ struct vkms_device {
 	container_of(target, struct vkms_plane_state, base.base)
 
 /* CRTC */
-int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
-		   struct drm_plane *primary, struct drm_plane *cursor);
+struct vkms_output *vkms_crtc_init(struct drm_device *dev,
+				   struct drm_plane *primary,
+				   struct drm_plane *cursor);
 
 int vkms_output_init(struct vkms_device *vkmsdev);
 
@@ -165,6 +165,7 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
 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 vkms_output *vkms_out);
 
 #endif /* _VKMS_DRV_H_ */
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 60d5365f8d41b8f20da489cfb9dbc85eb9df4916..a57a0cfa21964577f98e564acf87711b2e85fa67 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -29,11 +29,11 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
 
 int vkms_output_init(struct vkms_device *vkmsdev)
 {
-	struct vkms_output *output = &vkmsdev->output;
+
 	struct drm_device *dev = &vkmsdev->drm;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
-	struct drm_crtc *crtc = &output->crtc;
+	struct vkms_output *output;
 	struct vkms_plane *primary, *overlay, *cursor = NULL;
 	int ret;
 	int writeback;
@@ -49,31 +49,33 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 			return PTR_ERR(cursor);
 	}
 
-	ret = vkms_crtc_init(dev, crtc, &primary->base, &cursor->base);
-	if (ret)
-		return ret;
+	output = vkms_crtc_init(dev, &primary->base,
+				cursor ? &cursor->base : NULL);
+	if (IS_ERR(output)) {
+		DRM_ERROR("Failed to allocate CRTC\n");
+		return PTR_ERR(output);
+	}
 
 	if (vkmsdev->config->overlay) {
 		for (n = 0; n < NUM_OVERLAY_PLANES; n++) {
 			overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY);
 			if (IS_ERR(overlay))
 				return PTR_ERR(overlay);
-			overlay->base.possible_crtcs = drm_crtc_mask(crtc);
+			overlay->base.possible_crtcs = drm_crtc_mask(&output->crtc);
 		}
 	}
 
 	connector = drmm_kzalloc(dev, sizeof(*connector), GFP_KERNEL);
 	if (!connector) {
 		DRM_ERROR("Failed to allocate connector\n");
-		ret = -ENOMEM;
-		goto err_connector;
+		return -ENOMEM;
 	}
 
 	ret = drmm_connector_init(dev, connector, &vkms_connector_funcs,
 				  DRM_MODE_CONNECTOR_VIRTUAL, NULL);
 	if (ret) {
 		DRM_ERROR("Failed to init connector\n");
-		goto err_connector;
+		return ret;
 	}
 
 	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
@@ -81,34 +83,33 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 	encoder = drmm_kzalloc(dev, sizeof(*encoder), GFP_KERNEL);
 	if (!encoder) {
 		DRM_ERROR("Failed to allocate encoder\n");
-		ret = -ENOMEM;
-		goto err_connector;
+		return -ENOMEM;
 	}
 	ret = drmm_encoder_init(dev, encoder, NULL,
 				DRM_MODE_ENCODER_VIRTUAL, NULL);
 	if (ret) {
 		DRM_ERROR("Failed to init encoder\n");
-		goto err_connector;
+		return ret;
 	}
-	encoder->possible_crtcs = drm_crtc_mask(crtc);
+	encoder->possible_crtcs = drm_crtc_mask(&output->crtc);
 
+	/* Attach the encoder and the connector */
 	ret = drm_connector_attach_encoder(connector, encoder);
 	if (ret) {
 		DRM_ERROR("Failed to attach connector to encoder\n");
 		return ret;
 	}
 
+	/* Initialize the writeback component */
 	if (vkmsdev->config->writeback) {
-		writeback = vkms_enable_writeback_connector(vkmsdev);
-		if (writeback)
+		writeback = vkms_enable_writeback_connector(vkmsdev, output);
+		if (writeback) {
 			DRM_ERROR("Failed to init writeback connector\n");
+			return ret;
+		}
 	}
 
 	drm_mode_config_reset(dev);
 
 	return 0;
-
-err_connector:
-	drm_crtc_cleanup(crtc);
-	return ret;
 }
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
index a948f4598764efef971f76e1016fc1a963fbbba7..f91c6418480f71ab3ec9989ea85814460e10d231 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -105,7 +105,9 @@ static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector,
 				struct drm_writeback_job *job)
 {
 	struct vkms_writeback_job *vkmsjob = job->priv;
-	struct vkms_device *vkmsdev;
+	struct vkms_output *vkms_output = container_of(connector,
+						       struct vkms_output,
+						       wb_connector);
 
 	if (!job->fb)
 		return;
@@ -114,8 +116,7 @@ static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector,
 
 	drm_framebuffer_put(vkmsjob->wb_frame_info.fb);
 
-	vkmsdev = drm_device_to_vkms_device(job->fb->dev);
-	vkms_set_composer(&vkmsdev->output, false);
+	vkms_set_composer(vkms_output, false);
 	kfree(vkmsjob);
 }
 
@@ -124,8 +125,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
 {
 	struct drm_connector_state *connector_state = drm_atomic_get_new_connector_state(state,
 											 conn);
-	struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev);
-	struct vkms_output *output = &vkmsdev->output;
+	struct vkms_output *output = drm_crtc_to_vkms_output(connector_state->crtc);
 	struct drm_writeback_connector *wb_conn = &output->wb_connector;
 	struct drm_connector_state *conn_state = wb_conn->base.state;
 	struct vkms_crtc_state *crtc_state = output->composer_state;
@@ -139,7 +139,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
 	if (!conn_state)
 		return;
 
-	vkms_set_composer(&vkmsdev->output, true);
+	vkms_set_composer(output, true);
 
 	active_wb = conn_state->writeback_job->priv;
 	wb_frame_info = &active_wb->wb_frame_info;
@@ -167,9 +167,10 @@ 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 vkms_output *vkms_output)
 {
-	struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
+	struct drm_writeback_connector *wb = &vkms_output->wb_connector;
 
 	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
 

-- 
2.44.2

[PATCH 3/3] drm/vkms: Switch to dynamic allocation for CRTC

[José Expósito]

Hi Louis,

Thanks for this series!

The first 2 patches look good to me. It could make sense to move the
alloc + init pair of calls to a function (vkms_connector_init() and
vkms_encoder_init() for example), but we can always move it in the
furure:

This one looks good, but I added couple of comments:

> specific allocation for the CRTC is not strictly necessary at this point,
> but in order to implement dynamic configuration of VKMS (configFS), it
> will be easier to have one allocation per CRTC.
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c      | 28 ++++++++++++++-----------
>  drivers/gpu/drm/vkms/vkms_drv.h       |  9 ++++----
>  drivers/gpu/drm/vkms/vkms_output.c    | 39 ++++++++++++++++++-----------------
>  drivers/gpu/drm/vkms/vkms_writeback.c | 17 ++++++++-------
>  4 files changed, 50 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 821b9ac746083630116e05c1cf8e3dc2424ac66a..1169eb5a5e521fb42b1af85082425cd71aa2be4d 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -88,7 +88,7 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);

vkmsdev is unused.

> -	struct vkms_output *output = &vkmsdev->output;
> +	struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
>  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>  
>  	if (!READ_ONCE(vblank->enabled)) {
> @@ -281,19 +281,23 @@ static void vkms_crtc_destroy_workqueue(struct drm_device *dev,
>  	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)
> +struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *primary,
> +				   struct drm_plane *cursor)
>  {
> -	struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc);
> +	struct vkms_output *vkms_out;
> +	struct drm_crtc *crtc;
>  	int ret;
>  
> -	ret = drmm_crtc_init_with_planes(dev, crtc, primary, cursor,
> -					 &vkms_crtc_funcs, NULL);
> -	if (ret) {
> -		DRM_ERROR("Failed to init CRTC\n");
> -		return ret;
> +	vkms_out = drmm_crtc_alloc_with_planes(dev, struct vkms_output, crtc,
> +					       primary, cursor,
> +					       &vkms_crtc_funcs, NULL);
> +	if (IS_ERR(vkms_out)) {
> +		DRM_DEV_ERROR(dev->dev, "Failed to init CRTC\n");
> +		return vkms_out;
>  	}
>  
> +	crtc = &vkms_out->crtc;
> +
>  	drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
>  
>  	drm_mode_crtc_set_gamma_size(crtc, VKMS_LUT_SIZE);
> @@ -304,12 +308,12 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  
>  	vkms_out->composer_workq = alloc_ordered_workqueue("vkms_composer", 0);
>  	if (!vkms_out->composer_workq)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	ret = drmm_add_action_or_reset(dev, vkms_crtc_destroy_workqueue,
>  				       vkms_out);
>  	if (ret)
> -		return ret;
> +		return ERR_PTR(ret);
>  
> -	return ret;
> +	return vkms_out;
>  }
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 972aee6853f2b29909291e33652f68740fdc9dbc..a97164c0c2d62c4b6bb5641d09b3607a742cf585 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -126,7 +126,6 @@ struct vkms_config {
>  struct vkms_device {
>  	struct drm_device drm;
>  	struct platform_device *platform;
> -	struct vkms_output output;
>  	const struct vkms_config *config;
>  };
>  
> @@ -143,8 +142,9 @@ struct vkms_device {
>  	container_of(target, struct vkms_plane_state, base.base)
>  
>  /* CRTC */
> -int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> -		   struct drm_plane *primary, struct drm_plane *cursor);
> +struct vkms_output *vkms_crtc_init(struct drm_device *dev,
> +				   struct drm_plane *primary,
> +				   struct drm_plane *cursor);

Do you think that it could make sense to rename vkms_output to vkms_crtc
in a follow up patch?

I find a bit confusing that vkms_crtc_init returns a different type.
Renaming it would make drm_crtc_to_vkms_output() consistent with the
other container_of macros.

>  
>  int vkms_output_init(struct vkms_device *vkmsdev);
>  
> @@ -165,6 +165,7 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
>  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 vkms_output *vkms_out);
>  
>  #endif /* _VKMS_DRV_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 60d5365f8d41b8f20da489cfb9dbc85eb9df4916..a57a0cfa21964577f98e564acf87711b2e85fa67 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -29,11 +29,11 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
>  
>  int vkms_output_init(struct vkms_device *vkmsdev)
>  {
> -	struct vkms_output *output = &vkmsdev->output;
> +

Extra blank line.

>  	struct drm_device *dev = &vkmsdev->drm;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> -	struct drm_crtc *crtc = &output->crtc;
> +	struct vkms_output *output;
>  	struct vkms_plane *primary, *overlay, *cursor = NULL;
>  	int ret;
>  	int writeback;
> @@ -49,31 +49,33 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>  			return PTR_ERR(cursor);
>  	}
>  
> -	ret = vkms_crtc_init(dev, crtc, &primary->base, &cursor->base);
> -	if (ret)
> -		return ret;
> +	output = vkms_crtc_init(dev, &primary->base,
> +				cursor ? &cursor->base : NULL);
> +	if (IS_ERR(output)) {
> +		DRM_ERROR("Failed to allocate CRTC\n");
> +		return PTR_ERR(output);
> +	}
>  
>  	if (vkmsdev->config->overlay) {
>  		for (n = 0; n < NUM_OVERLAY_PLANES; n++) {
>  			overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY);
>  			if (IS_ERR(overlay))
>  				return PTR_ERR(overlay);
> -			overlay->base.possible_crtcs = drm_crtc_mask(crtc);
> +			overlay->base.possible_crtcs = drm_crtc_mask(&output->crtc);
>  		}
>  	}
>  
>  	connector = drmm_kzalloc(dev, sizeof(*connector), GFP_KERNEL);
>  	if (!connector) {
>  		DRM_ERROR("Failed to allocate connector\n");
> -		ret = -ENOMEM;
> -		goto err_connector;
> +		return -ENOMEM;
>  	}
>  
>  	ret = drmm_connector_init(dev, connector, &vkms_connector_funcs,
>  				  DRM_MODE_CONNECTOR_VIRTUAL, NULL);
>  	if (ret) {
>  		DRM_ERROR("Failed to init connector\n");
> -		goto err_connector;
> +		return ret;
>  	}
>  
>  	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
> @@ -81,34 +83,33 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>  	encoder = drmm_kzalloc(dev, sizeof(*encoder), GFP_KERNEL);
>  	if (!encoder) {
>  		DRM_ERROR("Failed to allocate encoder\n");
> -		ret = -ENOMEM;
> -		goto err_connector;
> +		return -ENOMEM;
>  	}
>  	ret = drmm_encoder_init(dev, encoder, NULL,
>  				DRM_MODE_ENCODER_VIRTUAL, NULL);
>  	if (ret) {
>  		DRM_ERROR("Failed to init encoder\n");
> -		goto err_connector;
> +		return ret;
>  	}
> -	encoder->possible_crtcs = drm_crtc_mask(crtc);
> +	encoder->possible_crtcs = drm_crtc_mask(&output->crtc);
>  
> +	/* Attach the encoder and the connector */
>  	ret = drm_connector_attach_encoder(connector, encoder);
>  	if (ret) {
>  		DRM_ERROR("Failed to attach connector to encoder\n");
>  		return ret;
>  	}
>  
> +	/* Initialize the writeback component */
>  	if (vkmsdev->config->writeback) {
> -		writeback = vkms_enable_writeback_connector(vkmsdev);
> -		if (writeback)
> +		writeback = vkms_enable_writeback_connector(vkmsdev, output);
> +		if (writeback) {
>  			DRM_ERROR("Failed to init writeback connector\n");
> +			return ret;
> +		}
>  	}
>  
>  	drm_mode_config_reset(dev);
>  
>  	return 0;
> -
> -err_connector:
> -	drm_crtc_cleanup(crtc);
> -	return ret;
>  }
> diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
> index a948f4598764efef971f76e1016fc1a963fbbba7..f91c6418480f71ab3ec9989ea85814460e10d231 100644
> --- a/drivers/gpu/drm/vkms/vkms_writeback.c
> +++ b/drivers/gpu/drm/vkms/vkms_writeback.c
> @@ -105,7 +105,9 @@ static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector,
>  				struct drm_writeback_job *job)
>  {
>  	struct vkms_writeback_job *vkmsjob = job->priv;
> -	struct vkms_device *vkmsdev;
> +	struct vkms_output *vkms_output = container_of(connector,
> +						       struct vkms_output,
> +						       wb_connector);
>  
>  	if (!job->fb)
>  		return;
> @@ -114,8 +116,7 @@ static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector,
>  
>  	drm_framebuffer_put(vkmsjob->wb_frame_info.fb);
>  
> -	vkmsdev = drm_device_to_vkms_device(job->fb->dev);
> -	vkms_set_composer(&vkmsdev->output, false);
> +	vkms_set_composer(vkms_output, false);
>  	kfree(vkmsjob);
>  }
>  
> @@ -124,8 +125,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
>  {
>  	struct drm_connector_state *connector_state = drm_atomic_get_new_connector_state(state,
>  											 conn);
> -	struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev);
> -	struct vkms_output *output = &vkmsdev->output;
> +	struct vkms_output *output = drm_crtc_to_vkms_output(connector_state->crtc);
>  	struct drm_writeback_connector *wb_conn = &output->wb_connector;
>  	struct drm_connector_state *conn_state = wb_conn->base.state;
>  	struct vkms_crtc_state *crtc_state = output->composer_state;
> @@ -139,7 +139,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
>  	if (!conn_state)
>  		return;
>  
> -	vkms_set_composer(&vkmsdev->output, true);
> +	vkms_set_composer(output, true);
>  
>  	active_wb = conn_state->writeback_job->priv;
>  	wb_frame_info = &active_wb->wb_frame_info;
> @@ -167,9 +167,10 @@ 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 vkms_output *vkms_output)
>  {
> -	struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
> +	struct drm_writeback_connector *wb = &vkms_output->wb_connector;
>  
>  	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
>  
> 

Re: [PATCH 3/3] drm/vkms: Switch to dynamic allocation for CRTC

[Louis Chauvet]

Le 17/09/24 - 18:02, José Expósito a écrit :
> Hi Louis,
> 
> Thanks for this series!
> 
> The first 2 patches look good to me. It could make sense to move the
> alloc + init pair of calls to a function (vkms_connector_init() and
> vkms_encoder_init() for example), but we can always move it in the
> furure:

I don't think this is strictly needed at this point, but as I want to 
introduce connector configuration, yes, I will probably create 
struct vkms_connector and _init function.
 
> This one looks good, but I added couple of comments:

I will send a v2 with your modification in one or two weeks, I am 
currently at LPC.

> > specific allocation for the CRTC is not strictly necessary at this point,
> > but in order to implement dynamic configuration of VKMS (configFS), it
> > will be easier to have one allocation per CRTC.
> > 
> > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> > ---
> >  drivers/gpu/drm/vkms/vkms_crtc.c      | 28 ++++++++++++++-----------
> >  drivers/gpu/drm/vkms/vkms_drv.h       |  9 ++++----
> >  drivers/gpu/drm/vkms/vkms_output.c    | 39 ++++++++++++++++++-----------------
> >  drivers/gpu/drm/vkms/vkms_writeback.c | 17 ++++++++-------
> >  4 files changed, 50 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > index 821b9ac746083630116e05c1cf8e3dc2424ac66a..1169eb5a5e521fb42b1af85082425cd71aa2be4d 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -88,7 +88,7 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
> >  {
> >  	struct drm_device *dev = crtc->dev;
> >  	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
> 
> vkmsdev is unused.
> 
> > -	struct vkms_output *output = &vkmsdev->output;
> > +	struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
> >  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> >  
> >  	if (!READ_ONCE(vblank->enabled)) {
> > @@ -281,19 +281,23 @@ static void vkms_crtc_destroy_workqueue(struct drm_device *dev,
> >  	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)
> > +struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *primary,
> > +				   struct drm_plane *cursor)
> >  {
> > -	struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc);
> > +	struct vkms_output *vkms_out;
> > +	struct drm_crtc *crtc;
> >  	int ret;
> >  
> > -	ret = drmm_crtc_init_with_planes(dev, crtc, primary, cursor,
> > -					 &vkms_crtc_funcs, NULL);
> > -	if (ret) {
> > -		DRM_ERROR("Failed to init CRTC\n");
> > -		return ret;
> > +	vkms_out = drmm_crtc_alloc_with_planes(dev, struct vkms_output, crtc,
> > +					       primary, cursor,
> > +					       &vkms_crtc_funcs, NULL);
> > +	if (IS_ERR(vkms_out)) {
> > +		DRM_DEV_ERROR(dev->dev, "Failed to init CRTC\n");
> > +		return vkms_out;
> >  	}
> >  
> > +	crtc = &vkms_out->crtc;
> > +
> >  	drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
> >  
> >  	drm_mode_crtc_set_gamma_size(crtc, VKMS_LUT_SIZE);
> > @@ -304,12 +308,12 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> >  
> >  	vkms_out->composer_workq = alloc_ordered_workqueue("vkms_composer", 0);
> >  	if (!vkms_out->composer_workq)
> > -		return -ENOMEM;
> > +		return ERR_PTR(-ENOMEM);
> >  
> >  	ret = drmm_add_action_or_reset(dev, vkms_crtc_destroy_workqueue,
> >  				       vkms_out);
> >  	if (ret)
> > -		return ret;
> > +		return ERR_PTR(ret);
> >  
> > -	return ret;
> > +	return vkms_out;
> >  }
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > index 972aee6853f2b29909291e33652f68740fdc9dbc..a97164c0c2d62c4b6bb5641d09b3607a742cf585 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -126,7 +126,6 @@ struct vkms_config {
> >  struct vkms_device {
> >  	struct drm_device drm;
> >  	struct platform_device *platform;
> > -	struct vkms_output output;
> >  	const struct vkms_config *config;
> >  };
> >  
> > @@ -143,8 +142,9 @@ struct vkms_device {
> >  	container_of(target, struct vkms_plane_state, base.base)
> >  
> >  /* CRTC */
> > -int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > -		   struct drm_plane *primary, struct drm_plane *cursor);
> > +struct vkms_output *vkms_crtc_init(struct drm_device *dev,
> > +				   struct drm_plane *primary,
> > +				   struct drm_plane *cursor);
> 
> Do you think that it could make sense to rename vkms_output to vkms_crtc
> in a follow up patch?

Do you mean the patch that I forgot to include in this series? [1] :') 
Definitly yes! I will add it in the v2.

[1]:https://lore.kernel.org/all/20240827-google-vkms-managed-v2-3-f41104553aeb@bootlin.com/

Thanks,
Louis chauvet

> I find a bit confusing that vkms_crtc_init returns a different type.
> Renaming it would make drm_crtc_to_vkms_output() consistent with the
> other container_of macros.
> 
> >  
> >  int vkms_output_init(struct vkms_device *vkmsdev);
> >  
> > @@ -165,6 +165,7 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
> >  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 vkms_output *vkms_out);
> >  
> >  #endif /* _VKMS_DRV_H_ */
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> > index 60d5365f8d41b8f20da489cfb9dbc85eb9df4916..a57a0cfa21964577f98e564acf87711b2e85fa67 100644
> > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -29,11 +29,11 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
> >  
> >  int vkms_output_init(struct vkms_device *vkmsdev)
> >  {
> > -	struct vkms_output *output = &vkmsdev->output;
> > +
> 
> Extra blank line.
> 
> >  	struct drm_device *dev = &vkmsdev->drm;
> >  	struct drm_connector *connector;
> >  	struct drm_encoder *encoder;
> > -	struct drm_crtc *crtc = &output->crtc;
> > +	struct vkms_output *output;
> >  	struct vkms_plane *primary, *overlay, *cursor = NULL;
> >  	int ret;
> >  	int writeback;
> > @@ -49,31 +49,33 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> >  			return PTR_ERR(cursor);
> >  	}
> >  
> > -	ret = vkms_crtc_init(dev, crtc, &primary->base, &cursor->base);
> > -	if (ret)
> > -		return ret;
> > +	output = vkms_crtc_init(dev, &primary->base,
> > +				cursor ? &cursor->base : NULL);
> > +	if (IS_ERR(output)) {
> > +		DRM_ERROR("Failed to allocate CRTC\n");
> > +		return PTR_ERR(output);
> > +	}
> >  
> >  	if (vkmsdev->config->overlay) {
> >  		for (n = 0; n < NUM_OVERLAY_PLANES; n++) {
> >  			overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY);
> >  			if (IS_ERR(overlay))
> >  				return PTR_ERR(overlay);
> > -			overlay->base.possible_crtcs = drm_crtc_mask(crtc);
> > +			overlay->base.possible_crtcs = drm_crtc_mask(&output->crtc);
> >  		}
> >  	}
> >  
> >  	connector = drmm_kzalloc(dev, sizeof(*connector), GFP_KERNEL);
> >  	if (!connector) {
> >  		DRM_ERROR("Failed to allocate connector\n");
> > -		ret = -ENOMEM;
> > -		goto err_connector;
> > +		return -ENOMEM;
> >  	}
> >  
> >  	ret = drmm_connector_init(dev, connector, &vkms_connector_funcs,
> >  				  DRM_MODE_CONNECTOR_VIRTUAL, NULL);
> >  	if (ret) {
> >  		DRM_ERROR("Failed to init connector\n");
> > -		goto err_connector;
> > +		return ret;
> >  	}
> >  
> >  	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
> > @@ -81,34 +83,33 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> >  	encoder = drmm_kzalloc(dev, sizeof(*encoder), GFP_KERNEL);
> >  	if (!encoder) {
> >  		DRM_ERROR("Failed to allocate encoder\n");
> > -		ret = -ENOMEM;
> > -		goto err_connector;
> > +		return -ENOMEM;
> >  	}
> >  	ret = drmm_encoder_init(dev, encoder, NULL,
> >  				DRM_MODE_ENCODER_VIRTUAL, NULL);
> >  	if (ret) {
> >  		DRM_ERROR("Failed to init encoder\n");
> > -		goto err_connector;
> > +		return ret;
> >  	}
> > -	encoder->possible_crtcs = drm_crtc_mask(crtc);
> > +	encoder->possible_crtcs = drm_crtc_mask(&output->crtc);
> >  
> > +	/* Attach the encoder and the connector */
> >  	ret = drm_connector_attach_encoder(connector, encoder);
> >  	if (ret) {
> >  		DRM_ERROR("Failed to attach connector to encoder\n");
> >  		return ret;
> >  	}
> >  
> > +	/* Initialize the writeback component */
> >  	if (vkmsdev->config->writeback) {
> > -		writeback = vkms_enable_writeback_connector(vkmsdev);
> > -		if (writeback)
> > +		writeback = vkms_enable_writeback_connector(vkmsdev, output);
> > +		if (writeback) {
> >  			DRM_ERROR("Failed to init writeback connector\n");
> > +			return ret;
> > +		}
> >  	}
> >  
> >  	drm_mode_config_reset(dev);
> >  
> >  	return 0;
> > -
> > -err_connector:
> > -	drm_crtc_cleanup(crtc);
> > -	return ret;
> >  }
> > diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
> > index a948f4598764efef971f76e1016fc1a963fbbba7..f91c6418480f71ab3ec9989ea85814460e10d231 100644
> > --- a/drivers/gpu/drm/vkms/vkms_writeback.c
> > +++ b/drivers/gpu/drm/vkms/vkms_writeback.c
> > @@ -105,7 +105,9 @@ static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector,
> >  				struct drm_writeback_job *job)
> >  {
> >  	struct vkms_writeback_job *vkmsjob = job->priv;
> > -	struct vkms_device *vkmsdev;
> > +	struct vkms_output *vkms_output = container_of(connector,
> > +						       struct vkms_output,
> > +						       wb_connector);
> >  
> >  	if (!job->fb)
> >  		return;
> > @@ -114,8 +116,7 @@ static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector,
> >  
> >  	drm_framebuffer_put(vkmsjob->wb_frame_info.fb);
> >  
> > -	vkmsdev = drm_device_to_vkms_device(job->fb->dev);
> > -	vkms_set_composer(&vkmsdev->output, false);
> > +	vkms_set_composer(vkms_output, false);
> >  	kfree(vkmsjob);
> >  }
> >  
> > @@ -124,8 +125,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
> >  {
> >  	struct drm_connector_state *connector_state = drm_atomic_get_new_connector_state(state,
> >  											 conn);
> > -	struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev);
> > -	struct vkms_output *output = &vkmsdev->output;
> > +	struct vkms_output *output = drm_crtc_to_vkms_output(connector_state->crtc);
> >  	struct drm_writeback_connector *wb_conn = &output->wb_connector;
> >  	struct drm_connector_state *conn_state = wb_conn->base.state;
> >  	struct vkms_crtc_state *crtc_state = output->composer_state;
> > @@ -139,7 +139,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
> >  	if (!conn_state)
> >  		return;
> >  
> > -	vkms_set_composer(&vkmsdev->output, true);
> > +	vkms_set_composer(output, true);
> >  
> >  	active_wb = conn_state->writeback_job->priv;
> >  	wb_frame_info = &active_wb->wb_frame_info;
> > @@ -167,9 +167,10 @@ 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 vkms_output *vkms_output)
> >  {
> > -	struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
> > +	struct drm_writeback_connector *wb = &vkms_output->wb_connector;
> >  
> >  	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
> >  
> >