[Intel-gfx] [PATCH 4/5] drm/i915: add helper IS_GEN_* macros
Jani Nikula
jani.nikula at linux.intel.com
Thu Nov 1 11:29:54 UTC 2018
On Thu, 01 Nov 2018, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:
> On 01/11/2018 08:35, Lucas De Marchi wrote:
>> This will allow us to make all gen comparisons simple gen comparisons
>> with the same kind of macro rather than resorting to a mix of IS_GEN(),
>> INTEL_GEN() >=, >, <=, <.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 8d22353a3da6..eb636dfe5228 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2370,6 +2370,11 @@ intel_info(const struct drm_i915_private *dev_priv)
>> #define IS_GEN(dev_priv, ...) \
>> CONCATENATE(_IS_GEN_ARG, COUNT_ARGS(__VA_ARGS__))((dev_priv), ##__VA_ARGS__)
>>
>> +#define IS_GEN_GT(dev_priv, g) (g > (dev_priv)->info.gen)
>
> Interesting :), I have at some point floated a similar patch and there
> was overwhelming dislike of the GT suffix, since we have GT2 GT3 etc..
> as established acronyms for GPU configurations.
To be honest even *without* the GPU GT clash, I think GT/GE/LT/LE are a
cognitive burden. Not a big one, but still less natural than >/>=/</<=.
Feels like assembly.
Not that I have better alternatives. IS_GEN_AFTER for GT,
IS_GEN_OR_LATER for GE? Feels a bit contrived?
> Furthermore, I am pretty sure I've done it using the gen bitmask so
> hypothetical checks like IS_GEN_LT(5) || IS_GEN_RANGE(8, 9) could still
> be compile time optimized to a single conditional. That would also now
> become more difficult due removal of GEN_FOREVER.
>
> To summarise, I don't see a advantage of hiding this in macros if we are
> not getting the above benefit.
Agreed. I didn't realize the GEN_FOREVER removal was merged already, I
wish you'd chimed in there. Of course, we can bring it back.
BR,
Jani.
>
> Regards,
>
> Tvrtko
>
>> +#define IS_GEN_GE(dev_priv, g) (g >= (dev_priv)->info.gen)
>> +#define IS_GEN_LT(dev_priv, g) (g < (dev_priv)->info.gen)
>> +#define IS_GEN_LE(dev_priv, g) (g <= (dev_priv)->info.gen)
>> +
>> /*
>> * Return true if revision is in range [since,until] inclusive.
>> *
>>
--
Jani Nikula, Intel Open Source Graphics Center
More information about the Intel-gfx
mailing list