diff mbox

[RFC] drm/i915: Eliminate devid sprinkle

Message ID 20180222080907.14716-1-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin Feb. 22, 2018, 8:09 a.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Introduce subplatform mask to eliminate throughout the code devid checking
sprinkle, mostly courtesy of IS_*_UL[TX] macros.

Subplatform mask initialization is moved either to static tables (Ironlake
M) or runtime device info init (Pineview, Haswell, Broadwell, Skylake,
Kabylake, Coffeelake and Cannonlake).

   text    data     bss     dec     hex filename
1673630   59691    5064 1738385  1a8691 i915.ko.0
1673214   59691    5064 1737969  1a84f1 i915.ko.1

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c          |  3 +-
 drivers/gpu/drm/i915/i915_drv.h          | 55 +++++++++++-----------------
 drivers/gpu/drm/i915/i915_pci.c          |  3 ++
 drivers/gpu/drm/i915/intel_device_info.c | 61 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_device_info.h | 23 +++++++++++-
 5 files changed, 108 insertions(+), 37 deletions(-)

Comments

Chris Wilson Feb. 22, 2018, 8:24 a.m. UTC | #1
Quoting Tvrtko Ursulin (2018-02-22 08:09:07)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Introduce subplatform mask to eliminate throughout the code devid checking
> sprinkle, mostly courtesy of IS_*_UL[TX] macros.
> 
> Subplatform mask initialization is moved either to static tables (Ironlake
> M) or runtime device info init (Pineview, Haswell, Broadwell, Skylake,
> Kabylake, Coffeelake and Cannonlake).
> 
>    text    data     bss     dec     hex filename
> 1673630   59691    5064 1738385  1a8691 i915.ko.0
> 1673214   59691    5064 1737969  1a84f1 i915.ko.1
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@intel.com>
> ---
> @@ -2624,38 +2631,19 @@ intel_info(const struct drm_i915_private *dev_priv)
>  #define IS_MOBILE(dev_priv)    ((dev_priv)->info.is_mobile)
>  #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
>                                     (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)

Killme. :)

> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 298f8996cc54..f5c9d29a7471 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -111,10 +111,11 @@ void intel_device_info_dump(const struct intel_device_info *info,
>         struct drm_i915_private *dev_priv =
>                 container_of(info, struct drm_i915_private, info);
>  
> -       drm_printf(p, "pciid=0x%04x rev=0x%02x platform=%s gen=%i\n",
> +       drm_printf(p, "pciid=0x%04x rev=0x%02x platform=%s (subplatform=%x) gen=%i\n",
>                    INTEL_DEVID(dev_priv),
>                    INTEL_REVID(dev_priv),
>                    intel_platform_name(info->platform),
> +                  info->subplatform_mask,
>                    info->gen);

Worth it.

>  
>         intel_device_info_dump_flags(info, p);
> @@ -458,6 +459,62 @@ static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
>         return 0;
>  }
>  
> +static void intel_device_info_subplatform_init(struct intel_device_info *info)
> +{
> +       struct drm_i915_private *i915 =
> +               container_of(info, struct drm_i915_private, info);
> +       u16 devid = INTEL_DEVID(i915);
> +
> +       if (IS_PINEVIEW(i915)) {

> -#define IS_PINEVIEW_G(dev_priv)        (INTEL_DEVID(dev_priv) == 0xa001)
> -#define IS_PINEVIEW_M(dev_priv)        (INTEL_DEVID(dev_priv) == 0xa011)

> +               if (devid == 0xa001)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_PINEVIEW_G;
> +               else if (devid == 0xa011)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_PINEVIEW_M;

Ok. Do we use IS_PINEVIEW_G/M? Should just be IS_PINEVIEW() &&
IS_MOBILE() but I guess after this change, IS_PINEVIEW_G/M is even
easier.

> +       } else if (IS_HASWELL(i915)) {
> +               if ((devid & 0xFF00) == 0x0A00)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
> +               /* ULX machines are also considered ULT. */
> +               if (devid == 0x0A0E || devid == 0x0A1E)
> +                       info->subplatform_mask |= INTEL_SUBPLATFORM_ULX;
> +       } else if (IS_BROADWELL(i915)) {

> -#define IS_BDW_ULT(dev_priv)   (IS_BROADWELL(dev_priv) && \
> -                                ((INTEL_DEVID(dev_priv) & 0xf) == 0x6 ||       \
> -                                (INTEL_DEVID(dev_priv) & 0xf) == 0xb ||        \
> -                                (INTEL_DEVID(dev_priv) & 0xf) == 0xe))

> +               if ((devid & 0xf) == 0x6 ||
> +                   (devid & 0xf) == 0xb ||
> +                   (devid & 0xf) == 0xe)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_ULT;

> -#define IS_BDW_ULX(dev_priv)   (IS_BROADWELL(dev_priv) && \
> -                                (INTEL_DEVID(dev_priv) & 0xf) == 0xe)

> +               /* ULX machines are also considered ULT. */
> +               if ((devid & 0xf) == 0xe)
> +                       info->subplatform_mask |= INTEL_SUBPLATFORM_ULX;

Ok.

> +       } else if (IS_SKYLAKE(i915)) {

> -#define IS_SKL_ULT(dev_priv)   (INTEL_DEVID(dev_priv) == 0x1906 || \
> -                                INTEL_DEVID(dev_priv) == 0x1913 || \
> -                                INTEL_DEVID(dev_priv) == 0x1916 || \
> -                                INTEL_DEVID(dev_priv) == 0x1921 || \
> -                                INTEL_DEVID(dev_priv) == 0x1926)

