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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 08 Apr 2024, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> On 14.02.2024 14:36, Jani Nikula wrote:
>> On Tue, 13 Feb 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>>> 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
>> 
>> Agreed.
>> 
>> Well, maybe not specifically types.h, but maybe a separate header.
>
> My attempts to promote this to a global macro reached dead end. To move
> forward please either engage in LKML [1] [2] discussions or accept this
> local solution or suggest other option.

Tried to nudge things forward. Fingers crossed.

BR,
Jani.

>
> Michal
>
> [1]
> https://lore.kernel.org/lkml/20240214155408.1569-1-michal.wajdeczko@intel.com/
>
> [2]
> https://lore.kernel.org/lkml/20240214214654.1700-1-michal.wajdeczko@intel.com/
>
>
>> 
>> There's a need for something like this:
>> 
>> $ git grep "hi[ )]*<< *\(8\|16\|32\)[ )]*|.*lo"
>> 
>> Ditto for taking higher/lower half of a variable.
>> 
>> 
>> BR,
>> Jani.
>> 
>> 
>>>
>>>>
>>>> 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
>>>>
>> 

-- 
Jani Nikula, Intel