A 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.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_crtc.c      | 32 +++++++++++++++++---------------
 drivers/gpu/drm/vkms/vkms_drv.h       |  8 ++++----
 drivers/gpu/drm/vkms/vkms_output.c    | 22 ++++++++++++----------
 drivers/gpu/drm/vkms/vkms_writeback.c | 23 ++++++++++++-----------
 4 files changed, 45 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 434c35d5e9477d2961826262591db8ab43838e09..cf229aec71c3f03bb1306095e8dd44d63f80cd2a 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -84,9 +84,7 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
 				      int *max_error, ktime_t *vblank_time,
 				      bool in_vblank_irq)
 {
-	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)) {
@@ -271,25 +269,29 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
 	.atomic_disable	= vkms_crtc_atomic_disable,
 };
 
-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);
 
 	ret = drm_mode_crtc_set_gamma_size(crtc, VKMS_LUT_SIZE);
 	if (ret) {
 		DRM_ERROR("Failed to set gamma size\n");
-		return ret;
+		return ERR_PTR(ret);
 	}
 
 	drm_crtc_enable_color_mgmt(crtc, 0, false, VKMS_LUT_SIZE);
@@ -299,9 +301,9 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 
 	vkms_out->composer_workq = drmm_alloc_ordered_workqueue(dev, "vkms_composer", 0);
 	if (IS_ERR(vkms_out->composer_workq))
-		return PTR_ERR(vkms_out->composer_workq);
+		return ERR_CAST(vkms_out->composer_workq);
 	if (!vkms_out->composer_workq)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