> +               if (devid == 0x1906 ||
> +                   devid == 0x1913 ||
> +                   devid == 0x1916 ||
> +                   devid == 0x1921 ||
> +                   devid == 0x1926)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_ULT;

> -#define IS_SKL_ULX(dev_priv)   (INTEL_DEVID(dev_priv) == 0x190E || \
> -                                INTEL_DEVID(dev_priv) == 0x1915 || \
> -                                INTEL_DEVID(dev_priv) == 0x191E)

> +               else if (devid == 0x190E ||
> +                        devid == 0x1915 ||
> +                        devid == 0x191E)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_ULX;

Ok.

> +       } else if (IS_KABYLAKE(i915)) {

> -#define IS_KBL_ULT(dev_priv)   (INTEL_DEVID(dev_priv) == 0x5906 || \
> -                                INTEL_DEVID(dev_priv) == 0x5913 || \
> -                                INTEL_DEVID(dev_priv) == 0x5916 || \
> -                                INTEL_DEVID(dev_priv) == 0x5921 || \
> -                                INTEL_DEVID(dev_priv) == 0x5926)

> +               if (devid == 0x5906 ||
> +                   devid == 0x5913 ||
> +                   devid == 0x5916 ||
> +                   devid == 0x5921 ||
> +                   devid  == 0x5926)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_ULT;

> -#define IS_KBL_ULX(dev_priv)   (INTEL_DEVID(dev_priv) == 0x590E || \
> -                                INTEL_DEVID(dev_priv) == 0x5915 || \
> -                                INTEL_DEVID(dev_priv) == 0x591E)

> +               else if (devid == 0x590E ||
> +                        devid == 0x5915 ||
> +                        devid == 0x591E)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_ULX;

Ok.

> +       } else if (IS_COFFEELAKE(i915)) {

> -#define IS_CFL_ULT(dev_priv)   (IS_COFFEELAKE(dev_priv) && \
> -                                (INTEL_DEVID(dev_priv) & 0x00F0) == 0x00A0)

> +                if ((devid & 0x00F0) == 0x00A0)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_ULT;

Ok.

