* [PATCH v2 0/8] Add _PICK_EVEN_2RANGES
@ 2023-01-20 19:34 Lucas De Marchi
2023-01-20 19:34 ` [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES() Lucas De Marchi
` (8 more replies)
0 siblings, 9 replies; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-20 19:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, dri-devel
Add a new macro, _PICK_EVEN_2RANGES, that supports using 2 address
ranges. This can be considered a v2 of
https://patchwork.freedesktop.org/series/109606/
I think I converted all the _PICK() uses that could be easily done
without making it much harder to read. We do have some cases of 3
ranges: I left those alone.
As commented in the original series and like Jani I think we may need
something else to cover all the use cases in future. Right now I don't
think we have a good alternative though. This new macro both improves
the current code and can be used for cases the ranges change in new
platforms, so I think it's good enough. In future I think just saving
the reg during initialization and using different functions if the
bitfields change may be an alternative.
This was lightly tested on ADL-S and DG2.
Lucas De Marchi (8):
drm/i915: Add _PICK_EVEN_2RANGES()
drm/i915: Fix coding style on DPLL*_ENABLE defines
drm/i915: Convert pll macros to _PICK_EVEN_2RANGES
drm/i915: Replace _MMIO_PHY3() with _PICK_EVEN_2RANGES()
drm/i915: Convert PIPE3/PORT3 to _PICK_EVEN_2RANGES()
drm/i915: Convert _FIA() to _PICK_EVEN_2RANGES()
drm/i915: Convert MBUS_ABOX_CTL() to _PICK_EVEN_2RANGES()
drm/i915: Convert PALETTE() to _PICK_EVEN_2RANGES()
.../drm/i915/display/intel_display_reg_defs.h | 10 +-
.../gpu/drm/i915/display/intel_mg_phy_regs.h | 4 +-
drivers/gpu/drm/i915/i915_reg.h | 106 +++++++++---------
drivers/gpu/drm/i915/i915_reg_defs.h | 28 +++++
4 files changed, 89 insertions(+), 59 deletions(-)
--
2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
@ 2023-01-20 19:34 ` Lucas De Marchi
2023-01-21 6:14 ` [Intel-gfx] " Srivatsa, Anusha
` (2 more replies)
2023-01-20 19:34 ` [PATCH v2 2/8] drm/i915: Fix coding style on DPLL*_ENABLE defines Lucas De Marchi
` (7 subsequent siblings)
8 siblings, 3 replies; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-20 19:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, dri-devel
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.
Add a variant of _PICK_EVEN() that works with 2 ranges and selects which
one to use depending on the index value.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/i915_reg_defs.h | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
index be43580a6979..b7ec87464d69 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -119,6 +119,34 @@
*/
#define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
+/*
+ * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
+ * The first range is used for indexes below @__c_index, and the second
+ * range is used for anything above it. Example::
+ *
+ * #define _FOO_A 0xf000
+ * #define _FOO_B 0xf004
+ * #define _FOO_C 0xf008
+ * #define _SUPER_FOO_A 0xa000
+ * #define _SUPER_FOO_B 0xa100
+ * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3, \
+ * _FOO_A, _FOO_B, \
+ * _SUPER_FOO_A, _SUPER_FOO_B))
+ *
+ * This expands to:
+ * 0: 0xf000,
+ * 1: 0xf004,
+ * 2: 0xf008,
+ * 3: 0xa100,
+ * 4: 0xa200,
+ * 5: 0xa300,
+ * ...
+ */
+#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) \
+ (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) + \
+ ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : \
+ _PICK_EVEN((__index) - (__c_index), __c, __d)))
+
/*
* Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
*
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 2/8] drm/i915: Fix coding style on DPLL*_ENABLE defines
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
2023-01-20 19:34 ` [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES() Lucas De Marchi
@ 2023-01-20 19:34 ` Lucas De Marchi
2023-01-20 20:14 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 3/8] drm/i915: Convert pll macros to _PICK_EVEN_2RANGES Lucas De Marchi
` (6 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-20 19:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, dri-devel
Abide by the rules in the top of the header: 2 spaces for bitfield,
prefix offsets with underscore and prefer the use of REG_BIT().
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3b2642397b82..8da3546d82fb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7224,20 +7224,20 @@ enum skl_power_gate {
ADLS_DPCLKA_DDIK_SEL_MASK)
/* ICL PLL */
-#define DPLL0_ENABLE 0x46010
-#define DPLL1_ENABLE 0x46014
+#define _DPLL0_ENABLE 0x46010
+#define _DPLL1_ENABLE 0x46014
#define _ADLS_DPLL2_ENABLE 0x46018
#define _ADLS_DPLL3_ENABLE 0x46030
-#define PLL_ENABLE (1 << 31)
-#define PLL_LOCK (1 << 30)
-#define PLL_POWER_ENABLE (1 << 27)
-#define PLL_POWER_STATE (1 << 26)
-#define ICL_DPLL_ENABLE(pll) _MMIO_PLL3(pll, DPLL0_ENABLE, DPLL1_ENABLE, \
+#define PLL_ENABLE REG_BIT(31)
+#define PLL_LOCK REG_BIT(30)
+#define PLL_POWER_ENABLE REG_BIT(27)
+#define PLL_POWER_STATE REG_BIT(26)
+#define ICL_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, _DPLL1_ENABLE, \
_ADLS_DPLL2_ENABLE, _ADLS_DPLL3_ENABLE)
#define _DG2_PLL3_ENABLE 0x4601C
-#define DG2_PLL_ENABLE(pll) _MMIO_PLL3(pll, DPLL0_ENABLE, DPLL1_ENABLE, \
+#define DG2_PLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, _DPLL1_ENABLE, \
_ADLS_DPLL2_ENABLE, _DG2_PLL3_ENABLE)
#define TBT_PLL_ENABLE _MMIO(0x46020)
@@ -7246,12 +7246,12 @@ enum skl_power_gate {
#define _MG_PLL2_ENABLE 0x46034
#define _MG_PLL3_ENABLE 0x46038
#define _MG_PLL4_ENABLE 0x4603C
-/* Bits are the same as DPLL0_ENABLE */
+/* Bits are the same as _DPLL0_ENABLE */
#define MG_PLL_ENABLE(tc_port) _MMIO_PORT((tc_port), _MG_PLL1_ENABLE, \
_MG_PLL2_ENABLE)
/* DG1 PLL */
-#define DG1_DPLL_ENABLE(pll) _MMIO_PLL3(pll, DPLL0_ENABLE, DPLL1_ENABLE, \
+#define DG1_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, _DPLL1_ENABLE, \
_MG_PLL1_ENABLE, _MG_PLL2_ENABLE)
/* ADL-P Type C PLL */
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 3/8] drm/i915: Convert pll macros to _PICK_EVEN_2RANGES
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
2023-01-20 19:34 ` [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES() Lucas De Marchi
2023-01-20 19:34 ` [PATCH v2 2/8] drm/i915: Fix coding style on DPLL*_ENABLE defines Lucas De Marchi
@ 2023-01-20 19:34 ` Lucas De Marchi
2023-01-23 19:12 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 4/8] drm/i915: Replace _MMIO_PHY3() with _PICK_EVEN_2RANGES() Lucas De Marchi
` (5 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-20 19:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, dri-devel
Avoid the array lookup, converting the PLL macros after ICL to
_PICK_EVEN_RANGES. This provides the following reduction in code size:
$ size build64/drivers/gpu/drm/i915/i915.o{.old,.new}
text data bss dec hex filename
4027456 185703 6984 4220143 4064ef build64/drivers/gpu/drm/i915/i915.o.old
4026997 185703 6984 4219684 406324 build64/drivers/gpu/drm/i915/i915.o.new
At the same time it's safer, avoiding out-of-bounds array access. This
allows to remove _MMIO_PLL3() that is now unused.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
.../drm/i915/display/intel_display_reg_defs.h | 1 -
drivers/gpu/drm/i915/i915_reg.h | 59 +++++++++----------
2 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
index 02605418ff08..a4ed1c530799 100644
--- a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
@@ -34,7 +34,6 @@
#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
#define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
-#define _MMIO_PLL3(pll, ...) _MMIO(_PICK(pll, __VA_ARGS__))
/*
* Device info offset array based helpers for groups of registers with unevenly
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8da3546d82fb..dd1eb8b10e0e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7232,13 +7232,15 @@ enum skl_power_gate {
#define PLL_LOCK REG_BIT(30)
#define PLL_POWER_ENABLE REG_BIT(27)
#define PLL_POWER_STATE REG_BIT(26)
-#define ICL_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, _DPLL1_ENABLE, \
- _ADLS_DPLL2_ENABLE, _ADLS_DPLL3_ENABLE)
+#define ICL_DPLL_ENABLE(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 3, \
+ _DPLL0_ENABLE, _DPLL1_ENABLE, \
+ _ADLS_DPLL3_ENABLE, _ADLS_DPLL3_ENABLE))
#define _DG2_PLL3_ENABLE 0x4601C
-#define DG2_PLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, _DPLL1_ENABLE, \
- _ADLS_DPLL2_ENABLE, _DG2_PLL3_ENABLE)
+#define DG2_PLL_ENABLE(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 3, \
+ _DPLL0_ENABLE, _DPLL1_ENABLE, \
+ _DG2_PLL3_ENABLE, _DG2_PLL3_ENABLE))
#define TBT_PLL_ENABLE _MMIO(0x46020)
@@ -7251,8 +7253,9 @@ enum skl_power_gate {
_MG_PLL2_ENABLE)
/* DG1 PLL */
-#define DG1_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, _DPLL1_ENABLE, \
- _MG_PLL1_ENABLE, _MG_PLL2_ENABLE)
+#define DG1_DPLL_ENABLE(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2, \
+ _DPLL0_ENABLE, _DPLL1_ENABLE, \
+ _MG_PLL1_ENABLE, _MG_PLL2_ENABLE))
/* ADL-P Type C PLL */
#define PORTTC1_PLL_ENABLE 0x46038
@@ -7312,9 +7315,9 @@ enum skl_power_gate {
#define _TGL_DPLL0_CFGCR0 0x164284
#define _TGL_DPLL1_CFGCR0 0x16428C
#define _TGL_TBTPLL_CFGCR0 0x16429C
-#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
- _TGL_DPLL1_CFGCR0, \
- _TGL_TBTPLL_CFGCR0)
+#define TGL_DPLL_CFGCR0(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2, \
+ _TGL_DPLL0_CFGCR0, _TGL_DPLL1_CFGCR0, \
+ _TGL_TBTPLL_CFGCR0, _TGL_TBTPLL_CFGCR0))
#define RKL_DPLL_CFGCR0(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR0, \
_TGL_DPLL1_CFGCR0)
@@ -7327,40 +7330,36 @@ enum skl_power_gate {
#define _TGL_DPLL0_CFGCR1 0x164288
#define _TGL_DPLL1_CFGCR1 0x164290
#define _TGL_TBTPLL_CFGCR1 0x1642A0
-#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
- _TGL_DPLL1_CFGCR1, \
- _TGL_TBTPLL_CFGCR1)
+#define TGL_DPLL_CFGCR1(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2, \
+ _TGL_DPLL0_CFGCR1, _TGL_DPLL1_CFGCR1, \
+ _TGL_TBTPLL_CFGCR1, _TGL_TBTPLL_CFGCR1))
#define RKL_DPLL_CFGCR1(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR1, \
_TGL_DPLL1_CFGCR1)
#define _DG1_DPLL2_CFGCR0 0x16C284
#define _DG1_DPLL3_CFGCR0 0x16C28C
-#define DG1_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
- _TGL_DPLL1_CFGCR0, \
- _DG1_DPLL2_CFGCR0, \
- _DG1_DPLL3_CFGCR0)
+#define DG1_DPLL_CFGCR0(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2, \
+ _TGL_DPLL0_CFGCR0, _TGL_DPLL1_CFGCR0, \
+ _DG1_DPLL2_CFGCR0, _DG1_DPLL3_CFGCR0))
#define _DG1_DPLL2_CFGCR1 0x16C288
#define _DG1_DPLL3_CFGCR1 0x16C290
-#define DG1_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
- _TGL_DPLL1_CFGCR1, \
- _DG1_DPLL2_CFGCR1, \
- _DG1_DPLL3_CFGCR1)
+#define DG1_DPLL_CFGCR1(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2, \
+ _TGL_DPLL0_CFGCR1, _TGL_DPLL1_CFGCR1, \
+ _DG1_DPLL2_CFGCR1, _DG1_DPLL3_CFGCR1))
/* For ADL-S DPLL4_CFGCR0/1 are used to control DPLL2 */
-#define _ADLS_DPLL3_CFGCR0 0x1642C0
#define _ADLS_DPLL4_CFGCR0 0x164294
-#define ADLS_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR0, \
- _TGL_DPLL1_CFGCR0, \
- _ADLS_DPLL4_CFGCR0, \
- _ADLS_DPLL3_CFGCR0)
+#define _ADLS_DPLL3_CFGCR0 0x1642C0
+#define ADLS_DPLL_CFGCR0(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2, \
+ _TGL_DPLL0_CFGCR0, _TGL_DPLL1_CFGCR0, \
+ _ADLS_DPLL4_CFGCR0, _ADLS_DPLL3_CFGCR0))
-#define _ADLS_DPLL3_CFGCR1 0x1642C4
#define _ADLS_DPLL4_CFGCR1 0x164298
-#define ADLS_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
- _TGL_DPLL1_CFGCR1, \
- _ADLS_DPLL4_CFGCR1, \
- _ADLS_DPLL3_CFGCR1)
+#define _ADLS_DPLL3_CFGCR1 0x1642C4
+#define ADLS_DPLL_CFGCR1(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2, \
+ _TGL_DPLL0_CFGCR1, _TGL_DPLL1_CFGCR1, \
+ _ADLS_DPLL4_CFGCR1, _ADLS_DPLL3_CFGCR1))
/* BXT display engine PLL */
#define BXT_DE_PLL_CTL _MMIO(0x6d000)
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 4/8] drm/i915: Replace _MMIO_PHY3() with _PICK_EVEN_2RANGES()
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
` (2 preceding siblings ...)
2023-01-20 19:34 ` [PATCH v2 3/8] drm/i915: Convert pll macros to _PICK_EVEN_2RANGES Lucas De Marchi
@ 2023-01-20 19:34 ` Lucas De Marchi
2023-01-21 5:58 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 5/8] drm/i915: Convert PIPE3/PORT3 to _PICK_EVEN_2RANGES() Lucas De Marchi
` (4 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-20 19:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, dri-devel
As done previously for pll, also convert users of _PHY3() to
_PICK_EVEN_2RANGES(). Size comparison of i915.o:
$ size build64/drivers/gpu/drm/i915/i915.o{.old,.new}
text data bss dec hex filename
4026997 185703 6984 4219684 406324 build64/drivers/gpu/drm/i915/i915.o.old
4026288 185703 6984 4218975 40605f build64/drivers/gpu/drm/i915/i915.o.new
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
.../drm/i915/display/intel_display_reg_defs.h | 3 ---
drivers/gpu/drm/i915/i915_reg.h | 16 +++++++++-------
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
index a4ed1c530799..f1681e1396b5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
@@ -29,11 +29,8 @@
#define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
#define _MMIO_PHY(phy, a, b) _MMIO(_PHY(phy, a, b))
-#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
-
#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
-#define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
/*
* Device info offset array based helpers for groups of registers with unevenly
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dd1eb8b10e0e..fe6385443c4a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -541,9 +541,10 @@
#define _BXT_PHY0_BASE 0x6C000
#define _BXT_PHY1_BASE 0x162000
#define _BXT_PHY2_BASE 0x163000
-#define BXT_PHY_BASE(phy) _PHY3((phy), _BXT_PHY0_BASE, \
- _BXT_PHY1_BASE, \
- _BXT_PHY2_BASE)
+#define BXT_PHY_BASE(phy) \
+ _PICK_EVEN_2RANGES(phy, 1, \
+ _BXT_PHY0_BASE, _BXT_PHY0_BASE, \
+ _BXT_PHY1_BASE, _BXT_PHY2_BASE)
#define _BXT_PHY(phy, reg) \
_MMIO(BXT_PHY_BASE(phy) - _BXT_PHY0_BASE + (reg))
@@ -566,13 +567,14 @@
#define BXT_PHY_CTL(port) _MMIO_PORT(port, _BXT_PHY_CTL_DDI_A, \
_BXT_PHY_CTL_DDI_B)
-#define _PHY_CTL_FAMILY_EDP 0x64C80
#define _PHY_CTL_FAMILY_DDI 0x64C90
+#define _PHY_CTL_FAMILY_EDP 0x64C80
#define _PHY_CTL_FAMILY_DDI_C 0x64CA0
#define COMMON_RESET_DIS (1 << 31)
-#define BXT_PHY_CTL_FAMILY(phy) _MMIO_PHY3((phy), _PHY_CTL_FAMILY_DDI, \
- _PHY_CTL_FAMILY_EDP, \
- _PHY_CTL_FAMILY_DDI_C)
+#define BXT_PHY_CTL_FAMILY(phy) \
+ _MMIO(_PICK_EVEN_2RANGES(phy, 1, \
+ _PHY_CTL_FAMILY_DDI, _PHY_CTL_FAMILY_DDI, \
+ _PHY_CTL_FAMILY_EDP, _PHY_CTL_FAMILY_DDI_C))
/* BXT PHY PLL registers */
#define _PORT_PLL_A 0x46074
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 5/8] drm/i915: Convert PIPE3/PORT3 to _PICK_EVEN_2RANGES()
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
` (3 preceding siblings ...)
2023-01-20 19:34 ` [PATCH v2 4/8] drm/i915: Replace _MMIO_PHY3() with _PICK_EVEN_2RANGES() Lucas De Marchi
@ 2023-01-20 19:34 ` Lucas De Marchi
2023-01-21 6:00 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 6/8] drm/i915: Convert _FIA() " Lucas De Marchi
` (3 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-20 19:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, dri-devel
Like done for when __var_args__ were used, but size-wise it's also
benefitial to avoid _PICK() used for 3 ports/pipes:
$ size build64/drivers/gpu/drm/i915/i915.o{.old,.new}
text data bss dec hex filename
4026288 185703 6984 4218975 40605f build64/drivers/gpu/drm/i915/i915.o.old
4025496 185703 6984 4218183 405d47 build64/drivers/gpu/drm/i915/i915.o.new
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/display/intel_display_reg_defs.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
index f1681e1396b5..755c1ea8225c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
@@ -13,7 +13,7 @@
#define VLV_DISPLAY_BASE 0x180000
/*
- * Named helper wrappers around _PICK_EVEN() and _PICK().
+ * Named helper wrappers around _PICK_EVEN() and _PICK_EVEN_2RANGES().
*/
#define _PIPE(pipe, a, b) _PICK_EVEN(pipe, a, b)
#define _PLANE(plane, a, b) _PICK_EVEN(plane, a, b)
@@ -29,8 +29,8 @@
#define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
#define _MMIO_PHY(phy, a, b) _MMIO(_PHY(phy, a, b))
-#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
-#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
+#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK_EVEN_2RANGES(pipe, 1, a, a, b, c))
+#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK_EVEN_2RANGES(pipe, 1, a, a, b, c))
/*
* Device info offset array based helpers for groups of registers with unevenly
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 6/8] drm/i915: Convert _FIA() to _PICK_EVEN_2RANGES()
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
` (4 preceding siblings ...)
2023-01-20 19:34 ` [PATCH v2 5/8] drm/i915: Convert PIPE3/PORT3 to _PICK_EVEN_2RANGES() Lucas De Marchi
@ 2023-01-20 19:34 ` Lucas De Marchi
2023-01-21 6:01 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 7/8] drm/i915: Convert MBUS_ABOX_CTL() " Lucas De Marchi
` (2 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-20 19:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, dri-devel
_FIA() can use _PICK_EVEN_2RANGES instead of _PICK, which reduces the
size and is safer.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/display/intel_mg_phy_regs.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h b/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h
index 0e8248bce52d..0306ade2bc30 100644
--- a/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h
@@ -142,7 +142,9 @@
#define FIA1_BASE 0x163000
#define FIA2_BASE 0x16E000
#define FIA3_BASE 0x16F000
-#define _FIA(fia) _PICK((fia), FIA1_BASE, FIA2_BASE, FIA3_BASE)
+#define _FIA(fia) _PICK_EVEN_2RANGES((fia), 1, \
+ FIA1_BASE, FIA1_BASE,\
+ FIA2_BASE, FIA3_BASE)
#define _MMIO_FIA(fia, off) _MMIO(_FIA(fia) + (off))
/* ICL PHY DFLEX registers */
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 7/8] drm/i915: Convert MBUS_ABOX_CTL() to _PICK_EVEN_2RANGES()
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
` (5 preceding siblings ...)
2023-01-20 19:34 ` [PATCH v2 6/8] drm/i915: Convert _FIA() " Lucas De Marchi
@ 2023-01-20 19:34 ` Lucas De Marchi
2023-01-21 6:04 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 8/8] drm/i915: Convert PALETTE() " Lucas De Marchi
2023-01-23 10:39 ` [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Jani Nikula
8 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-20 19:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, dri-devel
MBUS_ABOX_CTL() can use _PICK_EVEN_2RANGES instead of _PICK, which
reduces the size and is safer.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fe6385443c4a..3d6ad4424265 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1040,9 +1040,11 @@
#define _MBUS_ABOX0_CTL 0x45038
#define _MBUS_ABOX1_CTL 0x45048
#define _MBUS_ABOX2_CTL 0x4504C
-#define MBUS_ABOX_CTL(x) _MMIO(_PICK(x, _MBUS_ABOX0_CTL, \
- _MBUS_ABOX1_CTL, \
- _MBUS_ABOX2_CTL))
+#define MBUS_ABOX_CTL(x) \
+ _MMIO(_PICK_EVEN_2RANGES(x, 2, \
+ _MBUS_ABOX0_CTL, _MBUS_ABOX1_CTL, \
+ _MBUS_ABOX2_CTL, _MBUS_ABOX2_CTL))
+
#define MBUS_ABOX_BW_CREDIT_MASK (3 << 20)
#define MBUS_ABOX_BW_CREDIT(x) ((x) << 20)
#define MBUS_ABOX_B_CREDIT_MASK (0xF << 16)
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 8/8] drm/i915: Convert PALETTE() to _PICK_EVEN_2RANGES()
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
` (6 preceding siblings ...)
2023-01-20 19:34 ` [PATCH v2 7/8] drm/i915: Convert MBUS_ABOX_CTL() " Lucas De Marchi
@ 2023-01-20 19:34 ` Lucas De Marchi
2023-01-21 6:06 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-23 10:39 ` [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Jani Nikula
8 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-20 19:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, dri-devel
PALETTE() can use _PICK_EVEN_2RANGES instead of _PICK, which
reduces the size and is safer.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3d6ad4424265..b134a1f357c8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1734,10 +1734,11 @@
#define PALETTE_10BIT_BLUE_EXP_MASK REG_GENMASK(7, 6)
#define PALETTE_10BIT_BLUE_MANT_MASK REG_GENMASK(5, 2)
#define PALETTE_10BIT_BLUE_UDW_MASK REG_GENMASK(1, 0)
-#define PALETTE(pipe, i) _MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
- _PICK((pipe), _PALETTE_A, \
- _PALETTE_B, _CHV_PALETTE_C) + \
- (i) * 4)
+#define PALETTE(pipe, i) _MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
+ _PICK_EVEN_2RANGES(pipe, 2, \
+ _PALETTE_A, _PALETTE_B, \
+ _CHV_PALETTE_C, _CHV_PALETTE_C) + \
+ (i) * 4)
#define PEG_BAND_GAP_DATA _MMIO(0x14d68)
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* RE: [Intel-gfx] [PATCH v2 2/8] drm/i915: Fix coding style on DPLL*_ENABLE defines
2023-01-20 19:34 ` [PATCH v2 2/8] drm/i915: Fix coding style on DPLL*_ENABLE defines Lucas De Marchi
@ 2023-01-20 20:14 ` Srivatsa, Anusha
0 siblings, 0 replies; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-20 20:14 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx; +Cc: De Marchi, Lucas, dri-devel
Changes look good.
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
> De Marchi
> Sent: Friday, January 20, 2023 11:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
> devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 2/8] drm/i915: Fix coding style on DPLL*_ENABLE
> defines
>
> Abide by the rules in the top of the header: 2 spaces for bitfield, prefix offsets
> with underscore and prefer the use of REG_BIT().
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3b2642397b82..8da3546d82fb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7224,20 +7224,20 @@ enum skl_power_gate {
>
> ADLS_DPCLKA_DDIK_SEL_MASK)
>
> /* ICL PLL */
> -#define DPLL0_ENABLE 0x46010
> -#define DPLL1_ENABLE 0x46014
> +#define _DPLL0_ENABLE 0x46010
> +#define _DPLL1_ENABLE 0x46014
> #define _ADLS_DPLL2_ENABLE 0x46018
> #define _ADLS_DPLL3_ENABLE 0x46030
> -#define PLL_ENABLE (1 << 31)
> -#define PLL_LOCK (1 << 30)
> -#define PLL_POWER_ENABLE (1 << 27)
> -#define PLL_POWER_STATE (1 << 26)
> -#define ICL_DPLL_ENABLE(pll) _MMIO_PLL3(pll, DPLL0_ENABLE,
> DPLL1_ENABLE, \
> +#define PLL_ENABLE REG_BIT(31)
> +#define PLL_LOCK REG_BIT(30)
> +#define PLL_POWER_ENABLE REG_BIT(27)
> +#define PLL_POWER_STATE REG_BIT(26)
> +#define ICL_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE,
> _DPLL1_ENABLE, \
> _ADLS_DPLL2_ENABLE,
> _ADLS_DPLL3_ENABLE)
>
> #define _DG2_PLL3_ENABLE 0x4601C
>
> -#define DG2_PLL_ENABLE(pll) _MMIO_PLL3(pll, DPLL0_ENABLE,
> DPLL1_ENABLE, \
> +#define DG2_PLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE,
> +_DPLL1_ENABLE, \
> _ADLS_DPLL2_ENABLE,
> _DG2_PLL3_ENABLE)
>
> #define TBT_PLL_ENABLE _MMIO(0x46020)
> @@ -7246,12 +7246,12 @@ enum skl_power_gate {
> #define _MG_PLL2_ENABLE 0x46034
> #define _MG_PLL3_ENABLE 0x46038
> #define _MG_PLL4_ENABLE 0x4603C
> -/* Bits are the same as DPLL0_ENABLE */
> +/* Bits are the same as _DPLL0_ENABLE */
> #define MG_PLL_ENABLE(tc_port) _MMIO_PORT((tc_port),
> _MG_PLL1_ENABLE, \
> _MG_PLL2_ENABLE)
>
> /* DG1 PLL */
> -#define DG1_DPLL_ENABLE(pll) _MMIO_PLL3(pll, DPLL0_ENABLE,
> DPLL1_ENABLE, \
> +#define DG1_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE,
> _DPLL1_ENABLE, \
> _MG_PLL1_ENABLE,
> _MG_PLL2_ENABLE)
>
> /* ADL-P Type C PLL */
> --
> 2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [Intel-gfx] [PATCH v2 4/8] drm/i915: Replace _MMIO_PHY3() with _PICK_EVEN_2RANGES()
2023-01-20 19:34 ` [PATCH v2 4/8] drm/i915: Replace _MMIO_PHY3() with _PICK_EVEN_2RANGES() Lucas De Marchi
@ 2023-01-21 5:58 ` Srivatsa, Anusha
0 siblings, 0 replies; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-21 5:58 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx; +Cc: De Marchi, Lucas, dri-devel
Verified that the new macro evaluates to the right register offsets.
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
> De Marchi
> Sent: Friday, January 20, 2023 11:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
> devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 4/8] drm/i915: Replace _MMIO_PHY3() with
> _PICK_EVEN_2RANGES()
>
> As done previously for pll, also convert users of _PHY3() to
> _PICK_EVEN_2RANGES(). Size comparison of i915.o:
>
> $ size build64/drivers/gpu/drm/i915/i915.o{.old,.new}
> text data bss dec hex filename
> 4026997 185703 6984 4219684 406324
> build64/drivers/gpu/drm/i915/i915.o.old
> 4026288 185703 6984 4218975 40605f
> build64/drivers/gpu/drm/i915/i915.o.new
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> .../drm/i915/display/intel_display_reg_defs.h | 3 ---
> drivers/gpu/drm/i915/i915_reg.h | 16 +++++++++-------
> 2 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> index a4ed1c530799..f1681e1396b5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> @@ -29,11 +29,8 @@
> #define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
> #define _MMIO_PHY(phy, a, b) _MMIO(_PHY(phy, a, b))
>
> -#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
> -
> #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> -#define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
>
> /*
> * Device info offset array based helpers for groups of registers with unevenly
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index dd1eb8b10e0e..fe6385443c4a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -541,9 +541,10 @@
> #define _BXT_PHY0_BASE 0x6C000
> #define _BXT_PHY1_BASE 0x162000
> #define _BXT_PHY2_BASE 0x163000
> -#define BXT_PHY_BASE(phy) _PHY3((phy), _BXT_PHY0_BASE, \
> - _BXT_PHY1_BASE, \
> - _BXT_PHY2_BASE)
> +#define BXT_PHY_BASE(phy)
> \
> + _PICK_EVEN_2RANGES(phy, 1,
> \
> + _BXT_PHY0_BASE, _BXT_PHY0_BASE,
> \
> + _BXT_PHY1_BASE, _BXT_PHY2_BASE)
>
> #define _BXT_PHY(phy, reg) \
> _MMIO(BXT_PHY_BASE(phy) - _BXT_PHY0_BASE + (reg)) @@ -566,13
> +567,14 @@
> #define BXT_PHY_CTL(port) _MMIO_PORT(port,
> _BXT_PHY_CTL_DDI_A, \
>
> _BXT_PHY_CTL_DDI_B)
>
> -#define _PHY_CTL_FAMILY_EDP 0x64C80
> #define _PHY_CTL_FAMILY_DDI 0x64C90
> +#define _PHY_CTL_FAMILY_EDP 0x64C80
> #define _PHY_CTL_FAMILY_DDI_C 0x64CA0
> #define COMMON_RESET_DIS (1 << 31)
> -#define BXT_PHY_CTL_FAMILY(phy) _MMIO_PHY3((phy),
> _PHY_CTL_FAMILY_DDI, \
> -
> _PHY_CTL_FAMILY_EDP, \
> -
> _PHY_CTL_FAMILY_DDI_C)
> +#define BXT_PHY_CTL_FAMILY(phy)
> \
> + _MMIO(_PICK_EVEN_2RANGES(phy, 1,
> \
> + _PHY_CTL_FAMILY_DDI,
> _PHY_CTL_FAMILY_DDI, \
> + _PHY_CTL_FAMILY_EDP,
> _PHY_CTL_FAMILY_DDI_C))
>
> /* BXT PHY PLL registers */
> #define _PORT_PLL_A 0x46074
> --
> 2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [Intel-gfx] [PATCH v2 5/8] drm/i915: Convert PIPE3/PORT3 to _PICK_EVEN_2RANGES()
2023-01-20 19:34 ` [PATCH v2 5/8] drm/i915: Convert PIPE3/PORT3 to _PICK_EVEN_2RANGES() Lucas De Marchi
@ 2023-01-21 6:00 ` Srivatsa, Anusha
0 siblings, 0 replies; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-21 6:00 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx; +Cc: De Marchi, Lucas, dri-devel
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
> De Marchi
> Sent: Friday, January 20, 2023 11:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
> devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 5/8] drm/i915: Convert PIPE3/PORT3 to
> _PICK_EVEN_2RANGES()
>
> Like done for when __var_args__ were used, but size-wise it's also benefitial to
> avoid _PICK() used for 3 ports/pipes:
>
> $ size build64/drivers/gpu/drm/i915/i915.o{.old,.new}
> text data bss dec hex filename
> 4026288 185703 6984 4218975 40605f
> build64/drivers/gpu/drm/i915/i915.o.old
> 4025496 185703 6984 4218183 405d47
> build64/drivers/gpu/drm/i915/i915.o.new
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display_reg_defs.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> index f1681e1396b5..755c1ea8225c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> @@ -13,7 +13,7 @@
> #define VLV_DISPLAY_BASE 0x180000
>
> /*
> - * Named helper wrappers around _PICK_EVEN() and _PICK().
> + * Named helper wrappers around _PICK_EVEN() and _PICK_EVEN_2RANGES().
> */
> #define _PIPE(pipe, a, b) _PICK_EVEN(pipe, a, b)
> #define _PLANE(plane, a, b) _PICK_EVEN(plane, a, b)
> @@ -29,8 +29,8 @@
> #define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
> #define _MMIO_PHY(phy, a, b) _MMIO(_PHY(phy, a, b))
>
> -#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> -#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> +#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK_EVEN_2RANGES(pipe,
> 1, a, a, b, c))
> +#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK_EVEN_2RANGES(pipe,
> 1, a, a, b, c))
>
> /*
> * Device info offset array based helpers for groups of registers with unevenly
> --
> 2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [Intel-gfx] [PATCH v2 6/8] drm/i915: Convert _FIA() to _PICK_EVEN_2RANGES()
2023-01-20 19:34 ` [PATCH v2 6/8] drm/i915: Convert _FIA() " Lucas De Marchi
@ 2023-01-21 6:01 ` Srivatsa, Anusha
0 siblings, 0 replies; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-21 6:01 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx; +Cc: De Marchi, Lucas, dri-devel
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
> De Marchi
> Sent: Friday, January 20, 2023 11:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
> devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 6/8] drm/i915: Convert _FIA() to
> _PICK_EVEN_2RANGES()
>
> _FIA() can use _PICK_EVEN_2RANGES instead of _PICK, which reduces the size
> and is safer.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_mg_phy_regs.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h
> b/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h
> index 0e8248bce52d..0306ade2bc30 100644
> --- a/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h
> @@ -142,7 +142,9 @@
> #define FIA1_BASE 0x163000
> #define FIA2_BASE 0x16E000
> #define FIA3_BASE 0x16F000
> -#define _FIA(fia) _PICK((fia), FIA1_BASE, FIA2_BASE,
> FIA3_BASE)
> +#define _FIA(fia) _PICK_EVEN_2RANGES((fia), 1,
> \
> + FIA1_BASE,
> FIA1_BASE,\
> + FIA2_BASE,
> FIA3_BASE)
> #define _MMIO_FIA(fia, off) _MMIO(_FIA(fia) + (off))
>
> /* ICL PHY DFLEX registers */
> --
> 2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [Intel-gfx] [PATCH v2 7/8] drm/i915: Convert MBUS_ABOX_CTL() to _PICK_EVEN_2RANGES()
2023-01-20 19:34 ` [PATCH v2 7/8] drm/i915: Convert MBUS_ABOX_CTL() " Lucas De Marchi
@ 2023-01-21 6:04 ` Srivatsa, Anusha
0 siblings, 0 replies; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-21 6:04 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx; +Cc: De Marchi, Lucas, dri-devel
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
> De Marchi
> Sent: Friday, January 20, 2023 11:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
> devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 7/8] drm/i915: Convert MBUS_ABOX_CTL() to
> _PICK_EVEN_2RANGES()
>
> MBUS_ABOX_CTL() can use _PICK_EVEN_2RANGES instead of _PICK, which
> reduces the size and is safer.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Looks good!
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fe6385443c4a..3d6ad4424265 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1040,9 +1040,11 @@
> #define _MBUS_ABOX0_CTL 0x45038
> #define _MBUS_ABOX1_CTL 0x45048
> #define _MBUS_ABOX2_CTL 0x4504C
> -#define MBUS_ABOX_CTL(x) _MMIO(_PICK(x, _MBUS_ABOX0_CTL,
> \
> - _MBUS_ABOX1_CTL, \
> - _MBUS_ABOX2_CTL))
> +#define MBUS_ABOX_CTL(x)
> \
> + _MMIO(_PICK_EVEN_2RANGES(x, 2,
> \
> + _MBUS_ABOX0_CTL, _MBUS_ABOX1_CTL,
> \
> + _MBUS_ABOX2_CTL, _MBUS_ABOX2_CTL))
> +
> #define MBUS_ABOX_BW_CREDIT_MASK (3 << 20)
> #define MBUS_ABOX_BW_CREDIT(x) ((x) << 20)
> #define MBUS_ABOX_B_CREDIT_MASK (0xF << 16)
> --
> 2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [Intel-gfx] [PATCH v2 8/8] drm/i915: Convert PALETTE() to _PICK_EVEN_2RANGES()
2023-01-20 19:34 ` [PATCH v2 8/8] drm/i915: Convert PALETTE() " Lucas De Marchi
@ 2023-01-21 6:06 ` Srivatsa, Anusha
0 siblings, 0 replies; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-21 6:06 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx; +Cc: De Marchi, Lucas, dri-devel
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
> De Marchi
> Sent: Friday, January 20, 2023 11:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
> devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 8/8] drm/i915: Convert PALETTE() to
> _PICK_EVEN_2RANGES()
>
> PALETTE() can use _PICK_EVEN_2RANGES instead of _PICK, which reduces the
> size and is safer.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Anusha Srivatsa<anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3d6ad4424265..b134a1f357c8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1734,10 +1734,11 @@
> #define PALETTE_10BIT_BLUE_EXP_MASK REG_GENMASK(7, 6)
> #define PALETTE_10BIT_BLUE_MANT_MASK REG_GENMASK(5, 2)
> #define PALETTE_10BIT_BLUE_UDW_MASK REG_GENMASK(1, 0)
> -#define PALETTE(pipe, i) _MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
> - _PICK((pipe), _PALETTE_A, \
> - _PALETTE_B, _CHV_PALETTE_C) + \
> - (i) * 4)
> +#define PALETTE(pipe, i) _MMIO(DISPLAY_MMIO_BASE(dev_priv) +
> \
> + _PICK_EVEN_2RANGES(pipe, 2,
> \
> + _PALETTE_A, _PALETTE_B,
> \
> + _CHV_PALETTE_C,
> _CHV_PALETTE_C) + \
> + (i) * 4)
>
> #define PEG_BAND_GAP_DATA _MMIO(0x14d68)
>
> --
> 2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-20 19:34 ` [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES() Lucas De Marchi
@ 2023-01-21 6:14 ` Srivatsa, Anusha
2023-01-22 1:28 ` Lucas De Marchi
2023-01-23 10:38 ` Jani Nikula
2023-01-23 17:15 ` [PATCH v2.1] " Lucas De Marchi
2 siblings, 1 reply; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-21 6:14 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx; +Cc: De Marchi, Lucas, dri-devel
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
> De Marchi
> Sent: Friday, January 20, 2023 11:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
> devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
>
> 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.
>
> Add a variant of _PICK_EVEN() that works with 2 ranges and selects which one
> to use depending on the index value.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg_defs.h | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
> b/drivers/gpu/drm/i915/i915_reg_defs.h
> index be43580a6979..b7ec87464d69 100644
> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> @@ -119,6 +119,34 @@
> */
> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
>
> +/*
> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
> + * The first range is used for indexes below @__c_index, and the second
> + * range is used for anything above it. Example::
> + *
> + * #define _FOO_A 0xf000
> + * #define _FOO_B 0xf004
> + * #define _FOO_C 0xf008
> + * #define _SUPER_FOO_A 0xa000
> + * #define _SUPER_FOO_B 0xa100
> + * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3,
> \
> + * _FOO_A, _FOO_B,
> \
> + * _SUPER_FOO_A, _SUPER_FOO_B))
> + *
> + * This expands to:
> + * 0: 0xf000,
> + * 1: 0xf004,
> + * 2: 0xf008,
> + * 3: 0xa100,
You mean 3:0xa000
> + * 4: 0xa200,
4:0xa100
> + * 5: 0xa300,
5:0xa200
Anusha
> + * ...
> + */
> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)
> \
> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
> \
> + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :
> \
> + _PICK_EVEN((__index) - (__c_index), __c,
> __d)))
> +
> /*
> * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
> *
> --
> 2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-21 6:14 ` [Intel-gfx] " Srivatsa, Anusha
@ 2023-01-22 1:28 ` Lucas De Marchi
2023-01-23 11:00 ` Jani Nikula
0 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-22 1:28 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx, dri-devel
On Fri, Jan 20, 2023 at 10:14:19PM -0800, Anusha Srivatsa wrote:
>
>
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
>> De Marchi
>> Sent: Friday, January 20, 2023 11:35 AM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
>> devel@lists.freedesktop.org
>> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
>>
>> 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.
>>
>> Add a variant of _PICK_EVEN() that works with 2 ranges and selects which one
>> to use depending on the index value.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg_defs.h | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
>> b/drivers/gpu/drm/i915/i915_reg_defs.h
>> index be43580a6979..b7ec87464d69 100644
>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
>> @@ -119,6 +119,34 @@
>> */
>> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
>>
>> +/*
>> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
>> + * The first range is used for indexes below @__c_index, and the second
>> + * range is used for anything above it. Example::
>> + *
>> + * #define _FOO_A 0xf000
>> + * #define _FOO_B 0xf004
>> + * #define _FOO_C 0xf008
>> + * #define _SUPER_FOO_A 0xa000
>> + * #define _SUPER_FOO_B 0xa100
>> + * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3,
>> \
>> + * _FOO_A, _FOO_B,
>> \
>> + * _SUPER_FOO_A, _SUPER_FOO_B))
>> + *
>> + * This expands to:
>> + * 0: 0xf000,
>> + * 1: 0xf004,
>> + * 2: 0xf008,
>> + * 3: 0xa100,
>You mean 3:0xa000
doesn't really matter. This is an example of register addresses. They
don't need to start from 0, it's whatever the hw gives us.
Lucas De Marchi
>
>> + * 4: 0xa200,
>4:0xa100
>
>> + * 5: 0xa300,
>5:0xa200
>
>Anusha
>> + * ...
>> + */
>> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)
>> \
>> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
>> \
>> + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :
>> \
>> + _PICK_EVEN((__index) - (__c_index), __c,
>> __d)))
>> +
>> /*
>> * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
>> *
>> --
>> 2.39.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-20 19:34 ` [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES() Lucas De Marchi
2023-01-21 6:14 ` [Intel-gfx] " Srivatsa, Anusha
@ 2023-01-23 10:38 ` Jani Nikula
2023-01-24 7:45 ` Lucas De Marchi
2023-01-23 17:15 ` [PATCH v2.1] " Lucas De Marchi
2 siblings, 1 reply; 27+ messages in thread
From: Jani Nikula @ 2023-01-23 10:38 UTC (permalink / raw)
To: Lucas De Marchi, intel-gfx; +Cc: Lucas De Marchi, dri-devel
On Fri, 20 Jan 2023, Lucas De Marchi <lucas.demarchi@intel.com> 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
Would be interesting to see what this does, and whether the compiler has
the smarts to combine these within each file:
-#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
+#define _PICK(__index, ...) (((static const u32 []){ __VA_ARGS__ })[__index])
> 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.
>
> Add a variant of _PICK_EVEN() that works with 2 ranges and selects which
> one to use depending on the index value.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg_defs.h | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
> index be43580a6979..b7ec87464d69 100644
> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> @@ -119,6 +119,34 @@
> */
> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
>
> +/*
> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
> + * The first range is used for indexes below @__c_index, and the second
> + * range is used for anything above it. Example::
I'd like this to be clear about which range is used for index ==
__c_index, instead of saying "below" and "above".
No need for the double colon :: because this isn't a kernel-doc comment.
> + *
> + * #define _FOO_A 0xf000
> + * #define _FOO_B 0xf004
> + * #define _FOO_C 0xf008
> + * #define _SUPER_FOO_A 0xa000
> + * #define _SUPER_FOO_B 0xa100
> + * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3, \
The example uses a different name for the macro.
> + * _FOO_A, _FOO_B, \
> + * _SUPER_FOO_A, _SUPER_FOO_B))
> + *
> + * This expands to:
> + * 0: 0xf000,
> + * 1: 0xf004,
> + * 2: 0xf008,
> + * 3: 0xa100,
With the above definitions, this would be 3: 0xa000.
> + * 4: 0xa200,
> + * 5: 0xa300,
> + * ...
> + */
> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) \
> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) + \
> + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : \
> + _PICK_EVEN((__index) - (__c_index), __c, __d)))
> +
> /*
> * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
> *
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 0/8] Add _PICK_EVEN_2RANGES
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
` (7 preceding siblings ...)
2023-01-20 19:34 ` [PATCH v2 8/8] drm/i915: Convert PALETTE() " Lucas De Marchi
@ 2023-01-23 10:39 ` Jani Nikula
8 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2023-01-23 10:39 UTC (permalink / raw)
To: Lucas De Marchi, intel-gfx; +Cc: Lucas De Marchi, dri-devel
On Fri, 20 Jan 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> Add a new macro, _PICK_EVEN_2RANGES, that supports using 2 address
> ranges. This can be considered a v2 of
> https://patchwork.freedesktop.org/series/109606/
>
> I think I converted all the _PICK() uses that could be easily done
> without making it much harder to read. We do have some cases of 3
> ranges: I left those alone.
>
> As commented in the original series and like Jani I think we may need
> something else to cover all the use cases in future. Right now I don't
> think we have a good alternative though. This new macro both improves
> the current code and can be used for cases the ranges change in new
> platforms, so I think it's good enough. In future I think just saving
> the reg during initialization and using different functions if the
> bitfields change may be an alternative.
Did not review, but on the approach,
Acked-by: Jani Nikula <jani.nikula@intel.com>
>
> This was lightly tested on ADL-S and DG2.
>
> Lucas De Marchi (8):
> drm/i915: Add _PICK_EVEN_2RANGES()
> drm/i915: Fix coding style on DPLL*_ENABLE defines
> drm/i915: Convert pll macros to _PICK_EVEN_2RANGES
> drm/i915: Replace _MMIO_PHY3() with _PICK_EVEN_2RANGES()
> drm/i915: Convert PIPE3/PORT3 to _PICK_EVEN_2RANGES()
> drm/i915: Convert _FIA() to _PICK_EVEN_2RANGES()
> drm/i915: Convert MBUS_ABOX_CTL() to _PICK_EVEN_2RANGES()
> drm/i915: Convert PALETTE() to _PICK_EVEN_2RANGES()
>
> .../drm/i915/display/intel_display_reg_defs.h | 10 +-
> .../gpu/drm/i915/display/intel_mg_phy_regs.h | 4 +-
> drivers/gpu/drm/i915/i915_reg.h | 106 +++++++++---------
> drivers/gpu/drm/i915/i915_reg_defs.h | 28 +++++
> 4 files changed, 89 insertions(+), 59 deletions(-)
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-22 1:28 ` Lucas De Marchi
@ 2023-01-23 11:00 ` Jani Nikula
2023-01-23 16:15 ` Srivatsa, Anusha
0 siblings, 1 reply; 27+ messages in thread
From: Jani Nikula @ 2023-01-23 11:00 UTC (permalink / raw)
To: Lucas De Marchi, Srivatsa, Anusha; +Cc: intel-gfx, dri-devel
On Sat, 21 Jan 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Fri, Jan 20, 2023 at 10:14:19PM -0800, Anusha Srivatsa wrote:
>>
>>
>>> -----Original Message-----
>>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
>>> De Marchi
>>> Sent: Friday, January 20, 2023 11:35 AM
>>> To: intel-gfx@lists.freedesktop.org
>>> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
>>> devel@lists.freedesktop.org
>>> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
>>>
>>> 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.
>>>
>>> Add a variant of _PICK_EVEN() that works with 2 ranges and selects which one
>>> to use depending on the index value.
>>>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_reg_defs.h | 28 ++++++++++++++++++++++++++++
>>> 1 file changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
>>> b/drivers/gpu/drm/i915/i915_reg_defs.h
>>> index be43580a6979..b7ec87464d69 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
>>> @@ -119,6 +119,34 @@
>>> */
>>> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
>>>
>>> +/*
>>> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
>>> + * The first range is used for indexes below @__c_index, and the second
>>> + * range is used for anything above it. Example::
>>> + *
>>> + * #define _FOO_A 0xf000
>>> + * #define _FOO_B 0xf004
>>> + * #define _FOO_C 0xf008
>>> + * #define _SUPER_FOO_A 0xa000
>>> + * #define _SUPER_FOO_B 0xa100
>>> + * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3,
>>> \
>>> + * _FOO_A, _FOO_B,
>>> \
>>> + * _SUPER_FOO_A, _SUPER_FOO_B))
>>> + *
>>> + * This expands to:
>>> + * 0: 0xf000,
>>> + * 1: 0xf004,
>>> + * 2: 0xf008,
>>> + * 3: 0xa100,
>>You mean 3:0xa000
>
> doesn't really matter. This is an example of register addresses. They
> don't need to start from 0, it's whatever the hw gives us.
I think the point is that the example is inconsistent between
_SUPER_FOO_A and "3: 0xa100".
BR,
Jani.
>
> Lucas De Marchi
>
>>
>>> + * 4: 0xa200,
>>4:0xa100
>>
>>> + * 5: 0xa300,
>>5:0xa200
>>
>>Anusha
>>> + * ...
>>> + */
>>> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)
>>> \
>>> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
>>> \
>>> + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :
>>> \
>>> + _PICK_EVEN((__index) - (__c_index), __c,
>>> __d)))
>>> +
>>> /*
>>> * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
>>> *
>>> --
>>> 2.39.0
>>
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-23 11:00 ` Jani Nikula
@ 2023-01-23 16:15 ` Srivatsa, Anusha
2023-01-23 16:53 ` Lucas De Marchi
0 siblings, 1 reply; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-23 16:15 UTC (permalink / raw)
To: Jani Nikula, De Marchi, Lucas; +Cc: intel-gfx, dri-devel
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Monday, January 23, 2023 3:01 AM
> To: De Marchi, Lucas <lucas.demarchi@intel.com>; Srivatsa, Anusha
> <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
>
> On Sat, 21 Jan 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> > On Fri, Jan 20, 2023 at 10:14:19PM -0800, Anusha Srivatsa wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> >>> Of Lucas De Marchi
> >>> Sent: Friday, January 20, 2023 11:35 AM
> >>> To: intel-gfx@lists.freedesktop.org
> >>> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
> >>> devel@lists.freedesktop.org
> >>> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add
> >>> _PICK_EVEN_2RANGES()
> >>>
> >>> 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.
> >>>
> >>> Add a variant of _PICK_EVEN() that works with 2 ranges and selects
> >>> which one to use depending on the index value.
> >>>
> >>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >>> ---
> >>> drivers/gpu/drm/i915/i915_reg_defs.h | 28
> >>> ++++++++++++++++++++++++++++
> >>> 1 file changed, 28 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
> >>> b/drivers/gpu/drm/i915/i915_reg_defs.h
> >>> index be43580a6979..b7ec87464d69 100644
> >>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> >>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> >>> @@ -119,6 +119,34 @@
> >>> */
> >>> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) -
> >>> (__a)))
> >>>
> >>> +/*
> >>> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address
> offsets.
> >>> + * The first range is used for indexes below @__c_index, and the
> >>> +second
> >>> + * range is used for anything above it. Example::
> >>> + *
> >>> + * #define _FOO_A 0xf000
> >>> + * #define _FOO_B 0xf004
> >>> + * #define _FOO_C 0xf008
> >>> + * #define _SUPER_FOO_A 0xa000
> >>> + * #define _SUPER_FOO_B 0xa100
> >>> + * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3,
> >>> \
> >>> + * _FOO_A, _FOO_B,
> >>> \
> >>> + * _SUPER_FOO_A, _SUPER_FOO_B))
> >>> + *
> >>> + * This expands to:
> >>> + * 0: 0xf000,
> >>> + * 1: 0xf004,
> >>> + * 2: 0xf008,
> >>> + * 3: 0xa100,
> >>You mean 3:0xa000
> >
> > doesn't really matter. This is an example of register addresses. They
> > don't need to start from 0, it's whatever the hw gives us.
>
> I think the point is that the example is inconsistent between _SUPER_FOO_A and
> "3: 0xa100".
Yes. It's the inconsistency with _SUPER_FOO_A that I was pointing out.
Anusha
> BR,
> Jani.
>
> >
> > Lucas De Marchi
> >
> >>
> >>> + * 4: 0xa200,
> >>4:0xa100
> >>
> >>> + * 5: 0xa300,
> >>5:0xa200
> >>
> >>Anusha
> >>> + * ...
> >>> + */
> >>> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)
> >>> \
> >>> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
> >>> \
> >>> + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :
> >>> \
> >>> + _PICK_EVEN((__index) - (__c_index), __c,
> >>> __d)))
> >>> +
> >>> /*
> >>> * Given the arbitrary numbers in varargs, pick the 0-based __index'th
> number.
> >>> *
> >>> --
> >>> 2.39.0
> >>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-23 16:15 ` Srivatsa, Anusha
@ 2023-01-23 16:53 ` Lucas De Marchi
0 siblings, 0 replies; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-23 16:53 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx, dri-devel
On Mon, Jan 23, 2023 at 08:15:16AM -0800, Anusha Srivatsa wrote:
>
>
>> -----Original Message-----
>> From: Jani Nikula <jani.nikula@linux.intel.com>
>> Sent: Monday, January 23, 2023 3:01 AM
>> To: De Marchi, Lucas <lucas.demarchi@intel.com>; Srivatsa, Anusha
>> <anusha.srivatsa@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
>>
>> On Sat, 21 Jan 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> > On Fri, Jan 20, 2023 at 10:14:19PM -0800, Anusha Srivatsa wrote:
>> >>
>> >>
>> >>> -----Original Message-----
>> >>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>> >>> Of Lucas De Marchi
>> >>> Sent: Friday, January 20, 2023 11:35 AM
>> >>> To: intel-gfx@lists.freedesktop.org
>> >>> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
>> >>> devel@lists.freedesktop.org
>> >>> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add
>> >>> _PICK_EVEN_2RANGES()
>> >>>
>> >>> 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.
>> >>>
>> >>> Add a variant of _PICK_EVEN() that works with 2 ranges and selects
>> >>> which one to use depending on the index value.
>> >>>
>> >>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> >>> ---
>> >>> drivers/gpu/drm/i915/i915_reg_defs.h | 28
>> >>> ++++++++++++++++++++++++++++
>> >>> 1 file changed, 28 insertions(+)
>> >>>
>> >>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
>> >>> b/drivers/gpu/drm/i915/i915_reg_defs.h
>> >>> index be43580a6979..b7ec87464d69 100644
>> >>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
>> >>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
>> >>> @@ -119,6 +119,34 @@
>> >>> */
>> >>> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) -
>> >>> (__a)))
>> >>>
>> >>> +/*
>> >>> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address
>> offsets.
>> >>> + * The first range is used for indexes below @__c_index, and the
>> >>> +second
>> >>> + * range is used for anything above it. Example::
>> >>> + *
>> >>> + * #define _FOO_A 0xf000
>> >>> + * #define _FOO_B 0xf004
>> >>> + * #define _FOO_C 0xf008
>> >>> + * #define _SUPER_FOO_A 0xa000
>> >>> + * #define _SUPER_FOO_B 0xa100
>> >>> + * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3,
>> >>> \
>> >>> + * _FOO_A, _FOO_B,
>> >>> \
>> >>> + * _SUPER_FOO_A, _SUPER_FOO_B))
>> >>> + *
>> >>> + * This expands to:
>> >>> + * 0: 0xf000,
>> >>> + * 1: 0xf004,
>> >>> + * 2: 0xf008,
>> >>> + * 3: 0xa100,
>> >>You mean 3:0xa000
>> >
>> > doesn't really matter. This is an example of register addresses. They
>> > don't need to start from 0, it's whatever the hw gives us.
>>
>> I think the point is that the example is inconsistent between _SUPER_FOO_A and
>> "3: 0xa100".
>
>Yes. It's the inconsistency with _SUPER_FOO_A that I was pointing out.
ah, ok. I will send a fixup to this patch.
thanks
Lucas De Marchi
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2.1] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-20 19:34 ` [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES() Lucas De Marchi
2023-01-21 6:14 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-23 10:38 ` Jani Nikula
@ 2023-01-23 17:15 ` Lucas De Marchi
2023-01-23 17:49 ` Srivatsa, Anusha
2 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-23 17:15 UTC (permalink / raw)
To: intel-gfx, dri-devel; +Cc: Anusha Srivatsa, Lucas De Marchi
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.
Add a variant of _PICK_EVEN() that works with 2 ranges and selects which
one to use depending on the index value.
v2: Fix the address expansion in the example (Anusha)
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/i915_reg_defs.h | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
index be43580a6979..bab6a9ec2ddd 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -119,6 +119,34 @@
*/
#define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
+/*
+ * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
+ * The first range is used for indexes below @__c_index, and the second
+ * range is used for anything above it. Example::
+ *
+ * #define _FOO_A 0xf000
+ * #define _FOO_B 0xf004
+ * #define _FOO_C 0xf008
+ * #define _SUPER_FOO_A 0xa000
+ * #define _SUPER_FOO_B 0xa100
+ * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3, \
+ * _FOO_A, _FOO_B, \
+ * _SUPER_FOO_A, _SUPER_FOO_B))
+ *
+ * This expands to:
+ * 0: 0xf000,
+ * 1: 0xf004,
+ * 2: 0xf008,
+ * 3: 0xa000,
+ * 4: 0xa100,
+ * 5: 0xa200,
+ * ...
+ */
+#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) \
+ (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) + \
+ ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : \
+ _PICK_EVEN((__index) - (__c_index), __c, __d)))
+
/*
* Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
*
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* RE: [PATCH v2.1] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-23 17:15 ` [PATCH v2.1] " Lucas De Marchi
@ 2023-01-23 17:49 ` Srivatsa, Anusha
0 siblings, 0 replies; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-23 17:49 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx, dri-devel
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Monday, January 23, 2023 9:16 AM
> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Jani Nikula
> <jani.nikula@linux.intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
> Subject: [PATCH v2.1] drm/i915: Add _PICK_EVEN_2RANGES()
>
> 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.
>
> Add a variant of _PICK_EVEN() that works with 2 ranges and selects which one
> to use depending on the index value.
>
> v2: Fix the address expansion in the example (Anusha)
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg_defs.h | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
> b/drivers/gpu/drm/i915/i915_reg_defs.h
> index be43580a6979..bab6a9ec2ddd 100644
> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> @@ -119,6 +119,34 @@
> */
> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
>
> +/*
> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
> + * The first range is used for indexes below @__c_index, and the second
> + * range is used for anything above it. Example::
> + *
> + * #define _FOO_A 0xf000
> + * #define _FOO_B 0xf004
> + * #define _FOO_C 0xf008
> + * #define _SUPER_FOO_A 0xa000
> + * #define _SUPER_FOO_B 0xa100
> + * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3,
> \
> + * _FOO_A, _FOO_B,
> \
> + * _SUPER_FOO_A, _SUPER_FOO_B))
> + *
> + * This expands to:
> + * 0: 0xf000,
> + * 1: 0xf004,
> + * 2: 0xf008,
> + * 3: 0xa000,
> + * 4: 0xa100,
> + * 5: 0xa200,
> + * ...
> + */
> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)
> \
> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
> \
> + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :
> \
> + _PICK_EVEN((__index) - (__c_index), __c,
> __d)))
> +
> /*
> * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
> *
> --
> 2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [Intel-gfx] [PATCH v2 3/8] drm/i915: Convert pll macros to _PICK_EVEN_2RANGES
2023-01-20 19:34 ` [PATCH v2 3/8] drm/i915: Convert pll macros to _PICK_EVEN_2RANGES Lucas De Marchi
@ 2023-01-23 19:12 ` Srivatsa, Anusha
0 siblings, 0 replies; 27+ messages in thread
From: Srivatsa, Anusha @ 2023-01-23 19:12 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx; +Cc: De Marchi, Lucas, dri-devel
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lucas
> De Marchi
> Sent: Friday, January 20, 2023 11:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi@intel.com>; dri-
> devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 3/8] drm/i915: Convert pll macros to
> _PICK_EVEN_2RANGES
>
> Avoid the array lookup, converting the PLL macros after ICL to
> _PICK_EVEN_RANGES. This provides the following reduction in code size:
>
> $ size build64/drivers/gpu/drm/i915/i915.o{.old,.new}
> text data bss dec hex filename
> 4027456 185703 6984 4220143 4064ef
> build64/drivers/gpu/drm/i915/i915.o.old
> 4026997 185703 6984 4219684 406324
> build64/drivers/gpu/drm/i915/i915.o.new
>
> At the same time it's safer, avoiding out-of-bounds array access. This allows to
> remove _MMIO_PLL3() that is now unused.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
> .../drm/i915/display/intel_display_reg_defs.h | 1 -
> drivers/gpu/drm/i915/i915_reg.h | 59 +++++++++----------
> 2 files changed, 29 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> index 02605418ff08..a4ed1c530799 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h
> @@ -34,7 +34,6 @@
> #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
> -#define _MMIO_PLL3(pll, ...) _MMIO(_PICK(pll, __VA_ARGS__))
>
> /*
> * Device info offset array based helpers for groups of registers with unevenly
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8da3546d82fb..dd1eb8b10e0e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7232,13 +7232,15 @@ enum skl_power_gate {
> #define PLL_LOCK REG_BIT(30)
> #define PLL_POWER_ENABLE REG_BIT(27)
> #define PLL_POWER_STATE REG_BIT(26)
> -#define ICL_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE,
> _DPLL1_ENABLE, \
> - _ADLS_DPLL2_ENABLE,
> _ADLS_DPLL3_ENABLE)
> +#define ICL_DPLL_ENABLE(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 3,
> \
> + _DPLL0_ENABLE,
> _DPLL1_ENABLE, \
> +
> _ADLS_DPLL3_ENABLE, _ADLS_DPLL3_ENABLE))
>
> #define _DG2_PLL3_ENABLE 0x4601C
>
> -#define DG2_PLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE,
> _DPLL1_ENABLE, \
> - _ADLS_DPLL2_ENABLE,
> _DG2_PLL3_ENABLE)
> +#define DG2_PLL_ENABLE(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 3,
> \
> + _DPLL0_ENABLE,
> _DPLL1_ENABLE, \
> + _DG2_PLL3_ENABLE,
> _DG2_PLL3_ENABLE))
>
> #define TBT_PLL_ENABLE _MMIO(0x46020)
>
> @@ -7251,8 +7253,9 @@ enum skl_power_gate {
> _MG_PLL2_ENABLE)
>
> /* DG1 PLL */
> -#define DG1_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE,
> _DPLL1_ENABLE, \
> - _MG_PLL1_ENABLE,
> _MG_PLL2_ENABLE)
> +#define DG1_DPLL_ENABLE(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2,
> \
> + _DPLL0_ENABLE,
> _DPLL1_ENABLE, \
> + _MG_PLL1_ENABLE,
> _MG_PLL2_ENABLE))
>
> /* ADL-P Type C PLL */
> #define PORTTC1_PLL_ENABLE 0x46038
> @@ -7312,9 +7315,9 @@ enum skl_power_gate {
> #define _TGL_DPLL0_CFGCR0 0x164284
> #define _TGL_DPLL1_CFGCR0 0x16428C
> #define _TGL_TBTPLL_CFGCR0 0x16429C
> -#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL3(pll,
> _TGL_DPLL0_CFGCR0, \
> - _TGL_DPLL1_CFGCR0, \
> - _TGL_TBTPLL_CFGCR0)
> +#define TGL_DPLL_CFGCR0(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2,
> \
> + _TGL_DPLL0_CFGCR0,
> _TGL_DPLL1_CFGCR0, \
> + _TGL_TBTPLL_CFGCR0,
> _TGL_TBTPLL_CFGCR0))
> #define RKL_DPLL_CFGCR0(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR0,
> \
> _TGL_DPLL1_CFGCR0)
>
> @@ -7327,40 +7330,36 @@ enum skl_power_gate {
> #define _TGL_DPLL0_CFGCR1 0x164288
> #define _TGL_DPLL1_CFGCR1 0x164290
> #define _TGL_TBTPLL_CFGCR1 0x1642A0
> -#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL3(pll,
> _TGL_DPLL0_CFGCR1, \
> - _TGL_DPLL1_CFGCR1, \
> - _TGL_TBTPLL_CFGCR1)
> +#define TGL_DPLL_CFGCR1(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2,
> \
> + _TGL_DPLL0_CFGCR1,
> _TGL_DPLL1_CFGCR1, \
> + _TGL_TBTPLL_CFGCR1,
> _TGL_TBTPLL_CFGCR1))
> #define RKL_DPLL_CFGCR1(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR1,
> \
> _TGL_DPLL1_CFGCR1)
>
> #define _DG1_DPLL2_CFGCR0 0x16C284
> #define _DG1_DPLL3_CFGCR0 0x16C28C
> -#define DG1_DPLL_CFGCR0(pll) _MMIO_PLL3(pll,
> _TGL_DPLL0_CFGCR0, \
> - _TGL_DPLL1_CFGCR0, \
> - _DG1_DPLL2_CFGCR0, \
> - _DG1_DPLL3_CFGCR0)
> +#define DG1_DPLL_CFGCR0(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2,
> \
> + _TGL_DPLL0_CFGCR0,
> _TGL_DPLL1_CFGCR0, \
> + _DG1_DPLL2_CFGCR0,
> _DG1_DPLL3_CFGCR0))
>
> #define _DG1_DPLL2_CFGCR1 0x16C288
> #define _DG1_DPLL3_CFGCR1 0x16C290
> -#define DG1_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, _TGL_DPLL0_CFGCR1, \
> - _TGL_DPLL1_CFGCR1, \
> - _DG1_DPLL2_CFGCR1, \
> - _DG1_DPLL3_CFGCR1)
> +#define DG1_DPLL_CFGCR1(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2,
> \
> + _TGL_DPLL0_CFGCR1,
> _TGL_DPLL1_CFGCR1, \
> + _DG1_DPLL2_CFGCR1,
> _DG1_DPLL3_CFGCR1))
>
> /* For ADL-S DPLL4_CFGCR0/1 are used to control DPLL2 */
> -#define _ADLS_DPLL3_CFGCR0 0x1642C0
> #define _ADLS_DPLL4_CFGCR0 0x164294
> -#define ADLS_DPLL_CFGCR0(pll) _MMIO_PLL3(pll,
> _TGL_DPLL0_CFGCR0, \
> - _TGL_DPLL1_CFGCR0, \
> - _ADLS_DPLL4_CFGCR0, \
> - _ADLS_DPLL3_CFGCR0)
> +#define _ADLS_DPLL3_CFGCR0 0x1642C0
> +#define ADLS_DPLL_CFGCR0(pll)
> _MMIO(_PICK_EVEN_2RANGES(pll, 2, \
> + _TGL_DPLL0_CFGCR0,
> _TGL_DPLL1_CFGCR0, \
> + _ADLS_DPLL4_CFGCR0,
> _ADLS_DPLL3_CFGCR0))
>
> -#define _ADLS_DPLL3_CFGCR1 0x1642C4
> #define _ADLS_DPLL4_CFGCR1 0x164298
> -#define ADLS_DPLL_CFGCR1(pll) _MMIO_PLL3(pll,
> _TGL_DPLL0_CFGCR1, \
> - _TGL_DPLL1_CFGCR1, \
> - _ADLS_DPLL4_CFGCR1, \
> - _ADLS_DPLL3_CFGCR1)
> +#define _ADLS_DPLL3_CFGCR1 0x1642C4
> +#define ADLS_DPLL_CFGCR1(pll)
> _MMIO(_PICK_EVEN_2RANGES(pll, 2, \
> + _TGL_DPLL0_CFGCR1,
> _TGL_DPLL1_CFGCR1, \
> + _ADLS_DPLL4_CFGCR1,
> _ADLS_DPLL3_CFGCR1))
>
> /* BXT display engine PLL */
> #define BXT_DE_PLL_CTL _MMIO(0x6d000)
> --
> 2.39.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-23 10:38 ` Jani Nikula
@ 2023-01-24 7:45 ` Lucas De Marchi
2023-01-25 18:24 ` [PATCH v2.2] " Lucas De Marchi
0 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-24 7:45 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, dri-devel
(I missed this review you did before I had sent a v2.1, I will incorporate
what is missing in the next version)
On Mon, Jan 23, 2023 at 12:38:28PM +0200, Jani Nikula wrote:
>On Fri, 20 Jan 2023, Lucas De Marchi <lucas.demarchi@intel.com> 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
>
>Would be interesting to see what this does, and whether the compiler has
>the smarts to combine these within each file:
>
>-#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
>+#define _PICK(__index, ...) (((static const u32 []){ __VA_ARGS__ })[__index])
if the compiler is smart, it would be at least 1 per compilation unit.
gcc doesn't seem smart enough to even compile it though:
../drivers/gpu/drm/i915/i915_reg_defs.h:155:52: error: expected ‘)’ before ‘{’ token
155 | #define _PICK(__index, ...) (((static const u32 []){ __VA_ARGS__ })[__index])
| ~ ^
What I'm thinking for the remaining uses of _PICK() is to be explicit
and statically define them in a good .c depending on the use.
Then use that in the macro.
>
>> 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.
>>
>> Add a variant of _PICK_EVEN() that works with 2 ranges and selects which
>> one to use depending on the index value.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg_defs.h | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
>> index be43580a6979..b7ec87464d69 100644
>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
>> @@ -119,6 +119,34 @@
>> */
>> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
>>
>> +/*
>> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
>> + * The first range is used for indexes below @__c_index, and the second
>> + * range is used for anything above it. Example::
>
>I'd like this to be clear about which range is used for index ==
>__c_index, instead of saying "below" and "above".
>
>No need for the double colon :: because this isn't a kernel-doc comment.
ok, what about this?
* Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
* @__c_index corresponds to the index in which the second range starts
* to be used. Using math interval notation, the first range is used
* for indexes [ 0, @__c_index), while the second range is used for
* [ @__c_index, ... ). Example:
>
>> + *
>> + * #define _FOO_A 0xf000
>> + * #define _FOO_B 0xf004
>> + * #define _FOO_C 0xf008
>> + * #define _SUPER_FOO_A 0xa000
>> + * #define _SUPER_FOO_B 0xa100
>> + * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3, \
>
>The example uses a different name for the macro.
yeah, that was the previous name I was using... good catch, will fix.
>
>> + * _FOO_A, _FOO_B, \
>> + * _SUPER_FOO_A, _SUPER_FOO_B))
>> + *
>> + * This expands to:
>> + * 0: 0xf000,
>> + * 1: 0xf004,
>> + * 2: 0xf008,
>> + * 3: 0xa100,
>
>With the above definitions, this would be 3: 0xa000.
fixed
thanks
Lucas De Marchi
>
>> + * 4: 0xa200,
>> + * 5: 0xa300,
>> + * ...
>> + */
>> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) \
>> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) + \
>> + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : \
>> + _PICK_EVEN((__index) - (__c_index), __c, __d)))
>> +
>> /*
>> * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
>> *
>
>--
>Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2.2] drm/i915: Add _PICK_EVEN_2RANGES()
2023-01-24 7:45 ` Lucas De Marchi
@ 2023-01-25 18:24 ` Lucas De Marchi
0 siblings, 0 replies; 27+ messages in thread
From: Lucas De Marchi @ 2023-01-25 18:24 UTC (permalink / raw)
To: intel-gfx, dri-devel; +Cc: Anusha Srivatsa, Lucas De Marchi
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.
Add a variant of _PICK_EVEN() that works with 2 ranges and selects which
one to use depending on the index value.
v2: Fix the address expansion in the example (Anusha)
v3: Also rename macro to _PICK_EVEN_2RANGES() in the documentation
and reword it to clarify what ranges are chosen based on the index
(Jani)
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/i915_reg_defs.h | 29 ++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
index be43580a6979..983c5aa3045b 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -119,6 +119,35 @@
*/
#define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
+/*
+ * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
+ * @__c_index corresponds to the index in which the second range starts to be
+ * used. Using math interval notation, the first range is used for indexes [ 0,
+ * @__c_index), while the second range is used for [ @__c_index, ... ). Example:
+ *
+ * #define _FOO_A 0xf000
+ * #define _FOO_B 0xf004
+ * #define _FOO_C 0xf008
+ * #define _SUPER_FOO_A 0xa000
+ * #define _SUPER_FOO_B 0xa100
+ * #define FOO(x) _MMIO(_PICK_EVEN_2RANGES(x, 3, \
+ * _FOO_A, _FOO_B, \
+ * _SUPER_FOO_A, _SUPER_FOO_B))
+ *
+ * This expands to:
+ * 0: 0xf000,
+ * 1: 0xf004,
+ * 2: 0xf008,
+ * 3: 0xa000,
+ * 4: 0xa100,
+ * 5: 0xa200,
+ * ...
+ */
+#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) \
+ (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) + \
+ ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : \
+ _PICK_EVEN((__index) - (__c_index), __c, __d)))
+
/*
* Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
*
--
2.39.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
end of thread, other threads:[~2023-01-25 18:25 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-20 19:34 [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Lucas De Marchi
2023-01-20 19:34 ` [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES() Lucas De Marchi
2023-01-21 6:14 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-22 1:28 ` Lucas De Marchi
2023-01-23 11:00 ` Jani Nikula
2023-01-23 16:15 ` Srivatsa, Anusha
2023-01-23 16:53 ` Lucas De Marchi
2023-01-23 10:38 ` Jani Nikula
2023-01-24 7:45 ` Lucas De Marchi
2023-01-25 18:24 ` [PATCH v2.2] " Lucas De Marchi
2023-01-23 17:15 ` [PATCH v2.1] " Lucas De Marchi
2023-01-23 17:49 ` Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 2/8] drm/i915: Fix coding style on DPLL*_ENABLE defines Lucas De Marchi
2023-01-20 20:14 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 3/8] drm/i915: Convert pll macros to _PICK_EVEN_2RANGES Lucas De Marchi
2023-01-23 19:12 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 4/8] drm/i915: Replace _MMIO_PHY3() with _PICK_EVEN_2RANGES() Lucas De Marchi
2023-01-21 5:58 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 5/8] drm/i915: Convert PIPE3/PORT3 to _PICK_EVEN_2RANGES() Lucas De Marchi
2023-01-21 6:00 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 6/8] drm/i915: Convert _FIA() " Lucas De Marchi
2023-01-21 6:01 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 7/8] drm/i915: Convert MBUS_ABOX_CTL() " Lucas De Marchi
2023-01-21 6:04 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-20 19:34 ` [PATCH v2 8/8] drm/i915: Convert PALETTE() " Lucas De Marchi
2023-01-21 6:06 ` [Intel-gfx] " Srivatsa, Anusha
2023-01-23 10:39 ` [PATCH v2 0/8] Add _PICK_EVEN_2RANGES Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).