Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

2023-01-23 Thread Lucas De Marchi

(I missed this review you did before I had sent a v2.1, I will incorporate
what is missing in the next version)

On Mon, Jan 23, 2023 at 12:38:28PM +0200, Jani Nikula wrote:

On Fri, 20 Jan 2023, Lucas De Marchi  wrote:

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

1) It increases the code size since the array is declared
   in each call site


Would be interesting to see what this does, and whether the compiler has
the smarts to combine these within each file:

-#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
+#define _PICK(__index, ...) (((static const u32 []){ __VA_ARGS__ })[__index])


if the compiler is smart, it would be at least 1 per compilation unit.
gcc doesn't seem smart enough to even compile it though:

../drivers/gpu/drm/i915/i915_reg_defs.h:155:52: error: expected ‘)’ before ‘{’ token   
  155 | #define _PICK(__index, ...) (((static const u32 []){ __VA_ARGS__ })[__index])  
  |  ~ ^


What I'm thinking for the remaining uses of _PICK() is to be explicit
and statically define them in a good .c depending on the use.
Then use that in the macro.




2) Developers need to be careful not to incur an
   out-of-bounds array access
3) Developers need to be careful that the indexes match the
   table. For that it may be that the table needs to contain
   holes, making (1) even worse.

Add a variant of _PICK_EVEN() that works with 2 ranges and selects which
one to use depending on the index value.

Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/i915_reg_defs.h | 28 
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h 
b/drivers/gpu/drm/i915/i915_reg_defs.h
index be43580a6979..b7ec87464d69 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -119,6 +119,34 @@
  */
 #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))