> +       } else if (IS_CANNONLAKE(i915)) {

> -#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> -                                       (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)

> +               if ((devid & 0x0004) == 0x0004)
> +                       info->subplatform_mask = INTEL_SUBPLATFORM_PORTF;

Odd, but ok. (Wondering if this needs it own bit. It doesn't, ok.)

> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 71fdfb0451ef..7b6211061fba 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -74,6 +74,20 @@ enum intel_platform {
>         INTEL_MAX_PLATFORMS
>  };
>  
> +/* Subplatform flags share the same namespace per parent platform. */
> +
> +#define INTEL_SUBPLATFORM_BITS (2)

Enough space to do the same for GT (4 bits?) on top?

Looks good to me. I'm moaning at mkwrite_device_info() but that can
stand for now ;)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Chris Wilson Feb. 22, 2018, 8:35 a.m. UTC | #2
Quoting Patchwork (2018-02-22 08:30:36)
> == Series Details ==
> 
> Series: drm/i915: Eliminate devid sprinkle
> URL   : https://patchwork.freedesktop.org/series/38749/
> State : warning
> 
> == Summary ==
> 
> Series 38749v1 drm/i915: Eliminate devid sprinkle
> https://patchwork.freedesktop.org/api/1.0/series/38749/revisions/1/mbox/
> 
> Test gem_mmap_gtt:
>         Subgroup basic-small-bo-tiledx:
>                 fail       -> PASS       (fi-gdg-551) fdo#102575
> Test kms_busy:
>         Subgroup basic-flip-a:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-b:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-c:
>                 pass       -> SKIP       (fi-hsw-4770)
> Test kms_cursor_legacy:
>         Subgroup basic-busy-flip-before-cursor-atomic:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-busy-flip-before-cursor-legacy:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-after-cursor-atomic:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-after-cursor-legacy:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-after-cursor-varying-size:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-before-cursor-atomic:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-before-cursor-legacy:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-before-cursor-varying-size:
>                 pass       -> SKIP       (fi-hsw-4770)
> Test kms_flip:
>         Subgroup basic-flip-vs-dpms:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-vs-modeset:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-flip-vs-wf_vblank:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-plain-flip:
>                 pass       -> SKIP       (fi-hsw-4770)
> Test kms_frontbuffer_tracking:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-hsw-4770)
> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-a:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup hang-read-crc-pipe-b:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup hang-read-crc-pipe-c:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup nonblocking-crc-pipe-a:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup nonblocking-crc-pipe-a-frame-sequence:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup nonblocking-crc-pipe-b:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup nonblocking-crc-pipe-b-frame-sequence:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup nonblocking-crc-pipe-c:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup nonblocking-crc-pipe-c-frame-sequence:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup read-crc-pipe-a:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup read-crc-pipe-a-frame-sequence:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup read-crc-pipe-b:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup read-crc-pipe-b-frame-sequence:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup read-crc-pipe-c:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup read-crc-pipe-c-frame-sequence:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup suspend-read-crc-pipe-a:
>                 pass       -> SKIP       (fi-hsw-4770) fdo#104944
>         Subgroup suspend-read-crc-pipe-b:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup suspend-read-crc-pipe-c:
>                 pass       -> SKIP       (fi-hsw-4770)
>                 incomplete -> PASS       (fi-bxt-dsi) fdo#103927
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 pass       -> SKIP       (fi-hsw-4770)
>         Subgroup basic-rte:
>                 pass       -> SKIP       (fi-hsw-4770)
> Test prime_vgem:
>         Subgroup basic-fence-flip:
>                 pass       -> SKIP       (fi-hsw-4770)
> Test drv_module_reload:
>         Subgroup basic-reload:
>                 pass       -> DMESG-WARN (fi-hsw-4770)
>         Subgroup basic-no-display:
>                 pass       -> DMESG-WARN (fi-hsw-4770)
>         Subgroup basic-reload-inject:
>                 pass       -> DMESG-WARN (fi-hsw-4770)

I regret everything!

