On Fri, Mar 15, 2019 at 12:26:33PM +0000, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Concept of a sub-platform already exist in our code (like ULX and ULT
platform variants and similar),implemented via the macros which check a
list of device ids to determine a match.
With this patch we consolidate device ids checking into a single function
called during early driver load.
A few low bits in the platform mask are reserved for sub-platform
identification and defined as a per-platform namespace.
At the same time it future proofs the platform_mask handling by preparing
the code for easy extending, and tidies the very verbose WARN strings
generated when IS_PLATFORM macros are embedded into a WARN type
statements.
The approach is also beneficial to driver size, with an combined shrink of
code and strings of around 1.7 kiB.
v2: Fixed IS_SUBPLATFORM. Updated commit msg.
v3: Chris was right, there is an ordering problem.
v4:
* Catch-up with new sub-platforms.
* Rebase for RUNTIME_INFO.
* Drop subplatform mask union tricks and convert platform_mask to an
array for extensibility.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Cc: Jose Souza <jose.souza@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 7 +-
drivers/gpu/drm/i915/i915_drv.h | 110 +++++++++++++++--------
drivers/gpu/drm/i915/i915_pci.c | 2 +-
drivers/gpu/drm/i915/intel_device_info.c | 79 ++++++++++++++++
drivers/gpu/drm/i915/intel_device_info.h | 28 +++++-
5 files changed, 179 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0d743907e7bc..3218350cd225 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -863,6 +863,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
if (i915_inject_load_failure())
return -ENODEV;
+ intel_device_info_subplatform_init(dev_priv);
+
spin_lock_init(&dev_priv->irq_lock);
spin_lock_init(&dev_priv->gpu_error.lock);
mutex_init(&dev_priv->backlight_lock);
@@ -1752,10 +1754,11 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv)
if (drm_debug & DRM_UT_DRIVER) {
struct drm_printer p = drm_debug_printer("i915 device 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 (%x) gen=%i\n",
INTEL_DEVID(dev_priv),
INTEL_REVID(dev_priv),
intel_platform_name(INTEL_INFO(dev_priv)->platform),
+ RUNTIME_INFO(dev_priv)->platform_mask[INTEL_INFO(dev_priv)->platform / (BITS_PER_TYPE(RUNTIME_INFO(dev_priv)->platform_mask[0]) - INTEL_SUBPLATFORM_BITS)],
bug here, INTEL_SUBPLATFORM_BITS should be outside of []. Bad things
will happen for platform=32 /o\
INTEL_GEN(dev_priv));
intel_device_info_dump_flags(INTEL_INFO(dev_priv), &p);
@@ -1798,8 +1801,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;
- BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
- BITS_PER_TYPE(device_info->platform_mask));
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 dccb6006aabf..34282cf66cb0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2281,7 +2281,46 @@ static inline unsigned int i915_sg_segment_size(void)
#define IS_REVID(p, since, until) \
(INTEL_REVID(p) >= (since) && INTEL_REVID(p) <= (until))
-#define IS_PLATFORM(dev_priv, p) (INTEL_INFO(dev_priv)->platform_mask & BIT(p))
+#define __IS_PLATFORM(dev_priv, p) \
+({ \
+ const unsigned int pbits__ = \
+ BITS_PER_TYPE(RUNTIME_INFO(dev_priv)->platform_mask[0]) - \
+ INTEL_SUBPLATFORM_BITS; \
+ const unsigned int pi__ = (p) / pbits__; \
+ const unsigned int pb__ = (p) % pbits__ + INTEL_SUBPLATFORM_BITS; \
+\
+ BUILD_BUG_ON(!__builtin_constant_p(p)); \
+\
+ (RUNTIME_INFO(dev_priv)->platform_mask[pi__] & BIT(pb__)); \
Ugh. That double dword fiddling is way too ugly. IMO it is not buying us
anything. Just use a u64 rather than the double dword. Your approach may
have a small benefit on ARCH=i386, but has the burden of carrying all
this forward. The diff below (only build-tested) is on top of yours,
which is basically equivalent to "move to u64 and then add the
subplatform part".
text data bss dec hex filename
1834620 40454 4176 1879250 1cacd2 drivers/gpu/drm/i915/i915.o.yours
1834710 40454 4176 1879340 1cad2c drivers/gpu/drm/i915/i915.o
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3218350cd225..c8042f4579c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1754,11 +1754,11 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv)
if (drm_debug & DRM_UT_DRIVER) {
struct drm_printer p = drm_debug_printer("i915 device info:");
- drm_printf(&p, "pciid=0x%04x rev=0x%02x platform=%s (%x) gen=%i\n",
+ drm_printf(&p, "pciid=0x%04x rev=0x%02x platform=%s (%#llx) gen=%i\n",
INTEL_DEVID(dev_priv),
INTEL_REVID(dev_priv),
intel_platform_name(INTEL_INFO(dev_priv)->platform),
- RUNTIME_INFO(dev_priv)->platform_mask[INTEL_INFO(dev_priv)->platform / (BITS_PER_TYPE(RUNTIME_INFO(dev_priv)->platform_mask[0]) - INTEL_SUBPLATFORM_BITS)],
+ RUNTIME_INFO(dev_priv)->platform_mask,
INTEL_GEN(dev_priv));
intel_device_info_dump_flags(INTEL_INFO(dev_priv), &p);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 34282cf66cb0..bd5d31e07a13 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2283,43 +2283,33 @@ static inline unsigned int i915_sg_segment_size(void)
#define __IS_PLATFORM(dev_priv, p) \
({ \
- const unsigned int pbits__ = \
- BITS_PER_TYPE(RUNTIME_INFO(dev_priv)->platform_mask[0]) - \
- INTEL_SUBPLATFORM_BITS; \
- const unsigned int pi__ = (p) / pbits__; \
- const unsigned int pb__ = (p) % pbits__ + INTEL_SUBPLATFORM_BITS; \
-\
- BUILD_BUG_ON(!__builtin_constant_p(p)); \
-\
- (RUNTIME_INFO(dev_priv)->platform_mask[pi__] & BIT(pb__)); \
})
#define __IS_SUBPLATFORM(dev_priv, p, s) \
({ \
- const unsigned int pbits__ = \
- BITS_PER_TYPE(RUNTIME_INFO(dev_priv)->platform_mask[0]) - \
- INTEL_SUBPLATFORM_BITS; \
- const unsigned int pi__ = (p) / pbits__; \
- const unsigned int pb__ = (p) % pbits__ + INTEL_SUBPLATFORM_BITS; \
-\
- BUILD_BUG_ON(!__builtin_constant_p(p)); \
- BUILD_BUG_ON(!__builtin_constant_p(s)); \
- BUILD_BUG_ON((s) >= INTEL_SUBPLATFORM_BITS); \
-\
- (RUNTIME_INFO(dev_priv)->platform_mask[pi__] & (BIT(pb__) | BIT(s))); \
})
-static inline bool
+__always_inline static inline bool
IS_PLATFORM(const struct drm_i915_private *i915, enum intel_platform p)
{
- return __IS_PLATFORM(i915, p);
+ const unsigned int pbit = p + INTEL_SUBPLATFORM_BITS;
+
+ BUILD_BUG_ON(!__builtin_constant_p(p));
+
+ return RUNTIME_INFO(i915)->platform_mask & BIT(pbit);
}
-static inline bool
+__always_inline static inline bool
IS_SUBPLATFORM(const struct drm_i915_private *i915,
enum intel_platform p, unsigned int s)
{
- return __IS_SUBPLATFORM(i915, p, s);
+ const unsigned int pbit = p + INTEL_SUBPLATFORM_BITS;
+
+ BUILD_BUG_ON(!__builtin_constant_p(p));
+ BUILD_BUG_ON(!__builtin_constant_p(s));
+ BUILD_BUG_ON((s) >= INTEL_SUBPLATFORM_BITS);
+
+ return RUNTIME_INFO(i915)->platform_mask & (BIT(pbit) | BIT(s));
}
#define IS_I830(dev_priv) IS_PLATFORM(dev_priv, INTEL_I830)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index b8a7e17cb443..1cdd25486c04 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -709,80 +709,76 @@ static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
void intel_device_info_subplatform_init(struct drm_i915_private *i915)
{
- const unsigned int pbits =
- BITS_PER_TYPE(RUNTIME_INFO(i915)->platform_mask[0]) -
- INTEL_SUBPLATFORM_BITS;
- const unsigned int pi = INTEL_INFO(i915)->platform / pbits;
- const unsigned int pb =
- INTEL_INFO(i915)->platform % pbits + INTEL_SUBPLATFORM_BITS;
- struct intel_runtime_info *info = RUNTIME_INFO(i915);
+ struct intel_runtime_info *rinfo = RUNTIME_INFO(i915);
+ const unsigned int pbit = INTEL_INFO(i915)->platform
+ + INTEL_SUBPLATFORM_BITS;
u16 devid = INTEL_DEVID(i915);
- BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
- pbits * ARRAY_SIZE(RUNTIME_INFO(i915)->platform_mask));
+ BUILD_BUG_ON(INTEL_MAX_PLATFORMS - INTEL_SUBPLATFORM_BITS >
+ BITS_PER_TYPE(rinfo->platform_mask));
- info->platform_mask[pi] = BIT(pb);
+ rinfo->platform_mask = BIT_ULL(pbit);
if (IS_PINEVIEW(i915)) {
if (devid == 0xa001)
- info->platform_mask[pi] |=
+ rinfo->platform_mask |=
BIT(INTEL_SUBPLATFORM_PINEVIEW_G);
else if (devid == 0xa011)
- info->platform_mask[pi] |=
+ rinfo->platform_mask |=
BIT(INTEL_SUBPLATFORM_PINEVIEW_M);
} else if (IS_IRONLAKE(i915)) {
if (devid == 0x0046)
- info->platform_mask[pi] |=
+ rinfo->platform_mask |=
BIT(INTEL_SUBPLATFORM_IRONLAKE_M);
} else if (IS_HASWELL(i915)) {
if ((devid & 0xFF00) == 0x0A00)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_ULT);
/* ULX machines are also considered ULT. */
if (devid == 0x0A0E || devid == 0x0A1E)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULX);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_ULX);
} else if (IS_BROADWELL(i915)) {
if ((devid & 0xf) == 0x6 ||
(devid & 0xf) == 0xb ||
(devid & 0xf) == 0xe)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_ULT);
/* ULX machines are also considered ULT. */
if ((devid & 0xf) == 0xe)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULX);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_ULX);
} else if (IS_SKYLAKE(i915)) {
if (devid == 0x1906 ||
devid == 0x1913 ||
devid == 0x1916 ||
devid == 0x1921 ||
devid == 0x1926)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_ULT);
else if (devid == 0x190E ||
devid == 0x1915 ||
devid == 0x191E)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULX);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_ULX);
} else if (IS_KABYLAKE(i915)) {
if (devid == 0x5906 ||
devid == 0x5913 ||
devid == 0x5916 ||
devid == 0x5921 ||
devid == 0x5926)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_ULT);
else if (devid == 0x590E ||
devid == 0x5915 ||
devid == 0x591E)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULX);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_ULX);
else if (devid == 0x591C ||
devid == 0x87C0)
- info->platform_mask[pi] |=
+ rinfo->platform_mask |=
BIT(INTEL_SUBPLATFORM_AML_ULX);
} else if (IS_COFFEELAKE(i915)) {
if ((devid & 0x00F0) == 0x00A0)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_ULT);
} else if (IS_CANNONLAKE(i915)) {
if ((devid & 0x0004) == 0x0004)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_PORTF);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_PORTF);
} else if (IS_ICELAKE(i915)) {
if (devid != 0x8A51)
- info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_PORTF);
+ rinfo->platform_mask |= BIT(INTEL_SUBPLATFORM_PORTF);
}
}
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index b03fbd2e451a..12ff04a25b51 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -212,7 +212,7 @@ struct intel_device_info {
};
struct intel_runtime_info {
- u32 platform_mask[1];
+ u64 platform_mask;
u16 device_id;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
