diff mbox series

[1/3] drm/i915: Add _PICK_EVEN_RANGES()

Message ID 20221011-pick-even-ranges-v1-1-1aaea52752ed@intel.com (mailing list archive)
State New, archived
Headers show
Series Add _PICK_EVEN_RANGES | expand

Commit Message

Lucas De Marchi Oct. 12, 2022, 4:51 a.m. UTC
It's a constant pattern in the driver to need to use 2 ranges of MMIOs
based on port, phy, pll, etc. When that happens, instead of using
_PICK_EVEN(), _PICK() needs to be used.  Using _PICK() is discouraged
due to some reasons like:

	1) It increases the code size since the array is declared in each
	   call site
	2) Developers need to be careful not to incur an out-of-bounds array
	   access
	3) Developers need to be careful that the indexes match the
	   table. For that it may be that the table needs to contain
	   holes, making (1) even worse.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
(cherry picked from commit 55a65ca6e5d8f7f46fe4cf29c76a9f1b4ddef5ce)
---
 drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Lucas De Marchi Oct. 12, 2022, 5:13 a.m. UTC | #1
On Tue, Oct 11, 2022 at 09:51:08PM -0700, Lucas De Marchi wrote:
>It's a constant pattern in the driver to need to use 2 ranges of MMIOs
>based on port, phy, pll, etc. When that happens, instead of using
>_PICK_EVEN(), _PICK() needs to be used.  Using _PICK() is discouraged
>due to some reasons like:
>
>	1) It increases the code size since the array is declared in each
>	   call site
>	2) Developers need to be careful not to incur an out-of-bounds array
>	   access
>	3) Developers need to be careful that the indexes match the
>	   table. For that it may be that the table needs to contain
>	   holes, making (1) even worse.
>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>(cherry picked from commit 55a65ca6e5d8f7f46fe4cf29c76a9f1b4ddef5ce)
>---
> drivers/gpu/drm/i915/i915_reg.h | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index 3edfbe92c6dd..d157dd693e41 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -126,10 +126,24 @@
> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
>
> /*
>- * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
>+ * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced addres offsets. The
>+ * @__use_first_range argument selects if the first or second range should be
>+ * used. It's usually in the form like ``(pll) < n``, in which ``n`` is the
>+ * number of registers in the first range. Example::
>  *
>- * Always prefer _PICK_EVEN() over this if the numbers are evenly spaced.
>+ * #define _FOO_A			0xf000
>+ * #define _FOO_B			0xf004
>+ * #define _FOO_C			0xf008
>+ * #define _SUPER_FOO_A			0xa000
>+ * #define _SUPER_FOO_B			0xaf00
>+ * #define FOO(x)			_MMIO(_PICK_EVEN_RANGES(x, (x) < 3,	\
>+ *					      _FOO_A, _FOO_B,			\
>+ *					      _SUPER_FOO_A, _SUPER_FOO_B))
>  */
>+#define _PICK_EVEN_RANGES(__index, __use_first_range, __a, __b, __c, __d)	\
>+	 ((__use_first_range) ? _PICK_EVEN(__index, __a, __b) :			\
>+	  _PICK_EVEN(__index, __c, __d))

humn.. I tried to simplify this with a "__use_first_range" but now this
is broken as the index doesn't start on zero for the second range. This
should actually be something like:

#define _PICK_EVEN_RANGES(__index, __c_idx, __a, __b, __c, __d)		\
	 ((__index < __c_idx) ? _PICK_EVEN(__index, __a, __b) :		\
	  _PICK_EVEN((__index) - __c_idx, __c, __d))

Lucas De Marchi

>+
> #define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
>
> /*
>
>-- 
>b4 0.10.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3edfbe92c6dd..d157dd693e41 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -126,10 +126,24 @@ 
 #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
 
 /*
- * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
+ * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced addres offsets. The
+ * @__use_first_range argument selects if the first or second range should be
+ * used. It's usually in the form like ``(pll) < n``, in which ``n`` is the
+ * number of registers in the first range. Example::
  *
- * Always prefer _PICK_EVEN() over this if the numbers are evenly spaced.
+ * #define _FOO_A			0xf000
+ * #define _FOO_B			0xf004
+ * #define _FOO_C			0xf008
+ * #define _SUPER_FOO_A			0xa000
+ * #define _SUPER_FOO_B			0xaf00
+ * #define FOO(x)			_MMIO(_PICK_EVEN_RANGES(x, (x) < 3,	\
+ *					      _FOO_A, _FOO_B,			\
+ *					      _SUPER_FOO_A, _SUPER_FOO_B))
  */
+#define _PICK_EVEN_RANGES(__index, __use_first_range, __a, __b, __c, __d)	\
+	 ((__use_first_range) ? _PICK_EVEN(__index, __a, __b) :			\
+	  _PICK_EVEN(__index, __c, __d))
+
 #define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
 
 /*