WARN_ON(((dev_priv)->info.platform_subplatform_mask & ((1UL << (INTEL_HASWELL)) | (1UL << ((32 - (2)) + (0))))) || ((dev_priv)->info.platform_subplatform_mask & ((1UL << (INTEL_BROADWELL)) | (1UL << ((32 - (2)) + (0))))))
WARNING: CPU: 0 PID: 4450 at drivers/gpu/drm/i915/i915_drv.c:146 intel_pch_type+0x4e3/0x510 [i915]

Looks just to be an ordering issue.
-Chris
Tvrtko Ursulin Feb. 22, 2018, 8:59 a.m. UTC | #3
On 22/02/2018 08:35, Chris Wilson wrote:
> Quoting Patchwork (2018-02-22 08:30:36)
>> == Series Details ==
>>
>> Series: drm/i915: Eliminate devid sprinkle
>> URL   : https://patchwork.freedesktop.org/series/38749/
>> State : warning
>>
>> == Summary ==
>>
>> Series 38749v1 drm/i915: Eliminate devid sprinkle
>> https://patchwork.freedesktop.org/api/1.0/series/38749/revisions/1/mbox/
>>
>> Test gem_mmap_gtt:
>>          Subgroup basic-small-bo-tiledx:
>>                  fail       -> PASS       (fi-gdg-551) fdo#102575
>> Test kms_busy:
>>          Subgroup basic-flip-a:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-b:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-c:
>>                  pass       -> SKIP       (fi-hsw-4770)
>> Test kms_cursor_legacy:
>>          Subgroup basic-busy-flip-before-cursor-atomic:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-busy-flip-before-cursor-legacy:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-after-cursor-atomic:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-after-cursor-legacy:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-after-cursor-varying-size:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-before-cursor-atomic:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-before-cursor-legacy:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-before-cursor-varying-size:
>>                  pass       -> SKIP       (fi-hsw-4770)
>> Test kms_flip:
>>          Subgroup basic-flip-vs-dpms:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-vs-modeset:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-flip-vs-wf_vblank:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-plain-flip:
>>                  pass       -> SKIP       (fi-hsw-4770)
>> Test kms_frontbuffer_tracking:
>>          Subgroup basic:
>>                  pass       -> SKIP       (fi-hsw-4770)
>> Test kms_pipe_crc_basic:
>>          Subgroup hang-read-crc-pipe-a:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup hang-read-crc-pipe-b:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup hang-read-crc-pipe-c:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup nonblocking-crc-pipe-a:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup nonblocking-crc-pipe-a-frame-sequence:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup nonblocking-crc-pipe-b:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup nonblocking-crc-pipe-b-frame-sequence:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup nonblocking-crc-pipe-c:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup nonblocking-crc-pipe-c-frame-sequence:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup read-crc-pipe-a:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup read-crc-pipe-a-frame-sequence:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup read-crc-pipe-b:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup read-crc-pipe-b-frame-sequence:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup read-crc-pipe-c:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup read-crc-pipe-c-frame-sequence:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup suspend-read-crc-pipe-a:
>>                  pass       -> SKIP       (fi-hsw-4770) fdo#104944
>>          Subgroup suspend-read-crc-pipe-b:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup suspend-read-crc-pipe-c:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>                  incomplete -> PASS       (fi-bxt-dsi) fdo#103927
>> Test pm_rpm:
>>          Subgroup basic-pci-d3-state:
>>                  pass       -> SKIP       (fi-hsw-4770)
>>          Subgroup basic-rte:
>>                  pass       -> SKIP       (fi-hsw-4770)
>> Test prime_vgem:
>>          Subgroup basic-fence-flip:
>>                  pass       -> SKIP       (fi-hsw-4770)
>> Test drv_module_reload:
>>          Subgroup basic-reload:
>>                  pass       -> DMESG-WARN (fi-hsw-4770)
>>          Subgroup basic-no-display:
>>                  pass       -> DMESG-WARN (fi-hsw-4770)
>>          Subgroup basic-reload-inject:
>>                  pass       -> DMESG-WARN (fi-hsw-4770)
> 
> I regret everything!
> 
> WARN_ON(((dev_priv)->info.platform_subplatform_mask & ((1UL << (INTEL_HASWELL)) | (1UL << ((32 - (2)) + (0))))) || ((dev_priv)->info.platform_subplatform_mask & ((1UL << (INTEL_BROADWELL)) | (1UL << ((32 - (2)) + (0))))))
> WARNING: CPU: 0 PID: 4450 at drivers/gpu/drm/i915/i915_drv.c:146 intel_pch_type+0x4e3/0x510 [i915]
> 
> Looks just to be an ordering issue.