+/*
+ * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
+ * The first range is used for indexes below @__c_index, and the second
+ * range is used for anything above it. Example::


I'd like this to be clear about which range is used for index ==
__c_index, instead of saying "below" and "above".

No need for the double colon :: because this isn't a kernel-doc comment.


ok, what about this?

 * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
 * @__c_index corresponds to the index in which the second range starts
 * to be used. Using math interval notation, the first range is used
 * for indexes [ 0, @__c_index), while the second range is used for
 * [ @__c_index, ... ). Example:





+ *
+ * #define _FOO_A  0xf000
+ * #define _FOO_B  0xf004
+ * #define _FOO_C  0xf008
+ * #define _SUPER_FOO_A0xa000
+ * #define _SUPER_FOO_B0xa100
+ * #define FOO(x)  _MMIO(_PICK_EVEN_RANGES(x, 3,   
\


The example uses a different name for the macro.


yeah, that was the previous name I was using... good catch, will fix.




+ *   _FOO_A, _FOO_B,   
\
+ *   _SUPER_FOO_A, _SUPER_FOO_B))
+ *
+ * This expands to:
+ * 0: 0xf000,
+ * 1: 0xf004,
+ * 2: 0xf008,
+ * 3: 0xa100,


With the above definitions, this would be 3: 0xa000.


fixed


thanks
Lucas De Marchi




+ * 4: 0xa200,
+ * 5: 0xa300,
+ * ...
+ */
+#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) 
\
+   (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
\
+((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :  \
+  _PICK_EVEN((__index) - (__c_index), __c, 
__d)))
+
 /*
  * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
  *


--
Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

2023-01-23 Thread Lucas De Marchi

On Mon, Jan 23, 2023 at 08:15:16AM -0800, Anusha Srivatsa wrote:




-Original Message-
From: Jani Nikula 
Sent: Monday, January 23, 2023 3:01 AM
To: De Marchi, Lucas ; Srivatsa, Anusha

Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

On Sat, 21 Jan 2023, Lucas De Marchi  wrote:
> On Fri, Jan 20, 2023 at 10:14:19PM -0800, Anusha Srivatsa wrote:
>>
>>
>>> -Original Message-
>>> From: Intel-gfx  On Behalf
>>> Of Lucas De Marchi
>>> Sent: Friday, January 20, 2023 11:35 AM
>>> To: intel-gfx@lists.freedesktop.org
>>> Cc: De Marchi, Lucas ; dri-
>>> de...@lists.freedesktop.org
>>> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add
>>> _PICK_EVEN_2RANGES()
>>>
>>> It's a constant pattern in the driver to need to use 2 ranges of
>>> MMIOs based on port, phy, pll, etc. When that happens, instead of
>>> using _PICK_EVEN(), _PICK() needs to be used.  Using _PICK() is discouraged
due to some reasons like:
>>>
>>> 1) It increases the code size since the array is declared
>>>in each call site
>>> 2) Developers need to be careful not to incur an
>>>out-of-bounds array access
>>> 3) Developers need to be careful that the indexes match the
>>>table. For that it may be that the table needs to contain
>>>holes, making (1) even worse.
>>>
>>> Add a variant of _PICK_EVEN() that works with 2 ranges and selects
>>> which one to use depending on the index value.
>>>
>>> Signed-off-by: Lucas De Marchi 
>>> ---
>>>  drivers/gpu/drm/i915/i915_reg_defs.h | 28
>>> 
>>>  1 file changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
>>> b/drivers/gpu/drm/i915/i915_reg_defs.h
>>> index be43580a6979..b7ec87464d69 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
>>> @@ -119,6 +119,34 @@
>>>   */
>>>  #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) -
>>> (__a)))
>>>
>>> +/*
>>> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address
offsets.
>>> + * The first range is used for indexes below @__c_index, and the
>>> +second
>>> + * range is used for anything above it. Example::
>>> + *
>>> + * #define _FOO_A 0xf000
>>> + * #define _FOO_B 0xf004
>>> + * #define _FOO_C 0xf008
>>> + * #define _SUPER_FOO_A   0xa000
>>> + * #define _SUPER_FOO_B   0xa100
>>> + * #define FOO(x) _MMIO(_PICK_EVEN_RANGES(x, 3,
>>>\
>>> + *  _FOO_A, _FOO_B,
>>>\
>>> + *  _SUPER_FOO_A, 
_SUPER_FOO_B))
>>> + *
>>> + * This expands to:
>>> + *0: 0xf000,
>>> + *1: 0xf004,
>>> + *2: 0xf008,
>>> + *3: 0xa100,
>>You mean 3:0xa000
>
> doesn't really matter. This is an example of register addresses. They
> don't need to start from 0, it's whatever the hw gives us.

I think the point is that the example is inconsistent between _SUPER_FOO_A and
"3: 0xa100".


Yes. It's the inconsistency with _SUPER_FOO_A that I was pointing out.


ah, ok. I will send a fixup to this patch.

thanks
Lucas De Marchi


Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

2023-01-23 Thread Srivatsa, Anusha



> -Original Message-
> From: Jani Nikula 
> Sent: Monday, January 23, 2023 3:01 AM
> To: De Marchi, Lucas ; Srivatsa, Anusha
> 
> Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
> 
> On Sat, 21 Jan 2023, Lucas De Marchi  wrote:
> > On Fri, Jan 20, 2023 at 10:14:19PM -0800, Anusha Srivatsa wrote:
> >>
> >>
> >>> -Original Message-
> >>> From: Intel-gfx  On Behalf
> >>> Of Lucas De Marchi
> >>> Sent: Friday, January 20, 2023 11:35 AM
> >>> To: intel-gfx@lists.freedesktop.org
> >>> Cc: De Marchi, Lucas ; dri-
> >>> de...@lists.freedesktop.org
> >>> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add
> >>> _PICK_EVEN_2RANGES()
> >>>
> >>> It's a constant pattern in the driver to need to use 2 ranges of
> >>> MMIOs based on port, phy, pll, etc. When that happens, instead of
> >>> using _PICK_EVEN(), _PICK() needs to be used.  Using _PICK() is 
> >>> discouraged
> due to some reasons like:
> >>>
> >>> 1) It increases the code size since the array is declared
> >>>in each call site
> >>> 2) Developers need to be careful not to incur an
> >>>out-of-bounds array access
> >>> 3) Developers need to be careful that the indexes match the
> >>>table. For that it may be that the table needs to contain
> >>>holes, making (1) even worse.
> >>>
> >>> Add a variant of _PICK_EVEN() that works with 2 ranges and selects
> >>> which one to use depending on the index value.
> >>>
> >>> Signed-off-by: Lucas De Marchi 
> >>> ---
> >>>  drivers/gpu/drm/i915/i915_reg_defs.h | 28
> >>> 
> >>>  1 file changed, 28 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
> >>> b/drivers/gpu/drm/i915/i915_reg_defs.h
> >>> index be43580a6979..b7ec87464d69 100644
> >>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> >>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> >>> @@ -119,6 +119,34 @@
> >>>   */
> >>>  #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) -
> >>> (__a)))
> >>>
> >>> +/*
> >>> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address
> offsets.
> >>> + * The first range is used for indexes below @__c_index, and the
> >>> +second
> >>> + * range is used for anything above it. Example::
> >>> + *
> >>> + * #define _FOO_A0xf000
> >>> + * #define _FOO_B0xf004
> >>> + * #define _FOO_C0xf008
> >>> + * #define _SUPER_FOO_A  0xa000
> >>> + * #define _SUPER_FOO_B  0xa100
> >>> + * #define FOO(x)_MMIO(_PICK_EVEN_RANGES(x, 3,
> >>>   \
> >>> + * _FOO_A, _FOO_B,
> >>>   \
> >>> + * _SUPER_FOO_A, 
> >>> _SUPER_FOO_B))
> >>> + *
> >>> + * This expands to:
> >>> + *   0: 0xf000,
> >>> + *   1: 0xf004,
> >>> + *   2: 0xf008,
> >>> + *   3: 0xa100,
> >>You mean 3:0xa000
> >
> > doesn't really matter. This is an example of register addresses. They
> > don't need to start from 0, it's whatever the hw gives us.
> 
> I think the point is that the example is inconsistent between _SUPER_FOO_A and
> "3: 0xa100".

Yes. It's the inconsistency with _SUPER_FOO_A that I was pointing out.

Anusha
> BR,
> Jani.
> 
> >
> > Lucas De Marchi
> >
> >>
> >>> + *   4: 0xa200,
> >>4:0xa100
> >>
> >>> + *   5: 0xa300,
> >>5:0xa200
> >>
> >>Anusha
> >>> + *   ...
> >>> + */
> >>> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)
> >>>   \
> >>> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
> >>>   \
> >>> +  ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :
> >>>   \
> >>> +_PICK_EVEN((__index) - (__c_index), __c,
> >>> __d)))
> >>> +
> >>>  /*
> >>>   * Given the arbitrary numbers in varargs, pick the 0-based __index'th
> number.
> >>>   *
> >>> --
> >>> 2.39.0
> >>
> 
> --
> Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

2023-01-23 Thread Jani Nikula
On Sat, 21 Jan 2023, Lucas De Marchi  wrote:
> On Fri, Jan 20, 2023 at 10:14:19PM -0800, Anusha Srivatsa wrote:
>>
>>
>>> -Original Message-
>>> From: Intel-gfx  On Behalf Of Lucas
>>> De Marchi
>>> Sent: Friday, January 20, 2023 11:35 AM
>>> To: intel-gfx@lists.freedesktop.org
>>> Cc: De Marchi, Lucas ; dri-
>>> de...@lists.freedesktop.org
>>> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
>>>
>>> It's a constant pattern in the driver to need to use 2 ranges of MMIOs 
>>> based on
>>> port, phy, pll, etc. When that happens, instead of using _PICK_EVEN(), 
>>> _PICK()
>>> needs to be used.  Using _PICK() is discouraged due to some reasons like:
>>>
>>> 1) It increases the code size since the array is declared
>>>in each call site
>>> 2) Developers need to be careful not to incur an
>>>out-of-bounds array access
>>> 3) Developers need to be careful that the indexes match the
>>>table. For that it may be that the table needs to contain
>>>holes, making (1) even worse.
>>>
>>> Add a variant of _PICK_EVEN() that works with 2 ranges and selects which one
>>> to use depending on the index value.
>>>
>>> Signed-off-by: Lucas De Marchi 
>>> ---
>>>  drivers/gpu/drm/i915/i915_reg_defs.h | 28 
>>>  1 file changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
>>> b/drivers/gpu/drm/i915/i915_reg_defs.h
>>> index be43580a6979..b7ec87464d69 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
>>> @@ -119,6 +119,34 @@
>>>   */
>>>  #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
>>>
>>> +/*
>>> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address 
>>> offsets.
>>> + * The first range is used for indexes below @__c_index, and the second
>>> + * range is used for anything above it. Example::
>>> + *
>>> + * #define _FOO_A  0xf000
>>> + * #define _FOO_B  0xf004
>>> + * #define _FOO_C  0xf008
>>> + * #define _SUPER_FOO_A0xa000
>>> + * #define _SUPER_FOO_B0xa100
>>> + * #define FOO(x)  _MMIO(_PICK_EVEN_RANGES(x, 3,
>>> \
>>> + *   _FOO_A, _FOO_B,
>>> \
>>> + *   _SUPER_FOO_A, _SUPER_FOO_B))
>>> + *
>>> + * This expands to:
>>> + * 0: 0xf000,
>>> + * 1: 0xf004,
>>> + * 2: 0xf008,
>>> + * 3: 0xa100,
>>You mean 3:0xa000
>
> doesn't really matter. This is an example of register addresses. They
> don't need to start from 0, it's whatever the hw gives us.

I think the point is that the example is inconsistent between
_SUPER_FOO_A and "3: 0xa100".

BR,
Jani.

>
> Lucas De Marchi
>
>>
>>> + * 4: 0xa200,
>>4:0xa100
>>
>>> + * 5: 0xa300,
>>5:0xa200
>>
>>Anusha
>>> + * ...
>>> + */
>>> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)
>>> \
>>> +   (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
>>> \
>>> +((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :
>>> \
>>> +  _PICK_EVEN((__index) - (__c_index), __c,
>>> __d)))
>>> +
>>>  /*
>>>   * Given the arbitrary numbers in varargs, pick the 0-based __index'th 
>>> number.
>>>   *
>>> --
>>> 2.39.0
>>

-- 
Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

2023-01-23 Thread Jani Nikula
On Fri, 20 Jan 2023, Lucas De Marchi  wrote:
> It's a constant pattern in the driver to need to use 2 ranges of MMIOs
> based on port, phy, pll, etc. When that happens, instead of using
> _PICK_EVEN(), _PICK() needs to be used.  Using _PICK() is discouraged
> due to some reasons like:
>
> 1) It increases the code size since the array is declared
>in each call site

Would be interesting to see what this does, and whether the compiler has
the smarts to combine these within each file:

-#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
+#define _PICK(__index, ...) (((static const u32 []){ __VA_ARGS__ })[__index])

> 2) Developers need to be careful not to incur an
>out-of-bounds array access
> 3) Developers need to be careful that the indexes match the
>table. For that it may be that the table needs to contain
>holes, making (1) even worse.
>
> Add a variant of _PICK_EVEN() that works with 2 ranges and selects which
> one to use depending on the index value.
>
> Signed-off-by: Lucas De Marchi 
> ---
>  drivers/gpu/drm/i915/i915_reg_defs.h | 28 
>  1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h 
> b/drivers/gpu/drm/i915/i915_reg_defs.h
> index be43580a6979..b7ec87464d69 100644
> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> @@ -119,6 +119,34 @@
>   */
>  #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
>  
> +/*
> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
> + * The first range is used for indexes below @__c_index, and the second
> + * range is used for anything above it. Example::

I'd like this to be clear about which range is used for index ==
__c_index, instead of saying "below" and "above".

No need for the double colon :: because this isn't a kernel-doc comment.

> + *
> + * #define _FOO_A0xf000
> + * #define _FOO_B0xf004
> + * #define _FOO_C0xf008
> + * #define _SUPER_FOO_A  0xa000
> + * #define _SUPER_FOO_B  0xa100
> + * #define FOO(x)_MMIO(_PICK_EVEN_RANGES(x, 3,   
> \

The example uses a different name for the macro.

> + * _FOO_A, _FOO_B,   
> \
> + * _SUPER_FOO_A, _SUPER_FOO_B))
> + *
> + * This expands to:
> + *   0: 0xf000,
> + *   1: 0xf004,
> + *   2: 0xf008,
> + *   3: 0xa100,

With the above definitions, this would be 3: 0xa000.

> + *   4: 0xa200,
> + *   5: 0xa300,
> + *   ...
> + */
> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)   
> \
> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
> \
> +  ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : 
> \
> +_PICK_EVEN((__index) - (__c_index), __c, 
> __d)))
> +
>  /*
>   * Given the arbitrary numbers in varargs, pick the 0-based __index'th 
> number.
>   *

-- 
Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

2023-01-21 Thread Lucas De Marchi

On Fri, Jan 20, 2023 at 10:14:19PM -0800, Anusha Srivatsa wrote:




-Original Message-
From: Intel-gfx  On Behalf Of Lucas
De Marchi
Sent: Friday, January 20, 2023 11:35 AM
To: intel-gfx@lists.freedesktop.org
Cc: De Marchi, Lucas ; dri-
de...@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

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

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

Add a variant of _PICK_EVEN() that works with 2 ranges and selects which one
to use depending on the index value.

Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/i915_reg_defs.h | 28 
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
b/drivers/gpu/drm/i915/i915_reg_defs.h
index be43580a6979..b7ec87464d69 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -119,6 +119,34 @@
  */
 #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))

