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
>
> 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
> > ---
> > 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_f