Bug in the IS_SUBPLATFORM macro. It's checking for any bit set, but has 
to be both in this scheme. At least with that fixed text size is still 
in check.

Regards,

Tvrtko
Tvrtko Ursulin Feb. 22, 2018, 10:39 a.m. UTC | #4
On 22/02/2018 08:24, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-02-22 08:09:07)
> 
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
>> index 71fdfb0451ef..7b6211061fba 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.h
>> +++ b/drivers/gpu/drm/i915/intel_device_info.h
>> @@ -74,6 +74,20 @@ enum intel_platform {
>>          INTEL_MAX_PLATFORMS
>>   };
>>   
>> +/* Subplatform flags share the same namespace per parent platform. */
>> +
>> +#define INTEL_SUBPLATFORM_BITS (2)
> 
> Enough space to do the same for GT (4 bits?) on top?

Nope, only 1 bit remains with this patch. Maybe 2 if Jani lets me do 
BIT(platform - 1) for the mask. ;) You could also release your Gen1 bit. :D

Not sure if there is scope (makes sense or not) to free up some more 
bits by downgrading some of the I9[146]5GM platforms to be subplatforms 
of I9[146]G. G[M]45 also.

Or how IS_MOBILE fits in this scheme.

But anyway, not enough for gen in any case.

I think I've prototyped aggregated u64 platform+gen mask in some branch 
though. It is a small code increase overall due 64 bit immediates.

Regards,

Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index aaa861b51024..ca624285c96e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -905,7 +905,8 @@  static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	device_info->device_id = dev_priv->drm.pdev->device;
 
 	BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
