linux-kernel - Re: [PATCH v5 3/5] drm/vkms: Switch to managed for crtc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ffce2bf6-543b-4489-9e81-d6e76025035c@igalia.com>
Date: Mon, 25 Nov 2024 19:39:14 -0300
From: Maíra Canal <mcanal@...lia.com>
To: Louis Chauvet <louis.chauvet@...tlin.com>,
 Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>,
 Melissa Wen <melissa.srw@...il.com>,
 Haneen Mohammed <hamohammed.sa@...il.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Simona Vetter <simona.vetter@...ll.ch>
Cc: dri-devel@...ts.freedesktop.org, arthurgrillo@...eup.net,
 linux-kernel@...r.kernel.org, jeremie.dautheribes@...tlin.com,
 miquel.raynal@...tlin.com, thomas.petazzoni@...tlin.com,
 seanpaul@...gle.com, nicolejadeyee@...gle.com
Subject: Re: [PATCH v5 3/5] drm/vkms: Switch to managed for crtc

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@...tlin.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@...lia.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,
>