-	return ret;
+	return vkms_out;
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 333983bcf8d46ef85101e7c344e256df57551b78..abbb652be2b5389f96cec78837117ceb9acef656 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -215,7 +215,6 @@ struct vkms_config {
 struct vkms_device {
 	struct drm_device drm;
 	struct platform_device *platform;
-	struct vkms_output output;
 	const struct vkms_config *config;
 };
 
@@ -242,8 +241,9 @@ struct vkms_device {
  * @primary: primary plane to attach to the CRTC
  * @cursor: plane to attach to the 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);
 
 /**
  * vkms_output_init() - Initialize all sub-components needed for a VKMS device.
@@ -274,6 +274,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, struct drm_crtc *crtc);
+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 21ca975e424d148b0669b87784d86f5da2a8b333..22f0d678af3ac8177e43c4ea730af3f2109de5f3 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -30,11 +30,10 @@ 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;
@@ -56,9 +55,12 @@ 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++) {
@@ -67,7 +69,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 				DRM_DEV_ERROR(dev->dev, "Failed to init vkms plane\n");
 				return PTR_ERR(overlay);
 			}
-			overlay->base.possible_crtcs = drm_crtc_mask(crtc);
+			overlay->base.possible_crtcs = drm_crtc_mask(&output->crtc);
 		}
 	}
 
@@ -97,23 +99,23 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 		DRM_ERROR("Failed to init encoder\n");
 		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, crtc);
+		writeback = vkms_enable_writeback_connector(vkmsdev, output);
 		if (writeback)
 			DRM_ERROR("Failed to init writeback connector\n");
 	}
 
 	drm_mode_config_reset(dev);
 
-	return 0;
-
 	return ret;
 }
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
index 981975c2b0a0c75e4a3aceca2a965f5876ae0a8f..e9b5c74d7c58e7faed870a4368decbd67dab23d2 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;
@@ -162,22 +162,23 @@ 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, struct drm_crtc *crtc)
+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;
 	int ret;
 
-	ret = drmm_encoder_init(&vkmsdev->drm, &vkmsdev->output.wb_encoder,
+	ret = drmm_encoder_init(&vkmsdev->drm, &vkms_output->wb_encoder,
 				NULL, DRM_MODE_ENCODER_VIRTUAL, NULL);
 	if (ret)
 		return ret;
-	vkmsdev->output.wb_encoder.possible_crtcs |= drm_crtc_mask(crtc);
+	vkms_output->wb_encoder.possible_crtcs |= drm_crtc_mask(&vkms_output->crtc);
 
 	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
 
 	return drmm_writeback_connector_init(&vkmsdev->drm, wb,
 					     &vkms_wb_connector_funcs,
-					     &vkmsdev->output.wb_encoder,
+					     &vkms_output->wb_encoder,
 					     vkms_wb_formats,
 					     ARRAY_SIZE(vkms_wb_formats));
 }

-- 
2.47.1

> A 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.
> 
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c      | 32 +++++++++++++++++---------------
>  drivers/gpu/drm/vkms/vkms_drv.h       |  8 ++++----
>  drivers/gpu/drm/vkms/vkms_output.c    | 22 ++++++++++++----------
>  drivers/gpu/drm/vkms/vkms_writeback.c | 23 ++++++++++++-----------
>  4 files changed, 45 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 434c35d5e9477d2961826262591db8ab43838e09..cf229aec71c3f03bb1306095e8dd44d63f80cd2a 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -84,9 +84,7 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
>  				      int *max_error, ktime_t *vblank_time,
>  				      bool in_vblank_irq)
>  {
> -	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)) {
> @@ -271,25 +269,29 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
>  	.atomic_disable	= vkms_crtc_atomic_disable,
>  };
>  
> -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);
>  
>  	ret = drm_mode_crtc_set_gamma_size(crtc, VKMS_LUT_SIZE);
>  	if (ret) {
>  		DRM_ERROR("Failed to set gamma size\n");
> -		return ret;
> +		return ERR_PTR(ret);
>  	}
>  
>  	drm_crtc_enable_color_mgmt(crtc, 0, false, VKMS_LUT_SIZE);
> @@ -299,9 +301,9 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  
>  	vkms_out->composer_workq = drmm_alloc_ordered_workqueue(dev, "vkms_composer", 0);
>  	if (IS_ERR(vkms_out->composer_workq))
> -		return PTR_ERR(vkms_out->composer_workq);
> +		return ERR_CAST(vkms_out->composer_workq);
>  	if (!vkms_out->composer_workq)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
> -	return ret;
> +	return vkms_out;
>  }
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 333983bcf8d46ef85101e7c344e256df57551b78..abbb652be2b5389f96cec78837117ceb9acef656 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -215,7 +215,6 @@ struct vkms_config {
>  struct vkms_device {
>  	struct drm_device drm;
>  	struct platform_device *platform;
> -	struct vkms_output output;
>  	const struct vkms_config *config;
>  };
>  
> @@ -242,8 +241,9 @@ struct vkms_device {
>   * @primary: primary plane to attach to the CRTC
>   * @cursor: plane to attach to the 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);
>  
>  /**
>   * vkms_output_init() - Initialize all sub-components needed for a VKMS device.
> @@ -274,6 +274,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, struct drm_crtc *crtc);
> +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 21ca975e424d148b0669b87784d86f5da2a8b333..22f0d678af3ac8177e43c4ea730af3f2109de5f3 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -30,11 +30,10 @@ 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;
> @@ -56,9 +55,12 @@ 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++) {
> @@ -67,7 +69,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>  				DRM_DEV_ERROR(dev->dev, "Failed to init vkms plane\n");
>  				return PTR_ERR(overlay);
>  			}
> -			overlay->base.possible_crtcs = drm_crtc_mask(crtc);
> +			overlay->base.possible_crtcs = drm_crtc_mask(&output->crtc);
>  		}
>  	}
>  
> @@ -97,23 +99,23 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>  		DRM_ERROR("Failed to init encoder\n");
>  		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, crtc);
> +		writeback = vkms_enable_writeback_connector(vkmsdev, output);
>  		if (writeback)
>  			DRM_ERROR("Failed to init writeback connector\n");
>  	}

Hi Louis,

Thanks for fixing this error condition.

I have been working and running automated tests on top of this series and
I haven't found any other issue.

Reviewed-by: José Expósito <jose.exposito89@gmail.com>

>  
>  	drm_mode_config_reset(dev);
>  
> -	return 0;
> -
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
> index 981975c2b0a0c75e4a3aceca2a965f5876ae0a8f..e9b5c74d7c58e7faed870a4368decbd67dab23d2 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;
> @@ -162,22 +162,23 @@ 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, struct drm_crtc *crtc)
> +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;
>  	int ret;
>  
> -	ret = drmm_encoder_init(&vkmsdev->drm, &vkmsdev->output.wb_encoder,
> +	ret = drmm_encoder_init(&vkmsdev->drm, &vkms_output->wb_encoder,
>  				NULL, DRM_MODE_ENCODER_VIRTUAL, NULL);
>  	if (ret)
>  		return ret;
> -	vkmsdev->output.wb_encoder.possible_crtcs |= drm_crtc_mask(crtc);
> +	vkms_output->wb_encoder.possible_crtcs |= drm_crtc_mask(&vkms_output->crtc);
>  
>  	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
>  
>  	return drmm_writeback_connector_init(&vkmsdev->drm, wb,
>  					     &vkms_wb_connector_funcs,
> -					     &vkmsdev->output.wb_encoder,
> +					     &vkms_output->wb_encoder,
>  					     vkms_wb_formats,
>  					     ARRAY_SIZE(vkms_wb_formats));
>  }
> 

On 20/01/25 - 17:23, José Expósito wrote:
> > A 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.
> > 
> > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> > ---

[...]

> > +	/* Initialize the writeback component */
> >  	if (vkmsdev->config->writeback) {
> > -		writeback = vkms_enable_writeback_connector(vkmsdev, crtc);
> > +		writeback = vkms_enable_writeback_connector(vkmsdev, output);
> >  		if (writeback)
> >  			DRM_ERROR("Failed to init writeback connector\n");
> >  	}
> 
> Hi Louis,
> 
> Thanks for fixing this error condition.
> 
> I have been working and running automated tests on top of this series and
> I haven't found any other issue.
> 
> Reviewed-by: José Expósito <jose.exposito89@gmail.com>

Thanks a lot! I will merge this tomorrow.

What is your automated tests series? 

I will also send tomorrow a new rebased iteration for:
- https://patchwork.freedesktop.org/series/140786/
- https://patchwork.freedesktop.org/series/133698/
- https://patchwork.freedesktop.org/patch/625883/

If someone can look on them and leave some reviews, I will be very happy 
to apply them!

I will also send a first version of the configFS work (two distincts 
series to make the review easier). 

Thanks a lot,
Louis Chauvet

On Mon, Jan 20, 2025 at 06:26:07PM +0100, Louis Chauvet wrote:
> On 20/01/25 - 17:23, José Expósito wrote:
> > > A 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.
> > > 
> > > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> > > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> > > ---
> 
> [...]
> 
> > > +	/* Initialize the writeback component */
> > >  	if (vkmsdev->config->writeback) {
> > > -		writeback = vkms_enable_writeback_connector(vkmsdev, crtc);
> > > +		writeback = vkms_enable_writeback_connector(vkmsdev, output);
> > >  		if (writeback)
> > >  			DRM_ERROR("Failed to init writeback connector\n");
> > >  	}
> > 
> > Hi Louis,
> > 
> > Thanks for fixing this error condition.
> > 
> > I have been working and running automated tests on top of this series and
> > I haven't found any other issue.
> > 
> > Reviewed-by: José Expósito <jose.exposito89@gmail.com>
> 
> Thanks a lot! I will merge this tomorrow.
> 
> What is your automated tests series? 

On the kernel side, I keep working on the ConfigFS patches here:
https://github.com/JoseExposito/linux/commits/patch-vkms-configfs/

It sits on top of your work to switch to managed memory. But now that
the code is merged, it needs to be rebased.
You'll notice that I kept your signed-off-by in many patches, as I
tried to reuse as much common code as possible.

About the automated testing, the series could be split in two:
- vkms_config.h/c, which is tested with KUnit
- ConfigFS, tested with IGT:
  https://gitlab.freedesktop.org/jexposit/igt-gpu-tools/-/commits/vkms-configfs

I made some wrong assumptions with connectors, for example, it is
possible to create a device without connectors and hot-add/remove
them later, and I'm still fixing them and writing tests.
Once that work is done I'll send the series to the ML.

Jose

> I will also send tomorrow a new rebased iteration for:
> - https://patchwork.freedesktop.org/series/140786/
> - https://patchwork.freedesktop.org/series/133698/
> - https://patchwork.freedesktop.org/patch/625883/
> 
> If someone can look on them and leave some reviews, I will be very happy 
> to apply them!
> 
> I will also send a first version of the configFS work (two distincts 
> series to make the review easier). 
> 
> Thanks a lot,
> Louis Chauvet
> 

On 21/01/25 - 11:45, José Expósito wrote:
> On Mon, Jan 20, 2025 at 06:26:07PM +0100, Louis Chauvet wrote:
> > On 20/01/25 - 17:23, José Expósito wrote:
> > > > A 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.
> > > > 
> > > > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> > > > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> > > > ---
> > 
> > [...]
> > 
> > > > +	/* Initialize the writeback component */
> > > >  	if (vkmsdev->config->writeback) {
> > > > -		writeback = vkms_enable_writeback_connector(vkmsdev, crtc);
> > > > +		writeback = vkms_enable_writeback_connector(vkmsdev, output);
> > > >  		if (writeback)
> > > >  			DRM_ERROR("Failed to init writeback connector\n");
> > > >  	}
> > > 
> > > Hi Louis,
> > > 
> > > Thanks for fixing this error condition.
> > > 
> > > I have been working and running automated tests on top of this series and
> > > I haven't found any other issue.
> > > 
> > > Reviewed-by: José Expósito <jose.exposito89@gmail.com>
> > 
> > Thanks a lot! I will merge this tomorrow.
> > 
> > What is your automated tests series? 
> 
> On the kernel side, I keep working on the ConfigFS patches here:
> https://github.com/JoseExposito/linux/commits/patch-vkms-configfs/

I saw your message just after sending my "vkms-config" series, I am 
currently looking at your commits to see what are the changes.

I also sent an RFC for the ConfigFS interface, I did not change a lot of 
thing. I think the two major changes are:
- Adding format configuration
- Removing YUV related configuration (encoding/range)

> It sits on top of your work to switch to managed memory. But now that
> the code is merged, it needs to be rebased.
>
> You'll notice that I kept your signed-off-by in many patches, as I
> tried to reuse as much common code as possible.

Yes, thank you. I will compare with my work and understand your change.

> About the automated testing, the series could be split in two:

I agree, and indeed that what I meant in my previous mail :-)
I did not sent them earlier because I wanted to merge 
vkms-managed/allocated before.

