Re: [PATCH] drm/xe: Promote make_u64 macro to .h file

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Feb 13, 2024 at 10:05:43PM +0100, Michal Wajdeczko wrote:
> We shouldn't hide generic macros inside .c files as this may lead
> to unnecessary code duplication if similar functionality is needed
> elsewhere.  Promote make_u64 to .h as soon we will want to reuse it.

On the other hand, we shouldn't create an internal library of macros
that then start to get used indiscriminately as if it was Linux core
helpers.

Changes like this was how the i915_utils.h started.

If we really think that this macro is useful somewhere else, then
we should be looking to make that as part of the linux/types.h

> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_pagefault.c | 2 --
>  drivers/gpu/drm/xe/xe_macros.h       | 8 ++++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> index c26e4fcca01e..76a018ec8136 100644
> --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> @@ -557,8 +557,6 @@ static int handle_acc(struct xe_gt *gt, struct acc *acc)
>  	return ret;
>  }
>  
> -#define make_u64(hi__, low__)  ((u64)(hi__) << 32 | (u64)(low__))
> -
>  #define ACC_MSG_LEN_DW        4
>  
>  static bool get_acc(struct acc_queue *acc_queue, struct acc *acc)
> diff --git a/drivers/gpu/drm/xe/xe_macros.h b/drivers/gpu/drm/xe/xe_macros.h
> index daf56c846d03..b79d775c737e 100644
> --- a/drivers/gpu/drm/xe/xe_macros.h
> +++ b/drivers/gpu/drm/xe/xe_macros.h
> @@ -7,6 +7,7 @@
>  #define _XE_MACROS_H_
>  
>  #include <linux/bug.h>
> +#include <linux/types.h>
>  
>  #define XE_WARN_ON WARN_ON
>  
> @@ -15,4 +16,11 @@
>  			    "Ioctl argument check failed at %s:%d: %s", \
>  			    __FILE__, __LINE__, #cond), 1))
>  
> +/**
> + * make_u64 - make u64 value from two u32 values
> + * @hi__: value representing upper 32 bits
> + * @lo__: value representing lower 32 bits
> + */
> +#define make_u64(hi__, low__)  ((u64)(hi__) << 32 | (u64)(low__))
> +
>  #endif
> -- 
> 2.43.0
>