[3/3] drm/i915: Temporary conversion to new GENMASK/BIT macros
Commit Message
Convert the REG_* macros from i915_reg_defs.h to use the new macros
defined in linux/bits.h. This is just to help on the implementation
of the new macros and not intended to be applied.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/i915_reg_defs.h | 28 +++++-----------------------
1 file changed, 5 insertions(+), 23 deletions(-)
Comments
On Mon, 08 May 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> Convert the REG_* macros from i915_reg_defs.h to use the new macros
> defined in linux/bits.h. This is just to help on the implementation
> of the new macros and not intended to be applied.
This drops a number of build time input checks as well as casts to the
specified types.
BR,
Jani.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg_defs.h | 28 +++++-----------------------
> 1 file changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
> index 622d603080f9..61fbb8d62b25 100644
> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> @@ -17,10 +17,7 @@
> *
> * @return: Value with bit @__n set.
> */
> -#define REG_BIT(__n) \
> - ((u32)(BIT(__n) + \
> - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
> - ((__n) < 0 || (__n) > 31))))
> +#define REG_BIT(__n) BIT_U32(__n)
>
> /**
> * REG_BIT8() - Prepare a u8 bit value
> @@ -30,10 +27,7 @@
> *
> * @return: Value with bit @__n set.
> */
> -#define REG_BIT8(__n) \
> - ((u8)(BIT(__n) + \
> - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
> - ((__n) < 0 || (__n) > 7))))
> +#define REG_BIT8(__n) BIT_U8(__n)
>
> /**
> * REG_GENMASK() - Prepare a continuous u32 bitmask
> @@ -44,11 +38,7 @@
> *
> * @return: Continuous bitmask from @__high to @__low, inclusive.
> */
> -#define REG_GENMASK(__high, __low) \
> - ((u32)(GENMASK(__high, __low) + \
> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
> - __is_constexpr(__low) && \
> - ((__low) < 0 || (__high) > 31 || (__low) > (__high)))))
> +#define REG_GENMASK(__high, __low) GENMASK_U32(__high, __low)
>
> /**
> * REG_GENMASK64() - Prepare a continuous u64 bitmask
> @@ -59,11 +49,7 @@
> *
> * @return: Continuous bitmask from @__high to @__low, inclusive.
> */
> -#define REG_GENMASK64(__high, __low) \
> - ((u64)(GENMASK_ULL(__high, __low) + \
> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
> - __is_constexpr(__low) && \
> - ((__low) < 0 || (__high) > 63 || (__low) > (__high)))))
> +#define REG_GENMASK64(__high, __low) GENMASK_ULL(__high, __low)
>
> /**
> * REG_GENMASK8() - Prepare a continuous u8 bitmask
> @@ -74,11 +60,7 @@
> *
> * @return: Continuous bitmask from @__high to @__low, inclusive.
> */
> -#define REG_GENMASK8(__high, __low) \
> - ((u8)(GENMASK(__high, __low) + \
> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
> - __is_constexpr(__low) && \
> - ((__low) < 0 || (__high) > 7 || (__low) > (__high)))))
> +#define REG_GENMASK8(__high, __low) GENMASK_U8(__high, __low)
>
> /*
> * Local integer constant expression version of is_power_of_2().
On Tue, May 09, 2023 at 10:57:19AM +0300, Jani Nikula wrote:
>On Mon, 08 May 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> Convert the REG_* macros from i915_reg_defs.h to use the new macros
>> defined in linux/bits.h. This is just to help on the implementation
>> of the new macros and not intended to be applied.
>
>This drops a number of build time input checks as well as casts to the
>specified types.
the explicit checks... but the checks are still there and the compiler
still gives me a warning or error for using invalid values. See test program in
the second patch. Example:
static const u32 a2 = GENMASK_U32(32, 0);
In file included from /tmp/genmask.c:2:
include/linux/bits.h:47:19: warning: right shift count is negative [-Wshift-count-negative]
47 | (~U32(0) >> (32 - 1 - (h))))
| ^~
It's a warning, not an error though.
Same warning for the other fixed-widths with numbers above the supported width.
For negative values:
In file included from include/linux/bits.h:21,
from /tmp/genmask.c:2:
include/linux/build_bug.h:16:51: error: negative width in bit-field ‘<anonymous>’
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
The cast to the specified type we lose indeed. Could you give an example where
those are useful in the context they are used? I debated adding them, but couldn't find
a justified use of them.
Lucas De Marchi
>
>BR,
>Jani.
>
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg_defs.h | 28 +++++-----------------------
>> 1 file changed, 5 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
>> index 622d603080f9..61fbb8d62b25 100644
>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
>> @@ -17,10 +17,7 @@
>> *
>> * @return: Value with bit @__n set.
>> */
>> -#define REG_BIT(__n) \
>> - ((u32)(BIT(__n) + \
>> - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
>> - ((__n) < 0 || (__n) > 31))))
>> +#define REG_BIT(__n) BIT_U32(__n)
>>
>> /**
>> * REG_BIT8() - Prepare a u8 bit value
>> @@ -30,10 +27,7 @@
>> *
>> * @return: Value with bit @__n set.
>> */
>> -#define REG_BIT8(__n) \
>> - ((u8)(BIT(__n) + \
>> - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
>> - ((__n) < 0 || (__n) > 7))))
>> +#define REG_BIT8(__n) BIT_U8(__n)
>>
>> /**
>> * REG_GENMASK() - Prepare a continuous u32 bitmask
>> @@ -44,11 +38,7 @@
>> *
>> * @return: Continuous bitmask from @__high to @__low, inclusive.
>> */
>> -#define REG_GENMASK(__high, __low) \
>> - ((u32)(GENMASK(__high, __low) + \
>> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
>> - __is_constexpr(__low) && \
>> - ((__low) < 0 || (__high) > 31 || (__low) > (__high)))))
>> +#define REG_GENMASK(__high, __low) GENMASK_U32(__high, __low)
>>
>> /**
>> * REG_GENMASK64() - Prepare a continuous u64 bitmask
>> @@ -59,11 +49,7 @@
>> *
>> * @return: Continuous bitmask from @__high to @__low, inclusive.
>> */
>> -#define REG_GENMASK64(__high, __low) \
>> - ((u64)(GENMASK_ULL(__high, __low) + \
>> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
>> - __is_constexpr(__low) && \
>> - ((__low) < 0 || (__high) > 63 || (__low) > (__high)))))
>> +#define REG_GENMASK64(__high, __low) GENMASK_ULL(__high, __low)
>>
>> /**
>> * REG_GENMASK8() - Prepare a continuous u8 bitmask
>> @@ -74,11 +60,7 @@
>> *
>> * @return: Continuous bitmask from @__high to @__low, inclusive.
>> */
>> -#define REG_GENMASK8(__high, __low) \
>> - ((u8)(GENMASK(__high, __low) + \
>> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
>> - __is_constexpr(__low) && \
>> - ((__low) < 0 || (__high) > 7 || (__low) > (__high)))))
>> +#define REG_GENMASK8(__high, __low) GENMASK_U8(__high, __low)
>>
>> /*
>> * Local integer constant expression version of is_power_of_2().
>
>--
>Jani Nikula, Intel Open Source Graphics Center
@@ -17,10 +17,7 @@
*
* @return: Value with bit @__n set.
*/
-#define REG_BIT(__n) \
- ((u32)(BIT(__n) + \
- BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
- ((__n) < 0 || (__n) > 31))))
+#define REG_BIT(__n) BIT_U32(__n)
/**
* REG_BIT8() - Prepare a u8 bit value
@@ -30,10 +27,7 @@
*
* @return: Value with bit @__n set.
*/
-#define REG_BIT8(__n) \
- ((u8)(BIT(__n) + \
- BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
- ((__n) < 0 || (__n) > 7))))
+#define REG_BIT8(__n) BIT_U8(__n)
/**
* REG_GENMASK() - Prepare a continuous u32 bitmask
@@ -44,11 +38,7 @@
*
* @return: Continuous bitmask from @__high to @__low, inclusive.
*/
-#define REG_GENMASK(__high, __low) \
- ((u32)(GENMASK(__high, __low) + \
- BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
- __is_constexpr(__low) && \
- ((__low) < 0 || (__high) > 31 || (__low) > (__high)))))
+#define REG_GENMASK(__high, __low) GENMASK_U32(__high, __low)
/**
* REG_GENMASK64() - Prepare a continuous u64 bitmask
@@ -59,11 +49,7 @@
*
* @return: Continuous bitmask from @__high to @__low, inclusive.
*/
-#define REG_GENMASK64(__high, __low) \
- ((u64)(GENMASK_ULL(__high, __low) + \
- BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
- __is_constexpr(__low) && \
- ((__low) < 0 || (__high) > 63 || (__low) > (__high)))))
+#define REG_GENMASK64(__high, __low) GENMASK_ULL(__high, __low)
/**
* REG_GENMASK8() - Prepare a continuous u8 bitmask
@@ -74,11 +60,7 @@
*
* @return: Continuous bitmask from @__high to @__low, inclusive.
*/
-#define REG_GENMASK8(__high, __low) \
- ((u8)(GENMASK(__high, __low) + \
- BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
- __is_constexpr(__low) && \
- ((__low) < 0 || (__high) > 7 || (__low) > (__high)))))
+#define REG_GENMASK8(__high, __low) GENMASK_U8(__high, __low)
/*
* Local integer constant expression version of is_power_of_2().