linux-kernel - Re: [PATCH RESEND 1/2] drm/virtio: plane: use drm managed resources

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] [day] [month] [year] [list]

Message-ID: <2c3b55f9-7b18-4083-1b61-ad0ce6a13660@redhat.com>
Date:   Thu, 21 Jul 2022 12:38:39 +0200
From:   Danilo Krummrich <dakr@...hat.com>
To:     Sam Ravnborg <sam@...nborg.org>
Cc:     airlied@...ux.ie, dri-devel@...ts.freedesktop.org,
        kraxel@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND 1/2] drm/virtio: plane: use drm managed resources

Hi Sam,

On 7/20/22 17:17, Sam Ravnborg wrote:
> Hi Danilo,
> 
> thanks for submitting this patch.
> 
> On Wed, Jul 20, 2022 at 04:02:13PM +0200, Danilo Krummrich wrote:
>> Use drm managed resource allocation (drmm_universal_plane_alloc()) in
>> order to cleanup/simplify drm plane .destroy callback.
>>
>> Signed-off-by: Danilo Krummrich <dakr@...hat.com>
>> ---
>>   drivers/gpu/drm/virtio/virtgpu_plane.c | 30 +++++++-------------------
>>   1 file changed, 8 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> index 6d3cc9e238a4..3008551d6a05 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> @@ -67,16 +67,10 @@ uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
>>   	return format;
>>   }
>>   
>> -static void virtio_gpu_plane_destroy(struct drm_plane *plane)
>> -{
>> -	drm_plane_cleanup(plane);
>> -	kfree(plane);
>> -}
>> -
>>   static const struct drm_plane_funcs virtio_gpu_plane_funcs = {
>>   	.update_plane		= drm_atomic_helper_update_plane,
>>   	.disable_plane		= drm_atomic_helper_disable_plane,
>> -	.destroy		= virtio_gpu_plane_destroy,
>> +	.destroy		= drm_plane_cleanup,
> 
>  From the documentation of drmm_universal_plane_alloc:
> 
>      The @drm_plane_funcs.destroy hook must be NULL.
> 
> So the above assignment looks wrong.

Good catch! Indeed it registers the drm_plane_cleanup() with 
drmm_add_action_or_reset() already.

I'll send a v2.

> 
> The rest of this patch looks OK.
> 
> 	Sam

Danilo

> 
> 
>>   	.reset			= drm_atomic_helper_plane_reset,
>>   	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>>   	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
>> @@ -379,11 +373,7 @@ struct drm_plane *virtio_gpu_plane_init(struct virtio_gpu_device *vgdev,
>>   	const struct drm_plane_helper_funcs *funcs;
>>   	struct drm_plane *plane;
>>   	const uint32_t *formats;
>> -	int ret, nformats;
>> -
>> -	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
>> -	if (!plane)
>> -		return ERR_PTR(-ENOMEM);
>> +	int nformats;
>>   
>>   	if (type == DRM_PLANE_TYPE_CURSOR) {
>>   		formats = virtio_gpu_cursor_formats;
>> @@ -394,17 +384,13 @@ struct drm_plane *virtio_gpu_plane_init(struct virtio_gpu_device *vgdev,
>>   		nformats = ARRAY_SIZE(virtio_gpu_formats);
>>   		funcs = &virtio_gpu_primary_helper_funcs;
>>   	}
>> -	ret = drm_universal_plane_init(dev, plane, 1 << index,
>> -				       &virtio_gpu_plane_funcs,
>> -				       formats, nformats,
>> -				       NULL, type, NULL);
>> -	if (ret)
>> -		goto err_plane_init;
>> +
>> +	plane = drmm_universal_plane_alloc(dev, struct drm_plane, dev,
>> +					   1 << index, &virtio_gpu_plane_funcs,
>> +					   formats, nformats, NULL, type, NULL);
>> +	if (IS_ERR(plane))
>> +		return plane;
>>   
>>   	drm_plane_helper_add(plane, funcs);
>>   	return plane;
>> -
>> -err_plane_init:
>> -	kfree(plane);
>> -	return ERR_PTR(ret);
>>   }
>> -- 
>> 2.36.1
>