* [PATCH 0/7] Reorganize the register picking macros
@ 2017-06-13 19:33 Paulo Zanoni
2017-06-13 19:33 ` [PATCH 1/7] drm/i915: reorder " Paulo Zanoni
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: Paulo Zanoni @ 2017-06-13 19:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi
We have 3 different types of macros to pick registers with multiple instances
and each of these types is coded in a different way. I like consistency, so I
wrote this series in order to try to make our code a little more clear regarding
our options and their differences.
Besides the consistency aspect, this series also does a few renames. I'm kinda
afraid this series may get stuck in an endless bikeshed regarding the names. I
know the current names are not very good, but at least the naming is consistent
now, so I think there's value in merging the series. Regardless, I'm completely
open to good naming suggestions.
The only test I've done in the series was to boot SKL. Let's see what the CI
system has to say about it.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Paulo Zanoni (7):
drm/i915: reorder the register picking macros
drm/i915: _MMIO_PORT3 takes a port as an argument
drm/i915: use variable arguments for the macros that call _PICK
drm/i915: rename _PICK to _PICK3
drm/i915: add _PICK macro for the "a + index * (b - a)" macros
drm/i915: also move version 2 of the register picking macros up
drm/i915: extract a _PICK2 macro
drivers/gpu/drm/i915/i915_reg.h | 69 +++++++++++++++++++++--------------------
1 file changed, 36 insertions(+), 33 deletions(-)
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/7] drm/i915: reorder the register picking macros
2017-06-13 19:33 [PATCH 0/7] Reorganize the register picking macros Paulo Zanoni
@ 2017-06-13 19:33 ` Paulo Zanoni
2017-06-14 17:22 ` Rodrigo Vivi
2017-06-13 19:33 ` [PATCH 2/7] drm/i915: _MMIO_PORT3 takes a port as an argument Paulo Zanoni
` (6 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Paulo Zanoni @ 2017-06-13 19:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
We currently have pipe, plane, trans, port, pll and phy versions of
these macros. Reorder their definitions so all macros of each type are
in their own group, separated by blank lines.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 88e4707..a1d3cca 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -52,19 +52,24 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
#define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
+#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
+
#define _PLANE(plane, a, b) _PIPE(plane, a, b)
#define _MMIO_PLANE(plane, a, b) _MMIO_PIPE(plane, a, b)
+
#define _TRANS(tran, a, b) ((a) + (tran)*((b)-(a)))
#define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
+
#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
#define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, 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 _PLL(pll, a, b) ((a) + (pll)*((b)-(a)))
-#define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
#define _MMIO_PORT6(port, a, b, c, d, e, f) _MMIO(_PICK(port, a, b, c, d, e, f))
#define _MMIO_PORT6_LN(port, ln, a0, a1, b, c, d, e, f) \
_MMIO(_PICK(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
+
+#define _PLL(pll, a, b) ((a) + (pll)*((b)-(a)))
+#define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
+
#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
#define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/7] drm/i915: _MMIO_PORT3 takes a port as an argument
2017-06-13 19:33 [PATCH 0/7] Reorganize the register picking macros Paulo Zanoni
2017-06-13 19:33 ` [PATCH 1/7] drm/i915: reorder " Paulo Zanoni
@ 2017-06-13 19:33 ` Paulo Zanoni
2017-06-14 17:23 ` Rodrigo Vivi
2017-06-13 19:33 ` [PATCH 3/7] drm/i915: use variable arguments for the macros that call _PICK Paulo Zanoni
` (5 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Paulo Zanoni @ 2017-06-13 19:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
The macro takes a port as an argument, not a pipe.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a1d3cca..1983b75 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -62,7 +62,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
#define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
-#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
+#define _MMIO_PORT3(port, a, b, c) _MMIO(_PICK(port, a, b, c))
#define _MMIO_PORT6(port, a, b, c, d, e, f) _MMIO(_PICK(port, a, b, c, d, e, f))
#define _MMIO_PORT6_LN(port, ln, a0, a1, b, c, d, e, f) \
_MMIO(_PICK(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/7] drm/i915: use variable arguments for the macros that call _PICK
2017-06-13 19:33 [PATCH 0/7] Reorganize the register picking macros Paulo Zanoni
2017-06-13 19:33 ` [PATCH 1/7] drm/i915: reorder " Paulo Zanoni
2017-06-13 19:33 ` [PATCH 2/7] drm/i915: _MMIO_PORT3 takes a port as an argument Paulo Zanoni
@ 2017-06-13 19:33 ` Paulo Zanoni
2017-06-14 17:25 ` Rodrigo Vivi
2017-06-13 19:33 ` [PATCH 4/7] drm/i915: rename _PICK to _PICK3 Paulo Zanoni
` (4 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Paulo Zanoni @ 2017-06-13 19:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
There's no need to create a new macro every time the number of
parameters change.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1983b75..1d5a0f8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -52,7 +52,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
#define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
-#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
+#define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK(pipe, __VA_ARGS__))
#define _PLANE(plane, a, b) _PIPE(plane, a, b)
#define _MMIO_PLANE(plane, a, b) _MMIO_PIPE(plane, a, b)
@@ -62,8 +62,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
#define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
-#define _MMIO_PORT3(port, a, b, c) _MMIO(_PICK(port, a, b, c))
-#define _MMIO_PORT6(port, a, b, c, d, e, f) _MMIO(_PICK(port, a, b, c, d, e, f))
+#define _MMIO_PORT3(port, ...) _MMIO(_PICK(port, __VA_ARGS__))
+#define _MMIO_PORT6(port, ...) _MMIO(_PICK(port, __VA_ARGS__))
#define _MMIO_PORT6_LN(port, ln, a0, a1, b, c, d, e, f) \
_MMIO(_PICK(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
@@ -71,7 +71,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
-#define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
+#define _MMIO_PHY3(phy, ...) _MMIO(_PHY3(phy, __VA_ARGS__))
#define _MASKED_FIELD(mask, value) ({ \
if (__builtin_constant_p(mask)) \
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/7] drm/i915: rename _PICK to _PICK3
2017-06-13 19:33 [PATCH 0/7] Reorganize the register picking macros Paulo Zanoni
` (2 preceding siblings ...)
2017-06-13 19:33 ` [PATCH 3/7] drm/i915: use variable arguments for the macros that call _PICK Paulo Zanoni
@ 2017-06-13 19:33 ` Paulo Zanoni
2017-06-15 20:22 ` Jani Nikula
2017-06-13 19:33 ` [PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros Paulo Zanoni
` (3 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Paulo Zanoni @ 2017-06-13 19:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
All of the macros that call _PICK are named _X_3, so let's rename
_PICK to _PICK3. The reason we're doing this is because we're going to
have _PICK and _PICK2. Consider _PICK3 as the third variation of the
PICK macros (well, actually it *is* the third variation...).
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1d5a0f8..d271098 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -48,11 +48,11 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
}
-#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
+#define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
#define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
#define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
-#define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK(pipe, __VA_ARGS__))
+#define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
#define _PLANE(plane, a, b) _PIPE(plane, a, b)
#define _MMIO_PLANE(plane, a, b) _MMIO_PIPE(plane, a, b)
@@ -62,15 +62,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
#define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
-#define _MMIO_PORT3(port, ...) _MMIO(_PICK(port, __VA_ARGS__))
-#define _MMIO_PORT6(port, ...) _MMIO(_PICK(port, __VA_ARGS__))
-#define _MMIO_PORT6_LN(port, ln, a0, a1, b, c, d, e, f) \
- _MMIO(_PICK(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
+#define _MMIO_PORT3(port, ...) _MMIO(_PICK3(port, __VA_ARGS__))
+#define _MMIO_PORT3_LN(port, ln, a0, a1, b, c, d, e, f) \
+ _MMIO(_PICK3(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
#define _PLL(pll, a, b) ((a) + (pll)*((b)-(a)))
#define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
-#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
+#define _PHY3(phy, ...) _PICK3(phy, __VA_ARGS__)
#define _MMIO_PHY3(phy, ...) _MMIO(_PHY3(phy, __VA_ARGS__))
#define _MASKED_FIELD(mask, value) ({ \
@@ -1709,14 +1708,14 @@ enum skl_disp_power_wells {
#define _CNL_PORT_PCS_DW1_LN0_C 0x162C04
#define _CNL_PORT_PCS_DW1_LN0_D 0x162E04
#define _CNL_PORT_PCS_DW1_LN0_F 0x162804
-#define CNL_PORT_PCS_DW1_GRP(port) _MMIO_PORT6(port, \
+#define CNL_PORT_PCS_DW1_GRP(port) _MMIO_PORT3(port, \
_CNL_PORT_PCS_DW1_GRP_AE, \
_CNL_PORT_PCS_DW1_GRP_B, \
_CNL_PORT_PCS_DW1_GRP_C, \
_CNL_PORT_PCS_DW1_GRP_D, \
_CNL_PORT_PCS_DW1_GRP_AE, \
_CNL_PORT_PCS_DW1_GRP_F)
-#define CNL_PORT_PCS_DW1_LN0(port) _MMIO_PORT6(port, \
+#define CNL_PORT_PCS_DW1_LN0(port) _MMIO_PORT3(port, \
_CNL_PORT_PCS_DW1_LN0_AE, \
_CNL_PORT_PCS_DW1_LN0_B, \
_CNL_PORT_PCS_DW1_LN0_C, \
@@ -1735,14 +1734,14 @@ enum skl_disp_power_wells {
#define _CNL_PORT_TX_DW2_LN0_C 0x162C48
#define _CNL_PORT_TX_DW2_LN0_D 0x162E48
#define _CNL_PORT_TX_DW2_LN0_F 0x162A48
-#define CNL_PORT_TX_DW2_GRP(port) _MMIO_PORT6(port, \
+#define CNL_PORT_TX_DW2_GRP(port) _MMIO_PORT3(port, \
_CNL_PORT_TX_DW2_GRP_AE, \
_CNL_PORT_TX_DW2_GRP_B, \
_CNL_PORT_TX_DW2_GRP_C, \
_CNL_PORT_TX_DW2_GRP_D, \
_CNL_PORT_TX_DW2_GRP_AE, \
_CNL_PORT_TX_DW2_GRP_F)
-#define CNL_PORT_TX_DW2_LN0(port) _MMIO_PORT6(port, \
+#define CNL_PORT_TX_DW2_LN0(port) _MMIO_PORT3(port, \
_CNL_PORT_TX_DW2_LN0_AE, \
_CNL_PORT_TX_DW2_LN0_B, \
_CNL_PORT_TX_DW2_LN0_C, \
@@ -1764,14 +1763,14 @@ enum skl_disp_power_wells {
#define _CNL_PORT_TX_DW4_LN0_C 0x162C50
#define _CNL_PORT_TX_DW4_LN0_D 0x162E50
#define _CNL_PORT_TX_DW4_LN0_F 0x162850
-#define CNL_PORT_TX_DW4_GRP(port) _MMIO_PORT6(port, \
+#define CNL_PORT_TX_DW4_GRP(port) _MMIO_PORT3(port, \
_CNL_PORT_TX_DW4_GRP_AE, \
_CNL_PORT_TX_DW4_GRP_B, \
_CNL_PORT_TX_DW4_GRP_C, \
_CNL_PORT_TX_DW4_GRP_D, \
_CNL_PORT_TX_DW4_GRP_AE, \
_CNL_PORT_TX_DW4_GRP_F)
-#define CNL_PORT_TX_DW4_LN(port, ln) _MMIO_PORT6_LN(port, ln, \
+#define CNL_PORT_TX_DW4_LN(port, ln) _MMIO_PORT3_LN(port, ln, \
_CNL_PORT_TX_DW4_LN0_AE, \
_CNL_PORT_TX_DW4_LN1_AE, \
_CNL_PORT_TX_DW4_LN0_B, \
@@ -1794,14 +1793,14 @@ enum skl_disp_power_wells {
#define _CNL_PORT_TX_DW5_LN0_C 0x162C54
#define _CNL_PORT_TX_DW5_LN0_D 0x162ED4
#define _CNL_PORT_TX_DW5_LN0_F 0x162854
-#define CNL_PORT_TX_DW5_GRP(port) _MMIO_PORT6(port, \
+#define CNL_PORT_TX_DW5_GRP(port) _MMIO_PORT3(port, \
_CNL_PORT_TX_DW5_GRP_AE, \
_CNL_PORT_TX_DW5_GRP_B, \
_CNL_PORT_TX_DW5_GRP_C, \
_CNL_PORT_TX_DW5_GRP_D, \
_CNL_PORT_TX_DW5_GRP_AE, \
_CNL_PORT_TX_DW5_GRP_F)
-#define CNL_PORT_TX_DW5_LN0(port) _MMIO_PORT6(port, \
+#define CNL_PORT_TX_DW5_LN0(port) _MMIO_PORT3(port, \
_CNL_PORT_TX_DW5_LN0_AE, \
_CNL_PORT_TX_DW5_LN0_B, \
_CNL_PORT_TX_DW5_LN0_C, \
@@ -1823,14 +1822,14 @@ enum skl_disp_power_wells {
#define _CNL_PORT_TX_DW7_LN0_C 0x162C5C
#define _CNL_PORT_TX_DW7_LN0_D 0x162EDC
#define _CNL_PORT_TX_DW7_LN0_F 0x16285C
-#define CNL_PORT_TX_DW7_GRP(port) _MMIO_PORT6(port, \
+#define CNL_PORT_TX_DW7_GRP(port) _MMIO_PORT3(port, \
_CNL_PORT_TX_DW7_GRP_AE, \
_CNL_PORT_TX_DW7_GRP_B, \
_CNL_PORT_TX_DW7_GRP_C, \
_CNL_PORT_TX_DW7_GRP_D, \
_CNL_PORT_TX_DW7_GRP_AE, \
_CNL_PORT_TX_DW7_GRP_F)
-#define CNL_PORT_TX_DW7_LN0(port) _MMIO_PORT6(port, \
+#define CNL_PORT_TX_DW7_LN0(port) _MMIO_PORT3(port, \
_CNL_PORT_TX_DW7_LN0_AE, \
_CNL_PORT_TX_DW7_LN0_B, \
_CNL_PORT_TX_DW7_LN0_C, \
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros
2017-06-13 19:33 [PATCH 0/7] Reorganize the register picking macros Paulo Zanoni
` (3 preceding siblings ...)
2017-06-13 19:33 ` [PATCH 4/7] drm/i915: rename _PICK to _PICK3 Paulo Zanoni
@ 2017-06-13 19:33 ` Paulo Zanoni
2017-06-15 20:26 ` Jani Nikula
2017-06-13 19:33 ` [PATCH 6/7] drm/i915: also move version 2 of the register picking macros up Paulo Zanoni
` (2 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Paulo Zanoni @ 2017-06-13 19:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Instead of duplicating the macro everywhere, add a single definition
for it and call it just like we do with the _PICK3 macros.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d271098..3e46ba1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -48,25 +48,26 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
}
+#define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a)))
#define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
-#define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
+#define _PIPE(pipe, a, b) _PICK(pipe, a, b)
#define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
#define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
-#define _PLANE(plane, a, b) _PIPE(plane, a, b)
-#define _MMIO_PLANE(plane, a, b) _MMIO_PIPE(plane, a, b)
+#define _PLANE(plane, a, b) _PICK(plane, a, b)
+#define _MMIO_PLANE(plane, a, b) _MMIO(_PLANE(plane, a, b))
-#define _TRANS(tran, a, b) ((a) + (tran)*((b)-(a)))
+#define _TRANS(tran, a, b) _PICK(tran, a, b)
#define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
-#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
+#define _PORT(port, a, b) _PICK(port, a, b)
#define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
#define _MMIO_PORT3(port, ...) _MMIO(_PICK3(port, __VA_ARGS__))
#define _MMIO_PORT3_LN(port, ln, a0, a1, b, c, d, e, f) \
_MMIO(_PICK3(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
-#define _PLL(pll, a, b) ((a) + (pll)*((b)-(a)))
+#define _PLL(pll, a, b) _PICK(pll, a, b)
#define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
#define _PHY3(phy, ...) _PICK3(phy, __VA_ARGS__)
@@ -6415,7 +6416,7 @@ enum {
#define _PS_ECC_STAT_2B 0x68AD0
#define _PS_ECC_STAT_1C 0x691D0
-#define _ID(id, a, b) ((a) + (id)*((b)-(a)))
+#define _ID(id, a, b) _PICK(id, a, b)
#define SKL_PS_CTRL(pipe, id) _MMIO_PIPE(pipe, \
_ID(id, _PS_1A_CTRL, _PS_2A_CTRL), \
_ID(id, _PS_1B_CTRL, _PS_2B_CTRL))
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 6/7] drm/i915: also move version 2 of the register picking macros up
2017-06-13 19:33 [PATCH 0/7] Reorganize the register picking macros Paulo Zanoni
` (4 preceding siblings ...)
2017-06-13 19:33 ` [PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros Paulo Zanoni
@ 2017-06-13 19:33 ` Paulo Zanoni
2017-06-15 20:37 ` Jani Nikula
2017-06-13 19:33 ` [PATCH 7/7] drm/i915: extract a _PICK2 macro Paulo Zanoni
2017-06-13 19:49 ` ✓ Fi.CI.BAT: success for Reorganize the register picking macros Patchwork
7 siblings, 1 reply; 20+ messages in thread
From: Paulo Zanoni @ 2017-06-13 19:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Make sure all the macros are next to each other so it's easier to spot
all the options available.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3e46ba1..a97af4a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -53,6 +53,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _PIPE(pipe, a, b) _PICK(pipe, a, b)
#define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
+#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \
+ dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
+ dev_priv->info.display_mmio_offset)
#define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
#define _PLANE(plane, a, b) _PICK(plane, a, b)
@@ -60,6 +63,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _TRANS(tran, a, b) _PICK(tran, a, b)
#define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
+#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \
+ dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
+ dev_priv->info.display_mmio_offset)
#define _PORT(port, a, b) _PICK(port, a, b)
#define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
@@ -3700,10 +3706,6 @@ enum {
#define CHV_TRANSCODER_C_OFFSET 0x63000
#define TRANSCODER_EDP_OFFSET 0x6f000
-#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \
- dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
- dev_priv->info.display_mmio_offset)
-
#define HTOTAL(trans) _MMIO_TRANS2(trans, _HTOTAL_A)
#define HBLANK(trans) _MMIO_TRANS2(trans, _HBLANK_A)
#define HSYNC(trans) _MMIO_TRANS2(trans, _HSYNC_A)
@@ -5189,10 +5191,6 @@ enum {
*/
#define PIPE_EDP_OFFSET 0x7f000
-#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \
- dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
- dev_priv->info.display_mmio_offset)
-
#define PIPECONF(pipe) _MMIO_PIPE2(pipe, _PIPEACONF)
#define PIPEDSL(pipe) _MMIO_PIPE2(pipe, _PIPEADSL)
#define PIPEFRAME(pipe) _MMIO_PIPE2(pipe, _PIPEAFRAMEHIGH)
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 7/7] drm/i915: extract a _PICK2 macro
2017-06-13 19:33 [PATCH 0/7] Reorganize the register picking macros Paulo Zanoni
` (5 preceding siblings ...)
2017-06-13 19:33 ` [PATCH 6/7] drm/i915: also move version 2 of the register picking macros up Paulo Zanoni
@ 2017-06-13 19:33 ` Paulo Zanoni
2017-06-14 15:16 ` Ville Syrjälä
2017-06-13 19:49 ` ✓ Fi.CI.BAT: success for Reorganize the register picking macros Patchwork
7 siblings, 1 reply; 20+ messages in thread
From: Paulo Zanoni @ 2017-06-13 19:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Do it just like we do with _PICK and _PICK3, so our code looks a
little more uniform.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a97af4a..3fb7b63 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -49,13 +49,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
}
#define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a)))
+#define _PICK2(__index, __offsets, a) (__offsets[__index] - __offsets[0] + \
+ (a) + dev_priv->info.display_mmio_offset)
#define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
#define _PIPE(pipe, a, b) _PICK(pipe, a, b)
#define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
-#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \
- dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
- dev_priv->info.display_mmio_offset)
+#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv->info.pipe_offsets, \
+ reg))
#define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
#define _PLANE(plane, a, b) _PICK(plane, a, b)
@@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define _TRANS(tran, a, b) _PICK(tran, a, b)
#define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
-#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \
- dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
- dev_priv->info.display_mmio_offset)
+#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \
+ dev_priv->info.trans_offsets, reg))
#define _PORT(port, a, b) _PICK(port, a, b)
#define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
--
2.9.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for Reorganize the register picking macros
2017-06-13 19:33 [PATCH 0/7] Reorganize the register picking macros Paulo Zanoni
` (6 preceding siblings ...)
2017-06-13 19:33 ` [PATCH 7/7] drm/i915: extract a _PICK2 macro Paulo Zanoni
@ 2017-06-13 19:49 ` Patchwork
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-06-13 19:49 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== Series Details ==
Series: Reorganize the register picking macros
URL : https://patchwork.freedesktop.org/series/25730/
State : success
== Summary ==
Series 25730v1 Reorganize the register picking macros
https://patchwork.freedesktop.org/api/1.0/series/25730/revisions/1/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail -> PASS (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-kbl-r) fdo#100125
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:440s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:431s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:581s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:510s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:481s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:476s
fi-glk-2a total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:585s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:430s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:417s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:417s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:488s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:465s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:463s
fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:570s
fi-kbl-r total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time:579s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:468s
fi-skl-6700hq total:278 pass:228 dwarn:1 dfail:0 fail:27 skip:22 time:404s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:467s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:478s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:435s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:539s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:405s
d4bedb8b0f9ba91df2e8cb136a489145a83e96a7 drm-tip: 2017y-06m-13d-14h-22m-46s UTC integration manifest
293f16c drm/i915: extract a _PICK2 macro
d16974b drm/i915: also move version 2 of the register picking macros up
f88a397 drm/i915: add _PICK macro for the "a + index * (b - a)" macros
9beadfe drm/i915: rename _PICK to _PICK3
0b0036b drm/i915: use variable arguments for the macros that call _PICK
2c8d3c4 drm/i915: _MMIO_PORT3 takes a port as an argument
6311637 drm/i915: reorder the register picking macros
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4944/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 7/7] drm/i915: extract a _PICK2 macro
2017-06-13 19:33 ` [PATCH 7/7] drm/i915: extract a _PICK2 macro Paulo Zanoni
@ 2017-06-14 15:16 ` Ville Syrjälä
2017-06-14 17:34 ` Paulo Zanoni
2017-06-14 17:38 ` Rodrigo Vivi
0 siblings, 2 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-06-14 15:16 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Tue, Jun 13, 2017 at 04:33:50PM -0300, Paulo Zanoni wrote:
> Do it just like we do with _PICK and _PICK3, so our code looks a
> little more uniform.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a97af4a..3fb7b63 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -49,13 +49,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> }
>
> #define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a)))
> +#define _PICK2(__index, __offsets, a) (__offsets[__index] - __offsets[0] + \
> + (a) + dev_priv->info.display_mmio_offset)
> #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
>
> #define _PIPE(pipe, a, b) _PICK(pipe, a, b)
> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
> -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \
> - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
> - dev_priv->info.display_mmio_offset)
> +#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv->info.pipe_offsets, \
> + reg))
> #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
>
> #define _PLANE(plane, a, b) _PICK(plane, a, b)
> @@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define _TRANS(tran, a, b) _PICK(tran, a, b)
> #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
> -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \
> - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
> - dev_priv->info.display_mmio_offset)
> +#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \
> + dev_priv->info.trans_offsets, reg))
No love for cursor/palette offsets?
>
> #define _PORT(port, a, b) _PICK(port, a, b)
> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
> --
> 2.9.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/7] drm/i915: reorder the register picking macros
2017-06-13 19:33 ` [PATCH 1/7] drm/i915: reorder " Paulo Zanoni
@ 2017-06-14 17:22 ` Rodrigo Vivi
0 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Vivi @ 2017-06-14 17:22 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
On Tue, Jun 13, 2017 at 12:33 PM, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> We currently have pipe, plane, trans, port, pll and phy versions of
> these macros. Reorder their definitions so all macros of each type are
> in their own group, separated by blank lines.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 88e4707..a1d3cca 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -52,19 +52,24 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
> +#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> +
> #define _PLANE(plane, a, b) _PIPE(plane, a, b)
> #define _MMIO_PLANE(plane, a, b) _MMIO_PIPE(plane, a, b)
> +
> #define _TRANS(tran, a, b) ((a) + (tran)*((b)-(a)))
> #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
> +
> #define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, 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 _PLL(pll, a, b) ((a) + (pll)*((b)-(a)))
> -#define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
> #define _MMIO_PORT6(port, a, b, c, d, e, f) _MMIO(_PICK(port, a, b, c, d, e, f))
> #define _MMIO_PORT6_LN(port, ln, a0, a1, b, c, d, e, f) \
> _MMIO(_PICK(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
> +
> +#define _PLL(pll, a, b) ((a) + (pll)*((b)-(a)))
> +#define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
> +
> #define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
> #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
>
> --
> 2.9.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/7] drm/i915: _MMIO_PORT3 takes a port as an argument
2017-06-13 19:33 ` [PATCH 2/7] drm/i915: _MMIO_PORT3 takes a port as an argument Paulo Zanoni
@ 2017-06-14 17:23 ` Rodrigo Vivi
0 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Vivi @ 2017-06-14 17:23 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
Although I'd prefer to kill it, this patch is right, so
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
On Tue, Jun 13, 2017 at 12:33 PM, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> The macro takes a port as an argument, not a pipe.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a1d3cca..1983b75 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -62,7 +62,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
> -#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> +#define _MMIO_PORT3(port, a, b, c) _MMIO(_PICK(port, a, b, c))
> #define _MMIO_PORT6(port, a, b, c, d, e, f) _MMIO(_PICK(port, a, b, c, d, e, f))
> #define _MMIO_PORT6_LN(port, ln, a0, a1, b, c, d, e, f) \
> _MMIO(_PICK(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
> --
> 2.9.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/7] drm/i915: use variable arguments for the macros that call _PICK
2017-06-13 19:33 ` [PATCH 3/7] drm/i915: use variable arguments for the macros that call _PICK Paulo Zanoni
@ 2017-06-14 17:25 ` Rodrigo Vivi
0 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Vivi @ 2017-06-14 17:25 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Tue, Jun 13, 2017 at 12:33 PM, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> There's no need to create a new macro every time the number of
> parameters change.
fully agree
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1983b75..1d5a0f8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -52,7 +52,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
> -#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c))
> +#define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK(pipe, __VA_ARGS__))
>
> #define _PLANE(plane, a, b) _PIPE(plane, a, b)
> #define _MMIO_PLANE(plane, a, b) _MMIO_PIPE(plane, a, b)
> @@ -62,8 +62,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
> -#define _MMIO_PORT3(port, a, b, c) _MMIO(_PICK(port, a, b, c))
> -#define _MMIO_PORT6(port, a, b, c, d, e, f) _MMIO(_PICK(port, a, b, c, d, e, f))
> +#define _MMIO_PORT3(port, ...) _MMIO(_PICK(port, __VA_ARGS__))
> +#define _MMIO_PORT6(port, ...) _MMIO(_PICK(port, __VA_ARGS__))
so, what about unifying MMIO_PORT3 and MMIO_PORT6 at least?
MMIO_PORTS, MMIO_PICK?
> #define _MMIO_PORT6_LN(port, ln, a0, a1, b, c, d, e, f) \
> _MMIO(_PICK(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
>
> @@ -71,7 +71,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
>
> #define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
> -#define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
> +#define _MMIO_PHY3(phy, ...) _MMIO(_PHY3(phy, __VA_ARGS__))
>
> #define _MASKED_FIELD(mask, value) ({ \
> if (__builtin_constant_p(mask)) \
> --
> 2.9.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 7/7] drm/i915: extract a _PICK2 macro
2017-06-14 15:16 ` Ville Syrjälä
@ 2017-06-14 17:34 ` Paulo Zanoni
2017-06-14 17:38 ` Rodrigo Vivi
1 sibling, 0 replies; 20+ messages in thread
From: Paulo Zanoni @ 2017-06-14 17:34 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Em Qua, 2017-06-14 às 18:16 +0300, Ville Syrjälä escreveu:
> On Tue, Jun 13, 2017 at 04:33:50PM -0300, Paulo Zanoni wrote:
> > Do it just like we do with _PICK and _PICK3, so our code looks a
> > little more uniform.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index a97af4a..3fb7b63 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -49,13 +49,14 @@ static inline bool
> > i915_mmio_reg_valid(i915_reg_t reg)
> > }
> >
> > #define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a)))
> > +#define _PICK2(__index, __offsets, a) (__offsets[__index] -
> > __offsets[0] + \
> > + (a) + dev_priv-
> > >info.display_mmio_offset)
> > #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__
> > })[__index])
> >
> > #define _PIPE(pipe, a, b) _PICK(pipe, a, b)
> > #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
> > -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv-
> > >info.pipe_offsets[pipe] - \
> > - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
> > - dev_priv->info.display_mmio_offset)
> > +#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv-
> > >info.pipe_offsets, \
> > + reg))
> > #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
> >
> > #define _PLANE(plane, a, b) _PICK(plane, a, b)
> > @@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t
> > reg)
> >
> > #define _TRANS(tran, a, b) _PICK(tran, a, b)
> > #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
> > -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv-
> > >info.trans_offsets[(pipe)] - \
> > - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
> > - dev_priv->info.display_mmio_offset)
> > +#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \
> > + dev_priv-
> > >info.trans_offsets, reg))
>
> No love for cursor/palette offsets?
I missed them. _CURSOR definitely fits the model. I'll see what we can
do with PALLETTE.
>
> >
> > #define _PORT(port, a, b) _PICK(port, a, b)
> > #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
> > --
> > 2.9.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 7/7] drm/i915: extract a _PICK2 macro
2017-06-14 15:16 ` Ville Syrjälä
2017-06-14 17:34 ` Paulo Zanoni
@ 2017-06-14 17:38 ` Rodrigo Vivi
2017-06-15 20:33 ` Jani Nikula
1 sibling, 1 reply; 20+ messages in thread
From: Rodrigo Vivi @ 2017-06-14 17:38 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Paulo Zanoni
On Wed, Jun 14, 2017 at 8:16 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Tue, Jun 13, 2017 at 04:33:50PM -0300, Paulo Zanoni wrote:
>> Do it just like we do with _PICK and _PICK3, so our code looks a
>> little more uniform.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index a97af4a..3fb7b63 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -49,13 +49,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>> }
>>
yeap, as you predicted on the cover letter here starts the name bikeshedings ;)
>> #define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a)))
for me this is more like a "guess" based on the gap, but specially
I have concern about reusing the name that was for a totally different use.
PICK before this list is a "select from a list" and after this series is
"select base on uniform gap"
>> +#define _PICK2(__index, __offsets, a) (__offsets[__index] - __offsets[0] + \
>> + (a) + dev_priv->info.display_mmio_offset)
and this is a select based on common base offset.
>> #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
and this is actual pick from a list
also I don't like the numbers as versions of the macro...
I'd prefer to use just in number of arguments as before.
so, what about:
s/PICK(/PICK_GAP
s/PICK2/PICK_OFFSET
s/PICK3/PICK_LIST
?
or
s/PICK(/SELECT_GAP
s/PICK2/SELECT_OFFSET
s/PICK3/SELECT_LIST
?
>>
>> #define _PIPE(pipe, a, b) _PICK(pipe, a, b)
>> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
>> -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \
>> - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
>> - dev_priv->info.display_mmio_offset)
>> +#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv->info.pipe_offsets, \
>> + reg))
>> #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
>>
>> #define _PLANE(plane, a, b) _PICK(plane, a, b)
>> @@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>>
>> #define _TRANS(tran, a, b) _PICK(tran, a, b)
>> #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
>> -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \
>> - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
>> - dev_priv->info.display_mmio_offset)
>> +#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \
>> + dev_priv->info.trans_offsets, reg))
>
> No love for cursor/palette offsets?
>
>>
>> #define _PORT(port, a, b) _PICK(port, a, b)
>> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
>> --
>> 2.9.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/7] drm/i915: rename _PICK to _PICK3
2017-06-13 19:33 ` [PATCH 4/7] drm/i915: rename _PICK to _PICK3 Paulo Zanoni
@ 2017-06-15 20:22 ` Jani Nikula
0 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2017-06-15 20:22 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
On Tue, 13 Jun 2017, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> All of the macros that call _PICK are named _X_3, so let's rename
> _PICK to _PICK3. The reason we're doing this is because we're going to
> have _PICK and _PICK2. Consider _PICK3 as the third variation of the
> PICK macros (well, actually it *is* the third variation...).
I named it PICK because it "picks" one of a list, i.e. it doesn't
compute anything. I guess I'd like to keep the name.
BR,
Jani.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 33 ++++++++++++++++-----------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1d5a0f8..d271098 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -48,11 +48,11 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
> }
>
> -#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
> +#define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
>
> #define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
> -#define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK(pipe, __VA_ARGS__))
> +#define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
>
> #define _PLANE(plane, a, b) _PIPE(plane, a, b)
> #define _MMIO_PLANE(plane, a, b) _MMIO_PIPE(plane, a, b)
> @@ -62,15 +62,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
> -#define _MMIO_PORT3(port, ...) _MMIO(_PICK(port, __VA_ARGS__))
> -#define _MMIO_PORT6(port, ...) _MMIO(_PICK(port, __VA_ARGS__))
> -#define _MMIO_PORT6_LN(port, ln, a0, a1, b, c, d, e, f) \
> - _MMIO(_PICK(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
> +#define _MMIO_PORT3(port, ...) _MMIO(_PICK3(port, __VA_ARGS__))
> +#define _MMIO_PORT3_LN(port, ln, a0, a1, b, c, d, e, f) \
> + _MMIO(_PICK3(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
>
> #define _PLL(pll, a, b) ((a) + (pll)*((b)-(a)))
> #define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
>
> -#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
> +#define _PHY3(phy, ...) _PICK3(phy, __VA_ARGS__)
> #define _MMIO_PHY3(phy, ...) _MMIO(_PHY3(phy, __VA_ARGS__))
>
> #define _MASKED_FIELD(mask, value) ({ \
> @@ -1709,14 +1708,14 @@ enum skl_disp_power_wells {
> #define _CNL_PORT_PCS_DW1_LN0_C 0x162C04
> #define _CNL_PORT_PCS_DW1_LN0_D 0x162E04
> #define _CNL_PORT_PCS_DW1_LN0_F 0x162804
> -#define CNL_PORT_PCS_DW1_GRP(port) _MMIO_PORT6(port, \
> +#define CNL_PORT_PCS_DW1_GRP(port) _MMIO_PORT3(port, \
> _CNL_PORT_PCS_DW1_GRP_AE, \
> _CNL_PORT_PCS_DW1_GRP_B, \
> _CNL_PORT_PCS_DW1_GRP_C, \
> _CNL_PORT_PCS_DW1_GRP_D, \
> _CNL_PORT_PCS_DW1_GRP_AE, \
> _CNL_PORT_PCS_DW1_GRP_F)
> -#define CNL_PORT_PCS_DW1_LN0(port) _MMIO_PORT6(port, \
> +#define CNL_PORT_PCS_DW1_LN0(port) _MMIO_PORT3(port, \
> _CNL_PORT_PCS_DW1_LN0_AE, \
> _CNL_PORT_PCS_DW1_LN0_B, \
> _CNL_PORT_PCS_DW1_LN0_C, \
> @@ -1735,14 +1734,14 @@ enum skl_disp_power_wells {
> #define _CNL_PORT_TX_DW2_LN0_C 0x162C48
> #define _CNL_PORT_TX_DW2_LN0_D 0x162E48
> #define _CNL_PORT_TX_DW2_LN0_F 0x162A48
> -#define CNL_PORT_TX_DW2_GRP(port) _MMIO_PORT6(port, \
> +#define CNL_PORT_TX_DW2_GRP(port) _MMIO_PORT3(port, \
> _CNL_PORT_TX_DW2_GRP_AE, \
> _CNL_PORT_TX_DW2_GRP_B, \
> _CNL_PORT_TX_DW2_GRP_C, \
> _CNL_PORT_TX_DW2_GRP_D, \
> _CNL_PORT_TX_DW2_GRP_AE, \
> _CNL_PORT_TX_DW2_GRP_F)
> -#define CNL_PORT_TX_DW2_LN0(port) _MMIO_PORT6(port, \
> +#define CNL_PORT_TX_DW2_LN0(port) _MMIO_PORT3(port, \
> _CNL_PORT_TX_DW2_LN0_AE, \
> _CNL_PORT_TX_DW2_LN0_B, \
> _CNL_PORT_TX_DW2_LN0_C, \
> @@ -1764,14 +1763,14 @@ enum skl_disp_power_wells {
> #define _CNL_PORT_TX_DW4_LN0_C 0x162C50
> #define _CNL_PORT_TX_DW4_LN0_D 0x162E50
> #define _CNL_PORT_TX_DW4_LN0_F 0x162850
> -#define CNL_PORT_TX_DW4_GRP(port) _MMIO_PORT6(port, \
> +#define CNL_PORT_TX_DW4_GRP(port) _MMIO_PORT3(port, \
> _CNL_PORT_TX_DW4_GRP_AE, \
> _CNL_PORT_TX_DW4_GRP_B, \
> _CNL_PORT_TX_DW4_GRP_C, \
> _CNL_PORT_TX_DW4_GRP_D, \
> _CNL_PORT_TX_DW4_GRP_AE, \
> _CNL_PORT_TX_DW4_GRP_F)
> -#define CNL_PORT_TX_DW4_LN(port, ln) _MMIO_PORT6_LN(port, ln, \
> +#define CNL_PORT_TX_DW4_LN(port, ln) _MMIO_PORT3_LN(port, ln, \
> _CNL_PORT_TX_DW4_LN0_AE, \
> _CNL_PORT_TX_DW4_LN1_AE, \
> _CNL_PORT_TX_DW4_LN0_B, \
> @@ -1794,14 +1793,14 @@ enum skl_disp_power_wells {
> #define _CNL_PORT_TX_DW5_LN0_C 0x162C54
> #define _CNL_PORT_TX_DW5_LN0_D 0x162ED4
> #define _CNL_PORT_TX_DW5_LN0_F 0x162854
> -#define CNL_PORT_TX_DW5_GRP(port) _MMIO_PORT6(port, \
> +#define CNL_PORT_TX_DW5_GRP(port) _MMIO_PORT3(port, \
> _CNL_PORT_TX_DW5_GRP_AE, \
> _CNL_PORT_TX_DW5_GRP_B, \
> _CNL_PORT_TX_DW5_GRP_C, \
> _CNL_PORT_TX_DW5_GRP_D, \
> _CNL_PORT_TX_DW5_GRP_AE, \
> _CNL_PORT_TX_DW5_GRP_F)
> -#define CNL_PORT_TX_DW5_LN0(port) _MMIO_PORT6(port, \
> +#define CNL_PORT_TX_DW5_LN0(port) _MMIO_PORT3(port, \
> _CNL_PORT_TX_DW5_LN0_AE, \
> _CNL_PORT_TX_DW5_LN0_B, \
> _CNL_PORT_TX_DW5_LN0_C, \
> @@ -1823,14 +1822,14 @@ enum skl_disp_power_wells {
> #define _CNL_PORT_TX_DW7_LN0_C 0x162C5C
> #define _CNL_PORT_TX_DW7_LN0_D 0x162EDC
> #define _CNL_PORT_TX_DW7_LN0_F 0x16285C
> -#define CNL_PORT_TX_DW7_GRP(port) _MMIO_PORT6(port, \
> +#define CNL_PORT_TX_DW7_GRP(port) _MMIO_PORT3(port, \
> _CNL_PORT_TX_DW7_GRP_AE, \
> _CNL_PORT_TX_DW7_GRP_B, \
> _CNL_PORT_TX_DW7_GRP_C, \
> _CNL_PORT_TX_DW7_GRP_D, \
> _CNL_PORT_TX_DW7_GRP_AE, \
> _CNL_PORT_TX_DW7_GRP_F)
> -#define CNL_PORT_TX_DW7_LN0(port) _MMIO_PORT6(port, \
> +#define CNL_PORT_TX_DW7_LN0(port) _MMIO_PORT3(port, \
> _CNL_PORT_TX_DW7_LN0_AE, \
> _CNL_PORT_TX_DW7_LN0_B, \
> _CNL_PORT_TX_DW7_LN0_C, \
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros
2017-06-13 19:33 ` [PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros Paulo Zanoni
@ 2017-06-15 20:26 ` Jani Nikula
2017-06-19 10:14 ` Chauhan, Madhav
0 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2017-06-15 20:26 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
On Tue, 13 Jun 2017, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Instead of duplicating the macro everywhere, add a single definition
> for it and call it just like we do with the _PICK3 macros.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d271098..3e46ba1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -48,25 +48,26 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
> }
>
> +#define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a)))
The change overall is good but I'd name this in a way that emphasizes
the registers are evenly spaced. _LINEAR?
BR,
Jani.
> #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
>
> -#define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
> +#define _PIPE(pipe, a, b) _PICK(pipe, a, b)
> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
> #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
>
> -#define _PLANE(plane, a, b) _PIPE(plane, a, b)
> -#define _MMIO_PLANE(plane, a, b) _MMIO_PIPE(plane, a, b)
> +#define _PLANE(plane, a, b) _PICK(plane, a, b)
> +#define _MMIO_PLANE(plane, a, b) _MMIO(_PLANE(plane, a, b))
>
> -#define _TRANS(tran, a, b) ((a) + (tran)*((b)-(a)))
> +#define _TRANS(tran, a, b) _PICK(tran, a, b)
> #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
>
> -#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
> +#define _PORT(port, a, b) _PICK(port, a, b)
> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
> #define _MMIO_PORT3(port, ...) _MMIO(_PICK3(port, __VA_ARGS__))
> #define _MMIO_PORT3_LN(port, ln, a0, a1, b, c, d, e, f) \
> _MMIO(_PICK3(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
>
> -#define _PLL(pll, a, b) ((a) + (pll)*((b)-(a)))
> +#define _PLL(pll, a, b) _PICK(pll, a, b)
> #define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
>
> #define _PHY3(phy, ...) _PICK3(phy, __VA_ARGS__)
> @@ -6415,7 +6416,7 @@ enum {
> #define _PS_ECC_STAT_2B 0x68AD0
> #define _PS_ECC_STAT_1C 0x691D0
>
> -#define _ID(id, a, b) ((a) + (id)*((b)-(a)))
> +#define _ID(id, a, b) _PICK(id, a, b)
> #define SKL_PS_CTRL(pipe, id) _MMIO_PIPE(pipe, \
> _ID(id, _PS_1A_CTRL, _PS_2A_CTRL), \
> _ID(id, _PS_1B_CTRL, _PS_2B_CTRL))
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 7/7] drm/i915: extract a _PICK2 macro
2017-06-14 17:38 ` Rodrigo Vivi
@ 2017-06-15 20:33 ` Jani Nikula
0 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2017-06-15 20:33 UTC (permalink / raw)
To: Rodrigo Vivi, Ville Syrjälä; +Cc: intel-gfx, Paulo Zanoni
On Wed, 14 Jun 2017, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> On Wed, Jun 14, 2017 at 8:16 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>> On Tue, Jun 13, 2017 at 04:33:50PM -0300, Paulo Zanoni wrote:
>>> Do it just like we do with _PICK and _PICK3, so our code looks a
>>> little more uniform.
>>>
>>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------
>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index a97af4a..3fb7b63 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -49,13 +49,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>>> }
>>>
>
> yeap, as you predicted on the cover letter here starts the name bikeshedings ;)
>
>>> #define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a)))
>
> for me this is more like a "guess" based on the gap, but specially
> I have concern about reusing the name that was for a totally different use.
> PICK before this list is a "select from a list" and after this series is
> "select base on uniform gap"
>
>>> +#define _PICK2(__index, __offsets, a) (__offsets[__index] - __offsets[0] + \
>>> + (a) + dev_priv->info.display_mmio_offset)
>
> and this is a select based on common base offset.
>
>>> #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
>
> and this is actual pick from a list
>
> also I don't like the numbers as versions of the macro...
> I'd prefer to use just in number of arguments as before.
>
> so, what about:
>
> s/PICK(/PICK_GAP
> s/PICK2/PICK_OFFSET
> s/PICK3/PICK_LIST
Just _LINEAR, _ARRAY, _PICK.
Or PICK_LINEAR, PICK_ARRAY, PICK_VA (or PICK_ARGS).
BR,
Jani.
>
> ?
>
> or
>
> s/PICK(/SELECT_GAP
> s/PICK2/SELECT_OFFSET
> s/PICK3/SELECT_LIST
>
> ?
>
>>>
>>> #define _PIPE(pipe, a, b) _PICK(pipe, a, b)
>>> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
>>> -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \
>>> - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
>>> - dev_priv->info.display_mmio_offset)
>>> +#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv->info.pipe_offsets, \
>>> + reg))
>>> #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
>>>
>>> #define _PLANE(plane, a, b) _PICK(plane, a, b)
>>> @@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>>>
>>> #define _TRANS(tran, a, b) _PICK(tran, a, b)
>>> #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
>>> -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \
>>> - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
>>> - dev_priv->info.display_mmio_offset)
>>> +#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \
>>> + dev_priv->info.trans_offsets, reg))
>>
>> No love for cursor/palette offsets?
>>
>>>
>>> #define _PORT(port, a, b) _PICK(port, a, b)
>>> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
>>> --
>>> 2.9.4
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Ville Syrjälä
>> Intel OTC
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 6/7] drm/i915: also move version 2 of the register picking macros up
2017-06-13 19:33 ` [PATCH 6/7] drm/i915: also move version 2 of the register picking macros up Paulo Zanoni
@ 2017-06-15 20:37 ` Jani Nikula
0 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2017-06-15 20:37 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
On Tue, 13 Jun 2017, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Make sure all the macros are next to each other so it's easier to spot
> all the options available.
Please also move _MIPI_PORT and _MMIO_MIPI up. Btw they're another
variant...
BR,
Jani.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3e46ba1..a97af4a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -53,6 +53,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define _PIPE(pipe, a, b) _PICK(pipe, a, b)
> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
> +#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \
> + dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
> + dev_priv->info.display_mmio_offset)
> #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
>
> #define _PLANE(plane, a, b) _PICK(plane, a, b)
> @@ -60,6 +63,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>
> #define _TRANS(tran, a, b) _PICK(tran, a, b)
> #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
> +#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \
> + dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
> + dev_priv->info.display_mmio_offset)
>
> #define _PORT(port, a, b) _PICK(port, a, b)
> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
> @@ -3700,10 +3706,6 @@ enum {
> #define CHV_TRANSCODER_C_OFFSET 0x63000
> #define TRANSCODER_EDP_OFFSET 0x6f000
>
> -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \
> - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
> - dev_priv->info.display_mmio_offset)
> -
> #define HTOTAL(trans) _MMIO_TRANS2(trans, _HTOTAL_A)
> #define HBLANK(trans) _MMIO_TRANS2(trans, _HBLANK_A)
> #define HSYNC(trans) _MMIO_TRANS2(trans, _HSYNC_A)
> @@ -5189,10 +5191,6 @@ enum {
> */
> #define PIPE_EDP_OFFSET 0x7f000
>
> -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \
> - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
> - dev_priv->info.display_mmio_offset)
> -
> #define PIPECONF(pipe) _MMIO_PIPE2(pipe, _PIPEACONF)
> #define PIPEDSL(pipe) _MMIO_PIPE2(pipe, _PIPEADSL)
> #define PIPEFRAME(pipe) _MMIO_PIPE2(pipe, _PIPEAFRAMEHIGH)
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros
2017-06-15 20:26 ` Jani Nikula
@ 2017-06-19 10:14 ` Chauhan, Madhav
0 siblings, 0 replies; 20+ messages in thread
From: Chauhan, Madhav @ 2017-06-19 10:14 UTC (permalink / raw)
To: 'Jani Nikula', intel-gfx@lists.freedesktop.org; +Cc: Zanoni, Paulo R
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Friday, June 16, 2017 1:56 AM
> To: Zanoni, Paulo R <paulo.r.zanoni@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Zanoni, Paulo R <paulo.r.zanoni@intel.com>
> Subject: Re: [Intel-gfx] [PATCH 5/7] drm/i915: add _PICK macro for the "a +
> index * (b - a)" macros
>
> On Tue, 13 Jun 2017, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> > Instead of duplicating the macro everywhere, add a single definition
> > for it and call it just like we do with the _PICK3 macros.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 15 ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index d271098..3e46ba1 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -48,25 +48,26 @@ static inline bool i915_mmio_reg_valid(i915_reg_t
> reg)
> > return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG); }
> >
> > +#define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a)))
>
> The change overall is good but I'd name this in a way that emphasizes the
> registers are evenly spaced. _LINEAR?
Also as we discussed, will publish the changes for handling scenario
when a>b. Currently it will work only when b>a.
Regards,
Madhav
>
> BR,
> Jani.
>
> > #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__
> > })[__index])
> >
> > -#define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
> > +#define _PIPE(pipe, a, b) _PICK(pipe, a, b)
> > #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b)) #define
> > _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__))
> >
> > -#define _PLANE(plane, a, b) _PIPE(plane, a, b) -#define
> > _MMIO_PLANE(plane, a, b) _MMIO_PIPE(plane, a, b)
> > +#define _PLANE(plane, a, b) _PICK(plane, a, b) #define
> > +_MMIO_PLANE(plane, a, b) _MMIO(_PLANE(plane, a, b))
> >
> > -#define _TRANS(tran, a, b) ((a) + (tran)*((b)-(a)))
> > +#define _TRANS(tran, a, b) _PICK(tran, a, b)
> > #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
> >
> > -#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
> > +#define _PORT(port, a, b) _PICK(port, a, b)
> > #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b)) #define
> > _MMIO_PORT3(port, ...) _MMIO(_PICK3(port, __VA_ARGS__))
> > #define _MMIO_PORT3_LN(port, ln, a0, a1, b, c, d, e, f)
> \
> > _MMIO(_PICK3(port, a0, b, c, d, e, f) + (ln * (a1 - a0)))
> >
> > -#define _PLL(pll, a, b) ((a) + (pll)*((b)-(a)))
> > +#define _PLL(pll, a, b) _PICK(pll, a, b)
> > #define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b))
> >
> > #define _PHY3(phy, ...) _PICK3(phy, __VA_ARGS__) @@ -6415,7 +6416,7
> > @@ enum {
> > #define _PS_ECC_STAT_2B 0x68AD0
> > #define _PS_ECC_STAT_1C 0x691D0
> >
> > -#define _ID(id, a, b) ((a) + (id)*((b)-(a)))
> > +#define _ID(id, a, b) _PICK(id, a, b)
> > #define SKL_PS_CTRL(pipe, id) _MMIO_PIPE(pipe, \
> > _ID(id, _PS_1A_CTRL, _PS_2A_CTRL), \
> > _ID(id, _PS_1B_CTRL, _PS_2B_CTRL))
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-06-19 10:14 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-13 19:33 [PATCH 0/7] Reorganize the register picking macros Paulo Zanoni
2017-06-13 19:33 ` [PATCH 1/7] drm/i915: reorder " Paulo Zanoni
2017-06-14 17:22 ` Rodrigo Vivi
2017-06-13 19:33 ` [PATCH 2/7] drm/i915: _MMIO_PORT3 takes a port as an argument Paulo Zanoni
2017-06-14 17:23 ` Rodrigo Vivi
2017-06-13 19:33 ` [PATCH 3/7] drm/i915: use variable arguments for the macros that call _PICK Paulo Zanoni
2017-06-14 17:25 ` Rodrigo Vivi
2017-06-13 19:33 ` [PATCH 4/7] drm/i915: rename _PICK to _PICK3 Paulo Zanoni
2017-06-15 20:22 ` Jani Nikula
2017-06-13 19:33 ` [PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros Paulo Zanoni
2017-06-15 20:26 ` Jani Nikula
2017-06-19 10:14 ` Chauhan, Madhav
2017-06-13 19:33 ` [PATCH 6/7] drm/i915: also move version 2 of the register picking macros up Paulo Zanoni
2017-06-15 20:37 ` Jani Nikula
2017-06-13 19:33 ` [PATCH 7/7] drm/i915: extract a _PICK2 macro Paulo Zanoni
2017-06-14 15:16 ` Ville Syrjälä
2017-06-14 17:34 ` Paulo Zanoni
2017-06-14 17:38 ` Rodrigo Vivi
2017-06-15 20:33 ` Jani Nikula
2017-06-13 19:49 ` ✓ Fi.CI.BAT: success for Reorganize the register picking macros Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox