[PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t

[PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t

[Thomas Hellström]

The relatively recently introduced drm/exec utility was using uint32_t
in its interface, which was then also carried over to drm/gpuvm.

Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly.

Cc: Christian König <christian.koenig@amd.com>
Cc: Danilo Krummrich <dakr@redhat.com>
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/drm_exec.c | 2 +-
 include/drm/drm_exec.h     | 4 ++--
 include/drm/drm_gpuvm.h    | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
index 5d2809de4517..20e59d88218d 100644
--- a/drivers/gpu/drm/drm_exec.c
+++ b/drivers/gpu/drm/drm_exec.c
@@ -72,7 +72,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec)
  *
  * Initialize the object and make sure that we can track locked objects.
  */
-void drm_exec_init(struct drm_exec *exec, uint32_t flags)
+void drm_exec_init(struct drm_exec *exec, u32 flags)
 {
 	exec->flags = flags;
 	exec->objects = kmalloc(PAGE_SIZE, GFP_KERNEL);
diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
index b5bf0b6da791..187c3ec44606 100644
--- a/include/drm/drm_exec.h
+++ b/include/drm/drm_exec.h
@@ -18,7 +18,7 @@ struct drm_exec {
 	/**
 	 * @flags: Flags to control locking behavior
 	 */
-	uint32_t		flags;
+	u32                     flags;
 
 	/**
 	 * @ticket: WW ticket used for acquiring locks
@@ -135,7 +135,7 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec)
 	return !!exec->contended;
 }
 
-void drm_exec_init(struct drm_exec *exec, uint32_t flags);
+void drm_exec_init(struct drm_exec *exec, u32 flags);
 void drm_exec_fini(struct drm_exec *exec);
 bool drm_exec_cleanup(struct drm_exec *exec);
 int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj);
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 48311e6d664c..554046321d24 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -514,7 +514,7 @@ struct drm_gpuvm_exec {
 	/**
 	 * @flags: the flags for the struct drm_exec
 	 */
-	uint32_t flags;
+	u32 flags;
 
 	/**
 	 * @vm: the &drm_gpuvm to lock its DMA reservations
-- 
2.43.0

Re: [PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t

[Christian König]

Am 19.01.24 um 10:05 schrieb Thomas Hellström:
> The relatively recently introduced drm/exec utility was using uint32_t
> in its interface, which was then also carried over to drm/gpuvm.
>
> Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly.
>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Danilo Krummrich <dakr@redhat.com>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/drm_exec.c | 2 +-
>   include/drm/drm_exec.h     | 4 ++--
>   include/drm/drm_gpuvm.h    | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
> index 5d2809de4517..20e59d88218d 100644
> --- a/drivers/gpu/drm/drm_exec.c
> +++ b/drivers/gpu/drm/drm_exec.c
> @@ -72,7 +72,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec)
>    *
>    * Initialize the object and make sure that we can track locked objects.
>    */
> -void drm_exec_init(struct drm_exec *exec, uint32_t flags)
> +void drm_exec_init(struct drm_exec *exec, u32 flags)
>   {
>   	exec->flags = flags;
>   	exec->objects = kmalloc(PAGE_SIZE, GFP_KERNEL);
> diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
> index b5bf0b6da791..187c3ec44606 100644
> --- a/include/drm/drm_exec.h
> +++ b/include/drm/drm_exec.h
> @@ -18,7 +18,7 @@ struct drm_exec {
>   	/**
>   	 * @flags: Flags to control locking behavior
>   	 */
> -	uint32_t		flags;
> +	u32                     flags;
>   
>   	/**
>   	 * @ticket: WW ticket used for acquiring locks
> @@ -135,7 +135,7 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec)
>   	return !!exec->contended;
>   }
>   
> -void drm_exec_init(struct drm_exec *exec, uint32_t flags);
> +void drm_exec_init(struct drm_exec *exec, u32 flags);
>   void drm_exec_fini(struct drm_exec *exec);
>   bool drm_exec_cleanup(struct drm_exec *exec);
>   int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj);
> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> index 48311e6d664c..554046321d24 100644
> --- a/include/drm/drm_gpuvm.h
> +++ b/include/drm/drm_gpuvm.h
> @@ -514,7 +514,7 @@ struct drm_gpuvm_exec {
>   	/**
>   	 * @flags: the flags for the struct drm_exec
>   	 */
> -	uint32_t flags;
> +	u32 flags;
>   
>   	/**
>   	 * @vm: the &drm_gpuvm to lock its DMA reservations

✗ CI.Patch_applied: failure for drm/exec, drm/gpuvm: Prefer u32 over uint32_t

[Patchwork]

== Series Details ==

Series: drm/exec, drm/gpuvm: Prefer u32 over uint32_t
URL   : https://patchwork.freedesktop.org/series/128973/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: ad8fc2d4a drm-tip: 2024y-01m-19d-08h-36m-49s UTC integration manifest
=== git am output follows ===
error: patch failed: drivers/gpu/drm/drm_exec.c:72
error: drivers/gpu/drm/drm_exec.c: patch does not apply
error: patch failed: include/drm/drm_exec.h:135
error: include/drm/drm_exec.h: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch
Applying: drm/exec, drm/gpuvm: Prefer u32 over uint32_t
Patch failed at 0001 drm/exec, drm/gpuvm: Prefer u32 over uint32_t
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Re: [PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t

[Danilo Krummrich]

On 1/19/24 10:05, Thomas Hellström wrote:
> The relatively recently introduced drm/exec utility was using uint32_t
> in its interface, which was then also carried over to drm/gpuvm.
> 
> Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly.
> 
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Danilo Krummrich <dakr@redhat.com>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

Reviewed-by: Danilo Krummrich <dakr@redhat.com>

> ---
>   drivers/gpu/drm/drm_exec.c | 2 +-
>   include/drm/drm_exec.h     | 4 ++--
>   include/drm/drm_gpuvm.h    | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
> index 5d2809de4517..20e59d88218d 100644
> --- a/drivers/gpu/drm/drm_exec.c
> +++ b/drivers/gpu/drm/drm_exec.c
> @@ -72,7 +72,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec)
>    *
>    * Initialize the object and make sure that we can track locked objects.
>    */
> -void drm_exec_init(struct drm_exec *exec, uint32_t flags)
> +void drm_exec_init(struct drm_exec *exec, u32 flags)
>   {
>   	exec->flags = flags;
>   	exec->objects = kmalloc(PAGE_SIZE, GFP_KERNEL);
> diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
> index b5bf0b6da791..187c3ec44606 100644
> --- a/include/drm/drm_exec.h
> +++ b/include/drm/drm_exec.h
> @@ -18,7 +18,7 @@ struct drm_exec {
>   	/**
>   	 * @flags: Flags to control locking behavior
>   	 */
> -	uint32_t		flags;
> +	u32                     flags;
>   
>   	/**
>   	 * @ticket: WW ticket used for acquiring locks
> @@ -135,7 +135,7 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec)
>   	return !!exec->contended;
>   }
>   
> -void drm_exec_init(struct drm_exec *exec, uint32_t flags);
> +void drm_exec_init(struct drm_exec *exec, u32 flags);
>   void drm_exec_fini(struct drm_exec *exec);
>   bool drm_exec_cleanup(struct drm_exec *exec);
>   int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj);
> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> index 48311e6d664c..554046321d24 100644
> --- a/include/drm/drm_gpuvm.h
> +++ b/include/drm/drm_gpuvm.h
> @@ -514,7 +514,7 @@ struct drm_gpuvm_exec {
>   	/**
>   	 * @flags: the flags for the struct drm_exec
>   	 */
> -	uint32_t flags;
> +	u32 flags;
>   
>   	/**
>   	 * @vm: the &drm_gpuvm to lock its DMA reservations

Re: [PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t

[Lucas De Marchi]

On Fri, Jan 19, 2024 at 10:05:57AM +0100, Thomas Hellström wrote:
>The relatively recently introduced drm/exec utility was using uint32_t
>in its interface, which was then also carried over to drm/gpuvm.
>
>Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly.
>
>Cc: Christian König <christian.koenig@amd.com>
>Cc: Danilo Krummrich <dakr@redhat.com>
>Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>---
> drivers/gpu/drm/drm_exec.c | 2 +-
> include/drm/drm_exec.h     | 4 ++--
> include/drm/drm_gpuvm.h    | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

I was surprised we have quite a few places using the c99 types rather
than kernel types.

$ git grep -ce uint[0-9][0-9]_t drivers/gpu/drm/*.c
drivers/gpu/drm/drm_atomic.c:1
drivers/gpu/drm/drm_atomic_helper.c:7
drivers/gpu/drm/drm_atomic_state_helper.c:1
drivers/gpu/drm/drm_atomic_uapi.c:17
drivers/gpu/drm/drm_color_mgmt.c:4
drivers/gpu/drm/drm_connector.c:6
drivers/gpu/drm/drm_crtc.c:3
drivers/gpu/drm/drm_damage_helper.c:2
drivers/gpu/drm/drm_debugfs_crc.c:1
drivers/gpu/drm/drm_exec.c:1
drivers/gpu/drm/drm_fb_helper.c:10
drivers/gpu/drm/drm_format_helper.c:6
drivers/gpu/drm/drm_fourcc.c:6
drivers/gpu/drm/drm_framebuffer.c:5
drivers/gpu/drm/drm_gem.c:1
drivers/gpu/drm/drm_gem_dma_helper.c:1
drivers/gpu/drm/drm_gem_shmem_helper.c:1
drivers/gpu/drm/drm_gem_ttm_helper.c:1
drivers/gpu/drm/drm_gem_vram_helper.c:5
drivers/gpu/drm/drm_lease.c:6
drivers/gpu/drm/drm_mipi_dbi.c:3
drivers/gpu/drm/drm_mode_config.c:4
drivers/gpu/drm/drm_mode_object.c:20
drivers/gpu/drm/drm_modeset_helper.c:1
drivers/gpu/drm/drm_modeset_lock.c:1
drivers/gpu/drm/drm_of.c:3
drivers/gpu/drm/drm_plane.c:35
drivers/gpu/drm/drm_plane_helper.c:2
drivers/gpu/drm/drm_prime.c:9
drivers/gpu/drm/drm_probe_helper.c:3
drivers/gpu/drm/drm_property.c:11
drivers/gpu/drm/drm_simple_kms_helper.c:4
drivers/gpu/drm/drm_syncobj.c:26

but maybe not worth the churn for what is already there for a long time?

Lucas De Marchi

>
>diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
>index 5d2809de4517..20e59d88218d 100644
>--- a/drivers/gpu/drm/drm_exec.c
>+++ b/drivers/gpu/drm/drm_exec.c
>@@ -72,7 +72,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec)
>  *
>  * Initialize the object and make sure that we can track locked objects.
>  */
>-void drm_exec_init(struct drm_exec *exec, uint32_t flags)
>+void drm_exec_init(struct drm_exec *exec, u32 flags)
> {
> 	exec->flags = flags;
> 	exec->objects = kmalloc(PAGE_SIZE, GFP_KERNEL);
>diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
>index b5bf0b6da791..187c3ec44606 100644
>--- a/include/drm/drm_exec.h
>+++ b/include/drm/drm_exec.h
>@@ -18,7 +18,7 @@ struct drm_exec {
> 	/**
> 	 * @flags: Flags to control locking behavior
> 	 */
>-	uint32_t		flags;
>+	u32                     flags;
>
> 	/**
> 	 * @ticket: WW ticket used for acquiring locks
>@@ -135,7 +135,7 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec)
> 	return !!exec->contended;
> }
>
>-void drm_exec_init(struct drm_exec *exec, uint32_t flags);
>+void drm_exec_init(struct drm_exec *exec, u32 flags);
> void drm_exec_fini(struct drm_exec *exec);
> bool drm_exec_cleanup(struct drm_exec *exec);
> int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj);
>diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
>index 48311e6d664c..554046321d24 100644
>--- a/include/drm/drm_gpuvm.h
>+++ b/include/drm/drm_gpuvm.h
>@@ -514,7 +514,7 @@ struct drm_gpuvm_exec {
> 	/**
> 	 * @flags: the flags for the struct drm_exec
> 	 */
>-	uint32_t flags;
>+	u32 flags;
>
> 	/**
> 	 * @vm: the &drm_gpuvm to lock its DMA reservations
>-- 
>2.43.0
>

Re: [PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t

[Jani Nikula]

On Fri, 19 Jan 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Fri, Jan 19, 2024 at 10:05:57AM +0100, Thomas Hellström wrote:
>>The relatively recently introduced drm/exec utility was using uint32_t
>>in its interface, which was then also carried over to drm/gpuvm.
>>
>>Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly.
>>
>>Cc: Christian König <christian.koenig@amd.com>
>>Cc: Danilo Krummrich <dakr@redhat.com>
>>Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>>---
>> drivers/gpu/drm/drm_exec.c | 2 +-
>> include/drm/drm_exec.h     | 4 ++--
>> include/drm/drm_gpuvm.h    | 2 +-
>> 3 files changed, 4 insertions(+), 4 deletions(-)
>
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> I was surprised we have quite a few places using the c99 types rather
> than kernel types.
>
> $ git grep -ce uint[0-9][0-9]_t drivers/gpu/drm/*.c
> drivers/gpu/drm/drm_atomic.c:1
> drivers/gpu/drm/drm_atomic_helper.c:7
> drivers/gpu/drm/drm_atomic_state_helper.c:1
> drivers/gpu/drm/drm_atomic_uapi.c:17
> drivers/gpu/drm/drm_color_mgmt.c:4
> drivers/gpu/drm/drm_connector.c:6
> drivers/gpu/drm/drm_crtc.c:3
> drivers/gpu/drm/drm_damage_helper.c:2
> drivers/gpu/drm/drm_debugfs_crc.c:1
> drivers/gpu/drm/drm_exec.c:1
> drivers/gpu/drm/drm_fb_helper.c:10
> drivers/gpu/drm/drm_format_helper.c:6
> drivers/gpu/drm/drm_fourcc.c:6
> drivers/gpu/drm/drm_framebuffer.c:5
> drivers/gpu/drm/drm_gem.c:1
> drivers/gpu/drm/drm_gem_dma_helper.c:1
> drivers/gpu/drm/drm_gem_shmem_helper.c:1
> drivers/gpu/drm/drm_gem_ttm_helper.c:1
> drivers/gpu/drm/drm_gem_vram_helper.c:5
> drivers/gpu/drm/drm_lease.c:6
> drivers/gpu/drm/drm_mipi_dbi.c:3
> drivers/gpu/drm/drm_mode_config.c:4
> drivers/gpu/drm/drm_mode_object.c:20
> drivers/gpu/drm/drm_modeset_helper.c:1
> drivers/gpu/drm/drm_modeset_lock.c:1
> drivers/gpu/drm/drm_of.c:3
> drivers/gpu/drm/drm_plane.c:35
> drivers/gpu/drm/drm_plane_helper.c:2
> drivers/gpu/drm/drm_prime.c:9
> drivers/gpu/drm/drm_probe_helper.c:3
> drivers/gpu/drm/drm_property.c:11
> drivers/gpu/drm/drm_simple_kms_helper.c:4
> drivers/gpu/drm/drm_syncobj.c:26
>
> but maybe not worth the churn for what is already there for a long time?

Personally, I think the one time churn is worth it to unify and keep the
codebase in kernel types only. This is basically what we did in i915
years ago, and new c99 types don't really even creep in because there
are zero examples around. It's natural to follow the style around you
instead of mixing.

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH] drm/exec, drm/gpuvm: Prefer u32 over uint32_t

[Thomas Hellström]

Hi,

On 1/19/24 16:32, Jani Nikula wrote:
> On Fri, 19 Jan 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> On Fri, Jan 19, 2024 at 10:05:57AM +0100, Thomas Hellström wrote:
>>> The relatively recently introduced drm/exec utility was using uint32_t
>>> in its interface, which was then also carried over to drm/gpuvm.
>>>
>>> Prefer u32 in new code and update drm/exec and drm/gpuvm accordingly.
>>>
>>> Cc: Christian König <christian.koenig@amd.com>
>>> Cc: Danilo Krummrich <dakr@redhat.com>
>>> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/drm_exec.c | 2 +-
>>> include/drm/drm_exec.h     | 4 ++--
>>> include/drm/drm_gpuvm.h    | 2 +-
>>> 3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>> I was surprised we have quite a few places using the c99 types rather
>> than kernel types.
>>
>> $ git grep -ce uint[0-9][0-9]_t drivers/gpu/drm/*.c
>> drivers/gpu/drm/drm_atomic.c:1
>> drivers/gpu/drm/drm_atomic_helper.c:7
>> drivers/gpu/drm/drm_atomic_state_helper.c:1
>> drivers/gpu/drm/drm_atomic_uapi.c:17
>> drivers/gpu/drm/drm_color_mgmt.c:4
>> drivers/gpu/drm/drm_connector.c:6
>> drivers/gpu/drm/drm_crtc.c:3
>> drivers/gpu/drm/drm_damage_helper.c:2
>> drivers/gpu/drm/drm_debugfs_crc.c:1
>> drivers/gpu/drm/drm_exec.c:1
>> drivers/gpu/drm/drm_fb_helper.c:10
>> drivers/gpu/drm/drm_format_helper.c:6
>> drivers/gpu/drm/drm_fourcc.c:6
>> drivers/gpu/drm/drm_framebuffer.c:5
>> drivers/gpu/drm/drm_gem.c:1
>> drivers/gpu/drm/drm_gem_dma_helper.c:1
>> drivers/gpu/drm/drm_gem_shmem_helper.c:1
>> drivers/gpu/drm/drm_gem_ttm_helper.c:1
>> drivers/gpu/drm/drm_gem_vram_helper.c:5
>> drivers/gpu/drm/drm_lease.c:6
>> drivers/gpu/drm/drm_mipi_dbi.c:3
>> drivers/gpu/drm/drm_mode_config.c:4
>> drivers/gpu/drm/drm_mode_object.c:20
>> drivers/gpu/drm/drm_modeset_helper.c:1
>> drivers/gpu/drm/drm_modeset_lock.c:1
>> drivers/gpu/drm/drm_of.c:3
>> drivers/gpu/drm/drm_plane.c:35
>> drivers/gpu/drm/drm_plane_helper.c:2
>> drivers/gpu/drm/drm_prime.c:9
>> drivers/gpu/drm/drm_probe_helper.c:3
>> drivers/gpu/drm/drm_property.c:11
>> drivers/gpu/drm/drm_simple_kms_helper.c:4
>> drivers/gpu/drm/drm_syncobj.c:26
>>
>> but maybe not worth the churn for what is already there for a long time?

This originally dates back to around or slightly after when the drm code 
was a set of template headers with the objective of sharing code with 
some bsds, and then I guess it also leaked. The reason I sent this patch 
was I made a review comment of this for drm_gpuvm and then also promised 
to send a patch against drm_exec.

> Personally, I think the one time churn is worth it to unify and keep the
> codebase in kernel types only. This is basically what we did in i915
> years ago, and new c99 types don't really even creep in because there
> are zero examples around. It's natural to follow the style around you
> instead of mixing.

+1.

/Thomas


> BR,
> Jani.
>
>