-		     sizeof(device_info->platform_mask) * BITS_PER_BYTE);
+		     sizeof(device_info->platform_subplatform_mask) *
+		     BITS_PER_BYTE - INTEL_SUBPLATFORM_BITS);
 	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
 	spin_lock_init(&dev_priv->irq_lock);
 	spin_lock_init(&dev_priv->gpu_error.lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 82a106b1bdbc..400fff0320fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2588,6 +2588,9 @@  intel_info(const struct drm_i915_private *dev_priv)
 	(INTEL_REVID(p) >= (since) && INTEL_REVID(p) <= (until))
 
 #define IS_PLATFORM(dev_priv, p) ((dev_priv)->info.platform_mask & BIT(p))
+#define IS_SUBPLATFORM(dev_priv, p, s) \
+	((dev_priv)->info.platform_subplatform_mask & \
+	 (BIT(p) | BIT((32 - INTEL_SUBPLATFORM_BITS) + INTEL_SUBPLATFORM_##s)))
 
 #define IS_I830(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I830)
 #define IS_I845G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I845G)
@@ -2602,11 +2605,15 @@  intel_info(const struct drm_i915_private *dev_priv)
 #define IS_G45(dev_priv)	IS_PLATFORM(dev_priv, INTEL_G45)
 #define IS_GM45(dev_priv)	IS_PLATFORM(dev_priv, INTEL_GM45)
 #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
-#define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
-#define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
+#define IS_PINEVIEW_G(dev_priv)	\
+	IS_SUBPLATFORM(dev_priv, INTEL_PINEVIEW, PINEVIEW_G)
+#define IS_PINEVIEW_M(dev_priv)	\
+	IS_SUBPLATFORM(dev_priv, INTEL_PINEVIEW, PINEVIEW_M)
 #define IS_PINEVIEW(dev_priv)	IS_PLATFORM(dev_priv, INTEL_PINEVIEW)
 #define IS_G33(dev_priv)	IS_PLATFORM(dev_priv, INTEL_G33)
-#define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
+#define IS_IRONLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_IRONLAKE)
+#define IS_IRONLAKE_M(dev_priv)	\
+	IS_SUBPLATFORM(dev_priv, INTEL_IRONLAKE, IRONLAKE_M)
 #define IS_IVYBRIDGE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_IVYBRIDGE)
 #define IS_IVB_GT1(dev_priv)	(IS_IVYBRIDGE(dev_priv) && \
 				 (dev_priv)->info.gt == 1)
@@ -2624,38 +2631,19 @@  intel_info(const struct drm_i915_private *dev_priv)
 #define IS_MOBILE(dev_priv)	((dev_priv)->info.is_mobile)
 #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
 				    (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)
-#define IS_BDW_ULT(dev_priv)	(IS_BROADWELL(dev_priv) && \
-				 ((INTEL_DEVID(dev_priv) & 0xf) == 0x6 ||	\
-				 (INTEL_DEVID(dev_priv) & 0xf) == 0xb ||	\
-				 (INTEL_DEVID(dev_priv) & 0xf) == 0xe))
-/* ULX machines are also considered ULT. */
-#define IS_BDW_ULX(dev_priv)	(IS_BROADWELL(dev_priv) && \
-				 (INTEL_DEVID(dev_priv) & 0xf) == 0xe)
+#define IS_BDW_ULT(dev_priv)	IS_SUBPLATFORM(dev_priv, INTEL_BROADWELL, ULT)
+#define IS_BDW_ULX(dev_priv)	IS_SUBPLATFORM(dev_priv, INTEL_BROADWELL, ULX)
 #define IS_BDW_GT3(dev_priv)	(IS_BROADWELL(dev_priv) && \
 				 (dev_priv)->info.gt == 3)
-#define IS_HSW_ULT(dev_priv)	(IS_HASWELL(dev_priv) && \
-				 (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0A00)
+#define IS_HSW_ULT(dev_priv)	IS_SUBPLATFORM(dev_priv, INTEL_HASWELL, ULT)
 #define IS_HSW_GT3(dev_priv)	(IS_HASWELL(dev_priv) && \
 				 (dev_priv)->info.gt == 3)
 /* ULX machines are also considered ULT. */
-#define IS_HSW_ULX(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0A0E || \
-				 INTEL_DEVID(dev_priv) == 0x0A1E)
-#define IS_SKL_ULT(dev_priv)	(INTEL_DEVID(dev_priv) == 0x1906 || \
-				 INTEL_DEVID(dev_priv) == 0x1913 || \
-				 INTEL_DEVID(dev_priv) == 0x1916 || \
-				 INTEL_DEVID(dev_priv) == 0x1921 || \
-				 INTEL_DEVID(dev_priv) == 0x1926)
-#define IS_SKL_ULX(dev_priv)	(INTEL_DEVID(dev_priv) == 0x190E || \
-				 INTEL_DEVID(dev_priv) == 0x1915 || \
-				 INTEL_DEVID(dev_priv) == 0x191E)
-#define IS_KBL_ULT(dev_priv)	(INTEL_DEVID(dev_priv) == 0x5906 || \
-				 INTEL_DEVID(dev_priv) == 0x5913 || \
-				 INTEL_DEVID(dev_priv) == 0x5916 || \
-				 INTEL_DEVID(dev_priv) == 0x5921 || \
-				 INTEL_DEVID(dev_priv) == 0x5926)
-#define IS_KBL_ULX(dev_priv)	(INTEL_DEVID(dev_priv) == 0x590E || \
-				 INTEL_DEVID(dev_priv) == 0x5915 || \
-				 INTEL_DEVID(dev_priv) == 0x591E)
+#define IS_HSW_ULX(dev_priv)	IS_SUBPLATFORM(dev_priv, INTEL_HASWELL, ULX)
+#define IS_SKL_ULT(dev_priv)	IS_SUBPLATFORM(dev_priv, INTEL_SKYLAKE, ULT)
+#define IS_SKL_ULX(dev_priv)	IS_SUBPLATFORM(dev_priv, INTEL_SKYLAKE, ULX)
+#define IS_KBL_ULT(dev_priv)	IS_SUBPLATFORM(dev_priv, INTEL_KABYLAKE, ULT)
+#define IS_KBL_ULX(dev_priv)	IS_SUBPLATFORM(dev_priv, INTEL_KABYLAKE, ULX)
 #define IS_SKL_GT2(dev_priv)	(IS_SKYLAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 2)
 #define IS_SKL_GT3(dev_priv)	(IS_SKYLAKE(dev_priv) && \
