Re: [PATCH v2] drm/xe: Prefer BIT/GENMASK macros over shifts

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

On Tue, 30 Jan 2024, Matthew Brost <matthew.brost@intel.com> wrote:
> On Wed, Jan 24, 2024 at 11:45:24AM +0200, Jani Nikula wrote:
>> On Tue, 23 Jan 2024, Matthew Brost <matthew.brost@intel.com> wrote:
>> > Using BIT/GENMASK macros is a better convention than using manual shift
>> > and will also fix build errors [1].
>> 
>> One of the reasons i915 does not really use BIT and GENMASK directly is
>> that their type is unsigned long, and thus their size is different on 32
>> and 64 bit builds, while almost invariably you need one or the other to
>> describe hardware or firmware interfaces. Not something that depends on
>> the build.
>> 
>> Using %lu or %lx as format specifier should work, but people find it
>> weird to have to use that for essentially 32-bit things, and forget. And
>> there's confusion when you still have stuff like:
>> 
>> #define GUC_CTB_STATUS_NO_ERROR				0
>> 
>> instead of 0UL i.e. some of the macros end up being unsigned long with
>> variable size, and some, like this, not.
>> 
>> I'm sure the REG_BIT and REG_GENMASK macros feel off-putting for things
>> that aren't exactly registers (like the firmware interface), but let's
>> hope we get Lucas' fixed-size BIT and GENMASK macros merged [1], and can
>> use them.
>> 
>
> I'm fine with using these new macros but I believe we need to get a fix
> into 6.8 and unsure if Lucas's series is going to land before then. Are
> you ok with landing this series as is for now and updating all of the
> defines in Xe in a follow up? Or do you have other ideas of how to
> proceed for 6.8?

I'll defer to the xe maintainers on this one. Just giving some
background what the issues with BIT/GENMASK were.

BR,
Jani.


-- 
Jani Nikula, Intel