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