> - vkms_config.h/c, which is tested with KUnit

Yes! Thank you for this, I wrote only very basic tests, so thank you for 
your extended kunit tests. 

> - ConfigFS, tested with IGT:
>   https://gitlab.freedesktop.org/jexposit/igt-gpu-tools/-/commits/vkms-configfs

You did a really great job writting those tests! Please CC me 
when you will send them, I will be very happy to review them.

> I made some wrong assumptions with connectors, for example, it is
> possible to create a device without connectors and hot-add/remove
> them later, and I'm still fixing them and writing tests.
> Once that work is done I'll send the series to the ML.

I did the same asumption, so my series is also broken on this point. I 
don't want to duplicate the work, so I'm looking forward to your series!

Thanks for this amazing work,
Louis Chauvet

> Jose
> 
> > I will also send tomorrow a new rebased iteration for:
> > - https://patchwork.freedesktop.org/series/140786/
> > - https://patchwork.freedesktop.org/series/133698/
> > - https://patchwork.freedesktop.org/patch/625883/
> > 
> > If someone can look on them and leave some reviews, I will be very happy 
> > to apply them!
> > 
> > I will also send a first version of the configFS work (two distincts 
> > series to make the review easier). 
> > 
> > Thanks a lot,
> > Louis Chauvet
> >