+/*
+ * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
+ * The first range is used for indexes below @__c_index, and the second
+ * range is used for anything above it. Example::
+ *
+ * #define _FOO_A  0xf000
+ * #define _FOO_B  0xf004
+ * #define _FOO_C  0xf008
+ * #define _SUPER_FOO_A0xa000
+ * #define _SUPER_FOO_B0xa100
+ * #define FOO(x)  _MMIO(_PICK_EVEN_RANGES(x, 3,
\
+ *   _FOO_A, _FOO_B,
\
+ *   _SUPER_FOO_A, _SUPER_FOO_B))
+ *
+ * This expands to:
+ * 0: 0xf000,
+ * 1: 0xf004,
+ * 2: 0xf008,
+ * 3: 0xa100,

You mean 3:0xa000


doesn't really matter. This is an example of register addresses. They
don't need to start from 0, it's whatever the hw gives us.

Lucas De Marchi




+ * 4: 0xa200,

4:0xa100


+ * 5: 0xa300,

5:0xa200

Anusha

+ * ...
+ */
+#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)
\
+   (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
\
+((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :
\
+  _PICK_EVEN((__index) - (__c_index), __c,
__d)))
+
 /*
  * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
  *
--
2.39.0




Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

2023-01-20 Thread Srivatsa, Anusha



> -Original Message-
> From: Intel-gfx  On Behalf Of Lucas
> De Marchi
> Sent: Friday, January 20, 2023 11:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: De Marchi, Lucas ; dri-
> de...@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
> 
> It's a constant pattern in the driver to need to use 2 ranges of MMIOs based 
> on
> port, phy, pll, etc. When that happens, instead of using _PICK_EVEN(), _PICK()
> needs to be used.  Using _PICK() is discouraged due to some reasons like:
> 
> 1) It increases the code size since the array is declared
>in each call site
> 2) Developers need to be careful not to incur an
>out-of-bounds array access
> 3) Developers need to be careful that the indexes match the
>table. For that it may be that the table needs to contain
>holes, making (1) even worse.
> 
> Add a variant of _PICK_EVEN() that works with 2 ranges and selects which one
> to use depending on the index value.
> 
> Signed-off-by: Lucas De Marchi 
> ---
>  drivers/gpu/drm/i915/i915_reg_defs.h | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h
> b/drivers/gpu/drm/i915/i915_reg_defs.h
> index be43580a6979..b7ec87464d69 100644
> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> @@ -119,6 +119,34 @@
>   */
>  #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
> 
> +/*
> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
> + * The first range is used for indexes below @__c_index, and the second
> + * range is used for anything above it. Example::
> + *
> + * #define _FOO_A0xf000
> + * #define _FOO_B0xf004
> + * #define _FOO_C0xf008
> + * #define _SUPER_FOO_A  0xa000
> + * #define _SUPER_FOO_B  0xa100
> + * #define FOO(x)_MMIO(_PICK_EVEN_RANGES(x, 3,
>   \
> + * _FOO_A, _FOO_B,
>   \
> + * _SUPER_FOO_A, _SUPER_FOO_B))
> + *
> + * This expands to:
> + *   0: 0xf000,
> + *   1: 0xf004,
> + *   2: 0xf008,
> + *   3: 0xa100,
You mean 3:0xa000

> + *   4: 0xa200,
4:0xa100

> + *   5: 0xa300,
5:0xa200

Anusha 
> + *   ...
> + */
> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d)
>   \
> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
>   \
> +  ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) :
>   \
> +_PICK_EVEN((__index) - (__c_index), __c,
> __d)))
> +
>  /*
>   * Given the arbitrary numbers in varargs, pick the 0-based __index'th 
> number.
>   *
> --
> 2.39.0



[Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()

2023-01-20 Thread Lucas De Marchi
It's a constant pattern in the driver to need to use 2 ranges of MMIOs
based on port, phy, pll, etc. When that happens, instead of using
_PICK_EVEN(), _PICK() needs to be used.  Using _PICK() is discouraged
due to some reasons like:

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

Add a variant of _PICK_EVEN() that works with 2 ranges and selects which
one to use depending on the index value.

Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/i915_reg_defs.h | 28 
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h 
b/drivers/gpu/drm/i915/i915_reg_defs.h
index be43580a6979..b7ec87464d69 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -119,6 +119,34 @@
  */
 #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a)))
 
+/*
+ * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets.
+ * The first range is used for indexes below @__c_index, and the second
+ * range is used for anything above it. Example::
+ *
+ * #define _FOO_A  0xf000
+ * #define _FOO_B  0xf004
+ * #define _FOO_C  0xf008
+ * #define _SUPER_FOO_A0xa000
+ * #define _SUPER_FOO_B0xa100
+ * #define FOO(x)  _MMIO(_PICK_EVEN_RANGES(x, 3,   
\
+ *   _FOO_A, _FOO_B,   
\
+ *   _SUPER_FOO_A, _SUPER_FOO_B))
+ *
+ * This expands to:
+ * 0: 0xf000,
+ * 1: 0xf004,
+ * 2: 0xf008,
+ * 3: 0xa100,
+ * 4: 0xa200,
+ * 5: 0xa300,
+ * ...
+ */
+#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) 
\
+   (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) +
\
+((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : 
\
+  _PICK_EVEN((__index) - (__c_index), __c, 
__d)))
+
 /*
  * Given the arbitrary numbers in varargs, pick the 0-based __index'th number.
  *
-- 
2.39.0