On 14/01/25 - 10:06, Thomas Zimmermann wrote:
> Hi
>
>
> Am 13.01.25 um 18:09 schrieb Louis Chauvet:
> > 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 <mrip...@kernel.org>
> > Reviewed-by: Maíra Canal <mca...@igalia.com>
> > Signed-off-by: Louis Chauvet <louis.chau...@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>
>
> Alphabetical order please.
>
> > #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);
> > +}
> > +
>
> This could be implemented in drm_managed.c. drmm_alloc_ordered_workqueue()
> would call alloc_ordered_workqueue() and set up the managed callback as
> well.
Hello Thomas,
Thanks for this review. For the next iteration, I will add the macro
drmm_alloc_ordered_workqueue:
void __drmm_destroy_workqueue(struct drm_device *device, void* res)
{
struct workqueue_struct *wq = res;
destroy_workqueue(wq);
}
EXPORT_SYMBOL(__drmm_destroy_workqueue);
#define drmm_alloc_ordered_workqueue(dev, fmt, flags, args...)
\
({
\
struct workqueue_struct *wq = alloc_ordered_workqueue(fmt,
flags, ##args); \
wq ? ({
\
int ret = drmm_add_action_or_reset(dev,
__drmm_destroy_workqueue, wq); \
ret ? ERR_PTR(ret) : wq;
\
}) :
\
wq;
\
})
Besides this, is there anything else you noticed that I should change
for the next iteration in the remaining patches?
Thanks,
Louis Chauvet
> Best regards
> Thomas
>
> > 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
> > e0409aba93496932b32a130ebb608ee53b1a9c59..7c142bfc3bd9de9556621db3e7f570dc0a4fab3a
> > 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -53,14 +53,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;
> > @@ -108,7 +100,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,
> >
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>
