On Thu, 01 Nov 2018, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> 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@xxxxxxxxx> >> --- >> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
