Re: [Intel-gfx] [PATCH v8 4/4] drm/i915: Introduce concept of a sub-platform
On Fri, 29 Mar 2019, Tvrtko Ursulin wrote:
> On 29/03/2019 09:54, Jani Nikula wrote:
>> On Wed, 27 Mar 2019, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> v5:
>>> * Fix subplatform check.
>>> * Protect against forgetting to expand subplatform bits.
>>> * Remove platform enum tallying.
>>> * Add subplatform to error state. (Chris)
>>> * Drop macros and just use static inlines.
>>> * Remove redundant IRONLAKE_M. (Ville)
>>>
>>> v6:
>>> * Split out Ironlake change.
>>> * Optimize subplatform check.
>>> * Use __always_inline. (Lucas)
>>> * Add platform_mask comment. (Paulo)
>>> * Pass stored runtime info in error capture. (Chris)
>>>
>>> v7:
>>> * Rebased for new AML ULX device id.
>>> * Bump platform mask array size for EHL.
>>> * Stop mentioning device ids in intel_device_subplatform_init by using
>>> the trick of splitting macros i915_pciids.h. (Jani)
>>> * AML seems to be either a subplatform of KBL or CFL so express it like
>>> that.
>>>
>>> v8:
>>> * Use one device id table per subplatform. (Jani)
>>>
>>> Signed-off-by: Tvrtko Ursulin
>>> Suggested-by: Chris Wilson
>>> Cc: Chris Wilson
>>> Cc: Jani Nikula
>>> Cc: Lucas De Marchi
>>> Cc: Jose Souza
>>> Cc: Ville Syrjälä
>>> Cc: Paulo Zanoni
>>> Reviewed-by: Chris Wilson # v6
>>> ---
>>> drivers/gpu/drm/i915/i915_drv.c | 8 +-
>>> drivers/gpu/drm/i915/i915_drv.h | 123 ---
>>> drivers/gpu/drm/i915/i915_gpu_error.c| 3 +
>>> drivers/gpu/drm/i915/i915_pci.c | 2 +-
>>> drivers/gpu/drm/i915/intel_device_info.c | 93 +
>>> drivers/gpu/drm/i915/intel_device_info.h | 27 -
>>> 6 files changed, 214 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>>> b/drivers/gpu/drm/i915/i915_drv.c
>>> index f1334f5d4ead..74734d7661e5 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.c
>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>> @@ -868,6 +868,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);
>>> @@ -1718,10 +1720,12 @@ 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
>>> (subplatform=0x%x) gen=%i\n",
>>>INTEL_DEVID(dev_priv),
>>>INTEL_REVID(dev_priv),
>>>intel_platform_name(INTEL_INFO(dev_priv)->platform),
>>> + intel_subplatform(RUNTIME_INFO(dev_priv),
>>> +INTEL_INFO(dev_priv)->platform),
>>>INTEL_GEN(dev_priv));
>>>
>>> intel_device_info_dump_flags(INTEL_INFO(dev_priv), &p);
>>> @@ -1764,8 +1768,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 9d3cab9406e1..b7d3f3a45ed9 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -2298,7 +2298,67 @@ static inline unsigned int i915_sg_segment_size(void)
>>> #define IS_REVID(p, since, until) \
>>> (INTEL
Re: [Intel-gfx] [PATCH v8 4/4] drm/i915: Introduce concept of a sub-platform
On 29/03/2019 09:54, Jani Nikula wrote:
On Wed, 27 Mar 2019, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
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.
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.
v5:
* Fix subplatform check.
* Protect against forgetting to expand subplatform bits.
* Remove platform enum tallying.
* Add subplatform to error state. (Chris)
* Drop macros and just use static inlines.
* Remove redundant IRONLAKE_M. (Ville)
v6:
* Split out Ironlake change.
* Optimize subplatform check.
* Use __always_inline. (Lucas)
* Add platform_mask comment. (Paulo)
* Pass stored runtime info in error capture. (Chris)
v7:
* Rebased for new AML ULX device id.
* Bump platform mask array size for EHL.
* Stop mentioning device ids in intel_device_subplatform_init by using
the trick of splitting macros i915_pciids.h. (Jani)
* AML seems to be either a subplatform of KBL or CFL so express it like
that.
v8:
* Use one device id table per subplatform. (Jani)
Signed-off-by: Tvrtko Ursulin
Suggested-by: Chris Wilson
Cc: Chris Wilson
Cc: Jani Nikula
Cc: Lucas De Marchi
Cc: Jose Souza
Cc: Ville Syrjälä
Cc: Paulo Zanoni
Reviewed-by: Chris Wilson # v6
---
drivers/gpu/drm/i915/i915_drv.c | 8 +-
drivers/gpu/drm/i915/i915_drv.h | 123 ---
drivers/gpu/drm/i915/i915_gpu_error.c| 3 +
drivers/gpu/drm/i915/i915_pci.c | 2 +-
drivers/gpu/drm/i915/intel_device_info.c | 93 +
drivers/gpu/drm/i915/intel_device_info.h | 27 -
6 files changed, 214 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f1334f5d4ead..74734d7661e5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -868,6 +868,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);
@@ -1718,10 +1720,12 @@ 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
(subplatform=0x%x) gen=%i\n",
INTEL_DEVID(dev_priv),
INTEL_REVID(dev_priv),
intel_platform_name(INTEL_INFO(dev_priv)->platform),
+ intel_subplatform(RUNTIME_INFO(dev_priv),
+INTEL_INFO(dev_priv)->platform),
INTEL_GEN(dev_priv));
intel_device_info_dump_flags(INTEL_INFO(dev_priv), &p);
@@ -1764,8 +1768,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 9d3cab9406e1..b7d3f3a45ed9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2298,7 +2298,67 @@ 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))
+static __always_inline unsigned int
+__platform_mask_index(const struct intel_runtime_info *info,
+ enum intel_platform p)
+{
+ const unsigned int pbits =
+ BITS_PER_TYPE(info->platform_mask[0]) - INTEL_SUBPLATFORM_BITS;
+
+ /* Expand the platform_mask array if this fa
Re: [Intel-gfx] [PATCH v8 4/4] drm/i915: Introduce concept of a sub-platform
On Wed, 27 Mar 2019, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> 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.
>
> 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.
>
> v5:
> * Fix subplatform check.
> * Protect against forgetting to expand subplatform bits.
> * Remove platform enum tallying.
> * Add subplatform to error state. (Chris)
> * Drop macros and just use static inlines.
> * Remove redundant IRONLAKE_M. (Ville)
>
> v6:
> * Split out Ironlake change.
> * Optimize subplatform check.
> * Use __always_inline. (Lucas)
> * Add platform_mask comment. (Paulo)
> * Pass stored runtime info in error capture. (Chris)
>
> v7:
> * Rebased for new AML ULX device id.
> * Bump platform mask array size for EHL.
> * Stop mentioning device ids in intel_device_subplatform_init by using
>the trick of splitting macros i915_pciids.h. (Jani)
> * AML seems to be either a subplatform of KBL or CFL so express it like
>that.
>
> v8:
> * Use one device id table per subplatform. (Jani)
>
> Signed-off-by: Tvrtko Ursulin
> Suggested-by: Chris Wilson
> Cc: Chris Wilson
> Cc: Jani Nikula
> Cc: Lucas De Marchi
> Cc: Jose Souza
> Cc: Ville Syrjälä
> Cc: Paulo Zanoni
> Reviewed-by: Chris Wilson # v6
> ---
> drivers/gpu/drm/i915/i915_drv.c | 8 +-
> drivers/gpu/drm/i915/i915_drv.h | 123 ---
> drivers/gpu/drm/i915/i915_gpu_error.c| 3 +
> drivers/gpu/drm/i915/i915_pci.c | 2 +-
> drivers/gpu/drm/i915/intel_device_info.c | 93 +
> drivers/gpu/drm/i915/intel_device_info.h | 27 -
> 6 files changed, 214 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f1334f5d4ead..74734d7661e5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -868,6 +868,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);
> @@ -1718,10 +1720,12 @@ 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
> (subplatform=0x%x) gen=%i\n",
> INTEL_DEVID(dev_priv),
> INTEL_REVID(dev_priv),
> intel_platform_name(INTEL_INFO(dev_priv)->platform),
> +intel_subplatform(RUNTIME_INFO(dev_priv),
> + INTEL_INFO(dev_priv)->platform),
> INTEL_GEN(dev_priv));
>
> intel_device_info_dump_flags(INTEL_INFO(dev_priv), &p);
> @@ -1764,8 +1768,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 9d3cab9406e1..b7d3f3a45ed9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2298,7 +2298,67 @@ 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))
> +static __always_inline unsigned int
> +__platform_mask_index(const struct intel_runtime_info *info,
> + enum intel_platform p)
> +{
> + const unsigned int
[Intel-gfx] [PATCH v8 4/4] drm/i915: Introduce concept of a sub-platform
From: Tvrtko Ursulin
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.
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.
v5:
* Fix subplatform check.
* Protect against forgetting to expand subplatform bits.
* Remove platform enum tallying.
* Add subplatform to error state. (Chris)
* Drop macros and just use static inlines.
* Remove redundant IRONLAKE_M. (Ville)
v6:
* Split out Ironlake change.
* Optimize subplatform check.
* Use __always_inline. (Lucas)
* Add platform_mask comment. (Paulo)
* Pass stored runtime info in error capture. (Chris)
v7:
* Rebased for new AML ULX device id.
* Bump platform mask array size for EHL.
* Stop mentioning device ids in intel_device_subplatform_init by using
the trick of splitting macros i915_pciids.h. (Jani)
* AML seems to be either a subplatform of KBL or CFL so express it like
that.
v8:
* Use one device id table per subplatform. (Jani)
Signed-off-by: Tvrtko Ursulin
Suggested-by: Chris Wilson
Cc: Chris Wilson
Cc: Jani Nikula
Cc: Lucas De Marchi
Cc: Jose Souza
Cc: Ville Syrjälä
Cc: Paulo Zanoni
Reviewed-by: Chris Wilson # v6
---
drivers/gpu/drm/i915/i915_drv.c | 8 +-
drivers/gpu/drm/i915/i915_drv.h | 123 ---
drivers/gpu/drm/i915/i915_gpu_error.c| 3 +
drivers/gpu/drm/i915/i915_pci.c | 2 +-
drivers/gpu/drm/i915/intel_device_info.c | 93 +
drivers/gpu/drm/i915/intel_device_info.h | 27 -
6 files changed, 214 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f1334f5d4ead..74734d7661e5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -868,6 +868,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);
@@ -1718,10 +1720,12 @@ 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
(subplatform=0x%x) gen=%i\n",
INTEL_DEVID(dev_priv),
INTEL_REVID(dev_priv),
intel_platform_name(INTEL_INFO(dev_priv)->platform),
+ intel_subplatform(RUNTIME_INFO(dev_priv),
+INTEL_INFO(dev_priv)->platform),
INTEL_GEN(dev_priv));
intel_device_info_dump_flags(INTEL_INFO(dev_priv), &p);
@@ -1764,8 +1768,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 9d3cab9406e1..b7d3f3a45ed9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2298,7 +2298,67 @@ 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))
+static __always_inline unsigned int
+__platform_mask_index(const struct intel_runtime_info *info,
+ enum intel_platform p)
+{
+ 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_SI
