diff mbox series

[19/20] drm/xlnx: Initialize DRM driver instance with CMA helper macro

Message ID 20200813083644.31711-20-tzimmermann@suse.de (mailing list archive)
State Superseded
Headers show
Series Convert all remaining drivers to GEM object functions | expand

Commit Message

Thomas Zimmermann Aug. 13, 2020, 8:36 a.m. UTC
The xlnx driver uses CMA helpers with default callback functions.
Initialize the driver structure with the rsp CMA helper macro. The
driver is being converted to use GEM object functions as part of
this change.

Two callbacks, .dumb_destroy and .gem_prime_import, were initialized
to their default implementations, so they are just kept empty now.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

Comments

Laurent Pinchart Aug. 13, 2020, 1:36 p.m. UTC | #1
Hi Thomas,

Thank you for the patch.

On Thu, Aug 13, 2020 at 10:36:43AM +0200, Thomas Zimmermann wrote:
> The xlnx driver uses CMA helpers with default callback functions.
> Initialize the driver structure with the rsp CMA helper macro. The
> driver is being converted to use GEM object functions as part of
> this change.
> 
> Two callbacks, .dumb_destroy and .gem_prime_import, were initialized
> to their default implementations, so they are just kept empty now.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> index 26328c76305b..058044dcc062 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> @@ -80,19 +80,7 @@ static struct drm_driver zynqmp_dpsub_drm_driver = {
>  	.driver_features		= DRIVER_MODESET | DRIVER_GEM |
>  					  DRIVER_ATOMIC,
>  
> -	.prime_handle_to_fd		= drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle		= drm_gem_prime_fd_to_handle,
> -	.gem_prime_export		= drm_gem_prime_export,
> -	.gem_prime_import		= drm_gem_prime_import,
> -	.gem_prime_get_sg_table		= drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table	= drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap			= drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap		= drm_gem_cma_prime_vunmap,
> -	.gem_prime_mmap			= drm_gem_cma_prime_mmap,
> -	.gem_free_object_unlocked	= drm_gem_cma_free_object,
> -	.gem_vm_ops			= &drm_gem_cma_vm_ops,
> -	.dumb_create			= zynqmp_dpsub_dumb_create,
> -	.dumb_destroy			= drm_gem_dumb_destroy,
> +	DRM_GEM_CMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE(zynqmp_dpsub_dumb_create),

The only effective change here is

-	.gem_prime_import_sg_table	= drm_gem_cma_prime_import_sg_table,
-	.gem_prime_mmap			= drm_gem_cma_prime_mmap,
+	.gem_prime_import_sg_table	= drm_gem_cma_prime_import_sg_table_vmap,
+	.gem_prime_mmap			= drm_gem_prime_mmap,

The change is significant, and I have a hard time following the code to
verify that it's correct, or if it's an undesired side effect. If it's
correct, could the change be mentioned in the commit message, with at
least a brief explanation of why this is correct, and what the
consequences here ?

>  
>  	.fops				= &zynqmp_dpsub_drm_fops,
>
Thomas Zimmermann Aug. 13, 2020, 1:57 p.m. UTC | #2
Hi

Am 13.08.20 um 15:36 schrieb Laurent Pinchart:
> Hi Thomas,
> 
> Thank you for the patch.
> 
> On Thu, Aug 13, 2020 at 10:36:43AM +0200, Thomas Zimmermann wrote:
>> The xlnx driver uses CMA helpers with default callback functions.
>> Initialize the driver structure with the rsp CMA helper macro. The
>> driver is being converted to use GEM object functions as part of
>> this change.
>>
>> Two callbacks, .dumb_destroy and .gem_prime_import, were initialized
>> to their default implementations, so they are just kept empty now.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 14 +-------------
>>  1 file changed, 1 insertion(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
>> index 26328c76305b..058044dcc062 100644
>> --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
>> +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
>> @@ -80,19 +80,7 @@ static struct drm_driver zynqmp_dpsub_drm_driver = {
>>  	.driver_features		= DRIVER_MODESET | DRIVER_GEM |
>>  					  DRIVER_ATOMIC,
>>  
>> -	.prime_handle_to_fd		= drm_gem_prime_handle_to_fd,
>> -	.prime_fd_to_handle		= drm_gem_prime_fd_to_handle,
>> -	.gem_prime_export		= drm_gem_prime_export,
>> -	.gem_prime_import		= drm_gem_prime_import,
>> -	.gem_prime_get_sg_table		= drm_gem_cma_prime_get_sg_table,
>> -	.gem_prime_import_sg_table	= drm_gem_cma_prime_import_sg_table,
>> -	.gem_prime_vmap			= drm_gem_cma_prime_vmap,
>> -	.gem_prime_vunmap		= drm_gem_cma_prime_vunmap,
>> -	.gem_prime_mmap			= drm_gem_cma_prime_mmap,
>> -	.gem_free_object_unlocked	= drm_gem_cma_free_object,
>> -	.gem_vm_ops			= &drm_gem_cma_vm_ops,
>> -	.dumb_create			= zynqmp_dpsub_dumb_create,
>> -	.dumb_destroy			= drm_gem_dumb_destroy,
>> +	DRM_GEM_CMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE(zynqmp_dpsub_dumb_create),
> 
> The only effective change here is
> 
> -	.gem_prime_import_sg_table	= drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_mmap			= drm_gem_cma_prime_mmap,
> +	.gem_prime_import_sg_table	= drm_gem_cma_prime_import_sg_table_vmap,
> +	.gem_prime_mmap			= drm_gem_prime_mmap,
> 
> The change is significant, and I have a hard time following the code to
> verify that it's correct, or if it's an undesired side effect. If it's
> correct, could the change be mentioned in the commit message, with at
> least a brief explanation of why this is correct, and what the
> consequences here ?

I think this is a mistake that I didn't notice. Thanks for spotting it.
Initializing the driver structure
DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE should fix it. I'll change this
for the patch's next revision.

Best regards
Thomas

> 
>>  
>>  	.fops				= &zynqmp_dpsub_drm_fops,
>>  
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
index 26328c76305b..058044dcc062 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
@@ -80,19 +80,7 @@  static struct drm_driver zynqmp_dpsub_drm_driver = {
 	.driver_features		= DRIVER_MODESET | DRIVER_GEM |
 					  DRIVER_ATOMIC,
 
-	.prime_handle_to_fd		= drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle		= drm_gem_prime_fd_to_handle,
-	.gem_prime_export		= drm_gem_prime_export,
-	.gem_prime_import		= drm_gem_prime_import,
-	.gem_prime_get_sg_table		= drm_gem_cma_prime_get_sg_table,
-	.gem_prime_import_sg_table	= drm_gem_cma_prime_import_sg_table,
-	.gem_prime_vmap			= drm_gem_cma_prime_vmap,
-	.gem_prime_vunmap		= drm_gem_cma_prime_vunmap,
-	.gem_prime_mmap			= drm_gem_cma_prime_mmap,
-	.gem_free_object_unlocked	= drm_gem_cma_free_object,
-	.gem_vm_ops			= &drm_gem_cma_vm_ops,
-	.dumb_create			= zynqmp_dpsub_dumb_create,
-	.dumb_destroy			= drm_gem_dumb_destroy,
+	DRM_GEM_CMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE(zynqmp_dpsub_dumb_create),
 
 	.fops				= &zynqmp_dpsub_drm_fops,