| Message ID | 20210413051002.92589-9-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Extend GEN renames to the rest of the driver | expand |
On Mon, 12 Apr 2021, Lucas De Marchi <lucas.demarchi@intel.com> wrote: > Now that it's not used anywhere, remove it from struct > intel_device_info. To allow a period in which code will be converted to > the new macro, keep IS_GEN_RANGE() around, just redefining it to use > the new fields. The size advantage from IS_GEN_RANGE() using a mask is > not that big as it has pretty limited use througout the driver: > > text data bss dec hex filename > 2758497 95965 6496 2860958 2ba79e drivers/gpu/drm/i915/i915.ko.old > 2758586 95953 6496 2861035 2ba7eb drivers/gpu/drm/i915/i915.ko.new > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 -- > drivers/gpu/drm/i915/i915_drv.h | 13 ++++--------- > drivers/gpu/drm/i915/i915_pci.c | 1 - > drivers/gpu/drm/i915/intel_device_info.h | 2 -- > 4 files changed, 4 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 305557e1942a..825b45cb3543 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -768,8 +768,6 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) > memcpy(device_info, match_info, sizeof(*device_info)); > RUNTIME_INFO(i915)->device_id = pdev->device; > > - BUG_ON(device_info->gen > BITS_PER_TYPE(device_info->gen_mask)); > - > return i915; > } > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index cb59eb0f6c5b..b984a340b21f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1241,6 +1241,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev) > * GRAPHICS_VER(), MEDIA_VER and DISPLAY_VER() > */ > #define INTEL_GEN(dev_priv) (INTEL_INFO(dev_priv)->gen) > +/* > + * Deprecated: use IS_GRAPHICS_VER() > + */ Nitpick, I think this should also mention IS_MEDIA_VER() and DISPLAY_VER() to not have people blindly use IS_GRAPHICS_VER(). Reviewed-by: Jani Nikula <jani.nikula@intel.com> > +#define IS_GEN_RANGE(dev_priv, s, e) IS_GRAPHICS_VER(dev_priv, (s), (e)) > > #define GRAPHICS_VER(i915) (INTEL_INFO(i915)->graphics_ver) > #define IS_GRAPHICS_VER(i915, from, until) \ > @@ -1257,15 +1261,6 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev) > #define REVID_FOREVER 0xff > #define INTEL_REVID(dev_priv) (to_pci_dev((dev_priv)->drm.dev)->revision) > > -#define INTEL_GEN_MASK(s, e) ( \ > - BUILD_BUG_ON_ZERO(!__builtin_constant_p(s)) + \ > - BUILD_BUG_ON_ZERO(!__builtin_constant_p(e)) + \ > - GENMASK((e) - 1, (s) - 1)) > - > -/* Returns true if Gen is in inclusive range [Start, End] */ > -#define IS_GEN_RANGE(dev_priv, s, e) \ > - (!!(INTEL_INFO(dev_priv)->gen_mask & INTEL_GEN_MASK((s), (e)))) > - > #define IS_GEN(dev_priv, n) \ > (BUILD_BUG_ON_ZERO(!__builtin_constant_p(n)) + \ > INTEL_INFO(dev_priv)->gen == (n)) > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 97ab73276334..3b9cd1af0f28 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -37,7 +37,6 @@ > > #define PLATFORM(x) .platform = (x) > #define GEN(x) \ > - .gen_mask = BIT((x) - 1), \ > .gen = (x), \ > .graphics_ver = (x), \ > .media_ver = (x), \ > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 405883a8cc84..b8f7b996f140 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -160,8 +160,6 @@ enum intel_ppgtt_type { > func(supports_tv); > > struct intel_device_info { > - u16 gen_mask; > - > u8 graphics_ver; > u8 media_ver;
On 13/04/2021 06:09, Lucas De Marchi wrote: > Now that it's not used anywhere, remove it from struct > intel_device_info. To allow a period in which code will be converted to > the new macro, keep IS_GEN_RANGE() around, just redefining it to use > the new fields. The size advantage from IS_GEN_RANGE() using a mask is > not that big as it has pretty limited use througout the driver: > > text data bss dec hex filename > 2758497 95965 6496 2860958 2ba79e drivers/gpu/drm/i915/i915.ko.old > 2758586 95953 6496 2861035 2ba7eb drivers/gpu/drm/i915/i915.ko.new This delta refers to this patch - I mean this point in the series? Asking because it may not be 100% representative since some of the previous patches have already removed some gen mask usages. While I am here, I am a bit fond of the mask approach and wonder if using it for all (gt/media/whatelse) new fields would still make sense. Presence of the range check helpers suggests that it might, but I haven't looked at how prevalent their usage ends up after the series is done. So just in principle, I don't see why not still go with masks since that guarantees elegant check at each range check site. It would be all hidden in the macro implementation so easy. Also for historical reference, another reason why I went for masks everywhere approach is that at some point we had a feature request to allow compiling out platforms/gens. I *think* that was much easier to do with masking and in experiments back then I was able for instance to build just for Gen9+ and drop like 30% of the binary size. Oh I found the branch now.. The reason for IS_GEN(p, v) was also in that series. I don't know if I ever RFC-ed or trybotted it.. google suggests no and I neither can find it in my mailboxes. I could send out the old patches for reference? But to be honest I have no idea if this feature request (targeted driver builds) will ever resurface.. Regards, Tvrtko > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 -- > drivers/gpu/drm/i915/i915_drv.h | 13 ++++--------- > drivers/gpu/drm/i915/i915_pci.c | 1 - > drivers/gpu/drm/i915/intel_device_info.h | 2 -- > 4 files changed, 4 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 305557e1942a..825b45cb3543 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -768,8 +768,6 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) > memcpy(device_info, match_info, sizeof(*device_info)); > RUNTIME_INFO(i915)->device_id = pdev->device; > > - BUG_ON(device_info->gen > BITS_PER_TYPE(device_info->gen_mask)); > - > return i915; > } > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index cb59eb0f6c5b..b984a340b21f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1241,6 +1241,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev) > * GRAPHICS_VER(), MEDIA_VER and DISPLAY_VER() > */ > #define INTEL_GEN(dev_priv) (INTEL_INFO(dev_priv)->gen) > +/* > + * Deprecated: use IS_GRAPHICS_VER() > + */ > +#define IS_GEN_RANGE(dev_priv, s, e) IS_GRAPHICS_VER(dev_priv, (s), (e)) > > #define GRAPHICS_VER(i915) (INTEL_INFO(i915)->graphics_ver) > #define IS_GRAPHICS_VER(i915, from, until) \ > @@ -1257,15 +1261,6 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev) > #define REVID_FOREVER 0xff > #define INTEL_REVID(dev_priv) (to_pci_dev((dev_priv)->drm.dev)->revision) > > -#define INTEL_GEN_MASK(s, e) ( \ > - BUILD_BUG_ON_ZERO(!__builtin_constant_p(s)) + \ > - BUILD_BUG_ON_ZERO(!__builtin_constant_p(e)) + \ > - GENMASK((e) - 1, (s) - 1)) > - > -/* Returns true if Gen is in inclusive range [Start, End] */ > -#define IS_GEN_RANGE(dev_priv, s, e) \ > - (!!(INTEL_INFO(dev_priv)->gen_mask & INTEL_GEN_MASK((s), (e)))) > - > #define IS_GEN(dev_priv, n) \ > (BUILD_BUG_ON_ZERO(!__builtin_constant_p(n)) + \ > INTEL_INFO(dev_priv)->gen == (n)) > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 97ab73276334..3b9cd1af0f28 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -37,7 +37,6 @@ > > #define PLATFORM(x) .platform = (x) > #define GEN(x) \ > - .gen_mask = BIT((x) - 1), \ > .gen = (x), \ > .graphics_ver = (x), \ > .media_ver = (x), \ > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 405883a8cc84..b8f7b996f140 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -160,8 +160,6 @@ enum intel_ppgtt_type { > func(supports_tv); > > struct intel_device_info { > - u16 gen_mask; > - > u8 graphics_ver; > u8 media_ver; > >
On Wed, 14 Apr 2021, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > On 13/04/2021 06:09, Lucas De Marchi wrote: >> Now that it's not used anywhere, remove it from struct >> intel_device_info. To allow a period in which code will be converted to >> the new macro, keep IS_GEN_RANGE() around, just redefining it to use >> the new fields. The size advantage from IS_GEN_RANGE() using a mask is >> not that big as it has pretty limited use througout the driver: >> >> text data bss dec hex filename >> 2758497 95965 6496 2860958 2ba79e drivers/gpu/drm/i915/i915.ko.old >> 2758586 95953 6496 2861035 2ba7eb drivers/gpu/drm/i915/i915.ko.new > > This delta refers to this patch - I mean this point in the series? > Asking because it may not be 100% representative since some of the > previous patches have already removed some gen mask usages. > > While I am here, I am a bit fond of the mask approach and wonder if > using it for all (gt/media/whatelse) new fields would still make sense. > > Presence of the range check helpers suggests that it might, but I > haven't looked at how prevalent their usage ends up after the series is > done. So just in principle, I don't see why not still go with masks > since that guarantees elegant check at each range check site. It would > be all hidden in the macro implementation so easy. > > Also for historical reference, another reason why I went for masks > everywhere approach is that at some point we had a feature request to > allow compiling out platforms/gens. I *think* that was much easier to do > with masking and in experiments back then I was able for instance to > build just for Gen9+ and drop like 30% of the binary size. > > Oh I found the branch now.. The reason for IS_GEN(p, v) was also in that > series. I don't know if I ever RFC-ed or trybotted it.. google suggests > no and I neither can find it in my mailboxes. I could send out the old > patches for reference? But to be honest I have no idea if this feature > request (targeted driver builds) will ever resurface.. I completely agreed with the direction of using the masks way back when, especially with the goal of the conditional/targeted compilation. I think the question now is whether we want to keep maintaining them just for the sake of the masks. Keeping them means having three masks instead of one. And we wouldn't be using most of the benefits with them, we'd mostly just get the downsides. Having the masks per se is not such a big deal, but they're also not such a big deal to add back later on if needed. It's the codebase all over that's the hard part. And arguably it's not getting that much different with the series at hand; the direct uses of INTEL_GEN() and DISPLAY_VER() vastly outnumber IS_GEN(), IS_GEN_RANGE() and IS_DISPLAY_RANGE() which could benefit from the mask. We'd still be retaining the range macros as IS_GRAPHICS_VER(), IS_MEDIA_VER() and IS_DISPLAY_VER(), although more for clarity than for any other reason. BR, Jani.
On 14/04/2021 14:13, Jani Nikula wrote: > On Wed, 14 Apr 2021, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: >> On 13/04/2021 06:09, Lucas De Marchi wrote: >>> Now that it's not used anywhere, remove it from struct >>> intel_device_info. To allow a period in which code will be converted to >>> the new macro, keep IS_GEN_RANGE() around, just redefining it to use >>> the new fields. The size advantage from IS_GEN_RANGE() using a mask is >>> not that big as it has pretty limited use througout the driver: >>> >>> text data bss dec hex filename >>> 2758497 95965 6496 2860958 2ba79e drivers/gpu/drm/i915/i915.ko.old >>> 2758586 95953 6496 2861035 2ba7eb drivers/gpu/drm/i915/i915.ko.new >> >> This delta refers to this patch - I mean this point in the series? >> Asking because it may not be 100% representative since some of the >> previous patches have already removed some gen mask usages. >> >> While I am here, I am a bit fond of the mask approach and wonder if >> using it for all (gt/media/whatelse) new fields would still make sense. >> >> Presence of the range check helpers suggests that it might, but I >> haven't looked at how prevalent their usage ends up after the series is >> done. So just in principle, I don't see why not still go with masks >> since that guarantees elegant check at each range check site. It would >> be all hidden in the macro implementation so easy. >> >> Also for historical reference, another reason why I went for masks >> everywhere approach is that at some point we had a feature request to >> allow compiling out platforms/gens. I *think* that was much easier to do >> with masking and in experiments back then I was able for instance to >> build just for Gen9+ and drop like 30% of the binary size. >> >> Oh I found the branch now.. The reason for IS_GEN(p, v) was also in that >> series. I don't know if I ever RFC-ed or trybotted it.. google suggests >> no and I neither can find it in my mailboxes. I could send out the old >> patches for reference? But to be honest I have no idea if this feature >> request (targeted driver builds) will ever resurface.. > > I completely agreed with the direction of using the masks way back when, > especially with the goal of the conditional/targeted compilation. > > I think the question now is whether we want to keep maintaining them > just for the sake of the masks. Keeping them means having three masks > instead of one. And we wouldn't be using most of the benefits with them, > we'd mostly just get the downsides. > > Having the masks per se is not such a big deal, but they're also not > such a big deal to add back later on if needed. It's the codebase all > over that's the hard part. And arguably it's not getting that much > different with the series at hand; the direct uses of INTEL_GEN() and > DISPLAY_VER() vastly outnumber IS_GEN(), IS_GEN_RANGE() and > IS_DISPLAY_RANGE() which could benefit from the mask. > > We'd still be retaining the range macros as IS_GRAPHICS_VER(), > IS_MEDIA_VER() and IS_DISPLAY_VER(), although more for clarity than for > any other reason. Adding masks later would not a big deal, but another cycle of changing "xxx_VER == n" to "IS_xxx_VER(n)" is a churn which could presumably be avoided. It is moot yes, but I don't see a clear case for doing the reversal as part of this series. With a disclaimer that I only glanced over the commit messages today for the first time. So I think from me its neither ack or nack, at least since I don't understand the attractiveness of using the "ver == n" and numerical range check forms everywhere. As said, if we are churning I'd rather go the other direction. But that's a soft objection only so feel free to proceed. Regards, Tvrtko
On Wed, Apr 14, 2021 at 12:38:44PM +0100, Tvrtko Ursulin wrote: > >On 13/04/2021 06:09, Lucas De Marchi wrote: >>Now that it's not used anywhere, remove it from struct >>intel_device_info. To allow a period in which code will be converted to >>the new macro, keep IS_GEN_RANGE() around, just redefining it to use >>the new fields. The size advantage from IS_GEN_RANGE() using a mask is >>not that big as it has pretty limited use througout the driver: >> >> text data bss dec hex filename >>2758497 95965 6496 2860958 2ba79e drivers/gpu/drm/i915/i915.ko.old >>2758586 95953 6496 2861035 2ba7eb drivers/gpu/drm/i915/i915.ko.new > >This delta refers to this patch - I mean this point in the series? >Asking because it may not be 100% representative since some of the >previous patches have already removed some gen mask usages. yes, it doesn't consider the other patches. These numbers are also for v1, not v2, as I didn't update the commit mesage. I don't think the numbers will be too different though. > >While I am here, I am a bit fond of the mask approach and wonder if >using it for all (gt/media/whatelse) new fields would still make >sense. > >Presence of the range check helpers suggests that it might, but I >haven't looked at how prevalent their usage ends up after the series >is done. So just in principle, I don't see why not still go with masks >since that guarantees elegant check at each range check site. It would >be all hidden in the macro implementation so easy. > >Also for historical reference, another reason why I went for masks >everywhere approach is that at some point we had a feature request to >allow compiling out platforms/gens. I *think* that was much easier to >do with masking and in experiments back then I was able for instance >to build just for Gen9+ and drop like 30% of the binary size. > >Oh I found the branch now.. The reason for IS_GEN(p, v) was also in >that series. I don't know if I ever RFC-ed or trybotted it.. google >suggests no and I neither can find it in my mailboxes. I could send >out the old patches for reference? But to be honest I have no idea if >this feature request (targeted driver builds) will ever resurface.. At the time I also liked having the macros. Looking back and checking if we really took advantage of it, I lean towards a "no". Even when and if we are interested in compiling out some platforms, I think a better code split would be deserved rather relying on this. Lucas De Marchi
On Wed, 14 Apr 2021, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > On 14/04/2021 14:13, Jani Nikula wrote: >> On Wed, 14 Apr 2021, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: >>> On 13/04/2021 06:09, Lucas De Marchi wrote: >>>> Now that it's not used anywhere, remove it from struct >>>> intel_device_info. To allow a period in which code will be converted to >>>> the new macro, keep IS_GEN_RANGE() around, just redefining it to use >>>> the new fields. The size advantage from IS_GEN_RANGE() using a mask is >>>> not that big as it has pretty limited use througout the driver: >>>> >>>> text data bss dec hex filename >>>> 2758497 95965 6496 2860958 2ba79e drivers/gpu/drm/i915/i915.ko.old >>>> 2758586 95953 6496 2861035 2ba7eb drivers/gpu/drm/i915/i915.ko.new >>> >>> This delta refers to this patch - I mean this point in the series? >>> Asking because it may not be 100% representative since some of the >>> previous patches have already removed some gen mask usages. >>> >>> While I am here, I am a bit fond of the mask approach and wonder if >>> using it for all (gt/media/whatelse) new fields would still make sense. >>> >>> Presence of the range check helpers suggests that it might, but I >>> haven't looked at how prevalent their usage ends up after the series is >>> done. So just in principle, I don't see why not still go with masks >>> since that guarantees elegant check at each range check site. It would >>> be all hidden in the macro implementation so easy. >>> >>> Also for historical reference, another reason why I went for masks >>> everywhere approach is that at some point we had a feature request to >>> allow compiling out platforms/gens. I *think* that was much easier to do >>> with masking and in experiments back then I was able for instance to >>> build just for Gen9+ and drop like 30% of the binary size. >>> >>> Oh I found the branch now.. The reason for IS_GEN(p, v) was also in that >>> series. I don't know if I ever RFC-ed or trybotted it.. google suggests >>> no and I neither can find it in my mailboxes. I could send out the old >>> patches for reference? But to be honest I have no idea if this feature >>> request (targeted driver builds) will ever resurface.. >> >> I completely agreed with the direction of using the masks way back when, >> especially with the goal of the conditional/targeted compilation. >> >> I think the question now is whether we want to keep maintaining them >> just for the sake of the masks. Keeping them means having three masks >> instead of one. And we wouldn't be using most of the benefits with them, >> we'd mostly just get the downsides. >> >> Having the masks per se is not such a big deal, but they're also not >> such a big deal to add back later on if needed. It's the codebase all >> over that's the hard part. And arguably it's not getting that much >> different with the series at hand; the direct uses of INTEL_GEN() and >> DISPLAY_VER() vastly outnumber IS_GEN(), IS_GEN_RANGE() and >> IS_DISPLAY_RANGE() which could benefit from the mask. >> >> We'd still be retaining the range macros as IS_GRAPHICS_VER(), >> IS_MEDIA_VER() and IS_DISPLAY_VER(), although more for clarity than for >> any other reason. > > Adding masks later would not a big deal, but another cycle of changing > "xxx_VER == n" to "IS_xxx_VER(n)" is a churn which could presumably be > avoided. Direct xxx_VER <, >, <= and >= already exist all over the place, and their numbers trump the == cases. Seems confusing to treat == differently. > It is moot yes, but I don't see a clear case for doing the reversal as > part of this series. With a disclaimer that I only glanced over the > commit messages today for the first time. So I wanted to keep using the range check macros for a couple of reasons. Having (VER >= x && VER <= y) gets long, it needs braces, and we use a bunch of negation !(VER >= x && VER <= y) vs. VER < x || VER > y. !IS_GEN_RANGE() has more clarity. Now, adding IS_GRAPHICS_VER_RANGE() gets long. Dropping the VER for IS_GRAPHICS_RANGE() gets confusing ("what graphics range?"). Now, if we use == for specific version check, we can repurpose IS_GRAPHICS_VER() to do the ranges. > So I think from me its neither ack or nack, at least since I don't > understand the attractiveness of using the "ver == n" and numerical > range check forms everywhere. As said, if we are churning I'd rather go > the other direction. But that's a soft objection only so feel free to > proceed. Thanks, noted. However, unless stronger objections arise, I think we're going to go with the patches at hand. BR, Jani.
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 305557e1942a..825b45cb3543 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -768,8 +768,6 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) memcpy(device_info, match_info, sizeof(*device_info)); RUNTIME_INFO(i915)->device_id = pdev->device; - BUG_ON(device_info->gen > BITS_PER_TYPE(device_info->gen_mask)); - return i915; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index cb59eb0f6c5b..b984a340b21f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1241,6 +1241,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev) * GRAPHICS_VER(), MEDIA_VER and DISPLAY_VER() */ #define INTEL_GEN(dev_priv) (INTEL_INFO(dev_priv)->gen) +/* + * Deprecated: use IS_GRAPHICS_VER() + */ +#define IS_GEN_RANGE(dev_priv, s, e) IS_GRAPHICS_VER(dev_priv, (s), (e)) #define GRAPHICS_VER(i915) (INTEL_INFO(i915)->graphics_ver) #define IS_GRAPHICS_VER(i915, from, until) \ @@ -1257,15 +1261,6 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev) #define REVID_FOREVER 0xff #define INTEL_REVID(dev_priv) (to_pci_dev((dev_priv)->drm.dev)->revision) -#define INTEL_GEN_MASK(s, e) ( \ - BUILD_BUG_ON_ZERO(!__builtin_constant_p(s)) + \ - BUILD_BUG_ON_ZERO(!__builtin_constant_p(e)) + \ - GENMASK((e) - 1, (s) - 1)) - -/* Returns true if Gen is in inclusive range [Start, End] */ -#define IS_GEN_RANGE(dev_priv, s, e) \ - (!!(INTEL_INFO(dev_priv)->gen_mask & INTEL_GEN_MASK((s), (e)))) - #define IS_GEN(dev_priv, n) \ (BUILD_BUG_ON_ZERO(!__builtin_constant_p(n)) + \ INTEL_INFO(dev_priv)->gen == (n)) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 97ab73276334..3b9cd1af0f28 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -37,7 +37,6 @@ #define PLATFORM(x) .platform = (x) #define GEN(x) \ - .gen_mask = BIT((x) - 1), \ .gen = (x), \ .graphics_ver = (x), \ .media_ver = (x), \ diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 405883a8cc84..b8f7b996f140 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -160,8 +160,6 @@ enum intel_ppgtt_type { func(supports_tv); struct intel_device_info { - u16 gen_mask; - u8 graphics_ver; u8 media_ver;
Now that it's not used anywhere, remove it from struct intel_device_info. To allow a period in which code will be converted to the new macro, keep IS_GEN_RANGE() around, just redefining it to use the new fields. The size advantage from IS_GEN_RANGE() using a mask is not that big as it has pretty limited use througout the driver: text data bss dec hex filename 2758497 95965 6496 2860958 2ba79e drivers/gpu/drm/i915/i915.ko.old 2758586 95953 6496 2861035 2ba7eb drivers/gpu/drm/i915/i915.ko.new Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 2 -- drivers/gpu/drm/i915/i915_drv.h | 13 ++++--------- drivers/gpu/drm/i915/i915_pci.c | 1 - drivers/gpu/drm/i915/intel_device_info.h | 2 -- 4 files changed, 4 insertions(+), 14 deletions(-)