[Intel-gfx] [PATCH v2] drm/i915: use unsigned long for platform_mask

[Lucas De Marchi]

On Tue, Apr 2, 2019 at 11:58 PM Tvrtko Ursulin
<tvrtko.ursulin at linux.intel.com> wrote:
>
>
> On 03/04/2019 02:46, Lucas De Marchi wrote:
> > No reason to stick to u32 for platform mask if we can just use more bits
> > on 64 bit platforms.
> >
> > $ size drivers/gpu/drm/i915/i915.ko*
> >     text         data     bss     dec     hex filename
> > 1884779         41334    5408 1931521  1d7901 drivers/gpu/drm/i915/i915.ko
> > 1886693         41358    5408 1933459  1d8093 drivers/gpu/drm/i915/i915.ko.old
>
> How did you get such a large difference, and decrease even? Could you
> check in the code what is happening? Because I get an increase with this
> patch:
>
>     text    data     bss     dec     hex filename
> 1905314   43903    7424 1956641  1ddb21 i915.ko.orig
> 1905796   43903    7424 1957123  1ddd03 i915.ko.patch

the only explanation I really have is that my measurement was bogus.
Some possible explanations...
1) I compared a i386 to a x86-64 build; 2) somehow a config changed
between the builds;
3) when preparing the patch I rebased on upstream between the builds.

Checking (1), no... that's in the ~400k range. So no idea, sorry.

So I think the only useful thing in this patch is to make the array to
grow automatically. Or maybe not even that?

Lucas De Marchi

>
> Regards,
>
> Tvrtko
>
> >
> > Now on 64 bits we have only one long as opposed to 2 u32:
> >
> > $ pahole -C intel_runtime_info drivers/gpu/drm/i915/i915.ko
> > struct intel_runtime_info {
> >       long unsigned int          platform_mask[1];     /*     0     8 */
> > ...
> > }
> >
> > On 32 bits we still have the same thing as before:
> > $ size drivers/gpu/drm/i915/i915.ko*
> >     text         data     bss     dec     hex filename
> > 1489839         32485    2816 1525140  174594 drivers/gpu/drm/i915/i915.ko
> > 1489839         32485    2816 1525140  174594 drivers/gpu/drm/i915/i915.ko.old
> >
> > Besides reducing the code on x86-64 now the array size is automatically
> > calculated and we don't have to worry about extending it anymore.
> >
> > v2: fix sparse and checkpatch warnings
> >
> > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_drv.h          | 6 +-----
> >   drivers/gpu/drm/i915/intel_device_info.h | 7 +++----
> >   2 files changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 0ab4826921f7..9fe765ffe878 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2309,10 +2309,6 @@ __platform_mask_index(const struct intel_runtime_info *info,
> >       const unsigned int pbits =
> >               BITS_PER_TYPE(info->platform_mask[0]) - INTEL_SUBPLATFORM_BITS;
> >
> > -     /* Expand the platform_mask array if this fails. */
> > -     BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
> > -                  pbits * ARRAY_SIZE(info->platform_mask));
> > -
> >       return p / pbits;
> >   }
> >
> > @@ -2354,7 +2350,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> >       const unsigned int pi = __platform_mask_index(info, p);
> >       const unsigned int pb = __platform_mask_bit(info, p);
> >       const unsigned int msb = BITS_PER_TYPE(info->platform_mask[0]) - 1;
> > -     const u32 mask = info->platform_mask[pi];
> > +     const unsigned long mask = info->platform_mask[pi];
> >
> >       BUILD_BUG_ON(!__builtin_constant_p(p));
> >       BUILD_BUG_ON(!__builtin_constant_p(s));
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> > index 0e579f158016..2f5ca2b6f094 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -214,11 +214,10 @@ struct intel_runtime_info {
> >        * Platform mask is used for optimizing or-ed IS_PLATFORM calls into
> >        * into single runtime conditionals, and also to provide groundwork
> >        * for future per platform, or per SKU build optimizations.
> > -      *
> > -      * Array can be extended when necessary if the corresponding
> > -      * BUILD_BUG_ON is hit.
> >        */
> > -     u32 platform_mask[2];
> > +     unsigned long platform_mask[DIV_ROUND_UP(INTEL_MAX_PLATFORMS,
> > +                                              BITS_PER_TYPE(unsigned long)
> > +                                              - INTEL_SUBPLATFORM_BITS)];
> >
> >       u16 device_id;
> >
> >
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Lucas De Marchi