[PATCH 0/7] Reorganize the register picking macros

[PATCH 0/7] Reorganize the register picking macros

[Paulo Zanoni]

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

[PATCH 1/7] drm/i915: reorder the register picking macros

[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

Re: [PATCH 1/7] drm/i915: reorder the register picking macros

[Rodrigo Vivi]

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

[PATCH 2/7] drm/i915: _MMIO_PORT3 takes a port as an argument

[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

Re: [PATCH 2/7] drm/i915: _MMIO_PORT3 takes a port as an argument

[Rodrigo Vivi]

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

[PATCH 3/7] drm/i915: use variable arguments for the macros that call _PICK

[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

Re: [PATCH 3/7] drm/i915: use variable arguments for the macros that call _PICK

[Rodrigo Vivi]

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

[PATCH 4/7] drm/i915: rename _PICK to _PICK3

[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

Re: [PATCH 4/7] drm/i915: rename _PICK to _PICK3

[Jani Nikula]

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

[PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros

[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

Re: [PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros

[Jani Nikula]

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

Re: [PATCH 5/7] drm/i915: add _PICK macro for the "a + index * (b - a)" macros

[Chauhan, Madhav]

> -----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

[PATCH 6/7] drm/i915: also move version 2 of the register picking macros up

[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

Re: [PATCH 6/7] drm/i915: also move version 2 of the register picking macros up

[Jani Nikula]

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

[PATCH 7/7] drm/i915: extract a _PICK2 macro

[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

Re: [PATCH 7/7] drm/i915: extract a _PICK2 macro

[Ville Syrjälä]

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

Re: [PATCH 7/7] drm/i915: extract a _PICK2 macro

[Paulo Zanoni]

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

Re: [PATCH 7/7] drm/i915: extract a _PICK2 macro

[Rodrigo Vivi]

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

Re: [PATCH 7/7] drm/i915: extract a _PICK2 macro

[Jani Nikula]

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

✓ Fi.CI.BAT: success for Reorganize the register picking macros

[Patchwork]

== 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