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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


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

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

thanks
Lucas De Marchi



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux