diff mbox series

[v3,3/3] drm/i915: Convert REG_GENMASK* to fixed-width GENMASK_*

Message ID 20240208074521.577076-4-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series Fixed-type GENMASK/BIT | expand

Commit Message

Lucas De Marchi Feb. 8, 2024, 7:45 a.m. UTC 
Now that include/linux/bits.h implements fixed-width GENMASK_*, use them
to implement the i915/xe specific macros. Converting each driver to use
the generic macros are left for later, when/if other driver-specific
macros are also generalized.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg_defs.h | 108 +++------------------------
 1 file changed, 11 insertions(+), 97 deletions(-)

Comments

Jani Nikula Feb. 8, 2024, 8:49 a.m. UTC | #1 
On Wed, 07 Feb 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> Now that include/linux/bits.h implements fixed-width GENMASK_*, use them
> to implement the i915/xe specific macros. Converting each driver to use
> the generic macros are left for later, when/if other driver-specific
> macros are also generalized.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

Thanks for doing this!

This is fine to merge via whichever tree you find best. The i915 parts
have very little conflict potential, there haven't been changes here in
a while.

BR,
Jani.
Andi Shyti Feb. 8, 2024, 8:07 p.m. UTC | #2 
Hi Lucas,

...

> +#define REG_GENMASK(__high, __low)	GENMASK_U32(__high, __low)
> +#define REG_GENMASK64(__high, __low)	GENMASK_U64(__high, __low)
> +#define REG_GENMASK16(__high, __low)	GENMASK_U16(__high, __low)
> +#define REG_GENMASK8(__high, __low)	GENMASK_U8(__high, __low)

I was hoping to use directly GENMASK_U*() functions.

Andi
Lucas De Marchi Feb. 8, 2024, 9:21 p.m. UTC | #3 
On Thu, Feb 08, 2024 at 09:07:32PM +0100, Andi Shyti wrote:
>Hi Lucas,
>
>...
>
>> +#define REG_GENMASK(__high, __low)	GENMASK_U32(__high, __low)
>> +#define REG_GENMASK64(__high, __low)	GENMASK_U64(__high, __low)
>> +#define REG_GENMASK16(__high, __low)	GENMASK_U16(__high, __low)
>> +#define REG_GENMASK8(__high, __low)	GENMASK_U8(__high, __low)
>
>I was hoping to use directly GENMASK_U*() functions.

this would be something to be done on top on each drivers' trees, not
something to apply here.

Lucas De Marchi

>
>Andi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
index a685db1e815d..52f99eb96f86 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -9,76 +9,19 @@ 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
 
-/**
- * REG_BIT() - Prepare a u32 bit value
- * @__n: 0-based bit number
- *
- * Local wrapper for BIT() to force u32, with compile time checks.
- *
- * @return: Value with bit @__n set.
- */
-#define REG_BIT(__n)							\
-	((u32)(BIT(__n) +						\
-	       BUILD_BUG_ON_ZERO(__is_constexpr(__n) &&		\
-				 ((__n) < 0 || (__n) > 31))))
-
-/**
- * REG_BIT8() - Prepare a u8 bit value
- * @__n: 0-based bit number
- *
- * Local wrapper for BIT() to force u8, with compile time checks.
- *
- * @return: Value with bit @__n set.
- */
-#define REG_BIT8(__n)                                                   \
-	((u8)(BIT(__n) +                                                \
-	       BUILD_BUG_ON_ZERO(__is_constexpr(__n) &&         \
-				 ((__n) < 0 || (__n) > 7))))
-
-/**
- * REG_GENMASK() - Prepare a continuous u32 bitmask
- * @__high: 0-based high bit
- * @__low: 0-based low bit
- *
- * Local wrapper for GENMASK() to force u32, with compile time checks.
- *
- * @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)))))
-
-/**
- * REG_GENMASK64() - Prepare a continuous u64 bitmask
- * @__high: 0-based high bit
- * @__low: 0-based low bit
- *
- * Local wrapper for GENMASK_ULL() to force u64, with compile time checks.
- *
- * @return: Continuous bitmask from @__high to @__low, inclusive.
+/*
+ * Wrappers over the generic BIT_* and GENMASK_* implementations,
+ * for compatibility reasons with previous implementation
  */
-#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_GENMASK(__high, __low)	GENMASK_U32(__high, __low)
+#define REG_GENMASK64(__high, __low)	GENMASK_U64(__high, __low)
+#define REG_GENMASK16(__high, __low)	GENMASK_U16(__high, __low)
+#define REG_GENMASK8(__high, __low)	GENMASK_U8(__high, __low)
 
-/**
- * REG_GENMASK8() - Prepare a continuous u8 bitmask
- * @__high: 0-based high bit
- * @__low: 0-based low bit
- *
- * Local wrapper for GENMASK() to force u8, with compile time checks.
- *
- * @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_BIT(__n)			BIT_U32(__n)
+#define REG_BIT64(__n)			BIT_U64(__n)
+#define REG_BIT16(__n)			BIT_U16(__n)
+#define REG_BIT8(__n)			BIT_U8(__n)
 
 /*
  * Local integer constant expression version of is_power_of_2().
@@ -143,35 +86,6 @@ 
  */
 #define REG_FIELD_GET64(__mask, __val)	((u64)FIELD_GET(__mask, __val))
 
-/**
- * REG_BIT16() - Prepare a u16 bit value
- * @__n: 0-based bit number
- *
- * Local wrapper for BIT() to force u16, with compile time
- * checks.
- *
- * @return: Value with bit @__n set.
- */
-#define REG_BIT16(__n)                                                   \
-	((u16)(BIT(__n) +                                                \
-	       BUILD_BUG_ON_ZERO(__is_constexpr(__n) &&         \
-				 ((__n) < 0 || (__n) > 15))))
-
-/**
- * REG_GENMASK16() - Prepare a continuous u8 bitmask
- * @__high: 0-based high bit
- * @__low: 0-based low bit
- *
- * Local wrapper for GENMASK() to force u16, with compile time
- * checks.
- *
- * @return: Continuous bitmask from @__high to @__low, inclusive.
- */
-#define REG_GENMASK16(__high, __low)                                     \
-	((u16)(GENMASK(__high, __low) +                                  \
-	       BUILD_BUG_ON_ZERO(__is_constexpr(__high) &&      \
-				 __is_constexpr(__low) &&               \
-				 ((__low) < 0 || (__high) > 15 || (__low) > (__high)))))
 
 /**
  * REG_FIELD_PREP16() - Prepare a u16 bitfield value