On Wed, Oct 31, 2018 at 10:13:43AM +0200, Jani Nikula wrote: > On Tue, 30 Oct 2018, Lucas De Marchi <lucas.de.marchi@xxxxxxxxx> wrote: > > On Tue, Oct 30, 2018 at 11:52:30AM +0200, Jani Nikula wrote: > >> On Mon, 29 Oct 2018, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote: > >> > +#define IS_DISPLAY_GEN2(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(2))) > >> > +#define IS_DISPLAY_GEN3(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(3))) > >> > +#define IS_DISPLAY_GEN4(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(4))) > >> > +#define IS_DISPLAY_GEN5(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(5))) > >> > +#define IS_DISPLAY_GEN6(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(6))) > >> > +#define IS_DISPLAY_GEN7(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(7))) > >> > +#define IS_DISPLAY_GEN8(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(8))) > >> > +#define IS_DISPLAY_GEN9(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(9))) > >> > +#define IS_DISPLAY_GEN10(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(10))) > >> > +#define IS_DISPLAY_GEN11(dev_priv) (!!((dev_priv)->info.display_gen_mask \ > >> > + & BIT(11))) > >> > >> I know this is the same pattern as in IS_GEN<N> above, but shouldn't the > >> compiler end up with the same result if these were simply: > >> > >> #define IS_DISPLAY_GEN2(dev_priv) IS_DISPLAY_GEN(dev_priv, 2, 2) > > > > > > humn... maybe this is too magic, but it works for me and I didn't add any additional > > macro to the kernel to implement it :) > > > > [CI, DON'T TEST THIS] diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > [CI, DON'T TEST THIS] index 554627dc623c..02a8b51fd733 100644 > > [CI, DON'T TEST THIS] --- a/drivers/gpu/drm/i915/i915_drv.h > > [CI, DON'T TEST THIS] +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2385,9 +2385,15 @@ intel_info(const struct drm_i915_private *dev_priv) > > GENMASK((e) - 1, (s) - 1)) > > > > /* Returns true if Gen is in inclusive range [Start, End] */ > > -#define IS_GEN(dev_priv, s, e) \ > > +#define _IS_GEN_ARG2(dev_priv, s, e) \ > > (!!((dev_priv)->info.gen_mask & INTEL_GEN_MASK((s), (e)))) > > > > +#define _IS_GEN_ARG1(dev_priv, g) \ > > + (!!((dev_priv)->info.gen_mask & BIT((g) - 1))) > > + > > +#define IS_GEN(dev_priv, ...) \ > > + CONCATENATE(_IS_GEN_ARG, COUNT_ARGS(__VA_ARGS__))((dev_priv), ##__VA_ARGS__) > > + > > /* > > * Return true if revision is in range [since,until] inclusive. > > * > > > > > > So we could use IS_GEN(dev_priv, 2) as well as IS_GEN(dev_priv, 2, 4), which IMO is very clear. > > The same would apply for IS_DISPLAY_GEN() version. And if they generate the same code, we could > > just change the expansion to repeat the argument. > > I like this stuff. > > So I'd prefer IS_GEN(dev_priv, 2) in favor of IS_GEN2(dev_priv) > throughout. > > As long as it doesn't increase the code size too much, but this being > macro magic I don't think it should. > > Care to cook up a patch against current tip to make IS_GEN() take 1 or 2 > args? If I read the above right, you'll get a build error for using > IS_GEN(dev_priv, 1, 2, 3), is that correct? Correct. I'll prepare a patch for that so we can discuss on the approach with a concrete example. Lucas De Marchi > > BR, > Jani. > > > -- > Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
