1. Patchew
  2. linux
  3. [PATCH v7 0/7] drm/vkms: Switch all vkms object to DRM managed objects
  4. View patch

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

Louis Chauvet posted 7 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v7 7/7] drm/vkms: Switch to managed for writeback connector
Posted by Louis Chauvet 11 months, 1 week ago 
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
Posted by Louis Chauvet 11 months, 1 week ago 
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 7/7] drm/vkms: Switch to managed for writeback connector
Posted by José Expósito 11 months ago 
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
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.