| Message ID | 20241122-google-vkms-managed-v5-3-1ab60403e960@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/vkms: Switch all vkms object to DRM managed objects | expand |
On Fri, 22 Nov 2024 17:27:58 +0100, Louis Chauvet wrote: > 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> Thanks! Maxime
Hi Louis, On 22/11/24 13:27, Louis Chauvet wrote: > 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. > > 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; > + Small nit: no need for `if (ret)` here. Anyway, Reviewed-by: Maíra Canal <mcanal@igalia.com> Best Regards, - Maíra > return ret; > } > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index fa3331f612e34e0a48cef34effc169dea46d77df..c54504e590a18ae8af07cc1cc48179c38c4e6c0f 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -54,14 +54,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; > @@ -109,7 +101,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, >
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 fa3331f612e34e0a48cef34effc169dea46d77df..c54504e590a18ae8af07cc1cc48179c38c4e6c0f 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -54,14 +54,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; @@ -109,7 +101,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,
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. 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(-)