@@ -2666,14 +2654,13 @@  intel_info(const struct drm_i915_private *dev_priv)
 				 (dev_priv)->info.gt == 2)
 #define IS_KBL_GT3(dev_priv)	(IS_KABYLAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 3)
-#define IS_CFL_ULT(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
-				 (INTEL_DEVID(dev_priv) & 0x00F0) == 0x00A0)
+#define IS_CFL_ULT(dev_priv)	IS_SUBPLATFORM(dev_priv, INTEL_COFFEELAKE, ULT)
 #define IS_CFL_GT2(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 2)
 #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 3)
-#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
-					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
+#define IS_CNL_WITH_PORT_F(dev_priv) \
+	IS_SUBPLATFORM(dev_priv, INTEL_CANNONLAKE, PORTF)
 
 #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
 
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 1eaabf28d7b7..9e2967f7c583 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -30,6 +30,7 @@ 
 #include "i915_selftest.h"
 
 #define PLATFORM(x) .platform = (x), .platform_mask = BIT(x)
+#define SUBPLATFORM(x) .subplatform_mask = BIT(INTEL_SUBPLATFORM_##x)
 #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
 
 #define GEN_DEFAULT_PIPEOFFSETS \
@@ -234,6 +235,7 @@  static const struct intel_device_info intel_ironlake_d_info = {
 static const struct intel_device_info intel_ironlake_m_info = {
 	GEN5_FEATURES,
 	PLATFORM(INTEL_IRONLAKE),
+	SUBPLATFORM(IRONLAKE_M),
 	.is_mobile = 1, .has_fbc = 1,
 };
 
@@ -605,6 +607,7 @@  static const struct intel_device_info intel_icelake_11_info = {
 
 #undef GEN
 #undef PLATFORM
+#undef SUBPLATFORM
 
 /*
  * Make sure any device matches here are from most specific to most
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 298f8996cc54..f5c9d29a7471 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -111,10 +111,11 @@  void intel_device_info_dump(const struct intel_device_info *info,
 	struct drm_i915_private *dev_priv =
 		container_of(info, struct drm_i915_private, info);
 
-	drm_printf(p, "pciid=0x%04x rev=0x%02x platform=%s gen=%i\n",
+	drm_printf(p, "pciid=0x%04x rev=0x%02x platform=%s (subplatform=%x) gen=%i\n",
 		   INTEL_DEVID(dev_priv),
 		   INTEL_REVID(dev_priv),
 		   intel_platform_name(info->platform),
+		   info->subplatform_mask,
 		   info->gen);
 
 	intel_device_info_dump_flags(info, p);
@@ -458,6 +459,62 @@  static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
+static void intel_device_info_subplatform_init(struct intel_device_info *info)
+{
+	struct drm_i915_private *i915 =
+		container_of(info, struct drm_i915_private, info);
+	u16 devid = INTEL_DEVID(i915);
+
+	if (IS_PINEVIEW(i915)) {
+		if (devid == 0xa001)
+			info->subplatform_mask = INTEL_SUBPLATFORM_PINEVIEW_G;
+		else if (devid == 0xa011)
+			info->subplatform_mask = INTEL_SUBPLATFORM_PINEVIEW_M;
+	} else if (IS_HASWELL(i915)) {
+		if ((devid & 0xFF00) == 0x0A00)
+			info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
+		/* ULX machines are also considered ULT. */
+		if (devid == 0x0A0E || devid == 0x0A1E)
+			info->subplatform_mask |= INTEL_SUBPLATFORM_ULX;
+	} else if (IS_BROADWELL(i915)) {
+		if ((devid & 0xf) == 0x6 ||
+		    (devid & 0xf) == 0xb ||
+		    (devid & 0xf) == 0xe)
+			info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
+		/* ULX machines are also considered ULT. */
+		if ((devid & 0xf) == 0xe)
+			info->subplatform_mask |= INTEL_SUBPLATFORM_ULX;
+	} else if (IS_SKYLAKE(i915)) {
+		if (devid == 0x1906 ||
+		    devid == 0x1913 ||
+		    devid == 0x1916 ||
+		    devid == 0x1921 ||
+		    devid == 0x1926)
+			info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
+		else if (devid == 0x190E ||
+			 devid == 0x1915 ||
+			 devid == 0x191E)
+			info->subplatform_mask = INTEL_SUBPLATFORM_ULX;
+	} else if (IS_KABYLAKE(i915)) {
+		if (devid == 0x5906 ||
+		    devid == 0x5913 ||
+		    devid == 0x5916 ||
+		    devid == 0x5921 ||
+		    devid  == 0x5926)
+			info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
+		else if (devid == 0x590E ||
+			 devid == 0x5915 ||
+			 devid == 0x591E)
+			info->subplatform_mask = INTEL_SUBPLATFORM_ULX;
+	} else if (IS_COFFEELAKE(i915)) {
+		 if ((devid & 0x00F0) == 0x00A0)
+			info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
+	} else if (IS_CANNONLAKE(i915)) {
+		if ((devid & 0x0004) == 0x0004)
+			info->subplatform_mask = INTEL_SUBPLATFORM_PORTF;
+	}
+}
+
 /**
  * intel_device_info_runtime_init - initialize runtime info
  * @info: intel device info struct
@@ -480,6 +537,8 @@  void intel_device_info_runtime_init(struct intel_device_info *info)
 		container_of(info, struct drm_i915_private, info);
 	enum pipe pipe;
 
+	intel_device_info_subplatform_init(info);
+
 	if (INTEL_GEN(dev_priv) >= 10) {
 		for_each_pipe(dev_priv, pipe)
 			info->num_scalers[pipe] = 2;
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 71fdfb0451ef..7b6211061fba 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -74,6 +74,20 @@  enum intel_platform {
 	INTEL_MAX_PLATFORMS
 };
 
+/* Subplatform flags share the same namespace per parent platform. */
+
+#define INTEL_SUBPLATFORM_BITS (2)
+
+#define INTEL_SUBPLATFORM_IRONLAKE_M (0)
+
+#define INTEL_SUBPLATFORM_PINEVIEW_G (0)
+#define INTEL_SUBPLATFORM_PINEVIEW_M (1)
+
+#define INTEL_SUBPLATFORM_ULT (0)
+#define INTEL_SUBPLATFORM_ULX (1)
+
+#define INTEL_SUBPLATFORM_PORTF (0)
+
 #define DEV_INFO_FOR_EACH_FLAG(func) \
 	func(is_mobile); \
 	func(is_lp); \
@@ -135,7 +149,14 @@  struct intel_device_info {
 	u8 ring_mask; /* Rings supported by the HW */
 
 	enum intel_platform platform;
-	u32 platform_mask;
+
+	union {
+		u32 platform_subplatform_mask;
+		struct {
+			u32 platform_mask : (32 - INTEL_SUBPLATFORM_BITS);
+			u32 subplatform_mask : INTEL_SUBPLATFORM_BITS;
+		};
+	};
 
 	u32 display_mmio_offset;